Merge branch 'pks-gittest-bin-path' into 'master'

gittest: Accept Gitaly configuration instead of binary paths

See merge request gitlab-org/gitaly!4203

13 files changed, 202 insertions, 212 deletions

diff --git a/internal/git/gittest/commit.go b/internal/git/gittest/commit.go
index 1e49149d6..50ab3aa2c 100644
--- a/internal/git/gittest/commit.go
+++ b/internal/git/gittest/commit.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"testing"
 
@@ -21,11 +20,12 @@ const (
 )
 
 type writeCommitConfig struct {
-	branch        string
-	parents       []git.ObjectID
-	committerName string
-	message       string
-	treeEntries   []TreeEntry
+	branch             string
+	parents            []git.ObjectID
+	committerName      string
+	message            string
+	treeEntries        []TreeEntry
+	alternateObjectDir string
 }
 
 // WriteCommitOption is an option which can be passed to WriteCommit.
@@ -74,6 +74,15 @@ func WithCommitterName(name string) WriteCommitOption {
 	}
 }
 
+// WithAlternateObjectDirectory will cause the commit to be written into the given alternate object
+// directory. This can either be an absolute path or a relative path. In the latter case the path
+// is considered to be relative to the repository path.
+func WithAlternateObjectDirectory(alternateObjectDir string) WriteCommitOption {
+	return func(cfg *writeCommitConfig) {
+		cfg.alternateObjectDir = alternateObjectDir
+	}
+}
+
 // WriteCommit writes a new commit into the target repository.
 func WriteCommit(t testing.TB, cfg config.Cfg, repoPath string, opts ...WriteCommitOption) git.ObjectID {
 	t.Helper()
@@ -119,50 +128,38 @@ func WriteCommit(t testing.TB, cfg config.Cfg, repoPath string, opts ...WriteCom
 		"commit-tree", "-F", "-", tree,
 	}
 
+	var env []string
+	if writeCommitConfig.alternateObjectDir != "" {
+		require.True(t, filepath.IsAbs(writeCommitConfig.alternateObjectDir),
+			"alternate object directory must be an absolute path")
+		require.NoError(t, os.MkdirAll(writeCommitConfig.alternateObjectDir, 0o755))
+
+		env = append(env,
+			fmt.Sprintf("GIT_OBJECT_DIRECTORY=%s", writeCommitConfig.alternateObjectDir),
+			fmt.Sprintf("GIT_ALTERNATE_OBJECT_DIRECTORIES=%s", filepath.Join(repoPath, "objects")),
+		)
+	}
+
 	for _, parent := range parents {
 		commitArgs = append(commitArgs, "-p", parent.String())
 	}
 
-	stdout := ExecOpts(t, cfg, ExecConfig{Stdin: stdin}, commitArgs...)
+	stdout := ExecOpts(t, cfg, ExecConfig{
+		Stdin: stdin,
+		Env:   env,
+	}, commitArgs...)
 	oid, err := git.NewObjectIDFromHex(text.ChompBytes(stdout))
 	require.NoError(t, err)
 
 	if writeCommitConfig.branch != "" {
-		Exec(t, cfg, "-C", repoPath, "update-ref", "refs/heads/"+writeCommitConfig.branch, oid.String())
+		ExecOpts(t, cfg, ExecConfig{
+			Env: env,
+		}, "-C", repoPath, "update-ref", "refs/heads/"+writeCommitConfig.branch, oid.String())
 	}
 
 	return oid
 }
 
-// CreateCommitInAlternateObjectDirectory runs a command such that its created
-// objects will live in an alternate objects directory. It returns the current
-// head after the command is run and the alternate objects directory path
-func CreateCommitInAlternateObjectDirectory(t testing.TB, gitBin, repoPath, altObjectsDir string, cmd *exec.Cmd) (currentHead []byte) {
-	gitPath := filepath.Join(repoPath, ".git")
-
-	altObjectsPath := filepath.Join(gitPath, altObjectsDir)
-	gitObjectEnv := []string{
-		fmt.Sprintf("GIT_OBJECT_DIRECTORY=%s", altObjectsPath),
-		fmt.Sprintf("GIT_ALTERNATE_OBJECT_DIRECTORIES=%s", filepath.Join(gitPath, "objects")),
-	}
-	require.NoError(t, os.MkdirAll(altObjectsPath, 0o755))
-
-	// Because we set 'gitObjectEnv', the new objects created by this command
-	// will go into 'find-commits-alt-test-repo/.git/alt-objects'.
-	cmd.Env = append(cmd.Env, gitObjectEnv...)
-	if output, err := cmd.Output(); err != nil {
-		stderr := err.(*exec.ExitError).Stderr
-		t.Fatalf("stdout: %s, stderr: %s", output, stderr)
-	}
-
-	cmd = exec.Command(gitBin, "-C", repoPath, "rev-parse", "HEAD")
-	cmd.Env = gitObjectEnv
-	currentHead, err := cmd.Output()
-	require.NoError(t, err)
-
-	return currentHead[:len(currentHead)-1]
-}
-
 func authorEqualIgnoringDate(t testing.TB, expected *gitalypb.CommitAuthor, actual *gitalypb.CommitAuthor) {
 	t.Helper()
 	require.Equal(t, expected.GetName(), actual.GetName(), "author name does not match")
diff --git a/internal/git/gittest/hooks.go b/internal/git/gittest/hooks.go
index 6f0504c96..1c7914de4 100644
--- a/internal/git/gittest/hooks.go
+++ b/internal/git/gittest/hooks.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/hooks"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 )
 
@@ -27,14 +28,14 @@ func WriteEnvToCustomHook(t testing.TB, repoPath, hookName string) string {
 // if it can find the object in the quarantine directory. if
 // GIT_OBJECT_DIRECTORY and GIT_ALTERNATE_OBJECT_DIRECTORIES were not passed
 // through correctly to the hooks, it will fail
-func WriteCheckNewObjectExistsHook(t testing.TB, gitBin, repoPath string) {
+func WriteCheckNewObjectExistsHook(t testing.TB, cfg config.Cfg, repoPath string) {
 	hook := fmt.Sprintf(`#!/usr/bin/env ruby
 STDIN.each_line do |line|
   new_object = line.split(' ')[1]
   exit 1 unless new_object
   exit 1 unless	system(*%%W[%s cat-file -e #{new_object}])
 end
-`, gitBin)
+`, cfg.Git.BinPath)
 
 	WriteCustomHook(t, repoPath, "pre-receive", []byte(hook))
 }
diff --git a/internal/git/gittest/objects.go b/internal/git/gittest/objects.go
index ea9b9f1e4..c879adb51 100644
--- a/internal/git/gittest/objects.go
+++ b/internal/git/gittest/objects.go
@@ -15,18 +15,20 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 )
 
-// GitObjectMustExist is a test assertion that fails unless the git repo in repoPath contains sha
-func GitObjectMustExist(t testing.TB, gitBin, repoPath, sha string) {
-	gitObjectExists(t, gitBin, repoPath, sha, true)
+// RequireObjectExists asserts that the given repository does contain an object with the specified
+// object ID.
+func RequireObjectExists(t testing.TB, cfg config.Cfg, repoPath string, objectID git.ObjectID) {
+	requireObjectExists(t, cfg, repoPath, objectID, true)
 }
 
-// GitObjectMustNotExist is a test assertion that fails unless the git repo in repoPath contains sha
-func GitObjectMustNotExist(t testing.TB, gitBin, repoPath, sha string) {
-	gitObjectExists(t, gitBin, repoPath, sha, false)
+// RequireObjectNotExists asserts that the given repository does not contain an object with the
+// specified object ID.
+func RequireObjectNotExists(t testing.TB, cfg config.Cfg, repoPath string, objectID git.ObjectID) {
+	requireObjectExists(t, cfg, repoPath, objectID, false)
 }
 
-func gitObjectExists(t testing.TB, gitBin, repoPath, sha string, exists bool) {
-	cmd := exec.Command(gitBin, "-C", repoPath, "cat-file", "-e", sha)
+func requireObjectExists(t testing.TB, cfg config.Cfg, repoPath string, objectID git.ObjectID, exists bool) {
+	cmd := exec.Command(cfg.Git.BinPath, "-C", repoPath, "cat-file", "-e", objectID.String())
 	cmd.Env = []string{
 		"GIT_ALLOW_PROTOCOL=", // To prevent partial clone reaching remote repo over SSH
 	}
diff --git a/internal/gitaly/service/commit/find_commits_test.go b/internal/gitaly/service/commit/find_commits_test.go
index f995aa31a..144039415 100644
--- a/internal/gitaly/service/commit/find_commits_test.go
+++ b/internal/gitaly/service/commit/find_commits_test.go
@@ -4,7 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"os/exec"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -436,17 +436,12 @@ func TestSuccessfulFindCommitsRequest(t *testing.T) {
 
 func TestSuccessfulFindCommitsRequestWithAltGitObjectDirs(t *testing.T) {
 	t.Parallel()
-	cfg, repo, repoPath, client := setupCommitServiceWithRepo(t, false)
-
-	committerName := "Scrooge McDuck"
-	committerEmail := "scrooge@mcduck.com"
+	cfg, repo, repoPath, client := setupCommitServiceWithRepo(t, true)
 
-	cmd := exec.Command(cfg.Git.BinPath, "-C", repoPath,
-		"-c", fmt.Sprintf("user.name=%s", committerName),
-		"-c", fmt.Sprintf("user.email=%s", committerEmail),
-		"commit", "--allow-empty", "-m", "An empty commit")
 	altObjectsDir := "./alt-objects"
-	currentHead := gittest.CreateCommitInAlternateObjectDirectory(t, cfg.Git.BinPath, repoPath, altObjectsDir, cmd)
+	commitID := gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithAlternateObjectDirectory(filepath.Join(repoPath, altObjectsDir)),
+	)
 
 	testCases := []struct {
 		desc          string
@@ -470,7 +465,7 @@ func TestSuccessfulFindCommitsRequestWithAltGitObjectDirs(t *testing.T) {
 			repo.GitAlternateObjectDirectories = testCase.altDirs
 			request := &gitalypb.FindCommitsRequest{
 				Repository: repo,
-				Revision:   currentHead,
+				Revision:   []byte(commitID.String()),
 				Limit:      1,
 			}
 
diff --git a/internal/gitaly/service/commit/isancestor_test.go b/internal/gitaly/service/commit/isancestor_test.go
index afa3ab778..eafa73aa5 100644
--- a/internal/gitaly/service/commit/isancestor_test.go
+++ b/internal/gitaly/service/commit/isancestor_test.go
@@ -2,12 +2,14 @@ package commit
 
 import (
 	"fmt"
-	"os/exec"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -162,19 +164,15 @@ func TestCommitIsAncestorSuccess(t *testing.T) {
 
 func TestSuccessfulIsAncestorRequestWithAltGitObjectDirs(t *testing.T) {
 	t.Parallel()
-	cfg, repo, repoPath, client := setupCommitServiceWithRepo(t, false)
+	cfg, repo, repoPath, client := setupCommitServiceWithRepo(t, true)
 
-	committerName := "Scrooge McDuck"
-	committerEmail := "scrooge@mcduck.com"
+	parentCommitID := git.ObjectID(text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "--verify", "HEAD")))
 
-	previousHead := gittest.Exec(t, cfg, "-C", repoPath, "show", "--format=format:%H", "--no-patch", "HEAD")
-
-	cmd := exec.Command(cfg.Git.BinPath, "-C", repoPath,
-		"-c", fmt.Sprintf("user.name=%s", committerName),
-		"-c", fmt.Sprintf("user.email=%s", committerEmail),
-		"commit", "--allow-empty", "-m", "An empty commit")
 	altObjectsDir := "./alt-objects"
-	currentHead := gittest.CreateCommitInAlternateObjectDirectory(t, cfg.Git.BinPath, repoPath, altObjectsDir, cmd)
+	commitID := gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithParents(parentCommitID),
+		gittest.WithAlternateObjectDirectory(filepath.Join(repoPath, altObjectsDir)),
+	)
 
 	testCases := []struct {
 		desc    string
@@ -198,8 +196,8 @@ func TestSuccessfulIsAncestorRequestWithAltGitObjectDirs(t *testing.T) {
 			repo.GitAlternateObjectDirectories = testCase.altDirs
 			request := &gitalypb.CommitIsAncestorRequest{
 				Repository: repo,
-				AncestorId: string(previousHead),
-				ChildId:    string(currentHead),
+				AncestorId: string(parentCommitID),
+				ChildId:    commitID.String(),
 			}
 
 			ctx, cancel := testhelper.Context()
diff --git a/internal/gitaly/service/commit/list_all_commits_test.go b/internal/gitaly/service/commit/list_all_commits_test.go
index dbbbb53da..2f4eb401f 100644
--- a/internal/gitaly/service/commit/list_all_commits_test.go
+++ b/internal/gitaly/service/commit/list_all_commits_test.go
@@ -2,16 +2,13 @@ package commit
 
 import (
 	"errors"
-	"fmt"
 	"io"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/protobuf/types/known/timestamppb"
@@ -116,23 +113,12 @@ func TestListAllCommits(t *testing.T) {
 		require.NoError(t, err)
 		require.Empty(t, receiveCommits(t, stream))
 
-		treeID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "HEAD^{tree}"))
-
 		// We cannot easily spawn a command with an object directory, so we just do so
 		// manually here and write the commit into the quarantine object directory.
-		commitTree := exec.Command(cfg.Git.BinPath, "-C", repoPath,
-			"-c", "user.name=John Doe",
-			"-c", "user.email=john.doe@example.com",
-			"commit-tree", treeID, "-m", "An empty commit")
-		commitTree.Env = []string{
-			"GIT_AUTHOR_DATE=1600000000 +0200",
-			"GIT_COMMITTER_DATE=1600000001 +0200",
-			fmt.Sprintf("GIT_OBJECT_DIRECTORY=%s", quarantineDir),
-			fmt.Sprintf("GIT_ALTERNATE_OBJECT_DIRECTORIES=%s", filepath.Join(repoPath, "objects")),
-		}
-
-		commitID, err := commitTree.Output()
-		require.NoError(t, err)
+		commitID := gittest.WriteCommit(t, cfg, repoPath,
+			gittest.WithAlternateObjectDirectory(filepath.Join(repoPath, quarantineDir)),
+			gittest.WithParents(),
+		)
 
 		// We now expect only the quarantined commit to be returned.
 		stream, err = client.ListAllCommits(ctx, &gitalypb.ListAllCommitsRequest{
@@ -141,22 +127,22 @@ func TestListAllCommits(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, []*gitalypb.GitCommit{{
-			Id:       text.ChompBytes(commitID),
-			Subject:  []byte("An empty commit"),
-			Body:     []byte("An empty commit\n"),
-			BodySize: 16,
-			TreeId:   treeID,
+			Id:       commitID.String(),
+			Subject:  []byte("message"),
+			Body:     []byte("message"),
+			BodySize: 7,
+			TreeId:   "4b825dc642cb6eb9a060e54bf8d69288fbee4904",
 			Author: &gitalypb.CommitAuthor{
-				Name:     []byte("John Doe"),
-				Email:    []byte("john.doe@example.com"),
-				Date:     &timestamppb.Timestamp{Seconds: 1600000000},
-				Timezone: []byte("+0200"),
+				Name:     []byte("Scrooge McDuck"),
+				Email:    []byte("scrooge@mcduck.com"),
+				Date:     &timestamppb.Timestamp{Seconds: 1572776879},
+				Timezone: []byte("+0100"),
 			},
 			Committer: &gitalypb.CommitAuthor{
-				Name:     []byte("John Doe"),
-				Email:    []byte("john.doe@example.com"),
-				Date:     &timestamppb.Timestamp{Seconds: 1600000001},
-				Timezone: []byte("+0200"),
+				Name:     []byte("Scrooge McDuck"),
+				Email:    []byte("scrooge@mcduck.com"),
+				Date:     &timestamppb.Timestamp{Seconds: 1572776879},
+				Timezone: []byte("+0100"),
 			},
 		}}, receiveCommits(t, stream))
 	})
diff --git a/internal/gitaly/service/repository/rename_test.go b/internal/gitaly/service/repository/rename_test.go
index 17c20606f..768f766dd 100644
--- a/internal/gitaly/service/repository/rename_test.go
+++ b/internal/gitaly/service/repository/rename_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
@@ -38,7 +39,7 @@ func TestRenameRepository_success(t *testing.T) {
 	require.True(t, storage.IsGitDirectory(newDirectory), "moved Git repository has been corrupted")
 
 	// ensure the git directory that got renamed contains a sha in the seed repo
-	gittest.GitObjectMustExist(t, cfg.Git.BinPath, newDirectory, "913c66a37b4a45b9769037c55c2d238bd0942d2e")
+	gittest.RequireObjectExists(t, cfg, newDirectory, git.ObjectID("913c66a37b4a45b9769037c55c2d238bd0942d2e"))
 }
 
 func TestRenameRepository_DestinationExists(t *testing.T) {
@@ -69,7 +70,7 @@ func TestRenameRepository_DestinationExists(t *testing.T) {
 	testhelper.RequireGrpcCode(t, err, codes.AlreadyExists)
 
 	// ensure the git directory that already existed didn't get overwritten
-	gittest.GitObjectMustExist(t, cfg.Git.BinPath, destinationRepoPath, commitID.String())
+	gittest.RequireObjectExists(t, cfg, destinationRepoPath, commitID)
 }
 
 func TestRenameRepository_invalidRequest(t *testing.T) {
diff --git a/internal/gitaly/service/repository/repack_test.go b/internal/gitaly/service/repository/repack_test.go
index 643e5bea6..9757f9459 100644
--- a/internal/gitaly/service/repository/repack_test.go
+++ b/internal/gitaly/service/repository/repack_test.go
@@ -1,7 +1,6 @@
 package repository
 
 import (
-	"os/exec"
 	"path/filepath"
 	"testing"
 	"time"
@@ -60,18 +59,23 @@ func TestRepackIncrementalCollectLogStatistics(t *testing.T) {
 
 func TestRepackLocal(t *testing.T) {
 	t.Parallel()
-	cfg, repo, repoPath, client := setupRepositoryServiceWithWorktree(t)
 
-	commiterArgs := []string{"-c", "user.name=Scrooge McDuck", "-c", "user.email=scrooge@mcduck.com"}
-	cmdArgs := append(commiterArgs, "-C", repoPath, "commit", "--allow-empty", "-m", "An empty commit")
-	cmd := exec.Command(cfg.Git.BinPath, cmdArgs...)
+	cfg, repo, repoPath, client := setupRepositoryService(t)
+
 	altObjectsDir := "./alt-objects"
-	altDirsCommit := gittest.CreateCommitInAlternateObjectDirectory(t, cfg.Git.BinPath, repoPath, altObjectsDir, cmd)
+	alternateCommit := gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithMessage("alternate commit"),
+		gittest.WithAlternateObjectDirectory(filepath.Join(repoPath, altObjectsDir)),
+		gittest.WithBranch("alternate-odb"),
+	)
 
-	repoCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(t.Name()))
+	repoCommit := gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithMessage("main commit"),
+		gittest.WithBranch("main-odb"),
+	)
 
-	ctx, cancelFn := testhelper.Context()
-	defer cancelFn()
+	ctx, cancel := testhelper.Context()
+	defer cancel()
 
 	// Set GIT_ALTERNATE_OBJECT_DIRECTORIES on the outgoing request. The
 	// intended use case of the behavior we're testing here is that
@@ -82,13 +86,13 @@ func TestRepackLocal(t *testing.T) {
 	_, err := client.RepackFull(ctx, &gitalypb.RepackFullRequest{Repository: repo})
 	require.NoError(t, err)
 
-	packFiles, err := filepath.Glob(filepath.Join(repoPath, ".git", "objects", "pack", "pack-*.pack"))
+	packFiles, err := filepath.Glob(filepath.Join(repoPath, "objects", "pack", "pack-*.pack"))
 	require.NoError(t, err)
 	require.Len(t, packFiles, 1)
 
 	packContents := gittest.Exec(t, cfg, "-C", repoPath, "verify-pack", "-v", packFiles[0])
-	require.NotContains(t, string(packContents), string(altDirsCommit))
-	require.Contains(t, string(packContents), repoCommit)
+	require.NotContains(t, string(packContents), alternateCommit.String())
+	require.Contains(t, string(packContents), repoCommit.String())
 }
 
 func TestRepackIncrementalFailure(t *testing.T) {
diff --git a/internal/gitaly/service/repository/snapshot_test.go b/internal/gitaly/service/repository/snapshot_test.go
index bf276a189..321909607 100644
--- a/internal/gitaly/service/repository/snapshot_test.go
+++ b/internal/gitaly/service/repository/snapshot_test.go
@@ -7,17 +7,12 @@ import (
 	"io"
 	"net/http/httptest"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/archive"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
@@ -95,7 +90,7 @@ func TestGetSnapshotWithDedupe(t *testing.T) {
 func testGetSnapshotWithDedupe(t *testing.T, ctx context.Context) {
 	for _, tc := range []struct {
 		desc              string
-		alternatePathFunc func(t *testing.T, storageDir, objDir string) string
+		alternatePathFunc func(t *testing.T, storageDir, repoPath string) string
 	}{
 		{
 			desc:              "subdirectory",
@@ -119,46 +114,40 @@ func testGetSnapshotWithDedupe(t *testing.T, ctx context.Context) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			cfg, repoProto, repoPath, client := setupRepositoryServiceWithWorktree(t)
-			repo := localrepo.NewTestRepo(t, cfg, repoProto)
+			cfg, repoProto, repoPath, client := setupRepositoryService(t)
 
-			const committerName = "Scrooge McDuck"
-			const committerEmail = "scrooge@mcduck.com"
-
-			cmd := exec.Command(cfg.Git.BinPath, "-C", repoPath,
-				"-c", fmt.Sprintf("user.name=%s", committerName),
-				"-c", fmt.Sprintf("user.email=%s", committerEmail),
-				"commit", "--allow-empty", "-m", "An empty commit")
 			alternateObjDir := tc.alternatePathFunc(t, cfg.Storages[0].Path, filepath.Join(repoPath, "objects"))
-			commitSha := gittest.CreateCommitInAlternateObjectDirectory(t, cfg.Git.BinPath, repoPath, alternateObjDir, cmd)
-			originalAlternatesCommit := string(commitSha)
+			absoluteAlternateObjDir := alternateObjDir
+			if !filepath.IsAbs(alternateObjDir) {
+				absoluteAlternateObjDir = filepath.Join(repoPath, "objects", alternateObjDir)
+			}
+
+			firstCommitID := gittest.WriteCommit(t, cfg, repoPath,
+				gittest.WithMessage("An empty commit"),
+				gittest.WithAlternateObjectDirectory(absoluteAlternateObjDir),
+			)
 
 			locator := config.NewLocator(cfg)
-			catfileCache := catfile.NewCache(cfg)
-			defer catfileCache.Stop()
 
-			// ensure commit cannot be found in current repository
-			objectInfoReader, err := catfileCache.ObjectInfoReader(ctx, repo)
-			require.NoError(t, err)
-			_, err = objectInfoReader.Info(ctx, git.Revision(originalAlternatesCommit))
-			require.True(t, catfile.IsNotFound(err))
+			// We haven't yet written the alternates file, and thus we shouldn't be able
+			// to find this commit yet.
+			gittest.RequireObjectNotExists(t, cfg, repoPath, firstCommitID)
 
-			// write alternates file to point to alt objects folder
+			// Write alternates file to point to alt objects folder.
 			alternatesPath, err := locator.InfoAlternatesPath(repoProto)
 			require.NoError(t, err)
-			require.NoError(t, os.WriteFile(alternatesPath, []byte(filepath.Join(repoPath, ".git", fmt.Sprintf("%s\n", alternateObjDir))), 0o644))
+			require.NoError(t, os.WriteFile(alternatesPath, []byte(fmt.Sprintf("%s\n", alternateObjDir)), 0o644))
 
-			// write another commit and ensure we can find it
-			cmd = exec.Command(cfg.Git.BinPath, "-C", repoPath,
-				"-c", fmt.Sprintf("user.name=%s", committerName),
-				"-c", fmt.Sprintf("user.email=%s", committerEmail),
-				"commit", "--allow-empty", "-m", "Another empty commit")
-			commitSha = gittest.CreateCommitInAlternateObjectDirectory(t, cfg.Git.BinPath, repoPath, alternateObjDir, cmd)
+			// Write another commit into the alternate object directory.
+			secondCommitID := gittest.WriteCommit(t, cfg, repoPath,
+				gittest.WithMessage("Another empty commit"),
+				gittest.WithAlternateObjectDirectory(absoluteAlternateObjDir),
+			)
 
-			objectInfoReader, err = catfileCache.ObjectInfoReader(ctx, repo)
-			require.NoError(t, err)
-			_, err = objectInfoReader.Info(ctx, git.Revision(commitSha))
-			require.NoError(t, err)
+			// We should now be able to find both commits given that the alternates file
+			// points to the object directory we've created them in.
+			gittest.RequireObjectExists(t, cfg, repoPath, firstCommitID)
+			gittest.RequireObjectExists(t, cfg, repoPath, secondCommitID)
 
 			repoCopy, _ := copyRepoUsingSnapshot(t, ctx, cfg, client, repoProto)
 			repoCopy.RelativePath = getReplicaPath(ctx, t, client, repoCopy)
@@ -166,75 +155,92 @@ func testGetSnapshotWithDedupe(t *testing.T, ctx context.Context) {
 			require.NoError(t, err)
 
 			// ensure the sha committed to the alternates directory can be accessed
-			gittest.Exec(t, cfg, "-C", repoCopyPath, "cat-file", "-p", originalAlternatesCommit)
+			gittest.Exec(t, cfg, "-C", repoCopyPath, "cat-file", "-p", firstCommitID.String())
 			gittest.Exec(t, cfg, "-C", repoCopyPath, "fsck")
 		})
 	}
 }
 
-func TestGetSnapshotWithDedupeSoftFailures(t *testing.T) {
+func TestGetSnapshot_alternateObjectDirectory(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.TxAtomicRepositoryCreation).Run(t, testGetSnapshotWithDedupeSoftFailures)
+	testhelper.NewFeatureSets(featureflag.TxAtomicRepositoryCreation).Run(t, testGetSnapshotAlternateObjectDirectory)
 }
 
-func testGetSnapshotWithDedupeSoftFailures(t *testing.T, ctx context.Context) {
+func testGetSnapshotAlternateObjectDirectory(t *testing.T, ctx context.Context) {
 	cfg, client := setupRepositoryServiceWithoutRepo(t)
 
-	testRepo, repoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0], gittest.CloneRepoOpts{
-		WithWorktree: true,
-	})
+	repo, repoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
 
 	locator := config.NewLocator(cfg)
-
-	// write alternates file to point to alternates objects folder that doesn't exist
-	alternateObjDir := "./alt-objects"
-	alternateObjPath := filepath.Join(repoPath, ".git", alternateObjDir)
-	alternatesPath, err := locator.InfoAlternatesPath(testRepo)
+	alternatesFile, err := locator.InfoAlternatesPath(repo)
 	require.NoError(t, err)
-	require.NoError(t, os.WriteFile(alternatesPath, []byte(fmt.Sprintf("%s\n", alternateObjPath)), 0o644))
 
-	req := &gitalypb.GetSnapshotRequest{Repository: testRepo}
-	_, err = getSnapshot(client, req)
-	assert.NoError(t, err)
-	require.NoError(t, os.Remove(alternatesPath))
+	req := &gitalypb.GetSnapshotRequest{Repository: repo}
+
+	t.Run("nonexistent", func(t *testing.T) {
+		alternateObjectDir := filepath.Join(repoPath, "does-not-exist")
 
-	// write alternates file to point outside storage root
-	storageRoot, err := locator.GetStorageByName(testRepo.GetStorageName())
-	require.NoError(t, err)
-	require.NoError(t, os.WriteFile(alternatesPath, []byte(filepath.Join(storageRoot, "..")), 0o600))
+		require.NoError(t, os.WriteFile(alternatesFile, []byte(fmt.Sprintf("%s\n", alternateObjectDir)), 0o644))
+		defer func() {
+			require.NoError(t, os.Remove(alternatesFile))
+		}()
+
+		_, err = getSnapshot(client, req)
+		require.NoError(t, err)
+	})
 
-	_, err = getSnapshot(client, &gitalypb.GetSnapshotRequest{Repository: testRepo})
-	assert.NoError(t, err)
-	require.NoError(t, os.Remove(alternatesPath))
+	t.Run("escape storage root", func(t *testing.T) {
+		storageRoot, err := locator.GetStorageByName(repo.GetStorageName())
+		require.NoError(t, err)
 
-	// write alternates file with bad permissions
-	require.NoError(t, os.WriteFile(alternatesPath, []byte(fmt.Sprintf("%s\n", alternateObjPath)), 0o000))
-	_, err = getSnapshot(client, req)
-	assert.NoError(t, err)
-	require.NoError(t, os.Remove(alternatesPath))
+		alternateObjectDir := filepath.Join(storageRoot, "..")
 
-	// write alternates file without newline
-	committerName := "Scrooge McDuck"
-	committerEmail := "scrooge@mcduck.com"
+		require.NoError(t, os.WriteFile(alternatesFile, []byte(alternateObjectDir), 0o600))
+		defer func() {
+			require.NoError(t, os.Remove(alternatesFile))
+		}()
 
-	cmd := exec.Command(cfg.Git.BinPath, "-C", repoPath,
-		"-c", fmt.Sprintf("user.name=%s", committerName),
-		"-c", fmt.Sprintf("user.email=%s", committerEmail),
-		"commit", "--allow-empty", "-m", "An empty commit")
+		_, err = getSnapshot(client, &gitalypb.GetSnapshotRequest{Repository: repo})
+		require.NoError(t, err)
+	})
 
-	commitSha := gittest.CreateCommitInAlternateObjectDirectory(t, cfg.Git.BinPath, repoPath, alternateObjDir, cmd)
-	originalAlternatesCommit := string(commitSha)
+	t.Run("bad permissions", func(t *testing.T) {
+		alternateObjectDir := filepath.Join(repoPath, "bad-permissions")
 
-	require.NoError(t, os.WriteFile(alternatesPath, []byte(alternateObjPath), 0o644))
+		require.NoError(t, os.WriteFile(alternatesFile, []byte(fmt.Sprintf("%s\n", alternateObjectDir)), 0o000))
+		defer func() {
+			require.NoError(t, os.Remove(alternatesFile))
+		}()
 
-	repoCopy, _ := copyRepoUsingSnapshot(t, ctx, cfg, client, testRepo)
-	repoCopy.RelativePath = getReplicaPath(ctx, t, client, repoCopy)
-	repoCopyPath, err := locator.GetRepoPath(repoCopy)
-	require.NoError(t, err)
+		_, err = getSnapshot(client, req)
+		require.NoError(t, err)
+	})
 
-	// ensure the sha committed to the alternates directory can be accessed
-	gittest.Exec(t, cfg, "-C", repoCopyPath, "cat-file", "-p", originalAlternatesCommit)
-	gittest.Exec(t, cfg, "-C", repoCopyPath, "fsck")
+	t.Run("valid alternate object directory", func(t *testing.T) {
+		alternateObjectDir := filepath.Join(repoPath, "valid-odb")
+
+		commitID := gittest.WriteCommit(t, cfg, repoPath,
+			gittest.WithAlternateObjectDirectory(alternateObjectDir),
+			// Create a branch with the commit such that the snapshot would indeed treat
+			// this commit as referenced.
+			gittest.WithBranch("some-branch"),
+		)
+
+		require.NoError(t, os.WriteFile(alternatesFile, []byte(alternateObjectDir), 0o644))
+		defer func() {
+			require.NoError(t, os.Remove(alternatesFile))
+		}()
+
+		repoCopy, _ := copyRepoUsingSnapshot(t, ctx, cfg, client, repo)
+		repoCopy.RelativePath = getReplicaPath(ctx, t, client, repoCopy)
+		repoCopyPath, err := locator.GetRepoPath(repoCopy)
+		require.NoError(t, err)
+
+		// Ensure the object committed to the alternates directory can be accessed and that
+		// the repository is consistent.
+		gittest.Exec(t, cfg, "-C", repoCopyPath, "cat-file", "-p", commitID.String())
+		gittest.Exec(t, cfg, "-C", repoCopyPath, "fsck")
+	})
 }
 
 // copyRepoUsingSnapshot creates a tarball snapshot, then creates a new repository from that snapshot
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index d4ec53ffc..928740899 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -576,7 +576,7 @@ func TestPostReceivePackToHooks(t *testing.T) {
 	})
 	defer cleanup()
 
-	gittest.WriteCheckNewObjectExistsHook(t, cfg.Git.BinPath, testRepoPath)
+	gittest.WriteCheckNewObjectExistsHook(t, cfg, testRepoPath)
 
 	ctx, cancel := testhelper.Context()
 	defer cancel()
diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go
index 4f141ebea..7c165dbf1 100644
--- a/internal/gitaly/service/smarthttp/upload_pack_test.go
+++ b/internal/gitaly/service/smarthttp/upload_pack_test.go
@@ -92,7 +92,7 @@ func testServerPostUpload(t *testing.T, ctx context.Context, makeRequest request
 		"-C", localRepoPath, "unpack-objects", fmt.Sprintf("--pack_header=%d,%d", version, entries),
 	)
 
-	gittest.GitObjectMustExist(t, cfg.Git.BinPath, localRepoPath, newCommit.String())
+	gittest.RequireObjectExists(t, cfg, localRepoPath, newCommit)
 
 	metric, err := negotiationMetrics.GetMetricWithLabelValues("have")
 	require.NoError(t, err)
@@ -385,14 +385,14 @@ func testServerPostUploadPackPartialClone(t *testing.T, ctx context.Context, mak
 	)
 
 	// a4a132b1b0d6720ca9254440a7ba8a6b9bbd69ec is README.md, which is a small file
-	blobLessThanLimit := "a4a132b1b0d6720ca9254440a7ba8a6b9bbd69ec"
+	blobLessThanLimit := git.ObjectID("a4a132b1b0d6720ca9254440a7ba8a6b9bbd69ec")
 
 	// c1788657b95998a2f177a4f86d68a60f2a80117f is CONTRIBUTING.md, which is > 200 bytese
-	blobGreaterThanLimit := "c1788657b95998a2f177a4f86d68a60f2a80117f"
+	blobGreaterThanLimit := git.ObjectID("c1788657b95998a2f177a4f86d68a60f2a80117f")
 
-	gittest.GitObjectMustExist(t, cfg.Git.BinPath, localRepoPath, blobLessThanLimit)
-	gittest.GitObjectMustExist(t, cfg.Git.BinPath, repoPath, blobGreaterThanLimit)
-	gittest.GitObjectMustNotExist(t, cfg.Git.BinPath, localRepoPath, blobGreaterThanLimit)
+	gittest.RequireObjectExists(t, cfg, localRepoPath, blobLessThanLimit)
+	gittest.RequireObjectExists(t, cfg, repoPath, blobGreaterThanLimit)
+	gittest.RequireObjectNotExists(t, cfg, localRepoPath, blobGreaterThanLimit)
 
 	metric, err := negotiationMetrics.GetMetricWithLabelValues("filter")
 	require.NoError(t, err)
@@ -438,7 +438,7 @@ func testServerPostUploadPackAllowAnySHA1InWant(t *testing.T, ctx context.Contex
 		"-C", localRepoPath, "unpack-objects", fmt.Sprintf("--pack_header=%d,%d", version, entries),
 	)
 
-	gittest.GitObjectMustExist(t, cfg.Git.BinPath, localRepoPath, newCommit.String())
+	gittest.RequireObjectExists(t, cfg, localRepoPath, newCommit)
 }
 
 func makePostUploadPackRequest(ctx context.Context, t *testing.T, serverSocketPath, token string, in *gitalypb.PostUploadPackRequest, body io.Reader) (*bytes.Buffer, error) {
diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go
index 56d9ac5c3..9bf85fa52 100644
--- a/internal/gitaly/service/ssh/receive_pack_test.go
+++ b/internal/gitaly/service/ssh/receive_pack_test.go
@@ -515,7 +515,7 @@ func TestSSHReceivePackToHooks(t *testing.T) {
 	cfg.Gitlab.URL = serverURL
 	cfg.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret")
 
-	gittest.WriteCheckNewObjectExistsHook(t, cfg.Git.BinPath, cloneDetails.RemoteRepoPath)
+	gittest.WriteCheckNewObjectExistsHook(t, cfg, cloneDetails.RemoteRepoPath)
 
 	serverSocketPath := runSSHServer(t, cfg)
 
diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go
index 599f85c13..eb25a7e74 100644
--- a/internal/gitaly/service/ssh/upload_pack_test.go
+++ b/internal/gitaly/service/ssh/upload_pack_test.go
@@ -373,9 +373,9 @@ func testUploadPackCloneWithPartialCloneFilter(t *testing.T, opts ...testcfg.Opt
 	serverSocketPath := runSSHServer(t, cfg)
 
 	// Ruby file which is ~1kB in size and not present in HEAD
-	blobLessThanLimit := "6ee41e85cc9bf33c10b690df09ca735b22f3790f"
+	blobLessThanLimit := git.ObjectID("6ee41e85cc9bf33c10b690df09ca735b22f3790f")
 	// Image which is ~100kB in size and not present in HEAD
-	blobGreaterThanLimit := "18079e308ff9b3a5e304941020747e5c39b46c88"
+	blobGreaterThanLimit := git.ObjectID("18079e308ff9b3a5e304941020747e5c39b46c88")
 
 	tests := []struct {
 		desc     string
@@ -385,7 +385,7 @@ func testUploadPackCloneWithPartialCloneFilter(t *testing.T, opts ...testcfg.Opt
 		{
 			desc: "full_clone",
 			repoTest: func(t *testing.T, repoPath string) {
-				gittest.GitObjectMustExist(t, cfg.Git.BinPath, repoPath, blobGreaterThanLimit)
+				gittest.RequireObjectExists(t, cfg, repoPath, blobGreaterThanLimit)
 			},
 			cmd: git.SubCmd{
 				Name: "clone",
@@ -394,7 +394,7 @@ func testUploadPackCloneWithPartialCloneFilter(t *testing.T, opts ...testcfg.Opt
 		{
 			desc: "partial_clone",
 			repoTest: func(t *testing.T, repoPath string) {
-				gittest.GitObjectMustNotExist(t, cfg.Git.BinPath, repoPath, blobGreaterThanLimit)
+				gittest.RequireObjectNotExists(t, cfg, repoPath, blobGreaterThanLimit)
 			},
 			cmd: git.SubCmd{
 				Name: "clone",
@@ -424,7 +424,7 @@ func testUploadPackCloneWithPartialCloneFilter(t *testing.T, opts ...testcfg.Opt
 			defer func() { require.NoError(t, os.RemoveAll(localPath)) }()
 			require.NoError(t, err, "clone failed")
 
-			gittest.GitObjectMustExist(t, cfg.Git.BinPath, localPath, blobLessThanLimit)
+			gittest.RequireObjectExists(t, cfg, localPath, blobLessThanLimit)
 			tc.repoTest(t, localPath)
 		})
 	}