uploadpack: Fix racy tests for pack-objects hookpks-upload-pack-racy-hook-tests

The pack-objects hook tests are racy. While I cannot reproduce the
flakiness on my machine, it seems like the transport is closing before
the error message which we inject via hook is sent.

Let's rewrite the tests such that they do not produce an error, but
instead simply intercept the git-pack-objects(1) invocation to write a
given output file and then afterwards execute the real git binary. This
should remove the racy error case altogether as there are no errors
anymore.

2 files changed, 12 insertions, 6 deletions

diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go
index d02bef953..782b9a516 100644
--- a/internal/gitaly/service/smarthttp/upload_pack_test.go
+++ b/internal/gitaly/service/smarthttp/upload_pack_test.go
@@ -256,7 +256,8 @@ func TestUploadPackWithPackObjectsHook(t *testing.T) {
 	config.Config.BinDir = hookDir
 
 	outputPath := filepath.Join(hookDir, "output")
-	script := fmt.Sprintf("#!/bin/sh\necho 'I was invoked' >%s\nexit 1\n", outputPath)
+	hookScript := fmt.Sprintf("#!/bin/sh\necho 'I was invoked' >'%s'\nshift\nexec '%s' \"$@\"\n",
+		outputPath, config.Config.Git.BinPath)
 
 	// We're using a custom pack-objects hook for git-upload-pack. In order
 	// to assure that it's getting executed as expected, we're writing a
@@ -265,7 +266,7 @@ func TestUploadPackWithPackObjectsHook(t *testing.T) {
 	// out. In the best case we'd have just printed the error to stderr and
 	// check the return error message. But it's unfortunately not
 	// transferred back.
-	cleanup = testhelper.WriteExecutable(t, filepath.Join(hookDir, "gitaly-hooks"), []byte(script))
+	cleanup = testhelper.WriteExecutable(t, filepath.Join(hookDir, "gitaly-hooks"), []byte(hookScript))
 	defer cleanup()
 
 	serverSocketPath, stop := runSmartHTTPServer(t)
@@ -289,7 +290,7 @@ func TestUploadPackWithPackObjectsHook(t *testing.T) {
 	_, err := makePostUploadPackRequest(ctx, t, serverSocketPath, &gitalypb.PostUploadPackRequest{
 		Repository: repo,
 	}, requestBuffer)
-	require.Error(t, err)
+	require.NoError(t, err)
 
 	contents := testhelper.MustReadFile(t, outputPath)
 	require.Equal(t, "I was invoked\n", string(contents))
diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go
index 1f777ca3f..e88fc7585 100644
--- a/internal/gitaly/service/ssh/upload_pack_test.go
+++ b/internal/gitaly/service/ssh/upload_pack_test.go
@@ -270,19 +270,22 @@ func TestUploadPackCloneSuccess(t *testing.T) {
 func TestUploadPackWithPackObjectsHook(t *testing.T) {
 	filterDir, cleanup := testhelper.TempDir(t)
 	defer cleanup()
+	outputPath := filepath.Join(filterDir, "output")
 
 	defer func(old string) {
 		config.Config.BinDir = old
 	}(config.Config.BinDir)
 	config.Config.BinDir = filterDir
 
+	hookScript := fmt.Sprintf("#!/bin/sh\necho 'I was invoked' >'%s'\nshift\nexec '%s' \"$@\"\n",
+		outputPath, config.Config.Git.BinPath)
+
 	// We're using a custom pack-objetcs hook for git-upload-pack. In order
 	// to assure that it's getting executed as expected, we're writing a
 	// custom script which replaces the hook binary. It doesn't do anything
 	// special, but writes an error message and errors out and should thus
 	// cause the clone to fail with this error message.
-	testhelper.WriteExecutable(t, filepath.Join(filterDir, "gitaly-hooks"),
-		[]byte("#!/bin/sh\necho 'I was invoked' >&2\nexit 73\n"))
+	testhelper.WriteExecutable(t, filepath.Join(filterDir, "gitaly-hooks"), []byte(hookScript))
 
 	serverSocketPath, stop := runSSHServer(t)
 	defer stop()
@@ -301,7 +304,9 @@ func TestUploadPackWithPackObjectsHook(t *testing.T) {
 		},
 		server: serverSocketPath,
 	}.execute(t)
-	require.Contains(t, err.Error(), "remote: I was invoked")
+	require.NoError(t, err)
+
+	require.Equal(t, []byte("I was invoked\n"), testhelper.MustReadFile(t, outputPath))
 }
 
 func TestUploadPackWithoutSideband(t *testing.T) {