remote: Fix flaky test caused by unconsumed hook stdin

The test for `FetchInternalRemote()` has by now been flaky for quite a
long time, and various changes which tried to fix it were unsuccessful.
Now that we've amended the test to assert contents of the hook logs we
finally got a hint about what's wrong:

    fetch_internal_remote_test.go:138:
        	Error Trace:	fetch_internal_remote_test.go:138
        	Error:      	Not equal:
        	            	expected: ""
        	            	actual  : "time=\"2021-10-22T06:57:31.170Z\" level=fatal msg=\"error when receiving data for reference-transaction hook: stdin send error: EOF\"\n"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1,2 @@
        	            	+time="2021-10-22T06:57:31.170Z" level=fatal msg="error when receiving data for reference-transaction hook: stdin send error: EOF"

        	Test:       	TestFetchInternalRemote_successful

The error happens because the standard input reader which relays
gitaly-hooks' stdin to the hook service is seeing an unexpected EOF
because the remote side (which is the hook service in this case) doesn't
consume it. And this is not much of a surprise: the mocked hook manager
we inject into the server just ignores the stdin reader completely. In
the past, we have held the opinion that it's a bug if the
reference-transaction hook doesn't fully consume stdin because it may
hint either at missing votes or at a vote being computed from partially
read data.

Let's finally fix this bug by just discarding input in our mocked hook
manager.

1 files changed, 13 insertions, 6 deletions

diff --git a/internal/gitaly/service/remote/fetch_internal_remote_test.go b/internal/gitaly/service/remote/fetch_internal_remote_test.go
index a93114cb9..e9839647d 100644
--- a/internal/gitaly/service/remote/fetch_internal_remote_test.go
+++ b/internal/gitaly/service/remote/fetch_internal_remote_test.go
@@ -114,10 +114,17 @@ func TestFetchInternalRemote_successful(t *testing.T) {
 			deps.GetGitCmdFactory(),
 			deps.GetPackObjectsCache(),
 		))
-	}, testserver.WithHookManager(gitalyhook.NewMockManager(t, nil, nil, nil, func(*testing.T, context.Context, gitalyhook.ReferenceTransactionState, []string, io.Reader) error {
-		referenceTransactionHookCalled++
-		return nil
-	})), testserver.WithDisablePraefect())
+	}, testserver.WithHookManager(gitalyhook.NewMockManager(t, nil, nil, nil,
+		func(t *testing.T, _ context.Context, _ gitalyhook.ReferenceTransactionState, _ []string, stdin io.Reader) error {
+			// We need to discard stdin or otherwise the sending Goroutine may return an
+			// EOF error and cause the test to fail.
+			_, err := io.Copy(io.Discard, stdin)
+			require.NoError(t, err)
+
+			referenceTransactionHookCalled++
+			return nil
+		}),
+	), testserver.WithDisablePraefect())
 
 	ctx, err := storage.InjectGitalyServers(ctx, remoteRepo.GetStorageName(), remoteAddr, remoteCfg.Auth.Token)
 	require.NoError(t, err)
@@ -128,8 +135,8 @@ func TestFetchInternalRemote_successful(t *testing.T) {
 	connsPool := client.NewPool()
 	defer connsPool.Close()
 
-	// Use the `assert` package while we haven't figured out this failure yet such that we can
-	// also verify the hook logs and hopefully get a lead.
+	// Use the `assert` package such that we can get information about why hooks have failed via
+	// the hook logs in case it did fail unexpectedly.
 	assert.NoError(t, FetchInternalRemote(ctx, localCfg, connsPool, localRepo, remoteRepo))
 
 	hookLogs := filepath.Join(localCfg.Logging.Dir, "gitaly_hooks.log")