Use t.Cleanup to remove temporally directories and set environment variables

cmd_create_test.go

@@ -27,9 +27,8 @@ func TestDoCreate(t *testing.T) {
 	_home = ""
 	homeOnce = &sync.Once{}
 	tmpd := newTempDir(t)
-	defer os.RemoveAll(tmpd)
 	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
-	defer tmpEnv(envGhqRoot, tmpd)()
+	setEnv(t, envGhqRoot, tmpd)
 	_localRepositoryRoots = nil
 	localRepoOnce = &sync.Once{}
 
@@ -39,7 +38,7 @@ func TestDoCreate(t *testing.T) {
 		want      []string
 		wantDir   string
 		errStr    string
-		setup     func() func()
+		setup     func(t *testing.T)
 		cmdRun    func(cmd *exec.Cmd) error
 		skipOnWin bool
 	}{{
@@ -51,9 +50,8 @@ func TestDoCreate(t *testing.T) {
 		name:  "empty directory exists",
 		input: []string{"create", "motemen/ghqqq"},
 		want:  []string{"git", "init"},
-		setup: func() func() {
+		setup: func(t *testing.T) {
 			os.MkdirAll(filepath.Join(tmpd, "github.com/motemen/ghqqq"), 0755)
-			return func() {}
 		},
 		wantDir: filepath.Join(tmpd, "github.com/motemen/ghqqq"),
 	}, {
@@ -91,22 +89,19 @@ func TestDoCreate(t *testing.T) {
 	}, {
 		name:  "not permitted",
 		input: []string{"create", "motemen/ghq-notpermitted"},
-		setup: func() func() {
+		setup: func(t *testing.T) {
 			f := filepath.Join(tmpd, "github.com/motemen/ghq-notpermitted")
 			os.MkdirAll(f, 0)
-			return func() {
-				os.Chmod(f, 0755)
-			}
+			t.Cleanup(func() { os.Chmod(f, 0755) })
 		},
 		errStr:    "permission denied",
 		skipOnWin: true,
 	}, {
 		name:  "not empty",
 		input: []string{"create", "motemen/ghq-notempty"},
-		setup: func() func() {
+		setup: func(t *testing.T) {
 			f := filepath.Join(tmpd, "github.com/motemen/ghq-notempty", "dummy")
 			os.MkdirAll(f, 0755)
-			return func() {}
 		},
 		errStr: "already exists and not empty",
 	}}
@@ -118,8 +113,7 @@ func TestDoCreate(t *testing.T) {
 			}
 			lastCmd = nil
 			if tc.setup != nil {
-				teardown := tc.setup()
-				defer teardown()
+				tc.setup(t)
 			}
 
 			cmdutil.CommandRunner = commandRunner

cmd_get_test.go

@@ -173,11 +173,10 @@ func TestCommandGet(t *testing.T) {
 		name: "ghq.<url>.root",
 		scenario: func(t *testing.T, tmpRoot string, cloneArgs *_cloneArgs, updateArgs *_updateArgs) {
 			tmpd := newTempDir(t)
-			defer os.RemoveAll(tmpd)
-			defer gitconfig.WithConfig(t, fmt.Sprintf(`
+			t.Cleanup(gitconfig.WithConfig(t, fmt.Sprintf(`
 [ghq "https://github.com/motemen"]
   root = "%s"
-`, filepath.ToSlash(tmpd)))()
+`, filepath.ToSlash(tmpd))))
 			app.Run([]string{"", "get", "motemen/ghq-test-repo"})
 
 			localDir := filepath.Join(tmpd, "github.com", "motemen", "ghq-test-repo")

cmd_list_test.go

@@ -189,17 +189,14 @@ func TestDoList_query(t *testing.T) {
 
 func TestDoList_unique(t *testing.T) {
 	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
-	defer func(orig string) { os.Setenv(envGhqRoot, orig) }(os.Getenv(envGhqRoot))
 
 	tmp1 := newTempDir(t)
-	defer os.RemoveAll(tmp1)
 	tmp2 := newTempDir(t)
-	defer os.RemoveAll(tmp2)
 
 	_localRepositoryRoots = nil
 	localRepoOnce = &sync.Once{}
 	rootPaths := []string{tmp1, tmp2}
-	os.Setenv(envGhqRoot, strings.Join(rootPaths, string(os.PathListSeparator)))
+	setEnv(t, envGhqRoot, strings.Join(rootPaths, string(os.PathListSeparator)))
 	for _, rootPath := range rootPaths {
 		os.MkdirAll(filepath.Join(rootPath, "github.com/motemen/ghq/.git"), 0755)
 	}
@@ -213,7 +210,7 @@ func TestDoList_unique(t *testing.T) {
 
 func TestDoList_unknownRoot(t *testing.T) {
 	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
-	defer tmpEnv(envGhqRoot, "/path/to/unknown-ghq")()
+	setEnv(t, envGhqRoot, "/path/to/unknown-ghq")
 	_localRepositoryRoots = nil
 	localRepoOnce = &sync.Once{}
 
@@ -229,11 +226,8 @@ func TestDoList_notPermittedRoot(t *testing.T) {
 	}
 	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
 	tmpdir := newTempDir(t)
-	defer func(dir string) {
-		os.Chmod(dir, 0755)
-		os.RemoveAll(dir)
-	}(tmpdir)
-	defer tmpEnv(envGhqRoot, tmpdir)()
+	defer os.Chmod(tmpdir, 0755)
+	setEnv(t, envGhqRoot, tmpdir)
 
 	_localRepositoryRoots = nil
 	localRepoOnce = &sync.Once{}
@@ -253,11 +247,8 @@ func TestDoList_withSystemHiddenDir(t *testing.T) {
 	tmpdir := newTempDir(t)
 	systemHidden := filepath.Join(tmpdir, ".system")
 	os.MkdirAll(systemHidden, 0000)
-	defer func(dir string) {
-		os.Chmod(systemHidden, 0755)
-		os.RemoveAll(dir)
-	}(tmpdir)
-	defer tmpEnv(envGhqRoot, tmpdir)()
+	defer os.Chmod(systemHidden, 0755)
+	setEnv(t, envGhqRoot, tmpdir)
 
 	_localRepositoryRoots = nil
 	localRepoOnce = &sync.Once{}

cmd_root_test.go

@@ -38,38 +38,31 @@ func samePaths(lhs, rhs string) bool {
 func TestDoRoot(t *testing.T) {
 	testCases := []struct {
 		name              string
-		setup             func() func()
+		setup             func(t *testing.T)
 		expect, allExpect string
 	}{{
 		name: "env",
-		setup: func() func() {
-			orig := os.Getenv(envGhqRoot)
-			os.Setenv(envGhqRoot, "/path/to/ghqroot1"+string(os.PathListSeparator)+"/path/to/ghqroot2")
-			return func() { os.Setenv(envGhqRoot, orig) }
+		setup: func(t *testing.T) {
+			setEnv(t, envGhqRoot, "/path/to/ghqroot1"+string(os.PathListSeparator)+"/path/to/ghqroot2")
 		},
 		expect:    "/path/to/ghqroot1\n",
 		allExpect: "/path/to/ghqroot1\n/path/to/ghqroot2\n",
 	}, {
 		name: "gitconfig",
-		setup: func() func() {
-			orig := os.Getenv(envGhqRoot)
-			os.Setenv(envGhqRoot, "")
-			teardown := gitconfig.WithConfig(t, `
+		setup: func(t *testing.T) {
+			setEnv(t, envGhqRoot, "")
+			t.Cleanup(gitconfig.WithConfig(t, `
 [ghq]
   root = /path/to/ghqroot12
   root = /path/to/ghqroot12
   root = /path/to/ghqroot11
-`)
-			return func() {
-				os.Setenv(envGhqRoot, orig)
-				teardown()
-			}
+`))
 		},
 		expect:    "/path/to/ghqroot11\n",
 		allExpect: "/path/to/ghqroot11\n/path/to/ghqroot12\n",
 	}, {
 		name: "default home",
-		setup: func() func() {
+		setup: func(t *testing.T) {
 			tmpd := newTempDir(t)
 			fpath := filepath.Join(tmpd, "unknown-ghq-dummy")
 			f, err := os.Create(fpath)
@@ -78,16 +71,9 @@ func TestDoRoot(t *testing.T) {
 			}
 			f.Close()
 
-			restore1 := tmpEnv(envGhqRoot, "")
-			restore2 := tmpEnv("GIT_CONFIG", fpath)
-			restore3 := tmpEnv("HOME", "/path/to/ghqhome")
-
-			return func() {
-				os.RemoveAll(tmpd)
-				restore1()
-				restore2()
-				restore3()
-			}
+			setEnv(t, envGhqRoot, "")
+			setEnv(t, "GIT_CONFIG", fpath)
+			setEnv(t, "HOME", "/path/to/ghqhome")
 		},
 		expect:    "/path/to/ghqhome/ghq\n",
 		allExpect: "/path/to/ghqhome/ghq\n",
@@ -101,7 +87,7 @@ func TestDoRoot(t *testing.T) {
 			defer func(orig string) { _home = orig }(_home)
 			_home = ""
 			homeOnce = &sync.Once{}
-			defer tc.setup()()
+			tc.setup(t)
 			out, _, _ := capture(func() {
 				newApp().Run([]string{"", "root"})
 			})

commands_test.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"net/url"
-	"os"
 	"path/filepath"
 	"sync"
 	"testing"
@@ -23,7 +22,6 @@ type _updateArgs struct {
 
 func withFakeGitBackend(t *testing.T, block func(*testing.T, string, *_cloneArgs, *_updateArgs)) {
 	tmpRoot := newTempDir(t)
-	defer os.RemoveAll(tmpRoot)
 
 	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
 	_localRepositoryRoots = []string{tmpRoot}

helpers_test.go

@@ -89,6 +89,8 @@ func newTempDir(t *testing.T) string {
 	if err != nil {
 		t.Fatal(err)
 	}
+	t.Cleanup(func() { os.RemoveAll(tmpdir) })
+
 	// Resolve /var/folders/.../T/... to /private/var/... in OSX
 	wd, err := os.Getwd()
 	if err != nil {
@@ -119,15 +121,15 @@ func newTempDir(t *testing.T) string {
 	return s
 }
 
-func tmpEnv(key, val string) func() {
+func setEnv(t *testing.T, key, val string) {
 	orig, ok := os.LookupEnv(key)
 	os.Setenv(key, val)
 
-	return func() {
+	t.Cleanup(func() {
 		if ok {
 			os.Setenv(key, orig)
 		} else {
 			os.Unsetenv(key)
 		}
-	}
+	})
 }