From: Bryan C. Mills Date: Tue, 12 Sep 2023 18:50:50 +0000 (-0400) Subject: os/exec: simplify Windows-specific tests X-Git-Tag: go1.22rc1~865 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=105f9d51691d996c1698811ca3906b505639f49b;p=gostls13.git os/exec: simplify Windows-specific tests - Use the test binary itself for printing paths instead of building a separate binary and running it through additional subprocesses. - Factor out a common chdir helper. - Use t.Setenv where appropriate. - Reduce indirection in test helpers. - Set NoDefaultCurrentDirectoryInExePath consistently in the environment. Also add a test case demonstrating an interesting behavior for relative paths that may interact with #62596. Fixes #62594. For #62596. Change-Id: I19b9325034edf78cd0ca747594476cd7432bb451 Reviewed-on: https://go-review.googlesource.com/c/go/+/528035 Reviewed-by: Ian Lance Taylor Auto-Submit: Bryan Mills LUCI-TryBot-Result: Go LUCI --- diff --git a/src/os/exec/dot_test.go b/src/os/exec/dot_test.go index 66c92f7abd..ed4bad23b1 100644 --- a/src/os/exec/dot_test.go +++ b/src/os/exec/dot_test.go @@ -24,7 +24,7 @@ var pathVar string = func() string { func TestLookPath(t *testing.T) { testenv.MustHaveExec(t) - // Not parallel: uses os.Chdir and t.Setenv. + // Not parallel: uses Chdir and Setenv. tmpDir := filepath.Join(t.TempDir(), "testdir") if err := os.Mkdir(tmpDir, 0777); err != nil { @@ -38,18 +38,7 @@ func TestLookPath(t *testing.T) { if err := os.WriteFile(filepath.Join(tmpDir, executable), []byte{1, 2, 3}, 0777); err != nil { t.Fatal(err) } - cwd, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - defer func() { - if err := os.Chdir(cwd); err != nil { - panic(err) - } - }() - if err = os.Chdir(tmpDir); err != nil { - t.Fatal(err) - } + chdir(t, tmpDir) t.Setenv("PWD", tmpDir) t.Logf(". is %#q", tmpDir) diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index 473f92ba8e..9783a133ba 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -77,6 +77,21 @@ func TestMain(m *testing.M) { if os.Getenv("GO_EXEC_TEST_PID") == "" { os.Setenv("GO_EXEC_TEST_PID", strconv.Itoa(pid)) + if runtime.GOOS == "windows" { + // Normalize environment so that test behavior is consistent. + // (The behavior of LookPath varies depending on this variable.) + // + // Ideally we would test both with the variable set and with it cleared, + // but I (bcmills) am not sure that that's feasible: it may already be set + // in the Windows registry, and I'm not sure if it is possible to remove + // a registry variable in a program's environment. + // + // Per https://learn.microsoft.com/en-us/windows/win32/api/processenv/nf-processenv-needcurrentdirectoryforexepathw#remarks, + // “the existence of the NoDefaultCurrentDirectoryInExePath environment + // variable is checked, and not its value.” + os.Setenv("NoDefaultCurrentDirectoryInExePath", "TRUE") + } + code := m.Run() if code == 0 && flag.Lookup("test.run").Value.String() == "" && flag.Lookup("test.list").Value.String() == "" { for cmd := range helperCommands { @@ -180,6 +195,28 @@ var exeOnce struct { sync.Once } +func chdir(t *testing.T, dir string) { + t.Helper() + + prev, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + t.Logf("Chdir(%#q)", dir) + + t.Cleanup(func() { + if err := os.Chdir(prev); err != nil { + // Couldn't chdir back to the original working directory. + // panic instead of t.Fatal so that we don't run other tests + // in an unexpected location. + panic("couldn't restore working directory: " + err.Error()) + } + }) +} + var helperCommandUsed sync.Map var helperCommands = map[string]func(...string){ diff --git a/src/os/exec/lp_unix_test.go b/src/os/exec/lp_unix_test.go index 181b1f025f..1503ddae93 100644 --- a/src/os/exec/lp_unix_test.go +++ b/src/os/exec/lp_unix_test.go @@ -4,30 +4,19 @@ //go:build unix -package exec +package exec_test import ( "os" + "os/exec" "testing" ) func TestLookPathUnixEmptyPath(t *testing.T) { - // Not parallel: uses os.Chdir. + // Not parallel: uses Chdir and Setenv. - tmp, err := os.MkdirTemp("", "TestLookPathUnixEmptyPath") - if err != nil { - t.Fatal("TempDir failed: ", err) - } - defer os.RemoveAll(tmp) - wd, err := os.Getwd() - if err != nil { - t.Fatal("Getwd failed: ", err) - } - err = os.Chdir(tmp) - if err != nil { - t.Fatal("Chdir failed: ", err) - } - defer os.Chdir(wd) + tmp := t.TempDir() + chdir(t, tmp) f, err := os.OpenFile("exec_me", os.O_CREATE|os.O_EXCL, 0700) if err != nil { @@ -40,7 +29,7 @@ func TestLookPathUnixEmptyPath(t *testing.T) { t.Setenv("PATH", "") - path, err := LookPath("exec_me") + path, err := exec.LookPath("exec_me") if err == nil { t.Fatal("LookPath found exec_me in empty $PATH") } diff --git a/src/os/exec/lp_windows_test.go b/src/os/exec/lp_windows_test.go index efa26e7c46..f2c56ccce4 100644 --- a/src/os/exec/lp_windows_test.go +++ b/src/os/exec/lp_windows_test.go @@ -12,496 +12,435 @@ import ( "fmt" "internal/testenv" "io" + "io/fs" "os" "os/exec" "path/filepath" - "strconv" + "slices" "strings" "testing" ) func init() { - registerHelperCommand("exec", cmdExec) - registerHelperCommand("lookpath", cmdLookPath) + registerHelperCommand("printpath", cmdPrintPath) } -func cmdLookPath(args ...string) { - p, err := exec.LookPath(args[0]) +func cmdPrintPath(args ...string) { + exe, err := os.Executable() if err != nil { - fmt.Fprintf(os.Stderr, "LookPath failed: %v\n", err) + fmt.Fprintf(os.Stderr, "Executable: %v\n", err) os.Exit(1) } - fmt.Print(p) + fmt.Println(exe) } -func cmdExec(args ...string) { - cmd := exec.Command(args[1]) - cmd.Dir = args[0] - if errors.Is(cmd.Err, exec.ErrDot) { - cmd.Err = nil - } - output, err := cmd.CombinedOutput() - if err != nil { - fmt.Fprintf(os.Stderr, "Child: %s %s", err, string(output)) - os.Exit(1) +// installProgs creates executable files (or symlinks to executable files) at +// multiple destination paths. It uses root as prefix for all destination files. +func installProgs(t *testing.T, root string, files []string) { + for _, f := range files { + dstPath := filepath.Join(root, f) + + dir := filepath.Dir(dstPath) + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + + if os.IsPathSeparator(f[len(f)-1]) { + continue // directory and PATH entry only. + } + if strings.EqualFold(filepath.Ext(f), ".bat") { + installBat(t, dstPath) + } else { + installExe(t, dstPath) + } } - fmt.Printf("%s", output) } -func installExe(t *testing.T, dest, src string) { - fsrc, err := os.Open(src) +// installExe installs a copy of the test executable +// at the given location, creating directories as needed. +// +// (We use a copy instead of just a symlink to ensure that os.Executable +// always reports an unambiguous path, regardless of how it is implemented.) +func installExe(t *testing.T, dstPath string) { + src, err := os.Open(exePath(t)) if err != nil { - t.Fatal("os.Open failed: ", err) + t.Fatal(err) } - defer fsrc.Close() - fdest, err := os.Create(dest) - if err != nil { - t.Fatal("os.Create failed: ", err) - } - defer fdest.Close() - _, err = io.Copy(fdest, fsrc) - if err != nil { - t.Fatal("io.Copy failed: ", err) - } -} + defer src.Close() -func installBat(t *testing.T, dest string) { - f, err := os.Create(dest) + dst, err := os.OpenFile(dstPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o777) if err != nil { - t.Fatalf("failed to create batch file: %v", err) + t.Fatal(err) } - defer f.Close() - fmt.Fprintf(f, "@echo %s\n", dest) -} + defer func() { + if err := dst.Close(); err != nil { + t.Fatal(err) + } + }() -func installProg(t *testing.T, dest, srcExe string) { - err := os.MkdirAll(filepath.Dir(dest), 0700) + _, err = io.Copy(dst, src) if err != nil { - t.Fatal("os.MkdirAll failed: ", err) + t.Fatal(err) } - if strings.ToLower(filepath.Ext(dest)) == ".bat" { - installBat(t, dest) - return - } - installExe(t, dest, srcExe) } -type lookPathTest struct { - rootDir string - PATH string - PATHEXT string - files []string - searchFor string - fails bool // test is expected to fail -} - -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() - if (err != nil) != test.fails { - if test.fails { - t.Fatalf("test=%+v: %s succeeded, but expected to fail", test, cmdText) - } - t.Fatalf("test=%+v: %s failed, but expected to succeed: %v - %v", test, cmdText, err, string(out)) - } +// installBat creates a batch file at dst that prints its own +// path when run. +func installBat(t *testing.T, dstPath string) { + dst, err := os.OpenFile(dstPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o777) if err != nil { - return "", fmt.Errorf("test=%+v: %s failed: %v - %v", test, cmdText, err, string(out)) + t.Fatal(err) } - // normalise program output - p := string(out) - // trim terminating \r and \n that batch file outputs - for len(p) > 0 && (p[len(p)-1] == '\n' || p[len(p)-1] == '\r') { - p = p[:len(p)-1] - } - if !filepath.IsAbs(p) { - return p, nil - } - if p[:len(test.rootDir)] != test.rootDir { - t.Fatalf("test=%+v: %s output is wrong: %q must have %q prefix", test, cmdText, p, test.rootDir) - } - return p[len(test.rootDir)+1:], nil -} - -func updateEnv(env []string, name, value string) []string { - for i, e := range env { - if strings.HasPrefix(strings.ToUpper(e), name+"=") { - env[i] = name + "=" + value - return env + defer func() { + if err := dst.Close(); err != nil { + t.Fatal(err) } - } - return append(env, name+"="+value) -} + }() -func createEnv(dir, PATH, PATHEXT string) []string { - env := os.Environ() - env = updateEnv(env, "PATHEXT", PATHEXT) - // Add dir in front of every directory in the PATH. - dirs := filepath.SplitList(PATH) - for i := range dirs { - dirs[i] = filepath.Join(dir, dirs[i]) + if _, err := fmt.Fprintf(dst, "@echo %s\r\n", dstPath); err != nil { + t.Fatal(err) } - path := strings.Join(dirs, ";") - env = updateEnv(env, "PATH", os.Getenv("SystemRoot")+"/System32;"+path) - return env } -// createFiles copies srcPath file into multiply files. -// It uses dir as prefix for all destination files. -func createFiles(t *testing.T, dir string, files []string, srcPath string) { - for _, f := range files { - installProg(t, filepath.Join(dir, f), srcPath) - } -} - -func (test lookPathTest) run(t *testing.T, tmpdir, printpathExe string) { - test.rootDir = tmpdir - createFiles(t, test.rootDir, test.files, printpathExe) - env := createEnv(test.rootDir, test.PATH, test.PATHEXT) - // 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, testenv.Command(t, "cmd", "/c", test.searchFor)) - // Run the lookpath program with new environment and work directory set. - have, errLP := test.runProg(t, env, helperCommand(t, "lookpath", test.searchFor)) - // Compare results. - if errCmd == nil && errLP == nil { - // both succeeded - if should != have { - t.Fatalf("test=%+v:\ncmd /c ran: %s\nlookpath found: %s", test, should, have) - } - return - } - if errCmd != nil && errLP != nil { - // both failed -> continue - return - } - if errCmd != nil { - t.Fatal(errCmd) - } - if errLP != nil { - t.Fatal(errLP) - } +type lookPathTest struct { + name string + PATHEXT string // empty to use default + files []string // PATH contains all named directories + searchFor string + want string + wantErr error } var lookPathTests = []lookPathTest{ { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, + name: "first match", files: []string{`p1\a.exe`, `p2\a.exe`, `p2\a`}, searchFor: `a`, + want: `p1\a.exe`, }, { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1.dir;p2.dir`, + name: "dirs with extensions", files: []string{`p1.dir\a`, `p2.dir\a.exe`}, searchFor: `a`, + want: `p2.dir\a.exe`, }, { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, + name: "first with extension", files: []string{`p1\a.exe`, `p2\a.exe`}, searchFor: `a.exe`, + want: `p1\a.exe`, }, { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, + name: "specific name", files: []string{`p1\a.exe`, `p2\b.exe`}, searchFor: `b`, + want: `p2\b.exe`, }, { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, + name: "no extension", files: []string{`p1\b`, `p2\a`}, searchFor: `a`, - fails: true, // TODO(brainman): do not know why this fails + wantErr: exec.ErrNotFound, }, - // If the command name specifies a path, the shell searches - // the specified path for an executable file matching - // the command name. If a match is found, the external - // command (the executable file) executes. { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, + name: "directory, no extension", files: []string{`p1\a.exe`, `p2\a.exe`}, searchFor: `p2\a`, + want: `p2\a.exe`, }, - // If the command name specifies a path, the shell searches - // the specified path for an executable file matching the command - // name. ... If no match is found, the shell reports an error - // and command processing completes. { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, + name: "no match", + files: []string{`p1\a.exe`, `p2\a.exe`}, + searchFor: `b`, + wantErr: exec.ErrNotFound, + }, + { + name: "no match with dir", files: []string{`p1\b.exe`, `p2\a.exe`}, searchFor: `p2\b`, - fails: true, + wantErr: fs.ErrNotExist, }, - // If the command name does not specify a path, the shell - // searches the current directory for an executable file - // matching the command name. If a match is found, the external - // command (the executable file) executes. { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, + name: "extensionless file in CWD ignored", files: []string{`a`, `p1\a.exe`, `p2\a.exe`}, searchFor: `a`, + want: `p1\a.exe`, }, - // The shell now searches each directory specified by the - // PATH environment variable, in the order listed, for an - // executable file matching the command name. If a match - // is found, the external command (the executable file) executes. { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, - files: []string{`p1\a.exe`, `p2\a.exe`}, + name: "extensionless file in PATH ignored", + files: []string{`p1\a`, `p2\a.exe`}, searchFor: `a`, + want: `p2\a.exe`, }, - // The shell now searches each directory specified by the - // PATH environment variable, in the order listed, for an - // executable file matching the command name. If no match - // is found, the shell reports an error and command processing - // completes. { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, - files: []string{`p1\a.exe`, `p2\a.exe`}, - searchFor: `b`, - fails: true, - }, - // If the command name includes a file extension, the shell - // searches each directory for the exact file name specified - // by the command name. - { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, - files: []string{`p1\a.exe`, `p2\a.exe`}, - searchFor: `a.exe`, + name: "specific extension", + files: []string{`p1\a.exe`, `p2\a.bat`}, + searchFor: `a.bat`, + want: `p2\a.bat`, }, { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, + name: "mismatched extension", files: []string{`p1\a.exe`, `p2\a.exe`}, searchFor: `a.com`, - fails: true, // includes extension and not exact file name match + wantErr: exec.ErrNotFound, }, { - PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1`, + name: "doubled extension", files: []string{`p1\a.exe.exe`}, searchFor: `a.exe`, + want: `p1\a.exe.exe`, }, { + name: "extension not in PATHEXT", PATHEXT: `.COM;.BAT`, - PATH: `p1;p2`, files: []string{`p1\a.exe`, `p2\a.exe`}, searchFor: `a.exe`, + want: `p1\a.exe`, }, - // If the command name does not include a file extension, the shell - // adds the extensions listed in the PATHEXT environment variable, - // one by one, and searches the directory for that file name. Note - // that the shell tries all possible file extensions in a specific - // directory before moving on to search the next directory - // (if there is one). { + name: "first allowed by PATHEXT", PATHEXT: `.COM;.EXE`, - PATH: `p1;p2`, files: []string{`p1\a.bat`, `p2\a.exe`}, searchFor: `a`, + want: `p2\a.exe`, }, { + name: "first directory containing a PATHEXT match", PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, files: []string{`p1\a.bat`, `p2\a.exe`}, searchFor: `a`, + want: `p1\a.bat`, }, { + name: "first PATHEXT entry", PATHEXT: `.COM;.EXE;.BAT`, - PATH: `p1;p2`, files: []string{`p1\a.bat`, `p1\a.exe`, `p2\a.bat`, `p2\a.exe`}, searchFor: `a`, + want: `p1\a.exe`, }, { - PATHEXT: `.COM`, - PATH: `p1;p2`, - files: []string{`p1\a.bat`, `p2\a.exe`}, + name: "ignore dir with PATHEXT extension", + files: []string{`a.exe\`}, searchFor: `a`, - fails: true, // tried all extensions in PATHEXT, but none matches + wantErr: exec.ErrNotFound, }, } 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() + // Not parallel: uses Chdir and Setenv. - tmp := t.TempDir() - printpathExe := buildPrintPathExe(t, tmp) + // We are using the "printpath" command mode to test exec.Command here, + // so we won't be calling helperCommand to resolve it. + // That may cause it to appear to be unused. + maySkipHelperCommand("printpath") - // Run all tests. - for i, test := range lookPathTests { - i, test := i, test - t.Run(fmt.Sprint(i), func(t *testing.T) { - t.Parallel() + // Before we begin, find the absolute path to cmd.exe. + // In non-short mode, we will use it to check the ground truth + // of the test's "want" field. + cmdExe, err := exec.LookPath("cmd") + if err != nil { + t.Fatal(err) + } - dir := filepath.Join(tmp, "d"+strconv.Itoa(i)) - err := os.Mkdir(dir, 0700) - if err != nil { - t.Fatal("Mkdir failed: ", err) + for _, tt := range lookPathTests { + t.Run(tt.name, func(t *testing.T) { + if tt.want == "" && tt.wantErr == nil { + t.Fatalf("test must specify either want or wantErr") } - test.run(t, dir, printpathExe) - }) - } -} -type commandTest struct { - PATH string - files []string - dir string - arg0 string - want string - fails bool // test is expected to fail -} + root := t.TempDir() + installProgs(t, root, tt.files) -func (test commandTest) isSuccess(rootDir, output string, err error) error { - if err != nil { - return fmt.Errorf("test=%+v: exec: %v %v", test, err, output) - } - path := output - if path[:len(rootDir)] != rootDir { - return fmt.Errorf("test=%+v: %q must have %q prefix", test, path, rootDir) - } - path = path[len(rootDir)+1:] - if path != test.want { - return fmt.Errorf("test=%+v: want %q, got %q", test, test.want, path) - } - return nil -} + if tt.PATHEXT != "" { + t.Setenv("PATHEXT", tt.PATHEXT) + t.Logf("set PATHEXT=%s", tt.PATHEXT) + } -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 { - t.Errorf("test=%+v: succeeded, but expected to fail", test) - } else { - t.Error(err) - } + var pathVar string + { + paths := make([]string, 0, len(tt.files)) + for _, f := range tt.files { + dir := filepath.Join(root, filepath.Dir(f)) + if !slices.Contains(paths, dir) { + paths = append(paths, dir) + } + } + pathVar = strings.Join(paths, string(os.PathListSeparator)) + } + t.Setenv("PATH", pathVar) + t.Logf("set PATH=%s", pathVar) + + chdir(t, root) + + if !testing.Short() { + // Check that cmd.exe, which is our source of ground truth, + // agrees that our test case is correct. + cmd := testenv.Command(t, cmdExe, "/c", tt.searchFor, "printpath") + out, err := cmd.Output() + if err == nil { + gotAbs := strings.TrimSpace(string(out)) + wantAbs := "" + if tt.want != "" { + wantAbs = filepath.Join(root, tt.want) + } + if gotAbs != wantAbs { + // cmd.exe disagrees. Probably the test case is wrong? + t.Fatalf("%v\n\tresolved to %s\n\twant %s", cmd, gotAbs, wantAbs) + } + } else if tt.wantErr == nil { + if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { + t.Fatalf("%v: %v\n%s", cmd, err, ee.Stderr) + } + t.Fatalf("%v: %v", cmd, err) + } + } + + got, err := exec.LookPath(tt.searchFor) + if filepath.IsAbs(got) { + got, err = filepath.Rel(root, got) + if err != nil { + t.Fatal(err) + } + } + if got != tt.want { + t.Errorf("LookPath(%#q) = %#q; want %#q", tt.searchFor, got, tt.want) + } + if !errors.Is(err, tt.wantErr) { + t.Errorf("LookPath(%#q): %v; want %v", tt.searchFor, err, tt.wantErr) + } + }) } } -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) - test.runOne(t, rootDir, env, test.dir, test.arg0) +type commandTest struct { + name string + PATH []string + files []string + dir string + arg0 string + want string + wantPath string // the resolved c.Path, if different from want + wantErrDot bool + wantRunErr error } var commandTests = []commandTest{ // testing commands with no slash, like `a.exe` { - // should find a.exe in current directory - files: []string{`a.exe`}, - arg0: `a.exe`, - want: `a.exe`, + name: "current directory", + files: []string{`a.exe`}, + PATH: []string{"."}, + arg0: `a.exe`, + want: `a.exe`, + wantErrDot: true, }, { - // like above, but add PATH in attempt to break the test - PATH: `p2;p`, - files: []string{`a.exe`, `p\a.exe`, `p2\a.exe`}, - arg0: `a.exe`, - want: `a.exe`, + name: "with extra PATH", + files: []string{`a.exe`, `p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2", "p"}, + arg0: `a.exe`, + want: `a.exe`, + wantErrDot: true, }, { - // like above, but use "a" instead of "a.exe" for command - PATH: `p2;p`, - files: []string{`a.exe`, `p\a.exe`, `p2\a.exe`}, - arg0: `a`, - want: `a.exe`, + name: "with extra PATH and no extension", + files: []string{`a.exe`, `p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2", "p"}, + arg0: `a`, + want: `a.exe`, + wantErrDot: true, }, // testing commands with slash, like `.\a.exe` { - // should find p\a.exe + name: "with dir", files: []string{`p\a.exe`}, + PATH: []string{"."}, arg0: `p\a.exe`, want: `p\a.exe`, }, { - // like above, but adding `.` in front of executable should still be OK + name: "with explicit dot", files: []string{`p\a.exe`}, + PATH: []string{"."}, arg0: `.\p\a.exe`, want: `p\a.exe`, }, { - // like above, but with PATH added in attempt to break it - PATH: `p2`, + name: "with irrelevant PATH", files: []string{`p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2"}, arg0: `p\a.exe`, want: `p\a.exe`, }, { - // like above, but make sure .exe is tried even for commands with slash - PATH: `p2`, + name: "with slash and no extension", files: []string{`p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2"}, arg0: `p\a`, want: `p\a.exe`, }, // tests commands, like `a.exe`, with c.Dir set { - // should not find a.exe in p, because LookPath(`a.exe`) will fail - files: []string{`p\a.exe`}, - dir: `p`, - arg0: `a.exe`, - want: `p\a.exe`, - fails: true, + // should not find a.exe in p, because LookPath(`a.exe`) will fail when + // called by Command (before Dir is set), and that error is sticky. + name: "not found before Dir", + files: []string{`p\a.exe`}, + PATH: []string{"."}, + dir: `p`, + arg0: `a.exe`, + want: `p\a.exe`, + wantRunErr: exec.ErrNotFound, }, { - // LookPath(`a.exe`) will find `.\a.exe`, but prefixing that with + // LookPath(`a.exe`) will resolve to `.\a.exe`, but prefixing that with // dir `p\a.exe` will refer to a non-existent file - files: []string{`a.exe`, `p\not_important_file`}, - dir: `p`, - arg0: `a.exe`, - want: `a.exe`, - fails: true, + name: "resolved before Dir", + files: []string{`a.exe`, `p\not_important_file`}, + PATH: []string{"."}, + dir: `p`, + arg0: `a.exe`, + want: `a.exe`, + wantErrDot: true, + wantRunErr: fs.ErrNotExist, }, { // like above, but making test succeed by installing file // in referred destination (so LookPath(`a.exe`) will still // find `.\a.exe`, but we successfully execute `p\a.exe`) - files: []string{`a.exe`, `p\a.exe`}, - dir: `p`, - arg0: `a.exe`, - want: `p\a.exe`, + name: "relative to Dir", + files: []string{`a.exe`, `p\a.exe`}, + PATH: []string{"."}, + dir: `p`, + arg0: `a.exe`, + want: `p\a.exe`, + wantErrDot: true, }, { // like above, but add PATH in attempt to break the test - PATH: `p2;p`, - files: []string{`a.exe`, `p\a.exe`, `p2\a.exe`}, - dir: `p`, - arg0: `a.exe`, - want: `p\a.exe`, + name: "relative to Dir with extra PATH", + files: []string{`a.exe`, `p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2", "p"}, + dir: `p`, + arg0: `a.exe`, + want: `p\a.exe`, + wantErrDot: true, }, { // like above, but use "a" instead of "a.exe" for command - PATH: `p2;p`, - files: []string{`a.exe`, `p\a.exe`, `p2\a.exe`}, - dir: `p`, - arg0: `a`, - want: `p\a.exe`, - }, - { - // finds `a.exe` in the PATH regardless of dir set - // because LookPath returns full path in that case - PATH: `p2;p`, + name: "relative to Dir with extra PATH and no extension", + files: []string{`a.exe`, `p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2", "p"}, + dir: `p`, + arg0: `a`, + want: `p\a.exe`, + wantErrDot: true, + }, + { + // finds `a.exe` in the PATH regardless of Dir because Command resolves the + // full path (using LookPath) before Dir is set. + name: "from PATH with no match in Dir", files: []string{`p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2", "p"}, dir: `p`, arg0: `a.exe`, want: `p2\a.exe`, @@ -509,104 +448,127 @@ var commandTests = []commandTest{ // tests commands, like `.\a.exe`, with c.Dir set { // should use dir when command is path, like ".\a.exe" + name: "relative to Dir with explicit dot", files: []string{`p\a.exe`}, + PATH: []string{"."}, dir: `p`, arg0: `.\a.exe`, want: `p\a.exe`, }, { // like above, but with PATH added in attempt to break it - PATH: `p2`, + name: "relative to Dir with dot and extra PATH", files: []string{`p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2"}, dir: `p`, arg0: `.\a.exe`, want: `p\a.exe`, }, { - // like above, but make sure .exe is tried even for commands with slash - PATH: `p2`, + // LookPath(".\a") will fail before Dir is set, and that error is sticky. + name: "relative to Dir with dot and extra PATH and no extension", files: []string{`p\a.exe`, `p2\a.exe`}, + PATH: []string{".", "p2"}, dir: `p`, arg0: `.\a`, want: `p\a.exe`, }, + { + // LookPath(".\a") will fail before Dir is set, and that error is sticky. + name: "relative to Dir with different extension", + files: []string{`a.exe`, `p\a.bat`}, + PATH: []string{"."}, + dir: `p`, + arg0: `.\a`, + want: `p\a.bat`, + }, } 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) + // Not parallel: uses Chdir and Setenv. - // Run all tests. - for i, test := range commandTests { - i, test := i, test - t.Run(fmt.Sprint(i), func(t *testing.T) { - t.Parallel() + // We are using the "printpath" command mode to test exec.Command here, + // so we won't be calling helperCommand to resolve it. + // That may cause it to appear to be unused. + maySkipHelperCommand("printpath") - dir := filepath.Join(tmp, "d"+strconv.Itoa(i)) - err := os.Mkdir(dir, 0700) - if err != nil { - t.Fatal("Mkdir failed: ", err) + for _, tt := range commandTests { + t.Run(tt.name, func(t *testing.T) { + if tt.PATH == nil { + t.Fatalf("test must specify PATH") } - test.run(t, dir, printpathExe) - }) - } -} -// buildPrintPathExe creates a Go program that prints its own path. -// dir is a temp directory where executable will be created. -// The function returns full path to the created program. -func buildPrintPathExe(t *testing.T, dir string) string { - const name = "printpath" - srcname := name + ".go" - err := os.WriteFile(filepath.Join(dir, srcname), []byte(printpathSrc), 0644) - if err != nil { - t.Fatalf("failed to create source: %v", err) - } - if err != nil { - t.Fatalf("failed to execute template: %v", err) - } - outname := name + ".exe" - cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-o", outname, srcname) - cmd.Dir = dir - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("failed to build executable: %v - %v", err, string(out)) - } - return filepath.Join(dir, outname) -} + root := t.TempDir() + installProgs(t, root, tt.files) -const printpathSrc = ` -package main + paths := make([]string, 0, len(tt.PATH)) + for _, p := range tt.PATH { + if p == "." { + paths = append(paths, ".") + } else { + paths = append(paths, filepath.Join(root, p)) + } + } + pathVar := strings.Join(paths, string(os.PathListSeparator)) + t.Setenv("PATH", pathVar) + t.Logf("set PATH=%s", pathVar) + + chdir(t, root) + + cmd := exec.Command(tt.arg0, "printpath") + cmd.Dir = filepath.Join(root, tt.dir) + if tt.wantErrDot { + if errors.Is(cmd.Err, exec.ErrDot) { + cmd.Err = nil + } else { + t.Fatalf("cmd.Err = %v; want ErrDot", cmd.Err) + } + } -import ( - "os" - "syscall" - "unsafe" -) + out, err := cmd.Output() + if err != nil { + if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { + t.Logf("%v: %v\n%s", cmd, err, ee.Stderr) + } else { + t.Logf("%v: %v", cmd, err) + } + if !errors.Is(err, tt.wantRunErr) { + t.Errorf("want %v", tt.wantRunErr) + } + return + } -func getMyName() (string, error) { - var sysproc = syscall.MustLoadDLL("kernel32.dll").MustFindProc("GetModuleFileNameW") - b := make([]uint16, syscall.MAX_PATH) - r, _, err := sysproc.Call(0, uintptr(unsafe.Pointer(&b[0])), uintptr(len(b))) - n := uint32(r) - if n == 0 { - return "", err - } - return syscall.UTF16ToString(b[0:n]), nil -} + got := strings.TrimSpace(string(out)) + if filepath.IsAbs(got) { + got, err = filepath.Rel(root, got) + if err != nil { + t.Fatal(err) + } + } + if got != tt.want { + t.Errorf("\nran %#q\nwant %#q", got, tt.want) + } -func main() { - path, err := getMyName() - if err != nil { - os.Stderr.Write([]byte("getMyName failed: " + err.Error() + "\n")) - os.Exit(1) + gotPath := cmd.Path + wantPath := tt.wantPath + if wantPath == "" { + if strings.Contains(tt.arg0, `\`) { + wantPath = tt.arg0 + if filepath.Ext(wantPath) == "" { + wantPath += filepath.Ext(tt.want) + } + } else if tt.wantErrDot { + wantPath = strings.TrimPrefix(tt.want, tt.dir+`\`) + if filepath.Base(wantPath) == wantPath { + wantPath = `.\` + wantPath + } + } else { + wantPath = filepath.Join(root, tt.want) + } + } + if gotPath != wantPath { + t.Errorf("\ncmd.Path = %#q\nwant %#q", gotPath, wantPath) + } + }) } - os.Stdout.Write([]byte(path)) } -`