repository: Test FindMergeBase with SHA256

Refactor the FindMergeBase tests to generate test data at runtime so
that we can test with the SHA256 object format.

1 files changed, 118 insertions, 71 deletions

diff --git a/internal/gitaly/service/repository/merge_base_test.go b/internal/gitaly/service/repository/merge_base_test.go
index 48e367dbe..e8ec0299c 100644
--- a/internal/gitaly/service/repository/merge_base_test.go
+++ b/internal/gitaly/service/repository/merge_base_test.go
@@ -1,112 +1,159 @@
-//go:build !gitaly_test_sha256
-
 package repository
 
 import (
 	"testing"
 
-	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
-func TestSuccessfulFindFindMergeBaseRequest(t *testing.T) {
+func TestFindMergeBase(t *testing.T) {
 	t.Parallel()
 
 	ctx := testhelper.Context(t)
-	_, repo, _, client := setupRepositoryService(t, ctx)
+	cfg, client := setupRepositoryServiceWithoutRepo(t)
+
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+
+	// We set up the following history with a criss-cross merge so that the merge base becomes ambiguous:
+	//
+	//        l1  l2  l3
+	//        o---o---o
+	//       / \   \ /
+	// base o   \   X
+	//       \   \ / \
+	//        o---o---o
+	//        r1  r2  r3
+	base := gittest.WriteCommit(t, cfg, repoPath)
+	unrelated := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("unrelated"))
+	// Normal commits.
+	l1 := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("l1"), gittest.WithParents(base), gittest.WithBranch("l1"))
+	r1 := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("r1"), gittest.WithParents(base), gittest.WithBranch("r1"))
+	l2 := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(l1))
+	// Simple merge commit.
+	r2 := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(r1, l1))
+	// Criss-cross merges.
+	l3 := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(l2, r2))
+	r3 := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(r2, l2))
 
-	testCases := []struct {
-		desc      string
-		revisions [][]byte
-		base      string
+	for _, tc := range []struct {
+		desc             string
+		request          *gitalypb.FindMergeBaseRequest
+		expectedErr      error
+		expectedResponse *gitalypb.FindMergeBaseResponse
 	}{
 		{
-			desc: "oid revisions",
-			revisions: [][]byte{
-				[]byte("372ab6950519549b14d220271ee2322caa44d4eb"),
-				[]byte("8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab"),
+			desc: "no repository provided",
+			request: &gitalypb.FindMergeBaseRequest{
+				Repository: nil,
 			},
-			base: "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab",
+			expectedErr: structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
 		},
 		{
-			desc: "branch revisions",
-			revisions: [][]byte{
-				[]byte("master"),
-				[]byte("gitaly-stuff"),
+			desc: "not enough revisions",
+			request: &gitalypb.FindMergeBaseRequest{
+				Repository: repo,
+				Revisions:  [][]byte{[]byte(l1)},
 			},
-			base: "b83d6e391c22777fca1ed3012fce84f633d7fed0",
+			expectedErr: structerr.NewInvalidArgument("at least 2 revisions are required"),
 		},
 		{
-			desc: "non-existent merge base",
-			revisions: [][]byte{
-				[]byte("master"),
-				[]byte("orphaned-branch"),
+			desc: "simple merge base with object IDs",
+			request: &gitalypb.FindMergeBaseRequest{
+				Repository: repo,
+				Revisions: [][]byte{
+					[]byte(l1),
+					[]byte(r1),
+				},
+			},
+			expectedResponse: &gitalypb.FindMergeBaseResponse{
+				Base: base.String(),
 			},
-			base: "",
 		},
 		{
-			desc: "non-existent branch",
-			revisions: [][]byte{
-				[]byte("master"),
-				[]byte("a-branch-that-does-not-exist"),
+			desc: "simple merge base with references",
+			request: &gitalypb.FindMergeBaseRequest{
+				Repository: repo,
+				Revisions: [][]byte{
+					[]byte("l1"),
+					[]byte("r1"),
+				},
+			},
+			expectedResponse: &gitalypb.FindMergeBaseResponse{
+				Base: base.String(),
 			},
-			base: "",
 		},
 		{
-			desc: "2+ revisions",
-			revisions: [][]byte{
-				[]byte("few-commits"),
-				[]byte("master"),
-				[]byte("570e7b2abdd848b95f2f578043fc23bd6f6fd24d"),
+			desc: "non-existent merge base",
+			request: &gitalypb.FindMergeBaseRequest{
+				Repository: repo,
+				Revisions: [][]byte{
+					[]byte(base),
+					[]byte(unrelated),
+				},
+			},
+			expectedResponse: &gitalypb.FindMergeBaseResponse{
+				Base: "",
 			},
-			base: "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863",
 		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			request := &gitalypb.FindMergeBaseRequest{
+		{
+			desc: "non-existent branch",
+			request: &gitalypb.FindMergeBaseRequest{
 				Repository: repo,
-				Revisions:  testCase.revisions,
-			}
-
-			response, err := client.FindMergeBase(ctx, request)
-			require.NoError(t, err)
-
-			require.Equal(t, testCase.base, response.Base)
-		})
-	}
-}
-
-func TestFailedFindMergeBaseRequestDueToValidations(t *testing.T) {
-	t.Parallel()
-	ctx := testhelper.Context(t)
-	_, repo, _, client := setupRepositoryService(t, ctx)
-	for _, tc := range []struct {
-		desc        string
-		req         *gitalypb.FindMergeBaseRequest
-		expectedErr error
-	}{
+				Revisions: [][]byte{
+					[]byte("l1"),
+					[]byte("a-branch-that-does-not-exist"),
+				},
+			},
+			expectedResponse: &gitalypb.FindMergeBaseResponse{
+				Base: "",
+			},
+		},
 		{
-			desc:        "no repository provided",
-			req:         &gitalypb.FindMergeBaseRequest{Repository: nil},
-			expectedErr: structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
+			desc: "multiple revisions",
+			request: &gitalypb.FindMergeBaseRequest{
+				Repository: repo,
+				Revisions: [][]byte{
+					[]byte(l1),
+					[]byte(l2),
+					[]byte(r1),
+				},
+			},
+			// With more than two revisions all but the first commit will be treated as a virtual merge
+			// base. That is, in this Git will treat l2 and r1 as a single merge commit of those two
+			// references. Consequentially, given that l1 is an ancestor of the merge of l2 and r1, it
+			// becomes the best common ancestor.
+			expectedResponse: &gitalypb.FindMergeBaseResponse{
+				Base: l1.String(),
+			},
 		},
 		{
-			desc: "no enough revisions",
-			req: &gitalypb.FindMergeBaseRequest{
+			desc: "ambiguous merge base due to criss-cross merge",
+			request: &gitalypb.FindMergeBaseRequest{
 				Repository: repo,
-				Revisions:  [][]byte{[]byte("372ab6950519549b14d220271ee2322caa44d4eb")},
+				Revisions: [][]byte{
+					[]byte(l3),
+					[]byte(r3),
+				},
+			},
+			// We return the best merge base returned by git-merge-base(1) even though it is ambiguous in
+			// the case of criss-cross merges.
+			expectedResponse: &gitalypb.FindMergeBaseResponse{
+				Base: l2.String(),
 			},
-			expectedErr: status.Error(codes.InvalidArgument, "at least 2 revisions are required"),
 		},
 	} {
-		_, err := client.FindMergeBase(ctx, tc.req)
-		testhelper.RequireGrpcError(t, tc.expectedErr, err)
+		tc := tc
+
+		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+
+			response, err := client.FindMergeBase(ctx, tc.request)
+			testhelper.RequireGrpcError(t, tc.expectedErr, err)
+			testhelper.ProtoEqual(t, tc.expectedResponse, response)
+		})
 	}
 }