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 <cespare@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Carlos Amedee <carlos@golang.org>

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