From: Michał Łowicki <mlowicki@gmail.com>
Date: Mon, 4 May 2020 21:23:28 +0000 (+0100)
Subject: testing: fix reported caller name for funcs passed to Cleanup
X-Git-Tag: go1.15beta1~225
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=7db566f9c26236f852fa0f980e6c4e8cf86890f3;p=gostls13.git

testing: fix reported caller name for funcs passed to Cleanup

Record the caller when Cleanup is called to report it with t.Log
instead of unhelpful line in testing.go.

Fixes #38800

Change-Id: I3136f5d92a0e5a48f8b32a2e13b2521bc91d72d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/232237
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
---

diff --git a/src/testing/helper_test.go b/src/testing/helper_test.go
index fe8ff056ab..7ce58c67fb 100644
--- a/src/testing/helper_test.go
+++ b/src/testing/helper_test.go
@@ -33,6 +33,8 @@ helperfuncs_test.go:45: 5
 helperfuncs_test.go:21: 6
 helperfuncs_test.go:44: 7
 helperfuncs_test.go:56: 8
+helperfuncs_test.go:64: 9
+helperfuncs_test.go:60: 10
 `
 	lines := strings.Split(buf.String(), "\n")
 	durationRE := regexp.MustCompile(`\(.*\)$`)
diff --git a/src/testing/helperfuncs_test.go b/src/testing/helperfuncs_test.go
index f2d54b3a99..df0476ed73 100644
--- a/src/testing/helperfuncs_test.go
+++ b/src/testing/helperfuncs_test.go
@@ -54,6 +54,17 @@ func testHelper(t *T) {
 	// has no effect.
 	t.Helper()
 	t.Error("8")
+
+	// Check that right caller is reported for func passed to Cleanup when
+	// multiple cleanup functions have been registered.
+	t.Cleanup(func() {
+		t.Helper()
+		t.Error("10")
+	})
+	t.Cleanup(func() {
+		t.Helper()
+		t.Error("9")
+	})
 }
 
 func parallelTestHelper(t *T) {
diff --git a/src/testing/testing.go b/src/testing/testing.go
index ed88ba51fb..90c15a2cff 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -338,15 +338,17 @@ const maxStackLen = 50
 // common holds the elements common between T and B and
 // captures common methods such as Errorf.
 type common struct {
-	mu      sync.RWMutex        // guards this group of fields
-	output  []byte              // Output generated by test or benchmark.
-	w       io.Writer           // For flushToParent.
-	ran     bool                // Test or benchmark (or one of its subtests) was executed.
-	failed  bool                // Test or benchmark has failed.
-	skipped bool                // Test of benchmark has been skipped.
-	done    bool                // Test is finished and all subtests have completed.
-	helpers map[string]struct{} // functions to be skipped when writing file/line info
-	cleanup func()              // optional function to be called at the end of the test
+	mu          sync.RWMutex        // guards this group of fields
+	output      []byte              // Output generated by test or benchmark.
+	w           io.Writer           // For flushToParent.
+	ran         bool                // Test or benchmark (or one of its subtests) was executed.
+	failed      bool                // Test or benchmark has failed.
+	skipped     bool                // Test of benchmark has been skipped.
+	done        bool                // Test is finished and all subtests have completed.
+	helpers     map[string]struct{} // functions to be skipped when writing file/line info
+	cleanup     func()              // optional function to be called at the end of the test
+	cleanupName string              // Name of the cleanup function.
+	cleanupPc   []uintptr           // The stack trace at the point where Cleanup was called.
 
 	chatty     bool   // A copy of the chatty flag.
 	finished   bool   // Test function has completed.
@@ -426,6 +428,10 @@ 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 == c.cleanupName {
+			frames = runtime.CallersFrames(c.cleanupPc)
+			continue
+		}
 		if firstFrame.PC == 0 {
 			firstFrame = frame
 		}
@@ -789,12 +795,21 @@ func (c *common) Cleanup(f func()) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	oldCleanup := c.cleanup
+	oldCleanupPc := c.cleanupPc
 	c.cleanup = func() {
 		if oldCleanup != nil {
-			defer oldCleanup()
+			defer func() {
+				c.cleanupPc = oldCleanupPc
+				oldCleanup()
+			}()
 		}
+		c.cleanupName = callerName(0)
 		f()
 	}
+	var pc [maxStackLen]uintptr
+	// Skip two extra frames to account for this function and runtime.Callers itself.
+	n := runtime.Callers(2, pc[:])
+	c.cleanupPc = pc[:n]
 }
 
 var tempDirReplacer struct {