diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index fb011d4c03f18f2ed50938c86372cf27789eb0f3..f4ce355189c0b18452b88f4cd544e237684e7c3b 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 diff --git a/src/cmd/go/testdata/script/test_exit.txt b/src/cmd/go/testdata/script/test_exit.txt new file mode 100644 index 0000000000000000000000000000000000000000..2ab3d59b271032a3541d499059d3ac21782eacdc --- /dev/null +++ b/src/cmd/go/testdata/script/test_exit.txt @@ -0,0 +1,152 @@ +env GO111MODULE=on + +# If a test exits with a zero status code, 'go test' prints its own error +# message and fails. +! go test ./zero +! stdout ^ok +! stdout 'exit status' +stdout 'did not print anything' +stdout ^FAIL + +# If a test exits with a non-zero status code, 'go test' fails normally. +! go test ./one +! stdout ^ok +stdout 'exit status' +! stdout 'did not print anything' +stdout ^FAIL + +# Ensure that other flags still do the right thing. +go test -list=. ./zero +stdout ExitZero + +! go test -bench=. ./zero +stdout 'did not print anything' + +# 'go test' with no args streams output without buffering. Ensure that it still +# catches a zero exit with missing output. +cd zero +! go test +stdout 'did not print anything' +cd ../normal +go test +stdout ^ok +cd .. + +# If a TestMain prints something and exits with a zero status code, 'go test' +# shouldn't complain about that. It's a common way to skip testing a package +# entirely. +go test ./main_zero_warning +! stdout 'skipping all tests' +stdout ^ok + +# With -v, we'll see the warning from TestMain. +go test -v ./main_zero_warning +stdout 'skipping all tests' +stdout ^ok + +# Listing all tests won't actually give a result if TestMain exits. That's okay, +# because this is how TestMain works. If we decide to support -list even when +# TestMain is used to skip entire packages, we can change this test case. +go test -list=. ./main_zero_warning +stdout 'skipping all tests' +! stdout TestNotListed + +# If a TestMain prints nothing and exits with a zero status code, 'go test' +# should fail. +! go test ./main_zero_nowarning +stdout 'did not print anything' + +# A test that simply prints "PASS" and exits with a zero status code shouldn't +# be OK, but we don't catch that at the moment. It's hard to tell if any test +# started but didn't finish without using -test.v. +go test ./fake_pass +stdout ^ok + +-- go.mod -- +module m + +-- ./normal/normal.go -- +package normal +-- ./normal/normal_test.go -- +package normal + +import "testing" + +func TestExitZero(t *testing.T) { +} + +-- ./zero/zero.go -- +package zero +-- ./zero/zero_test.go -- +package zero + +import ( + "os" + "testing" +) + +func TestExitZero(t *testing.T) { + os.Exit(0) +} + +-- ./one/one.go -- +package one +-- ./one/one_test.go -- +package one + +import ( + "os" + "testing" +) + +func TestExitOne(t *testing.T) { + os.Exit(1) +} + +-- ./main_zero_warning/zero.go -- +package zero +-- ./main_zero_warning/zero_test.go -- +package zero + +import ( + "fmt" + "os" + "testing" +) + +func TestMain(m *testing.M) { + fmt.Println("skipping all tests") + os.Exit(0) +} + +func TestNotListed(t *testing.T) {} + +-- ./main_zero_nowarning/zero.go -- +package zero +-- ./main_zero_nowarning/zero_test.go -- +package zero + +import ( + "os" + "testing" +) + +func TestMain(m *testing.M) { + os.Exit(0) +} + +-- ./fake_pass/fake.go -- +package fake +-- ./fake_pass/fake_test.go -- +package fake + +import ( + "fmt" + "os" + "testing" +) + +func TestFakePass(t *testing.T) { + fmt.Println("PASS") + os.Exit(0) +} diff --git a/src/cmd/internal/goobj/goobj_test.go b/src/cmd/internal/goobj/goobj_test.go index 4a4d35a413a533ede04d77cd877908865e8ecc36..4658e7d6716d89e6f1911fd5f727d75643d76216 100644 --- a/src/cmd/internal/goobj/goobj_test.go +++ b/src/cmd/internal/goobj/goobj_test.go @@ -29,9 +29,7 @@ var ( ) func TestMain(m *testing.M) { - if !testenv.HasGoBuild() { - return - } + testenv.MainMust(testenv.HasGoBuild) if err := buildGoobj(); err != nil { fmt.Println(err) diff --git a/src/cmd/nm/nm_test.go b/src/cmd/nm/nm_test.go index dcd9f360050ceb06bac399955fff4e6677c683a3..018252793e015ac361e8d183654b29a247043606 100644 --- a/src/cmd/nm/nm_test.go +++ b/src/cmd/nm/nm_test.go @@ -26,9 +26,7 @@ func TestMain(m *testing.M) { } func testMain(m *testing.M) int { - if !testenv.HasGoBuild() { - return 0 - } + testenv.MainMust(testenv.HasGoBuild) tmpDir, err := ioutil.TempDir("", "TestNM") if err != nil { diff --git a/src/cmd/objdump/objdump_test.go b/src/cmd/objdump/objdump_test.go index b24371ddea567b04b2aeec3afb8fcdb8c3669c53..e4ae9babcbb152247c23724973de04aa91561264 100644 --- a/src/cmd/objdump/objdump_test.go +++ b/src/cmd/objdump/objdump_test.go @@ -22,9 +22,7 @@ import ( var tmp, exe string // populated by buildObjdump func TestMain(m *testing.M) { - if !testenv.HasGoBuild() { - return - } + testenv.MainMust(testenv.HasGoBuild) var exitcode int if err := buildObjdump(); err == nil { diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 6443094515da8222ac0d300d14120722ed5ca646..30edc38f480aed17b88192a1ed2139f2b7639262 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -202,7 +202,7 @@ var pkgDeps = map[string][]string{ "testing": {"L2", "flag", "fmt", "internal/race", "os", "runtime/debug", "runtime/pprof", "runtime/trace", "time"}, "testing/iotest": {"L2", "log"}, "testing/quick": {"L2", "flag", "fmt", "reflect", "time"}, - "internal/testenv": {"L2", "OS", "flag", "testing", "syscall", "internal/cfg"}, + "internal/testenv": {"L2", "OS", "flag", "fmt", "testing", "syscall", "internal/cfg"}, "internal/lazyregexp": {"L2", "OS", "regexp"}, "internal/lazytemplate": {"L2", "OS", "text/template"}, diff --git a/src/internal/testenv/testenv.go b/src/internal/testenv/testenv.go index b036aa6ebc78ce976e8735634e639cc81775ba09..5cb132760e5d24e4e0ec01de7486823adf3bee70 100644 --- a/src/internal/testenv/testenv.go +++ b/src/internal/testenv/testenv.go @@ -13,6 +13,7 @@ package testenv import ( "errors" "flag" + "fmt" "internal/cfg" "os" "os/exec" @@ -32,6 +33,14 @@ func Builder() string { return os.Getenv("GO_BUILDER_NAME") } +func MainMust(cond func() bool) { + if !cond() { + fmt.Println("testenv: warning: can't run any tests") + fmt.Println("SKIP") + os.Exit(0) + } +} + // HasGoBuild reports whether the current system can build programs with ``go build'' // and then run them with os.StartProcess or exec.Command. func HasGoBuild() bool {