From 969fc17b96a99d4ffab4671573144a7e8265af12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=ABl=20Stemmer?= <stemmertech@gmail.com>
Date: Fri, 1 Apr 2022 00:29:54 +0100
Subject: [PATCH] junit: Improve benchmark grouping and add tests

---
 pkg/junit/junit.go      | 40 +++++++++++++++++++++++++++++++--------
 pkg/junit/junit_test.go | 42 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/pkg/junit/junit.go b/pkg/junit/junit.go
index 557e09e..dfcdfc7 100644
--- a/pkg/junit/junit.go
+++ b/pkg/junit/junit.go
@@ -265,8 +265,11 @@ func formatOutput(output []string, indent int) string {
 }
 
 func groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark {
-	var grouped []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 {
@@ -276,19 +279,40 @@ func groupBenchmarksByName(benchmarks []gtr.Benchmark) []gtr.Benchmark {
 	}
 
 	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++
 		}
-		n := len(benchmap[bm.Name])
-		grouped[i] = benchmap[bm.Name][0]
-		grouped[i].NsPerOp = bm.NsPerOp / float64(n)
-		grouped[i].MBPerSec = bm.MBPerSec / float64(n)
-		grouped[i].BytesPerOp = bm.BytesPerOp / int64(n)
-		grouped[i].AllocsPerOp = bm.AllocsPerOp / int64(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
+}
diff --git a/pkg/junit/junit_test.go b/pkg/junit/junit_test.go
index dfb2f2a..9700374 100644
--- a/pkg/junit/junit_test.go
+++ b/pkg/junit/junit_test.go
@@ -115,3 +115,45 @@ 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(got, test.want); diff != "" {
+			t.Errorf("groupBenchmarksByName result incorrect, diff (-want, +got):\n%s\n", diff)
+		}
+	}
+}