]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/dist: convert most remaining tests to use goTest
authorAustin Clements <austin@google.com>
Fri, 4 Nov 2022 18:54:04 +0000 (14:54 -0400)
committerAustin Clements <austin@google.com>
Wed, 16 Nov 2022 21:35:23 +0000 (21:35 +0000)
This converts most of the remaining manual "go test" command line
construction in cmd/dist to use the goTest abstraction and
registerTest.

At this point, the only remaining place that directly constructs go
test command lines is runHostTest.

This fixes a bug in the "nolibgcc:os/user" test. It was clearly
supposed to pass "-run=^Test[^CS]", but the logic to override the
"-run" flag for "nolibgcc:net" caused "nolibgcc:os/user" to pass
*both* "-run=^Test[^CS]" and "-run=". This was then rewritten into
just "-run=" by flattenCmdline, which caused all os/user tests to run,
and not actually skip the expensive tests as intended. (This is a
great example of why the new abstraction is much more robust than
command line construction.)

I traced all exec calls from cmd/dist on linux/amd64 and, other than
the fix to nolibgcc:os/user, this makes only no-op changes (such as
re-arranging the order of flags).

For #37486.

Change-Id: Ie8546bacc56640ea39f2804a87795c14a3fe4c7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/450018
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
src/cmd/dist/test.go

index e096c43806e2760af5e8e05f5192660eb46cf8ff..fe386d7765a7a7a8048dbc0dfd8b71c1bde7c5dc 100644 (file)
@@ -634,28 +634,23 @@ func (t *tester) registerTests() {
 
        // Test the os/user package in the pure-Go mode too.
        if !t.compileOnly {
-               t.tests = append(t.tests, distTest{
-                       name:    "osusergo",
-                       heading: "os/user with tag osusergo",
-                       fn: func(dt *distTest) error {
-                               t.addCmd(dt, "src", t.goTest(), t.timeout(300), "-tags=osusergo", "os/user")
-                               return nil
-                       },
-               })
+               t.registerTest("osusergo", "os/user with tag osusergo",
+                       &goTest{
+                               timeout: 300 * time.Second,
+                               tags:    []string{"osusergo"},
+                               pkg:     "os/user",
+                       })
        }
 
        // Test ios/amd64 for the iOS simulator.
        if goos == "darwin" && goarch == "amd64" && t.cgoEnabled {
-               t.tests = append(t.tests, distTest{
-                       name:    "amd64ios",
-                       heading: "GOOS=ios on darwin/amd64",
-                       fn: func(dt *distTest) error {
-                               cmd := t.addCmd(dt, "src", t.goTest(), t.timeout(300), "-run=SystemRoots", "crypto/x509")
-                               setEnv(cmd, "GOOS", "ios")
-                               setEnv(cmd, "CGO_ENABLED", "1")
-                               return nil
-                       },
-               })
+               t.registerTest("amd64ios", "GOOS=ios on darwin/amd64",
+                       &goTest{
+                               timeout:  300 * time.Second,
+                               runTests: "SystemRoots",
+                               env:      []string{"GOOS=ios", "CGO_ENABLED=1"},
+                               pkg:      "crypto/x509",
+                       })
        }
 
        if t.race {
@@ -664,18 +659,17 @@ func (t *tester) registerTests() {
 
        // Runtime CPU tests.
        if !t.compileOnly && goos != "js" { // js can't handle -cpu != 1
-               testName := "runtime:cpu124"
-               t.tests = append(t.tests, distTest{
-                       name:    testName,
-                       heading: "GOMAXPROCS=2 runtime -cpu=1,2,4 -quick",
-                       fn: func(dt *distTest) error {
-                               cmd := t.addCmd(dt, "src", t.goTest(), "-short=true", t.timeout(300), "runtime", "-cpu=1,2,4", "-quick")
+               t.registerTest("runtime:cpu124", "GOMAXPROCS=2 runtime -cpu=1,2,4 -quick",
+                       &goTest{
+                               timeout:   300 * time.Second,
+                               cpu:       "1,2,4",
+                               short:     true,
+                               testFlags: []string{"-quick"},
                                // We set GOMAXPROCS=2 in addition to -cpu=1,2,4 in order to test runtime bootstrap code,
                                // creation of first goroutines and first garbage collections in the parallel setting.
-                               setEnv(cmd, "GOMAXPROCS", "2")
-                               return nil
-                       },
-               })
+                               env: []string{"GOMAXPROCS=2"},
+                               pkg: "runtime",
+                       })
        }
 
        // morestack tests. We only run these on in long-test mode
@@ -712,17 +706,13 @@ func (t *tester) registerTests() {
                        goFlags := strings.Join(goFlagsList, " ")
 
                        for _, pkg := range pkgs {
-                               pkg := pkg
-                               testName := hook + ":" + pkg
-                               t.tests = append(t.tests, distTest{
-                                       name:    testName,
-                                       heading: "maymorestack=" + hook,
-                                       fn: func(dt *distTest) error {
-                                               cmd := t.addCmd(dt, "src", t.goTest(), t.timeout(600), pkg, "-short")
-                                               setEnv(cmd, "GOFLAGS", goFlags)
-                                               return nil
-                                       },
-                               })
+                               t.registerTest(hook+":"+pkg, "maymorestack="+hook,
+                                       &goTest{
+                                               timeout: 600 * time.Second,
+                                               short:   true,
+                                               env:     []string{"GOFLAGS=" + goFlags},
+                                               pkg:     pkg,
+                                       })
                        }
                }
        }
@@ -788,21 +778,18 @@ func (t *tester) registerTests() {
                        break
                }
 
-               pkg := pkg
-               var run string
+               // What matters is that the tests build and start up.
+               // Skip expensive tests, especially x509 TestSystemRoots.
+               run := "^Test[^CS]"
                if pkg == "net" {
                        run = "TestTCPStress"
                }
-               t.tests = append(t.tests, distTest{
-                       name:    "nolibgcc:" + pkg,
-                       heading: "Testing without libgcc.",
-                       fn: func(dt *distTest) error {
-                               // What matters is that the tests build and start up.
-                               // Skip expensive tests, especially x509 TestSystemRoots.
-                               t.addCmd(dt, "src", t.goTest(), "-ldflags=-linkmode=internal -libgcc=none", "-run=^Test[^CS]", pkg, t.runFlag(run))
-                               return nil
-                       },
-               })
+               t.registerTest("nolibgcc:"+pkg, "Testing without libgcc.",
+                       &goTest{
+                               ldflags:  "-linkmode=internal -libgcc=none",
+                               runTests: run,
+                               pkg:      pkg,
+                       })
        }
 
        // Stub out following test on alpine until 54354 resolved.
@@ -811,46 +798,38 @@ func (t *tester) registerTests() {
 
        // Test internal linking of PIE binaries where it is supported.
        if t.internalLinkPIE() && !disablePIE {
-               t.tests = append(t.tests, distTest{
-                       name:    "pie_internal",
-                       heading: "internal linking of -buildmode=pie",
-                       fn: func(dt *distTest) error {
-                               cmd := t.addCmd(dt, "src", t.goTest(), "reflect", "-buildmode=pie", "-ldflags=-linkmode=internal", t.timeout(60))
-                               setEnv(cmd, "CGO_ENABLED", "0")
-                               return nil
-                       },
-               })
+               t.registerTest("pie_internal", "internal linking of -buildmode=pie",
+                       &goTest{
+                               timeout:   60 * time.Second,
+                               buildmode: "pie",
+                               ldflags:   "-linkmode=internal",
+                               env:       []string{"CGO_ENABLED=0"},
+                               pkg:       "reflect",
+                       })
                // Also test a cgo package.
                if t.cgoEnabled && t.internalLink() && !disablePIE {
-                       t.tests = append(t.tests, distTest{
-                               name:    "pie_internal_cgo",
-                               heading: "internal linking of -buildmode=pie",
-                               fn: func(dt *distTest) error {
-                                       t.addCmd(dt, "src", t.goTest(), "os/user", "-buildmode=pie", "-ldflags=-linkmode=internal", t.timeout(60))
-                                       return nil
-                               },
-                       })
+                       t.registerTest("pie_internal_cgo", "internal linking of -buildmode=pie",
+                               &goTest{
+                                       timeout:   60 * time.Second,
+                                       buildmode: "pie",
+                                       ldflags:   "-linkmode=internal",
+                                       pkg:       "os/user",
+                               })
                }
        }
 
        // sync tests
        if goos != "js" { // js doesn't support -cpu=10
-               t.tests = append(t.tests, distTest{
-                       name:    "sync_cpu",
-                       heading: "sync -cpu=10",
-                       fn: func(dt *distTest) error {
-                               t.addCmd(dt, "src", t.goTest(), "sync", t.timeout(120), "-cpu=10", t.runFlag(""))
-                               return nil
-                       },
-               })
+               t.registerTest("sync_cpu", "sync -cpu=10",
+                       &goTest{
+                               timeout: 120 * time.Second,
+                               cpu:     "10",
+                               pkg:     "sync",
+                       })
        }
 
        if t.raceDetectorSupported() {
-               t.tests = append(t.tests, distTest{
-                       name:    "race",
-                       heading: "Testing race detector",
-                       fn:      t.raceTest,
-               })
+               t.registerRaceTests()
        }
 
        if t.cgoEnabled && !t.iOS() {
@@ -861,36 +840,19 @@ func (t *tester) registerTests() {
                        t.registerHostTest("cgo_fortran", "../misc/cgo/fortran", "misc/cgo/fortran", ".")
                }
                if t.hasSwig() && goos != "android" {
-                       t.tests = append(t.tests, distTest{
-                               name:    "swig_stdio",
-                               heading: "../misc/swig/stdio",
-                               fn: func(dt *distTest) error {
-                                       t.addCmd(dt, "misc/swig/stdio", t.goTest(), ".")
-                                       return nil
-                               },
-                       })
+                       t.registerTest("swig_stdio", "", &goTest{dir: "../misc/swig/stdio"})
                        if t.hasCxx() {
-                               t.tests = append(t.tests,
-                                       distTest{
-                                               name:    "swig_callback",
-                                               heading: "../misc/swig/callback",
-                                               fn: func(dt *distTest) error {
-                                                       t.addCmd(dt, "misc/swig/callback", t.goTest(), ".")
-                                                       return nil
-                                               },
-                                       },
-                                       distTest{
-                                               name:    "swig_callback_lto",
-                                               heading: "../misc/swig/callback",
-                                               fn: func(dt *distTest) error {
-                                                       cmd := t.addCmd(dt, "misc/swig/callback", t.goTest(), ".")
-                                                       setEnv(cmd, "CGO_CFLAGS", "-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option")
-                                                       setEnv(cmd, "CGO_CXXFLAGS", "-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option")
-                                                       setEnv(cmd, "CGO_LDFLAGS", "-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option")
-                                                       return nil
+                               t.registerTest("swig_callback", "", &goTest{dir: "../misc/swig/callback"})
+                               const cflags = "-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option"
+                               t.registerTest("swig_callback_lto", "",
+                                       &goTest{
+                                               dir: "../misc/swig/callback",
+                                               env: []string{
+                                                       "CGO_CFLAGS=" + cflags,
+                                                       "CGO_CXXFLAGS=" + cflags,
+                                                       "CGO_LDFLAGS=" + cflags,
                                                },
-                                       },
-                               )
+                                       })
                        }
                }
        }
@@ -1640,27 +1602,42 @@ func (t *tester) runFlag(rx string) string {
        return "-run=" + rx
 }
 
-func (t *tester) raceTest(dt *distTest) error {
-       t.addCmd(dt, "src", t.goTest(), "-race", t.runFlag("Output"), "runtime/race")
-       t.addCmd(dt, "src", t.goTest(), "-race", t.runFlag("TestParse|TestEcho|TestStdinCloseRace|TestClosedPipeRace|TestTypeRace|TestFdRace|TestFdReadRace|TestFileCloseRace"), "flag", "net", "os", "os/exec", "encoding/gob")
+func (t *tester) registerRaceTests() {
+       hdr := "Testing race detector"
+       t.registerTest("race:runtime/race", hdr,
+               &goTest{
+                       race:     true,
+                       runTests: "Output",
+                       pkg:      "runtime/race",
+               })
+       t.registerTest("race", hdr,
+               &goTest{
+                       race:     true,
+                       runTests: "TestParse|TestEcho|TestStdinCloseRace|TestClosedPipeRace|TestTypeRace|TestFdRace|TestFdReadRace|TestFileCloseRace",
+                       pkgs:     []string{"flag", "net", "os", "os/exec", "encoding/gob"},
+               })
        // We don't want the following line, because it
        // slows down all.bash (by 10 seconds on my laptop).
        // The race builder should catch any error here, but doesn't.
        // TODO(iant): Figure out how to catch this.
-       // t.addCmd(dt, "src", t.goTest(),  "-race", "-run=TestParallelTest", "cmd/go")
+       // t.registerTest("race:cmd/go", hdr, &goTest{race: true, runTests: "TestParallelTest", pkg: "cmd/go"})
        if t.cgoEnabled {
                // Building misc/cgo/test takes a long time.
                // There are already cgo-enabled packages being tested with the race detector.
                // We shouldn't need to redo all of misc/cgo/test too.
                // The race buildler will take care of this.
-               // cmd := t.addCmd(dt, "misc/cgo/test", t.goTest(), "-race")
-               // setEnv(cmd, "GOTRACEBACK", "2")
+               // t.registerTest("race:misc/cgo/test", hdr, &goTest{dir: "../misc/cgo/test", race: true, env: []string{"GOTRACEBACK=2"}})
        }
        if t.extLink() {
                // Test with external linking; see issue 9133.
-               t.addCmd(dt, "src", t.goTest(), "-race", "-ldflags=-linkmode=external", t.runFlag("TestParse|TestEcho|TestStdinCloseRace"), "flag", "os/exec")
+               t.registerTest("race:external", hdr,
+                       &goTest{
+                               race:     true,
+                               ldflags:  "-linkmode=external",
+                               runTests: "TestParse|TestEcho|TestStdinCloseRace",
+                               pkgs:     []string{"flag", "os/exec"},
+                       })
        }
-       return nil
 }
 
 var runtest struct {