From cb055227b72565f4bfa00c8aa89f3a659346a433 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Wed, 8 Jun 2022 22:51:54 +0100 Subject: [PATCH] parser/gotest: Improve gotest output handling The reportBuilder has been updated to use the ordered output collector to keep track of go test output. This makes it possible to include benchmark output in the generated report and makes sure that output is preserved when deleting subtest parents from the report. --- parser/gotest/gotest_test.go | 9 +++- parser/gotest/report_builder.go | 66 +++++++++++++--------------- parser/gotest/report_builder_test.go | 3 +- testdata/022-report.xml | 9 +++- testdata/109-report.xml | 9 +++- 5 files changed, 56 insertions(+), 40 deletions(-) diff --git a/parser/gotest/gotest_test.go b/parser/gotest/gotest_test.go index faa07d8..763a407 100644 --- a/parser/gotest/gotest_test.go +++ b/parser/gotest/gotest_test.go @@ -325,14 +325,17 @@ func TestReport(t *testing.T) { func TestSubtestModes(t *testing.T) { events := []Event{ {Type: "run_test", Name: "TestParent"}, - {Type: "output", Data: "TestParent output"}, + {Type: "output", Data: "TestParent before"}, {Type: "run_test", Name: "TestParent/Subtest#1"}, {Type: "output", Data: "Subtest#1 output"}, {Type: "run_test", Name: "TestParent/Subtest#2"}, {Type: "output", Data: "Subtest#2 output"}, + {Type: "cont_test", Name: "TestParent"}, + {Type: "output", Data: "TestParent after"}, {Type: "end_test", Name: "TestParent", Result: "PASS", Duration: 1 * time.Millisecond}, {Type: "end_test", Name: "TestParent/Subtest#1", Result: "FAIL", Duration: 2 * time.Millisecond}, {Type: "end_test", Name: "TestParent/Subtest#2", Result: "PASS", Duration: 3 * time.Millisecond}, + {Type: "output", Data: "output"}, {Type: "summary", Result: "FAIL", Name: "package/name", Duration: 1 * time.Millisecond}, } @@ -356,7 +359,7 @@ func TestSubtestModes(t *testing.T) { Name: "TestParent", Duration: 1 * time.Millisecond, Result: gtr.Pass, - Output: []string{"TestParent output"}, + Output: []string{"TestParent before", "TestParent after"}, }, { ID: 2, @@ -373,6 +376,7 @@ func TestSubtestModes(t *testing.T) { Output: []string{"Subtest#2 output"}, }, }, + Output: []string{"output"}, }, }, }, @@ -402,6 +406,7 @@ func TestSubtestModes(t *testing.T) { Output: []string{"Subtest#2 output"}, }, }, + Output: []string{"TestParent before", "TestParent after", "output"}, }, }, }, diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index ede083f..c33af98 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -5,6 +5,11 @@ import ( "time" "github.com/jstemmer/go-junit-report/v2/gtr" + "github.com/jstemmer/go-junit-report/v2/parser/gotest/internal/collector" +) + +const ( + globalID = 0 ) // reportBuilder helps build a test Report from a collection of events. @@ -23,11 +28,11 @@ type reportBuilder struct { runErrors map[int]gtr.Error // state - nextID int // next free unused id - lastID int // most recently created id - output []string // output that does not belong to any test - coverage float64 // coverage percentage - parentIDs map[int]struct{} // set of test id's that contain subtests + nextID int // next free unused id + lastID int // most recently created id + output *collector.Output // output collected for each id + coverage float64 // coverage percentage + parentIDs map[int]struct{} // set of test id's that contain subtests // options packageName string @@ -43,6 +48,7 @@ func newReportBuilder() *reportBuilder { buildErrors: make(map[int]gtr.Error), runErrors: make(map[int]gtr.Error), nextID: 1, + output: collector.New(), parentIDs: make(map[int]struct{}), timestampFunc: time.Now, } @@ -202,6 +208,8 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio buildErr.ID = id buildErr.Duration = duration buildErr.Cause = data + buildErr.Output = b.output.Get(id) + pkg.BuildError = buildErr b.packages = append(b.packages, pkg) @@ -214,17 +222,15 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio // If we've collected output, but there were no tests or benchmarks then // either there were no tests, or there was some other non-build error. - if len(b.output) > 0 && len(b.tests) == 0 && len(b.benchmarks) == 0 { + if b.output.Contains(globalID) && len(b.tests) == 0 && len(b.benchmarks) == 0 { if parseResult(result) == gtr.Fail { pkg.RunError = gtr.Error{ Name: name, - Output: b.output, + Output: b.output.Get(globalID), } } b.packages = append(b.packages, pkg) - - // TODO: reset state - b.output = nil + b.output.Clear(globalID) return } @@ -233,9 +239,9 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio if parseResult(result) == gtr.Fail && (len(b.tests) > 0 || len(b.benchmarks) > 0) && !b.containsFailingTest() { pkg.RunError = gtr.Error{ Name: name, - Output: b.output, + Output: b.output.Get(globalID), } - b.output = nil + b.output.Clear(globalID) } // Collect tests and benchmarks for this package, maintaining insertion order. @@ -247,28 +253,31 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio if b.subtestMode == IgnoreParentResults { t.Result = gtr.Pass } else if b.subtestMode == ExcludeParents { + b.output.Merge(id, globalID) continue } } + t.Output = b.output.Get(id) tests = append(tests, t) continue } if bm, ok := b.benchmarks[id]; ok { + bm.Output = b.output.Get(id) benchmarks = append(benchmarks, bm) continue } } pkg.Coverage = b.coverage - pkg.Output = b.output + pkg.Output = b.output.Get(globalID) pkg.Tests = tests - pkg.Benchmarks = groupBenchmarksByName(benchmarks) + pkg.Benchmarks = b.groupBenchmarksByName(benchmarks) b.packages = append(b.packages, pkg) // reset state, except for nextID to ensure all id's are unique. b.lastID = 0 - b.output = nil + b.output.Clear(globalID) b.coverage = 0 b.tests = make(map[int]gtr.Test) b.benchmarks = make(map[int]gtr.Benchmark) @@ -280,26 +289,10 @@ func (b *reportBuilder) Coverage(pct float64, packages []string) { b.coverage = pct } -// AppendOutput appends the given line to the currently active context. If no +// AppendOutput appends the given text to the currently active context. If no // active context exists, the output is assumed to belong to the package. -func (b *reportBuilder) AppendOutput(line string) { - if b.lastID <= 0 { - b.output = append(b.output, line) - return - } - - if t, ok := b.tests[b.lastID]; ok { - t.Output = append(t.Output, line) - b.tests[b.lastID] = t - } else if bm, ok := b.benchmarks[b.lastID]; ok { - bm.Output = append(bm.Output, line) - b.benchmarks[b.lastID] = bm - } else if be, ok := b.buildErrors[b.lastID]; ok { - be.Output = append(be.Output, line) - b.buildErrors[b.lastID] = be - } else { - b.output = append(b.output, line) - } +func (b *reportBuilder) AppendOutput(text string) { + b.output.Append(b.lastID, text) } // findTest returns the id of the most recently created test with the given @@ -382,7 +375,7 @@ func parseResult(r string) gtr.Result { } } -func groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark { +func (b *reportBuilder) groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark { if len(benchmarks) == 0 { return nil } @@ -397,8 +390,10 @@ func groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark { } for i, group := range grouped { + var ids []int count := 0 for _, bm := range byName[group.Name] { + ids = append(ids, bm.ID) if bm.Result != gtr.Pass { continue } @@ -411,6 +406,7 @@ func groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark { } group.Result = groupResults(byName[group.Name]) + group.Output = b.output.GetAll(ids...) if count > 0 { group.NsPerOp /= float64(count) group.MBPerSec /= float64(count) diff --git a/parser/gotest/report_builder_test.go b/parser/gotest/report_builder_test.go index 33cb712..89ede6f 100644 --- a/parser/gotest/report_builder_test.go +++ b/parser/gotest/report_builder_test.go @@ -43,7 +43,8 @@ func TestGroupBenchmarksByName(t *testing.T) { } for _, test := range tests { - got := groupBenchmarksByName(test.in) + b := newReportBuilder() + got := b.groupBenchmarksByName(test.in) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("groupBenchmarksByName result incorrect, diff (-want, +got):\n%s\n", diff) } diff --git a/testdata/022-report.xml b/testdata/022-report.xml index a3ae751..d5d3699 100644 --- a/testdata/022-report.xml +++ b/testdata/022-report.xml @@ -7,7 +7,14 @@ - + + + - + + +