os/exec: use a TestMain to avoid hijacking stdout for helper commands
The previous implementation of helperCommand relied on running a
well-known Test function which implemented all known commands.
That not only added Skip noise in the test's output, but also (and
more importantly) meant that the commands could not write directly to
stdout in the usual way, since the testing package hijacks os.Stdout
for its own use.
The new implementation addresses the above issues, and also ensures
that all registered commands are actually used, reducing the risk of
an unused command sticking around after refactoring.
It also sets the subprocess environment variable directly in the test
process, instead of on each individual helper command's Env field,
allowing helper commands to be used without an explicit Env.
Updates #50599.
(Also for #50436.)
Change-Id: I189c7bed9a07cfe47a084b657b88575b1ee370b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/401934
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
diff --git a/src/os/exec/exec_linux_test.go b/src/os/exec/exec_linux_test.go
index 4a37c96..b9f6b7b 100644
--- a/src/os/exec/exec_linux_test.go
+++ b/src/os/exec/exec_linux_test.go
@@ -22,7 +22,7 @@
)
func init() {
- if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
+ if os.Getenv("GO_EXEC_TEST_PID") == "" {
return
}
diff --git a/src/os/exec/exec_posix_test.go b/src/os/exec/exec_posix_test.go
index e583039..f040137 100644
--- a/src/os/exec/exec_posix_test.go
+++ b/src/os/exec/exec_posix_test.go
@@ -7,9 +7,9 @@
package exec_test
import (
+ "fmt"
"internal/testenv"
"os"
- "os/exec"
"os/user"
"path/filepath"
"reflect"
@@ -21,8 +21,32 @@
"time"
)
+func init() {
+ registerHelperCommand("pwd", cmdPwd)
+ registerHelperCommand("sleep", cmdSleep)
+}
+
+func cmdPwd(...string) {
+ pwd, err := os.Getwd()
+ if err != nil {
+ fmt.Fprintln(os.Stderr, err)
+ os.Exit(1)
+ }
+ fmt.Println(pwd)
+}
+
+func cmdSleep(args ...string) {
+ n, err := strconv.Atoi(args[0])
+ if err != nil {
+ fmt.Println(err)
+ os.Exit(1)
+ }
+ time.Sleep(time.Duration(n) * time.Second)
+}
+
func TestCredentialNoSetGroups(t *testing.T) {
if runtime.GOOS == "android" {
+ maySkipHelperCommand("echo")
t.Skip("unsupported on Android")
}
@@ -61,7 +85,7 @@
func TestWaitid(t *testing.T) {
t.Parallel()
- cmd := helperCommand(t, "sleep")
+ cmd := helperCommand(t, "sleep", "3")
if err := cmd.Start(); err != nil {
t.Fatal(err)
}
@@ -97,9 +121,6 @@
// implicitly update PWD to the correct path, and Environ should list the
// updated value.
func TestImplicitPWD(t *testing.T) {
- testenv.MustHaveExec(t)
- _, pwdErr := exec.LookPath("pwd")
-
t.Parallel()
cwd, err := os.Getwd()
@@ -124,12 +145,10 @@
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
- // Note: we're using the actual "pwd" command here (instead of helperCommand)
- // because the implementation of helperCommand requires a non-empty Env.
- // (We could perhaps refactor helperCommand to use a flag or switch on the
- // value of argv[0] instead, but that doesn't seem worth the trouble at
- // the moment.)
- cmd := exec.Command("pwd", "-L")
+ cmd := helperCommand(t, "pwd")
+ if cmd.Env != nil {
+ t.Fatalf("test requires helperCommand not to set Env field")
+ }
cmd.Dir = tc.dir
var pwds []string
@@ -149,9 +168,6 @@
t.Errorf("PWD entries in cmd.Environ():\n\t%s\nwant:\n\t%s", strings.Join(pwds, "\n\t"), strings.Join(wantPWDs, "\n\t"))
}
- if pwdErr != nil {
- t.Skipf("not running `pwd` because it was not found: %v", pwdErr)
- }
cmd.Stderr = new(strings.Builder)
out, err := cmd.Output()
if err != nil {
@@ -170,6 +186,7 @@
// (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) {
+ maySkipHelperCommand("pwd")
testenv.MustHaveSymlink(t)
cwd, err := os.Getwd()
diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go
index f90066c..c593cbd 100644
--- a/src/os/exec/exec_test.go
+++ b/src/os/exec/exec_test.go
@@ -11,6 +11,7 @@
"bufio"
"bytes"
"context"
+ "flag"
"fmt"
"internal/poll"
"internal/testenv"
@@ -27,6 +28,7 @@
"runtime"
"strconv"
"strings"
+ "sync"
"testing"
"time"
)
@@ -36,7 +38,7 @@
var haveUnexpectedFDs bool
func init() {
- if os.Getenv("GO_WANT_HELPER_PROCESS") == "1" {
+ if os.Getenv("GO_EXEC_TEST_PID") != "" {
return
}
if runtime.GOOS == "windows" {
@@ -54,30 +56,253 @@
}
}
-func helperCommandContext(t *testing.T, ctx context.Context, s ...string) (cmd *exec.Cmd) {
+// TestMain allows the test binary to impersonate many other binaries,
+// some of which may manipulate os.Stdin, os.Stdout, and/or os.Stderr
+// (and thus cannot run as an ordinary Test function, since the testing
+// package monkey-patches those variables before running tests).
+func TestMain(m *testing.M) {
+ flag.Parse()
+
+ pid := os.Getpid()
+ if os.Getenv("GO_EXEC_TEST_PID") == "" {
+ os.Setenv("GO_EXEC_TEST_PID", strconv.Itoa(pid))
+
+ code := m.Run()
+ if code == 0 && flag.Lookup("test.run").Value.String() == "" && flag.Lookup("test.list").Value.String() == "" {
+ for cmd := range helperCommands {
+ if _, ok := helperCommandUsed.Load(cmd); !ok {
+ fmt.Fprintf(os.Stderr, "helper command unused: %q\n", cmd)
+ code = 1
+ }
+ }
+ }
+ os.Exit(code)
+ }
+
+ args := flag.Args()
+ if len(args) == 0 {
+ fmt.Fprintf(os.Stderr, "No command\n")
+ os.Exit(2)
+ }
+
+ cmd, args := args[0], args[1:]
+ f, ok := helperCommands[cmd]
+ if !ok {
+ fmt.Fprintf(os.Stderr, "Unknown command %q\n", cmd)
+ os.Exit(2)
+ }
+ f(args...)
+ os.Exit(0)
+}
+
+// registerHelperCommand registers a command that the test process can impersonate.
+// A command should be registered in the same source file in which it is used.
+// If all tests are run and pass, all registered commands must be used.
+// (This prevents stale commands from accreting if tests are removed or
+// refactored over time.)
+func registerHelperCommand(name string, f func(...string)) {
+ if helperCommands[name] != nil {
+ panic("duplicate command registered: " + name)
+ }
+ helperCommands[name] = f
+}
+
+// maySkipHelperCommand records that the test that uses the named helper command
+// was invoked, but may call Skip on the test before actually calling
+// helperCommand.
+func maySkipHelperCommand(name string) {
+ helperCommandUsed.Store(name, true)
+}
+
+// helperCommand returns an exec.Cmd that will run the named helper command.
+func helperCommand(t *testing.T, name string, args ...string) *exec.Cmd {
+ t.Helper()
+ return helperCommandContext(t, nil, name, args...)
+}
+
+// helperCommandContext is like helperCommand, but also accepts a Context under
+// which to run the command.
+func helperCommandContext(t *testing.T, ctx context.Context, name string, args ...string) (cmd *exec.Cmd) {
+ helperCommandUsed.LoadOrStore(name, true)
+
+ t.Helper()
testenv.MustHaveExec(t)
- // Use os.Executable instead of os.Args[0] in case the caller modifies
- // cmd.Dir: if the test binary is invoked like "./exec.test", it should
- // not fail spuriously.
- exe, err := os.Executable()
- if err != nil {
- t.Fatal(err)
- }
-
- cs := []string{"-test.run=TestHelperProcess", "--"}
- cs = append(cs, s...)
+ cs := append([]string{name}, args...)
if ctx != nil {
- cmd = exec.CommandContext(ctx, exe, cs...)
+ cmd = exec.CommandContext(ctx, exePath(t), cs...)
} else {
- cmd = exec.Command(exe, cs...)
+ cmd = exec.Command(exePath(t), cs...)
}
- cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
return cmd
}
-func helperCommand(t *testing.T, s ...string) *exec.Cmd {
- return helperCommandContext(t, nil, s...)
+// exePath returns the path to the running executable.
+func exePath(t testing.TB) string {
+ exeOnce.Do(func() {
+ // Use os.Executable instead of os.Args[0] in case the caller modifies
+ // cmd.Dir: if the test binary is invoked like "./exec.test", it should
+ // not fail spuriously.
+ exeOnce.path, exeOnce.err = os.Executable()
+ })
+
+ if exeOnce.err != nil {
+ if t == nil {
+ panic(exeOnce.err)
+ }
+ t.Fatal(exeOnce.err)
+ }
+
+ return exeOnce.path
+}
+
+var exeOnce struct {
+ path string
+ err error
+ sync.Once
+}
+
+var helperCommandUsed sync.Map
+
+var helperCommands = map[string]func(...string){
+ "echo": cmdEcho,
+ "echoenv": cmdEchoEnv,
+ "cat": cmdCat,
+ "pipetest": cmdPipeTest,
+ "stdinClose": cmdStdinClose,
+ "exit": cmdExit,
+ "describefiles": cmdDescribeFiles,
+ "extraFilesAndPipes": cmdExtraFilesAndPipes,
+ "stderrfail": cmdStderrFail,
+ "yes": cmdYes,
+}
+
+func cmdEcho(args ...string) {
+ iargs := []any{}
+ for _, s := range args {
+ iargs = append(iargs, s)
+ }
+ fmt.Println(iargs...)
+}
+
+func cmdEchoEnv(args ...string) {
+ for _, s := range args {
+ fmt.Println(os.Getenv(s))
+ }
+}
+
+func cmdCat(args ...string) {
+ if len(args) == 0 {
+ io.Copy(os.Stdout, os.Stdin)
+ return
+ }
+ exit := 0
+ for _, fn := range args {
+ f, err := os.Open(fn)
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+ exit = 2
+ } else {
+ defer f.Close()
+ io.Copy(os.Stdout, f)
+ }
+ }
+ os.Exit(exit)
+}
+
+func cmdPipeTest(...string) {
+ bufr := bufio.NewReader(os.Stdin)
+ for {
+ line, _, err := bufr.ReadLine()
+ if err == io.EOF {
+ break
+ } else if err != nil {
+ os.Exit(1)
+ }
+ if bytes.HasPrefix(line, []byte("O:")) {
+ os.Stdout.Write(line)
+ os.Stdout.Write([]byte{'\n'})
+ } else if bytes.HasPrefix(line, []byte("E:")) {
+ os.Stderr.Write(line)
+ os.Stderr.Write([]byte{'\n'})
+ } else {
+ os.Exit(1)
+ }
+ }
+}
+
+func cmdStdinClose(...string) {
+ b, err := io.ReadAll(os.Stdin)
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+ os.Exit(1)
+ }
+ if s := string(b); s != stdinCloseTestString {
+ fmt.Fprintf(os.Stderr, "Error: Read %q, want %q", s, stdinCloseTestString)
+ os.Exit(1)
+ }
+}
+
+func cmdExit(args ...string) {
+ n, _ := strconv.Atoi(args[0])
+ os.Exit(n)
+}
+
+func cmdDescribeFiles(args ...string) {
+ f := os.NewFile(3, fmt.Sprintf("fd3"))
+ ln, err := net.FileListener(f)
+ if err == nil {
+ fmt.Printf("fd3: listener %s\n", ln.Addr())
+ ln.Close()
+ }
+}
+
+func cmdExtraFilesAndPipes(args ...string) {
+ n, _ := strconv.Atoi(args[0])
+ pipes := make([]*os.File, n)
+ for i := 0; i < n; i++ {
+ pipes[i] = os.NewFile(uintptr(3+i), strconv.Itoa(i))
+ }
+ response := ""
+ for i, r := range pipes {
+ ch := make(chan string, 1)
+ go func(c chan string) {
+ buf := make([]byte, 10)
+ n, err := r.Read(buf)
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "Child: read error: %v on pipe %d\n", err, i)
+ os.Exit(1)
+ }
+ c <- string(buf[:n])
+ close(c)
+ }(ch)
+ select {
+ case m := <-ch:
+ response = response + m
+ case <-time.After(5 * time.Second):
+ fmt.Fprintf(os.Stderr, "Child: Timeout reading from pipe: %d\n", i)
+ os.Exit(1)
+ }
+ }
+ fmt.Fprintf(os.Stderr, "child: %s", response)
+}
+
+func cmdStderrFail(...string) {
+ fmt.Fprintf(os.Stderr, "some stderr text\n")
+ os.Exit(1)
+}
+
+func cmdYes(args ...string) {
+ if len(args) == 0 {
+ args = []string{"y"}
+ }
+ s := strings.Join(args, " ") + "\n"
+ for {
+ _, err := os.Stdout.WriteString(s)
+ if err != nil {
+ os.Exit(1)
+ }
+ }
}
func TestEcho(t *testing.T) {
@@ -91,7 +316,7 @@
}
func TestCommandRelativeName(t *testing.T) {
- testenv.MustHaveExec(t)
+ cmd := helperCommand(t, "echo", "foo")
// Run our own binary as a relative path
// (e.g. "_test/exec.test") our parent directory.
@@ -106,9 +331,8 @@
t.Skipf("skipping; unexpected shallow dir of %q", dir)
}
- cmd := exec.Command(filepath.Join(dirBase, base), "-test.run=TestHelperProcess", "--", "echo", "foo")
+ cmd.Path = filepath.Join(dirBase, base)
cmd.Dir = parentDir
- cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"}
out, err := cmd.Output()
if err != nil {
@@ -167,7 +391,7 @@
if !strings.HasPrefix(errLine, "Error: open /bogus/file.foo") {
t.Errorf("expected stderr to complain about file; got %q", errLine)
}
- if !strings.Contains(body, "func TestHelperProcess(t *testing.T)") {
+ if !strings.Contains(body, "func TestCatGoodAndBadFile(t *testing.T)") {
t.Errorf("expected test code; got %q (len %d)", body, len(body))
}
}
@@ -402,6 +626,7 @@
}
func TestExtraFilesFDShuffle(t *testing.T) {
+ maySkipHelperCommand("extraFilesAndPipes")
testenv.SkipFlaky(t, 5780)
switch runtime.GOOS {
case "windows":
@@ -627,6 +852,7 @@
func TestExtraFilesRace(t *testing.T) {
if runtime.GOOS == "windows" {
+ maySkipHelperCommand("describefiles")
t.Skip("no operating system support; skipping")
}
listen := func() net.Listener {
@@ -684,175 +910,6 @@
}
}
-// TestHelperProcess isn't a real test. It's used as a helper process
-// for TestParameterRun.
-func TestHelperProcess(*testing.T) {
- if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
- return
- }
- defer os.Exit(0)
-
- args := os.Args
- for len(args) > 0 {
- if args[0] == "--" {
- args = args[1:]
- break
- }
- args = args[1:]
- }
- if len(args) == 0 {
- fmt.Fprintf(os.Stderr, "No command\n")
- os.Exit(2)
- }
-
- cmd, args := args[0], args[1:]
- switch cmd {
- case "echo":
- iargs := []any{}
- for _, s := range args {
- iargs = append(iargs, s)
- }
- fmt.Println(iargs...)
- case "echoenv":
- for _, s := range args {
- fmt.Println(os.Getenv(s))
- }
- os.Exit(0)
- case "cat":
- if len(args) == 0 {
- io.Copy(os.Stdout, os.Stdin)
- return
- }
- exit := 0
- for _, fn := range args {
- f, err := os.Open(fn)
- if err != nil {
- fmt.Fprintf(os.Stderr, "Error: %v\n", err)
- exit = 2
- } else {
- defer f.Close()
- io.Copy(os.Stdout, f)
- }
- }
- os.Exit(exit)
- case "pipetest":
- bufr := bufio.NewReader(os.Stdin)
- for {
- line, _, err := bufr.ReadLine()
- if err == io.EOF {
- break
- } else if err != nil {
- os.Exit(1)
- }
- if bytes.HasPrefix(line, []byte("O:")) {
- os.Stdout.Write(line)
- os.Stdout.Write([]byte{'\n'})
- } else if bytes.HasPrefix(line, []byte("E:")) {
- os.Stderr.Write(line)
- os.Stderr.Write([]byte{'\n'})
- } else {
- os.Exit(1)
- }
- }
- case "stdinClose":
- b, err := io.ReadAll(os.Stdin)
- if err != nil {
- fmt.Fprintf(os.Stderr, "Error: %v\n", err)
- os.Exit(1)
- }
- if s := string(b); s != stdinCloseTestString {
- fmt.Fprintf(os.Stderr, "Error: Read %q, want %q", s, stdinCloseTestString)
- os.Exit(1)
- }
- os.Exit(0)
- case "exit":
- n, _ := strconv.Atoi(args[0])
- os.Exit(n)
- case "describefiles":
- f := os.NewFile(3, fmt.Sprintf("fd3"))
- ln, err := net.FileListener(f)
- if err == nil {
- fmt.Printf("fd3: listener %s\n", ln.Addr())
- ln.Close()
- }
- os.Exit(0)
- case "extraFilesAndPipes":
- n, _ := strconv.Atoi(args[0])
- pipes := make([]*os.File, n)
- for i := 0; i < n; i++ {
- pipes[i] = os.NewFile(uintptr(3+i), strconv.Itoa(i))
- }
- response := ""
- for i, r := range pipes {
- ch := make(chan string, 1)
- go func(c chan string) {
- buf := make([]byte, 10)
- n, err := r.Read(buf)
- if err != nil {
- fmt.Fprintf(os.Stderr, "Child: read error: %v on pipe %d\n", err, i)
- os.Exit(1)
- }
- c <- string(buf[:n])
- close(c)
- }(ch)
- select {
- case m := <-ch:
- response = response + m
- case <-time.After(5 * time.Second):
- fmt.Fprintf(os.Stderr, "Child: Timeout reading from pipe: %d\n", i)
- os.Exit(1)
- }
- }
- fmt.Fprintf(os.Stderr, "child: %s", response)
- os.Exit(0)
- case "exec":
- cmd := exec.Command(args[1])
- cmd.Dir = args[0]
- output, err := cmd.CombinedOutput()
- if err != nil {
- fmt.Fprintf(os.Stderr, "Child: %s %s", err, string(output))
- os.Exit(1)
- }
- fmt.Printf("%s", string(output))
- os.Exit(0)
- case "lookpath":
- p, err := exec.LookPath(args[0])
- if err != nil {
- fmt.Fprintf(os.Stderr, "LookPath failed: %v\n", err)
- os.Exit(1)
- }
- fmt.Print(p)
- os.Exit(0)
- case "stderrfail":
- fmt.Fprintf(os.Stderr, "some stderr text\n")
- os.Exit(1)
- case "sleep":
- time.Sleep(3 * time.Second)
- os.Exit(0)
- case "pipehandle":
- handle, _ := strconv.ParseUint(args[0], 16, 64)
- pipe := os.NewFile(uintptr(handle), "")
- _, err := fmt.Fprint(pipe, args[1])
- if err != nil {
- fmt.Fprintf(os.Stderr, "writing to pipe failed: %v\n", err)
- os.Exit(1)
- }
- pipe.Close()
- os.Exit(0)
- case "pwd":
- pwd, err := os.Getwd()
- if err != nil {
- fmt.Fprintln(os.Stderr, err)
- os.Exit(1)
- }
- fmt.Println(pwd)
- os.Exit(0)
- default:
- fmt.Fprintf(os.Stderr, "Unknown command %q\n", cmd)
- os.Exit(2)
- }
-}
-
type delayedInfiniteReader struct{}
func (delayedInfiniteReader) Read(b []byte) (int, error) {
@@ -865,8 +922,6 @@
// Issue 9173: ignore stdin pipe writes if the program completes successfully.
func TestIgnorePipeErrorOnSuccess(t *testing.T) {
- testenv.MustHaveExec(t)
-
testWith := func(r io.Reader) func(*testing.T) {
return func(t *testing.T) {
cmd := helperCommand(t, "echo", "foo")
@@ -892,12 +947,7 @@
}
func TestClosePipeOnCopyError(t *testing.T) {
- testenv.MustHaveExec(t)
-
- if runtime.GOOS == "windows" || runtime.GOOS == "plan9" {
- t.Skipf("skipping test on %s - no yes command", runtime.GOOS)
- }
- cmd := exec.Command("yes")
+ cmd := helperCommand(t, "yes")
cmd.Stdout = new(badWriter)
c := make(chan int, 1)
go func() {
@@ -916,8 +966,6 @@
}
func TestOutputStderrCapture(t *testing.T) {
- testenv.MustHaveExec(t)
-
cmd := helperCommand(t, "stderrfail")
_, err := cmd.Output()
ee, ok := err.(*exec.ExitError)
@@ -971,6 +1019,7 @@
func TestContextCancel(t *testing.T) {
if runtime.GOOS == "netbsd" && runtime.GOARCH == "arm64" {
+ maySkipHelperCommand("cat")
testenv.SkipFlaky(t, 42061)
}
@@ -1032,10 +1081,8 @@
// test that environment variables are de-duped.
func TestDedupEnvEcho(t *testing.T) {
- testenv.MustHaveExec(t)
-
cmd := helperCommand(t, "echoenv", "FOO")
- cmd.Env = append(cmd.Env, "FOO=bad", "FOO=good")
+ cmd.Env = append(cmd.Environ(), "FOO=bad", "FOO=good")
out, err := cmd.CombinedOutput()
if err != nil {
t.Fatal(err)
@@ -1078,22 +1125,3 @@
t.Errorf("String(%q, %q) = %q, want %q", "makemeasandwich", "-lettuce", got, want)
}
}
-
-// start a child process without the user code explicitly starting
-// with a copy of the parent's. (The Windows SYSTEMROOT issue: Issue
-// 25210)
-func TestChildCriticalEnv(t *testing.T) {
- testenv.MustHaveExec(t)
- if runtime.GOOS != "windows" {
- t.Skip("only testing on Windows")
- }
- cmd := helperCommand(t, "echoenv", "SYSTEMROOT")
- cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"}
- out, err := cmd.CombinedOutput()
- if err != nil {
- t.Fatal(err)
- }
- if strings.TrimSpace(string(out)) == "" {
- t.Error("no SYSTEMROOT found")
- }
-}
diff --git a/src/os/exec/exec_windows_test.go b/src/os/exec/exec_windows_test.go
index 8e31e47..35ae0b0 100644
--- a/src/os/exec/exec_windows_test.go
+++ b/src/os/exec/exec_windows_test.go
@@ -7,14 +7,31 @@
package exec_test
import (
+ "fmt"
"io"
"os"
"os/exec"
"strconv"
+ "strings"
"syscall"
"testing"
)
+func init() {
+ registerHelperCommand("pipehandle", cmdPipeHandle)
+}
+
+func cmdPipeHandle(args ...string) {
+ handle, _ := strconv.ParseUint(args[0], 16, 64)
+ pipe := os.NewFile(uintptr(handle), "")
+ _, err := fmt.Fprint(pipe, args[1])
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "writing to pipe failed: %v\n", err)
+ os.Exit(1)
+ }
+ pipe.Close()
+}
+
func TestPipePassing(t *testing.T) {
r, w, err := os.Pipe()
if err != nil {
@@ -54,3 +71,28 @@
t.Fatalf("got exit code %d; want 88", exitError.ExitCode())
}
}
+
+// start a child process without the user code explicitly starting
+// with a copy of the parent's SYSTEMROOT.
+// (See issue 25210.)
+func TestChildCriticalEnv(t *testing.T) {
+ cmd := helperCommand(t, "echoenv", "SYSTEMROOT")
+
+ // Explicitly remove SYSTEMROOT from the command's environment.
+ var env []string
+ for _, kv := range cmd.Environ() {
+ k, _, ok := strings.Cut(kv, "=")
+ if !ok || !strings.EqualFold(k, "SYSTEMROOT") {
+ env = append(env, kv)
+ }
+ }
+ cmd.Env = env
+
+ out, err := cmd.CombinedOutput()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if strings.TrimSpace(string(out)) == "" {
+ t.Error("no SYSTEMROOT found")
+ }
+}
diff --git a/src/os/exec/lp_windows_test.go b/src/os/exec/lp_windows_test.go
index bbf6a9b..34abe09 100644
--- a/src/os/exec/lp_windows_test.go
+++ b/src/os/exec/lp_windows_test.go
@@ -19,6 +19,31 @@
"testing"
)
+func init() {
+ registerHelperCommand("exec", cmdExec)
+ registerHelperCommand("lookpath", cmdLookPath)
+}
+
+func cmdLookPath(args ...string) {
+ p, err := exec.LookPath(args[0])
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "LookPath failed: %v\n", err)
+ os.Exit(1)
+ }
+ fmt.Print(p)
+}
+
+func cmdExec(args ...string) {
+ cmd := exec.Command(args[1])
+ cmd.Dir = args[0]
+ output, err := cmd.CombinedOutput()
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "Child: %s %s", err, string(output))
+ os.Exit(1)
+ }
+ fmt.Printf("%s", string(output))
+}
+
func installExe(t *testing.T, dest, src string) {
fsrc, err := os.Open(src)
if err != nil {
@@ -66,10 +91,10 @@
fails bool // test is expected to fail
}
-func (test lookPathTest) runProg(t *testing.T, env []string, args ...string) (string, error) {
- cmd := exec.Command(args[0], args[1:]...)
+func (test lookPathTest) runProg(t *testing.T, env []string, cmd *exec.Cmd) (string, error) {
cmd.Env = env
cmd.Dir = test.rootDir
+ args := append([]string(nil), cmd.Args...)
args[0] = filepath.Base(args[0])
cmdText := fmt.Sprintf("%q command", strings.Join(args, " "))
out, err := cmd.CombinedOutput()
@@ -135,10 +160,9 @@
// Run "cmd.exe /c test.searchFor" with new environment and
// work directory set. All candidates are copies of printpath.exe.
// These will output their program paths when run.
- should, errCmd := test.runProg(t, env, "cmd", "/c", test.searchFor)
+ should, errCmd := test.runProg(t, env, exec.Command("cmd", "/c", test.searchFor))
// Run the lookpath program with new environment and work directory set.
- env = append(env, "GO_WANT_HELPER_PROCESS=1")
- have, errLP := test.runProg(t, env, os.Args[0], "-test.run=TestHelperProcess", "--", "lookpath", test.searchFor)
+ have, errLP := test.runProg(t, env, helperCommand(t, "lookpath", test.searchFor))
// Compare results.
if errCmd == nil && errLP == nil {
// both succeeded
@@ -346,30 +370,26 @@
return nil
}
-func (test commandTest) runOne(rootDir string, env []string, dir, arg0 string) error {
- cmd := exec.Command(os.Args[0], "-test.run=TestHelperProcess", "--", "exec", dir, arg0)
+func (test commandTest) runOne(t *testing.T, rootDir string, env []string, dir, arg0 string) {
+ cmd := helperCommand(t, "exec", dir, arg0)
cmd.Dir = rootDir
cmd.Env = env
output, err := cmd.CombinedOutput()
err = test.isSuccess(rootDir, string(output), err)
if (err != nil) != test.fails {
if test.fails {
- return fmt.Errorf("test=%+v: succeeded, but expected to fail", test)
+ t.Errorf("test=%+v: succeeded, but expected to fail", test)
+ } else {
+ t.Error(err)
}
- return err
}
- return nil
}
func (test commandTest) run(t *testing.T, rootDir, printpathExe string) {
createFiles(t, rootDir, test.files, printpathExe)
PATHEXT := `.COM;.EXE;.BAT`
env := createEnv(rootDir, test.PATH, PATHEXT)
- env = append(env, "GO_WANT_HELPER_PROCESS=1")
- err := test.runOne(rootDir, env, test.dir, test.arg0)
- if err != nil {
- t.Error(err)
- }
+ test.runOne(t, rootDir, env, test.dir, test.arg0)
}
var commandTests = []commandTest{
diff --git a/src/os/exec/read3.go b/src/os/exec/read3.go
index 10cbfbd..8327d73 100644
--- a/src/os/exec/read3.go
+++ b/src/os/exec/read3.go
@@ -6,7 +6,7 @@
// This is a test program that verifies that it can read from
// descriptor 3 and that no other descriptors are open.
-// This is not done via TestHelperProcess and GO_WANT_HELPER_PROCESS
+// This is not done via TestHelperProcess and GO_EXEC_TEST_PID
// because we want to ensure that this program does not use cgo,
// because C libraries can open file descriptors behind our backs
// and confuse the test. See issue 25628.
