From b7f8c2a11058fe266fa5ddd0bc80dbf69b03b172 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Thu, 20 Mar 2025 10:26:54 -0400 Subject: testing: detect early return from B.Loop Currently, if a benchmark function returns prior to B.Loop() returning false, we'll report a bogus result. While there was no way to detect this with b.N-style benchmarks, one way b.Loop()-style benchmarks are more robust is that we *can* detect it. This CL adds a flag to B that tracks if B.Loop() has finished and checks it after the benchmark completes. If there was an early exit (not caused by another error), it reports a B.Error. Fixes #72933. Updates #72971. Change-Id: I731c1350e6df938c0ffa08fcedc11dc147e78854 Reviewed-on: https://go-review.googlesource.com/c/go/+/659656 LUCI-TryBot-Result: Go LUCI Auto-Submit: Austin Clements Reviewed-by: Michael Pratt Reviewed-by: Junyang Shao --- src/testing/loop_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) (limited to 'src/testing/loop_test.go') diff --git a/src/testing/loop_test.go b/src/testing/loop_test.go index 7a42919643..423094fbbd 100644 --- a/src/testing/loop_test.go +++ b/src/testing/loop_test.go @@ -4,6 +4,13 @@ package testing +import ( + "bytes" + "strings" +) + +// See also TestBenchmarkBLoop* in other files. + func TestBenchmarkBLoop(t *T) { var initialStart highPrecisionTime var firstStart highPrecisionTime @@ -68,4 +75,57 @@ func TestBenchmarkBLoop(t *T) { } } -// See also TestBenchmarkBLoop* in other files. +func TestBenchmarkBLoopBreak(t *T) { + var bState *B + var bLog bytes.Buffer + bRet := Benchmark(func(b *B) { + // The Benchmark function provides no access to the failure state and + // discards the log, so capture the B and save its log. + bState = b + b.common.w = &bLog + + for i := 0; b.Loop(); i++ { + if i == 2 { + break + } + } + }) + if !bState.failed { + t.Errorf("benchmark should have failed") + } + const wantLog = "benchmark function returned without B.Loop" + if log := bLog.String(); !strings.Contains(log, wantLog) { + t.Errorf("missing error %q in output:\n%s", wantLog, log) + } + // A benchmark that exits early should not report its target iteration count + // because it's not meaningful. + if bRet.N != 0 { + t.Errorf("want N == 0, got %d", bRet.N) + } +} + +func TestBenchmarkBLoopError(t *T) { + // Test that a benchmark that exits early because of an error doesn't *also* + // complain that the benchmark exited early. + var bState *B + var bLog bytes.Buffer + bRet := Benchmark(func(b *B) { + bState = b + b.common.w = &bLog + + for i := 0; b.Loop(); i++ { + b.Error("error") + return + } + }) + if !bState.failed { + t.Errorf("benchmark should have failed") + } + const noWantLog = "benchmark function returned without B.Loop" + if log := bLog.String(); strings.Contains(log, noWantLog) { + t.Errorf("unexpected error %q in output:\n%s", noWantLog, log) + } + if bRet.N != 0 { + t.Errorf("want N == 0, got %d", bRet.N) + } +} -- cgit v1.3