parser/gotest,junit: Move benchmark grouping to parser/gotest package

Having multiple benchmarks for a single name is something specific to
the gotest output, so grouping benchmarks by name is not something that
belongs in the junit package.

junit/junit.go

@@ -160,7 +160,7 @@ func CreateFromReport(report gtr.Report, hostname string) Testsuites {
 			suite.AddTestcase(createTestcaseForTest(pkg.Name, test))
 		}
 
-		for _, bm := range groupBenchmarksByName(pkg.Benchmarks) {
+		for _, bm := range pkg.Benchmarks {
 			suite.AddTestcase(createTestcaseForBenchmark(pkg.Name, bm))
 		}
 
@@ -268,56 +268,3 @@ func formatBenchmarkTime(d time.Duration) string {
 func formatOutput(output []string, indent int) string {
 	return strings.Join(output, "\n")
 }
-
-func groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark {
-	if len(benchmarks) == 0 {
-		return nil
-	}
-
-	var grouped []gtr.Benchmark
-	benchmap := make(map[string][]gtr.Benchmark)
-	for _, bm := range benchmarks {
-		if _, ok := benchmap[bm.Name]; !ok {
-			grouped = append(grouped, gtr.Benchmark{Name: bm.Name})
-		}
-		benchmap[bm.Name] = append(benchmap[bm.Name], bm)
-	}
-
-	for i, bm := range grouped {
-		n := 0
-		for _, b := range benchmap[bm.Name] {
-			if b.Result != gtr.Pass {
-				continue
-			}
-			bm.Iterations += b.Iterations
-			bm.NsPerOp += b.NsPerOp
-			bm.MBPerSec += b.MBPerSec
-			bm.BytesPerOp += b.BytesPerOp
-			bm.AllocsPerOp += b.AllocsPerOp
-			n++
-		}
-
-		bm.Result = groupResults(benchmap[bm.Name])
-		if n > 0 {
-			bm.NsPerOp = bm.NsPerOp / float64(n)
-			bm.MBPerSec = bm.MBPerSec / float64(n)
-			bm.BytesPerOp = bm.BytesPerOp / int64(n)
-			bm.AllocsPerOp = bm.AllocsPerOp / int64(n)
-		}
-		grouped[i] = bm
-	}
-	return grouped
-}
-
-func groupResults(benchmarks []gtr.Benchmark) gtr.Result {
-	var result gtr.Result
-	for _, bm := range benchmarks {
-		if bm.Result == gtr.Fail {
-			return gtr.Fail
-		}
-		if result != gtr.Pass {
-			result = bm.Result
-		}
-	}
-	return result
-}

junit/junit_test.go

@@ -116,45 +116,3 @@ func properties(keyvals ...string) *[]Property {
 	}
 	return &props
 }
-
-func TestGroupBenchmarksByName(t *testing.T) {
-	tests := []struct {
-		in   []gtr.Benchmark
-		want []gtr.Benchmark
-	}{
-		{nil, nil},
-		{
-			[]gtr.Benchmark{{Name: "BenchmarkFailed", Result: gtr.Fail}},
-			[]gtr.Benchmark{{Name: "BenchmarkFailed", Result: gtr.Fail}},
-		},
-		{
-			[]gtr.Benchmark{
-				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 10, MBPerSec: 400, BytesPerOp: 1, AllocsPerOp: 2},
-				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 20, MBPerSec: 300, BytesPerOp: 1, AllocsPerOp: 4},
-				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 30, MBPerSec: 200, BytesPerOp: 1, AllocsPerOp: 8},
-				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 40, MBPerSec: 100, BytesPerOp: 5, AllocsPerOp: 2},
-			},
-			[]gtr.Benchmark{
-				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 4},
-			},
-		},
-		{
-			[]gtr.Benchmark{
-				{Name: "BenchmarkMixed", Result: gtr.Unknown},
-				{Name: "BenchmarkMixed", Result: gtr.Pass, NsPerOp: 10, MBPerSec: 400, BytesPerOp: 1, AllocsPerOp: 2},
-				{Name: "BenchmarkMixed", Result: gtr.Pass, NsPerOp: 40, MBPerSec: 100, BytesPerOp: 3, AllocsPerOp: 4},
-				{Name: "BenchmarkMixed", Result: gtr.Fail},
-			},
-			[]gtr.Benchmark{
-				{Name: "BenchmarkMixed", Result: gtr.Fail, NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 3},
-			},
-		},
-	}
-
-	for _, test := range tests {
-		got := groupBenchmarksByName(test.in)
-		if diff := cmp.Diff(test.want, got); diff != "" {
-			t.Errorf("groupBenchmarksByName result incorrect, diff (-want, +got):\n%s\n", diff)
-		}
-	}
-}

parser/gotest/report_builder.go

@@ -260,7 +260,7 @@ func (b *reportBuilder) CreatePackage(name, result string, duration time.Duratio
 	pkg.Coverage = b.coverage
 	pkg.Output = b.output
 	pkg.Tests = tests
-	pkg.Benchmarks = benchmarks
+	pkg.Benchmarks = groupBenchmarksByName(benchmarks)
 	b.packages = append(b.packages, pkg)
 
 	// reset state, except for nextID to ensure all id's are unique.
@@ -378,3 +378,56 @@ func parseResult(r string) gtr.Result {
 		return gtr.Unknown
 	}
 }
+
+func groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark {
+	if len(benchmarks) == 0 {
+		return nil
+	}
+
+	var grouped []gtr.Benchmark
+	byName := make(map[string][]gtr.Benchmark)
+	for _, bm := range benchmarks {
+		if _, ok := byName[bm.Name]; !ok {
+			grouped = append(grouped, gtr.Benchmark{Name: bm.Name})
+		}
+		byName[bm.Name] = append(byName[bm.Name], bm)
+	}
+
+	for i, group := range grouped {
+		count := 0
+		for _, bm := range byName[group.Name] {
+			if bm.Result != gtr.Pass {
+				continue
+			}
+			group.Iterations += bm.Iterations
+			group.NsPerOp += bm.NsPerOp
+			group.MBPerSec += bm.MBPerSec
+			group.BytesPerOp += bm.BytesPerOp
+			group.AllocsPerOp += bm.AllocsPerOp
+			count++
+		}
+
+		group.Result = groupResults(byName[group.Name])
+		if count > 0 {
+			group.NsPerOp /= float64(count)
+			group.MBPerSec /= float64(count)
+			group.BytesPerOp /= int64(count)
+			group.AllocsPerOp /= int64(count)
+		}
+		grouped[i] = group
+	}
+	return grouped
+}
+
+func groupResults(benchmarks []gtr.Benchmark) gtr.Result {
+	var result gtr.Result
+	for _, bm := range benchmarks {
+		if bm.Result == gtr.Fail {
+			return gtr.Fail
+		}
+		if result != gtr.Pass {
+			result = bm.Result
+		}
+	}
+	return result
+}

parser/gotest/report_builder_test.go

@@ -0,0 +1,51 @@
+package gotest
+
+import (
+	"testing"
+
+	"github.com/jstemmer/go-junit-report/v2/gtr"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestGroupBenchmarksByName(t *testing.T) {
+	tests := []struct {
+		in   []gtr.Benchmark
+		want []gtr.Benchmark
+	}{
+		{nil, nil},
+		{
+			[]gtr.Benchmark{{Name: "BenchmarkFailed", Result: gtr.Fail}},
+			[]gtr.Benchmark{{Name: "BenchmarkFailed", Result: gtr.Fail}},
+		},
+		{
+			[]gtr.Benchmark{
+				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 10, MBPerSec: 400, BytesPerOp: 1, AllocsPerOp: 2},
+				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 20, MBPerSec: 300, BytesPerOp: 1, AllocsPerOp: 4},
+				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 30, MBPerSec: 200, BytesPerOp: 1, AllocsPerOp: 8},
+				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 40, MBPerSec: 100, BytesPerOp: 5, AllocsPerOp: 2},
+			},
+			[]gtr.Benchmark{
+				{Name: "BenchmarkOne", Result: gtr.Pass, NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 4},
+			},
+		},
+		{
+			[]gtr.Benchmark{
+				{Name: "BenchmarkMixed", Result: gtr.Unknown},
+				{Name: "BenchmarkMixed", Result: gtr.Pass, NsPerOp: 10, MBPerSec: 400, BytesPerOp: 1, AllocsPerOp: 2},
+				{Name: "BenchmarkMixed", Result: gtr.Pass, NsPerOp: 40, MBPerSec: 100, BytesPerOp: 3, AllocsPerOp: 4},
+				{Name: "BenchmarkMixed", Result: gtr.Fail},
+			},
+			[]gtr.Benchmark{
+				{Name: "BenchmarkMixed", Result: gtr.Fail, NsPerOp: 25, MBPerSec: 250, BytesPerOp: 2, AllocsPerOp: 3},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		got := groupBenchmarksByName(test.in)
+		if diff := cmp.Diff(test.want, got); diff != "" {
+			t.Errorf("groupBenchmarksByName result incorrect, diff (-want, +got):\n%s\n", diff)
+		}
+	}
+}