]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/dist, test: convert test/run.go runner to a cmd/go test
authorDmitri Shuralyov <dmitshur@golang.org>
Wed, 8 Feb 2023 16:40:06 +0000 (11:40 -0500)
committerGopher Robot <gobot@golang.org>
Tue, 28 Feb 2023 01:11:37 +0000 (01:11 +0000)
As motivated on the issue, we want to move the functionality of the
run.go program to happen via a normal go test. Each .go test case in
the GOROOT/test directory gets a subtest, and cmd/go's support for
parallel test execution replaces run.go's own implementation thereof.

The goal of this change is to have fairly minimal and readable diff
while making an atomic changeover. The working directory is modified
during the test execution to be GOROOT/test as it was with run.go,
and most of the test struct and its run method are kept unchanged.
The next CL in the stack applies further simplifications and cleanups
that become viable.

There's no noticeable difference in test execution time: it takes around
60-80 seconds both before and after on my machine. Test caching, which
the previous runner lacked, can shorten the time significantly.

For #37486.
Fixes #56844.

Change-Id: I209619dc9d90e7529624e49c01efeadfbeb5c9ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/463276
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/dist/test.go
src/internal/testdir/testdir_test.go [moved from test/run.go with 93% similarity]
test/README.md
test/codegen/README

index bc58f0936bcea6f207074aafaa4554aaa75cf739..a906c0dbdbcfdeadaed12e1a9b77c1a91234545f 100644 (file)
@@ -18,7 +18,6 @@ import (
        "runtime"
        "strconv"
        "strings"
-       "sync"
        "time"
 )
 
@@ -594,6 +593,12 @@ func (t *tester) registerRaceBenchTest(pkg string) {
 }
 
 func (t *tester) registerTests() {
+       // registerStdTestSpecially tracks import paths in the standard library
+       // whose test registration happens in a special way.
+       registerStdTestSpecially := map[string]bool{
+               "internal/testdir": true, // Registered at the bottom with sharding.
+       }
+
        // Fast path to avoid the ~1 second of `go list std cmd` when
        // the caller lists specific tests to run. (as the continuous
        // build coordinator does).
@@ -621,10 +626,16 @@ func (t *tester) registerTests() {
                }
                pkgs := strings.Fields(string(all))
                for _, pkg := range pkgs {
+                       if registerStdTestSpecially[pkg] {
+                               continue
+                       }
                        t.registerStdTest(pkg)
                }
                if t.race {
                        for _, pkg := range pkgs {
+                               if registerStdTestSpecially[pkg] {
+                                       continue
+                               }
                                if t.packageHasBenchmarks(pkg) {
                                        t.registerRaceBenchTest(pkg)
                                }
@@ -907,12 +918,15 @@ func (t *tester) registerTests() {
                        nShards = n
                }
                for shard := 0; shard < nShards; shard++ {
-                       shard := shard
-                       t.tests = append(t.tests, distTest{
-                               name:    fmt.Sprintf("test:%d_%d", shard, nShards),
-                               heading: "../test",
-                               fn:      func(dt *distTest) error { return t.testDirTest(dt, shard, nShards) },
-                       })
+                       t.registerTest(
+                               fmt.Sprintf("test:%d_%d", shard, nShards),
+                               "../test",
+                               &goTest{
+                                       dir:       "internal/testdir",
+                                       testFlags: []string{fmt.Sprintf("-shard=%d", shard), fmt.Sprintf("-shards=%d", nShards)},
+                               },
+                               rtHostTest{},
+                       )
                }
        }
        // Only run the API check on fast development platforms.
@@ -1514,44 +1528,6 @@ func (t *tester) registerRaceTests() {
        }
 }
 
-var runtest struct {
-       sync.Once
-       exe string
-       err error
-}
-
-func (t *tester) testDirTest(dt *distTest, shard, shards int) error {
-       runtest.Do(func() {
-               f, err := os.CreateTemp("", "runtest-*.exe") // named exe for Windows, but harmless elsewhere
-               if err != nil {
-                       runtest.err = err
-                       return
-               }
-               f.Close()
-
-               runtest.exe = f.Name()
-               xatexit(func() {
-                       os.Remove(runtest.exe)
-               })
-
-               cmd := t.dirCmd("test", gorootBinGo, "build", "-o", runtest.exe, "run.go")
-               setEnv(cmd, "GOOS", gohostos)
-               setEnv(cmd, "GOARCH", gohostarch)
-               runtest.err = cmd.Run()
-       })
-       if runtest.err != nil {
-               return runtest.err
-       }
-       if t.compileOnly {
-               return nil
-       }
-       t.addCmd(dt, "test", runtest.exe,
-               fmt.Sprintf("--shard=%d", shard),
-               fmt.Sprintf("--shards=%d", shards),
-       )
-       return nil
-}
-
 // cgoPackages is the standard packages that use cgo.
 var cgoPackages = []string{
        "net",
similarity index 93%
rename from test/run.go
rename to src/internal/testdir/testdir_test.go
index 611fb02d72989cdc2bea259187960553d0298c99..f6b8e1da893bcd035874b0e5d57c461ef1743031 100644 (file)
@@ -1,11 +1,9 @@
-// skip
-
 // Copyright 2012 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.
 
-// Run runs tests in the test directory.
-package main
+// Package testdir_test runs tests in the GOROOT/test directory.
+package testdir_test
 
 import (
        "bytes"
@@ -30,6 +28,7 @@ import (
        "strconv"
        "strings"
        "sync"
+       "testing"
        "time"
        "unicode"
 )
@@ -37,10 +36,7 @@ import (
 var (
        verbose        = flag.Bool("v", false, "verbose. if set, parallelism is set to 1.")
        keep           = flag.Bool("k", false, "keep. keep temporary directory.")
-       numParallel    = flag.Int("n", runtime.NumCPU(), "number of parallel tests to run")
-       summary        = flag.Bool("summary", false, "show summary of results")
        allCodegen     = flag.Bool("all_codegen", defaultAllCodeGen(), "run all goos/goarch for codegen")
-       showSkips      = flag.Bool("show_skips", false, "show skipped tests")
        runSkips       = flag.Bool("run_skips", false, "run skipped tests (ignore skip and build tags)")
        linkshared     = flag.Bool("linkshared", false, "")
        updateErrors   = flag.Bool("update_errors", false, "update error messages in test file based on compiler output")
@@ -93,114 +89,45 @@ var (
        // TODO(bradfitz): just use all directories?
        dirs = []string{".", "ken", "chan", "interface", "syntax", "dwarf", "fixedbugs", "codegen", "runtime", "abi", "typeparam", "typeparam/mdempsky"}
 
-       // ratec controls the max number of tests running at a time.
-       ratec chan bool
-
-       // toRun is the channel of tests to run.
-       // It is nil until the first test is started.
-       toRun chan *test
-
        // rungatec controls the max number of runoutput tests
        // executed in parallel as they can each consume a lot of memory.
        rungatec chan bool
 )
 
-// maxTests is an upper bound on the total number of tests.
-// It is used as a channel buffer size to make sure sends don't block.
-const maxTests = 5000
-
-func main() {
-       flag.Parse()
-
-       findExecCmd()
-
-       // Disable parallelism if printing or if using a simulator.
-       if *verbose || len(findExecCmd()) > 0 {
-               *numParallel = 1
-               *runoutputLimit = 1
+// Test is the main entrypoint that runs tests in the GOROOT/test directory.
+// Each .go file test case in GOROOT/test is registered as a subtest.
+func Test(t *testing.T) {
+       // TODO(go.dev/issue/56844): There's only a few subprocesses started, so it might be viable/safe enough to set cmd.Dir of subprocessess instead of doing it globally here.
+       err := os.Chdir("../../../test")
+       if err != nil {
+               t.Fatal(err)
        }
+       t.Cleanup(func() { os.Chdir("../src/internal/testdir") })
 
-       ratec = make(chan bool, *numParallel)
        rungatec = make(chan bool, *runoutputLimit)
 
-       var tests []*test
-       if flag.NArg() > 0 {
-               for _, arg := range flag.Args() {
-                       if arg == "-" || arg == "--" {
-                               // Permit running:
-                               // $ go run run.go - env.go
-                               // $ go run run.go -- env.go
-                               // $ go run run.go - ./fixedbugs
-                               // $ go run run.go -- ./fixedbugs
-                               continue
-                       }
-                       if fi, err := os.Stat(arg); err == nil && fi.IsDir() {
-                               for _, baseGoFile := range goFiles(arg) {
-                                       tests = append(tests, startTest(arg, baseGoFile))
+       for _, dir := range dirs {
+               for _, goFile := range goFiles(dir) {
+                       test := test{dir: dir, gofile: goFile}
+                       t.Run(path.Join(test.dir, test.gofile), func(t *testing.T) {
+                               t.Parallel()
+                               test.T = t
+                               test.run()
+                               if e, isSkip := test.err.(skipError); isSkip {
+                                       t.Fatal("unexpected skip:", e)
                                }
-                       } else if strings.HasSuffix(arg, ".go") {
-                               dir, file := filepath.Split(arg)
-                               tests = append(tests, startTest(dir, file))
-                       } else {
-                               log.Fatalf("can't yet deal with non-directory and non-go file %q", arg)
-                       }
-               }
-       } else {
-               for _, dir := range dirs {
-                       for _, baseGoFile := range goFiles(dir) {
-                               tests = append(tests, startTest(dir, baseGoFile))
-                       }
-               }
-       }
-
-       failed := false
-       resCount := map[string]int{}
-       for _, test := range tests {
-               <-test.donec
-               status := "ok  "
-               errStr := ""
-               if e, isSkip := test.err.(skipError); isSkip {
-                       test.err = nil
-                       errStr = "unexpected skip for " + path.Join(test.dir, test.gofile) + ": " + string(e)
-                       status = "FAIL"
-               }
-               if test.err != nil {
-                       errStr = test.err.Error()
-                       if test.expectFail {
-                               errStr += " (expected)"
-                       } else {
-                               status = "FAIL"
-                       }
-               } else if test.expectFail {
-                       status = "FAIL"
-                       errStr = "unexpected success"
-               }
-               if status == "FAIL" {
-                       failed = true
-               }
-               resCount[status]++
-               dt := fmt.Sprintf("%.3fs", test.dt.Seconds())
-               if status == "FAIL" {
-                       fmt.Printf("# go run run.go -- %s\n%s\nFAIL\t%s\t%s\n",
-                               path.Join(test.dir, test.gofile),
-                               errStr, test.goFileName(), dt)
-                       continue
-               }
-               if !*verbose {
-                       continue
-               }
-               fmt.Printf("%s\t%s\t%s\n", status, test.goFileName(), dt)
-       }
-
-       if *summary {
-               for k, v := range resCount {
-                       fmt.Printf("%5d %s\n", v, k)
+                               if test.err != nil {
+                                       if test.expectFail {
+                                               t.Log(test.err.Error() + " (expected)")
+                                       } else {
+                                               t.Fatal(test.err)
+                                       }
+                               } else if test.expectFail {
+                                       t.Fatal("unexpected success")
+                               }
+                       })
                }
        }
-
-       if failed {
-               os.Exit(1)
-       }
 }
 
 // goTool reports the path of the go tool to use to run the tests.
@@ -220,7 +147,7 @@ func goTool() string {
 }
 
 func shardMatch(name string) bool {
-       if *shards == 0 {
+       if *shards <= 1 {
                return true
        }
        h := fnv.New32()
@@ -334,11 +261,9 @@ func (s skipError) Error() string { return string(s) }
 
 // test holds the state of a test.
 type test struct {
-       dir, gofile string
-       donec       chan bool // closed when done
-       dt          time.Duration
+       *testing.T
 
-       src string
+       dir, gofile string
 
        tempDir string
        err     error
@@ -375,39 +300,6 @@ func (t *test) initExpectFail() {
        }
 }
 
-func startTest(dir, gofile string) *test {
-       t := &test{
-               dir:    dir,
-               gofile: gofile,
-               donec:  make(chan bool, 1),
-       }
-       if toRun == nil {
-               toRun = make(chan *test, maxTests)
-               go runTests()
-       }
-       select {
-       case toRun <- t:
-       default:
-               panic("toRun buffer size (maxTests) is too small")
-       }
-       return t
-}
-
-// runTests runs tests in parallel, but respecting the order they
-// were enqueued on the toRun channel.
-func runTests() {
-       for {
-               ratec <- true
-               t := <-toRun
-               go func() {
-                       t.run()
-                       <-ratec
-               }()
-       }
-}
-
-var cwd, _ = os.Getwd()
-
 func (t *test) goFileName() string {
        return filepath.Join(t.dir, t.gofile)
 }
@@ -550,10 +442,6 @@ func (ctxt *context) match(name string) bool {
        return false
 }
 
-func init() {
-       checkShouldTest()
-}
-
 // goGcflags returns the -gcflags argument to use with go build / go run.
 // This must match the flags used for building the standard library,
 // or else the commands will rebuild any needed packages (like runtime)
@@ -570,25 +458,18 @@ var errTimeout = errors.New("command exceeded time limit")
 
 // run runs a test.
 func (t *test) run() {
-       start := time.Now()
-       defer func() {
-               t.dt = time.Since(start)
-               close(t.donec)
-       }()
-
        srcBytes, err := ioutil.ReadFile(t.goFileName())
        if err != nil {
                t.err = err
                return
-       }
-       t.src = string(srcBytes)
-       if t.src[0] == '\n' {
+       } else if bytes.HasPrefix(srcBytes, []byte{'\n'}) {
                t.err = skipError("starts with newline")
                return
        }
+       src := string(srcBytes)
 
        // Execution recipe stops at first blank line.
-       action, _, ok := strings.Cut(t.src, "\n\n")
+       action, _, ok := strings.Cut(src, "\n\n")
        if !ok {
                t.err = fmt.Errorf("double newline ending execution recipe not found in %s", t.goFileName())
                return
@@ -600,15 +481,12 @@ func (t *test) run() {
        action = strings.TrimPrefix(action, "//")
 
        // Check for build constraints only up to the actual code.
-       header, _, ok := strings.Cut(t.src, "\npackage")
+       header, _, ok := strings.Cut(src, "\npackage")
        if !ok {
                header = action // some files are intentionally malformed
        }
        if ok, why := shouldTest(header, goos, goarch); !ok {
-               if *showSkips {
-                       fmt.Printf("%-20s %-20s: %s\n", "skip", t.goFileName(), why)
-               }
-               return
+               t.Skip(why)
        }
 
        var args, flags, runenv []string
@@ -642,7 +520,7 @@ func (t *test) run() {
                if *runSkips {
                        break
                }
-               return
+               t.Skip("skip")
        default:
                t.err = skipError("skipped; unknown pattern: " + action)
                return
@@ -806,6 +684,10 @@ func (t *test) run() {
                return filename
        }
 
+       cwd, err := os.Getwd()
+       if err != nil {
+               t.Fatal(err)
+       }
        long := filepath.Join(cwd, t.goFileName())
        switch action {
        default:
@@ -1284,20 +1166,17 @@ func (t *test) run() {
        }
 }
 
+var execCmdOnce sync.Once
 var execCmd []string
 
 func findExecCmd() []string {
-       if execCmd != nil {
-               return execCmd
-       }
-       execCmd = []string{} // avoid work the second time
-       if goos == runtime.GOOS && goarch == runtime.GOARCH {
-               return execCmd
-       }
-       path, err := exec.LookPath(fmt.Sprintf("go_%s_%s_exec", goos, goarch))
-       if err == nil {
-               execCmd = []string{path}
-       }
+       execCmdOnce.Do(func() {
+               if goos == runtime.GOOS && goarch == runtime.GOARCH {
+                       // Do nothing.
+               } else if path, err := exec.LookPath(fmt.Sprintf("go_%s_%s_exec", goos, goarch)); err == nil {
+                       execCmd = []string{path}
+               }
+       })
        return execCmd
 }
 
@@ -1904,14 +1783,18 @@ func defaultRunOutputLimit() int {
        return cpu
 }
 
-// checkShouldTest runs sanity checks on the shouldTest function.
-func checkShouldTest() {
+func TestShouldTest(t *testing.T) {
+       if *shard != 0 {
+               t.Skipf("nothing to test on shard index %d", *shard)
+       }
+
        assert := func(ok bool, _ string) {
+               t.Helper()
                if !ok {
-                       panic("fail")
+                       t.Error("test case failed")
                }
        }
-       assertNot := func(ok bool, _ string) { assert(!ok, "") }
+       assertNot := func(ok bool, _ string) { t.Helper(); assert(!ok, "") }
 
        // Simple tests.
        assert(shouldTest("// +build linux", "linux", "arm"))
index 432d36b653f62d4f9583370abb1f7793ce17c955..7e3948f94749df3a6526a00ab35bc920dde955b9 100644 (file)
@@ -4,11 +4,11 @@ They are run as part of all.bash.
 
 To run just these tests, execute:
 
-    ../bin/go run run.go
+       ../bin/go test internal/testdir
 
 To run just tests from specified files in this directory, execute:
 
-    ../bin/go run run.go -- file1.go file2.go ...
+       ../bin/go test internal/testdir -run='Test/(file1.go|file2.go|...)'
 
 Standard library tests should be written as regular Go tests in the appropriate package.
 
index b803fe585fa5792794e5f038c78d25393c1abda9..1d68780394121be973d55c917350fc65fa07a235 100644 (file)
@@ -11,23 +11,22 @@ compiler.
 The test harness compiles Go code inside files in this directory and
 matches the generated assembly (the output of `go tool compile -S`)
 against a set of regexps to be specified in comments that follow a
-special syntax (described below). The test driver is implemented as a
-step of the top-level test/run.go suite, called "asmcheck".
+special syntax (described below). The test driver is implemented as
+an action within the GOROOT/test test suite, called "asmcheck".
 
 The codegen harness is part of the all.bash test suite, but for
 performance reasons only the codegen tests for the host machine's
 GOARCH are enabled by default, and only on GOOS=linux.
 
 To perform comprehensive tests for all the supported architectures
-(even on a non-Linux system), one can run the following command
+(even on a non-Linux system), one can run the following command:
 
-  $ ../bin/go run run.go -all_codegen -v codegen
+  $ ../../bin/go test internal/testdir -run='Test/codegen' -all_codegen -v
 
-in the top-level test directory. This is recommended after any change
-that affect the compiler's code.
+This is recommended after any change that affect the compiler's code.
 
 The test harness compiles the tests with the same go toolchain that is
-used to run run.go. After writing tests for a newly added codegen
+used to run the test. After writing tests for a newly added codegen
 transformation, it can be useful to first run the test harness with a
 toolchain from a released Go version (and verify that the new tests
 fail), and then re-runnig the tests using the devel toolchain.