From 77475bf23b8e08eedfdf1b34a5b6f0b760d3c24b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 17 Sep 2022 00:03:11 +0100 Subject: [PATCH 1/7] parser/gotest: Fix test error messages in report_builder_test.go --- parser/gotest/report_builder_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parser/gotest/report_builder_test.go b/parser/gotest/report_builder_test.go index c89855b..99adfca 100644 --- a/parser/gotest/report_builder_test.go +++ b/parser/gotest/report_builder_test.go @@ -121,7 +121,7 @@ func TestReport(t *testing.T) { } got := rb.Build() if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("FromEvents report incorrect, diff (-want, +got):\n%v", diff) + t.Errorf("Incorrect report created, diff (-want, +got):\n%v", diff) } } @@ -181,7 +181,7 @@ func TestBuildReportMultiplePackages(t *testing.T) { } got := rb.Build() if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("FromEvents report incorrect, diff (-want, +got):\n%v", diff) + t.Errorf("Incorrect report created, diff (-want, +got):\n%v", diff) } } From cb3436fc5f5313516740d5dfe1926ebe6fad0b06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 17 Sep 2022 00:06:11 +0100 Subject: [PATCH 2/7] parser/gotest: Do not ignore failures in a test summary The ReportBuilder ignores summary results if we never collected any events for that package. While under normal circumstances we wouldn't expect this to happen (unless some output was lost or due to a bug in go-junit-report), we should at the very least make sure that failed results are not ignored. Refs #145 --- parser/gotest/report_builder.go | 9 +++++++++ parser/gotest/report_builder_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index d53b323..bb4676c 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -164,7 +164,16 @@ func (b *reportBuilder) CreatePackage(packageName, newPackageName, result string delete(b.packageBuilders, packageName) pb.output.SetActiveID(0) + // If the packageBuilder is empty, we never received any events for this + // package so there's no need to continue. if pb.IsEmpty() { + // However, we should at least report an error if the result says we + // failed. + if parseResult(result) == gtr.Fail { + pkg.RunError = gtr.Error{ + Name: newPackageName, + } + } return pkg } diff --git a/parser/gotest/report_builder_test.go b/parser/gotest/report_builder_test.go index 99adfca..ac3ed53 100644 --- a/parser/gotest/report_builder_test.go +++ b/parser/gotest/report_builder_test.go @@ -349,3 +349,31 @@ func TestGroupBenchmarksByName(t *testing.T) { }) } } + +func TestReportFailedSummary(t *testing.T) { + events := []Event{ + {Type: "summary", Result: "FAIL", Name: "package/name", Duration: 1 * time.Millisecond}, + } + want := gtr.Report{ + Packages: []gtr.Package{ + { + Name: "package/name", + Duration: 1 * time.Millisecond, + Timestamp: testTimestamp, + RunError: gtr.Error{ + Name: "package/name", + }, + }, + }, + } + + rb := newReportBuilder() + rb.timestampFunc = testTimestampFunc + for _, ev := range events { + rb.ProcessEvent(ev) + } + got := rb.Build() + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Incorrect report created, diff (-want, +got):\n%v\n", diff) + } +} From 81e5aaaaf16d20efaa5babe706eab92e73bb1149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 17 Sep 2022 00:57:10 +0100 Subject: [PATCH 3/7] parser/gotest: Make sure every build error is processed Make sure we don't ignore any build error that did not belong to a package. This isn't expected to normally happen, but we need to handle it anyway to prevent accidentally ignoring potential errors. Refs #145 --- parser/gotest/report_builder.go | 5 ++ parser/gotest/report_builder_test.go | 71 ++++++++++++++++++++-------- 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index bb4676c..5d6b25b 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -120,6 +120,11 @@ func (b *reportBuilder) Build() gtr.Report { } b.packages = append(b.packages, b.CreatePackage(name, b.packageName, "", 0, "")) } + + // Create packages for any leftover build errors. + for _, buildErr := range b.buildErrors { + b.packages = append(b.packages, b.CreatePackage("", buildErr.Name, "", 0, "")) + } return gtr.Report{Packages: b.packages} } diff --git a/parser/gotest/report_builder_test.go b/parser/gotest/report_builder_test.go index ac3ed53..e4f2661 100644 --- a/parser/gotest/report_builder_test.go +++ b/parser/gotest/report_builder_test.go @@ -350,30 +350,61 @@ func TestGroupBenchmarksByName(t *testing.T) { } } -func TestReportFailedSummary(t *testing.T) { - events := []Event{ - {Type: "summary", Result: "FAIL", Name: "package/name", Duration: 1 * time.Millisecond}, - } - want := gtr.Report{ - Packages: []gtr.Package{ - { - Name: "package/name", - Duration: 1 * time.Millisecond, - Timestamp: testTimestamp, - RunError: gtr.Error{ - Name: "package/name", +func TestReportSpecialCases(t *testing.T) { + tests := []struct { + name string + events []Event + want gtr.Report + }{ + { + "failed-summary-only", + []Event{{Type: "summary", Result: "FAIL", Name: "package/name", Duration: 1 * time.Millisecond}}, + gtr.Report{ + Packages: []gtr.Package{ + { + Name: "package/name", + Duration: 1 * time.Millisecond, + Timestamp: testTimestamp, + RunError: gtr.Error{ + Name: "package/name", + }, + }, + }, + }, + }, + { + "leftover-builderror", + []Event{ + {Type: "build_output", Name: "package/name"}, + {Type: "output", Data: "error message"}, + }, + gtr.Report{ + Packages: []gtr.Package{ + { + Name: "package/name", + Timestamp: testTimestamp, + BuildError: gtr.Error{ + ID: 1, + Name: "package/name", + Output: []string{"error message"}, + }, + }, }, }, }, } - rb := newReportBuilder() - rb.timestampFunc = testTimestampFunc - for _, ev := range events { - rb.ProcessEvent(ev) - } - got := rb.Build() - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("Incorrect report created, diff (-want, +got):\n%v\n", diff) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + rb := newReportBuilder() + rb.timestampFunc = testTimestampFunc + for _, ev := range test.events { + rb.ProcessEvent(ev) + } + got := rb.Build() + if diff := cmp.Diff(test.want, got); diff != "" { + t.Errorf("Incorrect report created, diff (-want, +got):\n%v\n", diff) + } + }) } } From 97e0285183f03a62dee963073e102f0aa08f1a90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 17 Sep 2022 01:05:30 +0100 Subject: [PATCH 4/7] parser/gotest: Handle build errors in test packages with _test suffix It's possible for test files to declare a package with the "_test" suffix. If these packages contain build errors, they were not correctly matched to the package without the "_test" suffix. Refs #145 --- parser/gotest/report_builder.go | 2 +- parser/gotest/report_builder_test.go | 20 ++++++++++++++++++++ testdata/038-report.xml | 11 +++++++++++ testdata/038-test-pkg-name.txt | 4 ++++ 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 testdata/038-report.xml create mode 100644 testdata/038-test-pkg-name.txt diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index 5d6b25b..5369d8b 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -150,7 +150,7 @@ func (b *reportBuilder) CreatePackage(packageName, newPackageName, result string // First check if this package contained a build error. If that's the case, // we won't find any tests in this package. for id, buildErr := range b.buildErrors { - if buildErr.Name == newPackageName { + if buildErr.Name == newPackageName || strings.TrimSuffix(buildErr.Name, "_test") == newPackageName { pkg.BuildError = buildErr pkg.BuildError.ID = id pkg.BuildError.Duration = duration diff --git a/parser/gotest/report_builder_test.go b/parser/gotest/report_builder_test.go index e4f2661..bd24bf5 100644 --- a/parser/gotest/report_builder_test.go +++ b/parser/gotest/report_builder_test.go @@ -392,6 +392,26 @@ func TestReportSpecialCases(t *testing.T) { }, }, }, + { + "build error in package with _test suffix", + []Event{ + {Type: "build_output", Name: "package/name_test"}, + {Type: "summary", Name: "package/name", Result: "FAIL", Data: "[build failed]"}, + }, + gtr.Report{ + Packages: []gtr.Package{ + { + Name: "package/name", + Timestamp: testTimestamp, + BuildError: gtr.Error{ + ID: 1, + Name: "package/name_test", + Cause: "[build failed]", + }, + }, + }, + }, + }, } for _, test := range tests { diff --git a/testdata/038-report.xml b/testdata/038-report.xml new file mode 100644 index 0000000..edb773a --- /dev/null +++ b/testdata/038-report.xml @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/testdata/038-test-pkg-name.txt b/testdata/038-test-pkg-name.txt new file mode 100644 index 0000000..2763c5a --- /dev/null +++ b/testdata/038-test-pkg-name.txt @@ -0,0 +1,4 @@ +# package/testpkg/pkg_test [package/testpkg/pkg.test] +pkg/pkg_test.go:5:2: imported and not used: "fmt" +FAIL package/testpkg/pkg [build failed] +FAIL From 84a5190347b894713ef8a26764a4e869a63f01a3 Mon Sep 17 00:00:00 2001 From: Tomasz Janiszewski Date: Sat, 17 Sep 2022 23:26:44 +0200 Subject: [PATCH 5/7] Escape illegal XML characters in junit output (#140) --- junit/junit.go | 24 +++++++++++++++++++++++- junit/junit_test.go | 15 +++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/junit/junit.go b/junit/junit.go index be0313a..65117a2 100644 --- a/junit/junit.go +++ b/junit/junit.go @@ -236,5 +236,27 @@ func formatDuration(d time.Duration) string { // formatOutput combines the lines from the given output into a single string. func formatOutput(output []string) string { - return strings.Join(output, "\n") + return escapeIllegalChars(strings.Join(output, "\n")) +} + +func escapeIllegalChars(str string) string { + return strings.Map(func(r rune) rune { + if isInCharacterRange(r) { + return r + } + return '�' + }, str) +} + +// Decide whether the given rune is in the XML Character Range, per +// the Char production of https://www.xml.com/axml/testaxml.htm, +// Section 2.2 Characters. +// Form: encoding/xml/xml.go +func isInCharacterRange(r rune) (inrange bool) { + return r == 0x09 || + r == 0x0A || + r == 0x0D || + r >= 0x20 && r <= 0xD7FF || + r >= 0xE000 && r <= 0xFFFD || + r >= 0x10000 && r <= 0x10FFFF } diff --git a/junit/junit_test.go b/junit/junit_test.go index a19478c..4c51c76 100644 --- a/junit/junit_test.go +++ b/junit/junit_test.go @@ -26,6 +26,11 @@ func TestCreateFromReport(t *testing.T) { Result: gtr.Pass, Output: []string{"ok"}, }, + { + Name: "TestEscapeOutput", + Result: gtr.Pass, + Output: []string{"\x00\v\f \t\\"}, + }, { Name: "TestFail", Result: gtr.Fail, @@ -47,14 +52,14 @@ func TestCreateFromReport(t *testing.T) { } want := Testsuites{ - Tests: 6, + Tests: 7, Errors: 3, Failures: 1, Skipped: 1, Suites: []Testsuite{ { Name: "package/name", - Tests: 6, + Tests: 7, Errors: 3, ID: 0, Failures: 1, @@ -72,6 +77,12 @@ func TestCreateFromReport(t *testing.T) { Time: "0.000", SystemOut: &Output{Data: "ok"}, }, + { + Name: "TestEscapeOutput", + Classname: "package/name", + Time: "0.000", + SystemOut: &Output{Data: `��� \`}, + }, { Name: "TestFail", Classname: "package/name", From 2b3d79c5727c4b21527ffdc62296023a89931760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 17 Sep 2022 22:33:04 +0100 Subject: [PATCH 6/7] junit: Specify unicode replacement character as \uFFFD This is just to make the exact character we want to return explicit, otherwise it could be confused by any unsupported character in your text editor. --- junit/junit.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/junit/junit.go b/junit/junit.go index 65117a2..38c8cfd 100644 --- a/junit/junit.go +++ b/junit/junit.go @@ -244,14 +244,14 @@ func escapeIllegalChars(str string) string { if isInCharacterRange(r) { return r } - return '�' + return '\uFFFD' }, str) } // Decide whether the given rune is in the XML Character Range, per // the Char production of https://www.xml.com/axml/testaxml.htm, // Section 2.2 Characters. -// Form: encoding/xml/xml.go +// From: encoding/xml/xml.go func isInCharacterRange(r rune) (inrange bool) { return r == 0x09 || r == 0x0A || From 934b104ddd2eff7894e541a729cdee0c16a7a894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 17 Sep 2022 22:37:47 +0100 Subject: [PATCH 7/7] junit: Remove properties helper function in junit_test.go --- junit/junit_test.go | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/junit/junit_test.go b/junit/junit_test.go index 4c51c76..627428c 100644 --- a/junit/junit_test.go +++ b/junit/junit_test.go @@ -134,17 +134,19 @@ func TestMarshalUnmarshal(t *testing.T) { Disabled: 1, Suites: []Testsuite{ { - Name: "suite1", - Tests: 1, - Errors: 1, - Failures: 1, - Hostname: "localhost", - ID: 0, - Package: "package", - Skipped: 1, - Time: "12.345", - Timestamp: "2012-03-09T14:38:06+01:00", - Properties: properties("key", "value"), + Name: "suite1", + Tests: 1, + Errors: 1, + Failures: 1, + Hostname: "localhost", + ID: 0, + Package: "package", + Skipped: 1, + Time: "12.345", + Timestamp: "2012-03-09T14:38:06+01:00", + Properties: &[]Property{ + {Name: "key", Value: "value"}, + }, Testcases: []Testcase{ { Name: "test1", @@ -179,14 +181,3 @@ func TestMarshalUnmarshal(t *testing.T) { t.Errorf("Unmarshal result incorrect, diff (-want +got):\n%s\n", diff) } } - -func properties(keyvals ...string) *[]Property { - if len(keyvals)%2 != 0 { - panic("invalid keyvals specified") - } - var props []Property - for i := 0; i < len(keyvals); i += 2 { - props = append(props, Property{keyvals[i], keyvals[i+1]}) - } - return &props -}