Skip to content

Commit a2a3ace

Browse files
rscrsc
committed
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 <[email protected]> Reviewed-by: Marcel van Lohuizen <[email protected]>
1 parent be855e3 commit a2a3ace

File tree

8 files changed

+142
-43
lines changed

8 files changed

+142
-43
lines changed

‎src/cmd/go/go_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4194,3 +4194,61 @@ func main() {}`)
41944194
tg.setenv("GOARM", "7")
41954195
}))
41964196
}
4197+
4198+
func TestTestRegexps(t *testing.T) {
4199+
tg := testgo(t)
4200+
defer tg.cleanup()
4201+
tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata"))
4202+
tg.run("test", "-cpu=1", "-run=X/Y", "-bench=X/Y", "-count=2", "-v", "testregexp")
4203+
var lines []string
4204+
for _, line := range strings.SplitAfter(tg.getStdout(), "\n") {
4205+
if strings.Contains(line, "=== RUN") || strings.Contains(line, "--- BENCH") || strings.Contains(line, "LOG") {
4206+
lines = append(lines, line)
4207+
}
4208+
}
4209+
4210+
// Important parts:
4211+
// TestX is run, twice
4212+
// TestX/Y is run, twice
4213+
// TestXX is run, twice
4214+
// TestZ is not run
4215+
// BenchmarkX is run but only with N=1, once
4216+
// BenchmarkXX is run but only with N=1, once
4217+
// BenchmarkX/Y is run in full, twice
4218+
want := `=== RUN TestX
4219+
=== RUN TestX/Y
4220+
x_test.go:6: LOG: X running
4221+
x_test.go:8: LOG: Y running
4222+
=== RUN TestXX
4223+
z_test.go:10: LOG: XX running
4224+
=== RUN TestX
4225+
=== RUN TestX/Y
4226+
x_test.go:6: LOG: X running
4227+
x_test.go:8: LOG: Y running
4228+
=== RUN TestXX
4229+
z_test.go:10: LOG: XX running
4230+
--- BENCH: BenchmarkX/Y
4231+
x_test.go:15: LOG: Y running N=1
4232+
x_test.go:15: LOG: Y running N=100
4233+
x_test.go:15: LOG: Y running N=10000
4234+
x_test.go:15: LOG: Y running N=1000000
4235+
x_test.go:15: LOG: Y running N=100000000
4236+
x_test.go:15: LOG: Y running N=2000000000
4237+
--- BENCH: BenchmarkX/Y
4238+
x_test.go:15: LOG: Y running N=1
4239+
x_test.go:15: LOG: Y running N=100
4240+
x_test.go:15: LOG: Y running N=10000
4241+
x_test.go:15: LOG: Y running N=1000000
4242+
x_test.go:15: LOG: Y running N=100000000
4243+
x_test.go:15: LOG: Y running N=2000000000
4244+
--- BENCH: BenchmarkX
4245+
x_test.go:13: LOG: X running N=1
4246+
--- BENCH: BenchmarkXX
4247+
z_test.go:18: LOG: XX running N=1
4248+
`
4249+
4250+
have := strings.Join(lines, "")
4251+
if have != want {
4252+
t.Errorf("reduced output:<<<\n%s>>> want:<<<\n%s>>>", have, want)
4253+
}
4254+
}

‎src/cmd/go/testdata/src/testregexp/x_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package x
2+
3+
import "testing"
4+
5+
func TestX(t *testing.T) {
6+
t.Logf("LOG: X running")
7+
t.Run("Y", func(t *testing.T) {
8+
t.Logf("LOG: Y running")
9+
})
10+
}
11+
12+
func BenchmarkX(b *testing.B) {
13+
b.Logf("LOG: X running N=%d", b.N)
14+
b.Run("Y", func(b *testing.B) {
15+
b.Logf("LOG: Y running N=%d", b.N)
16+
})
17+
}

‎src/cmd/go/testdata/src/testregexp/z_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package x
2+
3+
import "testing"
4+
5+
func TestZ(t *testing.T) {
6+
t.Logf("LOG: Z running")
7+
}
8+
9+
func TestXX(t *testing.T) {
10+
t.Logf("LOG: XX running")
11+
}
12+
13+
func BenchmarkZ(b *testing.B) {
14+
b.Logf("LOG: Z running N=%d", b.N)
15+
}
16+
17+
func BenchmarkXX(b *testing.B) {
18+
b.Logf("LOG: XX running N=%d", b.N)
19+
}

‎src/testing/benchmark.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ func runBenchmarks(importPath string, matchString func(pat, str string) (bool, e
405405
}
406406
var bs []InternalBenchmark
407407
for _, Benchmark := range benchmarks {
408-
if _, matched := ctx.match.fullName(nil, Benchmark.Name); matched {
408+
if _, matched, _ := ctx.match.fullName(nil, Benchmark.Name); matched {
409409
bs = append(bs, Benchmark)
410410
benchName := benchmarkName(Benchmark.Name, maxprocs)
411411
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 {
492492
benchmarkLock.Unlock()
493493
defer benchmarkLock.Lock()
494494

495-
benchName, ok := b.name, true
495+
benchName, ok, partial := b.name, true, false
496496
if b.context != nil {
497-
benchName, ok = b.context.match.fullName(&b.common, name)
497+
benchName, ok, partial = b.context.match.fullName(&b.common, name)
498498
}
499499
if !ok {
500500
return true
@@ -513,6 +513,11 @@ func (b *B) Run(name string, f func(b *B)) bool {
513513
benchTime: b.benchTime,
514514
context: b.context,
515515
}
516+
if partial {
517+
// Partial name match, like -bench=X/Y matching BenchmarkX.
518+
// Only process sub-benchmarks, if any.
519+
atomic.StoreInt32(&sub.hasSub, 1)
520+
}
516521
if sub.run1() {
517522
sub.run()
518523
}

‎src/testing/match.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func newMatcher(matchString func(pat, str string) (bool, error), patterns, name
4747
}
4848
}
4949

50-
func (m *matcher) fullName(c *common, subname string) (name string, ok bool) {
50+
func (m *matcher) fullName(c *common, subname string) (name string, ok, partial bool) {
5151
name = subname
5252

5353
m.mu.Lock()
@@ -62,15 +62,16 @@ func (m *matcher) fullName(c *common, subname string) (name string, ok bool) {
6262

6363
// We check the full array of paths each time to allow for the case that
6464
// a pattern contains a '/'.
65-
for i, s := range strings.Split(name, "/") {
65+
elem := strings.Split(name, "/")
66+
for i, s := range elem {
6667
if i >= len(m.filter) {
6768
break
6869
}
6970
if ok, _ := m.matchFunc(m.filter[i], s); !ok {
70-
return name, false
71+
return name, false, false
7172
}
7273
}
73-
return name, true
74+
return name, true, len(elem) < len(m.filter)
7475
}
7576

7677
func splitRegexp(s string) []string {

‎src/testing/match_test.go

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -88,43 +88,44 @@ func TestMatcher(t *T) {
8888
pattern string
8989
parent, sub string
9090
ok bool
91+
partial bool
9192
}{
9293
// Behavior without subtests.
93-
{"", "", "TestFoo", true},
94-
{"TestFoo", "", "TestFoo", true},
95-
{"TestFoo/", "", "TestFoo", true},
96-
{"TestFoo/bar/baz", "", "TestFoo", true},
97-
{"TestFoo", "", "TestBar", false},
98-
{"TestFoo/", "", "TestBar", false},
99-
{"TestFoo/bar/baz", "", "TestBar/bar/baz", false},
94+
{"", "", "TestFoo", true, false},
95+
{"TestFoo", "", "TestFoo", true, false},
96+
{"TestFoo/", "", "TestFoo", true, true},
97+
{"TestFoo/bar/baz", "", "TestFoo", true, true},
98+
{"TestFoo", "", "TestBar", false, false},
99+
{"TestFoo/", "", "TestBar", false, false},
100+
{"TestFoo/bar/baz", "", "TestBar/bar/baz", false, false},
100101

101102
// with subtests
102-
{"", "TestFoo", "x", true},
103-
{"TestFoo", "TestFoo", "x", true},
104-
{"TestFoo/", "TestFoo", "x", true},
105-
{"TestFoo/bar/baz", "TestFoo", "bar", true},
103+
{"", "TestFoo", "x", true, false},
104+
{"TestFoo", "TestFoo", "x", true, false},
105+
{"TestFoo/", "TestFoo", "x", true, false},
106+
{"TestFoo/bar/baz", "TestFoo", "bar", true, true},
106107
// Subtest with a '/' in its name still allows for copy and pasted names
107108
// to match.
108-
{"TestFoo/bar/baz", "TestFoo", "bar/baz", true},
109-
{"TestFoo/bar/baz", "TestFoo/bar", "baz", true},
110-
{"TestFoo/bar/baz", "TestFoo", "x", false},
111-
{"TestFoo", "TestBar", "x", false},
112-
{"TestFoo/", "TestBar", "x", false},
113-
{"TestFoo/bar/baz", "TestBar", "x/bar/baz", false},
109+
{"TestFoo/bar/baz", "TestFoo", "bar/baz", true, false},
110+
{"TestFoo/bar/baz", "TestFoo/bar", "baz", true, false},
111+
{"TestFoo/bar/baz", "TestFoo", "x", false, false},
112+
{"TestFoo", "TestBar", "x", false, false},
113+
{"TestFoo/", "TestBar", "x", false, false},
114+
{"TestFoo/bar/baz", "TestBar", "x/bar/baz", false, false},
114115

115116
// subtests only
116-
{"", "TestFoo", "x", true},
117-
{"/", "TestFoo", "x", true},
118-
{"./", "TestFoo", "x", true},
119-
{"./.", "TestFoo", "x", true},
120-
{"/bar/baz", "TestFoo", "bar", true},
121-
{"/bar/baz", "TestFoo", "bar/baz", true},
122-
{"//baz", "TestFoo", "bar/baz", true},
123-
{"//", "TestFoo", "bar/baz", true},
124-
{"/bar/baz", "TestFoo/bar", "baz", true},
125-
{"//foo", "TestFoo", "bar/baz", false},
126-
{"/bar/baz", "TestFoo", "x", false},
127-
{"/bar/baz", "TestBar", "x/bar/baz", false},
117+
{"", "TestFoo", "x", true, false},
118+
{"/", "TestFoo", "x", true, false},
119+
{"./", "TestFoo", "x", true, false},
120+
{"./.", "TestFoo", "x", true, false},
121+
{"/bar/baz", "TestFoo", "bar", true, true},
122+
{"/bar/baz", "TestFoo", "bar/baz", true, false},
123+
{"//baz", "TestFoo", "bar/baz", true, false},
124+
{"//", "TestFoo", "bar/baz", true, false},
125+
{"/bar/baz", "TestFoo/bar", "baz", true, false},
126+
{"//foo", "TestFoo", "bar/baz", false, false},
127+
{"/bar/baz", "TestFoo", "x", false, false},
128+
{"/bar/baz", "TestBar", "x/bar/baz", false, false},
128129
}
129130

130131
for _, tc := range testCases {
@@ -134,9 +135,9 @@ func TestMatcher(t *T) {
134135
if tc.parent != "" {
135136
parent.level = 1
136137
}
137-
if n, ok := m.fullName(parent, tc.sub); ok != tc.ok {
138-
t.Errorf("for pattern %q, fullName(parent=%q, sub=%q) = %q, ok %v; want ok %v",
139-
tc.pattern, tc.parent, tc.sub, n, ok, tc.ok)
138+
if n, ok, partial := m.fullName(parent, tc.sub); ok != tc.ok || partial != tc.partial {
139+
t.Errorf("for pattern %q, fullName(parent=%q, sub=%q) = %q, ok %v partial %v; want ok %v partial %v",
140+
tc.pattern, tc.parent, tc.sub, n, ok, partial, tc.ok, tc.partial)
140141
}
141142
}
142143
}
@@ -178,7 +179,7 @@ func TestNaming(t *T) {
178179
}
179180

180181
for i, tc := range testCases {
181-
if got, _ := m.fullName(parent, tc.name); got != tc.want {
182+
if got, _, _ := m.fullName(parent, tc.name); got != tc.want {
182183
t.Errorf("%d:%s: got %q; want %q", i, tc.name, got, tc.want)
183184
}
184185
}

‎src/testing/sub_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package testing
77
import (
88
"bytes"
99
"fmt"
10-
"os"
1110
"regexp"
1211
"runtime"
1312
"strings"
@@ -602,7 +601,6 @@ func TestBenchmark(t *T) {
602601
res := Benchmark(func(b *B) {
603602
for i := 0; i < 5; i++ {
604603
b.Run("", func(b *B) {
605-
fmt.Fprintf(os.Stderr, "b.N: %v\n", b.N)
606604
for i := 0; i < b.N; i++ {
607605
time.Sleep(time.Millisecond)
608606
}

‎src/testing/testing.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ func tRunner(t *T, fn func(t *T)) {
763763
// must happen before the outer test function for t returns.
764764
func (t *T) Run(name string, f func(t *T)) bool {
765765
atomic.StoreInt32(&t.hasSub, 1)
766-
testName, ok := t.context.match.fullName(&t.common, name)
766+
testName, ok, _ := t.context.match.fullName(&t.common, name)
767767
if !ok {
768768
return true
769769
}

0 commit comments

Comments
 (0)