From 4efdaa7bc7220fad0177842d0009285ca429a823 Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 28 Aug 2021 16:06:43 -0700 Subject: testing: skip panics when picking the line number for decoration Fixes #31154 Change-Id: I4cfd98b5e79f1abdc93044fb66855ac2cc0a9a49 Reviewed-on: https://go-review.googlesource.com/c/go/+/345909 Run-TryBot: Caleb Spare TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Trust: Carlos Amedee --- src/testing/helper_test.go | 35 +++-------------------------------- src/testing/helperfuncs_test.go | 32 ++++++++++++++++++++++++++++++++ src/testing/testing.go | 3 +++ 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/testing/helper_test.go b/src/testing/helper_test.go index b27fd62ee8..6175410f18 100644 --- a/src/testing/helper_test.go +++ b/src/testing/helper_test.go @@ -33,6 +33,9 @@ helperfuncs_test.go:45: 5 helperfuncs_test.go:21: 6 helperfuncs_test.go:44: 7 helperfuncs_test.go:56: 8 +--- FAIL: Test/sub2 (?s) +helperfuncs_test.go:71: 11 +helperfuncs_test.go:75: recover 12 helperfuncs_test.go:64: 9 helperfuncs_test.go:60: 10 ` @@ -71,38 +74,6 @@ func TestTBHelperParallel(t *T) { } } -func TestTBHelperLineNumer(t *T) { - var buf bytes.Buffer - ctx := newTestContext(1, newMatcher(regexp.MatchString, "", "")) - t1 := &T{ - common: common{ - signal: make(chan bool), - w: &buf, - }, - context: ctx, - } - t1.Run("Test", func(t *T) { - helperA := func(t *T) { - t.Helper() - t.Run("subtest", func(t *T) { - t.Helper() - t.Fatal("fatal error message") - }) - } - helperA(t) - }) - - want := "helper_test.go:92: fatal error message" - got := "" - lines := strings.Split(strings.TrimSpace(buf.String()), "\n") - if len(lines) > 0 { - got = strings.TrimSpace(lines[len(lines)-1]) - } - if got != want { - t.Errorf("got output:\n\n%v\nwant:\n\n%v", got, want) - } -} - type noopWriter int func (nw *noopWriter) Write(b []byte) (int, error) { return len(b), nil } diff --git a/src/testing/helperfuncs_test.go b/src/testing/helperfuncs_test.go index df0476ed73..272b33c0e5 100644 --- a/src/testing/helperfuncs_test.go +++ b/src/testing/helperfuncs_test.go @@ -65,6 +65,14 @@ func testHelper(t *T) { t.Helper() t.Error("9") }) + + // Check that helper-ness propagates up through subtests + // to helpers above. See https://golang.org/issue/44887. + helperSubCallingHelper(t, "11") + + // Check that helper-ness propagates up through panic/recover. + // See https://golang.org/issue/31154. + recoverHelper(t, "12") } func parallelTestHelper(t *T) { @@ -78,3 +86,27 @@ func parallelTestHelper(t *T) { } wg.Wait() } + +func helperSubCallingHelper(t *T, msg string) { + t.Helper() + t.Run("sub2", func(t *T) { + t.Helper() + t.Fatal(msg) + }) +} + +func recoverHelper(t *T, msg string) { + t.Helper() + defer func() { + t.Helper() + if err := recover(); err != nil { + t.Errorf("recover %s", err) + } + }() + doPanic(t, msg) +} + +func doPanic(t *T, msg string) { + t.Helper() + panic(msg) +} diff --git a/src/testing/testing.go b/src/testing/testing.go index a19238d31e..2239e01e22 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -483,6 +483,9 @@ func (c *common) frameSkip(skip int) runtime.Frame { var firstFrame, prevFrame, frame runtime.Frame for more := true; more; prevFrame = frame { frame, more = frames.Next() + if frame.Function == "runtime.gopanic" { + continue + } if frame.Function == c.cleanupName { frames = runtime.CallersFrames(c.cleanupPc) continue -- cgit v1.2.3-54-g00ecf