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/19889Reviewed-by: Ian Lance Taylor <iant@golang.org>

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")
+}

src/cmd/go/testdata/src/benchfatal/x_test.go

+package benchfatal
+
+import "testing"
+
+func BenchmarkThatCallsFatal(b *testing.B) {
+	b.Fatal("called by benchmark")
+}

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.

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
 }