From 2a6c3e70f9091c860a0aada8c8221491cabec39a Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 18 Feb 2026 05:29:47 +0100 Subject: [PATCH 1/2] Update services/pull/comment.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: silverwind --- services/pull/comment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/comment.go b/services/pull/comment.go index 0ad0419441..c6a4ea5d50 100644 --- a/services/pull/comment.go +++ b/services/pull/comment.go @@ -92,7 +92,7 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss } } - if isForcePush { // if it's a force push, we needs to add a force push comment + if isForcePush { // if it's a force push, we need to add a force push comment data.CommitIDs = []string{oldCommitID, newCommitID} data.IsForcePush = true dataJSON, err := json.Marshal(data) From 02241a94dfc6261479ed5b1f81b1f12ff35e51ed Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 19 Feb 2026 11:48:22 -0800 Subject: [PATCH 2/2] Update services/pull/comment_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lunny Xiao --- services/pull/comment_test.go | 213 +++++++++++++++++++++++----------- 1 file changed, 146 insertions(+), 67 deletions(-) diff --git a/services/pull/comment_test.go b/services/pull/comment_test.go index a91bb6f00d..a01fe3c524 100644 --- a/services/pull/comment_test.go +++ b/services/pull/comment_test.go @@ -16,79 +16,158 @@ import ( ) func TestCreatePushPullCommentForcePushDeletesOldComments(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) + t.Run("base-branch-only", func(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) - assert.NoError(t, pr.LoadIssue(t.Context())) - assert.NoError(t, pr.LoadBaseRepo(t.Context())) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pr.LoadIssue(t.Context())) + assert.NoError(t, pr.LoadBaseRepo(t.Context())) - pusher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + pusher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - _, err := issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{ - Type: issues_model.CommentTypePullRequestPush, - Doer: pusher, - Repo: pr.BaseRepo, - Issue: pr.Issue, - Content: "{}", - }) - assert.NoError(t, err) - _, err = issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{ - Type: issues_model.CommentTypePullRequestPush, - Doer: pusher, - Repo: pr.BaseRepo, - Issue: pr.Issue, - Content: "{}", - }) - assert.NoError(t, err) - - comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ - IssueID: pr.IssueID, - Type: issues_model.CommentTypePullRequestPush, - }) - assert.NoError(t, err) - assert.Len(t, comments, 2) - - gitRepo, err := gitrepo.OpenRepository(t.Context(), pr.BaseRepo) - assert.NoError(t, err) - defer gitRepo.Close() - - headCommit, err := gitRepo.GetBranchCommit(pr.BaseBranch) - assert.NoError(t, err) - oldCommit := headCommit - if headCommit.ParentCount() > 0 { - parentCommit, err := headCommit.Parent(0) + _, err := issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypePullRequestPush, + Doer: pusher, + Repo: pr.BaseRepo, + Issue: pr.Issue, + Content: "{}", + }) + assert.NoError(t, err) + _, err = issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypePullRequestPush, + Doer: pusher, + Repo: pr.BaseRepo, + Issue: pr.Issue, + Content: "{}", + }) assert.NoError(t, err) - oldCommit = parentCommit - } - comment, err := CreatePushPullComment(t.Context(), pusher, pr, oldCommit.ID.String(), headCommit.ID.String(), true) - assert.NoError(t, err) - assert.NotNil(t, comment) - var createdData issues_model.PushActionContent - assert.NoError(t, json.Unmarshal([]byte(comment.Content), &createdData)) - assert.True(t, createdData.IsForcePush) + comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ + IssueID: pr.IssueID, + Type: issues_model.CommentTypePullRequestPush, + }) + assert.NoError(t, err) + assert.Len(t, comments, 2) - commits, err := gitRepo.CommitsBetweenNotBase(headCommit, oldCommit, pr.BaseBranch) - assert.NoError(t, err) - expectedCount := 1 - if len(commits) > 0 { - expectedCount = 2 - } + gitRepo, err := gitrepo.OpenRepository(t.Context(), pr.BaseRepo) + assert.NoError(t, err) + defer gitRepo.Close() - comments, err = issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ - IssueID: pr.IssueID, - Type: issues_model.CommentTypePullRequestPush, - }) - assert.NoError(t, err) - assert.Len(t, comments, expectedCount) - - forcePushCount := 0 - for _, comment := range comments { - var pushData issues_model.PushActionContent - assert.NoError(t, json.Unmarshal([]byte(comment.Content), &pushData)) - if pushData.IsForcePush { - forcePushCount++ + headCommit, err := gitRepo.GetBranchCommit(pr.BaseBranch) + assert.NoError(t, err) + oldCommit := headCommit + if headCommit.ParentCount() > 0 { + parentCommit, err := headCommit.Parent(0) + assert.NoError(t, err) + oldCommit = parentCommit } - } - assert.Equal(t, 1, forcePushCount) + + comment, err := CreatePushPullComment(t.Context(), pusher, pr, oldCommit.ID.String(), headCommit.ID.String(), true) + assert.NoError(t, err) + assert.NotNil(t, comment) + var createdData issues_model.PushActionContent + assert.NoError(t, json.Unmarshal([]byte(comment.Content), &createdData)) + assert.True(t, createdData.IsForcePush) + + // When both commits are on the base branch, CommitsBetweenNotBase should + // typically return no commits, so only the force-push comment is expected. + commits, err := gitRepo.CommitsBetweenNotBase(headCommit, oldCommit, pr.BaseBranch) + assert.NoError(t, err) + assert.Len(t, commits, 0) + + comments, err = issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ + IssueID: pr.IssueID, + Type: issues_model.CommentTypePullRequestPush, + }) + assert.NoError(t, err) + assert.Len(t, comments, 1) + + forcePushCount := 0 + for _, comment := range comments { + var pushData issues_model.PushActionContent + assert.NoError(t, json.Unmarshal([]byte(comment.Content), &pushData)) + if pushData.IsForcePush { + forcePushCount++ + } + } + assert.Equal(t, 1, forcePushCount) + }) + + t.Run("head-vs-base-branch", func(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2}) + assert.NoError(t, pr.LoadIssue(t.Context())) + assert.NoError(t, pr.LoadBaseRepo(t.Context())) + + pusher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + _, err := issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypePullRequestPush, + Doer: pusher, + Repo: pr.BaseRepo, + Issue: pr.Issue, + Content: "{}", + }) + assert.NoError(t, err) + _, err = issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{ + Type: issues_model.CommentTypePullRequestPush, + Doer: pusher, + Repo: pr.BaseRepo, + Issue: pr.Issue, + Content: "{}", + }) + assert.NoError(t, err) + + comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ + IssueID: pr.IssueID, + Type: issues_model.CommentTypePullRequestPush, + }) + assert.NoError(t, err) + assert.Len(t, comments, 2) + + gitRepo, err := gitrepo.OpenRepository(t.Context(), pr.BaseRepo) + assert.NoError(t, err) + defer gitRepo.Close() + + // In this subtest, use the head branch for the new commit and the base branch + // for the old commit so that CommitsBetweenNotBase returns non-empty results. + headCommit, err := gitRepo.GetBranchCommit(pr.HeadBranch) + assert.NoError(t, err) + + baseCommit, err := gitRepo.GetBranchCommit(pr.BaseBranch) + assert.NoError(t, err) + oldCommit := baseCommit + + comment, err := CreatePushPullComment(t.Context(), pusher, pr, oldCommit.ID.String(), headCommit.ID.String(), true) + assert.NoError(t, err) + assert.NotNil(t, comment) + var createdData issues_model.PushActionContent + assert.NoError(t, json.Unmarshal([]byte(comment.Content), &createdData)) + assert.True(t, createdData.IsForcePush) + + commits, err := gitRepo.CommitsBetweenNotBase(headCommit, oldCommit, pr.BaseBranch) + assert.NoError(t, err) + // For this scenario we expect at least one commit between head and base + // that is not on the base branch, so data.CommitIDs should be non-empty. + assert.Greater(t, len(commits), 0) + + comments, err = issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ + IssueID: pr.IssueID, + Type: issues_model.CommentTypePullRequestPush, + }) + assert.NoError(t, err) + // Two comments should exist now: one regular push comment and one force-push comment. + assert.Len(t, comments, 2) + + forcePushCount := 0 + for _, comment := range comments { + var pushData issues_model.PushActionContent + assert.NoError(t, json.Unmarshal([]byte(comment.Content), &pushData)) + if pushData.IsForcePush { + forcePushCount++ + } + } + assert.Equal(t, 1, forcePushCount) + }) }