diff options
| author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-12-02 14:48:39 +0300 |
|---|---|---|
| committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-12-02 14:59:35 +0300 |
| commit | 5c107f34961e99bb80d6c8b09a6725499844b773 (patch) | |
| tree | c0ed8ebefe581210f6f66e2b5521a952967b3242 | |
| parent | a5a5d83630f13c3eb3e1650a24423fc5e9bc47d2 (diff) | |
operations: Fix wrong ordering of merge parents for UserMergeBranch
While our tests do verify that generated merge commits from
UserMergeBranch have expected merge parents, the order isn't verified at
all. Unfortunately, ordering is important and not checking it has
allowed us to sneak in a bug in the Go implementation which ordered
parents the wrong way.
Fix the test to verify order and fix the Go implementation to order
merge parents correctly.
| -rw-r--r-- | changelogs/unreleased/pks-go-user-merge-branch-reversed-parents.yml | 5 | ||||
| -rw-r--r-- | internal/gitaly/service/operations/merge.go | 4 | ||||
| -rw-r--r-- | internal/gitaly/service/operations/merge_test.go | 3 |
3 files changed, 8 insertions, 4 deletions
diff --git a/changelogs/unreleased/pks-go-user-merge-branch-reversed-parents.yml b/changelogs/unreleased/pks-go-user-merge-branch-reversed-parents.yml new file mode 100644 index 000000000..6a59a8ee9 --- /dev/null +++ b/changelogs/unreleased/pks-go-user-merge-branch-reversed-parents.yml @@ -0,0 +1,5 @@ +--- +title: 'operations: Fix wrong ordering of merge parents for UserMergeBranch' +merge_request: 2868 +author: +type: fixed diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 64c6ad276..93fd77829 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -123,8 +123,8 @@ func (s *Server) userMergeBranch(stream gitalypb.OperationService_UserMergeBranc AuthorName: string(firstRequest.User.Name), AuthorMail: string(firstRequest.User.Email), Message: string(firstRequest.Message), - Ours: firstRequest.CommitId, - Theirs: revision, + Ours: revision, + Theirs: firstRequest.CommitId, }.Run(ctx, s.cfg) if err != nil { if errors.Is(err, git2go.ErrInvalidArgument) { diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index e1e0a7759..c9b467fb1 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -104,8 +104,7 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { require.Equal(t, gitalypb.OperationBranchUpdate{CommitId: commit.Id}, *(secondResponse.BranchUpdate)) - require.Contains(t, commit.ParentIds, mergeBranchHeadBefore, "merge parents must include previous HEAD of branch") - require.Contains(t, commit.ParentIds, commitToMerge, "merge parents must include commit to merge") + require.Equal(t, commit.ParentIds, []string{mergeBranchHeadBefore, commitToMerge}) require.True(t, strings.HasPrefix(string(commit.Body), mergeCommitMessage), "expected %q to start with %q", commit.Body, mergeCommitMessage) |