diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-08-26 13:34:25 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-08-30 12:12:43 +0300 |
commit | 15c64f82e0a07be5c83756ad7358f06856622e85 (patch) | |
tree | ed3629f8b27462cbd6c596ba356c2379d32f7681 /cmd | |
parent | 640dabb32775a518e2a96ed2ecaa333ba82f86f0 (diff) |
git2go: Support driving merge command via gob
The gitaly-git2go merge subcommand has learned to use gob for
serialization of parameters and results. This commit now teaches the
internal package to drive the subcommand with gob.
This change is implemented behind a feature flag because of backwards
compatibility.
Diffstat (limited to 'cmd')
-rw-r--r-- | cmd/gitaly-git2go/merge_test.go | 75 |
1 files changed, 50 insertions, 25 deletions
diff --git a/cmd/gitaly-git2go/merge_test.go b/cmd/gitaly-git2go/merge_test.go index 270e3dae9..ef1c2100b 100644 --- a/cmd/gitaly-git2go/merge_test.go +++ b/cmd/gitaly-git2go/merge_test.go @@ -3,6 +3,8 @@ package main import ( + "context" + "errors" "fmt" "testing" "time" @@ -15,11 +17,18 @@ import ( "gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v14/internal/git2go" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg" ) func TestMergeFailsWithMissingArguments(t *testing.T) { + testhelper.NewFeatureSets([]featureflag.FeatureFlag{ + featureflag.Git2GoMergeGob, + }).Run(t, testMergeFailsWithMissingArguments) +} + +func testMergeFailsWithMissingArguments(t *testing.T, ctx context.Context) { cfg, repo, repoPath := testcfg.BuildWithRepo(t) executor := git2go.NewExecutor(cfg, config.NewLocator(cfg)) @@ -66,9 +75,6 @@ func TestMergeFailsWithMissingArguments(t *testing.T) { for _, tc := range testcases { t.Run(tc.desc, func(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() - _, err := executor.Merge(ctx, repo, tc.request) require.Error(t, err) require.Equal(t, tc.expectedErr, err.Error()) @@ -77,13 +83,16 @@ func TestMergeFailsWithMissingArguments(t *testing.T) { } func TestMergeFailsWithInvalidRepositoryPath(t *testing.T) { + testhelper.NewFeatureSets([]featureflag.FeatureFlag{ + featureflag.Git2GoMergeGob, + }).Run(t, testMergeFailsWithInvalidRepositoryPath) +} + +func testMergeFailsWithInvalidRepositoryPath(t *testing.T, ctx context.Context) { cfg, repo, _ := testcfg.BuildWithRepo(t) testhelper.BuildGitalyGit2Go(t, cfg) executor := git2go.NewExecutor(cfg, config.NewLocator(cfg)) - ctx, cancel := testhelper.Context() - defer cancel() - _, err := executor.Merge(ctx, repo, git2go.MergeCommand{ Repository: "/does/not/exist", AuthorName: "Foo", AuthorMail: "foo@example.com", Message: "Foo", Ours: "HEAD", Theirs: "HEAD", }) @@ -92,14 +101,21 @@ func TestMergeFailsWithInvalidRepositoryPath(t *testing.T) { } func TestMergeTrees(t *testing.T) { + testhelper.NewFeatureSets([]featureflag.FeatureFlag{ + featureflag.Git2GoMergeGob, + }).Run(t, testMergeTrees) +} + +func testMergeTrees(t *testing.T, ctx context.Context) { testcases := []struct { - desc string - base map[string]string - ours map[string]string - theirs map[string]string - expected map[string]string - expectedResponse git2go.MergeResult - expectedStderr string + desc string + base map[string]string + ours map[string]string + theirs map[string]string + expected map[string]string + expectedResponse git2go.MergeResult + expectedErr error + expectedErrWithoutGob error }{ { desc: "trivial merge succeeds", @@ -174,7 +190,9 @@ func TestMergeTrees(t *testing.T) { theirs: map[string]string{ "1": "qux", }, - expectedStderr: "merge: could not auto-merge due to conflicts\n", + expectedErr: fmt.Errorf("merge: %w", git2go.SerializableError(errors.New("could not auto-merge due to conflicts"))), + //nolint:revive + expectedErrWithoutGob: errors.New("merge: could not auto-merge due to conflicts\n"), }, } @@ -190,9 +208,6 @@ func TestMergeTrees(t *testing.T) { authorDate := time.Date(2020, 7, 30, 7, 45, 50, 0, time.FixedZone("UTC+2", +2*60*60)) t.Run(tc.desc, func(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() - response, err := executor.Merge(ctx, repoProto, git2go.MergeCommand{ Repository: repoPath, AuthorName: "John Doe", @@ -203,9 +218,12 @@ func TestMergeTrees(t *testing.T) { Theirs: theirs.String(), }) - if tc.expectedStderr != "" { - require.Error(t, err) - require.Contains(t, err.Error(), tc.expectedStderr) + if tc.expectedErr != nil { + if featureflag.Git2GoMergeGob.IsEnabled(ctx) { + require.Equal(t, tc.expectedErr, err) + } else { + require.Equal(t, tc.expectedErrWithoutGob, err) + } return } @@ -239,6 +257,12 @@ func TestMergeTrees(t *testing.T) { } func TestMerge_recursive(t *testing.T) { + testhelper.NewFeatureSets([]featureflag.FeatureFlag{ + featureflag.Git2GoMergeGob, + }).Run(t, testMergeRecursive) +} + +func testMergeRecursive(t *testing.T, ctx context.Context) { cfg := testcfg.Build(t) testhelper.BuildGitalyGit2Go(t, cfg) executor := git2go.NewExecutor(cfg, config.NewLocator(cfg)) @@ -278,9 +302,6 @@ func TestMerge_recursive(t *testing.T) { authorDate := time.Date(2020, 7, 30, 7, 45, 50, 0, time.FixedZone("UTC+2", +2*60*60)) - ctx, cancel := testhelper.Context() - defer cancel() - // When creating the criss-cross merges, we have been doing evil merges // as each merge has applied changes from the other side while at the // same time incrementing the own file contents. As we exceed the merge @@ -308,8 +329,12 @@ func TestMerge_recursive(t *testing.T) { Ours: ours[len(ours)-1].String(), Theirs: theirs[len(theirs)-1].String(), }) - require.Error(t, err) - require.Equal(t, err.Error(), "merge: could not auto-merge due to conflicts\n") + if featureflag.Git2GoMergeGob.IsEnabled(ctx) { + require.Equal(t, fmt.Errorf("merge: %w", git2go.SerializableError(errors.New("could not auto-merge due to conflicts"))), err) + } else { + //nolint:revive + require.Equal(t, errors.New("merge: could not auto-merge due to conflicts\n"), err) + } // Otherwise, if we're merging an earlier criss-cross merge which has // half of the limit many criss-cross patterns, we exactly hit the |