test: use T.TempDir to create temporary test directory (#4587)

* test: use `T.TempDir` to create temporary test directory

This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.

Prior to this commit, temporary directory created using `ioutil.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
	defer func() {
		if err := os.RemoveAll(dir); err != nil {
			t.Fatal(err)
		}
	}
is also tedious, but `t.TempDir` handles this for us nicely.

Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>

* test: fix `TestInit_noargs` on Windows

--- FAIL: TestInit_noargs (0.01s)
    testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestInit_noargs3136084632\001: The process cannot access the file because it is being used by another process.

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>

* test: fix `TestTreeCommandDefaultCurDir_files` on Windows

--- FAIL: TestTreeCommandDefaultCurDir_files (0.01s)
    testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestTreeCommandDefaultCurDir_files716679291\001: The process cannot access the file because it is being used by another process.

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>

* test: fix `TestCreateSetterCommand` on Windows

--- FAIL: TestCreateSetterCommand (13.27s)
    --- FAIL: TestCreateSetterCommand/add_replicas (1.45s)
        testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestCreateSetterCommandadd_replicas176222064\001\k8s-cli-487197005.yaml: The process cannot access the file because it is being used by another process.
    ...
    and all subtests
    ...

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>

* test: fix `TestCreateSubstitutionCommand` on Windows

--- FAIL: TestCreateSubstitutionCommand (4.16s)
    --- FAIL: TestCreateSubstitutionCommand/substitution_replicas (1.30s)
        testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestCreateSubstitutionCommandsubstitution_replicas1352417113\001\k8s-cli-3183612276.yaml: The process cannot access the file because it is being used by another process.
    ...
    and all subtests
    ...

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>

* test: fix `TestDeleteSetterCommand` on Windows

--- FAIL: TestDeleteSetterCommand (4.36s)
    --- FAIL: TestDeleteSetterCommand/delete_replicas (1.31s)
        testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestDeleteSetterCommanddelete_replicas3949811929\001\k8s-cli-957077271.yaml: The process cannot access the file because it is being used by another process.
    ...
    and all subtests
    ...

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>

* test: fix `TestDeleteSubstitutionCommand` on Windows

--- FAIL: TestDeleteSubstitutionCommand (2.35s)
    --- FAIL: TestDeleteSubstitutionCommand/delete_only_subst_if_setter_has_same_name_-_long_ref (1.15s)
        testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestDeleteSubstitutionCommanddelete_only_subst_if_setter_has_same_name_-_long_ref2039728641\001\k8s-cli-1602861478.yaml: The process cannot access the file because it is being used by another process.
    ...
    and all subtests
    ...

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>

* test: fix `TestSetCommand` on Windows

--- FAIL: TestSetCommand (13.76s)
    --- FAIL: TestSetCommand/set_replicas (1.13s)
        testing.go:1090: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestSetCommandset_replicas3781384539\001\k8s-cli-1030344459.yaml: The process cannot access the file because it is being used by another process.
    ...
    and all subtests
    ...

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>

api/krusty/pluginenv_test.go

@@ -4,7 +4,6 @@
 package krusty_test
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -27,8 +26,13 @@ func TestPluginEnvironment(t *testing.T) {
 		kusttest_test.MakeHarnessWithFs(t, filesys.MakeFsInMemory()),
 		filesys.Separator)
 
-	dir := makeTmpDir(t)
-	defer os.RemoveAll(dir)
+	// On MacOS, $TMPDIR is by default set to /var/folders/…, with /var a
+	// symlink to /private/var , which does not match our expectations
+	dir, err := filepath.EvalSymlinks(t.TempDir())
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
 	confirmBehavior(
 		kusttest_test.MakeHarnessWithFs(t, filesys.MakeFsOnDisk()),
 		dir)
@@ -65,16 +69,3 @@ metadata:
   name: hello
 `)
 }
-
-func makeTmpDir(t *testing.T) string {
-	t.Helper()
-	base, err := os.Getwd()
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	dir, err := ioutil.TempDir(base, "kustomize-tmp-test-")
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	return dir
-}

api/loader/fileloader_test.go

@@ -213,11 +213,9 @@ const (
 //   │   └── symLinkToExteriorData -> ../exteriorData
 //   └── exteriorData
 //
-func commonSetupForLoaderRestrictionTest() (string, filesys.FileSystem, error) {
-	dir, err := ioutil.TempDir("", "kustomize-test-")
-	if err != nil {
-		return "", nil, err
-	}
+func commonSetupForLoaderRestrictionTest(t *testing.T) (string, filesys.FileSystem) {
+	t.Helper()
+	dir := t.TempDir()
 	fSys := filesys.MakeFsOnDisk()
 	fSys.Mkdir(filepath.Join(dir, "base"))
 
@@ -233,7 +231,7 @@ func commonSetupForLoaderRestrictionTest() (string, filesys.FileSystem, error) {
 	os.Symlink(
 		filepath.Join(dir, "exteriorData"),
 		filepath.Join(dir, "base", "symLinkToExteriorData"))
-	return dir, fSys, nil
+	return dir, fSys
 }
 
 // Make sure everything works when loading files
@@ -283,11 +281,7 @@ func doSanityChecksAndDropIntoBase(
 }
 
 func TestRestrictionRootOnlyInRealLoader(t *testing.T) {
-	dir, fSys, err := commonSetupForLoaderRestrictionTest()
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	defer os.RemoveAll(dir)
+	dir, fSys := commonSetupForLoaderRestrictionTest(t)
 
 	var l ifc.Loader
 
@@ -296,7 +290,7 @@ func TestRestrictionRootOnlyInRealLoader(t *testing.T) {
 	l = doSanityChecksAndDropIntoBase(t, l)
 
 	// Reading symlink to exteriorData fails.
-	_, err = l.Load("symLinkToExteriorData")
+	_, err := l.Load("symLinkToExteriorData")
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -316,11 +310,7 @@ func TestRestrictionRootOnlyInRealLoader(t *testing.T) {
 }
 
 func TestRestrictionNoneInRealLoader(t *testing.T) {
-	dir, fSys, err := commonSetupForLoaderRestrictionTest()
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	defer os.RemoveAll(dir)
+	dir, fSys := commonSetupForLoaderRestrictionTest(t)
 
 	var l ifc.Loader
 
@@ -329,7 +319,7 @@ func TestRestrictionNoneInRealLoader(t *testing.T) {
 	l = doSanityChecksAndDropIntoBase(t, l)
 
 	// Reading symlink to exteriorData works.
-	_, err = l.Load("symLinkToExteriorData")
+	_, err := l.Load("symLinkToExteriorData")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}

cmd/config/internal/commands/annotate_test.go

@@ -6,7 +6,6 @@ package commands
 import (
 	"bytes"
 	"io/ioutil"
-	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -58,7 +57,6 @@ func TestAnnotateCommand(t *testing.T) {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			d := initTestDir(t)
-			defer os.RemoveAll(d)
 
 			a := NewAnnotateRunner("")
 			a.Command.SetArgs(append([]string{d}, tt.args...))
@@ -89,18 +87,13 @@ func TestAnnotateCommand(t *testing.T) {
 
 func initTestDir(t *testing.T) string {
 	t.Helper()
-	d, err := ioutil.TempDir("", "kustomize-annotate-test")
+	d := t.TempDir()
+	err := ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(f1Input), 0600)
 	if !assert.NoError(t, err) {
 		t.FailNow()
 	}
-	err = ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(f1Input), 0600)
-	if !assert.NoError(t, err) {
-		defer os.RemoveAll(d)
-		t.FailNow()
-	}
 	err = ioutil.WriteFile(filepath.Join(d, "f2.yaml"), []byte(f2Input), 0600)
 	if !assert.NoError(t, err) {
-		defer os.RemoveAll(d)
 		t.FailNow()
 	}
 	return d
@@ -586,17 +579,13 @@ added annotations in the package
 			openapi.ResetOpenAPI()
 			defer openapi.ResetOpenAPI()
 			sourceDir := filepath.Join("test", "testdata", test.dataset)
-			baseDir, err := ioutil.TempDir("", "")
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
+			baseDir := t.TempDir()
 			copyutil.CopyDir(sourceDir, baseDir)
-			defer os.RemoveAll(baseDir)
 			runner := NewAnnotateRunner("")
 			actual := &bytes.Buffer{}
 			runner.Command.SetOut(actual)
 			runner.Command.SetArgs(append([]string{filepath.Join(baseDir, test.packagePath)}, test.args...))
-			err = runner.Command.Execute()
+			err := runner.Command.Execute()
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}

cmd/config/internal/commands/cat_test.go

@@ -20,13 +20,9 @@ import (
 // TODO(pwittrock): write tests for reading / writing ResourceLists
 
 func TestCmd_files(t *testing.T) {
-	d, err := ioutil.TempDir("", "kustomize-cat-test")
-	if !assert.NoError(t, err) {
-		return
-	}
-	defer os.RemoveAll(d)
+	d := t.TempDir()
 
-	err = ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
+	err := ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
 kind: Deployment
 metadata:
   labels:
@@ -121,13 +117,9 @@ spec:
 }
 
 func TestCmd_filesWithReconcilers(t *testing.T) {
-	d, err := ioutil.TempDir("", "kustomize-cat-test")
-	if !assert.NoError(t, err) {
-		return
-	}
-	defer os.RemoveAll(d)
+	d := t.TempDir()
 
-	err = ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
+	err := ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
 kind: Deployment
 metadata:
   labels:
@@ -231,13 +223,9 @@ spec:
 }
 
 func TestCmd_filesWithoutNonReconcilers(t *testing.T) {
-	d, err := ioutil.TempDir("", "kustomize-cat-test")
-	if !assert.NoError(t, err) {
-		return
-	}
-	defer os.RemoveAll(d)
+	d := t.TempDir()
 
-	err = ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
+	err := ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
 kind: Deployment
 metadata:
   labels:
@@ -313,13 +301,9 @@ spec:
 }
 
 func TestCmd_outputTruncateFile(t *testing.T) {
-	d, err := ioutil.TempDir("", "kustomize-cat-test")
-	if !assert.NoError(t, err) {
-		return
-	}
-	defer os.RemoveAll(d)
+	d := t.TempDir()
 
-	err = ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
+	err := ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
 kind: Deployment
 metadata:
   labels:
@@ -427,13 +411,9 @@ spec:
 }
 
 func TestCmd_outputCreateFile(t *testing.T) {
-	d, err := ioutil.TempDir("", "kustomize-cat-test")
-	if !assert.NoError(t, err) {
-		return
-	}
-	defer os.RemoveAll(d)
+	d := t.TempDir()
 
-	err = ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
+	err := ioutil.WriteFile(filepath.Join(d, "f1.yaml"), []byte(`
 kind: Deployment
 metadata:
   labels:
@@ -483,11 +463,7 @@ spec:
 		return
 	}
 
-	d2, err := ioutil.TempDir("", "kustomize-cat-test-dest")
-	if !assert.NoError(t, err) {
-		return
-	}
-	defer os.RemoveAll(d2)
+	d2 := t.TempDir()
 	f := filepath.Join(d2, "output.yaml")
 
 	// fmt the files
@@ -638,17 +614,13 @@ spec:
 			openapi.ResetOpenAPI()
 			defer openapi.ResetOpenAPI()
 			sourceDir := filepath.Join("test", "testdata", test.dataset)
-			baseDir, err := ioutil.TempDir("", "")
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
+			baseDir := t.TempDir()
 			copyutil.CopyDir(sourceDir, baseDir)
-			defer os.RemoveAll(baseDir)
 			runner := commands.GetCatRunner("")
 			actual := &bytes.Buffer{}
 			runner.Command.SetOut(actual)
 			runner.Command.SetArgs(append([]string{filepath.Join(baseDir, test.packagePath)}, test.args...))
-			err = runner.Command.Execute()
+			err := runner.Command.Execute()
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}

cmd/config/internal/commands/cmdcreatesetter_test.go

@@ -699,14 +699,10 @@ spec:
 			openapi.ResetOpenAPI()
 			defer openapi.ResetOpenAPI()
 
-			baseDir, err := ioutil.TempDir("", "")
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
-			defer os.RemoveAll(baseDir)
+			baseDir := t.TempDir()
 
 			f := filepath.Join(baseDir, "Krmfile")
-			err = ioutil.WriteFile(f, []byte(test.inputOpenAPI), 0600)
+			err := ioutil.WriteFile(f, []byte(test.inputOpenAPI), 0600)
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}
@@ -716,7 +712,10 @@ spec:
 				if !assert.NoError(t, err) {
 					t.FailNow()
 				}
-				defer os.Remove(sch.Name())
+				t.Cleanup(func() {
+					sch.Close()
+					os.Remove(sch.Name())
+				})
 
 				err = ioutil.WriteFile(sch.Name(), []byte(test.schema), 0600)
 				if !assert.NoError(t, err) {
@@ -730,7 +729,7 @@ spec:
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}
-			defer os.Remove(r.Name())
+			t.Cleanup(func() { r.Close() })
 			err = ioutil.WriteFile(r.Name(), []byte(test.input), 0600)
 			if !assert.NoError(t, err) {
 				t.FailNow()
@@ -847,17 +846,13 @@ setter with name "namespace" already exists, if you want to modify it, please de
 			openapi.ResetOpenAPI()
 			defer openapi.ResetOpenAPI()
 			sourceDir := filepath.Join("test", "testdata", test.dataset)
-			baseDir, err := ioutil.TempDir("", "")
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
+			baseDir := t.TempDir()
 			copyutil.CopyDir(sourceDir, baseDir)
-			defer os.RemoveAll(baseDir)
 			runner := commands.NewCreateSetterRunner("")
 			actual := &bytes.Buffer{}
 			runner.Command.SetOut(actual)
 			runner.Command.SetArgs(append([]string{filepath.Join(baseDir, test.packagePath)}, test.args...))
-			err = runner.Command.Execute()
+			err := runner.Command.Execute()
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}

cmd/config/internal/commands/cmdcreatesubstitution_test.go

@@ -6,7 +6,6 @@ package commands_test
 import (
 	"bytes"
 	"io/ioutil"
-	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -355,13 +354,9 @@ spec:
 			openapi.ResetOpenAPI()
 			defer openapi.ResetOpenAPI()
 
-			baseDir, err := ioutil.TempDir("", "")
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
-			defer os.RemoveAll(baseDir)
+			baseDir := t.TempDir()
 			f := filepath.Join(baseDir, "Krmfile")
-			err = ioutil.WriteFile(f, []byte(test.inputOpenAPI), 0600)
+			err := ioutil.WriteFile(f, []byte(test.inputOpenAPI), 0600)
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}
@@ -370,7 +365,7 @@ spec:
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}
-			defer os.Remove(r.Name())
+			t.Cleanup(func() { r.Close() })
 			err = ioutil.WriteFile(r.Name(), []byte(test.input), 0600)
 			if !assert.NoError(t, err) {
 				t.FailNow()
@@ -484,17 +479,13 @@ created substitution "image-tag"`,
 			openapi.ResetOpenAPI()
 			defer openapi.ResetOpenAPI()
 			sourceDir := filepath.Join("test", "testdata", test.dataset)
-			baseDir, err := ioutil.TempDir("", "")
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
+			baseDir := t.TempDir()
 			copyutil.CopyDir(sourceDir, baseDir)
-			defer os.RemoveAll(baseDir)
 			runner := commands.NewCreateSubstitutionRunner("")
 			actual := &bytes.Buffer{}
 			runner.Command.SetOut(actual)
 			runner.Command.SetArgs(append([]string{filepath.Join(baseDir, test.packagePath)}, test.args...))
-			err = runner.Command.Execute()
+			err := runner.Command.Execute()
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}