Replace most uses of `ioutil.TempDir()` with `testhelper.TempDir()`

While many of our tests use `ioutil.TempDir`, this is bad practice as
it'll typically create directories directly in `/tmp`. Instead, tests
are expected to call `testhelper.TempDir`, which does roughly the same
but instead creates temporary directories in the global test directory.

This commit converts many users of `ioutil.TempDir()` to
`testhelper.TempDir()`. Remaining users either have no `testing.T`
around or cannot use `internal/testhelper` due to cyclic dependencies.

15 files changed, 69 insertions, 79 deletions

diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index bb81ec32a..bbe14e87f 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -535,11 +535,8 @@ func TestCheckOK(t *testing.T) {
 	serverURL, cleanup := testhelper.NewGitlabTestServer(t, c)
 	defer cleanup()
 
-	tempDir, err := ioutil.TempDir("", t.Name())
-	require.NoError(t, err)
-	defer func() {
-		os.RemoveAll(tempDir)
-	}()
+	tempDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	gitlabShellDir := filepath.Join(tempDir, "gitlab-shell")
 	require.NoError(t, os.MkdirAll(gitlabShellDir, 0755))
@@ -554,7 +551,7 @@ func TestCheckOK(t *testing.T) {
 	cmd.Stderr = &stderr
 	cmd.Stdout = &stdout
 
-	err = cmd.Run()
+	err := cmd.Run()
 	require.NoError(t, err)
 	require.Empty(t, stderr.String())
 
@@ -583,11 +580,8 @@ func TestCheckBadCreds(t *testing.T) {
 	serverURL, cleanup := testhelper.NewGitlabTestServer(t, c)
 	defer cleanup()
 
-	tempDir, err := ioutil.TempDir("", t.Name())
-	require.NoError(t, err)
-	defer func() {
-		os.RemoveAll(tempDir)
-	}()
+	tempDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	gitlabShellDir := filepath.Join(tempDir, "gitlab-shell")
 	require.NoError(t, os.MkdirAll(gitlabShellDir, 0755))
diff --git a/cmd/gitaly-lfs-smudge/lfs_smudge_test.go b/cmd/gitaly-lfs-smudge/lfs_smudge_test.go
index 244cd16d5..a84cf204f 100644
--- a/cmd/gitaly-lfs-smudge/lfs_smudge_test.go
+++ b/cmd/gitaly-lfs-smudge/lfs_smudge_test.go
@@ -91,9 +91,8 @@ func TestSuccessfulLfsSmudge(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	tmpDir, err := ioutil.TempDir("", "")
-	require.NoError(t, err)
-	defer os.RemoveAll(tmpDir)
+	tmpDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	env := map[string]string{
 		"GL_REPOSITORY":      "project-1",
@@ -186,9 +185,8 @@ func TestUnsuccessfulLfsSmudge(t *testing.T) {
 			tlsCfg, err := json.Marshal(tc.tlsCfg)
 			require.NoError(t, err)
 
-			tmpDir, err := ioutil.TempDir("", "")
-			require.NoError(t, err)
-			defer os.RemoveAll(tmpDir)
+			tmpDir, cleanup := testhelper.TempDir(t)
+			defer cleanup()
 
 			env := map[string]string{
 				"GL_REPOSITORY":      "project-1",
diff --git a/internal/bootstrap/bootstrap_test.go b/internal/bootstrap/bootstrap_test.go
index de824fc2b..bf8230c93 100644
--- a/internal/bootstrap/bootstrap_test.go
+++ b/internal/bootstrap/bootstrap_test.go
@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
 type mockUpgrader struct {
@@ -59,7 +60,10 @@ func (s *testServer) slowRequest(duration time.Duration) <-chan error {
 }
 
 func TestCreateUnixListener(t *testing.T) {
-	socketPath := filepath.Join(os.TempDir(), "gitaly-test-unix-socket")
+	tempDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
+
+	socketPath := filepath.Join(tempDir, "gitaly-test-unix-socket")
 	if err := os.Remove(socketPath); err != nil {
 		require.True(t, os.IsNotExist(err), "cannot delete dangling socket: %v", err)
 	}
@@ -288,9 +292,12 @@ func makeBootstrap(t *testing.T) (*Bootstrap, *testServer) {
 		}
 	}
 
+	tempDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
+
 	for network, address := range map[string]string{
 		"tcp":  "127.0.0.1:0",
-		"unix": filepath.Join(os.TempDir(), "gitaly-test-unix-socket"),
+		"unix": filepath.Join(tempDir, "gitaly-test-unix-socket"),
 	} {
 		b.RegisterStarter(start(network, address))
 	}
diff --git a/internal/bootstrap/testhelper_test.go b/internal/bootstrap/testhelper_test.go
new file mode 100644
index 000000000..ad7c28be0
--- /dev/null
+++ b/internal/bootstrap/testhelper_test.go
@@ -0,0 +1,18 @@
+package bootstrap
+
+import (
+	"os"
+	"testing"
+
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestMain(m *testing.M) {
+	os.Exit(testMain(m))
+}
+
+func testMain(m *testing.M) int {
+	cleanup := testhelper.Configure()
+	defer cleanup()
+	return m.Run()
+}
diff --git a/internal/cache/cachedb_test.go b/internal/cache/cachedb_test.go
index d18e0355f..161e24981 100644
--- a/internal/cache/cachedb_test.go
+++ b/internal/cache/cachedb_test.go
@@ -134,8 +134,7 @@ func TestStreamDBNaiveKeyer(t *testing.T) {
 
 func injectTempStorage(t testing.TB) (string, testhelper.Cleanup) {
 	oldStorages := config.Config.Storages
-	tmpDir, err := ioutil.TempDir("", t.Name())
-	require.NoError(t, err)
+	tmpDir, cleanup := testhelper.TempDir(t)
 
 	name := filepath.Base(tmpDir)
 	config.Config.Storages = append(config.Config.Storages, config.Storage{
@@ -143,12 +142,10 @@ func injectTempStorage(t testing.TB) (string, testhelper.Cleanup) {
 		Path: tmpDir,
 	})
 
-	cleanup := func() {
+	return name, func() {
 		config.Config.Storages = oldStorages
-		require.NoError(t, os.RemoveAll(tmpDir))
+		cleanup()
 	}
-
-	return name, cleanup
 }
 
 func TestLoserCount(t *testing.T) {
diff --git a/internal/cache/walker_internal_test.go b/internal/cache/walker_internal_test.go
index bd5d3e768..438978f77 100644
--- a/internal/cache/walker_internal_test.go
+++ b/internal/cache/walker_internal_test.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
 func TestCleanWalkDirNotExists(t *testing.T) {
@@ -19,9 +20,8 @@ func TestCleanWalkDirNotExists(t *testing.T) {
 }
 
 func TestCleanWalkEmptyDirs(t *testing.T) {
-	tmp, err := ioutil.TempDir("", t.Name())
-	require.NoError(t, err)
-	defer func() { require.NoError(t, os.RemoveAll(tmp)) }()
+	tmp, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	for _, tt := range []struct {
 		path  string
diff --git a/internal/cache/walker_test.go b/internal/cache/walker_test.go
index a469ffbd1..d4c341299 100644
--- a/internal/cache/walker_test.go
+++ b/internal/cache/walker_test.go
@@ -93,8 +93,7 @@ func TestDiskCacheInitialClear(t *testing.T) {
 }
 
 func setupDiskCacheWalker(t testing.TB) func() {
-	tmpPath, err := ioutil.TempDir("", t.Name())
-	require.NoError(t, err)
+	tmpPath, cleanup := testhelper.TempDir(t)
 
 	oldStorages := config.Config.Storages
 	config.Config.Storages = []config.Storage{
@@ -104,12 +103,10 @@ func setupDiskCacheWalker(t testing.TB) func() {
 		},
 	}
 
-	cleanup := func() {
+	return func() {
 		config.Config.Storages = oldStorages
-		require.NoError(t, os.RemoveAll(tmpPath))
+		cleanup()
 	}
-
-	return cleanup
 }
 
 func pollCountersUntil(t testing.TB, expectRemovals int) {
diff --git a/internal/git/command_test.go b/internal/git/command_test.go
index 746baf856..ce29dc35d 100644
--- a/internal/git/command_test.go
+++ b/internal/git/command_test.go
@@ -1,7 +1,6 @@
 package git
 
 import (
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -27,9 +26,8 @@ func TestGitCommandProxy(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	dir, err := ioutil.TempDir("", "test-clone")
-	require.NoError(t, err)
-	defer os.RemoveAll(dir)
+	dir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	cmd, err := unsafeCmdWithoutRepo(ctx, CmdStream{}, "clone", "http://gitlab.com/bogus-repo", dir)
 	require.NoError(t, err)
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index f9a1c54ba..65ffefa7f 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"os/exec"
 	"path/filepath"
 	"testing"
@@ -108,15 +107,12 @@ unless out.chomp == ARGV[0]
   abort "error: expected #{ARGV[0]} object, got #{out}"
 end`, command.GitPath())
 
-	dir, err := ioutil.TempDir("", "gitaly-temp-dir-*")
-	require.NoError(t, err)
+	dir, cleanup := testhelper.TempDir(t)
 	hookPath := filepath.Join(dir, "pre-receive")
 
 	require.NoError(t, ioutil.WriteFile(hookPath, []byte(hook), 0755))
 
-	return hookPath, func() {
-		os.RemoveAll(dir)
-	}
+	return hookPath, cleanup
 }
 
 func writeAssertObjectTypeUpdateHook(t *testing.T) (string, func()) {
@@ -136,15 +132,12 @@ unless out.chomp == expected_object_type
   abort "error: expected #{expected_object_type} object, got #{out}"
 end`, command.GitPath())
 
-	dir, err := ioutil.TempDir("", "gitaly-temp-dir-*")
-	require.NoError(t, err)
+	dir, cleanup := testhelper.TempDir(t)
 	hookPath := filepath.Join(dir, "pre-receive")
 
 	require.NoError(t, ioutil.WriteFile(hookPath, []byte(hook), 0755))
 
-	return hookPath, func() {
-		os.RemoveAll(dir)
-	}
+	return hookPath, cleanup
 }
 
 func TestSuccessfulUserCreateTagRequest(t *testing.T) {
diff --git a/internal/gitaly/service/repository/archive_test.go b/internal/gitaly/service/repository/archive_test.go
index 1188d3344..dfba15ee9 100644
--- a/internal/gitaly/service/repository/archive_test.go
+++ b/internal/gitaly/service/repository/archive_test.go
@@ -429,9 +429,8 @@ func TestGetArchivePathInjection(t *testing.T) {
 	defer cancel()
 
 	// Adding a temp directory representing the .ssh directory
-	sshDirectory, err := ioutil.TempDir("", ".ssh")
-	require.NoError(t, err)
-	require.NoError(t, os.MkdirAll(sshDirectory, os.ModeDir|0755))
+	sshDirectory, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	// Adding an empty authorized_keys file
 	authorizedKeysPath := filepath.Join(sshDirectory, "authorized_keys")
diff --git a/internal/gitaly/service/repository/repository_test.go b/internal/gitaly/service/repository/repository_test.go
index 156350b91..7cbc736af 100644
--- a/internal/gitaly/service/repository/repository_test.go
+++ b/internal/gitaly/service/repository/repository_test.go
@@ -1,7 +1,6 @@
 package repository
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -15,9 +14,8 @@ import (
 )
 
 func TestRepositoryExists(t *testing.T) {
-	storageOtherDir, err := ioutil.TempDir("", "gitaly-repository-exists-test")
-	require.NoError(t, err, "tempdir")
-	defer os.Remove(storageOtherDir)
+	storageOtherDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	// Setup storage paths
 	testStorages := []config.Storage{
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index a939c98d1..9c719ee74 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -138,9 +138,8 @@ func TestFailedReceivePackRequestWithGitOpts(t *testing.T) {
 }
 
 func TestFailedReceivePackRequestDueToHooksFailure(t *testing.T) {
-	hookDir, err := ioutil.TempDir("", "gitaly-tmp-hooks")
-	require.NoError(t, err)
-	defer os.RemoveAll(hookDir)
+	hookDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	defer func(override string) {
 		hooks.Override = override
diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go
index 9647e74a4..eb35b143f 100644
--- a/internal/gitaly/service/ssh/receive_pack_test.go
+++ b/internal/gitaly/service/ssh/receive_pack_test.go
@@ -144,9 +144,8 @@ func TestReceivePackPushHookFailure(t *testing.T) {
 	serverSocketPath, stop := runSSHServer(t)
 	defer stop()
 
-	hookDir, err := ioutil.TempDir("", "gitaly-tmp-hooks")
-	require.NoError(t, err)
-	defer os.RemoveAll(hookDir)
+	hookDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	defer func(old string) { hooks.Override = old }(hooks.Override)
 	hooks.Override = hookDir
@@ -156,7 +155,7 @@ func TestReceivePackPushHookFailure(t *testing.T) {
 	hookContent := []byte("#!/bin/sh\nexit 1")
 	ioutil.WriteFile(filepath.Join(hooks.Path(), "pre-receive"), hookContent, 0755)
 
-	_, _, err = testCloneAndPush(t, serverSocketPath, pushParams{storageName: testRepo.GetStorageName(), glID: "1"})
+	_, _, err := testCloneAndPush(t, serverSocketPath, pushParams{storageName: testRepo.GetStorageName(), glID: "1"})
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "(pre-receive hook declined)")
 }
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 114cfbe14..2629cad84 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -2,7 +2,6 @@ package praefect
 
 import (
 	"context"
-	"io/ioutil"
 	"net"
 	"os"
 	"path/filepath"
@@ -82,10 +81,9 @@ func testMain(m *testing.M) int {
 func TestReplMgr_ProcessBacklog(t *testing.T) {
 	backupStorageName := "backup"
 
-	backupDir, err := ioutil.TempDir(testhelper.GitlabTestStoragePath(), backupStorageName)
-	require.NoError(t, err)
+	backupDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
-	defer func() { os.RemoveAll(backupDir) }()
 	defer func(oldStorages []gitaly_config.Storage) { gitaly_config.Config.Storages = oldStorages }(gitaly_config.Config.Storages)
 
 	gitaly_config.Config.Storages = append(gitaly_config.Config.Storages,
@@ -537,9 +535,8 @@ func TestConfirmReplication(t *testing.T) {
 func TestProcessBacklog_FailedJobs(t *testing.T) {
 	backupStorageName := "backup"
 
-	backupDir, err := ioutil.TempDir(testhelper.GitlabTestStoragePath(), backupStorageName)
-	require.NoError(t, err)
-	defer os.RemoveAll(backupDir)
+	backupDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 	defer func(oldStorages []gitaly_config.Storage) { gitaly_config.Config.Storages = oldStorages }(gitaly_config.Config.Storages)
 
 	gitaly_config.Config.Storages = append(gitaly_config.Config.Storages, gitaly_config.Storage{
@@ -668,9 +665,8 @@ func TestProcessBacklog_FailedJobs(t *testing.T) {
 func TestProcessBacklog_Success(t *testing.T) {
 	defer func(oldStorages []gitaly_config.Storage) { gitaly_config.Config.Storages = oldStorages }(gitaly_config.Config.Storages)
 
-	backupDir, err := ioutil.TempDir("", "")
-	require.NoError(t, err)
-	defer os.RemoveAll(backupDir)
+	backupDir, cleanup := testhelper.TempDir(t)
+	defer cleanup()
 
 	gitaly_config.Config.Storages = append(gitaly_config.Config.Storages, gitaly_config.Storage{
 		Name: "backup",
@@ -740,7 +736,7 @@ func TestProcessBacklog_Success(t *testing.T) {
 		},
 	}
 
-	_, err = queueInterceptor.Enqueue(ctx, eventType1)
+	_, err := queueInterceptor.Enqueue(ctx, eventType1)
 	require.NoError(t, err)
 
 	_, err = queueInterceptor.Enqueue(ctx, eventType1)
diff --git a/internal/testhelper/testserver.go b/internal/testhelper/testserver.go
index b20415bc8..7b5f8285e 100644
--- a/internal/testhelper/testserver.go
+++ b/internal/testhelper/testserver.go
@@ -885,11 +885,8 @@ func startSocketHTTPServer(t FatalLogger, mux *http.ServeMux, tlsCfg *tls.Config
 // CreateTemporaryGitlabShellDir creates a temporary gitlab shell directory. It returns the path to the directory
 // and a cleanup function
 func CreateTemporaryGitlabShellDir(t testing.TB) (string, func()) {
-	tempDir, err := ioutil.TempDir("", "gitlab-shell")
-	require.NoError(t, err)
-	return tempDir, func() {
-		require.NoError(t, os.RemoveAll(tempDir))
-	}
+	tempDir, cleanup := TempDir(t)
+	return tempDir, cleanup
 }
 
 // WriteTemporaryGitlabShellConfigFile writes a gitlab shell config.yml in a temporary directory. It returns the path