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] 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) }