testing: harmonize handling of prefix-matched benchmarks

If you have BenchmarkX1 with sub-benchmark Y
and you have BenchmarkX2 with no sub-benchmarks,
then

	go test -bench=X/Y

runs BenchmarkX1 once with b.N=1 (to find out about Y)
and then not again, because it has sub-benchmarks,
but arguably also because we're interested in Y.

In contrast, it runs BenchmarkX2 in full, even though clearly
that is not relevant to the match X/Y. We do have to run X2
once with b.N=1 to probe for having X2/Y, but we should not
run it with larger b.N.

Fixes #20589.

Change-Id: Ib86907e844f34dcaac6cd05757f57db1019201d0
Reviewed-on: https://go-review.googlesource.com/46031
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index a59da8bc903c8cad07058bb2398f3e79ff701f4c..e7fc5fc1038be56c5c81296d1860805099b05d21 100644 (file)
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -4194,3 +4194,61 @@ func main() {}`)
                tg.setenv("GOARM", "7")
        }))
 }
+
+func TestTestRegexps(t *testing.T) {
+       tg := testgo(t)
+       defer tg.cleanup()
+       tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata"))
+       tg.run("test", "-cpu=1", "-run=X/Y", "-bench=X/Y", "-count=2", "-v", "testregexp")
+       var lines []string
+       for _, line := range strings.SplitAfter(tg.getStdout(), "\n") {
+               if strings.Contains(line, "=== RUN") || strings.Contains(line, "--- BENCH") || strings.Contains(line, "LOG") {
+                       lines = append(lines, line)
+               }
+       }
+
+       // Important parts:
+       //      TestX is run, twice
+       //      TestX/Y is run, twice
+       //      TestXX is run, twice
+       //      TestZ is not run
+       //      BenchmarkX is run but only with N=1, once
+       //      BenchmarkXX is run but only with N=1, once
+       //      BenchmarkX/Y is run in full, twice
+       want := `=== RUN   TestX
+=== RUN   TestX/Y
+       x_test.go:6: LOG: X running
+       x_test.go:8: LOG: Y running
+=== RUN   TestXX
+       z_test.go:10: LOG: XX running
+=== RUN   TestX
+=== RUN   TestX/Y
+       x_test.go:6: LOG: X running
+       x_test.go:8: LOG: Y running
+=== RUN   TestXX
+       z_test.go:10: LOG: XX running
+--- BENCH: BenchmarkX/Y
+       x_test.go:15: LOG: Y running N=1
+       x_test.go:15: LOG: Y running N=100
+       x_test.go:15: LOG: Y running N=10000
+       x_test.go:15: LOG: Y running N=1000000
+       x_test.go:15: LOG: Y running N=100000000
+       x_test.go:15: LOG: Y running N=2000000000
+--- BENCH: BenchmarkX/Y
+       x_test.go:15: LOG: Y running N=1
+       x_test.go:15: LOG: Y running N=100
+       x_test.go:15: LOG: Y running N=10000
+       x_test.go:15: LOG: Y running N=1000000
+       x_test.go:15: LOG: Y running N=100000000
+       x_test.go:15: LOG: Y running N=2000000000
+--- BENCH: BenchmarkX
+       x_test.go:13: LOG: X running N=1
+--- BENCH: BenchmarkXX
+       z_test.go:18: LOG: XX running N=1
+`
+
+       have := strings.Join(lines, "")
+       if have != want {
+               t.Errorf("reduced output:<<<\n%s>>> want:<<<\n%s>>>", have, want)
+       }
+}

diff --git a/src/cmd/go/testdata/src/testregexp/x_test.go b/src/cmd/go/testdata/src/testregexp/x_test.go
new file mode 100644 (file)
index 0000000..7573e79
--- /dev/null
+++ b/src/cmd/go/testdata/src/testregexp/x_test.go
@@ -0,0 +1,17 @@
+package x
+
+import "testing"
+
+func TestX(t *testing.T) {
+       t.Logf("LOG: X running")
+       t.Run("Y", func(t *testing.T) {
+               t.Logf("LOG: Y running")
+       })
+}
+
+func BenchmarkX(b *testing.B) {
+       b.Logf("LOG: X running N=%d", b.N)
+       b.Run("Y", func(b *testing.B) {
+               b.Logf("LOG: Y running N=%d", b.N)
+       })
+}

diff --git a/src/cmd/go/testdata/src/testregexp/z_test.go b/src/cmd/go/testdata/src/testregexp/z_test.go
new file mode 100644 (file)
index 0000000..4fd1979
--- /dev/null
+++ b/src/cmd/go/testdata/src/testregexp/z_test.go
@@ -0,0 +1,19 @@
+package x
+
+import "testing"
+
+func TestZ(t *testing.T) {
+       t.Logf("LOG: Z running")
+}
+
+func TestXX(t *testing.T) {
+       t.Logf("LOG: XX running")
+}
+
+func BenchmarkZ(b *testing.B) {
+       b.Logf("LOG: Z running N=%d", b.N)
+}
+
+func BenchmarkXX(b *testing.B) {
+       b.Logf("LOG: XX running N=%d", b.N)
+}

diff --git a/src/testing/benchmark.go b/src/testing/benchmark.go
index 18a46d93bf0edc80e11b1dc668de1ec825eb7726..8b7f5cebaf08cfdeeec79f9a2fd3503e633b0c22 100644 (file)
--- a/src/testing/benchmark.go
+++ b/src/testing/benchmark.go
@@ -405,7 +405,7 @@ func runBenchmarks(importPath string, matchString func(pat, str string) (bool, e
        }
        var bs []InternalBenchmark
        for _, Benchmark := range benchmarks {
-               if _, matched := ctx.match.fullName(nil, Benchmark.Name); matched {
+               if _, matched, _ := ctx.match.fullName(nil, Benchmark.Name); matched {
                        bs = append(bs, Benchmark)
                        benchName := benchmarkName(Benchmark.Name, maxprocs)
                        if l := len(benchName) + ctx.extLen + 1; l > ctx.maxLen {
@@ -492,9 +492,9 @@ func (b *B) Run(name string, f func(b *B)) bool {
        benchmarkLock.Unlock()
        defer benchmarkLock.Lock()
 
-       benchName, ok := b.name, true
+       benchName, ok, partial := b.name, true, false
        if b.context != nil {
-               benchName, ok = b.context.match.fullName(&b.common, name)
+               benchName, ok, partial = b.context.match.fullName(&b.common, name)
        }
        if !ok {
                return true
@@ -513,6 +513,11 @@ func (b *B) Run(name string, f func(b *B)) bool {
                benchTime:  b.benchTime,
                context:    b.context,
        }
+       if partial {
+               // Partial name match, like -bench=X/Y matching BenchmarkX.
+               // Only process sub-benchmarks, if any.
+               atomic.StoreInt32(&sub.hasSub, 1)
+       }
        if sub.run1() {
                sub.run()
        }

diff --git a/src/testing/match.go b/src/testing/match.go
index 77510357602c9b87f4b7ad886f26bbfb027db3c2..89e30d01a75da74ee72ff5f907fa4b0b8becdb16 100644 (file)
--- a/src/testing/match.go
+++ b/src/testing/match.go
@@ -47,7 +47,7 @@ func newMatcher(matchString func(pat, str string) (bool, error), patterns, name
        }
 }
 
-func (m *matcher) fullName(c *common, subname string) (name string, ok bool) {
+func (m *matcher) fullName(c *common, subname string) (name string, ok, partial bool) {
        name = subname
 
        m.mu.Lock()
@@ -62,15 +62,16 @@ func (m *matcher) fullName(c *common, subname string) (name string, ok bool) {
 
        // We check the full array of paths each time to allow for the case that
        // a pattern contains a '/'.
-       for i, s := range strings.Split(name, "/") {
+       elem := strings.Split(name, "/")
+       for i, s := range elem {
                if i >= len(m.filter) {
                        break
                }
                if ok, _ := m.matchFunc(m.filter[i], s); !ok {
-                       return name, false
+                       return name, false, false
                }
        }
-       return name, true
+       return name, true, len(elem) < len(m.filter)
 }
 
 func splitRegexp(s string) []string {

diff --git a/src/testing/match_test.go b/src/testing/match_test.go
index 8c1c5f4452c5c0c852d3ff1b33e5029b9d9d479d..8c09dc660fb1a42b164ae8977ba87b8b61b3f294 100644 (file)
--- a/src/testing/match_test.go
+++ b/src/testing/match_test.go
@@ -88,43 +88,44 @@ func TestMatcher(t *T) {
                pattern     string
                parent, sub string
                ok          bool
+               partial     bool
        }{
                // Behavior without subtests.
-               {"", "", "TestFoo", true},
-               {"TestFoo", "", "TestFoo", true},
-               {"TestFoo/", "", "TestFoo", true},
-               {"TestFoo/bar/baz", "", "TestFoo", true},
-               {"TestFoo", "", "TestBar", false},
-               {"TestFoo/", "", "TestBar", false},
-               {"TestFoo/bar/baz", "", "TestBar/bar/baz", false},
+               {"", "", "TestFoo", true, false},
+               {"TestFoo", "", "TestFoo", true, false},
+               {"TestFoo/", "", "TestFoo", true, true},
+               {"TestFoo/bar/baz", "", "TestFoo", true, true},
+               {"TestFoo", "", "TestBar", false, false},
+               {"TestFoo/", "", "TestBar", false, false},
+               {"TestFoo/bar/baz", "", "TestBar/bar/baz", false, false},
 
                // with subtests
-               {"", "TestFoo", "x", true},
-               {"TestFoo", "TestFoo", "x", true},
-               {"TestFoo/", "TestFoo", "x", true},
-               {"TestFoo/bar/baz", "TestFoo", "bar", true},
+               {"", "TestFoo", "x", true, false},
+               {"TestFoo", "TestFoo", "x", true, false},
+               {"TestFoo/", "TestFoo", "x", true, false},
+               {"TestFoo/bar/baz", "TestFoo", "bar", true, true},
                // Subtest with a '/' in its name still allows for copy and pasted names
                // to match.
-               {"TestFoo/bar/baz", "TestFoo", "bar/baz", true},
-               {"TestFoo/bar/baz", "TestFoo/bar", "baz", true},
-               {"TestFoo/bar/baz", "TestFoo", "x", false},
-               {"TestFoo", "TestBar", "x", false},
-               {"TestFoo/", "TestBar", "x", false},
-               {"TestFoo/bar/baz", "TestBar", "x/bar/baz", false},
+               {"TestFoo/bar/baz", "TestFoo", "bar/baz", true, false},
+               {"TestFoo/bar/baz", "TestFoo/bar", "baz", true, false},
+               {"TestFoo/bar/baz", "TestFoo", "x", false, false},
+               {"TestFoo", "TestBar", "x", false, false},
+               {"TestFoo/", "TestBar", "x", false, false},
+               {"TestFoo/bar/baz", "TestBar", "x/bar/baz", false, false},
 
                // subtests only
-               {"", "TestFoo", "x", true},
-               {"/", "TestFoo", "x", true},
-               {"./", "TestFoo", "x", true},
-               {"./.", "TestFoo", "x", true},
-               {"/bar/baz", "TestFoo", "bar", true},
-               {"/bar/baz", "TestFoo", "bar/baz", true},
-               {"//baz", "TestFoo", "bar/baz", true},
-               {"//", "TestFoo", "bar/baz", true},
-               {"/bar/baz", "TestFoo/bar", "baz", true},
-               {"//foo", "TestFoo", "bar/baz", false},
-               {"/bar/baz", "TestFoo", "x", false},
-               {"/bar/baz", "TestBar", "x/bar/baz", false},
+               {"", "TestFoo", "x", true, false},
+               {"/", "TestFoo", "x", true, false},
+               {"./", "TestFoo", "x", true, false},
+               {"./.", "TestFoo", "x", true, false},
+               {"/bar/baz", "TestFoo", "bar", true, true},
+               {"/bar/baz", "TestFoo", "bar/baz", true, false},
+               {"//baz", "TestFoo", "bar/baz", true, false},
+               {"//", "TestFoo", "bar/baz", true, false},
+               {"/bar/baz", "TestFoo/bar", "baz", true, false},
+               {"//foo", "TestFoo", "bar/baz", false, false},
+               {"/bar/baz", "TestFoo", "x", false, false},
+               {"/bar/baz", "TestBar", "x/bar/baz", false, false},
        }
 
        for _, tc := range testCases {
@@ -134,9 +135,9 @@ func TestMatcher(t *T) {
                if tc.parent != "" {
                        parent.level = 1
                }
-               if n, ok := m.fullName(parent, tc.sub); ok != tc.ok {
-                       t.Errorf("for pattern %q, fullName(parent=%q, sub=%q) = %q, ok %v; want ok %v",
-                               tc.pattern, tc.parent, tc.sub, n, ok, tc.ok)
+               if n, ok, partial := m.fullName(parent, tc.sub); ok != tc.ok || partial != tc.partial {
+                       t.Errorf("for pattern %q, fullName(parent=%q, sub=%q) = %q, ok %v partial %v; want ok %v partial %v",
+                               tc.pattern, tc.parent, tc.sub, n, ok, partial, tc.ok, tc.partial)
                }
        }
 }
@@ -178,7 +179,7 @@ func TestNaming(t *T) {
        }
 
        for i, tc := range testCases {
-               if got, _ := m.fullName(parent, tc.name); got != tc.want {
+               if got, _, _ := m.fullName(parent, tc.name); got != tc.want {
                        t.Errorf("%d:%s: got %q; want %q", i, tc.name, got, tc.want)
                }
        }

diff --git a/src/testing/sub_test.go b/src/testing/sub_test.go
index ab145b5bf4f5deaf30f275ab299da91479d604e5..af2d39c5be13d5bed840bef1cda0aaaadf7015da 100644 (file)
--- a/src/testing/sub_test.go
+++ b/src/testing/sub_test.go
@@ -7,7 +7,6 @@ package testing
 import (
        "bytes"
        "fmt"
-       "os"
        "regexp"
        "runtime"
        "strings"
@@ -602,7 +601,6 @@ func TestBenchmark(t *T) {
        res := Benchmark(func(b *B) {
                for i := 0; i < 5; i++ {
                        b.Run("", func(b *B) {
-                               fmt.Fprintf(os.Stderr, "b.N: %v\n", b.N)
                                for i := 0; i < b.N; i++ {
                                        time.Sleep(time.Millisecond)
                                }

diff --git a/src/testing/testing.go b/src/testing/testing.go
index fa6c36c6d3ff66bfbc527f7e705dda68be6a13a7..96c34a5aea7e2193f6217c84bb72657d36c4415c 100644 (file)
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -763,7 +763,7 @@ func tRunner(t *T, fn func(t *T)) {
 // must happen before the outer test function for t returns.
 func (t *T) Run(name string, f func(t *T)) bool {
        atomic.StoreInt32(&t.hasSub, 1)
-       testName, ok := t.context.match.fullName(&t.common, name)
+       testName, ok, _ := t.context.match.fullName(&t.common, name)
        if !ok {
                return true
        }