cmd/go: make go test -json report failures for panicking/exiting tests

'go test -json' should report that a test failed if the test binary
did not exit normally with status 0. This covers panics, non-zero
exits, and abnormal terminations.

These tests don't print a final result when run with -test.v (which is
used by 'go test -json'). The final result should be "PASS" or "FAIL"
on a line by itself. 'go test' prints "FAIL" in this case, but
includes error information.

test2json was changed in CL 192104 to report that a test passed if it
does not report a final status. This caused 'go test -json' to report
that a test passed after a panic or non-zero exit.

With this change, test2json treats "FAIL" with error information the
same as "FAIL" on a line by itself. This is intended to be a minimal
fix for backporting, but it will likely be replaced by a complete
solution for #29062.

Fixes #37555
Updates #29062
Updates #31969

Change-Id: Icb67bcd36bed97e6a8d51f4d14bf71f73c83ac3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/222243
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 1c6fb0b97fb1c4ecc8e7946f7090b455d83adda6..dbb899219dc195caa1c0a1888a30f653802ec68b 100644 (file)
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1239,6 +1239,14 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
                if len(out) == 0 {
                        fmt.Fprintf(cmd.Stdout, "%s\n", err)
                }
+               // NOTE(golang.org/issue/37555): test2json reports that a test passes
+               // unless "FAIL" is printed at the beginning of a line. The test may not
+               // actually print that if it panics, exits, or terminates abnormally,
+               // so we print it here. We can't always check whether it was printed
+               // because some tests need stdout to be a terminal (golang.org/issue/34791),
+               // not a pipe.
+               // TODO(golang.org/issue/29062): tests that exit with status 0 without
+               // printing a final result should fail.
                fmt.Fprintf(cmd.Stdout, "FAIL\t%s\t%s\n", a.Package.ImportPath, t)
        }
 

diff --git a/src/cmd/go/testdata/script/test_json_panic_exit.txt b/src/cmd/go/testdata/script/test_json_panic_exit.txt
new file mode 100644 (file)
index 0000000..d0a7991
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_json_panic_exit.txt
@@ -0,0 +1,69 @@
+# Verifies golang.org/issue/37555.
+
+[short] skip
+
+# 'go test -json' should say a test passes if it says it passes.
+go test -json ./pass
+stdout '"Action":"pass".*\n\z'
+! stdout '"Test":.*\n\z'
+
+# 'go test -json' should say a test passes if it exits 0 and prints nothing.
+# TODO(golang.org/issue/29062): this should fail in the future.
+go test -json ./exit0main
+stdout '"Action":"pass".*\n\z'
+! stdout '"Test":.*\n\z'
+
+# 'go test -json' should say a test fails if it exits 1 and prints nothing.
+! go test -json ./exit1main
+stdout '"Action":"fail".*\n\z'
+! stdout '"Test":.*\n\z'
+
+# 'go test -json' should say a test fails if it panics.
+! go test -json ./panic
+stdout '"Action":"fail".*\n\z'
+! stdout '"Test":.*\n\z'
+
+-- go.mod --
+module example.com/test
+
+go 1.14
+
+-- pass/pass_test.go --
+package pass_test
+
+import "testing"
+
+func TestPass(t *testing.T) {}
+
+-- exit0main/exit0main_test.go --
+package exit0_test
+
+import (
+       "os"
+       "testing"
+)
+
+func TestMain(m *testing.M) {
+       os.Exit(0)
+}
+
+-- exit1main/exit1main_test.go --
+package exit1_test
+
+import (
+       "os"
+       "testing"
+)
+
+func TestMain(m *testing.M) {
+       os.Exit(1)
+}
+
+-- panic/panic_test.go --
+package panic_test
+
+import "testing"
+
+func TestPanic(t *testing.T) {
+       panic("oh no")
+}

diff --git a/src/cmd/internal/test2json/test2json.go b/src/cmd/internal/test2json/test2json.go
index aa63c8b9a629f0d28ba3abb5dca5dca895f4f8b7..098128ef3ad4da874dac9ef64c8d761ff22fcbf5 100644 (file)
--- a/src/cmd/internal/test2json/test2json.go
+++ b/src/cmd/internal/test2json/test2json.go
@@ -128,9 +128,16 @@ func (c *converter) Write(b []byte) (int, error) {
 }
 
 var (
+       // printed by test on successful run.
        bigPass = []byte("PASS\n")
+
+       // printed by test after a normal test failure.
        bigFail = []byte("FAIL\n")
 
+       // printed by 'go test' along with an error if the test binary terminates
+       // with an error.
+       bigFailErrorPrefix = []byte("FAIL\t")
+
        updates = [][]byte{
                []byte("=== RUN   "),
                []byte("=== PAUSE "),
@@ -155,7 +162,7 @@ var (
 // before or after emitting other events.
 func (c *converter) handleInputLine(line []byte) {
        // Final PASS or FAIL.
-       if bytes.Equal(line, bigPass) || bytes.Equal(line, bigFail) {
+       if bytes.Equal(line, bigPass) || bytes.Equal(line, bigFail) || bytes.HasPrefix(line, bigFailErrorPrefix) {
                c.flushReport(0)
                c.output.write(line)
                if bytes.Equal(line, bigPass) {

diff --git a/src/cmd/internal/test2json/testdata/panic.json b/src/cmd/internal/test2json/testdata/panic.json
index f99679c2e2364dd4e196a0d69560b3365626520a..f7738142e60adc389ec413b2605890188c2f4482 100644 (file)
--- a/src/cmd/internal/test2json/testdata/panic.json
+++ b/src/cmd/internal/test2json/testdata/panic.json
@@ -13,7 +13,7 @@
 {"Action":"output","Test":"TestPanic","Output":"\tgo/src/testing/testing.go:909 +0xc9\n"}
 {"Action":"output","Test":"TestPanic","Output":"created by testing.(*T).Run\n"}
 {"Action":"output","Test":"TestPanic","Output":"\tgo/src/testing/testing.go:960 +0x350\n"}
-{"Action":"output","Test":"TestPanic","Output":"FAIL\tcommand-line-arguments\t0.042s\n"}
 {"Action":"fail","Test":"TestPanic"}
+{"Action":"output","Output":"FAIL\tcommand-line-arguments\t0.042s\n"}
 {"Action":"output","Output":"FAIL\n"}
 {"Action":"fail"}