Skip to content

Commit 81a74b4

Browse files
committed
testing: provide additional information when test funcs panic
Flush the output log up to the root when a test panics. Prior to this change, only the current test's output log was flushed to its parent, resulting in no output when a subtest panics. For the following test function: func Test(t *testing.T) { for i, test := range []int{1, 0, 2} { t.Run(fmt.Sprintf("%v/%v", i, test), func(t *testing.T) { _ = 1 / test }) } } Output before this change: panic: runtime error: integer divide by zero [recovered] panic: runtime error: integer divide by zero (stack trace follows) Output after this change: --- FAIL: Test (0.00s) --- FAIL: Test/1/0 (0.00s) panic: runtime error: integer divide by zero [recovered] (stack trace follows) Fixes #32121 Change-Id: Ifee07ccc005f0493a902190a8be734943123b6b7 Reviewed-on: https://go-review.googlesource.com/c/go/+/179599 Run-TryBot: Damien Neil <dneil@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
1 parent cd18da4 commit 81a74b4

File tree

4 files changed

+130
-2
lines changed

4 files changed

+130
-2
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{"Action":"output","Test":"TestPanic","Output":"--- FAIL: TestPanic (0.00s)\n"}
2+
{"Action":"output","Test":"TestPanic","Output":"panic: oops [recovered]\n"}
3+
{"Action":"output","Test":"TestPanic","Output":"\tpanic: oops\n"}
4+
{"Action":"output","Test":"TestPanic","Output":"\n"}
5+
{"Action":"output","Test":"TestPanic","Output":"goroutine 7 [running]:\n"}
6+
{"Action":"output","Test":"TestPanic","Output":"testing.tRunner.func1(0xc000092100)\n"}
7+
{"Action":"output","Test":"TestPanic","Output":"\t/go/src/testing/testing.go:874 +0x3a3\n"}
8+
{"Action":"output","Test":"TestPanic","Output":"panic(0x1110ea0, 0x116aea0)\n"}
9+
{"Action":"output","Test":"TestPanic","Output":"\t/go/src/runtime/panic.go:679 +0x1b2\n"}
10+
{"Action":"output","Test":"TestPanic","Output":"command-line-arguments.TestPanic(0xc000092100)\n"}
11+
{"Action":"output","Test":"TestPanic","Output":"\ta_test.go:6 +0x39\n"}
12+
{"Action":"output","Test":"TestPanic","Output":"testing.tRunner(0xc000092100, 0x114f500)\n"}
13+
{"Action":"output","Test":"TestPanic","Output":"\tgo/src/testing/testing.go:909 +0xc9\n"}
14+
{"Action":"output","Test":"TestPanic","Output":"created by testing.(*T).Run\n"}
15+
{"Action":"output","Test":"TestPanic","Output":"\tgo/src/testing/testing.go:960 +0x350\n"}
16+
{"Action":"output","Test":"TestPanic","Output":"FAIL\tcommand-line-arguments\t0.042s\n"}
17+
{"Action":"fail","Test":"TestPanic"}
18+
{"Action":"output","Output":"FAIL\n"}
19+
{"Action":"fail"}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--- FAIL: TestPanic (0.00s)
2+
panic: oops [recovered]
3+
panic: oops
4+
5+
goroutine 7 [running]:
6+
testing.tRunner.func1(0xc000092100)
7+
/go/src/testing/testing.go:874 +0x3a3
8+
panic(0x1110ea0, 0x116aea0)
9+
/go/src/runtime/panic.go:679 +0x1b2
10+
command-line-arguments.TestPanic(0xc000092100)
11+
a_test.go:6 +0x39
12+
testing.tRunner(0xc000092100, 0x114f500)
13+
go/src/testing/testing.go:909 +0xc9
14+
created by testing.(*T).Run
15+
go/src/testing/testing.go:960 +0x350
16+
FAIL command-line-arguments 0.042s
17+
FAIL

src/testing/panic_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package testing_test
6+
7+
import (
8+
"flag"
9+
"fmt"
10+
"internal/testenv"
11+
"os"
12+
"os/exec"
13+
"regexp"
14+
"strings"
15+
"testing"
16+
)
17+
18+
var testPanicTest = flag.String("test_panic_test", "", "TestPanic: indicates which test should panic")
19+
20+
func TestPanic(t *testing.T) {
21+
testenv.MustHaveExec(t)
22+
23+
testCases := []struct {
24+
desc string
25+
flags []string
26+
want string
27+
}{{
28+
desc: "root test panics",
29+
flags: []string{"-test_panic_test=TestPanicHelper"},
30+
want: `
31+
--- FAIL: TestPanicHelper (N.NNs)
32+
panic_test.go:NNN: TestPanicHelper
33+
`,
34+
}, {
35+
desc: "subtest panics",
36+
flags: []string{"-test_panic_test=TestPanicHelper/1"},
37+
want: `
38+
--- FAIL: TestPanicHelper (N.NNs)
39+
panic_test.go:NNN: TestPanicHelper
40+
--- FAIL: TestPanicHelper/1 (N.NNs)
41+
panic_test.go:NNN: TestPanicHelper/1
42+
`,
43+
}}
44+
for _, tc := range testCases {
45+
t.Run(tc.desc, func(t *testing.T) {
46+
cmd := exec.Command(os.Args[0], "-test.run=TestPanicHelper")
47+
cmd.Args = append(cmd.Args, tc.flags...)
48+
cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
49+
b, _ := cmd.CombinedOutput()
50+
got := string(b)
51+
want := strings.TrimSpace(tc.want)
52+
re := makeRegexp(want)
53+
if ok, err := regexp.MatchString(re, got); !ok || err != nil {
54+
t.Errorf("output:\ngot:\n%s\nwant:\n%s", got, want)
55+
}
56+
})
57+
}
58+
}
59+
60+
func makeRegexp(s string) string {
61+
s = regexp.QuoteMeta(s)
62+
s = strings.ReplaceAll(s, ":NNN:", `:\d+:`)
63+
s = strings.ReplaceAll(s, "N\\.NNs", `\d*\.\d*s`)
64+
return s
65+
}
66+
67+
func TestPanicHelper(t *testing.T) {
68+
if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
69+
return
70+
}
71+
t.Log(t.Name())
72+
if t.Name() == *testPanicTest {
73+
panic("panic")
74+
}
75+
for i := 0; i < 3; i++ {
76+
t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
77+
t.Log(t.Name())
78+
if t.Name() == *testPanicTest {
79+
panic("panic")
80+
}
81+
})
82+
}
83+
}

src/testing/testing.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,6 @@ func tRunner(t *T, fn func(t *T)) {
860860
t.Errorf("race detected during execution of test")
861861
}
862862

863-
t.duration += time.Since(t.start)
864863
// If the test panicked, print any test output before dying.
865864
err := recover()
866865
signal := true
@@ -877,10 +876,20 @@ func tRunner(t *T, fn func(t *T)) {
877876
}
878877
if err != nil {
879878
t.Fail()
880-
t.report()
879+
// Flush the output log up to the root before dying.
880+
t.mu.Lock()
881+
root := &t.common
882+
for ; root.parent != nil; root = root.parent {
883+
root.duration += time.Since(root.start)
884+
fmt.Fprintf(root.parent.w, "--- FAIL: %s (%s)\n", root.name, fmtDuration(root.duration))
885+
root.parent.mu.Lock()
886+
io.Copy(root.parent.w, bytes.NewReader(root.output))
887+
}
881888
panic(err)
882889
}
883890

891+
t.duration += time.Since(t.start)
892+
884893
if len(t.sub) > 0 {
885894
// Run parallel subtests.
886895
// Decrease the running count for this test.

0 commit comments

Comments
 (0)