From 34caac8501763e3ea700ee019bf2c29671a30c55 Mon Sep 17 00:00:00 2001
From: "Paul E. Murphy" <murp@ibm.com>
Date: Thu, 4 Apr 2024 16:06:16 -0500
Subject: [PATCH] benchseries: do not crash if no denominator is present

On ppc64le, I am running a slightly modified instance of x/build perf
to monitor performance between the latest release toolchain and
upstream.

I started to glob run the crypto benchmarks this release, and
unsuprisingly some were added during development. This caused a
failure to ingest new results when syncing with influx.

fixes golang/go#66685

Change-Id: Id5145b779f48afa2797a861d047d9d46a263b229
Reviewed-on: https://go-review.googlesource.com/c/perf/+/576695
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
---
 benchseries/benchseries.go      |  6 ++--
 benchseries/benchseries_test.go | 60 +++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/benchseries/benchseries.go b/benchseries/benchseries.go
index d40c827..d284a91 100644
--- a/benchseries/benchseries.go
+++ b/benchseries/benchseries.go
@@ -925,9 +925,11 @@ func (cs *ComparisonSeries) AddSummaries(confidence float64, N int) {
 				sum := c.Summary
 				if sum == nil || (!sum.Present && sum.comparison == nil) {
 					sum = &ComparisonSummary{comparison: c, Date: c.Date}
-					sum.Center, sum.Low, sum.High = fn(c)
-					sum.Present = true
 					c.Summary = sum
+					sum.Present = c.Denominator != nil
+					if sum.Present {
+						sum.Center, sum.Low, sum.High = fn(c)
+					}
 				}
 				row = append(row, sum)
 			} else {
diff --git a/benchseries/benchseries_test.go b/benchseries/benchseries_test.go
index d65b53f..369094a 100644
--- a/benchseries/benchseries_test.go
+++ b/benchseries/benchseries_test.go
@@ -187,3 +187,63 @@ func TestMissingCompare(t *testing.T) {
 		t.Errorf("len(Series) got %d want 0; Series: %+v", len(got.Series), got.Series)
 	}
 }
+
+// A test point with no denominator.
+func TestMissingDenominator(t *testing.T) {
+	results := []*benchfmt.Result{
+		{
+			Config: []benchfmt.Config{
+				makeConfig("compare", "numerator"),
+				makeConfig("runstamp", "2020-01-01T00:00:00Z"),
+				makeConfig("numerator-hash", "abcdef0123456789"),
+				makeConfig("denominator-hash", "9876543219fedcba"),
+				makeConfig("numerator-hash-time", "2020-02-02T00:00:00Z"),
+				makeConfig("residue", "foo"),
+			},
+			Name:  []byte("Foo"),
+			Iters: 10,
+			Values: []benchfmt.Value{
+				{Value: 10, Unit: "sec"},
+			},
+		},
+	}
+
+	opts := &BuilderOptions{
+		Filter:          ".unit:/.*/",
+		Series:          "numerator-hash-time",
+		Table:           "", // .unit only
+		Experiment:      "runstamp",
+		Compare:         "compare",
+		Numerator:       "numerator",
+		Denominator:     "denominator",
+		NumeratorHash:   "numerator-hash",
+		DenominatorHash: "denominator-hash",
+		Warn: func(format string, args ...interface{}) {
+			s := fmt.Sprintf(format, args...)
+			t.Errorf("benchseries warning: %s", s)
+		},
+	}
+	builder, err := NewBuilder(opts)
+	if err != nil {
+		t.Fatalf("NewBuilder get err %v, want err nil", err)
+	}
+
+	for _, r := range results {
+		builder.Add(r)
+	}
+
+	comparisons, err := builder.AllComparisonSeries(nil, DUPE_REPLACE)
+	if err != nil {
+		t.Fatalf("AllComparisonSeries got err %v want err nil", err)
+	}
+
+	if len(comparisons) != 1 {
+		t.Fatalf("len(comparisons) got %d want 1; comparisons: %+v", len(comparisons), comparisons)
+	}
+
+	got := comparisons[0]
+	got.AddSummaries(0.95, 1000)
+	if len(got.Summaries) != 1 {
+		t.Fatalf("Expect 1 comparison, got: %+v", len(got.Summaries))
+	}
+}