From 3fd27c2392831a268043c8e09d4599e7f1c441d3 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Thu, 22 Dec 2022 11:55:57 -0500 Subject: [PATCH] benchseries: return error on time parse error AllComparisonSeries shouldn't panic on bad input, it should return an error. For golang/go#53538. Change-Id: I988b546ea3ad5bb9026956b3aa4feec809915b73 Reviewed-on: https://go-review.googlesource.com/c/perf/+/459177 Auto-Submit: Michael Pratt Reviewed-by: David Chase Run-TryBot: Michael Pratt Reviewed-by: Michael Knyszek TryBot-Result: Gopher Robot --- benchseries/benchseries.go | 22 ++++++++++++++-------- cmd/benchseries/main.go | 5 ++++- cmd/benchseries/main_test.go | 5 ++++- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/benchseries/benchseries.go b/benchseries/benchseries.go index 8b7f110..cdf1e30 100644 --- a/benchseries/benchseries.go +++ b/benchseries/benchseries.go @@ -197,17 +197,17 @@ const RFC3339NanoNoZ = "2006-01-02T15:04:05.999999999-07:00" // NormalizeDateString converts dates in two formats used in bent/benchmarking // into UTC, so that all sort properly into a single order with no confusion. -func NormalizeDateString(in string) string { +func NormalizeDateString(in string) (string, error) { if noPuncDate.MatchString(in) { //20211229T213212 //2021-12-29T21:32:12 in = in[0:4] + "-" + in[4:6] + "-" + in[6:11] + ":" + in[11:13] + ":" + in[13:15] + "+00:00" } t, err := time.Parse(time.RFC3339Nano, in) - if err == nil { - return t.UTC().Format(RFC3339NanoNoZ) + if err != nil { + return "", err } - panic(err) + return t.UTC().Format(RFC3339NanoNoZ), nil } // ParseNormalizedDateString parses a time in the format returned by @@ -430,7 +430,7 @@ const ( // sensible order; this deals with that, including overlaps (depend on flag, either replaces old with // younger or combines, REPLACE IS PREFERRED and works properly with combining old summary data with // fresh benchmarking data) and possibly also with previously processed summaries. -func (b *Builder) AllComparisonSeries(existing []*ComparisonSeries, dupeHow int) []*ComparisonSeries { +func (b *Builder) AllComparisonSeries(existing []*ComparisonSeries, dupeHow int) ([]*ComparisonSeries, error) { old := make(map[string]*ComparisonSeries) for _, cs := range existing { old[cs.Unit] = cs @@ -484,13 +484,19 @@ func (b *Builder) AllComparisonSeries(existing []*ComparisonSeries, dupeHow int) for tk, tr := range t.cells { // tk == bench, experiment, tr == baseline, tests, tests == map hash -> cell. bench := tk.Benchmark - dateString := NormalizeDateString(tk.Experiment.StringValues()) + dateString, err := NormalizeDateString(tk.Experiment.StringValues()) + if err != nil { + return nil, fmt.Errorf("error parsing experiment date %q: %w", tk.Experiment.StringValues(), err) + } benchString := bench.StringValues() benches[benchString] = struct{}{} for hash, cell := range tr.tests { hashString := hash.StringValues() ser := b.hashToOrder[hash] - serString := NormalizeDateString(ser.StringValues()) + serString, err := NormalizeDateString(ser.StringValues()) + if err != nil { + return nil, fmt.Errorf("error parsing series date %q: %w", ser.StringValues(), err) + } sers[serString] = struct{}{} sk := SeriesKey{ Benchmark: benchString, @@ -637,7 +643,7 @@ func (b *Builder) AllComparisonSeries(existing []*ComparisonSeries, dupeHow int) } } - return css + return css, nil } func sortStringSet(m map[string]struct{}) []string { diff --git a/cmd/benchseries/main.go b/cmd/benchseries/main.go index 2337e50..dab8304 100644 --- a/cmd/benchseries/main.go +++ b/cmd/benchseries/main.go @@ -98,7 +98,10 @@ func main() { } // Rearrange into comparisons (not yet doing the statistical work) - comparisons = seriesBuilder.AllComparisonSeries(comparisons, benchseries.DUPE_REPLACE) + comparisons, err = seriesBuilder.AllComparisonSeries(comparisons, benchseries.DUPE_REPLACE) + if err != nil { + fail("Error building comparison series: %v", err) + } // Chat about residues, per-table for _, t := range comparisons { diff --git a/cmd/benchseries/main_test.go b/cmd/benchseries/main_test.go index 0319404..d390108 100644 --- a/cmd/benchseries/main_test.go +++ b/cmd/benchseries/main_test.go @@ -138,7 +138,10 @@ func combineFilesAndJson(t *testing.T, files []string, jsonFile string) []*bench f.Close() } - comparisons = builder.AllComparisonSeries(comparisons, benchseries.DUPE_REPLACE) + comparisons, err = builder.AllComparisonSeries(comparisons, benchseries.DUPE_REPLACE) + if err != nil { + t.Fatalf("AllComparisonSeries(%+v) got err %v want nil", comparisons, err) + } for _, c := range comparisons { c.AddSummaries(0.95, 500) // 500 is faster than 1000.