Merge branch 'pks-fix-flaky-apply-bfg-object-map-stream' into 'master'

cleanup: Fix flaky test for invalid requests

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5117

Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>

1 files changed, 37 insertions, 32 deletions

diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go
index 388c4b973..da808ce3d 100644
--- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go
+++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go
@@ -14,11 +14,10 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func TestApplyBfgObjectMapStreamSuccess(t *testing.T) {
@@ -65,7 +64,18 @@ func TestApplyBfgObjectMapStreamSuccess(t *testing.T) {
 		tagID, tagID,
 	)
 
-	entries, err := doStreamingRequest(t, ctx, protoRepo, client, objectMapData)
+	entries, err := doStreamingRequest(t, ctx, client,
+		&gitalypb.ApplyBfgObjectMapStreamRequest{
+			Repository: protoRepo,
+			ObjectMap:  []byte(objectMapData[:10]),
+		},
+		&gitalypb.ApplyBfgObjectMapStreamRequest{
+			ObjectMap: []byte(objectMapData[10:20]),
+		},
+		&gitalypb.ApplyBfgObjectMapStreamRequest{
+			ObjectMap: []byte(objectMapData[20:]),
+		},
+	)
 	require.NoError(t, err)
 
 	// Ensure that the internal refs are gone, but the others still exist
@@ -106,53 +116,48 @@ func requireEntry(t *testing.T, entry *gitalypb.ApplyBfgObjectMapStreamResponse_
 }
 
 func TestApplyBfgObjectMapStreamFailsOnInvalidInput(t *testing.T) {
-	ctx := testhelper.Context(t)
+	t.Parallel()
 
-	cfg, protoRepo, _, client := setupCleanupService(t, ctx)
+	ctx := testhelper.Context(t)
+	_, repoProto, _, client := setupCleanupService(t, ctx)
 
 	t.Run("invalid object map", func(t *testing.T) {
-		entries, err := doStreamingRequest(t, ctx, protoRepo, client, "invalid-data here as you can see")
-		require.Empty(t, entries)
-		testhelper.RequireGrpcCode(t, err, codes.InvalidArgument)
+		response, err := doStreamingRequest(t, ctx, client, &gitalypb.ApplyBfgObjectMapStreamRequest{
+			Repository: repoProto,
+			ObjectMap:  []byte("invalid-data here as you can see"),
+		})
+		require.Nil(t, response)
+		testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf("object map invalid at line 0"), err)
 	})
 
-	repo := localrepo.NewTestRepo(t, cfg, protoRepo)
-	headCommit, err := repo.ReadCommit(ctx, "HEAD")
-	require.NoError(t, err)
-
-	objectMapData := fmt.Sprintf(
-		"old                                      new\n%s %s\n",
-		headCommit.Id, git.ObjectHashSHA1.ZeroOID.String(),
-	)
-
 	t.Run("no repository provided", func(t *testing.T) {
-		entries, err := doStreamingRequest(t, ctx, nil, client, objectMapData)
-		require.Empty(t, entries)
-		testhelper.RequireGrpcError(t, status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect(
+		response, err := doStreamingRequest(t, ctx, client, &gitalypb.ApplyBfgObjectMapStreamRequest{
+			Repository: nil,
+			ObjectMap:  []byte("does not matter"),
+		})
+		require.Nil(t, response)
+		testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect(
 			"empty Repository",
 			"repo scoped: empty Repository",
 		)), err)
 	})
 }
 
-func doStreamingRequest(t *testing.T, ctx context.Context, repo *gitalypb.Repository, client gitalypb.CleanupServiceClient, objectMap string) ([]*gitalypb.ApplyBfgObjectMapStreamResponse_Entry, error) {
+func doStreamingRequest(
+	t *testing.T,
+	ctx context.Context,
+	client gitalypb.CleanupServiceClient,
+	requests ...*gitalypb.ApplyBfgObjectMapStreamRequest,
+) ([]*gitalypb.ApplyBfgObjectMapStreamResponse_Entry, error) {
 	t.Helper()
 
-	// Split the data across multiple requests
-	parts := strings.SplitN(objectMap, " ", 2)
-	req1 := &gitalypb.ApplyBfgObjectMapStreamRequest{
-		Repository: repo,
-		ObjectMap:  []byte(parts[0] + " "),
-	}
-	req2 := &gitalypb.ApplyBfgObjectMapStreamRequest{ObjectMap: []byte(parts[1])}
-
 	server, err := client.ApplyBfgObjectMapStream(ctx)
 	require.NoError(t, err)
-	require.NoError(t, server.Send(req1))
-	require.NoError(t, server.Send(req2))
+	for _, request := range requests {
+		require.NoError(t, server.Send(request))
+	}
 	require.NoError(t, server.CloseSend())
 
-	// receive all responses in a loop
 	var entries []*gitalypb.ApplyBfgObjectMapStreamResponse_Entry
 	for {
 		rsp, err := server.Recv()