Skip to content

Commit ec745fc

Browse files
committed
test: make test/run.go support sharding
Also modifies 'dist test' to use that sharding, and removes some old temporary stuff from dist test which are no longer required. 'dist test' now also supports running a list of tests given in arguments, mutually exclusive with the existing -run=REGEXP flag. The hacky fast paths for avoiding the 1 second "go list" latency are now removed and only apply to the case where partial tests are run via args, instead of regex. The build coordinator will use both styles for awhile. (the statically-sharded ARM builders on scaleway will continue to use regexps, but the dynamically-shared builders on GCE will use the list of tests) Updates #10029 Change-Id: I557800a54dfa6f3b5100ef4c26fe397ba5189813 Reviewed-on: https://go-review.googlesource.com/10688 Reviewed-by: Andrew Gerrand <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 54789ef commit ec745fc

File tree

2 files changed

+77
-82
lines changed

2 files changed

+77
-82
lines changed

src/cmd/dist/test.go

+62-81
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"os/exec"
1515
"path/filepath"
1616
"regexp"
17-
"runtime"
1817
"strconv"
1918
"strings"
2019
"time"
@@ -29,7 +28,7 @@ func cmdtest() {
2928
flag.StringVar(&t.runRxStr, "run", os.Getenv("GOTESTONLY"),
3029
"run only those tests matching the regular expression; empty means to run all. "+
3130
"Special exception: if the string begins with '!', the match is inverted.")
32-
xflagparse(0)
31+
xflagparse(-1) // any number of args
3332
t.run()
3433
}
3534

@@ -40,8 +39,9 @@ type tester struct {
4039
keepGoing bool
4140
runRxStr string
4241
runRx *regexp.Regexp
43-
runRxWant bool
44-
banner string // prefix, or "" for none
42+
runRxWant bool // want runRx to match (true) or not match (false)
43+
runNames []string // tests to run, exclusive with runRx; empty means all
44+
banner string // prefix, or "" for none
4545

4646
goroot string
4747
goarch string
@@ -83,6 +83,10 @@ func (t *tester) run() {
8383
log.Fatalf("Error running go env CGO_ENABLED: %v", err)
8484
}
8585
t.cgoEnabled, _ = strconv.ParseBool(strings.TrimSpace(string(slurp)))
86+
if flag.NArg() > 0 && t.runRxStr != "" {
87+
log.Fatalf("the -run regular expression flag is mutually exclusive with test name arguments")
88+
}
89+
t.runNames = flag.Args()
8690

8791
if t.hasBash() {
8892
if _, err := exec.LookPath("time"); err == nil {
@@ -135,13 +139,6 @@ func (t *tester) run() {
135139
}
136140

137141
if t.runRxStr != "" {
138-
// Temporary (2015-05-14) special case for "std",
139-
// which the plan9 builder was using for ages. Delete
140-
// this once we update dashboard/builders.go to use a
141-
// regexp instead.
142-
if runtime.GOOS == "plan9" && t.runRxStr == "std" {
143-
t.runRxStr = "^go_test:"
144-
}
145142
if t.runRxStr[0] == '!' {
146143
t.runRxWant = false
147144
t.runRxStr = t.runRxStr[1:]
@@ -164,10 +161,16 @@ func (t *tester) run() {
164161
// at least runtime/debug test will fail.
165162
os.Unsetenv("GOROOT_FINAL")
166163

164+
for _, name := range t.runNames {
165+
if !t.isRegisteredTestName(name) {
166+
log.Fatalf("unknown test %q", name)
167+
}
168+
}
169+
167170
var lastHeading string
168171
ok := true
169172
for _, dt := range t.tests {
170-
if t.runRx != nil && (t.runRx.MatchString(dt.name) != t.runRxWant) {
173+
if !t.shouldRunTest(dt.name) {
171174
t.partial = true
172175
continue
173176
}
@@ -197,6 +200,21 @@ func (t *tester) run() {
197200
}
198201
}
199202

203+
func (t *tester) shouldRunTest(name string) bool {
204+
if t.runRx != nil {
205+
return t.runRx.MatchString(name) == t.runRxWant
206+
}
207+
if len(t.runNames) == 0 {
208+
return true
209+
}
210+
for _, runName := range t.runNames {
211+
if runName == name {
212+
return true
213+
}
214+
}
215+
return false
216+
}
217+
200218
func (t *tester) timeout(sec int) string {
201219
return "-timeout=" + fmt.Sprint(time.Duration(sec)*time.Second*time.Duration(t.timeoutScale))
202220
}
@@ -235,36 +253,25 @@ func (t *tester) registerStdTest(pkg string) {
235253
})
236254
}
237255

238-
// validStdPkg reports whether pkg looks like a standard library package name.
239-
// Notably, it's not blank and doesn't contain regexp characters.
240-
func validStdPkg(pkg string) bool {
241-
if pkg == "" {
242-
return false
243-
}
244-
for _, r := range pkg {
245-
switch {
246-
case 'a' <= r && r <= 'z':
247-
case 'A' <= r && r <= 'Z':
248-
case '0' <= r && r <= '9':
249-
case r == '_':
250-
case r == '/':
251-
default:
252-
return false
253-
}
254-
}
255-
return true
256-
}
257-
258256
func (t *tester) registerTests() {
259257
// Fast path to avoid the ~1 second of `go list std cmd` when
260-
// the caller passed -run=^go_test:foo/bar$ (as the continuous
258+
// the caller lists specific tests to run. (as the continuous
261259
// build coordinator does).
262-
if strings.HasPrefix(t.runRxStr, "^go_test:") && strings.HasSuffix(t.runRxStr, "$") {
263-
pkg := strings.TrimPrefix(t.runRxStr, "^go_test:")
264-
pkg = strings.TrimSuffix(pkg, "$")
265-
if validStdPkg(pkg) {
260+
if len(t.runNames) > 0 {
261+
for _, name := range t.runNames {
262+
if strings.HasPrefix(name, "go_test:") {
263+
t.registerStdTest(strings.TrimPrefix(name, "go_test:"))
264+
}
265+
}
266+
} else {
267+
// Use a format string to only list packages and commands that have tests.
268+
const format = "{{if (or .TestGoFiles .XTestGoFiles)}}{{.ImportPath}}{{end}}"
269+
all, err := exec.Command("go", "list", "-f", format, "std", "cmd").Output()
270+
if err != nil {
271+
log.Fatalf("Error running go list std cmd: %v", err)
272+
}
273+
for _, pkg := range strings.Fields(string(all)) {
266274
t.registerStdTest(pkg)
267-
return
268275
}
269276
}
270277

@@ -372,14 +379,15 @@ func (t *tester) registerTests() {
372379
t.registerTest("bench_go1", "../test/bench/go1", "go", "test")
373380
}
374381
if t.goos != "android" && !t.iOS() {
375-
// TODO(bradfitz): shard down into these tests, as
376-
// this is one of the slowest (and most shardable)
377-
// tests.
378-
t.tests = append(t.tests, distTest{
379-
name: "test",
380-
heading: "../test",
381-
fn: t.testDirTest,
382-
})
382+
const nShards = 5
383+
for shard := 0; shard < nShards; shard++ {
384+
shard := shard
385+
t.tests = append(t.tests, distTest{
386+
name: fmt.Sprintf("test:%d_%d", shard, nShards),
387+
heading: "../test",
388+
fn: func() error { return t.testDirTest(shard, nShards) },
389+
})
390+
}
383391
}
384392
if t.goos != "nacl" && t.goos != "android" && !t.iOS() {
385393
t.tests = append(t.tests, distTest{
@@ -390,36 +398,6 @@ func (t *tester) registerTests() {
390398
},
391399
})
392400
}
393-
394-
// Register the standard library tests lasts, to avoid the ~1 second latency
395-
// of running `go list std cmd` if we're running a specific test.
396-
// Now we know the names of all the other tests registered so far.
397-
if !t.wantSpecificRegisteredTest() {
398-
// Use a format string to only list packages and commands that have tests.
399-
const format = "{{if (or .TestGoFiles .XTestGoFiles)}}{{.ImportPath}}{{end}}"
400-
all, err := exec.Command("go", "list", "-f", format, "std", "cmd").Output()
401-
if err != nil {
402-
log.Fatalf("Error running go list std cmd: %v", err)
403-
}
404-
// Put the standard library tests first.
405-
orig := t.tests
406-
t.tests = nil
407-
for _, pkg := range strings.Fields(string(all)) {
408-
t.registerStdTest(pkg)
409-
}
410-
t.tests = append(t.tests, orig...)
411-
}
412-
}
413-
414-
// wantSpecificRegisteredTest reports whether the caller is requesting a
415-
// run of a specific test via the flag -run=^TESTNAME$ (as is done by the
416-
// continuous build coordinator).
417-
func (t *tester) wantSpecificRegisteredTest() bool {
418-
if !strings.HasPrefix(t.runRxStr, "^") || !strings.HasSuffix(t.runRxStr, "$") {
419-
return false
420-
}
421-
test := t.runRxStr[1 : len(t.runRxStr)-1]
422-
return t.isRegisteredTestName(test)
423401
}
424402

425403
// isRegisteredTestName reports whether a test named testName has already
@@ -437,6 +415,9 @@ func (t *tester) registerTest(name, dirBanner, bin string, args ...string) {
437415
if bin == "time" && !t.haveTime {
438416
bin, args = args[0], args[1:]
439417
}
418+
if t.isRegisteredTestName(name) {
419+
panic("duplicate registered test name " + name)
420+
}
440421
t.tests = append(t.tests, distTest{
441422
name: name,
442423
heading: dirBanner,
@@ -716,7 +697,7 @@ func (t *tester) raceTest() error {
716697
return nil
717698
}
718699

719-
func (t *tester) testDirTest() error {
700+
func (t *tester) testDirTest(shard, shards int) error {
720701
const runExe = "runtest.exe" // named exe for Windows, but harmless elsewhere
721702
cmd := t.dirCmd("test", "go", "build", "-o", runExe, "run.go")
722703
cmd.Env = mergeEnvLists([]string{"GOOS=" + t.gohostos, "GOARCH=" + t.gohostarch, "GOMAXPROCS="}, os.Environ())
@@ -725,10 +706,10 @@ func (t *tester) testDirTest() error {
725706
}
726707
absExe := filepath.Join(cmd.Dir, runExe)
727708
defer os.Remove(absExe)
728-
if t.haveTime {
729-
return t.dirCmd("test", "time", absExe).Run()
730-
}
731-
return t.dirCmd("test", absExe).Run()
709+
return t.dirCmd("test", absExe,
710+
fmt.Sprintf("--shard=%d", shard),
711+
fmt.Sprintf("--shards=%d", shards),
712+
).Run()
732713
}
733714

734715
// mergeEnvLists merges the two environment lists such that

test/run.go

+15-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
"errors"
1616
"flag"
1717
"fmt"
18+
"hash/fnv"
19+
"io"
1820
"io/ioutil"
1921
"log"
2022
"os"
@@ -37,6 +39,9 @@ var (
3739
showSkips = flag.Bool("show_skips", false, "show skipped tests")
3840
updateErrors = flag.Bool("update_errors", false, "update error messages in test file based on compiler output")
3941
runoutputLimit = flag.Int("l", defaultRunOutputLimit(), "number of parallel runoutput tests to run")
42+
43+
shard = flag.Int("shard", 0, "shard index to run. Only applicable if -shards is non-zero.")
44+
shards = flag.Int("shards", 0, "number of shards. If 0, all tests are run. This is used by the continuous build.")
4045
)
4146

4247
var (
@@ -162,14 +167,23 @@ func toolPath(name string) string {
162167
return p
163168
}
164169

170+
func shardMatch(name string) bool {
171+
if *shards == 0 {
172+
return true
173+
}
174+
h := fnv.New32()
175+
io.WriteString(h, name)
176+
return int(h.Sum32()%uint32(*shards)) == *shard
177+
}
178+
165179
func goFiles(dir string) []string {
166180
f, err := os.Open(dir)
167181
check(err)
168182
dirnames, err := f.Readdirnames(-1)
169183
check(err)
170184
names := []string{}
171185
for _, name := range dirnames {
172-
if !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go") {
186+
if !strings.HasPrefix(name, ".") && strings.HasSuffix(name, ".go") && shardMatch(name) {
173187
names = append(names, name)
174188
}
175189
}

0 commit comments

Comments
 (0)