Skip to content

Commit 2af321a

Browse files
committed
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.
1 parent b73e4a9 commit 2af321a

File tree

2 files changed

+14
-8
lines changed

2 files changed

+14
-8
lines changed

parser/gotest/report_builder.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio
258258
continue
259259
}
260260
}
261-
tests = b.groupBenchmarksByName(tests)
261+
tests = groupBenchmarksByName(tests, b.output)
262262

263263
pkg.Coverage = b.coverage
264264
pkg.Output = b.output.Get(globalID)
@@ -345,7 +345,7 @@ func parseResult(r string) gtr.Result {
345345
}
346346
}
347347

348-
func (b *reportBuilder) groupBenchmarksByName(tests []gtr.Test) []gtr.Test {
348+
func groupBenchmarksByName(tests []gtr.Test, output *collector.Output) []gtr.Test {
349349
if len(tests) == 0 {
350350
return nil
351351
}
@@ -392,7 +392,7 @@ func (b *reportBuilder) groupBenchmarksByName(tests []gtr.Test) []gtr.Test {
392392

393393
group.Duration = combinedDuration(byName[group.Name])
394394
group.Result = groupResults(byName[group.Name])
395-
group.Output = b.output.GetAll(ids...)
395+
group.Output = output.GetAll(ids...)
396396
if count > 0 {
397397
total.Iterations /= int64(count)
398398
total.NsPerOp /= float64(count)

parser/gotest/report_builder_test.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package gotest
22

33
import (
4+
"fmt"
45
"testing"
56
"time"
67

78
"github.com/jstemmer/go-junit-report/v2/gtr"
9+
"github.com/jstemmer/go-junit-report/v2/parser/gotest/internal/collector"
810

911
"github.com/google/go-cmp/cmp"
1012
)
@@ -236,6 +238,11 @@ func TestSubtestModes(t *testing.T) {
236238
}
237239

238240
func TestGroupBenchmarksByName(t *testing.T) {
241+
output := collector.New()
242+
for i := 1; i <= 4; i++ {
243+
output.AppendToID(i, fmt.Sprintf("output-%d", i))
244+
}
245+
239246
tests := []struct {
240247
name string
241248
in []gtr.Test
@@ -245,7 +252,7 @@ func TestGroupBenchmarksByName(t *testing.T) {
245252
{
246253
"one failing benchmark",
247254
[]gtr.Test{{ID: 1, Name: "BenchmarkFailed", Result: gtr.Fail, Data: map[string]interface{}{}}},
248-
[]gtr.Test{{ID: 1, Name: "BenchmarkFailed", Result: gtr.Fail, Data: map[string]interface{}{}}},
255+
[]gtr.Test{{ID: 1, Name: "BenchmarkFailed", Result: gtr.Fail, Output: []string{"output-1"}, Data: map[string]interface{}{}}},
249256
},
250257
{
251258
"four passing benchmarks",
@@ -256,7 +263,7 @@ func TestGroupBenchmarksByName(t *testing.T) {
256263
{ID: 4, Name: "BenchmarkOne", Result: gtr.Pass, Data: map[string]interface{}{key: Benchmark{NsPerOp: 40, MBPerSec: 100, BytesPerOp: 5, AllocsPerOp: 2}}},
257264
},
258265
[]gtr.Test{
259-
{ID: 1, Name: "BenchmarkOne", Result: gtr.Pass, Data: map[string]interface{}{key: Benchmark{NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 4}}},
266+
{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}}},
260267
},
261268
},
262269
{
@@ -268,15 +275,14 @@ func TestGroupBenchmarksByName(t *testing.T) {
268275
{ID: 4, Name: "BenchmarkMixed", Result: gtr.Fail},
269276
},
270277
[]gtr.Test{
271-
{ID: 1, Name: "BenchmarkMixed", Result: gtr.Fail, Data: map[string]interface{}{key: Benchmark{NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 3}}},
278+
{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}}},
272279
},
273280
},
274281
}
275282

276283
for _, test := range tests {
277284
t.Run(test.name, func(t *testing.T) {
278-
b := newReportBuilder()
279-
got := b.groupBenchmarksByName(test.in)
285+
got := groupBenchmarksByName(test.in, output)
280286
if diff := cmp.Diff(test.want, got); diff != "" {
281287
t.Errorf("groupBenchmarksByName result incorrect, diff (-want, +got):\n%s\n", diff)
282288
}

0 commit comments

Comments
 (0)