From e79c39f004769fc55e60c2fb052155486295d533 Mon Sep 17 00:00:00 2001 From: Iskander Sharipov Date: Wed, 17 Nov 2021 17:46:22 +0300 Subject: [PATCH] encoding/xml: improve the test coverage, fix minor bugs MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 #49635 Fixes #49636 Change-Id: I50b2769896c197eb285672313b7148f4fe8bdb38 Reviewed-on: https://go-review.googlesource.com/c/go/+/364734 Trust: Bryan Mills Reviewed-by: Daniel Martí Trust: Daniel Martí Run-TryBot: Daniel Martí TryBot-Result: Gopher Robot --- src/encoding/xml/xml.go | 10 +-- src/encoding/xml/xml_test.go | 165 +++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 7 deletions(-) diff --git a/src/encoding/xml/xml.go b/src/encoding/xml/xml.go index 8a0a9c253a..ef51252dcb 100644 --- a/src/encoding/xml/xml.go +++ b/src/encoding/xml/xml.go @@ -10,9 +10,6 @@ package xml // Annotated XML spec: https://www.xml.com/axml/testaxml.htm // XML name spaces: https://www.w3.org/TR/REC-xml-names/ -// TODO(rsc): -// Test error handling. - import ( "bufio" "bytes" @@ -499,7 +496,7 @@ func (d *Decoder) popElement(t *EndElement) bool { return false case s.name.Space != name.Space: d.err = d.syntaxError("element <" + s.name.Local + "> in space " + s.name.Space + - "closed by in space " + name.Space) + " closed by in space " + name.Space) return false } @@ -523,12 +520,11 @@ func (d *Decoder) autoClose(t Token) (Token, bool) { if d.stk == nil || d.stk.kind != stkStart { return nil, false } - name := strings.ToLower(d.stk.name.Local) for _, s := range d.AutoClose { - if strings.ToLower(s) == name { + if strings.EqualFold(s, d.stk.name.Local) { // This one should be auto closed if t doesn't close it. et, ok := t.(EndElement) - if !ok || et.Name.Local != name { + if !ok || !strings.EqualFold(et.Name.Local, d.stk.name.Local) { return EndElement{d.stk.name}, true } break diff --git a/src/encoding/xml/xml_test.go b/src/encoding/xml/xml_test.go index 19152dbdb6..ab1dbf849b 100644 --- a/src/encoding/xml/xml_test.go +++ b/src/encoding/xml/xml_test.go @@ -673,6 +673,19 @@ func TestCopyTokenStartElement(t *testing.T) { } } +func TestCopyTokenComment(t *testing.T) { + data := []byte("") + var tok1 Token = Comment(data) + tok2 := CopyToken(tok1) + if !reflect.DeepEqual(tok1, tok2) { + t.Error("CopyToken(Comment) != Comment") + } + data[1] = 'o' + if reflect.DeepEqual(tok1, tok2) { + t.Error("CopyToken(Comment) uses same buffer.") + } +} + func TestSyntaxErrorLineNum(t *testing.T) { testInput := "

Foo

\n\n

Bar\n" d := NewDecoder(strings.NewReader(testInput)) @@ -1060,3 +1073,155 @@ func TestRoundTrip(t *testing.T) { t.Run(name, func(t *testing.T) { testRoundTrip(t, input) }) } } + +func TestParseErrors(t *testing.T) { + withDefaultHeader := func(s string) string { + return `` + s + } + tests := []struct { + src string + err string + }{ + {withDefaultHeader(``), `unexpected end element `}, + {withDefaultHeader(``), `element in space x closed by in space y`}, + {withDefaultHeader(``), `expected target name after `), `invalid sequence `), `invalid sequence `), `invalid `, `unsupported version "1.1"; only version 1.0 is supported`}, + + // Cases below are for "no errors". + {withDefaultHeader(``), ``}, + {withDefaultHeader(``), ``}, + } + + for _, test := range tests { + d := NewDecoder(strings.NewReader(test.src)) + var err error + for { + _, err = d.Token() + if err != nil { + break + } + } + if test.err == "" { + if err != io.EOF { + t.Errorf("parse %s: have %q error, expected none", test.src, err) + } + continue + } + if err == nil || err == io.EOF { + t.Errorf("parse %s: have no error, expected a non-nil error", test.src) + continue + } + if !strings.Contains(err.Error(), test.err) { + t.Errorf("parse %s: can't find %q error sudbstring\nerror: %q", test.src, test.err, err) + continue + } + } +} + +const testInputHTMLAutoClose = ` +
+

+

+

+
+

+

+
abc

` + +func BenchmarkHTMLAutoClose(b *testing.B) { + b.RunParallel(func(p *testing.PB) { + for p.Next() { + d := NewDecoder(strings.NewReader(testInputHTMLAutoClose)) + d.Strict = false + d.AutoClose = HTMLAutoClose + d.Entity = HTMLEntity + for { + _, err := d.Token() + if err != nil { + if err == io.EOF { + break + } + b.Fatalf("unexpected error: %v", err) + } + } + } + }) +} + +func TestHTMLAutoClose(t *testing.T) { + wantTokens := []Token{ + ProcInst{"xml", []byte(`version="1.0" encoding="UTF-8"`)}, + CharData("\n"), + StartElement{Name{"", "br"}, []Attr{}}, + EndElement{Name{"", "br"}}, + CharData("\n"), + StartElement{Name{"", "br"}, []Attr{}}, + EndElement{Name{"", "br"}}, + StartElement{Name{"", "br"}, []Attr{}}, + EndElement{Name{"", "br"}}, + CharData("\n"), + StartElement{Name{"", "br"}, []Attr{}}, + EndElement{Name{"", "br"}}, + StartElement{Name{"", "br"}, []Attr{}}, + EndElement{Name{"", "br"}}, + CharData("\n"), + StartElement{Name{"", "br"}, []Attr{}}, + EndElement{Name{"", "br"}}, + CharData("\n"), + StartElement{Name{"", "BR"}, []Attr{}}, + EndElement{Name{"", "BR"}}, + CharData("\n"), + StartElement{Name{"", "BR"}, []Attr{}}, + EndElement{Name{"", "BR"}}, + StartElement{Name{"", "BR"}, []Attr{}}, + EndElement{Name{"", "BR"}}, + CharData("\n"), + StartElement{Name{"", "Br"}, []Attr{}}, + EndElement{Name{"", "Br"}}, + CharData("\n"), + StartElement{Name{"", "BR"}, []Attr{}}, + EndElement{Name{"", "BR"}}, + StartElement{Name{"", "span"}, []Attr{{Name: Name{"", "id"}, Value: "test"}}}, + CharData("abc"), + EndElement{Name{"", "span"}}, + StartElement{Name{"", "br"}, []Attr{}}, + EndElement{Name{"", "br"}}, + StartElement{Name{"", "br"}, []Attr{}}, + EndElement{Name{"", "br"}}, + } + + d := NewDecoder(strings.NewReader(testInputHTMLAutoClose)) + d.Strict = false + d.AutoClose = HTMLAutoClose + d.Entity = HTMLEntity + var haveTokens []Token + for { + tok, err := d.Token() + if err != nil { + if err == io.EOF { + break + } + t.Fatalf("unexpected error: %v", err) + } + haveTokens = append(haveTokens, CopyToken(tok)) + } + if len(haveTokens) != len(wantTokens) { + t.Errorf("tokens count mismatch: have %d, want %d", len(haveTokens), len(wantTokens)) + } + for i, want := range wantTokens { + if i >= len(haveTokens) { + t.Errorf("token[%d] expected %#v, have no token", i, want) + } else { + have := haveTokens[i] + if !reflect.DeepEqual(have, want) { + t.Errorf("token[%d] mismatch:\nhave: %#v\nwant: %#v", i, have, want) + } + } + } +} -- 2.50.0