testing: add TB.Helper to better support test helpers

This CL implements the proposal at
https://github.com/golang/proposal/blob/master/design/4899-testing-helper.md.

It's based on Josh's CL 79890043 from a few years ago:
https://codereview.appspot.com/79890043 but makes several changes,
most notably by using the new CallersFrames API so that it works with
mid-stack inlining.

Another detail came up while I was working on this: I didn't want the
user to be able to call t.Helper from inside their TestXxx function
directly (which would mean we'd print a file:line from inside the
testing package itself), so I explicitly prevented this from working.

Fixes #4899.

Change-Id: I37493edcfb63307f950442bbaf993d1589515310
Reviewed-on: https://go-review.googlesource.com/38796
Run-TryBot: Caleb Spare <cespare@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

src/testing/helper_test.go

@@ -0,0 +1,70 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package testing
+
+import (
+	"bytes"
+	"regexp"
+	"strings"
+)
+
+func TestTBHelper(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", testHelper)
+
+	want := `--- FAIL: Test (?s)
+helperfuncs_test.go:12: 0
+helperfuncs_test.go:33: 1
+helperfuncs_test.go:21: 2
+helperfuncs_test.go:35: 3
+helperfuncs_test.go:42: 4
+helperfuncs_test.go:47: 5
+--- FAIL: Test/sub (?s)
+helperfuncs_test.go:50: 6
+helperfuncs_test.go:21: 7
+helperfuncs_test.go:53: 8
+`
+	lines := strings.Split(buf.String(), "\n")
+	durationRE := regexp.MustCompile(`\(.*\)$`)
+	for i, line := range lines {
+		line = strings.TrimSpace(line)
+		line = durationRE.ReplaceAllString(line, "(?s)")
+		lines[i] = line
+	}
+	got := strings.Join(lines, "\n")
+	if got != want {
+		t.Errorf("got output:\n\n%s\nwant:\n\n%s", got, want)
+	}
+}
+
+func TestTBHelperParallel(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", parallelTestHelper)
+
+	lines := strings.Split(strings.TrimSpace(buf.String()), "\n")
+	if len(lines) != 6 {
+		t.Fatalf("parallelTestHelper gave %d lines of output; want 6", len(lines))
+	}
+	want := "helperfuncs_test.go:21: parallel"
+	if got := strings.TrimSpace(lines[1]); got != want {
+		t.Errorf("got output line %q; want %q", got, want)
+	}
+}

src/testing/helperfuncs_test.go

@@ -0,0 +1,67 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package testing
+
+import "sync"
+
+// The line numbering of this file is important for TestTBHelper.
+
+func notHelper(t *T, msg string) {
+	t.Error(msg)
+}
+
+func helper(t *T, msg string) {
+	t.Helper()
+	t.Error(msg)
+}
+
+func notHelperCallingHelper(t *T, msg string) {
+	helper(t, msg)
+}
+
+func helperCallingHelper(t *T, msg string) {
+	t.Helper()
+	helper(t, msg)
+}
+
+func testHelper(t *T) {
+	// Check combinations of directly and indirectly
+	// calling helper functions.
+	notHelper(t, "0")
+	helper(t, "1")
+	notHelperCallingHelper(t, "2")
+	helperCallingHelper(t, "3")
+
+	// Check a function literal closing over t that uses Helper.
+	fn := func(msg string) {
+		t.Helper()
+		t.Error(msg)
+	}
+	fn("4")
+
+	// Check that calling Helper from inside this test entry function
+	// doesn't have an effect.
+	t.Helper()
+	t.Error("5")
+
+	t.Run("sub", func(t *T) {
+		helper(t, "6")
+		notHelperCallingHelper(t, "7")
+		t.Helper()
+		t.Error("8")
+	})
+}
+
+func parallelTestHelper(t *T) {
+	var wg sync.WaitGroup
+	for i := 0; i < 5; i++ {
+		wg.Add(1)
+		go func() {
+			notHelperCallingHelper(t, "parallel")
+			wg.Done()
+		}()
+	}
+	wg.Wait()
+}

src/testing/testing.go

@@ -273,17 +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 output, w, failed, and done.
-	output     []byte       // Output generated by test or benchmark.
-	w          io.Writer    // For flushToParent.
-	chatty     bool         // A copy of the chatty flag.
-	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.
-	finished   bool         // Test function has completed.
-	done       bool         // Test is finished and all subtests have completed.
-	hasSub     int32        // written atomically
-	raceErrors int          // number of races detected during 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[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
 
 	parent   *common
 	level    int       // Nesting depth of test or benchmark.
@@ -312,10 +315,48 @@ func Verbose() bool {
 	return *chatty
 }
 
+// frameSkip searches, starting after skip frames, for the first caller frame
+// in a function not marked as a helper and returns the frames to skip
+// to reach that site. The search stops if it finds a tRunner function that
+// was the entry point into the test.
+// This function must be called with c.mu held.
+func (c *common) frameSkip(skip int) int {
+	if c.helpers == nil {
+		return skip
+	}
+	var pc [50]uintptr
+	// Skip two extra frames to account for this function
+	// and runtime.Callers itself.
+	n := runtime.Callers(skip+2, pc[:])
+	if n == 0 {
+		panic("testing: zero callers found")
+	}
+	frames := runtime.CallersFrames(pc[:n])
+	var frame runtime.Frame
+	more := true
+	for i := 0; more; i++ {
+		frame, more = frames.Next()
+		if frame.Entry == 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 {
+			// Found a frame that wasn't inside a helper function.
+			return skip + i
+		}
+	}
+	return skip
+}
+
 // decorate prefixes the string with the file and line of the call site
 // and inserts the final newline if needed and indentation tabs for formatting.
-func decorate(s string) string {
-	_, file, line, ok := runtime.Caller(3) // decorate + log + public function.
+// This function must be called with c.mu held.
+func (c *common) decorate(s string) string {
+	skip := c.frameSkip(3) // decorate + log + public function.
+	_, file, line, ok := runtime.Caller(skip)
 	if ok {
 		// Truncate file name at last file name separator.
 		if index := strings.LastIndex(file, "/"); index >= 0 {
@@ -405,6 +446,7 @@ type TB interface {
 	SkipNow()
 	Skipf(format string, args ...interface{})
 	Skipped() bool
+	Helper()
 
 	// A private method to prevent users implementing the
 	// interface and so future additions to it will not
@@ -505,7 +547,7 @@ func (c *common) FailNow() {
 func (c *common) log(s string) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	c.output = append(c.output, decorate(s)...)
+	c.output = append(c.output, c.decorate(s)...)
 }
 
 // Log formats its arguments using default formatting, analogous to Println,
@@ -583,6 +625,33 @@ func (c *common) Skipped() bool {
 	return c.skipped
 }
 
+// Helper marks the calling function as a test helper function.
+// When printing file and line information, that function will be skipped.
+// Helper may be called simultaneously from multiple goroutines.
+// Helper has no effect if it is called directly from a TestXxx/BenchmarkXxx
+// function or a subtest/sub-benchmark function.
+func (c *common) Helper() {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if c.helpers == nil {
+		c.helpers = make(map[uintptr]struct{})
+	}
+	c.helpers[callerEntry(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
+	if n == 0 {
+		panic("testing: zero callers found")
+	}
+	frames := runtime.CallersFrames(pc[:])
+	frame, _ := frames.Next()
+	return frame.Entry
+}
+
 // Parallel signals that this test is to be run in parallel with (and only with)
 // other parallel tests. When a test is run multiple times due to use of
 // -test.count or -test.cpu, multiple instances of a single test never run in
@@ -617,6 +686,8 @@ type InternalTest struct {
 }
 
 func tRunner(t *T, fn func(t *T)) {
+	t.runner = callerEntry(0)
+
 	// When this goroutine is done, either because fn(t)
 	// returned normally or because a test failure triggered
 	// a call to runtime.Goexit, record the duration and send