From 318812e7c1b6b9ca890e62263a4cab1cb78ea6ed Mon Sep 17 00:00:00 2001 From: David Lazar Date: Thu, 20 Apr 2017 16:19:55 -0400 Subject: [PATCH] testing: use function names to identify helpers Previously, helpers were identified by entry PC, but this breaks if the helper is inlined (as in notHelperCallingHelper). Instead, identify helpers by function name (with package path). Now TestTBHelper and TestTBHelperParallel pass with -l=4. To keep the code unified, this change makes it so that the runner is also identified by function name instead of entry PC. Change-Id: I1b1987fc49d114e69d075fab56aeeacd5294982b Reviewed-on: https://go-review.googlesource.com/41257 Run-TryBot: David Lazar TryBot-Result: Gobot Gobot Reviewed-by: Josh Bleecher Snyder Reviewed-by: Austin Clements --- src/testing/testing.go | 53 +++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/src/testing/testing.go b/src/testing/testing.go index f95f4ec4a6..99e9af43b6 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -273,20 +273,20 @@ var ( // 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[uintptr]struct{} // functions to be skipped when writing file/line info - - chatty bool // A copy of the chatty flag. - finished bool // Test function has completed. - hasSub int32 // written atomically - raceErrors int // number of races detected during test - runner uintptr // entry pc of tRunner running 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 + + chatty bool // A copy of the chatty flag. + finished bool // Test function has completed. + hasSub int32 // written atomically + raceErrors int // number of races detected during test + runner string // function name of tRunner running the test parent *common level int // Nesting depth of test or benchmark. @@ -336,14 +336,14 @@ func (c *common) frameSkip(skip int) int { more := true for i := 0; more; i++ { frame, more = frames.Next() - if frame.Entry == c.runner { + if frame.Function == c.runner { // We've gone up all the way to the tRunner calling // the test function (so the user must have // called tb.Helper from inside that test function). // Only skip up to the test function itself. return skip + i - 1 } - if _, ok := c.helpers[frame.Entry]; !ok { + if _, ok := c.helpers[frame.Function]; !ok { // Found a frame that wasn't inside a helper function. return skip + i } @@ -634,22 +634,23 @@ func (c *common) Helper() { c.mu.Lock() defer c.mu.Unlock() if c.helpers == nil { - c.helpers = make(map[uintptr]struct{}) + c.helpers = make(map[string]struct{}) } - c.helpers[callerEntry(1)] = struct{}{} + c.helpers[callerName(1)] = struct{}{} } -// callerEntry gives the entry pc for the caller after skip frames -// (where 0 means the current function). -func callerEntry(skip int) uintptr { - var pc [1]uintptr - n := runtime.Callers(skip+2, pc[:]) // skip + runtime.Callers + callerEntry +// callerName gives the function name (qualified with a package path) +// for the caller after skip frames (where 0 means the current function). +func callerName(skip int) string { + // Make room for the skip PC. + var pc [2]uintptr + n := runtime.Callers(skip+2, pc[:]) // skip + runtime.Callers + callerName if n == 0 { panic("testing: zero callers found") } - frames := runtime.CallersFrames(pc[:]) + frames := runtime.CallersFrames(pc[:n]) frame, _ := frames.Next() - return frame.Entry + return frame.Function } // Parallel signals that this test is to be run in parallel with (and only with) @@ -686,7 +687,7 @@ type InternalTest struct { } func tRunner(t *T, fn func(t *T)) { - t.runner = callerEntry(0) + t.runner = callerName(0) // When this goroutine is done, either because fn(t) // returned normally or because a test failure triggered -- 2.48.1