From: Bryan C. Mills Date: Wed, 5 Oct 2022 19:21:06 +0000 (-0400) Subject: os/exec: parallelize more tests X-Git-Tag: go1.20rc1~716 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=515e3de2999b23da28e6d15ac485bfdd299ec83a;p=gostls13.git os/exec: parallelize more tests This cuts the wall duration for 'go test os/exec' and 'go test -race os/exec' roughly in half on my machine, which is an even more significant speedup with a high '-count'. For better or for worse, it may also increase the repro rate of #34988. Tests that use Setenv or Chdir or check for FDs opened during the test still cannot be parallelized, but they are only a few of those. Change-Id: I8d284d8bff05787853f825ef144aeb7a4126847f Reviewed-on: https://go-review.googlesource.com/c/go/+/439196 TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills --- diff --git a/src/os/exec/dot_test.go b/src/os/exec/dot_test.go index eeb59f13ef..66c92f7abd 100644 --- a/src/os/exec/dot_test.go +++ b/src/os/exec/dot_test.go @@ -24,6 +24,7 @@ var pathVar string = func() string { func TestLookPath(t *testing.T) { testenv.MustHaveExec(t) + // Not parallel: uses os.Chdir and t.Setenv. tmpDir := filepath.Join(t.TempDir(), "testdir") if err := os.Mkdir(tmpDir, 0777); err != nil { diff --git a/src/os/exec/env_test.go b/src/os/exec/env_test.go index 112f1e654a..cd5ba1e85f 100644 --- a/src/os/exec/env_test.go +++ b/src/os/exec/env_test.go @@ -10,6 +10,8 @@ import ( ) func TestDedupEnv(t *testing.T) { + t.Parallel() + tests := []struct { noCase bool in []string diff --git a/src/os/exec/exec_posix_test.go b/src/os/exec/exec_posix_test.go index f0401377e8..d366840bb1 100644 --- a/src/os/exec/exec_posix_test.go +++ b/src/os/exec/exec_posix_test.go @@ -49,6 +49,7 @@ func TestCredentialNoSetGroups(t *testing.T) { maySkipHelperCommand("echo") t.Skip("unsupported on Android") } + t.Parallel() u, err := user.Current() if err != nil { @@ -186,6 +187,8 @@ func TestImplicitPWD(t *testing.T) { // (This checks that the implementation for https://go.dev/issue/50599 doesn't // break existing users who may have explicitly mismatched the PWD variable.) func TestExplicitPWD(t *testing.T) { + t.Parallel() + maySkipHelperCommand("pwd") testenv.MustHaveSymlink(t) diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index 33f7022a6d..dc8aebd9aa 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -290,6 +290,8 @@ func cmdYes(args ...string) { } func TestEcho(t *testing.T) { + t.Parallel() + bs, err := helperCommand(t, "echo", "foo bar", "baz").Output() if err != nil { t.Errorf("echo: %v", err) @@ -300,6 +302,8 @@ func TestEcho(t *testing.T) { } func TestCommandRelativeName(t *testing.T) { + t.Parallel() + cmd := helperCommand(t, "echo", "foo") // Run our own binary as a relative path @@ -328,6 +332,8 @@ func TestCommandRelativeName(t *testing.T) { } func TestCatStdin(t *testing.T) { + t.Parallel() + // Cat, testing stdin and stdout. input := "Input string\nLine 2" p := helperCommand(t, "cat") @@ -343,6 +349,8 @@ func TestCatStdin(t *testing.T) { } func TestEchoFileRace(t *testing.T) { + t.Parallel() + cmd := helperCommand(t, "echo") stdin, err := cmd.StdinPipe() if err != nil { @@ -363,6 +371,8 @@ func TestEchoFileRace(t *testing.T) { } func TestCatGoodAndBadFile(t *testing.T) { + t.Parallel() + // Testing combined output and error values. bs, err := helperCommand(t, "cat", "/bogus/file.foo", "exec_test.go").CombinedOutput() if _, ok := err.(*exec.ExitError); !ok { @@ -381,6 +391,8 @@ func TestCatGoodAndBadFile(t *testing.T) { } func TestNoExistExecutable(t *testing.T) { + t.Parallel() + // Can't run a non-existent executable err := exec.Command("/no-exist-executable").Run() if err == nil { @@ -389,6 +401,8 @@ func TestNoExistExecutable(t *testing.T) { } func TestExitStatus(t *testing.T) { + t.Parallel() + // Test that exit values are returned correctly cmd := helperCommand(t, "exit", "42") err := cmd.Run() @@ -407,6 +421,8 @@ func TestExitStatus(t *testing.T) { } func TestExitCode(t *testing.T) { + t.Parallel() + // Test that exit code are returned correctly cmd := helperCommand(t, "exit", "42") cmd.Run() @@ -459,6 +475,8 @@ func TestExitCode(t *testing.T) { } func TestPipes(t *testing.T) { + t.Parallel() + check := func(what string, err error) { if err != nil { t.Fatalf("%s: %v", what, err) @@ -513,6 +531,8 @@ const stdinCloseTestString = "Some test string." // Issue 6270. func TestStdinClose(t *testing.T) { + t.Parallel() + check := func(what string, err error) { if err != nil { t.Fatalf("%s: %v", what, err) @@ -544,6 +564,8 @@ func TestStdinClose(t *testing.T) { // This test is run by cmd/dist under the race detector to verify that // the race detector no longer reports any problems. func TestStdinCloseRace(t *testing.T) { + t.Parallel() + cmd := helperCommand(t, "stdinClose") stdin, err := cmd.StdinPipe() if err != nil { @@ -582,6 +604,7 @@ func TestPipeLookPathLeak(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("we don't currently suppore counting open handles on windows") } + // Not parallel: checks for leaked file descriptors openFDs := func() []uintptr { var fds []uintptr @@ -610,6 +633,10 @@ func TestPipeLookPathLeak(t *testing.T) { } func TestExtraFiles(t *testing.T) { + if testing.Short() { + t.Skipf("skipping test in short mode that would build a helper binary") + } + if haveUnexpectedFDs { // The point of this test is to make sure that any // descriptors we open are marked close-on-exec. @@ -742,6 +769,8 @@ func TestExtraFilesRace(t *testing.T) { maySkipHelperCommand("describefiles") t.Skip("no operating system support; skipping") } + t.Parallel() + listen := func() net.Listener { ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -793,7 +822,6 @@ func TestExtraFilesRace(t *testing.T) { for _, f := range cb.ExtraFiles { f.Close() } - } } @@ -809,8 +837,12 @@ func (delayedInfiniteReader) Read(b []byte) (int, error) { // Issue 9173: ignore stdin pipe writes if the program completes successfully. func TestIgnorePipeErrorOnSuccess(t *testing.T) { + t.Parallel() + testWith := func(r io.Reader) func(*testing.T) { return func(t *testing.T) { + t.Parallel() + cmd := helperCommand(t, "echo", "foo") var out strings.Builder cmd.Stdin = r @@ -834,6 +866,8 @@ func (w *badWriter) Write(data []byte) (int, error) { } func TestClosePipeOnCopyError(t *testing.T) { + t.Parallel() + cmd := helperCommand(t, "yes") cmd.Stdout = new(badWriter) err := cmd.Run() @@ -843,6 +877,8 @@ func TestClosePipeOnCopyError(t *testing.T) { } func TestOutputStderrCapture(t *testing.T) { + t.Parallel() + cmd := helperCommand(t, "stderrfail") _, err := cmd.Output() ee, ok := err.(*exec.ExitError) @@ -857,6 +893,8 @@ func TestOutputStderrCapture(t *testing.T) { } func TestContext(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithCancel(context.Background()) c := helperCommandContext(t, ctx, "pipetest") stdin, err := c.StdinPipe() @@ -950,6 +988,8 @@ func TestContextCancel(t *testing.T) { // test that environment variables are de-duped. func TestDedupEnvEcho(t *testing.T) { + t.Parallel() + cmd := helperCommand(t, "echoenv", "FOO") cmd.Env = append(cmd.Environ(), "FOO=bad", "FOO=good") out, err := cmd.CombinedOutput() @@ -962,6 +1002,8 @@ func TestDedupEnvEcho(t *testing.T) { } func TestString(t *testing.T) { + t.Parallel() + echoPath, err := exec.LookPath("echo") if err != nil { t.Skip(err) @@ -984,10 +1026,13 @@ func TestString(t *testing.T) { } func TestStringPathNotResolved(t *testing.T) { + t.Parallel() + _, err := exec.LookPath("makemeasandwich") if err == nil { t.Skip("wow, thanks") } + cmd := exec.Command("makemeasandwich", "-lettuce") want := "makemeasandwich -lettuce" if got := cmd.String(); got != want { @@ -1007,6 +1052,8 @@ func TestNoPath(t *testing.T) { // Start twice, which returns an error on the second call, would spuriously // close the pipes established in the first call. func TestDoubleStartLeavesPipesOpen(t *testing.T) { + t.Parallel() + cmd := helperCommand(t, "pipetest") in, err := cmd.StdinPipe() if err != nil { diff --git a/src/os/exec/exec_windows_test.go b/src/os/exec/exec_windows_test.go index 35ae0b0b8a..9dec72b3e1 100644 --- a/src/os/exec/exec_windows_test.go +++ b/src/os/exec/exec_windows_test.go @@ -33,6 +33,8 @@ func cmdPipeHandle(args ...string) { } func TestPipePassing(t *testing.T) { + t.Parallel() + r, w, err := os.Pipe() if err != nil { t.Error(err) @@ -60,6 +62,8 @@ func TestPipePassing(t *testing.T) { } func TestNoInheritHandles(t *testing.T) { + t.Parallel() + cmd := exec.Command("cmd", "/c exit 88") cmd.SysProcAttr = &syscall.SysProcAttr{NoInheritHandles: true} err := cmd.Run() @@ -76,6 +80,7 @@ func TestNoInheritHandles(t *testing.T) { // with a copy of the parent's SYSTEMROOT. // (See issue 25210.) func TestChildCriticalEnv(t *testing.T) { + t.Parallel() cmd := helperCommand(t, "echoenv", "SYSTEMROOT") // Explicitly remove SYSTEMROOT from the command's environment. diff --git a/src/os/exec/lp_linux_test.go b/src/os/exec/lp_linux_test.go index 7ab19602e9..98c3a7b9e0 100644 --- a/src/os/exec/lp_linux_test.go +++ b/src/os/exec/lp_linux_test.go @@ -13,6 +13,8 @@ import ( ) func TestFindExecutableVsNoexec(t *testing.T) { + t.Parallel() + // This test case relies on faccessat2(2) syscall, which appeared in Linux v5.8. if major, minor := unix.KernelVersion(); major < 5 || (major == 5 && minor < 8) { t.Skip("requires Linux kernel v5.8 with faccessat2(2) syscall") diff --git a/src/os/exec/lp_unix_test.go b/src/os/exec/lp_unix_test.go index ebeb5bb3ec..181b1f025f 100644 --- a/src/os/exec/lp_unix_test.go +++ b/src/os/exec/lp_unix_test.go @@ -12,6 +12,8 @@ import ( ) func TestLookPathUnixEmptyPath(t *testing.T) { + // Not parallel: uses os.Chdir. + tmp, err := os.MkdirTemp("", "TestLookPathUnixEmptyPath") if err != nil { t.Fatal("TempDir failed: ", err) diff --git a/src/os/exec/lp_windows_test.go b/src/os/exec/lp_windows_test.go index 1f609fffd0..d797b6c53c 100644 --- a/src/os/exec/lp_windows_test.go +++ b/src/os/exec/lp_windows_test.go @@ -334,12 +334,21 @@ var lookPathTests = []lookPathTest{ } func TestLookPathWindows(t *testing.T) { + if testing.Short() { + maySkipHelperCommand("lookpath") + t.Skipf("skipping test in short mode that would build a helper binary") + } + t.Parallel() + tmp := t.TempDir() printpathExe := buildPrintPathExe(t, tmp) // Run all tests. for i, test := range lookPathTests { + i, test := i, test t.Run(fmt.Sprint(i), func(t *testing.T) { + t.Parallel() + dir := filepath.Join(tmp, "d"+strconv.Itoa(i)) err := os.Mkdir(dir, 0700) if err != nil { @@ -524,17 +533,28 @@ var commandTests = []commandTest{ } func TestCommand(t *testing.T) { + if testing.Short() { + maySkipHelperCommand("exec") + t.Skipf("skipping test in short mode that would build a helper binary") + } + t.Parallel() + tmp := t.TempDir() printpathExe := buildPrintPathExe(t, tmp) // Run all tests. for i, test := range commandTests { - dir := filepath.Join(tmp, "d"+strconv.Itoa(i)) - err := os.Mkdir(dir, 0700) - if err != nil { - t.Fatal("Mkdir failed: ", err) - } - test.run(t, dir, printpathExe) + i, test := i, test + t.Run(fmt.Sprint(i), func(t *testing.T) { + t.Parallel() + + dir := filepath.Join(tmp, "d"+strconv.Itoa(i)) + err := os.Mkdir(dir, 0700) + if err != nil { + t.Fatal("Mkdir failed: ", err) + } + test.run(t, dir, printpathExe) + }) } }