From 11f159456b1dba3ec499da916852dd188d1e04a7 Mon Sep 17 00:00:00 2001 From: Manlio Perillo Date: Wed, 7 Apr 2021 16:48:41 +0200 Subject: [PATCH] path/filepath: replace os.MkdirTemp with T.TempDir Add the tempDirCanonical function, for tests that need a temporary directory that does not contain symlinks. Updates #45402 Change-Id: I3d08ef32ef911331544acce3d7d013b4c3382960 Reviewed-on: https://go-review.googlesource.com/c/go/+/308011 Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Reviewed-by: Tobias Klauser --- src/path/filepath/match_test.go | 29 ++-------- src/path/filepath/path_test.go | 13 +++++ src/path/filepath/path_windows_test.go | 74 ++++---------------------- 3 files changed, 28 insertions(+), 88 deletions(-) diff --git a/src/path/filepath/match_test.go b/src/path/filepath/match_test.go index 48880ea4395..375c41a7e9d 100644 --- a/src/path/filepath/match_test.go +++ b/src/path/filepath/match_test.go @@ -181,12 +181,7 @@ var globSymlinkTests = []struct { func TestGlobSymlink(t *testing.T) { testenv.MustHaveSymlink(t) - tmpDir, err := os.MkdirTemp("", "globsymlink") - if err != nil { - t.Fatal("creating temp dir:", err) - } - defer os.RemoveAll(tmpDir) - + tmpDir := t.TempDir() for _, tt := range globSymlinkTests { path := Join(tmpDir, tt.path) dest := Join(tmpDir, tt.dest) @@ -267,18 +262,7 @@ func TestWindowsGlob(t *testing.T) { t.Skipf("skipping windows specific test") } - tmpDir, err := os.MkdirTemp("", "TestWindowsGlob") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpDir) - - // /tmp may itself be a symlink - tmpDir, err = EvalSymlinks(tmpDir) - if err != nil { - t.Fatal("eval symlink for tmp dir:", err) - } - + tmpDir := tempDirCanonical(t) if len(tmpDir) < 3 { t.Fatalf("tmpDir path %q is too short", tmpDir) } @@ -323,15 +307,13 @@ func TestWindowsGlob(t *testing.T) { // test absolute paths for _, test := range tests { var p string - err = test.globAbs(tmpDir, tmpDir) - if err != nil { + if err := test.globAbs(tmpDir, tmpDir); err != nil { t.Error(err) } // test C:\*Documents and Settings\... p = tmpDir p = strings.Replace(p, `:\`, `:\*`, 1) - err = test.globAbs(tmpDir, p) - if err != nil { + if err := test.globAbs(tmpDir, p); err != nil { t.Error(err) } // test C:\Documents and Settings*\... @@ -339,8 +321,7 @@ func TestWindowsGlob(t *testing.T) { p = strings.Replace(p, `:\`, `:`, 1) p = strings.Replace(p, `\`, `*\`, 1) p = strings.Replace(p, `:`, `:\`, 1) - err = test.globAbs(tmpDir, p) - if err != nil { + if err := test.globAbs(tmpDir, p); err != nil { t.Error(err) } } diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index bb6bc0dd873..cd107b6c85a 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -449,6 +449,19 @@ func chtmpdir(t *testing.T) (restore func()) { } } +// tempDirCanonical returns a temporary directory for the test to use, ensuring +// that the returned path does not contain symlinks. +func tempDirCanonical(t *testing.T) string { + dir := t.TempDir() + + cdir, err := filepath.EvalSymlinks(dir) + if err != nil { + t.Errorf("tempDirCanonical: %v", err) + } + + return cdir +} + func TestWalk(t *testing.T) { walk := func(root string, fn fs.WalkDirFunc) error { return filepath.Walk(root, func(path string, info fs.FileInfo, err error) error { diff --git a/src/path/filepath/path_windows_test.go b/src/path/filepath/path_windows_test.go index 1c3d84c62d1..76a459ac96b 100644 --- a/src/path/filepath/path_windows_test.go +++ b/src/path/filepath/path_windows_test.go @@ -37,12 +37,7 @@ func testWinSplitListTestIsValid(t *testing.T, ti int, tt SplitListTest, perm fs.FileMode = 0700 ) - tmp, err := os.MkdirTemp("", "testWinSplitListTestIsValid") - if err != nil { - t.Fatalf("TempDir failed: %v", err) - } - defer os.RemoveAll(tmp) - + tmp := t.TempDir() for i, d := range tt.result { if d == "" { continue @@ -57,12 +52,12 @@ func testWinSplitListTestIsValid(t *testing.T, ti int, tt SplitListTest, t.Errorf("%d,%d: %#q already exists", ti, i, d) return } - if err = os.MkdirAll(dd, perm); err != nil { + if err := os.MkdirAll(dd, perm); err != nil { t.Errorf("%d,%d: MkdirAll(%#q) failed: %v", ti, i, dd, err) return } fn, data := filepath.Join(dd, cmdfile), []byte("@echo "+d+"\r\n") - if err = os.WriteFile(fn, data, perm); err != nil { + if err := os.WriteFile(fn, data, perm); err != nil { t.Errorf("%d,%d: WriteFile(%#q) failed: %v", ti, i, fn, err) return } @@ -103,18 +98,7 @@ func testWinSplitListTestIsValid(t *testing.T, ti int, tt SplitListTest, func TestWindowsEvalSymlinks(t *testing.T) { testenv.MustHaveSymlink(t) - tmpDir, err := os.MkdirTemp("", "TestWindowsEvalSymlinks") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpDir) - - // /tmp may itself be a symlink! Avoid the confusion, although - // it means trusting the thing we're testing. - tmpDir, err = filepath.EvalSymlinks(tmpDir) - if err != nil { - t.Fatal(err) - } + tmpDir := tempDirCanonical(t) if len(tmpDir) < 3 { t.Fatalf("tmpDir path %q is too short", tmpDir) @@ -161,18 +145,7 @@ func TestWindowsEvalSymlinks(t *testing.T) { // TestEvalSymlinksCanonicalNames verify that EvalSymlinks // returns "canonical" path names on windows. func TestEvalSymlinksCanonicalNames(t *testing.T) { - tmp, err := os.MkdirTemp("", "evalsymlinkcanonical") - if err != nil { - t.Fatal("creating temp dir:", err) - } - defer os.RemoveAll(tmp) - - // os.MkdirTemp might return "non-canonical" name. - cTmpName, err := filepath.EvalSymlinks(tmp) - if err != nil { - t.Errorf("EvalSymlinks(%q) error: %v", tmp, err) - } - + ctmp := tempDirCanonical(t) dirs := []string{ "test", "test/dir", @@ -181,7 +154,7 @@ func TestEvalSymlinksCanonicalNames(t *testing.T) { } for _, d := range dirs { - dir := filepath.Join(cTmpName, d) + dir := filepath.Join(ctmp, d) err := os.Mkdir(dir, 0755) if err != nil { t.Fatal(err) @@ -417,25 +390,8 @@ func TestToNorm(t *testing.T) { {".", `\\localhost\c$`, `\\localhost\c$`}, } - tmp, err := os.MkdirTemp("", "testToNorm") - if err != nil { - t.Fatal(err) - } - defer func() { - err := os.RemoveAll(tmp) - if err != nil { - t.Fatal(err) - } - }() - - // os.MkdirTemp might return "non-canonical" name. - ctmp, err := filepath.EvalSymlinks(tmp) - if err != nil { - t.Fatal(err) - } - - err = os.MkdirAll(strings.ReplaceAll(testPath, "{{tmp}}", ctmp), 0777) - if err != nil { + ctmp := tempDirCanonical(t) + if err := os.MkdirAll(strings.ReplaceAll(testPath, "{{tmp}}", ctmp), 0777); err != nil { t.Fatal(err) } @@ -526,20 +482,10 @@ func TestNTNamespaceSymlink(t *testing.T) { t.Skip("skipping test because mklink command does not support junctions") } - tmpdir, err := os.MkdirTemp("", "TestNTNamespaceSymlink") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpdir) - - // Make sure tmpdir is not a symlink, otherwise tests will fail. - tmpdir, err = filepath.EvalSymlinks(tmpdir) - if err != nil { - t.Fatal(err) - } + tmpdir := tempDirCanonical(t) vol := filepath.VolumeName(tmpdir) - output, err = exec.Command("cmd", "/c", "mountvol", vol, "/L").CombinedOutput() + output, err := exec.Command("cmd", "/c", "mountvol", vol, "/L").CombinedOutput() if err != nil { t.Fatalf("failed to run mountvol %v /L: %v %q", vol, err, output) }