From 9d434fa4b493cf9b7b46690c5b50c0c934522a6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 13 Aug 2022 17:03:27 +0100 Subject: [PATCH 01/13] parser/gotest: Implement active id tracking in output collector --- parser/gotest/internal/collector/collector.go | 28 ++++++++--- .../internal/collector/collector_test.go | 48 ++++++++++++++----- 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/parser/gotest/internal/collector/collector.go b/parser/gotest/internal/collector/collector.go index 90ba052..92d5e1f 100644 --- a/parser/gotest/internal/collector/collector.go +++ b/parser/gotest/internal/collector/collector.go @@ -14,10 +14,14 @@ type line struct { } // Output stores output lines grouped by id. Output can be retrieved for one or -// more ids and output of different ids can be merged together, all while -// preserving their original order based on the time it was collected. +// more ids and output for different ids can be merged together, while +// preserving their insertion original order based on the time it was +// collected. +// Output also tracks the active id, so you can append output without providing +// an id. type Output struct { - m map[int][]line + m map[int][]line + id int // active id } // New returns a new output collector. @@ -27,11 +31,17 @@ func New() *Output { // Clear deletes all output for the given id. func (o *Output) Clear(id int) { - o.m[id] = nil + delete(o.m, id) } -// Append appends the given line of text to the output of the specified id. -func (o *Output) Append(id int, text string) { +// Append appends the given line of text to the output of the currently active +// id. +func (o *Output) Append(text string) { + o.m[o.id] = append(o.m[o.id], line{time.Now(), text}) +} + +// AppendToID appends the given line of text to the output of the given id. +func (o *Output) AppendToID(id int, text string) { o.m[id] = append(o.m[id], line{time.Now(), text}) } @@ -79,3 +89,9 @@ func (o *Output) Merge(fromID, intoID int) { o.m[intoID] = merged delete(o.m, fromID) } + +// SetActiveID sets the active id. Text appended to this output will be +// associated with the active id. +func (o *Output) SetActiveID(id int) { + o.id = id +} diff --git a/parser/gotest/internal/collector/collector_test.go b/parser/gotest/internal/collector/collector_test.go index 1479ba4..bfad6f9 100644 --- a/parser/gotest/internal/collector/collector_test.go +++ b/parser/gotest/internal/collector/collector_test.go @@ -9,8 +9,8 @@ import ( func TestClear(t *testing.T) { o := New() - o.Append(1, "1") - o.Append(2, "2") + o.AppendToID(1, "1") + o.AppendToID(2, "2") o.Clear(1) want := []string(nil) @@ -28,22 +28,22 @@ func TestClear(t *testing.T) { func TestAppendAndGet(t *testing.T) { o := New() - o.Append(1, "1.1") - o.Append(1, "1.2") - o.Append(2, "2") - o.Append(1, "1.3") + o.AppendToID(1, "1.1") + o.AppendToID(1, "1.2") + o.AppendToID(2, "2") + o.AppendToID(1, "1.3") want := []string{"1.1", "1.2", "1.3"} got := o.Get(1) if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("Append() incorrect (-want +got):\n%s", diff) + t.Errorf("AppendToID() incorrect (-want +got):\n%s", diff) } } func TestContains(t *testing.T) { o := New() - o.Append(1, "1") - o.Append(2, "2") + o.AppendToID(1, "1") + o.AppendToID(2, "2") o.Clear(1) if !o.Contains(2) { @@ -59,7 +59,7 @@ func TestContains(t *testing.T) { func TestGetAll(t *testing.T) { o := New() for i := 1; i <= 10; i++ { - o.Append(i%3, strconv.Itoa(i)) + o.AppendToID(i%3, strconv.Itoa(i)) } want := []string{"1", "2", "4", "5", "7", "8", "10"} @@ -72,7 +72,7 @@ func TestGetAll(t *testing.T) { func TestMerge(t *testing.T) { o := New() for i := 1; i <= 10; i++ { - o.Append(i%3, strconv.Itoa(i)) + o.AppendToID(i%3, strconv.Itoa(i)) } o.Merge(2, 1) @@ -89,3 +89,29 @@ func TestMerge(t *testing.T) { t.Errorf("Get(2) after Merge(2, 1) incorrect (-want +got):\n%s", diff) } } + +func TestActiveID(t *testing.T) { + o := New() + + o.Append("0") + o.SetActiveID(2) + o.Append("2") + o.SetActiveID(1) + o.Append("1") + o.SetActiveID(0) + o.Append("0") + + expected := [][]string{ + {"0", "0"}, + {"1"}, + {"2"}, + } + for i := 0; i < 2; i++ { + want := expected[i] + got := o.Get(i) + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Get(0) after SetActiveID incorrect (-want +got):\n%s", diff) + } + } + +} From 50c1179050b31dbc05fbb6c8fef40a3da83726a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 13 Aug 2022 17:06:14 +0100 Subject: [PATCH 02/13] parser/gotest: Remove benchmark specific methods from reportBuilder --- parser/gotest/report_builder.go | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index e005579..d979c21 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -63,11 +63,11 @@ func (b *reportBuilder) ProcessEvent(ev Event) { case "end_test": b.EndTest(ev.Name, ev.Result, ev.Duration, ev.Indent) case "run_benchmark": - b.CreateBenchmark(ev.Name) + b.CreateTest(ev.Name) case "benchmark": b.BenchmarkResult(ev.Name, ev.Iterations, ev.NsPerOp, ev.MBPerSec, ev.BytesPerOp, ev.AllocsPerOp) case "end_benchmark": - b.EndBenchmark(ev.Name, ev.Result) + b.EndTest(ev.Name, ev.Result, 0, 0) case "status": b.End() case "summary": @@ -157,14 +157,6 @@ func (b *reportBuilder) End() { b.lastID = 0 } -// CreateBenchmark adds a benchmark with the given name to the report, and -// marks it as active. If more than one benchmark exists with this name, the -// most recently created benchmark will be updated. If no benchmark exists with -// this name, a new benchmark is created. -func (b *reportBuilder) CreateBenchmark(name string) { - b.CreateTest(name) -} - // BenchmarkResult updates an existing or adds a new test with the given // results and marks it as active. If an existing test with this name exists // but without result, then that one is updated. Otherwise a new one is added @@ -184,14 +176,6 @@ func (b *reportBuilder) BenchmarkResult(name string, iterations int64, nsPerOp, b.tests[id] = test } -// EndBenchmark finds the benchmark with the given name and sets the result. If -// more than one benchmark exists with this name, the most recently created -// benchmark will be used. If no benchmark exists with this name, a new -// benchmark is created. -func (b *reportBuilder) EndBenchmark(name, result string) { - b.EndTest(name, result, 0, 0) -} - // CreateBuildError creates a new build error and marks it as active. func (b *reportBuilder) CreateBuildError(packageName string) { id := b.newID() From 83ca558534f7df07134c62c369119f44222ca865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 13 Aug 2022 17:08:43 +0100 Subject: [PATCH 03/13] parser/gotest: Return created id from CreateTest method --- parser/gotest/report_builder.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index d979c21..a2d6721 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -108,12 +108,13 @@ func (b *reportBuilder) Build() gtr.Report { // CreateTest adds a test with the given name to the report, and marks it as // active. -func (b *reportBuilder) CreateTest(name string) { +func (b *reportBuilder) CreateTest(name string) int { if parentID, ok := b.findTestParentID(name); ok { b.parentIDs[parentID] = struct{}{} } id := b.newID() b.tests[id] = gtr.NewTest(id, name) + return id } // PauseTest marks the active context as no longer active. Any results or @@ -140,8 +141,7 @@ func (b *reportBuilder) EndTest(name, result string, duration time.Duration, lev // test did not exist, create one // TODO: Likely reason is that the user ran go test without the -v // flag, should we report this somewhere? - b.CreateTest(name) - id = b.lastID + id = b.CreateTest(name) } t := b.tests[id] @@ -164,8 +164,7 @@ func (b *reportBuilder) End() { func (b *reportBuilder) BenchmarkResult(name string, iterations int64, nsPerOp, mbPerSec float64, bytesPerOp, allocsPerOp int64) { id, ok := b.findTest(name) if !ok || b.tests[id].Result != gtr.Unknown { - b.CreateTest(name) - id = b.lastID + id = b.CreateTest(name) } benchmark := Benchmark{iterations, nsPerOp, mbPerSec, bytesPerOp, allocsPerOp} From 1c826cb28df13f6c620abd62f8d38bb9199f65e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 13 Aug 2022 17:10:15 +0100 Subject: [PATCH 04/13] parser/gotest: Switch to output collector for active id tracking --- parser/gotest/report_builder.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index a2d6721..f69164b 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -27,7 +27,6 @@ type reportBuilder struct { // state 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 @@ -86,7 +85,6 @@ func (b *reportBuilder) ProcessEvent(ev Event) { // newID returns a new unique id and sets the active context this id. func (b *reportBuilder) newID() int { id := b.nextID - b.lastID = id b.nextID++ return id } @@ -113,6 +111,7 @@ func (b *reportBuilder) CreateTest(name string) int { b.parentIDs[parentID] = struct{}{} } id := b.newID() + b.output.SetActiveID(id) b.tests[id] = gtr.NewTest(id, name) return id } @@ -121,14 +120,15 @@ func (b *reportBuilder) CreateTest(name string) int { // output added to the report after calling PauseTest will no longer be assumed // to belong to this test. func (b *reportBuilder) PauseTest(name string) { - b.lastID = 0 + b.output.SetActiveID(0) } // ContinueTest finds the test with the given name and marks it as active. If // more than one test exist with this name, the most recently created test will // be used. func (b *reportBuilder) ContinueTest(name string) { - b.lastID, _ = b.findTest(name) + id, _ := b.findTest(name) + b.output.SetActiveID(id) } // EndTest finds the test with the given name, sets the result, duration and @@ -149,12 +149,12 @@ func (b *reportBuilder) EndTest(name, result string, duration time.Duration, lev t.Duration = duration t.Level = level b.tests[id] = t - b.lastID = 0 + b.output.SetActiveID(0) } // End marks the active context as no longer active. func (b *reportBuilder) End() { - b.lastID = 0 + b.output.SetActiveID(0) } // BenchmarkResult updates an existing or adds a new test with the given @@ -178,6 +178,7 @@ func (b *reportBuilder) BenchmarkResult(name string, iterations int64, nsPerOp, // CreateBuildError creates a new build error and marks it as active. func (b *reportBuilder) CreateBuildError(packageName string) { id := b.newID() + b.output.SetActiveID(id) b.buildErrors[id] = gtr.Error{ID: id, Name: packageName} } @@ -268,7 +269,7 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio b.packages = append(b.packages, pkg) // reset state, except for nextID to ensure all id's are unique. - b.lastID = 0 + b.output.SetActiveID(0) b.output.Clear(globalID) b.coverage = 0 b.tests = make(map[int]gtr.Test) @@ -283,16 +284,12 @@ func (b *reportBuilder) Coverage(pct float64, packages []string) { // 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(text string) { - b.output.Append(b.lastID, text) + b.output.Append(text) } // findTest returns the id of the most recently created test with the given // name if it exists. func (b *reportBuilder) findTest(name string) (int, bool) { - // check if this test was lastID - if t, ok := b.tests[b.lastID]; ok && t.Name == name { - return b.lastID, true - } for i := b.nextID; i > 0; i-- { if test, ok := b.tests[i]; ok && test.Name == name { return i, true From 4ce910564c2c604fc0f2a9ae9e70d817e4fc4646 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 13 Aug 2022 21:59:50 +0100 Subject: [PATCH 05/13] parser/gotest: timestampFunc should always be set, skip nil check --- parser/gotest/report_builder.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index f69164b..388d122 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -187,12 +187,9 @@ func (b *reportBuilder) CreateBuildError(packageName string) { // so far. Afterwards all state is reset. func (b *reportBuilder) CreatePackage(name, result string, duration time.Duration, data string) { pkg := gtr.Package{ - Name: name, - Duration: duration, - } - - if b.timestampFunc != nil { - pkg.Timestamp = b.timestampFunc() + Name: name, + Duration: duration, + Timestamp: b.timestampFunc(), } // Build errors are treated somewhat differently. Rather than having a From b73e4a9ed5a3153220dcaec257d9c6165d2354c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 13 Aug 2022 22:06:49 +0100 Subject: [PATCH 06/13] parser/gotest: Remove unnecessary if statement when creating a package We only execute this branch when we've captured some global output, so no need to check that again. --- parser/gotest/report_builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index 388d122..de8888c 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -223,7 +223,7 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio Name: name, Output: b.output.Get(globalID), } - } else if b.output.Contains(globalID) { + } else { pkg.Output = b.output.Get(globalID) } b.packages = append(b.packages, pkg) From 2af321a6973614e35434ed77f29ca2e5bc2a271f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Mon, 15 Aug 2022 22:05:38 +0100 Subject: [PATCH 07/13] parser/gotest: Refactor benchmark grouping The only reason groupBenchmarksByName needed the reportBuilder receiver was to access its output collector. By passing the output explicitly to the groupBenchmarksByName function we can remove the reportBuilder dependency. --- parser/gotest/report_builder.go | 6 +++--- parser/gotest/report_builder_test.go | 16 +++++++++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index de8888c..8a6df4c 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -258,7 +258,7 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio continue } } - tests = b.groupBenchmarksByName(tests) + tests = groupBenchmarksByName(tests, b.output) pkg.Coverage = b.coverage pkg.Output = b.output.Get(globalID) @@ -345,7 +345,7 @@ func parseResult(r string) gtr.Result { } } -func (b *reportBuilder) groupBenchmarksByName(tests []gtr.Test) []gtr.Test { +func groupBenchmarksByName(tests []gtr.Test, output *collector.Output) []gtr.Test { if len(tests) == 0 { return nil } @@ -392,7 +392,7 @@ func (b *reportBuilder) groupBenchmarksByName(tests []gtr.Test) []gtr.Test { group.Duration = combinedDuration(byName[group.Name]) group.Result = groupResults(byName[group.Name]) - group.Output = b.output.GetAll(ids...) + group.Output = output.GetAll(ids...) if count > 0 { total.Iterations /= int64(count) total.NsPerOp /= float64(count) diff --git a/parser/gotest/report_builder_test.go b/parser/gotest/report_builder_test.go index 4d88b2f..b73c64a 100644 --- a/parser/gotest/report_builder_test.go +++ b/parser/gotest/report_builder_test.go @@ -1,10 +1,12 @@ package gotest import ( + "fmt" "testing" "time" "github.com/jstemmer/go-junit-report/v2/gtr" + "github.com/jstemmer/go-junit-report/v2/parser/gotest/internal/collector" "github.com/google/go-cmp/cmp" ) @@ -236,6 +238,11 @@ func TestSubtestModes(t *testing.T) { } func TestGroupBenchmarksByName(t *testing.T) { + output := collector.New() + for i := 1; i <= 4; i++ { + output.AppendToID(i, fmt.Sprintf("output-%d", i)) + } + tests := []struct { name string in []gtr.Test @@ -245,7 +252,7 @@ func TestGroupBenchmarksByName(t *testing.T) { { "one failing benchmark", []gtr.Test{{ID: 1, Name: "BenchmarkFailed", Result: gtr.Fail, Data: map[string]interface{}{}}}, - []gtr.Test{{ID: 1, Name: "BenchmarkFailed", Result: gtr.Fail, Data: map[string]interface{}{}}}, + []gtr.Test{{ID: 1, Name: "BenchmarkFailed", Result: gtr.Fail, Output: []string{"output-1"}, Data: map[string]interface{}{}}}, }, { "four passing benchmarks", @@ -256,7 +263,7 @@ func TestGroupBenchmarksByName(t *testing.T) { {ID: 4, Name: "BenchmarkOne", Result: gtr.Pass, Data: map[string]interface{}{key: Benchmark{NsPerOp: 40, MBPerSec: 100, BytesPerOp: 5, AllocsPerOp: 2}}}, }, []gtr.Test{ - {ID: 1, Name: "BenchmarkOne", Result: gtr.Pass, Data: map[string]interface{}{key: Benchmark{NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 4}}}, + {ID: 1, Name: "BenchmarkOne", Result: gtr.Pass, Output: []string{"output-1", "output-2", "output-3", "output-4"}, Data: map[string]interface{}{key: Benchmark{NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 4}}}, }, }, { @@ -268,15 +275,14 @@ func TestGroupBenchmarksByName(t *testing.T) { {ID: 4, Name: "BenchmarkMixed", Result: gtr.Fail}, }, []gtr.Test{ - {ID: 1, Name: "BenchmarkMixed", Result: gtr.Fail, Data: map[string]interface{}{key: Benchmark{NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 3}}}, + {ID: 1, Name: "BenchmarkMixed", Result: gtr.Fail, Output: []string{"output-1", "output-2", "output-3", "output-4"}, Data: map[string]interface{}{key: Benchmark{NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 3}}}, }, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - b := newReportBuilder() - got := b.groupBenchmarksByName(test.in) + got := groupBenchmarksByName(test.in, output) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("groupBenchmarksByName result incorrect, diff (-want, +got):\n%s\n", diff) } From 27ad87e370c4a44a41afb2eee08f09716b6c11ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Thu, 11 Aug 2022 23:29:28 +0100 Subject: [PATCH 08/13] parser/gotest: parseLine now returns the events it creates --- parser/gotest/gotest.go | 108 +++++++++++++++++----------------- parser/gotest/gotest_test.go | 110 ++++++++++++++++------------------- 2 files changed, 105 insertions(+), 113 deletions(-) diff --git a/parser/gotest/gotest.go b/parser/gotest/gotest.go index b79daeb..ad0f5e0 100644 --- a/parser/gotest/gotest.go +++ b/parser/gotest/gotest.go @@ -143,6 +143,8 @@ func (p *Parser) parse(r *reader.LimitedLineReader) (gtr.Report, error) { return gtr.Report{}, err } + var evs []Event + // Lines that exceed bufio.MaxScanTokenSize are not expected to contain // any relevant test infrastructure output, so instead of parsing them // we treat them as regular output to increase performance. @@ -152,9 +154,13 @@ func (p *Parser) parse(r *reader.LimitedLineReader) (gtr.Report, error) { // turned out to be fine in almost all cases, it seemed an appropriate // value to use to decide whether or not to attempt parsing this line. if len(line) > bufio.MaxScanTokenSize { - p.output(line) + evs = p.output(line) } else { - p.parseLine(line) + evs = p.parseLine(line) + } + + for _, ev := range evs { + p.events = append(p.events, ev) } } return p.report(p.events), nil @@ -181,76 +187,70 @@ func (p *Parser) Events() []Event { return events } -func (p *Parser) parseLine(line string) { +func (p *Parser) parseLine(line string) (events []Event) { if strings.HasPrefix(line, "=== RUN ") { - p.runTest(strings.TrimSpace(line[8:])) + return p.runTest(strings.TrimSpace(line[8:])) } else if strings.HasPrefix(line, "=== PAUSE ") { - p.pauseTest(strings.TrimSpace(line[10:])) + return p.pauseTest(strings.TrimSpace(line[10:])) } else if strings.HasPrefix(line, "=== CONT ") { - p.contTest(strings.TrimSpace(line[9:])) + return p.contTest(strings.TrimSpace(line[9:])) } else if matches := regexEndTest.FindStringSubmatch(line); len(matches) == 5 { - p.endTest(line, matches[1], matches[2], matches[3], matches[4]) + return p.endTest(line, matches[1], matches[2], matches[3], matches[4]) } else if matches := regexStatus.FindStringSubmatch(line); len(matches) == 2 { - p.status(matches[1]) + return p.status(matches[1]) } else if matches := regexSummary.FindStringSubmatch(line); len(matches) == 8 { - p.summary(matches[1], matches[2], matches[3], matches[4], matches[5], matches[6], matches[7]) + return p.summary(matches[1], matches[2], matches[3], matches[4], matches[5], matches[6], matches[7]) } else if matches := regexCoverage.FindStringSubmatch(line); len(matches) == 3 { - p.coverage(matches[1], matches[2]) + return p.coverage(matches[1], matches[2]) } else if matches := regexBenchmark.FindStringSubmatch(line); len(matches) == 2 { - p.runBench(matches[1]) + return p.runBench(matches[1]) } else if matches := regexBenchSummary.FindStringSubmatch(line); len(matches) == 7 { - p.benchSummary(matches[1], matches[2], matches[3], matches[4], matches[5], matches[6]) + return p.benchSummary(matches[1], matches[2], matches[3], matches[4], matches[5], matches[6]) } else if matches := regexEndBenchmark.FindStringSubmatch(line); len(matches) == 3 { - p.endBench(matches[1], matches[2]) + return p.endBench(matches[1], matches[2]) } else if strings.HasPrefix(line, "# ") { - // TODO(jstemmer): this should just be output; we should detect build output when building report fields := strings.Fields(strings.TrimPrefix(line, "# ")) if len(fields) == 1 || len(fields) == 2 { - p.buildOutput(fields[0]) - } else { - p.output(line) + return p.buildOutput(fields[0]) } - } else { - p.output(line) } + return p.output(line) } -func (p *Parser) add(event Event) { - p.events = append(p.events, event) +func (p *Parser) runTest(name string) []Event { + return []Event{{Type: "run_test", Name: name}} } -func (p *Parser) runTest(name string) { - p.add(Event{Type: "run_test", Name: name}) +func (p *Parser) pauseTest(name string) []Event { + return []Event{{Type: "pause_test", Name: name}} } -func (p *Parser) pauseTest(name string) { - p.add(Event{Type: "pause_test", Name: name}) +func (p *Parser) contTest(name string) []Event { + return []Event{{Type: "cont_test", Name: name}} } -func (p *Parser) contTest(name string) { - p.add(Event{Type: "cont_test", Name: name}) -} - -func (p *Parser) endTest(line, indent, result, name, duration string) { +func (p *Parser) endTest(line, indent, result, name, duration string) []Event { + var events []Event if idx := strings.Index(line, fmt.Sprintf("%s--- %s:", indent, result)); idx > 0 { - p.output(line[:idx]) + events = append(events, p.output(line[:idx])...) } _, n := stripIndent(indent) - p.add(Event{ + events = append(events, Event{ Type: "end_test", Name: name, Result: result, Indent: n, Duration: parseSeconds(duration), }) + return events } -func (p *Parser) status(result string) { - p.add(Event{Type: "status", Result: result}) +func (p *Parser) status(result string) []Event { + return []Event{{Type: "status", Result: result}} } -func (p *Parser) summary(result, name, duration, cached, status, covpct, packages string) { - p.add(Event{ +func (p *Parser) summary(result, name, duration, cached, status, covpct, packages string) []Event { + return []Event{{ Type: "summary", Result: result, Name: name, @@ -258,26 +258,26 @@ func (p *Parser) summary(result, name, duration, cached, status, covpct, package Data: strings.TrimSpace(cached + " " + status), CovPct: parseFloat(covpct), CovPackages: parsePackages(packages), - }) + }} } -func (p *Parser) coverage(percent, packages string) { - p.add(Event{ +func (p *Parser) coverage(percent, packages string) []Event { + return []Event{{ Type: "coverage", CovPct: parseFloat(percent), CovPackages: parsePackages(packages), - }) + }} } -func (p *Parser) runBench(name string) { - p.add(Event{ +func (p *Parser) runBench(name string) []Event { + return []Event{{ Type: "run_benchmark", Name: name, - }) + }} } -func (p *Parser) benchSummary(name, iterations, nsPerOp, mbPerSec, bytesPerOp, allocsPerOp string) { - p.add(Event{ +func (p *Parser) benchSummary(name, iterations, nsPerOp, mbPerSec, bytesPerOp, allocsPerOp string) []Event { + return []Event{{ Type: "benchmark", Name: name, Iterations: parseInt(iterations), @@ -285,26 +285,26 @@ func (p *Parser) benchSummary(name, iterations, nsPerOp, mbPerSec, bytesPerOp, a MBPerSec: parseFloat(mbPerSec), BytesPerOp: parseInt(bytesPerOp), AllocsPerOp: parseInt(allocsPerOp), - }) + }} } -func (p *Parser) endBench(result, name string) { - p.add(Event{ +func (p *Parser) endBench(result, name string) []Event { + return []Event{{ Type: "end_benchmark", Name: name, Result: result, - }) + }} } -func (p *Parser) buildOutput(packageName string) { - p.add(Event{ +func (p *Parser) buildOutput(packageName string) []Event { + return []Event{{ Type: "build_output", Name: packageName, - }) + }} } -func (p *Parser) output(line string) { - p.add(Event{Type: "output", Data: line}) +func (p *Parser) output(line string) []Event { + return []Event{{Type: "output", Data: line}} } func parseSeconds(s string) time.Duration { diff --git a/parser/gotest/gotest_test.go b/parser/gotest/gotest_test.go index 7e66f1a..ff3e1ba 100644 --- a/parser/gotest/gotest_test.go +++ b/parser/gotest/gotest_test.go @@ -16,37 +16,45 @@ var ( type parseLineTest struct { input string - events interface{} + events []Event +} + +func (t parseLineTest) Name() string { + var types []string + for _, e := range t.events { + types = append(types, e.Type) + } + return strings.Join(types, "-") } var parseLineTests = []parseLineTest{ { "=== RUN TestOne", - Event{Type: "run_test", Name: "TestOne"}, + []Event{{Type: "run_test", Name: "TestOne"}}, }, { "=== RUN TestTwo/Subtest", - Event{Type: "run_test", Name: "TestTwo/Subtest"}, + []Event{{Type: "run_test", Name: "TestTwo/Subtest"}}, }, { "=== PAUSE TestOne", - Event{Type: "pause_test", Name: "TestOne"}, + []Event{{Type: "pause_test", Name: "TestOne"}}, }, { "=== CONT TestOne", - Event{Type: "cont_test", Name: "TestOne"}, + []Event{{Type: "cont_test", Name: "TestOne"}}, }, { "--- PASS: TestOne (12.34 seconds)", - Event{Type: "end_test", Name: "TestOne", Result: "PASS", Duration: 12_340 * time.Millisecond}, + []Event{{Type: "end_test", Name: "TestOne", Result: "PASS", Duration: 12_340 * time.Millisecond}}, }, { " --- SKIP: TestOne/Subtest (0.00s)", - Event{Type: "end_test", Name: "TestOne/Subtest", Result: "SKIP", Indent: 1}, + []Event{{Type: "end_test", Name: "TestOne/Subtest", Result: "SKIP", Indent: 1}}, }, { " --- FAIL: TestOne/Subtest/#01 (0.35s)", - Event{Type: "end_test", Name: "TestOne/Subtest/#01", Result: "FAIL", Duration: 350 * time.Millisecond, Indent: 2}, + []Event{{Type: "end_test", Name: "TestOne/Subtest/#01", Result: "FAIL", Duration: 350 * time.Millisecond, Indent: 2}}, }, { "some text--- PASS: TestTwo (0.06 seconds)", @@ -57,157 +65,141 @@ var parseLineTests = []parseLineTest{ }, { "PASS", - Event{Type: "status", Result: "PASS"}, + []Event{{Type: "status", Result: "PASS"}}, }, { "FAIL", - Event{Type: "status", Result: "FAIL"}, + []Event{{Type: "status", Result: "FAIL"}}, }, { "SKIP", - Event{Type: "status", Result: "SKIP"}, + []Event{{Type: "status", Result: "SKIP"}}, }, { "ok package/name/ok 0.100s", - Event{Type: "summary", Name: "package/name/ok", Result: "ok", Duration: 100 * time.Millisecond}, + []Event{{Type: "summary", Name: "package/name/ok", Result: "ok", Duration: 100 * time.Millisecond}}, }, { "FAIL package/name/failing [build failed]", - Event{Type: "summary", Name: "package/name/failing", Result: "FAIL", Data: "[build failed]"}, + []Event{{Type: "summary", Name: "package/name/failing", Result: "FAIL", Data: "[build failed]"}}, }, { "FAIL package/other/failing [setup failed]", - Event{Type: "summary", Name: "package/other/failing", Result: "FAIL", Data: "[setup failed]"}, + []Event{{Type: "summary", Name: "package/other/failing", Result: "FAIL", Data: "[setup failed]"}}, }, { "ok package/other (cached)", - Event{Type: "summary", Name: "package/other", Result: "ok", Data: "(cached)"}, + []Event{{Type: "summary", Name: "package/other", Result: "ok", Data: "(cached)"}}, }, { "ok package/name 0.400s coverage: 10.0% of statements", - Event{Type: "summary", Name: "package/name", Result: "ok", Duration: 400 * time.Millisecond, CovPct: 10}, + []Event{{Type: "summary", Name: "package/name", Result: "ok", Duration: 400 * time.Millisecond, CovPct: 10}}, }, { "ok package/name 4.200s coverage: 99.8% of statements in fmt, encoding/xml", - Event{Type: "summary", Name: "package/name", Result: "ok", Duration: 4200 * time.Millisecond, CovPct: 99.8, CovPackages: []string{"fmt", "encoding/xml"}}, + []Event{{Type: "summary", Name: "package/name", Result: "ok", Duration: 4200 * time.Millisecond, CovPct: 99.8, CovPackages: []string{"fmt", "encoding/xml"}}}, }, { "? package/name [no test files]", - Event{Type: "summary", Name: "package/name", Result: "?", Data: "[no test files]"}, + []Event{{Type: "summary", Name: "package/name", Result: "?", Data: "[no test files]"}}, }, { "ok package/name 0.001s [no tests to run]", - Event{Type: "summary", Name: "package/name", Result: "ok", Duration: 1 * time.Millisecond, Data: "[no tests to run]"}, + []Event{{Type: "summary", Name: "package/name", Result: "ok", Duration: 1 * time.Millisecond, Data: "[no tests to run]"}}, }, { "ok package/name (cached) [no tests to run]", - Event{Type: "summary", Name: "package/name", Result: "ok", Data: "(cached) [no tests to run]"}, + []Event{{Type: "summary", Name: "package/name", Result: "ok", Data: "(cached) [no tests to run]"}}, }, { "coverage: 10% of statements", - Event{Type: "coverage", CovPct: 10}, + []Event{{Type: "coverage", CovPct: 10}}, }, { "coverage: 10% of statements in fmt, encoding/xml", - Event{Type: "coverage", CovPct: 10, CovPackages: []string{"fmt", "encoding/xml"}}, + []Event{{Type: "coverage", CovPct: 10, CovPackages: []string{"fmt", "encoding/xml"}}}, }, { "coverage: 13.37% of statements", - Event{Type: "coverage", CovPct: 13.37}, + []Event{{Type: "coverage", CovPct: 13.37}}, }, { "coverage: 99.8% of statements in fmt, encoding/xml", - Event{Type: "coverage", CovPct: 99.8, CovPackages: []string{"fmt", "encoding/xml"}}, + []Event{{Type: "coverage", CovPct: 99.8, CovPackages: []string{"fmt", "encoding/xml"}}}, }, { "BenchmarkOK", - Event{Type: "run_benchmark", Name: "BenchmarkOK"}, + []Event{{Type: "run_benchmark", Name: "BenchmarkOK"}}, }, { "BenchmarkOne-8 2000000 604 ns/op", - Event{Type: "benchmark", Name: "BenchmarkOne", Iterations: 2_000_000, NsPerOp: 604}, + []Event{{Type: "benchmark", Name: "BenchmarkOne", Iterations: 2_000_000, NsPerOp: 604}}, }, { "BenchmarkTwo-16 30000 52568 ns/op 24879 B/op 494 allocs/op", - Event{Type: "benchmark", Name: "BenchmarkTwo", Iterations: 30_000, NsPerOp: 52_568, BytesPerOp: 24_879, AllocsPerOp: 494}, + []Event{{Type: "benchmark", Name: "BenchmarkTwo", Iterations: 30_000, NsPerOp: 52_568, BytesPerOp: 24_879, AllocsPerOp: 494}}, }, { "BenchmarkThree 2000000000 0.26 ns/op", - Event{Type: "benchmark", Name: "BenchmarkThree", Iterations: 2_000_000_000, NsPerOp: 0.26}, + []Event{{Type: "benchmark", Name: "BenchmarkThree", Iterations: 2_000_000_000, NsPerOp: 0.26}}, }, { "BenchmarkFour-8 10000 104427 ns/op 95.76 MB/s 40629 B/op 5 allocs/op", - Event{Type: "benchmark", Name: "BenchmarkFour", Iterations: 10_000, NsPerOp: 104_427, MBPerSec: 95.76, BytesPerOp: 40_629, AllocsPerOp: 5}, + []Event{{Type: "benchmark", Name: "BenchmarkFour", Iterations: 10_000, NsPerOp: 104_427, MBPerSec: 95.76, BytesPerOp: 40_629, AllocsPerOp: 5}}, }, { "--- BENCH: BenchmarkOK-8", - Event{Type: "end_benchmark", Name: "BenchmarkOK", Result: "BENCH"}, + []Event{{Type: "end_benchmark", Name: "BenchmarkOK", Result: "BENCH"}}, }, { "--- FAIL: BenchmarkError", - Event{Type: "end_benchmark", Name: "BenchmarkError", Result: "FAIL"}, + []Event{{Type: "end_benchmark", Name: "BenchmarkError", Result: "FAIL"}}, }, { "--- SKIP: BenchmarkSkip", - Event{Type: "end_benchmark", Name: "BenchmarkSkip", Result: "SKIP"}, + []Event{{Type: "end_benchmark", Name: "BenchmarkSkip", Result: "SKIP"}}, }, { "# package/name/failing1", - Event{Type: "build_output", Name: "package/name/failing1"}, + []Event{{Type: "build_output", Name: "package/name/failing1"}}, }, { "# package/name/failing2 [package/name/failing2.test]", - Event{Type: "build_output", Name: "package/name/failing2"}, + []Event{{Type: "build_output", Name: "package/name/failing2"}}, }, { "single line stdout", - Event{Type: "output", Data: "single line stdout"}, + []Event{{Type: "output", Data: "single line stdout"}}, }, { "# some more output", - Event{Type: "output", Data: "# some more output"}, + []Event{{Type: "output", Data: "# some more output"}}, }, { "\tfile_test.go:11: Error message", - Event{Type: "output", Data: "\tfile_test.go:11: Error message"}, + []Event{{Type: "output", Data: "\tfile_test.go:11: Error message"}}, }, { "\tfile_test.go:12: Longer", - Event{Type: "output", Data: "\tfile_test.go:12: Longer"}, + []Event{{Type: "output", Data: "\tfile_test.go:12: Longer"}}, }, { "\t\terror", - Event{Type: "output", Data: "\t\terror"}, + []Event{{Type: "output", Data: "\t\terror"}}, }, { "\t\tmessage.", - Event{Type: "output", Data: "\t\tmessage."}, + []Event{{Type: "output", Data: "\t\tmessage."}}, }, } func TestParseLine(t *testing.T) { for i, test := range parseLineTests { - var want []Event - switch e := test.events.(type) { - case Event: - want = []Event{e} - case []Event: - want = e - default: - panic("invalid events type") - } - - var types []string - for _, e := range want { - types = append(types, e.Type) - } - - name := fmt.Sprintf("%d %s", i+1, strings.Join(types, ",")) + name := fmt.Sprintf("%d-%s", i, test.Name()) t.Run(name, func(t *testing.T) { parser := NewParser() - parser.parseLine(test.input) - got := parser.events - if diff := cmp.Diff(want, got); diff != "" { + events := parser.parseLine(test.input) + if diff := cmp.Diff(events, test.events); diff != "" { t.Errorf("parseLine(%q) returned unexpected events, diff (-want, +got):\n%v", test.input, diff) } }) From f6f9df42b9191a477dbe185a5e0faf671927ceac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Thu, 11 Aug 2022 23:48:41 +0100 Subject: [PATCH 09/13] parser/gotest: Create interface for reading lines with metadata --- parser/gotest/gotest.go | 4 ++-- parser/gotest/internal/reader/reader.go | 22 +++++++++++++++----- parser/gotest/internal/reader/reader_test.go | 6 +++--- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/parser/gotest/gotest.go b/parser/gotest/gotest.go index ad0f5e0..4ffda64 100644 --- a/parser/gotest/gotest.go +++ b/parser/gotest/gotest.go @@ -133,10 +133,10 @@ func (p *Parser) Parse(r io.Reader) (gtr.Report, error) { return p.parse(reader.NewLimitedLineReader(r, maxLineSize)) } -func (p *Parser) parse(r *reader.LimitedLineReader) (gtr.Report, error) { +func (p *Parser) parse(r reader.LineReader) (gtr.Report, error) { p.events = nil for { - line, err := r.ReadLine() + line, _, err := r.ReadLine() if err == io.EOF { break } else if err != nil { diff --git a/parser/gotest/internal/reader/reader.go b/parser/gotest/internal/reader/reader.go index 57b2365..35a0dd9 100644 --- a/parser/gotest/internal/reader/reader.go +++ b/parser/gotest/internal/reader/reader.go @@ -6,6 +6,16 @@ import ( "io" ) +// LineReader is an interface to read lines with optional Metadata. +type LineReader interface { + ReadLine() (string, *Metadata, error) +} + +// Metadata contains metadata that belongs to a line. +type Metadata struct { + Package string +} + // LimitedLineReader reads lines from an io.Reader object with a configurable // line size limit. Lines exceeding the limit will be truncated, but read // completely from the underlying io.Reader. @@ -14,6 +24,8 @@ type LimitedLineReader struct { limit int } +var _ LineReader = &LimitedLineReader{} + // NewLimitedLineReader returns a LimitedLineReader to read lines from r with a // maximum line size of limit. func NewLimitedLineReader(r io.Reader, limit int) *LimitedLineReader { @@ -23,14 +35,14 @@ func NewLimitedLineReader(r io.Reader, limit int) *LimitedLineReader { // ReadLine returns the next line from the underlying reader. The length of the // line will not exceed the configured limit. ReadLine either returns a line or // it returns an error, never both. -func (r *LimitedLineReader) ReadLine() (string, error) { +func (r *LimitedLineReader) ReadLine() (string, *Metadata, error) { line, isPrefix, err := r.r.ReadLine() if err != nil { - return "", err + return "", nil, err } if !isPrefix { - return string(line), nil + return string(line), nil, nil } // Line is incomplete, keep reading until we reach the end of the line. @@ -39,7 +51,7 @@ func (r *LimitedLineReader) ReadLine() (string, error) { for isPrefix { line, isPrefix, err = r.r.ReadLine() if err != nil { - return "", err + return "", nil, err } if buf.Len() >= r.limit { @@ -54,5 +66,5 @@ func (r *LimitedLineReader) ReadLine() (string, error) { if buf.Len() > r.limit { buf.Truncate(r.limit) } - return buf.String(), nil + return buf.String(), nil, nil } diff --git a/parser/gotest/internal/reader/reader_test.go b/parser/gotest/internal/reader/reader_test.go index 8526b9d..f052495 100644 --- a/parser/gotest/internal/reader/reader_test.go +++ b/parser/gotest/internal/reader/reader_test.go @@ -34,7 +34,7 @@ func TestLimitedLineReader(t *testing.T) { input := strings.NewReader(strings.Join([]string{line1, line2}, "\n")) r := NewLimitedLineReader(input, testingLimit) - got, err := r.ReadLine() + got, _, err := r.ReadLine() if err != nil { t.Fatalf("ReadLine() returned error %v", err) } @@ -47,7 +47,7 @@ func TestLimitedLineReader(t *testing.T) { t.Fatalf("ReadLine() returned incorrect line, got len %d want len %d", len(got), len(want)) } - got, err = r.ReadLine() + got, _, err = r.ReadLine() if err != nil { t.Fatalf("ReadLine() returned error %v", err) } @@ -56,7 +56,7 @@ func TestLimitedLineReader(t *testing.T) { t.Fatalf("ReadLine() returned incorrect line, got len %d want len %d", len(got), len(want)) } - got, err = r.ReadLine() + got, _, err = r.ReadLine() if err != io.EOF { t.Fatalf("ReadLine() returned unexpected error, got %v want %v\n", err, io.EOF) } From bd21d5450128be588cbd8477bce4da1a94f2c46a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Thu, 11 Aug 2022 23:54:11 +0100 Subject: [PATCH 10/13] parser/gotest: Add Package to Event This allows each event to belong to a package, if we know it in advance. --- parser/gotest/event.go | 14 +++++++++++++- parser/gotest/gotest.go | 3 ++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/parser/gotest/event.go b/parser/gotest/event.go index 90ddded..6802c79 100644 --- a/parser/gotest/event.go +++ b/parser/gotest/event.go @@ -1,12 +1,17 @@ package gotest -import "time" +import ( + "time" + + "github.com/jstemmer/go-junit-report/v2/parser/gotest/internal/reader" +) // Event is a single event in a Go test or benchmark. type Event struct { Type string `json:"type"` Name string `json:"name,omitempty"` + Package string `json:"pkg,omitempty"` Result string `json:"result,omitempty"` Duration time.Duration `json:"duration,omitempty"` Data string `json:"data,omitempty"` @@ -23,3 +28,10 @@ type Event struct { BytesPerOp int64 `json:"benchmark_bytes_per_op,omitempty"` AllocsPerOp int64 `json:"benchmark_allocs_per_op,omitempty"` } + +func (e *Event) applyMetadata(m *reader.Metadata) { + if e == nil || m == nil { + return + } + e.Package = m.Package +} diff --git a/parser/gotest/gotest.go b/parser/gotest/gotest.go index 4ffda64..bc263fe 100644 --- a/parser/gotest/gotest.go +++ b/parser/gotest/gotest.go @@ -136,7 +136,7 @@ func (p *Parser) Parse(r io.Reader) (gtr.Report, error) { func (p *Parser) parse(r reader.LineReader) (gtr.Report, error) { p.events = nil for { - line, _, err := r.ReadLine() + line, metadata, err := r.ReadLine() if err == io.EOF { break } else if err != nil { @@ -160,6 +160,7 @@ func (p *Parser) parse(r reader.LineReader) (gtr.Report, error) { } for _, ev := range evs { + ev.applyMetadata(metadata) p.events = append(p.events, ev) } } From 85f2715ac92ec9b3b2b3b7885036487795e93d64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Thu, 11 Aug 2022 23:58:06 +0100 Subject: [PATCH 11/13] parser/gotest: Create JSONEventReader in internal reader package The JSONEventReader implements reading lines with metadata and replaces the gotest.jsonReader struct. --- parser/gotest/internal/reader/reader.go | 52 ++++++++++++++++ parser/gotest/internal/reader/reader_test.go | 33 ++++++++++ parser/gotest/json.go | 52 +--------------- parser/gotest/json_test.go | 65 -------------------- 4 files changed, 87 insertions(+), 115 deletions(-) delete mode 100644 parser/gotest/json_test.go diff --git a/parser/gotest/internal/reader/reader.go b/parser/gotest/internal/reader/reader.go index 35a0dd9..81ff2de 100644 --- a/parser/gotest/internal/reader/reader.go +++ b/parser/gotest/internal/reader/reader.go @@ -3,7 +3,10 @@ package reader import ( "bufio" "bytes" + "encoding/json" "io" + "strings" + "time" ) // LineReader is an interface to read lines with optional Metadata. @@ -68,3 +71,52 @@ func (r *LimitedLineReader) ReadLine() (string, *Metadata, error) { } return buf.String(), nil, nil } + +// Event represents a JSON event emitted by `go test -json`. +type Event struct { + Time time.Time + Action string + Package string + Test string + Elapsed float64 // seconds + Output string +} + +// JSONEventReader reads JSON events from an io.Reader object. +type JSONEventReader struct { + r *LimitedLineReader +} + +var _ LineReader = &JSONEventReader{} + +// jsonLineLimit is the maximum size of a single JSON line emitted by `go test +// -json`. +const jsonLineLimit = 64 * 1024 + +// NewJSONEventReader returns a JSONEventReader to read the data in JSON +// events from r. +func NewJSONEventReader(r io.Reader) *JSONEventReader { + return &JSONEventReader{NewLimitedLineReader(r, jsonLineLimit)} +} + +// ReadLine returns the next line from the underlying reader. +func (r *JSONEventReader) ReadLine() (string, *Metadata, error) { + for { + line, _, err := r.r.ReadLine() + if err != nil { + return "", nil, err + } + if len(line) == 0 || line[0] != '{' { + return line, nil, nil + } + event := &Event{} + if err := json.Unmarshal([]byte(line), event); err != nil { + return "", nil, err + } + if event.Output == "" { + // Skip events without output + continue + } + return strings.TrimSuffix(event.Output, "\n"), &Metadata{Package: event.Package}, nil + } +} diff --git a/parser/gotest/internal/reader/reader_test.go b/parser/gotest/internal/reader/reader_test.go index f052495..03664e3 100644 --- a/parser/gotest/internal/reader/reader_test.go +++ b/parser/gotest/internal/reader/reader_test.go @@ -5,6 +5,8 @@ import ( "io" "strings" "testing" + + "github.com/google/go-cmp/cmp" ) const testingLimit = 4 * 1024 * 1024 @@ -66,3 +68,34 @@ func TestLimitedLineReader(t *testing.T) { }) } } + +func TestJSONEventReader(t *testing.T) { + input := `some other output +{"Time":"2019-10-09T00:00:00.708139047+00:00","Action":"output","Package":"package/name/ok","Test":"TestOK"} +{"Time":"2019-10-09T00:00:00.708139047+00:00","Action":"output","Package":"package/name/ok","Test":"TestOK","Output":"=== RUN TestOK\n"} +` + want := []struct { + line string + metadata *Metadata + }{ + {"some other output", nil}, + {"=== RUN TestOK", &Metadata{Package: "package/name/ok"}}, + } + + r := NewJSONEventReader(strings.NewReader(input)) + for i := 0; i < len(want); i++ { + line, metadata, err := r.ReadLine() + if err == io.EOF { + return + } else if err != nil { + t.Fatalf("ReadLine() returned error %v", err) + } + + if diff := cmp.Diff(want[i].line, line); diff != "" { + t.Errorf("ReadLine() returned incorrect line, diff (-want, +got):\n%s\n", diff) + } + if diff := cmp.Diff(want[i].metadata, metadata); diff != "" { + t.Errorf("ReadLine() Returned incorrect metadata, diff (-want, +got):\n%s\n", diff) + } + } +} diff --git a/parser/gotest/json.go b/parser/gotest/json.go index 61471e9..bb0ccec 100644 --- a/parser/gotest/json.go +++ b/parser/gotest/json.go @@ -1,12 +1,10 @@ package gotest import ( - "bufio" - "encoding/json" "io" - "time" "github.com/jstemmer/go-junit-report/v2/gtr" + "github.com/jstemmer/go-junit-report/v2/parser/gotest/internal/reader" ) // NewJSONParser returns a new Go test json output parser. @@ -22,56 +20,10 @@ type JSONParser struct { // Parse parses Go test json output from the given io.Reader r and returns // gtr.Report. func (p *JSONParser) Parse(r io.Reader) (gtr.Report, error) { - return p.gp.Parse(newJSONReader(r)) + return p.gp.parse(reader.NewJSONEventReader(r)) } // Events returns the events created by the parser. func (p *JSONParser) Events() []Event { return p.gp.Events() } - -type jsonEvent struct { - Time time.Time - Action string - Package string - Test string - Elapsed float64 // seconds - Output string -} - -type jsonReader struct { - r *bufio.Reader - buf []byte -} - -func newJSONReader(reader io.Reader) *jsonReader { - return &jsonReader{r: bufio.NewReader(reader)} -} - -func (j *jsonReader) Read(p []byte) (int, error) { - var err error - for len(j.buf) == 0 { - j.buf, err = j.readNextLine() - if err != nil { - return 0, err - } - } - n := copy(p, j.buf) - j.buf = j.buf[n:] - return n, nil -} - -func (j jsonReader) readNextLine() ([]byte, error) { - line, err := j.r.ReadBytes('\n') - if err != nil { - return nil, err - } - if len(line) == 0 || line[0] != '{' { - return line, nil - } - var event jsonEvent - if err := json.Unmarshal(line, &event); err != nil { - return nil, err - } - return []byte(event.Output), nil -} diff --git a/parser/gotest/json_test.go b/parser/gotest/json_test.go deleted file mode 100644 index 85b4177..0000000 --- a/parser/gotest/json_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package gotest - -import ( - "io" - "io/ioutil" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" -) - -var input = `some other output -{"Time":"2019-10-09T00:00:00.708139047+00:00","Action":"output","Package":"package/name/ok","Test":"TestOK"} -{"Time":"2019-10-09T00:00:00.708139047+00:00","Action":"output","Package":"package/name/ok","Test":"TestOK","Output":"=== RUN TestOK\n"} -` - -func TestJSONReaderReadAll(t *testing.T) { - r := newJSONReader(strings.NewReader(input)) - got, err := ioutil.ReadAll(r) - if err != nil { - t.Fatal(err) - } - - want := `some other output -=== RUN TestOK -` - - if diff := cmp.Diff(want, string(got)); diff != "" { - t.Errorf("unexpected result from jsonReader, diff (-want, +got):\n%s\n", diff) - } -} - -func TestJSONReaderReadSmallBuffer(t *testing.T) { - expected := [][]byte{ - []byte("some"), - []byte(" oth"), - []byte("er o"), - []byte("utpu"), - []byte("t\n"), - []byte("=== "), - []byte("RUN "), - []byte(" Te"), - []byte("stOK"), - []byte("\n"), - } - - r := newJSONReader(strings.NewReader(input)) - buf := make([]byte, 4) - for _, want := range expected { - n, err := r.Read(buf) - if err != nil { - t.Fatalf("Read error: %v", err) - } - - got := buf[:n] - if diff := cmp.Diff(string(want), string(got)); diff != "" { - t.Fatalf("unexpected result from jsonReader, diff (-want, +got):\n%s\n", diff) - } - } - - _, err := r.Read(buf) - if err != io.EOF { - t.Fatalf("unexpected error from jsonReader: got %v, want %v", err, io.EOF) - } -} From 9357c180d45cf87633faf7b5bc17949fd484f839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Sat, 13 Aug 2022 22:26:06 +0100 Subject: [PATCH 12/13] parser/gotest: Add support for processing events for different packages The reportBuilder assumed we were always processing events for a single package at a time. This is not true however when running `go test -json -race` for example. In order to properly support processing events from different packages we now have packageBuilders per package name. Fixes #134 --- parser/gotest/report_builder.go | 519 +++++++++++++++------------ parser/gotest/report_builder_test.go | 60 ++++ 2 files changed, 348 insertions(+), 231 deletions(-) diff --git a/parser/gotest/report_builder.go b/parser/gotest/report_builder.go index 8a6df4c..d53b323 100644 --- a/parser/gotest/report_builder.go +++ b/parser/gotest/report_builder.go @@ -2,6 +2,7 @@ package gotest import ( "fmt" + "sort" "strings" "time" @@ -15,21 +16,20 @@ const ( // reportBuilder helps build a test Report from a collection of events. // -// The reportBuilder keeps track of the active context whenever a test or build -// error is created. This is necessary because the test parser do not contain -// any state themselves and simply just emit an event for every line that is -// read. By tracking the active context, any output that is appended to the -// reportBuilder gets attributed to the correct test or build error. +// The reportBuilder delegates to the packageBuilder for creating packages from +// basic test events, but keeps track of build errors itself. The reportBuilder +// is also responsible for generating unique test id's. +// +// Test output is collected by the output collector, which also keeps track of +// the currently active test so output is automatically associated with the +// correct test. type reportBuilder struct { - packages []gtr.Package - tests map[int]gtr.Test - buildErrors map[int]gtr.Error + packageBuilders map[string]*packageBuilder + buildErrors map[int]gtr.Error - // state - nextID int // next free unused 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 + nextID int // next free unused id + output *collector.Output // output collected for each id + packages []gtr.Package // completed packages // options packageName string @@ -40,296 +40,188 @@ type reportBuilder struct { // newReportBuilder creates a new reportBuilder. func newReportBuilder() *reportBuilder { return &reportBuilder{ - tests: make(map[int]gtr.Test), - buildErrors: make(map[int]gtr.Error), - nextID: 1, - output: collector.New(), - parentIDs: make(map[int]struct{}), - timestampFunc: time.Now, + packageBuilders: make(map[string]*packageBuilder), + buildErrors: make(map[int]gtr.Error), + nextID: 1, + output: collector.New(), + timestampFunc: time.Now, } } -// ProcessEvent gives an event to this reportBuilder to be processed for this -// report. +// getPackageBuilder returns the packageBuilder for the given packageName. If +// no packageBuilder exists for the given package, a new one is created. +func (b *reportBuilder) getPackageBuilder(packageName string) *packageBuilder { + pb, ok := b.packageBuilders[packageName] + if !ok { + output := b.output + if packageName != "" { + output = collector.New() + } + pb = newPackageBuilder(b.generateID, output) + b.packageBuilders[packageName] = pb + } + return pb +} + +// ProcessEvent takes a test event and adds it to the report. func (b *reportBuilder) ProcessEvent(ev Event) { switch ev.Type { case "run_test": - b.CreateTest(ev.Name) + b.getPackageBuilder(ev.Package).CreateTest(ev.Name) case "pause_test": - b.PauseTest(ev.Name) + b.getPackageBuilder(ev.Package).PauseTest(ev.Name) case "cont_test": - b.ContinueTest(ev.Name) + b.getPackageBuilder(ev.Package).ContinueTest(ev.Name) case "end_test": - b.EndTest(ev.Name, ev.Result, ev.Duration, ev.Indent) + b.getPackageBuilder(ev.Package).EndTest(ev.Name, ev.Result, ev.Duration, ev.Indent) case "run_benchmark": - b.CreateTest(ev.Name) + b.getPackageBuilder(ev.Package).CreateTest(ev.Name) case "benchmark": - b.BenchmarkResult(ev.Name, ev.Iterations, ev.NsPerOp, ev.MBPerSec, ev.BytesPerOp, ev.AllocsPerOp) + b.getPackageBuilder(ev.Package).BenchmarkResult(ev.Name, ev.Iterations, ev.NsPerOp, ev.MBPerSec, ev.BytesPerOp, ev.AllocsPerOp) case "end_benchmark": - b.EndTest(ev.Name, ev.Result, 0, 0) + b.getPackageBuilder(ev.Package).EndTest(ev.Name, ev.Result, 0, 0) case "status": - b.End() + b.getPackageBuilder(ev.Package).End() case "summary": - b.CreatePackage(ev.Name, ev.Result, ev.Duration, ev.Data) + // The summary marks the end of a package. We can now create the actual + // package from all the events we've processed so far for this package. + b.packages = append(b.packages, b.CreatePackage(ev.Package, ev.Name, ev.Result, ev.Duration, ev.Data)) case "coverage": - b.Coverage(ev.CovPct, ev.CovPackages) + b.getPackageBuilder(ev.Package).Coverage(ev.CovPct, ev.CovPackages) case "build_output": b.CreateBuildError(ev.Name) case "output": - b.AppendOutput(ev.Data) + if ev.Package != "" { + b.getPackageBuilder(ev.Package).Output(ev.Data) + } else { + b.output.Append(ev.Data) + } default: + // This shouldn't happen, but just in case print a warning and ignore + // this event. fmt.Printf("reportBuilder: unhandled event type: %v\n", ev.Type) } } -// newID returns a new unique id and sets the active context this id. -func (b *reportBuilder) newID() int { +// newID returns a new unique id. +func (b *reportBuilder) generateID() int { id := b.nextID b.nextID++ return id } -// flush creates a new package in this report containing any tests we've -// collected so far. This is necessary when a test did not end with a summary. -func (b *reportBuilder) flush() { - if len(b.tests) > 0 { - b.CreatePackage(b.packageName, "", 0, "") - } -} - -// Build returns the new Report containing all the tests and output created so -// far. +// Build returns the new Report containing all the tests, build errors and +// their output created from the processed events. func (b *reportBuilder) Build() gtr.Report { - b.flush() + // Create packages for any leftover package builders. + for name, pb := range b.packageBuilders { + if pb.IsEmpty() { + continue + } + b.packages = append(b.packages, b.CreatePackage(name, b.packageName, "", 0, "")) + } return gtr.Report{Packages: b.packages} } -// CreateTest adds a test with the given name to the report, and marks it as -// active. -func (b *reportBuilder) CreateTest(name string) int { - if parentID, ok := b.findTestParentID(name); ok { - b.parentIDs[parentID] = struct{}{} - } - id := b.newID() - b.output.SetActiveID(id) - b.tests[id] = gtr.NewTest(id, name) - return id -} - -// PauseTest marks the active context as no longer active. Any results or -// output added to the report after calling PauseTest will no longer be assumed -// to belong to this test. -func (b *reportBuilder) PauseTest(name string) { - b.output.SetActiveID(0) -} - -// ContinueTest finds the test with the given name and marks it as active. If -// more than one test exist with this name, the most recently created test will -// be used. -func (b *reportBuilder) ContinueTest(name string) { - id, _ := b.findTest(name) - b.output.SetActiveID(id) -} - -// EndTest finds the test with the given name, sets the result, duration and -// level. If more than one test exists with this name, the most recently -// created test will be used. If no test exists with this name, a new test is -// created. -func (b *reportBuilder) EndTest(name, result string, duration time.Duration, level int) { - id, ok := b.findTest(name) - if !ok { - // test did not exist, create one - // TODO: Likely reason is that the user ran go test without the -v - // flag, should we report this somewhere? - id = b.CreateTest(name) - } - - t := b.tests[id] - t.Result = parseResult(result) - t.Duration = duration - t.Level = level - b.tests[id] = t - b.output.SetActiveID(0) -} - -// End marks the active context as no longer active. -func (b *reportBuilder) End() { - b.output.SetActiveID(0) -} - -// BenchmarkResult updates an existing or adds a new test with the given -// results and marks it as active. If an existing test with this name exists -// but without result, then that one is updated. Otherwise a new one is added -// to the report. -func (b *reportBuilder) BenchmarkResult(name string, iterations int64, nsPerOp, mbPerSec float64, bytesPerOp, allocsPerOp int64) { - id, ok := b.findTest(name) - if !ok || b.tests[id].Result != gtr.Unknown { - id = b.CreateTest(name) - } - - benchmark := Benchmark{iterations, nsPerOp, mbPerSec, bytesPerOp, allocsPerOp} - test := gtr.NewTest(id, name) - test.Result = gtr.Pass - test.Duration = benchmark.ApproximateDuration() - SetBenchmarkData(&test, benchmark) - b.tests[id] = test -} - // CreateBuildError creates a new build error and marks it as active. func (b *reportBuilder) CreateBuildError(packageName string) { - id := b.newID() + id := b.generateID() b.output.SetActiveID(id) b.buildErrors[id] = gtr.Error{ID: id, Name: packageName} } -// CreatePackage adds a new package with the given name to the Report. This -// package contains all the build errors, output, tests and benchmarks created -// so far. Afterwards all state is reset. -func (b *reportBuilder) CreatePackage(name, result string, duration time.Duration, data string) { +// CreatePackage returns a new package containing all the build errors, output, +// tests and benchmarks created so far. The optional packageName is used to +// find the correct reportBuilder. The newPackageName is the actual package +// name that will be given to the returned package, which should be used in +// case the packageName was unknown until this point. +func (b *reportBuilder) CreatePackage(packageName, newPackageName, result string, duration time.Duration, data string) gtr.Package { pkg := gtr.Package{ - Name: name, + Name: newPackageName, Duration: duration, Timestamp: b.timestampFunc(), } - // Build errors are treated somewhat differently. Rather than having a - // single package with all build errors collected so far, we only care - // about the build errors for this particular package. + // 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 == name { - if len(b.tests) > 0 { - panic("unexpected tests found in build error package") - } - buildErr.ID = id - buildErr.Duration = duration - buildErr.Cause = data - buildErr.Output = b.output.Get(id) - + if buildErr.Name == newPackageName { pkg.BuildError = buildErr - b.packages = append(b.packages, pkg) + pkg.BuildError.ID = id + pkg.BuildError.Duration = duration + pkg.BuildError.Cause = data + pkg.BuildError.Output = b.output.Get(id) delete(b.buildErrors, id) - // TODO: reset state - // TODO: buildErrors shouldn't reset/use nextID/lastID, they're more like a global cache - return + b.output.SetActiveID(0) + return pkg } } - // If we've collected output, but there were no tests then either there - // actually were no tests, or there was some other non-build error. - if b.output.Contains(globalID) && len(b.tests) == 0 { + // Get the packageBuilder for this package and make sure it's deleted, so + // future events for this package will use a new packageBuilder. + pb := b.getPackageBuilder(packageName) + delete(b.packageBuilders, packageName) + pb.output.SetActiveID(0) + + if pb.IsEmpty() { + return pkg + } + + // If we've collected output, but there were no tests, then this package + // had a runtime error or it simply didn't have any tests. + if pb.output.Contains(globalID) && len(pb.tests) == 0 { if parseResult(result) == gtr.Fail { pkg.RunError = gtr.Error{ - Name: name, - Output: b.output.Get(globalID), + Name: newPackageName, + Output: pb.output.Get(globalID), } } else { - pkg.Output = b.output.Get(globalID) + pkg.Output = pb.output.Get(globalID) } - b.packages = append(b.packages, pkg) - b.output.Clear(globalID) - return + pb.output.Clear(globalID) + return pkg } // If the summary result says we failed, but there were no failing tests // then something else must have failed. - if parseResult(result) == gtr.Fail && len(b.tests) > 0 && !b.containsFailures() { + if parseResult(result) == gtr.Fail && len(pb.tests) > 0 && !pb.containsFailures() { pkg.RunError = gtr.Error{ - Name: name, - Output: b.output.Get(globalID), + Name: newPackageName, + Output: pb.output.Get(globalID), } - b.output.Clear(globalID) + pb.output.Clear(globalID) } - // Collect tests for this package, maintaining insertion order. + // Collect tests for this package var tests []gtr.Test - for id := 1; id < b.nextID; id++ { - if t, ok := b.tests[id]; ok { - if b.isParent(id) { - if b.subtestMode == IgnoreParentResults { - t.Result = gtr.Pass - } else if b.subtestMode == ExcludeParents { - b.output.Merge(id, globalID) - continue - } + for id, t := range pb.tests { + if pb.isParent(id) { + if b.subtestMode == IgnoreParentResults { + t.Result = gtr.Pass + } else if b.subtestMode == ExcludeParents { + pb.output.Merge(id, globalID) + continue } - t.Output = b.output.Get(id) - tests = append(tests, t) - continue } + t.Output = pb.output.Get(id) + tests = append(tests, t) } tests = groupBenchmarksByName(tests, b.output) - pkg.Coverage = b.coverage - pkg.Output = b.output.Get(globalID) - pkg.Tests = tests - b.packages = append(b.packages, pkg) + // Sort packages by id to ensure we maintain insertion order. + sort.Slice(tests, func(i, j int) bool { + return tests[i].ID < tests[j].ID + }) - // reset state, except for nextID to ensure all id's are unique. - b.output.SetActiveID(0) - b.output.Clear(globalID) - b.coverage = 0 - b.tests = make(map[int]gtr.Test) - b.parentIDs = make(map[int]struct{}) + pkg.Tests = groupBenchmarksByName(tests, pb.output) + pkg.Coverage = pb.coverage + pkg.Output = pb.output.Get(globalID) + pb.output.Clear(globalID) + return pkg } -// Coverage sets the code coverage percentage. -func (b *reportBuilder) Coverage(pct float64, packages []string) { - b.coverage = pct -} - -// 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(text string) { - b.output.Append(text) -} - -// findTest returns the id of the most recently created test with the given -// name if it exists. -func (b *reportBuilder) findTest(name string) (int, bool) { - for i := b.nextID; i > 0; i-- { - if test, ok := b.tests[i]; ok && test.Name == name { - return i, true - } - } - return 0, false -} - -func (b *reportBuilder) findTestParentID(name string) (int, bool) { - parent := dropLastSegment(name) - for parent != "" { - if id, ok := b.findTest(parent); ok { - return id, true - } - parent = dropLastSegment(parent) - } - return 0, false -} - -func (b *reportBuilder) isParent(id int) bool { - _, ok := b.parentIDs[id] - return ok -} - -func dropLastSegment(name string) string { - if idx := strings.LastIndexByte(name, '/'); idx >= 0 { - return name[:idx] - } - return "" -} - -// containsFailures return true if the current list of tests contains at least -// one failing test or an unknown result. -func (b *reportBuilder) containsFailures() bool { - for _, test := range b.tests { - if test.Result == gtr.Fail || test.Result == gtr.Unknown { - return true - } - } - return false -} - -// parseResult returns a Result for the given string r. +// parseResult returns a gtr.Result for the given result string r. func parseResult(r string) gtr.Result { switch r { case "PASS": @@ -345,6 +237,8 @@ func parseResult(r string) gtr.Result { } } +// groupBenchmarksByName groups tests with the Benchmark prefix if they have +// the same name and combines their output. func groupBenchmarksByName(tests []gtr.Test, output *collector.Output) []gtr.Test { if len(tests) == 0 { return nil @@ -406,6 +300,7 @@ func groupBenchmarksByName(tests []gtr.Test, output *collector.Output) []gtr.Tes return grouped } +// combinedDuration returns the sum of the durations of the given tests. func combinedDuration(tests []gtr.Test) time.Duration { var total time.Duration for _, test := range tests { @@ -414,6 +309,7 @@ func combinedDuration(tests []gtr.Test) time.Duration { return total } +// groupResults returns the result we should use for a collection of tests. func groupResults(tests []gtr.Test) gtr.Result { var result gtr.Result for _, test := range tests { @@ -426,3 +322,164 @@ func groupResults(tests []gtr.Test) gtr.Result { } return result } + +// packageBuilder helps build a gtr.Package from a collection of test events. +type packageBuilder struct { + generateID func() int + output *collector.Output + + tests map[int]gtr.Test + parentIDs map[int]struct{} // set of test id's that contain subtests + coverage float64 // coverage percentage +} + +// newPackageBuilder creates a new packageBuilder. New tests will be assigned +// an ID returned by the generateID function. The activeIDSetter is called to +// set or reset the active test id. +func newPackageBuilder(generateID func() int, output *collector.Output) *packageBuilder { + return &packageBuilder{ + generateID: generateID, + output: output, + tests: make(map[int]gtr.Test), + parentIDs: make(map[int]struct{}), + } +} + +// IsEmpty returns true if this package builder does not have any tests and has +// not collected any global output. +func (b packageBuilder) IsEmpty() bool { + return len(b.tests) == 0 && !b.output.Contains(0) +} + +// CreateTest adds a test with the given name to the package, marks it as +// active and returns its generated id. +func (b *packageBuilder) CreateTest(name string) int { + if parentID, ok := b.findTestParentID(name); ok { + b.parentIDs[parentID] = struct{}{} + } + id := b.generateID() + b.output.SetActiveID(id) + b.tests[id] = gtr.NewTest(id, name) + return id +} + +// PauseTest marks the test with the given name no longer active. Any results +// or output added to the package after calling PauseTest will no longer be +// associated with this test. +func (b *packageBuilder) PauseTest(name string) { + b.output.SetActiveID(0) +} + +// ContinueTest finds the test with the given name and marks it as active. If +// more than one test exist with this name, the most recently created test will +// be used. +func (b *packageBuilder) ContinueTest(name string) { + id, _ := b.findTest(name) + b.output.SetActiveID(id) +} + +// EndTest finds the test with the given name, sets the result, duration and +// level. If more than one test exists with this name, the most recently +// created test will be used. If no test exists with this name, a new test is +// created. The test is then marked as no longer active. +func (b *packageBuilder) EndTest(name, result string, duration time.Duration, level int) { + id, ok := b.findTest(name) + if !ok { + // test did not exist, create one + // TODO: Likely reason is that the user ran go test without the -v + // flag, should we report this somewhere? + id = b.CreateTest(name) + } + + t := b.tests[id] + t.Result = parseResult(result) + t.Duration = duration + t.Level = level + b.tests[id] = t + b.output.SetActiveID(0) +} + +// End resets the active test. +func (b *packageBuilder) End() { + b.output.SetActiveID(0) +} + +// BenchmarkResult updates an existing or adds a new test with the given +// results and marks it as active. If an existing test with this name exists +// but without result, then that one is updated. Otherwise a new one is added +// to the report. +func (b *packageBuilder) BenchmarkResult(name string, iterations int64, nsPerOp, mbPerSec float64, bytesPerOp, allocsPerOp int64) { + id, ok := b.findTest(name) + if !ok || b.tests[id].Result != gtr.Unknown { + id = b.CreateTest(name) + } + b.output.SetActiveID(id) + + benchmark := Benchmark{iterations, nsPerOp, mbPerSec, bytesPerOp, allocsPerOp} + test := gtr.NewTest(id, name) + test.Result = gtr.Pass + test.Duration = benchmark.ApproximateDuration() + SetBenchmarkData(&test, benchmark) + b.tests[id] = test +} + +// Coverage sets the code coverage percentage. +func (b *packageBuilder) Coverage(pct float64, packages []string) { + b.coverage = pct +} + +// Output appends data to the output of this package. +func (b *packageBuilder) Output(data string) { + b.output.Append(data) +} + +// findTest returns the id of the most recently created test with the given +// name if it exists. +func (b *packageBuilder) findTest(name string) (int, bool) { + var maxid int + for id, test := range b.tests { + if maxid < id && test.Name == name { + maxid = id + } + } + return maxid, maxid > 0 +} + +// findTestParentID searches the existing tests in this package for a parent of +// the test with the given name, and returns its id if one is found. +func (b *packageBuilder) findTestParentID(name string) (int, bool) { + parent := dropLastSegment(name) + for parent != "" { + if id, ok := b.findTest(parent); ok { + return id, true + } + parent = dropLastSegment(parent) + } + return 0, false +} + +// isParent returns true if the test with the given id has sub tests. +func (b *packageBuilder) isParent(id int) bool { + _, ok := b.parentIDs[id] + return ok +} + +// dropLastSegment strips the last `/` and everything following it from the +// given name. If no `/` was found, the empty string is returned. +func dropLastSegment(name string) string { + if idx := strings.LastIndexByte(name, '/'); idx >= 0 { + return name[:idx] + } + return "" +} + +// containsFailures return true if this package contains at least one failing +// test or a test with an unknown result. +func (b *packageBuilder) containsFailures() bool { + for _, test := range b.tests { + if test.Result == gtr.Fail || test.Result == gtr.Unknown { + return true + } + } + return false +} diff --git a/parser/gotest/report_builder_test.go b/parser/gotest/report_builder_test.go index b73c64a..c89855b 100644 --- a/parser/gotest/report_builder_test.go +++ b/parser/gotest/report_builder_test.go @@ -125,6 +125,66 @@ func TestReport(t *testing.T) { } } +func TestBuildReportMultiplePackages(t *testing.T) { + events := []Event{ + {Package: "package/name1", Type: "run_test", Name: "TestOne"}, + {Package: "package/name2", Type: "run_test", Name: "TestOne"}, + {Package: "package/name1", Type: "output", Data: "\tHello"}, + {Package: "package/name1", Type: "end_test", Name: "TestOne", Result: "PASS", Duration: 1 * time.Millisecond}, + {Package: "package/name2", Type: "output", Data: "\tfile_test.go:10: error"}, + {Package: "package/name2", Type: "end_test", Name: "TestOne", Result: "FAIL", Duration: 1 * time.Millisecond}, + {Package: "package/name2", Type: "status", Result: "FAIL"}, + {Package: "package/name2", Type: "summary", Result: "FAIL", Name: "package/name2", Duration: 1 * time.Millisecond}, + {Package: "package/name1", Type: "status", Result: "PASS"}, + {Package: "package/name1", Type: "summary", Result: "ok", Name: "package/name1", Duration: 1 * time.Millisecond}, + } + + want := gtr.Report{ + Packages: []gtr.Package{ + { + Name: "package/name2", + Duration: 1 * time.Millisecond, + Timestamp: testTimestamp, + Tests: []gtr.Test{ + { + ID: 2, + Name: "TestOne", + Duration: 1 * time.Millisecond, + Result: gtr.Fail, + Output: []string{"\tfile_test.go:10: error"}, + Data: make(map[string]interface{}), + }, + }, + }, + { + Name: "package/name1", + Duration: 1 * time.Millisecond, + Timestamp: testTimestamp, + Tests: []gtr.Test{ + { + ID: 1, + Name: "TestOne", + Duration: 1 * time.Millisecond, + Result: gtr.Pass, + Output: []string{"\tHello"}, + Data: make(map[string]interface{}), + }, + }, + }, + }, + } + + rb := newReportBuilder() + rb.timestampFunc = testTimestampFunc + for _, ev := range events { + rb.ProcessEvent(ev) + } + got := rb.Build() + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("FromEvents report incorrect, diff (-want, +got):\n%v", diff) + } +} + func TestSubtestModes(t *testing.T) { events := []Event{ {Type: "run_test", Name: "TestParent"}, From 4d0ed8b68186bbeabfdc91f3fbe72626be5f69c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= Date: Thu, 11 Aug 2022 23:59:19 +0100 Subject: [PATCH 13/13] Add testdata for `go test -json -race` tests. Refs #134 --- testdata/113-race.gojson.txt | 14 ++++++++++++++ testdata/113-report.xml | 15 +++++++++++++++ testdata/src/race-json/pkg1/pkg1_test.go | 6 ++++++ testdata/src/race-json/pkg2/pkg2_test.go | 6 ++++++ 4 files changed, 41 insertions(+) create mode 100644 testdata/113-race.gojson.txt create mode 100644 testdata/113-report.xml create mode 100644 testdata/src/race-json/pkg1/pkg1_test.go create mode 100644 testdata/src/race-json/pkg2/pkg2_test.go diff --git a/testdata/113-race.gojson.txt b/testdata/113-race.gojson.txt new file mode 100644 index 0000000..a8b8359 --- /dev/null +++ b/testdata/113-race.gojson.txt @@ -0,0 +1,14 @@ +{"Time":"2022-07-17T22:24:20.065393432+01:00","Action":"run","Package":"package/race-json/pkg1","Test":"TestPkg1"} +{"Time":"2022-07-17T22:24:20.065595209+01:00","Action":"output","Package":"package/race-json/pkg1","Test":"TestPkg1","Output":"=== RUN TestPkg1\n"} +{"Time":"2022-07-17T22:24:20.065621488+01:00","Action":"output","Package":"package/race-json/pkg1","Test":"TestPkg1","Output":"--- PASS: TestPkg1 (0.00s)\n"} +{"Time":"2022-07-17T22:24:20.065633674+01:00","Action":"pass","Package":"package/race-json/pkg1","Test":"TestPkg1","Elapsed":0} +{"Time":"2022-07-17T22:24:20.065647242+01:00","Action":"output","Package":"package/race-json/pkg1","Output":"PASS\n"} +{"Time":"2022-07-17T22:24:20.065407525+01:00","Action":"run","Package":"package/race-json/pkg2","Test":"TestPkg2"} +{"Time":"2022-07-17T22:24:20.06568802+01:00","Action":"output","Package":"package/race-json/pkg2","Test":"TestPkg2","Output":"=== RUN TestPkg2\n"} +{"Time":"2022-07-17T22:24:20.065713342+01:00","Action":"output","Package":"package/race-json/pkg2","Test":"TestPkg2","Output":"--- PASS: TestPkg2 (0.00s)\n"} +{"Time":"2022-07-17T22:24:20.065725102+01:00","Action":"pass","Package":"package/race-json/pkg2","Test":"TestPkg2","Elapsed":0} +{"Time":"2022-07-17T22:24:20.065736721+01:00","Action":"output","Package":"package/race-json/pkg2","Output":"PASS\n"} +{"Time":"2022-07-17T22:24:20.06574776+01:00","Action":"output","Package":"package/race-json/pkg2","Output":"ok \tpackage/race-json/pkg2\t(cached)\n"} +{"Time":"2022-07-17T22:24:20.065763529+01:00","Action":"pass","Package":"package/race-json/pkg2","Elapsed":0} +{"Time":"2022-07-17T22:24:20.065657773+01:00","Action":"output","Package":"package/race-json/pkg1","Output":"ok \tpackage/race-json/pkg1\t(cached)\n"} +{"Time":"2022-07-17T22:24:20.065831715+01:00","Action":"pass","Package":"package/race-json/pkg1","Elapsed":0} diff --git a/testdata/113-report.xml b/testdata/113-report.xml new file mode 100644 index 0000000..d973826 --- /dev/null +++ b/testdata/113-report.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/testdata/src/race-json/pkg1/pkg1_test.go b/testdata/src/race-json/pkg1/pkg1_test.go new file mode 100644 index 0000000..53f4fe9 --- /dev/null +++ b/testdata/src/race-json/pkg1/pkg1_test.go @@ -0,0 +1,6 @@ +package pkg1 + +import "testing" + +func TestPkg1(t *testing.T) { +} diff --git a/testdata/src/race-json/pkg2/pkg2_test.go b/testdata/src/race-json/pkg2/pkg2_test.go new file mode 100644 index 0000000..9a399f6 --- /dev/null +++ b/testdata/src/race-json/pkg2/pkg2_test.go @@ -0,0 +1,6 @@ +package pkg2 + +import "testing" + +func TestPkg2(t *testing.T) { +}