Skip to content

Commit

Permalink
os/exec: parallelize more tests
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
  • Loading branch information
Bryan C. Mills authored and gopherbot committed Oct 6, 2022
1 parent 274d3a0 commit 515e3de
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/os/exec/dot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions src/os/exec/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
)

func TestDedupEnv(t *testing.T) {
t.Parallel()

tests := []struct {
noCase bool
in []string
Expand Down
3 changes: 3 additions & 0 deletions src/os/exec/exec_posix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down
49 changes: 48 additions & 1 deletion src/os/exec/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -793,7 +822,6 @@ func TestExtraFilesRace(t *testing.T) {
for _, f := range cb.ExtraFiles {
f.Close()
}

}
}

Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions src/os/exec/exec_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions src/os/exec/lp_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions src/os/exec/lp_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 26 additions & 6 deletions src/os/exec/lp_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
})
}
}

Expand Down

0 comments on commit 515e3de

Please sign in to comment.