Skip to content

Commit e79c39f

Browse files
quasilytemvdan
quasilyte
authored and
mvdan
committed
encoding/xml: improve the test coverage, fix minor bugs
Improve the test coverage of encoding/xml package by adding the test cases for the execution paths that were not covered before. Since it reveals a couple of issues, fix them as well while we're at it. As I used an `strings.EqualFold` instead of adding one more `strings.ToLower`, our fix to `autoClose()` tends to run faster as well as a result. name old time/op new time/op delta HTMLAutoClose-8 5.93µs ± 2% 5.75µs ± 3% -3.16% (p=0.000 n=10+10) name old alloc/op new alloc/op delta HTMLAutoClose-8 2.60kB ± 0% 2.58kB ± 0% -0.46% (p=0.000 n=10+10) name old allocs/op new allocs/op delta HTMLAutoClose-8 72.0 ± 0% 67.0 ± 0% -6.94% (p=0.000 n=10+10) The overall `encoding/xml` test coverage increase is `88.1% -> 89.9%`; although it may look insignificant, this CL covers some important corner cases, like `autoClose()` functionality (that was not tested at all). Fixes golang#49635 Fixes golang#49636 Change-Id: I50b2769896c197eb285672313b7148f4fe8bdb38 Reviewed-on: https://go-review.googlesource.com/c/go/+/364734 Trust: Bryan Mills <[email protected]> Reviewed-by: Daniel Martí <[email protected]> Trust: Daniel Martí <[email protected]> Run-TryBot: Daniel Martí <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 2b8aa2b commit e79c39f

2 files changed

Lines changed: 168 additions & 7 deletions

File tree

‎src/encoding/xml/xml.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ package xml
1010
// Annotated XML spec: https://www.xml.com/axml/testaxml.htm
1111
// XML name spaces: https://www.w3.org/TR/REC-xml-names/
1212

13-
// TODO(rsc):
14-
// Test error handling.
15-
1613
import (
1714
"bufio"
1815
"bytes"
@@ -499,7 +496,7 @@ func (d *Decoder) popElement(t *EndElement) bool {
499496
return false
500497
case s.name.Space != name.Space:
501498
d.err = d.syntaxError("element <" + s.name.Local + "> in space " + s.name.Space +
502-
"closed by </" + name.Local + "> in space " + name.Space)
499+
" closed by </" + name.Local + "> in space " + name.Space)
503500
return false
504501
}
505502

@@ -523,12 +520,11 @@ func (d *Decoder) autoClose(t Token) (Token, bool) {
523520
if d.stk == nil || d.stk.kind != stkStart {
524521
return nil, false
525522
}
526-
name := strings.ToLower(d.stk.name.Local)
527523
for _, s := range d.AutoClose {
528-
if strings.ToLower(s) == name {
524+
if strings.EqualFold(s, d.stk.name.Local) {
529525
// This one should be auto closed if t doesn't close it.
530526
et, ok := t.(EndElement)
531-
if !ok || et.Name.Local != name {
527+
if !ok || !strings.EqualFold(et.Name.Local, d.stk.name.Local) {
532528
return EndElement{d.stk.name}, true
533529
}
534530
break

‎src/encoding/xml/xml_test.go‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,19 @@ func TestCopyTokenStartElement(t *testing.T) {
673673
}
674674
}
675675

676+
func TestCopyTokenComment(t *testing.T) {
677+
data := []byte("<!-- some comment -->")
678+
var tok1 Token = Comment(data)
679+
tok2 := CopyToken(tok1)
680+
if !reflect.DeepEqual(tok1, tok2) {
681+
t.Error("CopyToken(Comment) != Comment")
682+
}
683+
data[1] = 'o'
684+
if reflect.DeepEqual(tok1, tok2) {
685+
t.Error("CopyToken(Comment) uses same buffer.")
686+
}
687+
}
688+
676689
func TestSyntaxErrorLineNum(t *testing.T) {
677690
testInput := "<P>Foo<P>\n\n<P>Bar</>\n"
678691
d := NewDecoder(strings.NewReader(testInput))
@@ -1060,3 +1073,155 @@ func TestRoundTrip(t *testing.T) {
10601073
t.Run(name, func(t *testing.T) { testRoundTrip(t, input) })
10611074
}
10621075
}
1076+
1077+
func TestParseErrors(t *testing.T) {
1078+
withDefaultHeader := func(s string) string {
1079+
return `<?xml version="1.0" encoding="UTF-8"?>` + s
1080+
}
1081+
tests := []struct {
1082+
src string
1083+
err string
1084+
}{
1085+
{withDefaultHeader(`</foo>`), `unexpected end element </foo>`},
1086+
{withDefaultHeader(`<x:foo></y:foo>`), `element <foo> in space x closed by </foo> in space y`},
1087+
{withDefaultHeader(`<? not ok ?>`), `expected target name after <?`},
1088+
{withDefaultHeader(`<!- not ok -->`), `invalid sequence <!- not part of <!--`},
1089+
{withDefaultHeader(`<!-? not ok -->`), `invalid sequence <!- not part of <!--`},
1090+
{withDefaultHeader(`<![not ok]>`), `invalid <![ sequence`},
1091+
{withDefaultHeader("\xf1"), `invalid UTF-8`},
1092+
1093+
// Header-related errors.
1094+
{`<?xml version="1.1" encoding="UTF-8"?>`, `unsupported version "1.1"; only version 1.0 is supported`},
1095+
1096+
// Cases below are for "no errors".
1097+
{withDefaultHeader(`<?ok?>`), ``},
1098+
{withDefaultHeader(`<?ok version="ok"?>`), ``},
1099+
}
1100+
1101+
for _, test := range tests {
1102+
d := NewDecoder(strings.NewReader(test.src))
1103+
var err error
1104+
for {
1105+
_, err = d.Token()
1106+
if err != nil {
1107+
break
1108+
}
1109+
}
1110+
if test.err == "" {
1111+
if err != io.EOF {
1112+
t.Errorf("parse %s: have %q error, expected none", test.src, err)
1113+
}
1114+
continue
1115+
}
1116+
if err == nil || err == io.EOF {
1117+
t.Errorf("parse %s: have no error, expected a non-nil error", test.src)
1118+
continue
1119+
}
1120+
if !strings.Contains(err.Error(), test.err) {
1121+
t.Errorf("parse %s: can't find %q error sudbstring\nerror: %q", test.src, test.err, err)
1122+
continue
1123+
}
1124+
}
1125+
}
1126+
1127+
const testInputHTMLAutoClose = `<?xml version="1.0" encoding="UTF-8"?>
1128+
<br>
1129+
<br/><br/>
1130+
<br><br>
1131+
<br></br>
1132+
<BR>
1133+
<BR/><BR/>
1134+
<Br></Br>
1135+
<BR><span id="test">abc</span><br/><br/>`
1136+
1137+
func BenchmarkHTMLAutoClose(b *testing.B) {
1138+
b.RunParallel(func(p *testing.PB) {
1139+
for p.Next() {
1140+
d := NewDecoder(strings.NewReader(testInputHTMLAutoClose))
1141+
d.Strict = false
1142+
d.AutoClose = HTMLAutoClose
1143+
d.Entity = HTMLEntity
1144+
for {
1145+
_, err := d.Token()
1146+
if err != nil {
1147+
if err == io.EOF {
1148+
break
1149+
}
1150+
b.Fatalf("unexpected error: %v", err)
1151+
}
1152+
}
1153+
}
1154+
})
1155+
}
1156+
1157+
func TestHTMLAutoClose(t *testing.T) {
1158+
wantTokens := []Token{
1159+
ProcInst{"xml", []byte(`version="1.0" encoding="UTF-8"`)},
1160+
CharData("\n"),
1161+
StartElement{Name{"", "br"}, []Attr{}},
1162+
EndElement{Name{"", "br"}},
1163+
CharData("\n"),
1164+
StartElement{Name{"", "br"}, []Attr{}},
1165+
EndElement{Name{"", "br"}},
1166+
StartElement{Name{"", "br"}, []Attr{}},
1167+
EndElement{Name{"", "br"}},
1168+
CharData("\n"),
1169+
StartElement{Name{"", "br"}, []Attr{}},
1170+
EndElement{Name{"", "br"}},
1171+
StartElement{Name{"", "br"}, []Attr{}},
1172+
EndElement{Name{"", "br"}},
1173+
CharData("\n"),
1174+
StartElement{Name{"", "br"}, []Attr{}},
1175+
EndElement{Name{"", "br"}},
1176+
CharData("\n"),
1177+
StartElement{Name{"", "BR"}, []Attr{}},
1178+
EndElement{Name{"", "BR"}},
1179+
CharData("\n"),
1180+
StartElement{Name{"", "BR"}, []Attr{}},
1181+
EndElement{Name{"", "BR"}},
1182+
StartElement{Name{"", "BR"}, []Attr{}},
1183+
EndElement{Name{"", "BR"}},
1184+
CharData("\n"),
1185+
StartElement{Name{"", "Br"}, []Attr{}},
1186+
EndElement{Name{"", "Br"}},
1187+
CharData("\n"),
1188+
StartElement{Name{"", "BR"}, []Attr{}},
1189+
EndElement{Name{"", "BR"}},
1190+
StartElement{Name{"", "span"}, []Attr{{Name: Name{"", "id"}, Value: "test"}}},
1191+
CharData("abc"),
1192+
EndElement{Name{"", "span"}},
1193+
StartElement{Name{"", "br"}, []Attr{}},
1194+
EndElement{Name{"", "br"}},
1195+
StartElement{Name{"", "br"}, []Attr{}},
1196+
EndElement{Name{"", "br"}},
1197+
}
1198+
1199+
d := NewDecoder(strings.NewReader(testInputHTMLAutoClose))
1200+
d.Strict = false
1201+
d.AutoClose = HTMLAutoClose
1202+
d.Entity = HTMLEntity
1203+
var haveTokens []Token
1204+
for {
1205+
tok, err := d.Token()
1206+
if err != nil {
1207+
if err == io.EOF {
1208+
break
1209+
}
1210+
t.Fatalf("unexpected error: %v", err)
1211+
}
1212+
haveTokens = append(haveTokens, CopyToken(tok))
1213+
}
1214+
if len(haveTokens) != len(wantTokens) {
1215+
t.Errorf("tokens count mismatch: have %d, want %d", len(haveTokens), len(wantTokens))
1216+
}
1217+
for i, want := range wantTokens {
1218+
if i >= len(haveTokens) {
1219+
t.Errorf("token[%d] expected %#v, have no token", i, want)
1220+
} else {
1221+
have := haveTokens[i]
1222+
if !reflect.DeepEqual(have, want) {
1223+
t.Errorf("token[%d] mismatch:\nhave: %#v\nwant: %#v", i, have, want)
1224+
}
1225+
}
1226+
}
1227+
}

0 commit comments

Comments
 (0)