Skip to content

Commit

Permalink
os/exec: simplify Windows-specific tests
Browse files Browse the repository at this point in the history
- 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 <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
  • Loading branch information
Bryan C. Mills authored and gopherbot committed Sep 13, 2023
1 parent dd2279e commit 105f9d5
Show file tree
Hide file tree
Showing 4 changed files with 400 additions and 423 deletions.
15 changes: 2 additions & 13 deletions src/os/exec/dot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)

Expand Down
37 changes: 37 additions & 0 deletions src/os/exec/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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){
Expand Down
23 changes: 6 additions & 17 deletions src/os/exec/lp_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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")
}
Expand Down
Loading

0 comments on commit 105f9d5

Please sign in to comment.