config: Refactor locator tests

Due to dependency cycles, tests in the config module cannot make use of
any of our testing infrastructure like the testhelper or gittest. As a
result, the test for the locator is hand-crafting a pseudo storage and
then uses it to drive the test. It goes without saying that this pseudo
storage doesn't really match what we'd see in production systems, and it
is hard to reason about the current test setup.

Put the locator tests into a separate "config_test" module. We don't use
any internal symbols anyway, and like this we can refactor the test to
use the usual testing infrastructure.

1 files changed, 18 insertions, 40 deletions

diff --git a/internal/gitaly/config/locator_test.go b/internal/gitaly/config/locator_test.go
index 520fc8e1f..698b7273b 100644
--- a/internal/gitaly/config/locator_test.go
+++ b/internal/gitaly/config/locator_test.go
@@ -1,49 +1,27 @@
-package config
+package config_test
 
 import (
-	"io/ioutil"
-	"os"
-	"os/exec"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/proto"
 )
 
 func TestConfigLocator_GetObjectDirectoryPath(t *testing.T) {
-	tmpDir, err := ioutil.TempDir("", "")
-	require.NoError(t, err)
-	defer os.RemoveAll(tmpDir)
+	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	locator := config.NewLocator(cfg)
 
-	repoPath := filepath.Join(tmpDir, "relative")
-	require.NoError(t, os.MkdirAll(repoPath, 0755))
-
-	cfg := Cfg{
-		Storages: []Storage{{
-			Name: "gitaly-1",
-			Path: filepath.Dir(repoPath),
-		}},
+	repoWithGitObjDir := func(repo *gitalypb.Repository, dir string) *gitalypb.Repository {
+		repo = proto.Clone(repo).(*gitalypb.Repository)
+		repo.GitObjectDirectory = dir
+		return repo
 	}
-	require.NoError(t, cfg.SetGitPath())
-	cmd := exec.Command(cfg.Git.BinPath, "init", "--bare", "--quiet")
-	cmd.Dir = repoPath
-	require.NoError(t, cmd.Run())
-
-	locator := NewLocator(cfg)
-
-	repoWithGitObjDir := func(dir string) *gitalypb.Repository {
-		return &gitalypb.Repository{
-			StorageName:        "gitaly-1",
-			RelativePath:       filepath.Base(repoPath),
-			GlRepository:       "gl_repo",
-			GitObjectDirectory: dir,
-		}
-	}
-
-	testRepo := repoWithGitObjDir("")
 
 	testCases := []struct {
 		desc string
@@ -53,42 +31,42 @@ func TestConfigLocator_GetObjectDirectoryPath(t *testing.T) {
 	}{
 		{
 			desc: "storages configured",
-			repo: repoWithGitObjDir("objects/"),
+			repo: repoWithGitObjDir(repo, "objects/"),
 			path: filepath.Join(repoPath, "objects/"),
 		},
 		{
 			desc: "no GitObjectDirectoryPath",
-			repo: testRepo,
+			repo: repo,
 			err:  codes.InvalidArgument,
 		},
 		{
 			desc: "with directory traversal",
-			repo: repoWithGitObjDir("../bazqux.git"),
+			repo: repoWithGitObjDir(repo, "../bazqux.git"),
 			err:  codes.InvalidArgument,
 		},
 		{
 			desc: "valid path but doesn't exist",
-			repo: repoWithGitObjDir("foo../bazqux.git"),
+			repo: repoWithGitObjDir(repo, "foo../bazqux.git"),
 			err:  codes.NotFound,
 		},
 		{
 			desc: "with sneaky directory traversal",
-			repo: repoWithGitObjDir("/../bazqux.git"),
+			repo: repoWithGitObjDir(repo, "/../bazqux.git"),
 			err:  codes.InvalidArgument,
 		},
 		{
 			desc: "with traversal outside repository",
-			repo: repoWithGitObjDir("objects/../.."),
+			repo: repoWithGitObjDir(repo, "objects/../.."),
 			err:  codes.InvalidArgument,
 		},
 		{
 			desc: "with traversal outside repository with trailing separator",
-			repo: repoWithGitObjDir("objects/../../"),
+			repo: repoWithGitObjDir(repo, "objects/../../"),
 			err:  codes.InvalidArgument,
 		},
 		{
 			desc: "with deep traversal at the end",
-			repo: repoWithGitObjDir("bazqux.git/../.."),
+			repo: repoWithGitObjDir(repo, "bazqux.git/../.."),
 			err:  codes.InvalidArgument,
 		},
 	}