diff --git a/services/pull/check.go b/services/pull/check.go index 424d464d023..48ab332ed36 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -29,6 +29,7 @@ import ( "gitea.dev/modules/queue" "gitea.dev/modules/setting" "gitea.dev/modules/timeutil" + "gitea.dev/modules/util" asymkey_service "gitea.dev/services/asymkey" "gitea.dev/services/automergequeue" ) @@ -313,8 +314,7 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com cmd := gitcmd.NewCommand("merge-base", "--is-ancestor").AddDynamicArguments(prHeadRef, pr.BaseBranch) if err := cmd.WithRepo(pr.BaseRepo).RunWithStderr(ctx); err != nil { if gitcmd.IsErrorExitCode(err, 1) { - // prHeadRef is not an ancestor of the base branch - return nil, nil //nolint:nilnil // return nil to indicate that the PR head is not merged + return nil, util.ErrNotExist // prHeadRef is not an ancestor of the base branch, no merge commit is found } // Errors are signaled by a non-zero status that is not 1 return nil, fmt.Errorf("%-v git merge-base --is-ancestor: %w", pr, err) @@ -327,6 +327,9 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com if err != nil { return nil, fmt.Errorf("GetFullCommitID(%s) in %s: %w", prHeadRef, pr.BaseRepo.FullName(), err) } + if prHeadCommitID == pr.MergeBase { + return nil, util.ErrNotExist // empty PR, no merge commit is found + } gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo) if err != nil { @@ -403,13 +406,10 @@ func manuallyMergedByPushedCommit(ctx context.Context, pr *issues_model.PullRequ } commit, err := getMergeCommit(ctx, pr) - if err != nil { - log.Error("%-v getMergeCommit: %v", pr, err) + if errors.Is(err, util.ErrNotExist) { return false - } - - if commit == nil { - // no merge commit found + } else if err != nil { + log.Error("%-v getMergeCommit: %v", pr, err) return false } diff --git a/tests/integration/manual_merge_test.go b/tests/integration/manual_merge_test.go index ae94101844a..cf6781a1e9a 100644 --- a/tests/integration/manual_merge_test.go +++ b/tests/integration/manual_merge_test.go @@ -5,6 +5,7 @@ package integration import ( "fmt" + "net/http" "net/url" "strings" "testing" @@ -16,6 +17,7 @@ import ( "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/git/gitcmd" + "gitea.dev/modules/queue" "gitea.dev/modules/setting" api "gitea.dev/modules/structs" @@ -59,6 +61,10 @@ func TestManualMergeAutodetect(t *testing.T) { require.NoError(t, err) } + testAPICreateBranch(t, user2Ctx.Session, user2.Name, repoName, defaultBranch, "empty-pr", http.StatusCreated) + emptyAPIPull, err := doAPICreatePullRequest(user1Ctx, user2.Name, repoName, defaultBranch, "empty-pr")(t) + require.NoError(t, err) + // user1 clones, then merges every branch sequentially, then pushes once. // The first merge fast-forwards; the rest produce real merge commits, // which generates multiple commits for "git rev-list --ancestry-path --merges ...". @@ -100,5 +106,10 @@ func TestManualMergeAutodetect(t *testing.T) { assert.Equalf(t, user1.ID, pr.MergerID, "PR %d (%s) merger should be the pusher", i+1, branchNames[i]) assert.Equalf(t, expectedMergeCommits[i], pr.MergedCommitID, "PR %d (%s) should be attributed to its own merge commit, not another PR's", i+1, branchNames[i]) } + + require.NoError(t, queue.GetManager().FlushAll(t.Context(), 5*time.Second)) + emptyPR := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: emptyAPIPull.ID}) + assert.False(t, emptyPR.HasMerged, "empty PR must not be detected as manually merged") + assert.Equal(t, issues_model.PullRequestStatusAncestor, emptyPR.Status) }) }