testing: make failure in benchmark cause non-zero exit status

Moves the implementation of RunBenchmarks to a non-exported function
that returns whether the execution was OK, and uses that to identify
failure in benchmarks.The exported function is kept for compatibility.

Like before, benchmarks will only be executed if tests and examples
pass. The PASS message will not be printed if there was a failure in
a benchmark.

Example output

	BenchmarkThatCallsFatal-8	--- FAIL: BenchmarkThatCallsFatal-8
		x_test.go:6: called by benchmark
	FAIL
	exit status 1
	FAIL	_/.../src/cmd/go/testdata/src/benchfatal	0.009s

Fixes #14307.

Change-Id: I6f3ddadc7da8a250763168cc099ae8b325a79602
Reviewed-on: https://go-review.googlesource.com/19889
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index 928224cee69a8a5a3985b141fe981fa44f33b0b9..96b3f2d977c73d8ed3bb71364fe30c11b901d66e 100644 (file)
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -2801,3 +2801,11 @@ func TestGoGetUpdateAllDoesNotTryToLoadDuplicates(t *testing.T) {
        tg.run("get", "-u", ".../")
        tg.grepStderrNot("duplicate loads of", "did not remove old packages from cache")
 }
+
+func TestFatalInBenchmarkCauseNonZeroExitStatus(t *testing.T) {
+       tg := testgo(t)
+       defer tg.cleanup()
+       tg.runFail("test", "-bench", ".", "./testdata/src/benchfatal")
+       tg.grepBothNot("^ok", "test passed unexpectedly")
+       tg.grepBoth("FAIL.*benchfatal", "test did not run everything")
+}

diff --git a/src/cmd/go/testdata/src/benchfatal/x_test.go b/src/cmd/go/testdata/src/benchfatal/x_test.go
new file mode 100644 (file)
index 0000000..8d3a5de
--- /dev/null
+++ b/src/cmd/go/testdata/src/benchfatal/x_test.go
@@ -0,0 +1,7 @@
+package benchfatal
+
+import "testing"
+
+func BenchmarkThatCallsFatal(b *testing.B) {
+       b.Fatal("called by benchmark")
+}

diff --git a/src/testing/benchmark.go b/src/testing/benchmark.go
index 85178c2f867adddfe7c22d9e34004430eb6a2bba..39b8cff4d358898dd3f23d37e81d406bea183404 100644 (file)
--- a/src/testing/benchmark.go
+++ b/src/testing/benchmark.go
@@ -302,9 +302,13 @@ func benchmarkName(name string, n int) string {
 // An internal function but exported because it is cross-package; part of the implementation
 // of the "go test" command.
 func RunBenchmarks(matchString func(pat, str string) (bool, error), benchmarks []InternalBenchmark) {
+       runBenchmarksInternal(matchString, benchmarks)
+}
+
+func runBenchmarksInternal(matchString func(pat, str string) (bool, error), benchmarks []InternalBenchmark) bool {
        // If no flag was specified, don't run benchmarks.
        if len(*matchBenchmarks) == 0 {
-               return
+               return true
        }
        // Collect matching benchmarks and determine longest name.
        maxprocs := 1
@@ -329,6 +333,7 @@ func RunBenchmarks(matchString func(pat, str string) (bool, error), benchmarks [
                        }
                }
        }
+       ok := true
        for _, Benchmark := range bs {
                for _, procs := range cpuList {
                        runtime.GOMAXPROCS(procs)
@@ -342,6 +347,7 @@ func RunBenchmarks(matchString func(pat, str string) (bool, error), benchmarks [
                        fmt.Printf("%-*s\t", maxlen, benchName)
                        r := b.run()
                        if b.failed {
+                               ok = false
                                // The output could be very long here, but probably isn't.
                                // We print it all, regardless, because we don't want to trim the reason
                                // the benchmark failed.
@@ -364,6 +370,7 @@ func RunBenchmarks(matchString func(pat, str string) (bool, error), benchmarks [
                        }
                }
        }
+       return ok
 }
 
 // trimOutput shortens the output from a benchmark, which can be very long.

diff --git a/src/testing/testing.go b/src/testing/testing.go
index e4c4772fed2a8fc91cfaa5c1109257887a189b97..95182076ef11b0b8af39a96ca5465b57e3d627ec 100644 (file)
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -515,13 +515,12 @@ func (m *M) Run() int {
        testOk := RunTests(m.matchString, m.tests)
        exampleOk := RunExamples(m.matchString, m.examples)
        stopAlarm()
-       if !testOk || !exampleOk {
+       if !testOk || !exampleOk || !runBenchmarksInternal(m.matchString, m.benchmarks) {
                fmt.Println("FAIL")
                after()
                return 1
        }
        fmt.Println("PASS")
-       RunBenchmarks(m.matchString, m.benchmarks)
        after()
        return 0
 }