operations: Fix flakiness due to EOF timeout

In our merge-related tests, we're using testhelper.ReceiveEOFWithTimeout
to wait for the EOF error. If we fail to receive it after more than a
second, then the test fails. It's debatable what the real merit of this
timeout is: the code is not timing sensitive, but we just want to assert
that it does indeed succeed. Furthermore, given our recent shift to use
`t.Parallel()` more, we have seen that this timeout may be exceeded on
occasion.

Remove these timeouts and simply assert that we do get an EOF error and
drop the now-unused helper function.

2 files changed, 9 insertions, 46 deletions

diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index b5b2fe29a..ad6b79794 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"regexp"
@@ -91,10 +92,8 @@ func TestUserMergeBranch_successful(t *testing.T) {
 	secondResponse, err := mergeBidi.Recv()
 	require.NoError(t, err, "receive second response")
 
-	testhelper.ReceiveEOFWithTimeout(t, func() error {
-		_, err = mergeBidi.Recv()
-		return err
-	})
+	_, err = mergeBidi.Recv()
+	require.Equal(t, io.EOF, err)
 
 	commit, err := repo.ReadCommit(ctx, git.Revision(mergeBranchName))
 	require.NoError(t, err, "look up git commit after call has finished")
@@ -211,10 +210,8 @@ func TestUserMergeBranch_stableMergeIDs(t *testing.T) {
 	require.NoError(t, err, "receive second response")
 	require.Equal(t, expectedMergeID, response.BranchUpdate.CommitId)
 
-	testhelper.ReceiveEOFWithTimeout(t, func() error {
-		_, err = mergeBidi.Recv()
-		return err
-	})
+	_, err = mergeBidi.Recv()
+	require.Equal(t, io.EOF, err)
 
 	commit, err := repo.ReadCommit(ctx, git.Revision(mergeBranchName))
 	require.NoError(t, err, "look up git commit after call has finished")
@@ -410,10 +407,8 @@ func TestUserMergeBranch_ambiguousReference(t *testing.T) {
 	response, err := merge.Recv()
 	require.NoError(t, err, "receive second response")
 
-	testhelper.ReceiveEOFWithTimeout(t, func() error {
-		_, err = merge.Recv()
-		return err
-	})
+	_, err = merge.Recv()
+	require.Equal(t, io.EOF, err)
 
 	commit, err := repo.ReadCommit(ctx, git.Revision("refs/heads/"+mergeBranchName))
 	require.NoError(t, err, "look up git commit after call has finished")
@@ -469,10 +464,8 @@ func TestUserMergeBranch_failingHooks(t *testing.T) {
 				require.NoError(t, err, "receive second response")
 				require.Contains(t, secondResponse.PreReceiveError, "failure")
 
-				testhelper.ReceiveEOFWithTimeout(t, func() error {
-					_, err = mergeBidi.Recv()
-					return err
-				})
+				_, err = mergeBidi.Recv()
+				require.Equal(t, io.EOF, err)
 			}
 
 			currentBranchHead := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", mergeBranchName)
diff --git a/internal/testhelper/stream.go b/internal/testhelper/stream.go
deleted file mode 100644
index b625a48a0..000000000
--- a/internal/testhelper/stream.go
+++ /dev/null
@@ -1,30 +0,0 @@
-package testhelper
-
-import (
-	"fmt"
-	"io"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/require"
-)
-
-// ReceiveEOFWithTimeout reads to the end of the stream and will throw an
-// error if a deadlock is suspected
-func ReceiveEOFWithTimeout(t testing.TB, errorFunc func() error) {
-	errCh := make(chan error, 1)
-	go func() {
-		errCh <- errorFunc()
-	}()
-
-	var err error
-	select {
-	case err = <-errCh:
-	case <-time.After(1 * time.Second):
-		err = fmt.Errorf("timed out waiting for EOF")
-	}
-
-	if err != nil {
-		require.Equal(t, io.EOF, err)
-	}
-}