tests: Refactor to use `gittest` package instead of `NewWithoutRepo()`

We want to reduce the number of calls to `git.NewWithoutRepo()` to a
bare minimum. Refactor tests to use `gittest.NewCommand()` and similar
to get rid of some calls to `NewWithoutRepo()`.

3 files changed, 40 insertions, 74 deletions

diff --git a/cmd/gitaly-ssh/upload_pack_test.go b/cmd/gitaly-ssh/upload_pack_test.go
index 92f562960..b9cb51945 100644
--- a/cmd/gitaly-ssh/upload_pack_test.go
+++ b/cmd/gitaly-ssh/upload_pack_test.go
@@ -3,7 +3,6 @@
 package main
 
 import (
-	"bytes"
 	"fmt"
 	"os"
 	"testing"
@@ -40,7 +39,6 @@ func TestVisibilityOfHiddenRefs(t *testing.T) {
 	existingSha := git.ObjectID("1e292f8fedd741b75372e19097c76d327140c312")
 	keepAroundRef := fmt.Sprintf("%s/%s", keepAroundNamespace, existingSha)
 
-	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
 	localRepo := localrepo.NewTestRepo(t, cfg, repo)
 	updater, err := updateref.New(ctx, localRepo)
 
@@ -81,31 +79,20 @@ func TestVisibilityOfHiddenRefs(t *testing.T) {
 
 			require.NoError(t, err)
 
-			env := []string{
-				fmt.Sprintf("GITALY_PAYLOAD=%s", payload),
-				fmt.Sprintf("GITALY_ADDRESS=unix:%s", socketPath),
-				fmt.Sprintf("GITALY_WD=%s", wd),
-				fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")),
-				fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", cfg.BinaryPath("gitaly-ssh")),
-			}
-
-			stdout := &bytes.Buffer{}
-			cmd, err := gitCmdFactory.NewWithoutRepo(ctx, git.SubCmd{
-				Name: "ls-remote",
-				Args: []string{
-					fmt.Sprintf("%s:%s", "git@localhost", repoPath),
-					keepAroundRef,
+			stdout := gittest.ExecOpts(t, cfg, gittest.ExecConfig{
+				Env: []string{
+					fmt.Sprintf("GITALY_PAYLOAD=%s", payload),
+					fmt.Sprintf("GITALY_ADDRESS=unix:%s", socketPath),
+					fmt.Sprintf("GITALY_WD=%s", wd),
+					fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")),
+					fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", cfg.BinaryPath("gitaly-ssh")),
 				},
-			}, git.WithEnv(env...), git.WithStdout(stdout))
-			require.NoError(t, err)
-
-			err = cmd.Wait()
-			require.NoError(t, err)
+			}, "ls-remote", fmt.Sprintf("%s:%s", "git@localhost", repoPath), keepAroundRef)
 
 			if test.HiddenRefFound {
-				require.Equal(t, fmt.Sprintf("%s\t%s\n", existingSha, keepAroundRef), stdout.String())
+				require.Equal(t, fmt.Sprintf("%s\t%s\n", existingSha, keepAroundRef), string(stdout))
 			} else {
-				require.NotEqual(t, fmt.Sprintf("%s\t%s\n", existingSha, keepAroundRef), stdout.String())
+				require.NotEqual(t, fmt.Sprintf("%s\t%s\n", existingSha, keepAroundRef), string(stdout))
 			}
 		})
 	}
diff --git a/internal/gitaly/service/repository/redirecting_test_server_test.go b/internal/gitaly/service/repository/redirecting_test_server_test.go
index 2b3fe9e95..08125a366 100644
--- a/internal/gitaly/service/repository/redirecting_test_server_test.go
+++ b/internal/gitaly/service/repository/redirecting_test_server_test.go
@@ -9,7 +9,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
@@ -46,21 +45,11 @@ func TestRedirectingServerRedirects(t *testing.T) {
 	dir := testhelper.TempDir(t)
 
 	httpServerState, redirectingServer := StartRedirectingTestServer()
-	ctx := testhelper.Context(t)
 
 	var stderr bytes.Buffer
-	cmd, err := gittest.NewCommandFactory(t, cfg).NewWithoutRepo(ctx, git.SubCmd{
-		Name: "clone",
-		Flags: []git.Option{
-			git.Flag{Name: "--bare"},
-		},
-		Args: []string{
-			redirectingServer.URL, dir,
-		},
-	}, git.WithConfig(git.ConfigPair{Key: "http.followRedirects", Value: "true"}), git.WithDisabledHooks(), git.WithStderr(&stderr))
-	require.NoError(t, err)
-
-	require.Error(t, cmd.Wait())
+	cloneCmd := gittest.NewCommand(t, cfg, "clone", "--bare", redirectingServer.URL, dir)
+	cloneCmd.Stderr = &stderr
+	require.Error(t, cloneCmd.Run())
 	require.Contains(t, stderr.String(), "unable to update url base from redirection")
 
 	redirectingServer.Close()
diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go
index aeb0e7695..4c2c45472 100644
--- a/internal/gitaly/service/ssh/upload_pack_test.go
+++ b/internal/gitaly/service/ssh/upload_pack_test.go
@@ -51,8 +51,8 @@ func runClone(
 	ctx context.Context,
 	cfg config.Cfg,
 	withSidechannel bool,
-	cloneCmd git.Cmd,
 	request *gitalypb.SSHUploadPackRequest,
+	args ...string,
 ) error {
 	payload, err := protojson.Marshal(request)
 	require.NoError(t, err)
@@ -62,23 +62,21 @@ func runClone(
 		flagsWithValues = append(flagsWithValues, flag.FormatWithValue(value))
 	}
 
-	env := []string{
+	var output bytes.Buffer
+	cloneCmd := gittest.NewCommand(t, cfg, append([]string{"clone"}, args...)...)
+	cloneCmd.Stdout = &output
+	cloneCmd.Stderr = &output
+	cloneCmd.Env = append(cloneCmd.Env,
 		fmt.Sprintf("GITALY_ADDRESS=%s", cfg.SocketPath),
 		fmt.Sprintf("GITALY_PAYLOAD=%s", payload),
 		fmt.Sprintf("GITALY_FEATUREFLAGS=%s", strings.Join(flagsWithValues, ",")),
 		fmt.Sprintf(`GIT_SSH_COMMAND=%s upload-pack`, cfg.BinaryPath("gitaly-ssh")),
-	}
+	)
 	if withSidechannel {
-		env = append(env, "GITALY_USE_SIDECHANNEL=1")
+		cloneCmd.Env = append(cloneCmd.Env, "GITALY_USE_SIDECHANNEL=1")
 	}
 
-	var output bytes.Buffer
-	gitCommand, err := gittest.NewCommandFactory(t, cfg).NewWithoutRepo(ctx,
-		cloneCmd, git.WithStdout(&output), git.WithStderr(&output), git.WithEnv(env...), git.WithDisabledHooks(),
-	)
-	require.NoError(t, err)
-
-	if err := gitCommand.Wait(); err != nil {
+	if err := cloneCmd.Run(); err != nil {
 		return fmt.Errorf("Failed to run `git clone`: %q", output.Bytes())
 	}
 
@@ -537,7 +535,7 @@ func testUploadPackSuccessful(t *testing.T, sidechannel bool, opts ...testcfg.Op
 	for _, tc := range []struct {
 		desc             string
 		request          *gitalypb.SSHUploadPackRequest
-		cloneFlags       []git.Option
+		cloneArgs        []string
 		deepen           float64
 		verify           func(t *testing.T, localRepoPath string)
 		expectedProtocol string
@@ -561,8 +559,8 @@ func testUploadPackSuccessful(t *testing.T, sidechannel bool, opts ...testcfg.Op
 			request: &gitalypb.SSHUploadPackRequest{
 				Repository: repo,
 			},
-			cloneFlags: []git.Option{
-				git.ValueFlag{Name: "--depth", Value: "1"},
+			cloneArgs: []string{
+				"--depth=1",
 			},
 			deepen: 1,
 		},
@@ -572,8 +570,8 @@ func testUploadPackSuccessful(t *testing.T, sidechannel bool, opts ...testcfg.Op
 				Repository:  repo,
 				GitProtocol: git.ProtocolV2,
 			},
-			cloneFlags: []git.Option{
-				git.ValueFlag{Name: "--depth", Value: "1"},
+			cloneArgs: []string{
+				"--depth=1",
 			},
 			deepen:           1,
 			expectedProtocol: git.ProtocolV2,
@@ -583,8 +581,8 @@ func testUploadPackSuccessful(t *testing.T, sidechannel bool, opts ...testcfg.Op
 			request: &gitalypb.SSHUploadPackRequest{
 				Repository: repo,
 			},
-			cloneFlags: []git.Option{
-				git.ValueFlag{Name: "--filter", Value: "blob:limit=1024"},
+			cloneArgs: []string{
+				"--filter=blob:limit=1024",
 			},
 			verify: func(t *testing.T, repoPath string) {
 				gittest.RequireObjectNotExists(t, cfg, repoPath, largeBlobID)
@@ -593,8 +591,8 @@ func testUploadPackSuccessful(t *testing.T, sidechannel bool, opts ...testcfg.Op
 		},
 		{
 			desc: "hidden tags",
-			cloneFlags: []git.Option{
-				git.Flag{Name: "--mirror"},
+			cloneArgs: []string{
+				"--mirror",
 			},
 			request: &gitalypb.SSHUploadPackRequest{
 				Repository: repo,
@@ -619,11 +617,11 @@ func testUploadPackSuccessful(t *testing.T, sidechannel bool, opts ...testcfg.Op
 			negotiationMetrics.Reset()
 			protocolDetectingFactory.Reset(t)
 
-			require.NoError(t, runClone(t, ctx, cfg, sidechannel, git.SubCmd{
-				Name:  "clone",
-				Args:  []string{"git@localhost:test/test.git", localRepoPath},
-				Flags: tc.cloneFlags,
-			}, tc.request))
+			require.NoError(t, runClone(t, ctx, cfg, sidechannel, tc.request,
+				append([]string{
+					"git@localhost:test/test.git", localRepoPath,
+				}, tc.cloneArgs...)...,
+			))
 
 			requireRevisionsEqual(t, cfg, repoPath, localRepoPath, "refs/heads/main")
 
@@ -679,12 +677,9 @@ func TestUploadPack_packObjectsHook(t *testing.T) {
 
 	localRepoPath := testhelper.TempDir(t)
 
-	err := runClone(t, ctx, cfg, false, git.SubCmd{
-		Name: "clone", Args: []string{"git@localhost:test/test.git", localRepoPath},
-	}, &gitalypb.SSHUploadPackRequest{
+	require.NoError(t, runClone(t, ctx, cfg, false, &gitalypb.SSHUploadPackRequest{
 		Repository: repo,
-	})
-	require.NoError(t, err)
+	}, "git@localhost:test/test.git", localRepoPath))
 
 	require.Equal(t, []byte("I was invoked\n"), testhelper.MustReadFile(t, outputPath))
 }
@@ -756,17 +751,12 @@ func TestUploadPack_invalidStorage(t *testing.T) {
 
 	localRepoPath := testhelper.TempDir(t)
 
-	err := runClone(t, ctx, cfg, false, git.SubCmd{
-		Name: "clone",
-		Args: []string{
-			"git@localhost:test/test.git", localRepoPath,
-		},
-	}, &gitalypb.SSHUploadPackRequest{
+	err := runClone(t, ctx, cfg, false, &gitalypb.SSHUploadPackRequest{
 		Repository: &gitalypb.Repository{
 			StorageName:  "foobar",
 			RelativePath: repo.GetRelativePath(),
 		},
-	})
+	}, "git@localhost:test/test.git", localRepoPath)
 	require.Error(t, err)
 
 	if testhelper.IsPraefectEnabled() {