From 76a6adcf3a1258efad1751559b7364a4cb8ae28b Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Thu, 5 Mar 2020 11:11:47 -0500 Subject: [PATCH] [release-branch.go1.14] 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 #37671 Updates #37555 Updates #29062 Updates #31969 Change-Id: Icb67bcd36bed97e6a8d51f4d14bf71f73c83ac3d Reviewed-on: https://go-review.googlesource.com/c/go/+/222243 Run-TryBot: Jay Conrod TryBot-Result: Gobot Gobot Reviewed-by: Bryan C. Mills (cherry picked from commit 5ea58c63468bbc7e8705ee13d0bddbf3693785fe) Reviewed-on: https://go-review.googlesource.com/c/go/+/222658 Run-TryBot: Dmitri Shuralyov Reviewed-by: Jay Conrod --- src/cmd/go/internal/test/test.go | 8 +++ .../testdata/script/test_json_panic_exit.txt | 69 +++++++++++++++++++ src/cmd/internal/test2json/test2json.go | 9 ++- .../internal/test2json/testdata/panic.json | 2 +- 4 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 src/cmd/go/testdata/script/test_json_panic_exit.txt diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index fb011d4c03f18f..a89965f00aaadd 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -1224,6 +1224,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 index 00000000000000..d0a7991fe56c75 --- /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 aa63c8b9a629f0..098128ef3ad4da 100644 --- 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 f99679c2e2364d..f7738142e60adc 100644 --- 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"}