cmd/go: fail if a test binary exits with no output

For example, if a test calls os.Exit(0), that could trick a 'go test'
run into not running some of the other tests, and thinking that they all
succeeded. This can easily go unnoticed and cause developers headaches.

Add a simple sanity check as part of 'go test': if the test binary
succeeds and doesn't print anything, we should error, as something
clearly went very wrong.

This is done by inspecting each of the stdout writes from the spawned
process, since we don't want to read the entirety of the output into a
buffer. We need to introduce a "buffered" bool var, as there's now an
io.Writer layer between cmd.Stdout and &buf.

A few TestMain funcs in the standard library needed fixing, as they
returned without printing anything as a means to skip testing the entire
package. For that purpose add testenv.MainMust, which prints a warning
and prints SKIP, similar to when -run matches no tests.

Finally, add tests for both os.Exit(0) and os.Exit(1), both as part of
TestMain and as part of a single test, and test that the various stdout
modes still do the right thing.

Fixes #29062.

Change-Id: Ic6f8ef3387dfc64e4cd3e8f903d7ca5f5f38d397
Reviewed-on: https://go-review.googlesource.com/c/go/+/184457
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

1 files changed, 35 insertions, 2 deletions

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index fb011d4c03..f4ce355189 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1048,6 +1048,18 @@ func (lockedStdout) Write(b []byte) (int, error) {
 	return os.Stdout.Write(b)
 }
 
+type outputChecker struct {
+	w         io.Writer
+	anyOutput bool
+}
+
+func (o *outputChecker) Write(p []byte) (int, error) {
+	if !o.anyOutput && len(bytes.TrimSpace(p)) > 0 {
+		o.anyOutput = true
+	}
+	return o.w.Write(p)
+}
+
 // builderRunTest is the action for running a test binary.
 func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
 	if a.Failed {
@@ -1067,6 +1079,7 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
 	}
 
 	var buf bytes.Buffer
+	buffered := false
 	if len(pkgArgs) == 0 || testBench {
 		// Stream test output (no buffering) when no package has
 		// been given on the command line (implicit current directory)
@@ -1093,9 +1106,16 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
 			stdout = io.MultiWriter(stdout, &buf)
 		} else {
 			stdout = &buf
+			buffered = true
 		}
 	}
 
+	// Keep track of whether we've seen any output at all. This is useful
+	// later, to avoid succeeding if the test binary did nothing or didn't
+	// reach the end of testing.M.Run.
+	outCheck := outputChecker{w: stdout}
+	stdout = &outCheck
+
 	if c.buf == nil {
 		// We did not find a cached result using the link step action ID,
 		// so we ran the link step. Try again now with the link output
@@ -1109,7 +1129,7 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
 		c.tryCacheWithID(b, a, a.Deps[0].BuildContentID())
 	}
 	if c.buf != nil {
-		if stdout != &buf {
+		if !buffered {
 			stdout.Write(c.buf.Bytes())
 			c.buf.Reset()
 		}
@@ -1207,6 +1227,19 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
 
 	mergeCoverProfile(cmd.Stdout, a.Objdir+"_cover_.out")
 
+	if err == nil && !testList && !outCheck.anyOutput {
+		// If a test does os.Exit(0) by accident, 'go test' may succeed
+		// and it can take a while for a human to notice the package's
+		// tests didn't actually pass.
+		//
+		// If a test binary ran without error, it should have at least
+		// printed something, such as a PASS line.
+		//
+		// The only exceptions are when no tests have run, and the
+		// -test.list flag, which just prints the names of tests
+		// matching a pattern.
+		err = fmt.Errorf("test binary succeeded but did not print anything")
+	}
 	if err == nil {
 		norun := ""
 		if !testShowPass && !testJSON {
@@ -1227,7 +1260,7 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
 		fmt.Fprintf(cmd.Stdout, "FAIL\t%s\t%s\n", a.Package.ImportPath, t)
 	}
 
-	if cmd.Stdout != &buf {
+	if !buffered {
 		buf.Reset() // cmd.Stdout was going to os.Stdout already
 	}
 	return nil