From bc2931372243043842161c0a60bd2f86ef9696ee Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Wed, 29 Mar 2017 15:04:40 -0700 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/testing/helper_test.go | 70 +++++++++++++++++++++++ src/testing/helperfuncs_test.go | 67 ++++++++++++++++++++++ src/testing/testing.go | 99 ++++++++++++++++++++++++++++----- 3 files changed, 222 insertions(+), 14 deletions(-) create mode 100644 src/testing/helper_test.go create mode 100644 src/testing/helperfuncs_test.go diff --git a/src/testing/helper_test.go b/src/testing/helper_test.go new file mode 100644 index 0000000000..f5cb27c317 --- /dev/null +++ b/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) + } +} diff --git a/src/testing/helperfuncs_test.go b/src/testing/helperfuncs_test.go new file mode 100644 index 0000000000..7cb2e2cc56 --- /dev/null +++ b/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() +} diff --git a/src/testing/testing.go b/src/testing/testing.go index e5a3c3b93c..f95f4ec4a6 100644 --- a/src/testing/testing.go +++ b/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