Creating push comments before invoke pull request checking

services/issue/comments.go

@@ -164,6 +164,13 @@ func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) (err e
 
 	c.IsForcePush = data.IsForcePush
 
+	if err := c.LoadIssue(ctx); err != nil {
+		return err
+	}
+	if err := c.Issue.LoadRepo(ctx); err != nil {
+		return err
+	}
+
 	if c.IsForcePush {
 		if len(data.CommitIDs) != 2 {
 			return nil

services/pull/merge.go

@@ -248,6 +248,8 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U
 	}
 	defer releaser()
 	defer func() {
+		// FIXME: This is a duplicated call to AddTestPullRequestTask in case the merge or push successfully but
+		// the post-receive hook failed. This will ensure the test task is added.
 		go AddTestPullRequestTask(TestPullRequestOptions{
 			RepoID:      pr.BaseRepo.ID,
 			Doer:        doer,

services/pull/pull.go

@@ -376,7 +376,6 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
 	graceful.GetManager().RunWithShutdownContext(func(ctx context.Context) {
 		// There is no sensible way to shut this down ":-("
 		// If you don't let it run all the way then you will lose data
-		// TODO: graceful: AddTestPullRequestTask needs to become a queue!
 
 		repo, err := repo_model.GetRepositoryByID(ctx, opts.RepoID)
 		if err != nil {
@@ -402,11 +401,15 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
 				continue
 			}
 
-			StartPullRequestCheckImmediately(ctx, pr)
+			// create push comment before check pull request status,
+			// then when the status is mergeable, the comment is already in database, to make testing easy and stable
 			comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID, opts.IsForcePush)
 			if err == nil && comment != nil {
 				notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment)
 			}
+			// The caller can be in a goroutine or a "push queue", "conflict check" can be time-consuming,
+			// and the concurrency should be limited, so the conflict check will be done in another queue
+			StartPullRequestCheckImmediately(ctx, pr)
 		}
 
 		if opts.IsSync {

services/pull/update.go

@@ -63,6 +63,8 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
 	}
 
 	defer func() {
+		// FIXME: This is a duplicated call to AddTestPullRequestTask in case the git push successfully but
+		// the post-receive hook failed. This will ensure the test task is added.
 		go AddTestPullRequestTask(TestPullRequestOptions{
 			RepoID:      pr.BaseRepo.ID,
 			Doer:        doer,

services/repository/push.go

@@ -310,15 +310,16 @@ func pushUpdateBranch(_ context.Context, repo *repo_model.Repository, pusher *us
 	}
 
 	// only update branch can trigger pull request task because the pull request hasn't been created yet when creating a branch
-	go pull_service.AddTestPullRequestTask(pull_service.TestPullRequestOptions{
+	testPullRequestOpts := pull_service.TestPullRequestOptions{
 		RepoID:      repo.ID,
 		Doer:        pusher,
 		Branch:      branch,
 		IsSync:      true,
 		IsForcePush: isForcePush,
 		OldCommitID: opts.OldCommitID,
 		NewCommitID: opts.NewCommitID,
-	})
+	}
+	go pull_service.AddTestPullRequestTask(testPullRequestOpts)
 
 	if isForcePush {
 		log.Trace("Push %s is a force push", opts.NewCommitID)

tests/integration/pull_comment_test.go

@@ -62,7 +62,6 @@ func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) {
 	doGitPushTestRepositoryFail(dstPath, "base-repo", "local-branch/rebase:test-branch/rebase")(t)
 	doGitPushTestRepository(dstPath, "--force", "base-repo", "local-branch/rebase:test-branch/rebase")(t)
 
-	// reload the pr
 	prIssue := testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusMergeable)
 	comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{
 		IssueID: prIssue.ID,