testcfg: Convert callers of `BuildWithRepo()`

The `BuildWithRepo()` function builds a Gitaly configuration and at the
same time creates a seed repository. There's two issues with this:

    - It is a layering violation as the configuration-related code
      should not depend on the repository-related logic. This means that
      e.g. the `gittest` package cannot depend on the `testcfg` package
      even though it is at a conceptually higher level.

    - It hides away the parameters used to create the repository. This
      means that we use seed repositories by default, but also that we
      skip creation via the Repository service. Both are bad and should
      require the caller to explicitly state that they want this.

Convert callers of `BuildWithRepo()` to instead use `Build()` to build
the configuration and `gittest.CreateRepository()` to create the
repository. Remove the now-unused `BuildWithRepo()` function.

9 files changed, 156 insertions, 40 deletions

diff --git a/cmd/gitaly-git2go/cherry_pick_test.go b/cmd/gitaly-git2go/cherry_pick_test.go
index 93308da28..de3bc89ca 100644
--- a/cmd/gitaly-git2go/cherry_pick_test.go
+++ b/cmd/gitaly-git2go/cherry_pick_test.go
@@ -17,7 +17,13 @@ import (
 )
 
 func TestCherryPick_validation(t *testing.T) {
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
 	testcfg.BuildGitalyGit2Go(t, cfg)
 	executor := buildExecutor(t, cfg)
 
@@ -68,8 +74,6 @@ func TestCherryPick_validation(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx := testhelper.Context(t)
-
 			_, err := executor.CherryPick(ctx, repo, tc.request)
 			require.EqualError(t, err, tc.expectedErr)
 		})
@@ -146,7 +150,13 @@ func TestCherryPick(t *testing.T) {
 		},
 	}
 	for _, tc := range testcases {
-		cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+		ctx := testhelper.Context(t)
+		cfg := testcfg.Build(t)
+
+		repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+			SkipCreationViaService: true,
+			Seed:                   gittest.SeedGitLabTest,
+		})
 		testcfg.BuildGitalyGit2Go(t, cfg)
 		executor := buildExecutor(t, cfg)
 
@@ -161,8 +171,6 @@ func TestCherryPick(t *testing.T) {
 		}
 
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx := testhelper.Context(t)
-
 			committer := git.Signature{
 				Name:  "Baz",
 				Email: "baz@example.com",
@@ -221,8 +229,12 @@ func TestCherryPick(t *testing.T) {
 
 func TestCherryPickStructuredErrors(t *testing.T) {
 	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
 
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
 
 	testcfg.BuildGitalyGit2Go(t, cfg)
 	executor := buildExecutor(t, cfg)
diff --git a/cmd/gitaly-git2go/conflicts_test.go b/cmd/gitaly-git2go/conflicts_test.go
index c8e9848eb..741c1121d 100644
--- a/cmd/gitaly-git2go/conflicts_test.go
+++ b/cmd/gitaly-git2go/conflicts_test.go
@@ -167,7 +167,13 @@ func TestConflicts(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+		ctx := testhelper.Context(t)
+		cfg := testcfg.Build(t)
+
+		repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+			SkipCreationViaService: true,
+			Seed:                   gittest.SeedGitLabTest,
+		})
 		executor := buildExecutor(t, cfg)
 
 		testcfg.BuildGitalyGit2Go(t, cfg)
@@ -177,8 +183,6 @@ func TestConflicts(t *testing.T) {
 		theirs := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(base), gittest.WithTreeEntries(tc.theirs...))
 
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx := testhelper.Context(t)
-
 			response, err := executor.Conflicts(ctx, repo, git2go.ConflictsCommand{
 				Repository: repoPath,
 				Ours:       ours.String(),
@@ -192,7 +196,14 @@ func TestConflicts(t *testing.T) {
 }
 
 func TestConflicts_checkError(t *testing.T) {
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	base := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries())
 	validOID := glgit.ObjectID(base.String())
 	executor := buildExecutor(t, cfg)
@@ -263,7 +274,6 @@ func TestConflicts_checkError(t *testing.T) {
 			if tc.overrideRepoPath != "" {
 				repoPath = tc.overrideRepoPath
 			}
-			ctx := testhelper.Context(t)
 
 			_, err := executor.Conflicts(ctx, repo, git2go.ConflictsCommand{
 				Repository: repoPath,
diff --git a/cmd/gitaly-git2go/merge_test.go b/cmd/gitaly-git2go/merge_test.go
index 00cc5549f..27c68224a 100644
--- a/cmd/gitaly-git2go/merge_test.go
+++ b/cmd/gitaly-git2go/merge_test.go
@@ -20,9 +20,14 @@ import (
 
 func TestMerge_missingArguments(t *testing.T) {
 	t.Parallel()
+
 	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
 
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
 	executor := buildExecutor(t, cfg)
 
 	testcases := []struct {
@@ -93,9 +98,15 @@ func TestMerge_missingArguments(t *testing.T) {
 
 func TestMerge_invalidRepositoryPath(t *testing.T) {
 	t.Parallel()
+
 	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
 
-	cfg, repo, _ := testcfg.BuildWithRepo(t)
 	testcfg.BuildGitalyGit2Go(t, cfg)
 	executor := buildExecutor(t, cfg)
 
@@ -285,7 +296,14 @@ func TestMerge_trees(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+		cfg := testcfg.Build(t)
+
+		repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg,
+			gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+				Seed:                   gittest.SeedGitLabTest,
+			})
+
 		testcfg.BuildGitalyGit2Go(t, cfg)
 		executor := buildExecutor(t, cfg)
 
@@ -352,8 +370,13 @@ func TestMerge_squash(t *testing.T) {
 	t.Parallel()
 
 	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
 
-	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
 	testcfg.BuildGitalyGit2Go(t, cfg)
 	executor := buildExecutor(t, cfg)
 
diff --git a/cmd/gitaly-git2go/rebase_test.go b/cmd/gitaly-git2go/rebase_test.go
index 3f342cd3c..e6d8d3aeb 100644
--- a/cmd/gitaly-git2go/rebase_test.go
+++ b/cmd/gitaly-git2go/rebase_test.go
@@ -20,8 +20,16 @@ import (
 var masterRevision = "1e292f8fedd741b75372e19097c76d327140c312"
 
 func TestRebase_validation(t *testing.T) {
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
 	testcfg.BuildGitalyGit2Go(t, cfg)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	committer := git2go.NewSignature("Foo", "foo@example.com", time.Now())
 	executor := buildExecutor(t, cfg)
 
@@ -72,8 +80,6 @@ func TestRebase_validation(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx := testhelper.Context(t)
-
 			_, err := executor.Rebase(ctx, repo, tc.request)
 			require.EqualError(t, err, tc.expectedErr)
 		})
@@ -171,7 +177,11 @@ func TestRebase_rebase(t *testing.T) {
 				string(gittest.TestUser.Email),
 				time.Date(2021, 3, 1, 13, 45, 50, 0, time.FixedZone("", +2*60*60)))
 
-			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+			cfg := testcfg.Build(t)
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+				Seed:                   gittest.SeedGitLabTest,
+			})
 			testcfg.BuildGitalyGit2Go(t, cfg)
 			executor := buildExecutor(t, cfg)
 
@@ -239,10 +249,15 @@ func TestRebase_rebase(t *testing.T) {
 
 func TestRebase_skipEmptyCommit(t *testing.T) {
 	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
 
-	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
 	testcfg.BuildGitalyGit2Go(t, cfg)
 
+	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	// Set up history with two diverging lines of branches, where both sides have implemented
 	// the same changes. During rebase, the diff will thus become empty.
 	base := gittest.WriteCommit(t, cfg, repoPath,
diff --git a/cmd/gitaly-git2go/revert_test.go b/cmd/gitaly-git2go/revert_test.go
index b4f3fbe8c..4a41711b8 100644
--- a/cmd/gitaly-git2go/revert_test.go
+++ b/cmd/gitaly-git2go/revert_test.go
@@ -19,7 +19,14 @@ import (
 )
 
 func TestRevert_validation(t *testing.T) {
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	testcfg.BuildGitalyGit2Go(t, cfg)
 	executor := buildExecutor(t, cfg)
 
@@ -65,8 +72,6 @@ func TestRevert_validation(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx := testhelper.Context(t)
-
 			_, err := executor.Revert(ctx, repo, tc.request)
 			require.Error(t, err)
 			require.EqualError(t, err, tc.expectedErr)
@@ -171,12 +176,18 @@ func TestRevert_trees(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.desc, func(t *testing.T) {
-			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+			ctx := testhelper.Context(t)
+			cfg := testcfg.Build(t)
+
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+				Seed:                   gittest.SeedGitLabTest,
+			})
+
 			testcfg.BuildGitalyGit2Go(t, cfg)
 			executor := buildExecutor(t, cfg)
 
 			ours, revert := tc.setupRepo(t, cfg, repoPath)
-			ctx := testhelper.Context(t)
 
 			authorDate := time.Date(2020, 7, 30, 7, 45, 50, 0, time.FixedZone("UTC+2", +2*60*60))
 
diff --git a/cmd/gitaly-git2go/submodule_test.go b/cmd/gitaly-git2go/submodule_test.go
index c05bc0af5..0edb9cfe0 100644
--- a/cmd/gitaly-git2go/submodule_test.go
+++ b/cmd/gitaly-git2go/submodule_test.go
@@ -87,13 +87,19 @@ func TestSubmodule(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+			ctx := testhelper.Context(t)
+			cfg := testcfg.Build(t)
+
+			repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+				SkipCreationViaService: true,
+				Seed:                   gittest.SeedGitLabTest,
+			})
+
 			testcfg.BuildGitalyGit2Go(t, cfg)
 			repo := localrepo.NewTestRepo(t, cfg, repoProto)
 			executor := buildExecutor(t, cfg)
 
 			tc.command.Repository = repoPath
-			ctx := testhelper.Context(t)
 
 			response, err := executor.Submodule(ctx, repo, tc.command)
 			if tc.expectedStderr != "" {
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index b76176898..fbc722533 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -102,7 +102,13 @@ func TestMain(m *testing.M) {
 func TestHooksPrePostWithSymlinkedStoragePath(t *testing.T) {
 	tempDir := testhelper.TempDir(t)
 
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
 	testcfg.BuildGitalyHooks(t, cfg)
 	testcfg.BuildGitalySSH(t, cfg)
 
@@ -115,7 +121,14 @@ func TestHooksPrePostWithSymlinkedStoragePath(t *testing.T) {
 }
 
 func TestHooksPrePostReceive(t *testing.T) {
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	testcfg.BuildGitalyHooks(t, cfg)
 	testcfg.BuildGitalySSH(t, cfg)
 	testHooksPrePostReceive(t, cfg, repo, repoPath)
@@ -327,7 +340,14 @@ func TestHooksPostReceiveFailed(t *testing.T) {
 
 	logger, _ := test.NewNullLogger()
 
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t, testcfg.WithBase(config.Cfg{Auth: auth.Config{Token: "abc123"}}))
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t, testcfg.WithBase(config.Cfg{Auth: auth.Config{Token: "abc123"}}))
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	gitalyHooksPath := testcfg.BuildGitalyHooks(t, cfg)
 	testcfg.BuildGitalySSH(t, cfg)
 
@@ -394,8 +414,6 @@ func TestHooksPostReceiveFailed(t *testing.T) {
 
 	for _, tc := range testcases {
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx := testhelper.Context(t)
-
 			hooksPayload, err := git.NewHooksPayload(
 				cfg,
 				repo,
@@ -439,7 +457,14 @@ func TestHooksNotAllowed(t *testing.T) {
 
 	logger, _ := test.NewNullLogger()
 
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t, testcfg.WithBase(config.Cfg{Auth: auth.Config{Token: "abc123"}}))
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t, testcfg.WithBase(config.Cfg{Auth: auth.Config{Token: "abc123"}}))
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	gitalyHooksPath := testcfg.BuildGitalyHooks(t, cfg)
 	testcfg.BuildGitalySSH(t, cfg)
 
@@ -467,7 +492,6 @@ func TestHooksNotAllowed(t *testing.T) {
 	runHookServiceWithGitlabClient(t, cfg, gitlabClient)
 
 	var stderr, stdout bytes.Buffer
-	ctx := testhelper.Context(t)
 
 	cmd := exec.Command(gitalyHooksPath)
 	cmd.Args = []string{"pre-receive"}
@@ -548,11 +572,17 @@ func requireContainsOnce(t *testing.T, s string, contains string) {
 func TestGitalyHooksPackObjects(t *testing.T) {
 	logDir := testhelper.TempDir(t)
 
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t, testcfg.WithBase(config.Cfg{
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t, testcfg.WithBase(config.Cfg{
 		Auth:    auth.Config{Token: "abc123"},
 		Logging: config.Logging{Config: internallog.Config{Dir: logDir}},
 	}))
 
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	logger, hook := test.NewNullLogger()
 	runHookServiceServer(t, cfg, testserver.WithLogger(logger))
 
@@ -579,8 +609,6 @@ func TestGitalyHooksPackObjects(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx := testhelper.Context(t)
-
 			hook.Reset()
 
 			tempDir := testhelper.TempDir(t)
diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go
index 5e6b9e6f8..54149bcf6 100644
--- a/cmd/gitaly-ssh/auth_test.go
+++ b/cmd/gitaly-ssh/auth_test.go
@@ -22,11 +22,17 @@ import (
 )
 
 func TestConnectivity(t *testing.T) {
-	cfg, repo, _ := testcfg.BuildWithRepo(t)
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
 
 	testcfg.BuildGitalySSH(t, cfg)
 	testcfg.BuildGitalyHooks(t, cfg)
 
+	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	cwd, err := os.Getwd()
 	require.NoError(t, err)
 
diff --git a/cmd/gitaly-ssh/upload_pack_test.go b/cmd/gitaly-ssh/upload_pack_test.go
index efcf881d5..4da7f3ec9 100644
--- a/cmd/gitaly-ssh/upload_pack_test.go
+++ b/cmd/gitaly-ssh/upload_pack_test.go
@@ -27,11 +27,16 @@ func TestVisibilityOfHiddenRefs(t *testing.T) {
 	t.Parallel()
 
 	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
 
-	cfg, repo, repoPath := testcfg.BuildWithRepo(t)
 	testcfg.BuildGitalySSH(t, cfg)
 	testcfg.BuildGitalyHooks(t, cfg)
 
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+		SkipCreationViaService: true,
+		Seed:                   gittest.SeedGitLabTest,
+	})
+
 	address := testserver.RunGitalyServer(t, cfg, nil, setup.RegisterAll, testserver.WithDisablePraefect())
 
 	// Create a keep-around ref