From 23ed657872dd710235526b15fd51d30a74e7a152 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 30 Sep 2025 23:39:45 -0700 Subject: [PATCH 1/7] Add tests --- routers/web/repo/pull.go | 13 +- services/pull/comment.go | 1 + services/pull/pull.go | 11 ++ .../git_helper_for_declarative_test.go | 15 +++ tests/integration/pull_comment_test.go | 120 ++++++++++++++++++ 5 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 tests/integration/pull_comment_test.go diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 7970c92ebb52f..94fe7db788ad4 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1583,7 +1583,10 @@ func UpdatePullRequestTarget(ctx *context.Context) { } if err := pull_service.ChangeTargetBranch(ctx, pr, ctx.Doer, targetBranch); err != nil { - if issues_model.IsErrPullRequestAlreadyExists(err) { + switch { + case git_model.IsErrBranchNotExist(err): + ctx.HTTPError(http.StatusBadRequest) + case issues_model.IsErrPullRequestAlreadyExists(err): err := err.(issues_model.ErrPullRequestAlreadyExists) RepoRelPath := ctx.Repo.Owner.Name + "/" + ctx.Repo.Repository.Name @@ -1594,7 +1597,7 @@ func UpdatePullRequestTarget(ctx *context.Context) { "error": err.Error(), "user_error": errorMessage, }) - } else if issues_model.IsErrIssueIsClosed(err) { + case issues_model.IsErrIssueIsClosed(err): errorMessage := ctx.Tr("repo.pulls.is_closed") ctx.Flash.Error(errorMessage) @@ -1602,7 +1605,7 @@ func UpdatePullRequestTarget(ctx *context.Context) { "error": err.Error(), "user_error": errorMessage, }) - } else if pull_service.IsErrPullRequestHasMerged(err) { + case pull_service.IsErrPullRequestHasMerged(err): errorMessage := ctx.Tr("repo.pulls.has_merged") ctx.Flash.Error(errorMessage) @@ -1610,7 +1613,7 @@ func UpdatePullRequestTarget(ctx *context.Context) { "error": err.Error(), "user_error": errorMessage, }) - } else if git_model.IsErrBranchesEqual(err) { + case git_model.IsErrBranchesEqual(err): errorMessage := ctx.Tr("repo.pulls.nothing_to_compare") ctx.Flash.Error(errorMessage) @@ -1618,7 +1621,7 @@ func UpdatePullRequestTarget(ctx *context.Context) { "error": err.Error(), "user_error": errorMessage, }) - } else { + default: ctx.ServerError("UpdatePullRequestTarget", err) } return diff --git a/services/pull/comment.go b/services/pull/comment.go index f12edaf0326b0..ca156d3d924fb 100644 --- a/services/pull/comment.go +++ b/services/pull/comment.go @@ -63,6 +63,7 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss var data issues_model.PushActionContent if opts.IsForcePush { data.CommitIDs = []string{oldCommitID, newCommitID} + data.IsForcePush = true } else { data.CommitIDs, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch) if err != nil { diff --git a/services/pull/pull.go b/services/pull/pull.go index 7bf13733b2727..17349bb360aa6 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -243,6 +243,17 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer } } + exist, err := git_model.IsBranchExist(ctx, pr.BaseRepoID, targetBranch) + if err != nil { + return err + } + if !exist { + return git_model.ErrBranchNotExist{ + RepoID: pr.BaseRepoID, + BranchName: targetBranch, + } + } + // Check if branches are equal branchesEqual, err := IsHeadEqualWithBranch(ctx, pr, targetBranch) if err != nil { diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index 91c30f7278a3b..1f1b583f146cf 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -205,3 +205,18 @@ func doGitPull(dstPath string, args ...string) func(*testing.T) { assert.NoError(t, err) } } + +func doGitCommit(dstPath, commitMessage string) func(*testing.T) { + return func(t *testing.T) { + signature := git.Signature{ + Email: "test@test.test", + Name: "test", + When: time.Now(), + } + assert.NoError(t, git.CommitChanges(t.Context(), dstPath, git.CommitChangesOptions{ + Committer: &signature, + Author: &signature, + Message: commitMessage, + })) + } +} diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go new file mode 100644 index 0000000000000..6b24de6d84d82 --- /dev/null +++ b/tests/integration/pull_comment_test.go @@ -0,0 +1,120 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "fmt" + "net/http" + "net/url" + "os" + "testing" + "time" + + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/git/gitcmd" + issues_service "code.gitea.io/gitea/services/issue" + + "github.com/stretchr/testify/assert" +) + +func TestPull_RebaseComment(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + session := loginUser(t, "user1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") + testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") + + // create a conflict line on user2/repo1:master README.md + testEditFile(t, session, "user2", "repo1", "master", "README.md", "Hello, World (Edited Conflicted)\n") + + // Now the pull request status should be conflicted + prIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "This is a pull title"}) + assert.NoError(t, prIssue.LoadPullRequest(t.Context())) + assert.Equal(t, issues_model.PullRequestStatusConflict, prIssue.PullRequest.Status) + assert.NoError(t, prIssue.PullRequest.LoadBaseRepo(t.Context())) + assert.Equal(t, "user2/repo1", prIssue.PullRequest.BaseRepo.FullName()) + assert.NoError(t, prIssue.PullRequest.LoadHeadRepo(t.Context())) + assert.Equal(t, "user1/repo1", prIssue.PullRequest.HeadRepo.FullName()) + + dstPath := t.TempDir() + u.Path = "/user2/repo1.git" + doGitClone(dstPath, u)(t) + doGitCreateBranch(dstPath, "dev")(t) + content, err := os.ReadFile(dstPath + "/README.md") + assert.NoError(t, err) + assert.Equal(t, "Hello, World (Edited Conflicted)\n", string(content)) + + err = os.WriteFile(dstPath+"/README.md", []byte("Hello, World (Edited Conflict Resolved)\n"), 0o644) + assert.NoError(t, err) + _, _, err = gitcmd.NewCommand().AddArguments("add", "--all").RunStdString(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + doGitCommit(dstPath, "Resolve conflict")(t) + + // do force push + u.Path = "/user1/repo1.git" + u.User = url.UserPassword("user1", userPassword) + doGitAddRemote(dstPath, "fork", u)(t) + // non force push will fail + _, _, err = gitcmd.NewCommand().AddArguments("push", "fork", "dev:master").RunStdString(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.Error(t, err) + _, _, err = gitcmd.NewCommand().AddArguments("push", "--force", "fork", "dev:master").RunStdString(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + time.Sleep(time.Second) // wait for pull request conflict checking + + // reload the pr + prIssue = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "This is a pull title"}) + assert.NoError(t, prIssue.LoadPullRequest(t.Context())) + assert.Equal(t, issues_model.PullRequestStatusMergeable, prIssue.PullRequest.Status) + comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ + IssueID: prIssue.ID, + Type: issues_model.CommentTypeUndefined, // get all comments type + }) + assert.NoError(t, err) + lastComment := comments[len(comments)-1] + err = issues_service.LoadCommentPushCommits(t.Context(), lastComment) + assert.NoError(t, err) + assert.True(t, lastComment.IsForcePush) + }) +} + +func TestPull_RetargetComment(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + session := loginUser(t, "user1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") + testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") + + session2 := loginUser(t, "user2") + // create a non-conflict branch dev from master + testCreateBranch(t, session2, "user2", "repo1", "branch/master", "dev", http.StatusSeeOther) + + // create a conflict line on user2/repo1:master README.md + testEditFile(t, session2, "user2", "repo1", "master", "README.md", "Hello, World (Edited Conflicted)\n") + + // Now the pull request status should be conflicted + prIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "This is a pull title"}) + assert.NoError(t, prIssue.LoadPullRequest(t.Context())) + assert.Equal(t, issues_model.PullRequestStatusConflict, prIssue.PullRequest.Status) + assert.NoError(t, prIssue.PullRequest.LoadBaseRepo(t.Context())) + assert.Equal(t, "user2/repo1", prIssue.PullRequest.BaseRepo.FullName()) + assert.NoError(t, prIssue.PullRequest.LoadHeadRepo(t.Context())) + assert.Equal(t, "user1/repo1", prIssue.PullRequest.HeadRepo.FullName()) + + // do retarget + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/user2/repo1/pull/%d/target_branch", prIssue.PullRequest.Index), map[string]string{ + "_csrf": GetUserCSRFToken(t, session2), + "target_branch": "dev", + }) + session2.MakeRequest(t, req, http.StatusOK) + + time.Sleep(time.Second) // wait for pull request conflict checking + + // reload the pr + prIssue = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "This is a pull title"}) + assert.NoError(t, prIssue.LoadPullRequest(t.Context())) + assert.Equal(t, issues_model.PullRequestStatusMergeable, prIssue.PullRequest.Status) + }) +} From 343ca9c103dd1851ca8acd27df754f3c478b9186 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 1 Oct 2025 09:37:20 -0700 Subject: [PATCH 2/7] Fix test --- models/fixtures/branch.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 03e21d04b45e4..717230149be8a 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -213,3 +213,15 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 26 + repo_id: 10 + name: 'feature/1' + commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d' + commit_message: 'Initial commit' + commit_time: 1489950479 + pusher_id: 2 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 From 2bea40d41517d2dd84fc0cd61a57349d162d7a40 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 14:02:39 +0800 Subject: [PATCH 3/7] fix doGitCommit --- .../git_helper_for_declarative_test.go | 36 ++++++++----------- tests/integration/git_push_test.go | 8 +++-- tests/integration/pull_comment_test.go | 11 +++--- tests/integration/pull_create_test.go | 18 +++++----- 4 files changed, 36 insertions(+), 37 deletions(-) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index 1f1b583f146cf..b9e01f2715fcf 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -160,20 +160,27 @@ func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T } } -func doGitAddSomeCommits(dstPath, branch string) func(*testing.T) { - return func(t *testing.T) { - doGitCheckoutBranch(dstPath, branch)(t) +type localGitAddCommitOptions struct { + LocalRepoPath string + CheckoutBranch string + TreeFilePath string + TreeFileContent string +} - assert.NoError(t, os.WriteFile(filepath.Join(dstPath, fmt.Sprintf("file-%s.txt", branch)), []byte("file "+branch), 0o644)) - assert.NoError(t, git.AddChanges(t.Context(), dstPath, true)) +func doGitCheckoutWriteFileCommit(opts localGitAddCommitOptions) func(*testing.T) { + return func(t *testing.T) { + doGitCheckoutBranch(opts.LocalRepoPath, opts.CheckoutBranch)(t) + localFilePath := filepath.Join(opts.LocalRepoPath, opts.TreeFilePath) + assert.NoError(t, os.WriteFile(localFilePath, []byte(opts.TreeFileContent), 0o644)) + assert.NoError(t, git.AddChanges(t.Context(), opts.LocalRepoPath, true)) signature := git.Signature{ Email: "test@test.test", Name: "test", } - assert.NoError(t, git.CommitChanges(t.Context(), dstPath, git.CommitChangesOptions{ + assert.NoError(t, git.CommitChanges(t.Context(), opts.LocalRepoPath, git.CommitChangesOptions{ Committer: &signature, Author: &signature, - Message: "update " + branch, + Message: fmt.Sprintf("update %s @ %s", opts.TreeFilePath, opts.CheckoutBranch), })) } } @@ -205,18 +212,3 @@ func doGitPull(dstPath string, args ...string) func(*testing.T) { assert.NoError(t, err) } } - -func doGitCommit(dstPath, commitMessage string) func(*testing.T) { - return func(t *testing.T) { - signature := git.Signature{ - Email: "test@test.test", - Name: "test", - When: time.Now(), - } - assert.NoError(t, git.CommitChanges(t.Context(), dstPath, git.CommitChangesOptions{ - Committer: &signature, - Author: &signature, - Message: commitMessage, - })) - } -} diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go index b02387c3b98e9..a51bf5730fd92 100644 --- a/tests/integration/git_push_test.go +++ b/tests/integration/git_push_test.go @@ -58,8 +58,12 @@ func testGitPush(t *testing.T, u *url.URL) { for i := range 5 { branchName := fmt.Sprintf("branch-%d", i) pushed = append(pushed, branchName) - - doGitAddSomeCommits(gitPath, branchName)(t) + doGitCheckoutWriteFileCommit(localGitAddCommitOptions{ + LocalRepoPath: gitPath, + CheckoutBranch: branchName, + TreeFilePath: fmt.Sprintf("file-%s.txt", branchName), + TreeFileContent: "file " + branchName, + })(t) } for i := 5; i < 10; i++ { diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go index 6b24de6d84d82..302dc09bfc7e6 100644 --- a/tests/integration/pull_comment_test.go +++ b/tests/integration/pull_comment_test.go @@ -46,11 +46,12 @@ func TestPull_RebaseComment(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "Hello, World (Edited Conflicted)\n", string(content)) - err = os.WriteFile(dstPath+"/README.md", []byte("Hello, World (Edited Conflict Resolved)\n"), 0o644) - assert.NoError(t, err) - _, _, err = gitcmd.NewCommand().AddArguments("add", "--all").RunStdString(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - doGitCommit(dstPath, "Resolve conflict")(t) + doGitCheckoutWriteFileCommit(localGitAddCommitOptions{ + LocalRepoPath: dstPath, + CheckoutBranch: "dev", + TreeFilePath: "README.md", + TreeFileContent: "Hello, World (Edited Conflict Resolved)\n", + })(t) // do force push u.Path = "/user1/repo1.git" diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 18f7e80e8276f..2eb5e94cf9d98 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -260,14 +260,16 @@ func TestCreateAgitPullWithReadPermission(t *testing.T) { u.Path = "user2/repo1.git" u.User = url.UserPassword("user4", userPassword) - t.Run("Clone", doGitClone(dstPath, u)) - - t.Run("add commit", doGitAddSomeCommits(dstPath, "master")) - - t.Run("do agit pull create", func(t *testing.T) { - err := gitcmd.NewCommand("push", "origin", "HEAD:refs/for/master", "-o").AddDynamicArguments("topic="+"test-topic").Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - }) + doGitClone(dstPath, u)(t) + doGitCheckoutWriteFileCommit(localGitAddCommitOptions{ + LocalRepoPath: dstPath, + CheckoutBranch: "master", + TreeFilePath: "new-file-for-agit.txt", + TreeFileContent: "temp content", + })(t) + + err := gitcmd.NewCommand("push", "origin", "HEAD:refs/for/master", "-o").AddDynamicArguments("topic="+"test-topic").Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) }) } From 28f091d1824bc18ee6565dcff7ff89a5dcdd2005 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 14:12:37 +0800 Subject: [PATCH 4/7] use require for errors --- tests/integration/git_helper_for_declarative_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index b9e01f2715fcf..45c755582ec70 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func withKeyFile(t *testing.T, keyname string, callback func(string)) { @@ -171,13 +172,13 @@ func doGitCheckoutWriteFileCommit(opts localGitAddCommitOptions) func(*testing.T return func(t *testing.T) { doGitCheckoutBranch(opts.LocalRepoPath, opts.CheckoutBranch)(t) localFilePath := filepath.Join(opts.LocalRepoPath, opts.TreeFilePath) - assert.NoError(t, os.WriteFile(localFilePath, []byte(opts.TreeFileContent), 0o644)) - assert.NoError(t, git.AddChanges(t.Context(), opts.LocalRepoPath, true)) + require.NoError(t, os.WriteFile(localFilePath, []byte(opts.TreeFileContent), 0o644)) + require.NoError(t, git.AddChanges(t.Context(), opts.LocalRepoPath, true)) signature := git.Signature{ Email: "test@test.test", Name: "test", } - assert.NoError(t, git.CommitChanges(t.Context(), opts.LocalRepoPath, git.CommitChangesOptions{ + require.NoError(t, git.CommitChanges(t.Context(), opts.LocalRepoPath, git.CommitChangesOptions{ Committer: &signature, Author: &signature, Message: fmt.Sprintf("update %s @ %s", opts.TreeFilePath, opts.CheckoutBranch), From c5c1d5b6f57be69eedd5ed64ec2580c032dddc68 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 15:13:02 +0800 Subject: [PATCH 5/7] fix --- tests/integration/pull_comment_test.go | 175 ++++++++++++------------- 1 file changed, 82 insertions(+), 93 deletions(-) diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go index 302dc09bfc7e6..d29431b6c102f 100644 --- a/tests/integration/pull_comment_test.go +++ b/tests/integration/pull_comment_test.go @@ -13,109 +13,98 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/git/gitcmd" issues_service "code.gitea.io/gitea/services/issue" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestPull_RebaseComment(t *testing.T) { - onGiteaRun(t, func(t *testing.T, u *url.URL) { - session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") - testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") - testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") - - // create a conflict line on user2/repo1:master README.md - testEditFile(t, session, "user2", "repo1", "master", "README.md", "Hello, World (Edited Conflicted)\n") - - // Now the pull request status should be conflicted - prIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "This is a pull title"}) - assert.NoError(t, prIssue.LoadPullRequest(t.Context())) - assert.Equal(t, issues_model.PullRequestStatusConflict, prIssue.PullRequest.Status) - assert.NoError(t, prIssue.PullRequest.LoadBaseRepo(t.Context())) - assert.Equal(t, "user2/repo1", prIssue.PullRequest.BaseRepo.FullName()) - assert.NoError(t, prIssue.PullRequest.LoadHeadRepo(t.Context())) - assert.Equal(t, "user1/repo1", prIssue.PullRequest.HeadRepo.FullName()) - - dstPath := t.TempDir() - u.Path = "/user2/repo1.git" - doGitClone(dstPath, u)(t) - doGitCreateBranch(dstPath, "dev")(t) - content, err := os.ReadFile(dstPath + "/README.md") - assert.NoError(t, err) - assert.Equal(t, "Hello, World (Edited Conflicted)\n", string(content)) - - doGitCheckoutWriteFileCommit(localGitAddCommitOptions{ - LocalRepoPath: dstPath, - CheckoutBranch: "dev", - TreeFilePath: "README.md", - TreeFileContent: "Hello, World (Edited Conflict Resolved)\n", - })(t) - - // do force push - u.Path = "/user1/repo1.git" - u.User = url.UserPassword("user1", userPassword) - doGitAddRemote(dstPath, "fork", u)(t) - // non force push will fail - _, _, err = gitcmd.NewCommand().AddArguments("push", "fork", "dev:master").RunStdString(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) - assert.Error(t, err) - _, _, err = gitcmd.NewCommand().AddArguments("push", "--force", "fork", "dev:master").RunStdString(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - - time.Sleep(time.Second) // wait for pull request conflict checking - - // reload the pr - prIssue = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "This is a pull title"}) - assert.NoError(t, prIssue.LoadPullRequest(t.Context())) - assert.Equal(t, issues_model.PullRequestStatusMergeable, prIssue.PullRequest.Status) - comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{ - IssueID: prIssue.ID, - Type: issues_model.CommentTypeUndefined, // get all comments type - }) - assert.NoError(t, err) - lastComment := comments[len(comments)-1] - err = issues_service.LoadCommentPushCommits(t.Context(), lastComment) - assert.NoError(t, err) - assert.True(t, lastComment.IsForcePush) +func testWaitForPullRequestStatus(t *testing.T, prIssue *issues_model.Issue, expectedStatus issues_model.PullRequestStatus) (retIssue *issues_model.Issue) { + require.Eventually(t, func() bool { + prIssueCond := *prIssue + retIssue = unittest.AssertExistsAndLoadBean(t, &prIssueCond) + require.NoError(t, retIssue.LoadPullRequest(t.Context())) + return retIssue.PullRequest.Status == expectedStatus + }, 5*time.Second, 20*time.Millisecond) + return retIssue +} + +func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) { + testPRTitle := "Test PR for rebase comment" + testEditFile(t, session, "user1", "repo1", "test-branch/rebase", "README.md", "Hello, World (Edited)\n") + testPullCreate(t, session, "user1", "repo1", false, "test-branch/rebase", "test-branch/rebase", testPRTitle) + + // create a conflict line on user2/repo1:test-branch/rebase README.md + testEditFile(t, session, "user2", "repo1", "test-branch/rebase", "README.md", "Hello, World (Edited Conflicted)\n") + + // Now the pull request status should be conflicted + testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusConflict) + + dstPath := t.TempDir() + u.Path = "/user2/repo1.git" + doGitClone(dstPath, u)(t) + doGitCheckoutBranch(dstPath, "test-branch/rebase")(t) + doGitCreateBranch(dstPath, "local-branch/rebase")(t) + content, _ := os.ReadFile(dstPath + "/README.md") + require.Equal(t, "Hello, World (Edited Conflicted)\n", string(content)) + + doGitCheckoutWriteFileCommit(localGitAddCommitOptions{ + LocalRepoPath: dstPath, + CheckoutBranch: "local-branch/rebase", + TreeFilePath: "README.md", + TreeFileContent: "Hello, World (Edited Conflict Resolved)\n", + })(t) + + // do force push + u.Path = "/user1/repo1.git" + u.User = url.UserPassword("user1", userPassword) + doGitAddRemote(dstPath, "base-repo", u)(t) + 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, + Type: issues_model.CommentTypeUndefined, // get all comments type + }) + require.NoError(t, err) + lastComment := comments[len(comments)-1] + assert.NoError(t, issues_service.LoadCommentPushCommits(t.Context(), lastComment)) + assert.True(t, lastComment.IsForcePush) +} + +func testPullCommentRetarget(t *testing.T, u *url.URL, session *TestSession) { + testPRTitle := "Test PR for retarget comment" + + // create a non-conflict branch dev from master + testCreateBranch(t, session, "user2", "repo1", "branch/test-branch/retarget", "test-branch/retarget-no-conflict", http.StatusSeeOther) + // make a change on forked branch + testEditFile(t, session, "user1", "repo1", "test-branch/retarget", "README.md", "Hello, World (Edited)\n") + testPullCreate(t, session, "user1", "repo1", false, "test-branch/retarget", "test-branch/retarget", testPRTitle) + // create a conflict line on user2/repo1 README.md + testEditFile(t, session, "user2", "repo1", "test-branch/retarget", "README.md", "Hello, World (Edited Conflicted)\n") + + // Now the pull request status should be conflicted + prIssue := testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusConflict) + + // do retarget + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/user2/repo1/pull/%d/target_branch", prIssue.PullRequest.Index), map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "target_branch": "test-branch/retarget-no-conflict", }) + session.MakeRequest(t, req, http.StatusOK) + testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusMergeable) } -func TestPull_RetargetComment(t *testing.T) { +func TestPullComment(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") + testCreateBranch(t, session, "user2", "repo1", "branch/master", "test-branch/rebase", http.StatusSeeOther) + testCreateBranch(t, session, "user2", "repo1", "branch/master", "test-branch/retarget", http.StatusSeeOther) testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") - testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") - testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") - - session2 := loginUser(t, "user2") - // create a non-conflict branch dev from master - testCreateBranch(t, session2, "user2", "repo1", "branch/master", "dev", http.StatusSeeOther) - - // create a conflict line on user2/repo1:master README.md - testEditFile(t, session2, "user2", "repo1", "master", "README.md", "Hello, World (Edited Conflicted)\n") - - // Now the pull request status should be conflicted - prIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "This is a pull title"}) - assert.NoError(t, prIssue.LoadPullRequest(t.Context())) - assert.Equal(t, issues_model.PullRequestStatusConflict, prIssue.PullRequest.Status) - assert.NoError(t, prIssue.PullRequest.LoadBaseRepo(t.Context())) - assert.Equal(t, "user2/repo1", prIssue.PullRequest.BaseRepo.FullName()) - assert.NoError(t, prIssue.PullRequest.LoadHeadRepo(t.Context())) - assert.Equal(t, "user1/repo1", prIssue.PullRequest.HeadRepo.FullName()) - - // do retarget - req := NewRequestWithValues(t, "POST", fmt.Sprintf("/user2/repo1/pull/%d/target_branch", prIssue.PullRequest.Index), map[string]string{ - "_csrf": GetUserCSRFToken(t, session2), - "target_branch": "dev", - }) - session2.MakeRequest(t, req, http.StatusOK) - - time.Sleep(time.Second) // wait for pull request conflict checking - - // reload the pr - prIssue = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "This is a pull title"}) - assert.NoError(t, prIssue.LoadPullRequest(t.Context())) - assert.Equal(t, issues_model.PullRequestStatusMergeable, prIssue.PullRequest.Status) + + t.Run("RebaseComment", func(t *testing.T) { testPullCommentRebase(t, u, session) }) + t.Run("RetargetComment", func(t *testing.T) { testPullCommentRetarget(t, u, session) }) }) } From 30d77cdcd1845a7162892e6c1fea5d28792e9b1b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 2 Oct 2025 15:19:37 +0800 Subject: [PATCH 6/7] fix comment --- tests/integration/pull_comment_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go index d29431b6c102f..abab65247ba74 100644 --- a/tests/integration/pull_comment_test.go +++ b/tests/integration/pull_comment_test.go @@ -31,10 +31,10 @@ func testWaitForPullRequestStatus(t *testing.T, prIssue *issues_model.Issue, exp func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) { testPRTitle := "Test PR for rebase comment" + // make a change on forked branch testEditFile(t, session, "user1", "repo1", "test-branch/rebase", "README.md", "Hello, World (Edited)\n") testPullCreate(t, session, "user1", "repo1", false, "test-branch/rebase", "test-branch/rebase", testPRTitle) - - // create a conflict line on user2/repo1:test-branch/rebase README.md + // create a conflict on base repo branch testEditFile(t, session, "user2", "repo1", "test-branch/rebase", "README.md", "Hello, World (Edited Conflicted)\n") // Now the pull request status should be conflicted @@ -76,8 +76,7 @@ func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) { func testPullCommentRetarget(t *testing.T, u *url.URL, session *TestSession) { testPRTitle := "Test PR for retarget comment" - - // create a non-conflict branch dev from master + // keep a non-conflict branch testCreateBranch(t, session, "user2", "repo1", "branch/test-branch/retarget", "test-branch/retarget-no-conflict", http.StatusSeeOther) // make a change on forked branch testEditFile(t, session, "user1", "repo1", "test-branch/retarget", "README.md", "Hello, World (Edited)\n") From 9b148f2589fcaedf11ee530e70b645acb7082078 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 2 Oct 2025 13:28:35 -0700 Subject: [PATCH 7/7] return json --- routers/web/repo/pull.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 94fe7db788ad4..22e7cefe4ae9a 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1585,7 +1585,13 @@ func UpdatePullRequestTarget(ctx *context.Context) { if err := pull_service.ChangeTargetBranch(ctx, pr, ctx.Doer, targetBranch); err != nil { switch { case git_model.IsErrBranchNotExist(err): - ctx.HTTPError(http.StatusBadRequest) + errorMessage := ctx.Tr("form.target_branch_not_exist") + + ctx.Flash.Error(errorMessage) + ctx.JSON(http.StatusBadRequest, map[string]any{ + "error": err.Error(), + "user_error": errorMessage, + }) case issues_model.IsErrPullRequestAlreadyExists(err): err := err.(issues_model.ErrPullRequestAlreadyExists)