From dda296addc3c59afdece4f2c0f3f995c91442992 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 26 Sep 2025 20:37:13 -0700 Subject: [PATCH 01/12] Fix lint --- modules/git/diff.go | 27 +- modules/git/git.go | 2 + modules/gitrepo/fetch.go | 18 + modules/gitrepo/merge.go | 54 ++ routers/private/hook_pre_receive.go | 4 +- services/pull/check.go | 2 +- services/pull/conflicts.go | 141 +++++ services/pull/patch.go | 30 +- services/pull/pull.go | 20 +- .../git_helper_for_declarative_test.go | 15 + tests/integration/pull_commit_test.go | 104 ++++ tests/integration/pull_merge_test.go | 82 --- tests/integration/pull_status_test.go | 567 ++++++++++++++---- 13 files changed, 828 insertions(+), 238 deletions(-) create mode 100644 modules/gitrepo/fetch.go create mode 100644 modules/gitrepo/merge.go create mode 100644 services/pull/conflicts.go diff --git a/modules/git/diff.go b/modules/git/diff.go index d185cc9277650..377a845e89401 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -7,6 +7,7 @@ import ( "bufio" "bytes" "context" + "errors" "fmt" "io" "os" @@ -289,20 +290,18 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi } // GetAffectedFiles returns the affected files between two commits -func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID string, env []string) ([]string, error) { - if oldCommitID == emptySha1ObjectID.String() || oldCommitID == emptySha256ObjectID.String() { - startCommitID, err := repo.GetCommitBranchStart(env, branchName, newCommitID) - if err != nil { - return nil, err - } - if startCommitID == "" { - return nil, fmt.Errorf("cannot find the start commit of %s", newCommitID) - } - oldCommitID = startCommitID +func GetAffectedFiles(ctx context.Context, repoPath, oldCommitID, newCommitID string, env []string) ([]string, error) { + if oldCommitID == emptySha1ObjectID.String() { + oldCommitID = emptySha1ObjectID.Type().EmptyTree().String() + } else if oldCommitID == emptySha256ObjectID.String() { + oldCommitID = emptySha256ObjectID.Type().EmptyTree().String() + } else if oldCommitID == "" { + return nil, errors.New("oldCommitID is empty") } + stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { - log.Error("Unable to create os.Pipe for %s", repo.Path) + log.Error("Unable to create os.Pipe for %s", repoPath) return nil, err } defer func() { @@ -314,9 +313,9 @@ func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID str // Run `git diff --name-only` to get the names of the changed files err = gitcmd.NewCommand("diff", "--name-only").AddDynamicArguments(oldCommitID, newCommitID). - Run(repo.Ctx, &gitcmd.RunOpts{ + Run(ctx, &gitcmd.RunOpts{ Env: env, - Dir: repo.Path, + Dir: repoPath, Stdout: stdoutWriter, PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { // Close the writer end of the pipe to begin processing @@ -338,7 +337,7 @@ func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID str }, }) if err != nil { - log.Error("Unable to get affected files for commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.Path, err) + log.Error("Unable to get affected files for commits from %s to %s in %s: %v", oldCommitID, newCommitID, repoPath, err) } return affectedFiles, err diff --git a/modules/git/git.go b/modules/git/git.go index 161fa42196a1d..eeb2598d2ab78 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -31,6 +31,7 @@ type Features struct { SupportHashSha256 bool // >= 2.42, SHA-256 repositories no longer an ‘experimental curiosity’ SupportedObjectFormats []ObjectFormat // sha1, sha256 SupportCheckAttrOnBare bool // >= 2.40 + SupportGitMergeTree bool // >= 2.38 } var defaultFeatures *Features @@ -75,6 +76,7 @@ func loadGitVersionFeatures() (*Features, error) { features.SupportedObjectFormats = append(features.SupportedObjectFormats, Sha256ObjectFormat) } features.SupportCheckAttrOnBare = features.CheckVersionAtLeast("2.40") + features.SupportGitMergeTree = features.CheckVersionAtLeast("2.38") return features, nil } diff --git a/modules/gitrepo/fetch.go b/modules/gitrepo/fetch.go new file mode 100644 index 0000000000000..42c3f57cfca07 --- /dev/null +++ b/modules/gitrepo/fetch.go @@ -0,0 +1,18 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "context" + + "code.gitea.io/gitea/modules/git/gitcmd" +) + +func FetchRemoteCommit(ctx context.Context, repo, remoteRepo Repository, commitID string) error { + _, _, err := gitcmd.NewCommand("fetch", "--no-tags"). + AddDynamicArguments(repoPath(remoteRepo)). + AddDynamicArguments(commitID). + RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath(repo)}) + return err +} diff --git a/modules/gitrepo/merge.go b/modules/gitrepo/merge.go new file mode 100644 index 0000000000000..86f17fc2b67bf --- /dev/null +++ b/modules/gitrepo/merge.go @@ -0,0 +1,54 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "bytes" + "context" + "fmt" + "strings" + + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/log" +) + +func MergeBase(ctx context.Context, repo Repository, commit1, commit2 string) (string, error) { + mergeBase, _, err := gitcmd.NewCommand("merge-base", "--"). + AddDynamicArguments(commit1, commit2). + RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath(repo)}) + if err != nil { + return "", fmt.Errorf("unable to get merge-base of %s and %s: %w", commit1, commit2, err) + } + return strings.TrimSpace(mergeBase), nil +} + +func MergeTree(ctx context.Context, repo Repository, base, ours, theirs string) (string, bool, []string, error) { + cmd := gitcmd.NewCommand("merge-tree", "--write-tree", "-z", "--name-only", "--no-messages") + // https://git-scm.com/docs/git-merge-tree/2.40.0#_mistakes_to_avoid + if git.DefaultFeatures().CheckVersionAtLeast("2.40") && !git.DefaultFeatures().CheckVersionAtLeast("2.41") { + cmd.AddOptionFormat("--merge-base=%s", base) + } + + stdout := &bytes.Buffer{} + gitErr := cmd.AddDynamicArguments(ours, theirs).Run(ctx, &gitcmd.RunOpts{ + Dir: repoPath(repo), + Stdout: stdout, + }) + if gitErr != nil && !gitcmd.IsErrorExitCode(gitErr, 1) { + log.Error("Unable to run merge-tree: %v", gitErr) + return "", false, nil, fmt.Errorf("unable to run merge-tree: %w", gitErr) + } + + // There are two situations that we consider for the output: + // 1. Clean merge and the output is NUL + // 2. Merge conflict and the output is NULNUL + treeOID, conflictedFileInfo, _ := strings.Cut(stdout.String(), "\x00") + if len(conflictedFileInfo) == 0 { + return treeOID, gitcmd.IsErrorExitCode(gitErr, 1), nil, nil + } + + // Remove last NULL-byte from conflicted file info, then split with NULL byte as seperator. + return treeOID, true, strings.Split(conflictedFileInfo[:len(conflictedFileInfo)-1], "\x00"), nil +} diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 99a0b450d701f..8d66f36176caa 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -237,7 +237,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r globs := protectBranch.GetProtectedFilePatterns() if len(globs) > 0 { - _, err := pull_service.CheckFileProtection(gitRepo, branchName, oldCommitID, newCommitID, globs, 1, ctx.env) + _, err := pull_service.CheckFileProtection(ctx, repo.RepoPath(), oldCommitID, newCommitID, globs, 1, ctx.env) if err != nil { if !pull_service.IsErrFilePathProtected(err) { log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) @@ -295,7 +295,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r // Allow commits that only touch unprotected files globs := protectBranch.GetUnprotectedFilePatterns() if len(globs) > 0 { - unprotectedFilesOnly, err := pull_service.CheckUnprotectedFiles(gitRepo, branchName, oldCommitID, newCommitID, globs, ctx.env) + unprotectedFilesOnly, err := pull_service.CheckUnprotectedFiles(ctx, repo, oldCommitID, newCommitID, globs, ctx.env) if err != nil { log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) ctx.JSON(http.StatusInternalServerError, private.Response{ diff --git a/services/pull/check.go b/services/pull/check.go index 088fd3702c643..799cf540c886c 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -437,7 +437,7 @@ func checkPullRequestMergeable(id int64) { return } - if err := testPullRequestBranchMergeable(pr); err != nil { + if err := checkPullRequestMergeableAndUpdateStatus(ctx, pr); err != nil { log.Error("testPullRequestTmpRepoBranchMergeable[%-v]: %v", pr, err) pr.Status = issues_model.PullRequestStatusError if err := pr.UpdateCols(ctx, "status"); err != nil { diff --git a/services/pull/conflicts.go b/services/pull/conflicts.go new file mode 100644 index 0000000000000..edd87fb2d0269 --- /dev/null +++ b/services/pull/conflicts.go @@ -0,0 +1,141 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "context" + "errors" + "fmt" + + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/log" +) + +// checkPullRequestMergeableAndUpdateStatus checks whether a pull request is mergeable and updates its status accordingly. +// It uses 'git merge-tree' if supported by the Git version, otherwise it falls back to using a temporary repository. +// This function updates the pr.Status, pr.MergeBase and pr.ConflictedFiles fields as necessary. +func checkPullRequestMergeableAndUpdateStatus(ctx context.Context, pr *issues_model.PullRequest) error { + if git.DefaultFeatures().SupportGitMergeTree { + return checkPullRequestMergeableAndUpdateStatusMergeTree(ctx, pr) + } + + return checkPullRequestMergeableAndUpdateStatusTmpRepo(ctx, pr) +} + +// checkConflictsMergeTree uses git merge-tree to check for conflicts and if none are found checks if the patch is empty +// return true if there is conflicts otherwise return false +// pr.Status and pr.ConflictedFiles will be updated as necessary +func checkConflictsMergeTree(ctx context.Context, repoPath string, pr *issues_model.PullRequest, baseCommitID string) (bool, error) { + treeHash, conflict, conflictFiles, err := gitrepo.MergeTree(ctx, pr.BaseRepo, pr.MergeBase, baseCommitID, pr.HeadCommitID) + if err != nil { + return false, fmt.Errorf("MergeTree: %w", err) + } + if conflict { + pr.Status = issues_model.PullRequestStatusConflict + pr.ConflictedFiles = conflictFiles + + log.Trace("Found %d files conflicted: %v", len(pr.ConflictedFiles), pr.ConflictedFiles) + return true, nil + } + + // No conflicts were detected, now check if the pull request actually + // contains anything useful via a diff. git-diff-tree(1) with --quiet + // will return exit code 0 if there's no diff and exit code 1 if there's + // a diff. + isEmpty := true + if err = gitcmd.NewCommand("diff-tree", "--quiet").AddDynamicArguments(treeHash, pr.MergeBase). + Run(ctx, &gitcmd.RunOpts{ + Dir: repoPath, + }); err != nil { + if !gitcmd.IsErrorExitCode(err, 1) { + return false, fmt.Errorf("DiffTree: %w", err) + } + isEmpty = false + } + + if isEmpty { + log.Debug("PullRequest[%d]: Patch is empty - ignoring", pr.ID) + pr.Status = issues_model.PullRequestStatusEmpty + } + return false, nil +} + +func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr *issues_model.PullRequest) error { + // 1. Get head commit + if err := pr.LoadHeadRepo(ctx); err != nil { + return err + } + headGitRepo, err := gitrepo.OpenRepository(ctx, pr.HeadRepo) + if err != nil { + return fmt.Errorf("OpenRepository: %w", err) + } + defer headGitRepo.Close() + + if pr.Flow == issues_model.PullRequestFlowGithub { + pr.HeadCommitID, err = headGitRepo.GetRefCommitID(git.BranchPrefix + pr.HeadBranch) + if err != nil { + return fmt.Errorf("GetBranchCommitID: can't find commit ID for head: %w", err) + } + } else if pr.HeadCommitID == "" { + return errors.New("head commit ID is empty for pull request Agit flow") + } + + // 2. Get base commit id + var baseGitRepo *git.Repository + if pr.IsSameRepo() { + baseGitRepo = headGitRepo + } else { + baseGitRepo, err = gitrepo.OpenRepository(ctx, pr.BaseRepo) + if err != nil { + return fmt.Errorf("OpenRepository: %w", err) + } + defer baseGitRepo.Close() + // 2.1. fetch head commit id into the current repository + // it will be checked in 2 weeks by default from git if the pull request created failure. + if err := gitrepo.FetchRemoteCommit(ctx, pr.BaseRepo, pr.HeadRepo, pr.HeadCommitID); err != nil { + return fmt.Errorf("FetchRemoteCommit: %w", err) + } + } + baseCommitID, err := baseGitRepo.GetRefCommitID(git.BranchPrefix + pr.BaseBranch) + if err != nil { + return fmt.Errorf("GetBranchCommitID: can't find commit ID for base: %w", err) + } + + // 3. update merge base + pr.MergeBase, err = gitrepo.MergeBase(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID) + if err != nil { + log.Error("GetMergeBase: %v and can't find commit ID for base: %v", err, baseCommitID) + // FIXME: is this the right thing to do? + pr.MergeBase = baseCommitID + } + + // 4. if base == head, then it's an ancestor + if pr.HeadCommitID == pr.MergeBase { + pr.Status = issues_model.PullRequestStatusAncestor + return nil + } + + // 5. Check for conflicts + conflicted, err := checkConflictsMergeTree(ctx, pr.BaseRepo.RepoPath(), pr, baseCommitID) + if err != nil { + return fmt.Errorf("checkConflictsMergeTree: %w", err) + } + if conflicted || pr.Status == issues_model.PullRequestStatusEmpty { + return nil + } + + // 6. Check for protected files changes + if err = checkPullFilesProtection(ctx, pr, pr.BaseRepo.RepoPath()); err != nil { + return fmt.Errorf("pr.CheckPullFilesProtection(): %v", err) + } + if len(pr.ChangedProtectedFiles) > 0 { + log.Trace("Found %d protected files changed", len(pr.ChangedProtectedFiles)) + } + + pr.Status = issues_model.PullRequestStatusMergeable + return nil +} diff --git a/services/pull/patch.go b/services/pull/patch.go index 674ae01253505..44efdde415683 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -1,5 +1,4 @@ -// Copyright 2019 The Gitea Authors. -// All rights reserved. +// Copyright 2019 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package pull @@ -15,15 +14,14 @@ import ( git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/glob" - "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) @@ -67,10 +65,7 @@ var patchErrorSuffices = []string{ ": does not exist in index", } -func testPullRequestBranchMergeable(pr *issues_model.PullRequest) error { - ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("testPullRequestBranchMergeable: %s", pr)) - defer finished() - +func checkPullRequestMergeableAndUpdateStatusTmpRepo(ctx context.Context, pr *issues_model.PullRequest) error { prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) if err != nil { if !git_model.IsErrBranchNotExist(err) { @@ -80,10 +75,6 @@ func testPullRequestBranchMergeable(pr *issues_model.PullRequest) error { } defer cancel() - return testPullRequestTmpRepoBranchMergeable(ctx, prCtx, pr) -} - -func testPullRequestTmpRepoBranchMergeable(ctx context.Context, prCtx *prTmpRepoContext, pr *issues_model.PullRequest) error { gitRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath) if err != nil { return fmt.Errorf("OpenRepository: %w", err) @@ -115,7 +106,7 @@ func testPullRequestTmpRepoBranchMergeable(ctx context.Context, prCtx *prTmpRepo } // 3. Check for protected files changes - if err = checkPullFilesProtection(ctx, pr, gitRepo); err != nil { + if err = checkPullFilesProtection(ctx, pr, prCtx.tmpBasePath); err != nil { return fmt.Errorf("pr.CheckPullFilesProtection(): %v", err) } @@ -530,11 +521,11 @@ func (err ErrFilePathProtected) Unwrap() error { } // CheckFileProtection check file Protection -func CheckFileProtection(repo *git.Repository, branchName, oldCommitID, newCommitID string, patterns []glob.Glob, limit int, env []string) ([]string, error) { +func CheckFileProtection(ctx context.Context, repoPath, oldCommitID, newCommitID string, patterns []glob.Glob, limit int, env []string) ([]string, error) { if len(patterns) == 0 { return nil, nil } - affectedFiles, err := git.GetAffectedFiles(repo, branchName, oldCommitID, newCommitID, env) + affectedFiles, err := git.GetAffectedFiles(ctx, repoPath, oldCommitID, newCommitID, env) if err != nil { return nil, err } @@ -560,11 +551,11 @@ func CheckFileProtection(repo *git.Repository, branchName, oldCommitID, newCommi } // CheckUnprotectedFiles check if the commit only touches unprotected files -func CheckUnprotectedFiles(repo *git.Repository, branchName, oldCommitID, newCommitID string, patterns []glob.Glob, env []string) (bool, error) { +func CheckUnprotectedFiles(ctx context.Context, repo *repo_model.Repository, oldCommitID, newCommitID string, patterns []glob.Glob, env []string) (bool, error) { if len(patterns) == 0 { return false, nil } - affectedFiles, err := git.GetAffectedFiles(repo, branchName, oldCommitID, newCommitID, env) + affectedFiles, err := git.GetAffectedFiles(ctx, repo.RepoPath(), oldCommitID, newCommitID, env) if err != nil { return false, err } @@ -585,7 +576,8 @@ func CheckUnprotectedFiles(repo *git.Repository, branchName, oldCommitID, newCom } // checkPullFilesProtection check if pr changed protected files and save results -func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository) error { +// repoPath might be a temporary path so that we need to pass it in +func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest, repoPath string) error { if pr.Status == issues_model.PullRequestStatusEmpty { pr.ChangedProtectedFiles = nil return nil @@ -601,7 +593,7 @@ func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest, return nil } - pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.HeadBranch, pr.MergeBase, "tracking", pb.GetProtectedFilePatterns(), 10, os.Environ()) + pr.ChangedProtectedFiles, err = CheckFileProtection(ctx, repoPath, pr.MergeBase, pr.HeadCommitID, pb.GetProtectedFilePatterns(), 10, os.Environ()) if err != nil && !IsErrFilePathProtected(err) { return err } diff --git a/services/pull/pull.go b/services/pull/pull.go index 7bf13733b2727..6f4cb4cca8479 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -30,6 +30,7 @@ import ( "code.gitea.io/gitea/modules/globallock" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/process" repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -86,16 +87,12 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { } } - prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) - if err != nil { - if !git_model.IsErrBranchNotExist(err) { - log.Error("CreateTemporaryRepoForPR %-v: %v", pr, err) - } - return err - } - defer cancel() + ctx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("testPullRequestBranchMergeable: %s", pr)) + defer finished() - if err := testPullRequestTmpRepoBranchMergeable(ctx, prCtx, pr); err != nil { + // the pull request haven't been created + err := checkPullRequestMergeableAndUpdateStatus(ctx, pr) + if err != nil { return err } @@ -275,8 +272,11 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer oldBranch := pr.BaseBranch pr.BaseBranch = targetBranch + ctx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("testPullRequestBranchMergeable: %s", pr)) + defer finished() + // Refresh patch - if err := testPullRequestBranchMergeable(pr); err != nil { + if err := checkPullRequestMergeableAndUpdateStatus(ctx, pr); err != nil { return err } 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_commit_test.go b/tests/integration/pull_commit_test.go index 9f3b1a9ef53d0..4fa8e4d9d24e0 100644 --- a/tests/integration/pull_commit_test.go +++ b/tests/integration/pull_commit_test.go @@ -4,9 +4,18 @@ package integration import ( + "fmt" "net/http" + "net/url" + "path" "testing" + auth_model "code.gitea.io/gitea/models/auth" + git_model "code.gitea.io/gitea/models/git" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/commitstatus" + api "code.gitea.io/gitea/modules/structs" pull_service "code.gitea.io/gitea/services/pull" "code.gitea.io/gitea/tests" @@ -39,3 +48,98 @@ func TestListPullCommits(t *testing.T) { assert.Contains(t, resp.Body.String(), `# repo1`) }) } + +func TestPullCreate_CommitStatus(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + session := loginUser(t, "user1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "status1", "README.md", "status1") + + url := path.Join("user1", "repo1", "compare", "master...status1") + req := NewRequestWithValues(t, "POST", url, + map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "title": "pull request from status1", + }, + ) + session.MakeRequest(t, req, http.StatusOK) + + req = NewRequest(t, "GET", "/user1/repo1/pulls") + resp := session.MakeRequest(t, req, http.StatusOK) + NewHTMLParser(t, resp.Body) + + // Request repository commits page + req = NewRequest(t, "GET", "/user1/repo1/pulls/1/commits") + resp = session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + // Get first commit URL + commitURL, exists := doc.doc.Find("#commits-table tbody tr td.sha a").Last().Attr("href") + assert.True(t, exists) + assert.NotEmpty(t, commitURL) + + commitID := path.Base(commitURL) + + statusList := []commitstatus.CommitStatusState{ + commitstatus.CommitStatusPending, + commitstatus.CommitStatusError, + commitstatus.CommitStatusFailure, + commitstatus.CommitStatusSuccess, + commitstatus.CommitStatusWarning, + } + + statesIcons := map[commitstatus.CommitStatusState]string{ + commitstatus.CommitStatusPending: "octicon-dot-fill", + commitstatus.CommitStatusSuccess: "octicon-check", + commitstatus.CommitStatusError: "gitea-exclamation", + commitstatus.CommitStatusFailure: "octicon-x", + commitstatus.CommitStatusWarning: "gitea-exclamation", + } + + testCtx := NewAPITestContext(t, "user1", "repo1", auth_model.AccessTokenScopeWriteRepository) + + // Update commit status, and check if icon is updated as well + for _, status := range statusList { + // Call API to add status for commit + t.Run("CreateStatus", doAPICreateCommitStatus(testCtx, commitID, api.CreateStatusOption{ + State: status, + TargetURL: "http://test.ci/", + Description: "", + Context: "testci", + })) + + req = NewRequest(t, "GET", "/user1/repo1/pulls/1/commits") + resp = session.MakeRequest(t, req, http.StatusOK) + doc = NewHTMLParser(t, resp.Body) + + commitURL, exists = doc.doc.Find("#commits-table tbody tr td.sha a").Last().Attr("href") + assert.True(t, exists) + assert.NotEmpty(t, commitURL) + assert.Equal(t, commitID, path.Base(commitURL)) + + cls, ok := doc.doc.Find("#commits-table tbody tr td.message .commit-status").Last().Attr("class") + assert.True(t, ok) + assert.Contains(t, cls, statesIcons[status]) + } + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) + css := unittest.AssertExistsAndLoadBean(t, &git_model.CommitStatusSummary{RepoID: repo1.ID, SHA: commitID}) + assert.Equal(t, commitstatus.CommitStatusSuccess, css.State) + }) +} + +func doAPICreateCommitStatus(ctx APITestContext, commitID string, data api.CreateStatusOption) func(*testing.T) { + return func(t *testing.T) { + req := NewRequestWithJSON( + t, + http.MethodPost, + fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s", ctx.Username, ctx.Reponame, commitID), + data, + ).AddTokenAuth(ctx.Token) + if ctx.ExpectedCode != 0 { + ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) + return + } + ctx.Session.MakeRequest(t, req, http.StatusCreated) + } +} diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 72bf8ab4b032d..cbbe6b1c5da87 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -37,9 +37,7 @@ import ( "code.gitea.io/gitea/services/automerge" "code.gitea.io/gitea/services/automergequeue" pull_service "code.gitea.io/gitea/services/pull" - repo_service "code.gitea.io/gitea/services/repository" commitstatus_service "code.gitea.io/gitea/services/repository/commitstatus" - files_service "code.gitea.io/gitea/services/repository/files" "github.com/stretchr/testify/assert" ) @@ -457,86 +455,6 @@ func TestCantFastForwardOnlyMergeDiverging(t *testing.T) { }) } -func TestConflictChecking(t *testing.T) { - onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create new clean repo to test conflict checking. - baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ - Name: "conflict-checking", - Description: "Tempo repo", - AutoInit: true, - Readme: "Default", - DefaultBranch: "main", - }) - assert.NoError(t, err) - assert.NotEmpty(t, baseRepo) - - // create a commit on new branch. - _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "important_file", - ContentReader: strings.NewReader("Just a non-important file"), - }, - }, - Message: "Add a important file", - OldBranch: "main", - NewBranch: "important-secrets", - }) - assert.NoError(t, err) - - // create a commit on main branch. - _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "important_file", - ContentReader: strings.NewReader("Not the same content :P"), - }, - }, - Message: "Add a important file", - OldBranch: "main", - NewBranch: "main", - }) - assert.NoError(t, err) - - // create Pull to merge the important-secrets branch into main branch. - pullIssue := &issues_model.Issue{ - RepoID: baseRepo.ID, - Title: "PR with conflict!", - PosterID: user.ID, - Poster: user, - IsPull: true, - } - - pullRequest := &issues_model.PullRequest{ - HeadRepoID: baseRepo.ID, - BaseRepoID: baseRepo.ID, - HeadBranch: "important-secrets", - BaseBranch: "main", - HeadRepo: baseRepo, - BaseRepo: baseRepo, - Type: issues_model.PullRequestGitea, - } - prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} - err = pull_service.NewPullRequest(t.Context(), prOpts) - assert.NoError(t, err) - - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) - assert.NoError(t, issue.LoadPullRequest(t.Context())) - conflictingPR := issue.PullRequest - - // Ensure conflictedFiles is populated. - assert.Len(t, conflictingPR.ConflictedFiles, 1) - // Check if status is correct. - assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) - // Ensure that mergeable returns false - assert.False(t, conflictingPR.Mergeable(t.Context())) - }) -} - func TestPullRetargetChildOnBranchDelete(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 49326a594aee6..49d8de8c33729 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -7,119 +7,27 @@ import ( "fmt" "net/http" "net/url" + "os" "path" + "path/filepath" "strings" "testing" - auth_model "code.gitea.io/gitea/models/auth" - git_model "code.gitea.io/gitea/models/git" - "code.gitea.io/gitea/models/issues" + issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/commitstatus" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/setting" - api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" - "code.gitea.io/gitea/services/pull" + pull_service "code.gitea.io/gitea/services/pull" + repo_service "code.gitea.io/gitea/services/repository" + files_service "code.gitea.io/gitea/services/repository/files" "github.com/stretchr/testify/assert" ) -func TestPullCreate_CommitStatus(t *testing.T) { - onGiteaRun(t, func(t *testing.T, u *url.URL) { - session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") - testEditFileToNewBranch(t, session, "user1", "repo1", "master", "status1", "README.md", "status1") - - url := path.Join("user1", "repo1", "compare", "master...status1") - req := NewRequestWithValues(t, "POST", url, - map[string]string{ - "_csrf": GetUserCSRFToken(t, session), - "title": "pull request from status1", - }, - ) - session.MakeRequest(t, req, http.StatusOK) - - req = NewRequest(t, "GET", "/user1/repo1/pulls") - resp := session.MakeRequest(t, req, http.StatusOK) - NewHTMLParser(t, resp.Body) - - // Request repository commits page - req = NewRequest(t, "GET", "/user1/repo1/pulls/1/commits") - resp = session.MakeRequest(t, req, http.StatusOK) - doc := NewHTMLParser(t, resp.Body) - - // Get first commit URL - commitURL, exists := doc.doc.Find("#commits-table tbody tr td.sha a").Last().Attr("href") - assert.True(t, exists) - assert.NotEmpty(t, commitURL) - - commitID := path.Base(commitURL) - - statusList := []commitstatus.CommitStatusState{ - commitstatus.CommitStatusPending, - commitstatus.CommitStatusError, - commitstatus.CommitStatusFailure, - commitstatus.CommitStatusSuccess, - commitstatus.CommitStatusWarning, - } - - statesIcons := map[commitstatus.CommitStatusState]string{ - commitstatus.CommitStatusPending: "octicon-dot-fill", - commitstatus.CommitStatusSuccess: "octicon-check", - commitstatus.CommitStatusError: "gitea-exclamation", - commitstatus.CommitStatusFailure: "octicon-x", - commitstatus.CommitStatusWarning: "gitea-exclamation", - } - - testCtx := NewAPITestContext(t, "user1", "repo1", auth_model.AccessTokenScopeWriteRepository) - - // Update commit status, and check if icon is updated as well - for _, status := range statusList { - // Call API to add status for commit - t.Run("CreateStatus", doAPICreateCommitStatus(testCtx, commitID, api.CreateStatusOption{ - State: status, - TargetURL: "http://test.ci/", - Description: "", - Context: "testci", - })) - - req = NewRequest(t, "GET", "/user1/repo1/pulls/1/commits") - resp = session.MakeRequest(t, req, http.StatusOK) - doc = NewHTMLParser(t, resp.Body) - - commitURL, exists = doc.doc.Find("#commits-table tbody tr td.sha a").Last().Attr("href") - assert.True(t, exists) - assert.NotEmpty(t, commitURL) - assert.Equal(t, commitID, path.Base(commitURL)) - - cls, ok := doc.doc.Find("#commits-table tbody tr td.message .commit-status").Last().Attr("class") - assert.True(t, ok) - assert.Contains(t, cls, statesIcons[status]) - } - - repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) - css := unittest.AssertExistsAndLoadBean(t, &git_model.CommitStatusSummary{RepoID: repo1.ID, SHA: commitID}) - assert.Equal(t, commitstatus.CommitStatusSuccess, css.State) - }) -} - -func doAPICreateCommitStatus(ctx APITestContext, commitID string, data api.CreateStatusOption) func(*testing.T) { - return func(t *testing.T) { - req := NewRequestWithJSON( - t, - http.MethodPost, - fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s", ctx.Username, ctx.Reponame, commitID), - data, - ).AddTokenAuth(ctx.Token) - if ctx.ExpectedCode != 0 { - ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) - return - } - ctx.Session.MakeRequest(t, req, http.StatusCreated) - } -} - func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) { // Merge must continue if commits SHA are different, even if content is same // Reason: gitflow and merging master back into develop, where is high possibility, there are no changes @@ -174,16 +82,16 @@ func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) { func TestPullStatusDelayCheck(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { defer test.MockVariableValue(&setting.Repository.PullRequest.DelayCheckForInactiveDays, 1)() - defer test.MockVariableValue(&pull.AddPullRequestToCheckQueue)() + defer test.MockVariableValue(&pull_service.AddPullRequestToCheckQueue)() session := loginUser(t, "user2") - run := func(t *testing.T, fn func(*testing.T)) (issue3 *issues.Issue, checkedPrID int64) { - pull.AddPullRequestToCheckQueue = func(prID int64) { + run := func(t *testing.T, fn func(*testing.T)) (issue3 *issues_model.Issue, checkedPrID int64) { + pull_service.AddPullRequestToCheckQueue = func(prID int64) { checkedPrID = prID } fn(t) - issue3 = unittest.AssertExistsAndLoadBean(t, &issues.Issue{RepoID: 1, Index: 3}) + issue3 = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: 1, Index: 3}) _ = issue3.LoadPullRequest(t.Context()) return issue3, checkedPrID } @@ -201,7 +109,7 @@ func TestPullStatusDelayCheck(t *testing.T) { // PR issue3 is merageable at the beginning issue3, checkedPrID := run(t, func(t *testing.T) {}) - assert.Equal(t, issues.PullRequestStatusMergeable, issue3.PullRequest.Status) + assert.Equal(t, issues_model.PullRequestStatusMergeable, issue3.PullRequest.Status) assert.Zero(t, checkedPrID) assertReloadingInterval(t, "") // the PR is mergeable, so no need to reload the merge box @@ -213,7 +121,7 @@ func TestPullStatusDelayCheck(t *testing.T) { issue3, checkedPrID = run(t, func(t *testing.T) { testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 1") }) - assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) + assert.Equal(t, issues_model.PullRequestStatusChecking, issue3.PullRequest.Status) assert.Zero(t, checkedPrID) assertReloadingInterval(t, "2000") // the PR status is "checking", so try to reload the merge box @@ -222,14 +130,14 @@ func TestPullStatusDelayCheck(t *testing.T) { req := NewRequest(t, "GET", "/user2/repo1/pulls/3") session.MakeRequest(t, req, http.StatusOK) }) - assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) + assert.Equal(t, issues_model.PullRequestStatusChecking, issue3.PullRequest.Status) assert.Equal(t, issue3.PullRequest.ID, checkedPrID) // when base branch changes, still so no real check issue3, checkedPrID = run(t, func(t *testing.T) { testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 2") }) - assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) + assert.Equal(t, issues_model.PullRequestStatusChecking, issue3.PullRequest.Status) assert.Zero(t, checkedPrID) // then allow to check PRs without delay, when base branch changes, the PRs will be checked @@ -237,7 +145,446 @@ func TestPullStatusDelayCheck(t *testing.T) { issue3, checkedPrID = run(t, func(t *testing.T) { testEditFile(t, session, "user2", "repo1", "master", "README.md", "new content 3") }) - assert.Equal(t, issues.PullRequestStatusChecking, issue3.PullRequest.Status) + assert.Equal(t, issues_model.PullRequestStatusChecking, issue3.PullRequest.Status) assert.Equal(t, issue3.PullRequest.ID, checkedPrID) }) } + +func Test_PullRequestStatusChecking_Mergeable(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // create a commit on new branch. + _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Just a non-important file"), + }, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "important-secrets", + }) + assert.NoError(t, err) + + // create Pull to merge the important-secrets branch into main branch. + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "PR with no conflict", + PosterID: user.ID, + Poster: user, + IsPull: true, + } + + pullRequest := &issues_model.PullRequest{ + HeadRepoID: baseRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "important-secrets", + BaseBranch: "main", + HeadRepo: baseRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(t.Context(), prOpts) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with no conflict"}) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Empty(t, conflictingPR.ConflictedFiles) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) + // Ensure that mergeable returns true + assert.True(t, conflictingPR.Mergeable(t.Context())) + }) +} + +func Test_PullRequestStatusChecking_Conflicted(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // create a commit on new branch. + _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Just a non-important file"), + }, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "important-secrets", + }) + assert.NoError(t, err) + + // create a commit on main branch. + _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Not the same content :P"), + }, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "main", + }) + assert.NoError(t, err) + + // create Pull to merge the important-secrets branch into main branch. + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "PR with conflict!", + PosterID: user.ID, + Poster: user, + IsPull: true, + } + + pullRequest := &issues_model.PullRequest{ + HeadRepoID: baseRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "important-secrets", + BaseBranch: "main", + HeadRepo: baseRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(t.Context(), prOpts) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Len(t, conflictingPR.ConflictedFiles, 1) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) + // Ensure that mergeable returns false + assert.False(t, conflictingPR.Mergeable(t.Context())) + }) +} + +func Test_PullRequestStatusCheckingCrossRepo_Mergeable(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user.Name) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + testRepoFork(t, session, baseRepo.OwnerName, baseRepo.Name, "org3", "conflict-checking", "main") + + forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "org3", Name: "conflict-checking"}) + + // create a commit on new branch of forked repository + _, err = files_service.ChangeRepoFiles(t.Context(), forkRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Just a non-important file"), + }, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "important-secrets", + }) + assert.NoError(t, err) + + // create Pull to merge the important-secrets branch into main branch. + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "PR with no conflict", + PosterID: user.ID, + Poster: user, + IsPull: true, + } + + pullRequest := &issues_model.PullRequest{ + HeadRepoID: forkRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "important-secrets", + BaseBranch: "main", + HeadRepo: forkRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + prOpts := &pull_service.NewPullRequestOptions{ + Repo: baseRepo, + Issue: pullIssue, + PullRequest: pullRequest, + } + err = pull_service.NewPullRequest(t.Context(), prOpts) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with no conflict"}) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Empty(t, conflictingPR.ConflictedFiles) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) + // Ensure that mergeable returns true + assert.True(t, conflictingPR.Mergeable(t.Context())) + }) +} + +func Test_PullRequestStatusCheckingCrossRepo_Conflicted(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user.Name) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + testRepoFork(t, session, baseRepo.OwnerName, baseRepo.Name, "org3", "conflict-checking", "main") + + forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "org3", Name: "conflict-checking"}) + + // create a commit on new branch of forked repository + _, err = files_service.ChangeRepoFiles(t.Context(), forkRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Just a non-important file"), + }, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "important-secrets", + }) + assert.NoError(t, err) + + // create a commit on main branch of base repository. + _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Not the same content :P"), + }, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "main", + }) + assert.NoError(t, err) + + // create Pull to merge the important-secrets branch into main branch. + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "PR with conflict!", + PosterID: user.ID, + Poster: user, + IsPull: true, + } + + pullRequest := &issues_model.PullRequest{ + HeadRepoID: forkRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "important-secrets", + BaseBranch: "main", + HeadRepo: forkRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(t.Context(), prOpts) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Len(t, conflictingPR.ConflictedFiles, 1) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) + // Ensure that mergeable returns false + assert.False(t, conflictingPR.Mergeable(t.Context())) + }) +} + +func Test_PullRequest_AGit_StatusChecking_Mergeable(t *testing.T) { + // skip this test if git version is low + if !git.DefaultFeatures().SupportProcReceive { + return + } + + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // add something in local repository and push it to remote + dstPath := t.TempDir() + repoURL := *giteaURL + repoURL.Path = baseRepo.FullName() + ".git" + repoURL.User = url.UserPassword("user2", userPassword) + doGitClone(dstPath, &repoURL)(t) + + gitRepo, err := git.OpenRepository(t.Context(), dstPath) + assert.NoError(t, err) + defer gitRepo.Close() + + doGitCreateBranch(dstPath, "test-agit-push")(t) + + _, err = generateCommitWithNewData(t.Context(), testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") + assert.NoError(t, err) + + // push to create an agit pull request + err = gitcmd.NewCommand("push", "origin", + "-o", "title=agit-test-title", "-o", "description=agit-test-description", + "-o", "topic=head-branch-name", + "HEAD:refs/for/main", + ).Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "agit-test-title", + }) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Empty(t, conflictingPR.ConflictedFiles) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) + // Ensure that mergeable returns true + assert.True(t, conflictingPR.Mergeable(t.Context())) + }) +} + +func Test_PullRequest_AGit_StatusChecking_Conflicted(t *testing.T) { + // skip this test if git version is low + if !git.DefaultFeatures().SupportProcReceive { + return + } + + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // add something in local repository and push it to remote + dstPath := t.TempDir() + repoURL := *giteaURL + repoURL.Path = baseRepo.FullName() + ".git" + repoURL.User = url.UserPassword("user2", userPassword) + doGitClone(dstPath, &repoURL)(t) + + gitRepo, err := git.OpenRepository(t.Context(), dstPath) + assert.NoError(t, err) + defer gitRepo.Close() + + // create agit branch from current commit + doGitCreateBranch(dstPath, "test-agit-push")(t) + + // add something on the same file of main branch so that it causes conflict + doGitCheckoutBranch(dstPath, "main")(t) + + assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte("Some changes to README file to main cause conflict"), 0o644)) + assert.NoError(t, git.AddChanges(t.Context(), dstPath, true)) + doGitCommit(dstPath, "add something to main branch")(t) + + err = gitcmd.NewCommand("push", "origin", "main").Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + // check out back to agit branch and change the same file + doGitCheckoutBranch(dstPath, "test-agit-push")(t) + + assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte("Some changes to README file for agit branch"), 0o644)) + assert.NoError(t, git.AddChanges(t.Context(), dstPath, true)) + doGitCommit(dstPath, "add something to agit branch")(t) + + // push to create an agit pull request + err = gitcmd.NewCommand("push", "origin", + "-o", "title=agit-test-title", "-o", "description=agit-test-description", + "-o", "topic=head-branch-name", + "HEAD:refs/for/main", + ).Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "agit-test-title", + }) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Equal(t, []string{"README.md"}, conflictingPR.ConflictedFiles) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) + // Ensure that mergeable returns false + assert.False(t, conflictingPR.Mergeable(t.Context())) + }) +} From 21cc4aaf072b23db425e7dd0e868fda826113757 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 26 Sep 2025 21:00:31 -0700 Subject: [PATCH 02/12] improvements --- modules/gitrepo/merge.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/gitrepo/merge.go b/modules/gitrepo/merge.go index 86f17fc2b67bf..05f896a6cf49f 100644 --- a/modules/gitrepo/merge.go +++ b/modules/gitrepo/merge.go @@ -19,7 +19,7 @@ func MergeBase(ctx context.Context, repo Repository, commit1, commit2 string) (s AddDynamicArguments(commit1, commit2). RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath(repo)}) if err != nil { - return "", fmt.Errorf("unable to get merge-base of %s and %s: %w", commit1, commit2, err) + return "", fmt.Errorf("get merge-base of %s and %s failed: %w", commit1, commit2, err) } return strings.TrimSpace(mergeBase), nil } @@ -37,8 +37,8 @@ func MergeTree(ctx context.Context, repo Repository, base, ours, theirs string) Stdout: stdout, }) if gitErr != nil && !gitcmd.IsErrorExitCode(gitErr, 1) { - log.Error("Unable to run merge-tree: %v", gitErr) - return "", false, nil, fmt.Errorf("unable to run merge-tree: %w", gitErr) + log.Error("run merge-tree failed: %v", gitErr) + return "", false, nil, fmt.Errorf("run merge-tree failed: %w", gitErr) } // There are two situations that we consider for the output: @@ -49,6 +49,6 @@ func MergeTree(ctx context.Context, repo Repository, base, ours, theirs string) return treeOID, gitcmd.IsErrorExitCode(gitErr, 1), nil, nil } - // Remove last NULL-byte from conflicted file info, then split with NULL byte as seperator. + // Remove last NULL-byte from conflicted file info, then split with NULL byte as separator. return treeOID, true, strings.Split(conflictedFileInfo[:len(conflictedFileInfo)-1], "\x00"), nil } From 2f74aecf2ea1923721057970ab96d9352f05bafb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 26 Sep 2025 21:52:18 -0700 Subject: [PATCH 03/12] remove unused functions --- modules/git/commit_test.go | 16 ---------------- modules/git/repo_commit.go | 32 -------------------------------- 2 files changed, 48 deletions(-) diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index 688b4e294f5bb..9a41b4284c0c5 100644 --- a/modules/git/commit_test.go +++ b/modules/git/commit_test.go @@ -4,7 +4,6 @@ package git import ( - "os" "path/filepath" "strings" "testing" @@ -339,18 +338,3 @@ func TestGetCommitFileStatusMerges(t *testing.T) { assert.Equal(t, expected.Removed, commitFileStatus.Removed) assert.Equal(t, expected.Modified, commitFileStatus.Modified) } - -func Test_GetCommitBranchStart(t *testing.T) { - bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") - repo, err := OpenRepository(t.Context(), bareRepo1Path) - assert.NoError(t, err) - defer repo.Close() - commit, err := repo.GetBranchCommit("branch1") - assert.NoError(t, err) - assert.Equal(t, "2839944139e0de9737a044f78b0e4b40d989a9e3", commit.ID.String()) - - startCommitID, err := repo.GetCommitBranchStart(os.Environ(), "branch1", commit.ID.String()) - assert.NoError(t, err) - assert.NotEmpty(t, startCommitID) - assert.Equal(t, "95bb4d39648ee7e325106df01a621c530863a653", startCommitID) -} diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 6e5911f1dddf5..7b100c4688ea6 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -534,35 +534,3 @@ func (repo *Repository) AddLastCommitCache(cacheKey, fullName, sha string) error } return nil } - -// GetCommitBranchStart returns the commit where the branch diverged -func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID string) (string, error) { - cmd := gitcmd.NewCommand("log", prettyLogFormat) - cmd.AddDynamicArguments(endCommitID) - - stdout, _, runErr := cmd.RunStdBytes(repo.Ctx, &gitcmd.RunOpts{ - Dir: repo.Path, - Env: env, - }) - if runErr != nil { - return "", runErr - } - - parts := bytes.SplitSeq(bytes.TrimSpace(stdout), []byte{'\n'}) - - // check the commits one by one until we find a commit contained by another branch - // and we think this commit is the divergence point - for commitID := range parts { - branches, err := repo.getBranches(env, string(commitID), 2) - if err != nil { - return "", err - } - for _, b := range branches { - if b != branch { - return string(commitID), nil - } - } - } - - return "", nil -} From 3d0222c159237e3c744fb14d66ed25c079058cca Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 26 Sep 2025 22:36:15 -0700 Subject: [PATCH 04/12] allow empty pull request --- services/pull/conflicts.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/pull/conflicts.go b/services/pull/conflicts.go index edd87fb2d0269..423d8431fc22a 100644 --- a/services/pull/conflicts.go +++ b/services/pull/conflicts.go @@ -109,8 +109,8 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * pr.MergeBase, err = gitrepo.MergeBase(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID) if err != nil { log.Error("GetMergeBase: %v and can't find commit ID for base: %v", err, baseCommitID) - // FIXME: is this the right thing to do? - pr.MergeBase = baseCommitID + pr.Status = issues_model.PullRequestStatusEmpty // if there is no merge base, then it's empty but we still need to allow the pull request created + return nil } // 4. if base == head, then it's an ancestor @@ -122,7 +122,8 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * // 5. Check for conflicts conflicted, err := checkConflictsMergeTree(ctx, pr.BaseRepo.RepoPath(), pr, baseCommitID) if err != nil { - return fmt.Errorf("checkConflictsMergeTree: %w", err) + log.Error("checkConflictsMergeTree: %v", err) + pr.Status = issues_model.PullRequestStatusEmpty // if there is no merge base, then it's empty but we still need to allow the pull request created } if conflicted || pr.Status == issues_model.PullRequestStatusEmpty { return nil From 7e973b35d58b295a53fbc77ea73f9484ffbe895f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 27 Sep 2025 18:39:16 -0700 Subject: [PATCH 05/12] improvements --- modules/gitrepo/merge.go | 7 +++++++ services/pull/conflicts.go | 9 +++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/modules/gitrepo/merge.go b/modules/gitrepo/merge.go index 05f896a6cf49f..7fbaa2aa284dc 100644 --- a/modules/gitrepo/merge.go +++ b/modules/gitrepo/merge.go @@ -52,3 +52,10 @@ func MergeTree(ctx context.Context, repo Repository, base, ours, theirs string) // Remove last NULL-byte from conflicted file info, then split with NULL byte as separator. return treeOID, true, strings.Split(conflictedFileInfo[:len(conflictedFileInfo)-1], "\x00"), nil } + +func DiffTree(ctx context.Context, repo Repository, treeHash, mergeBase string) error { + return gitcmd.NewCommand("diff-tree", "--quiet").AddDynamicArguments(treeHash, mergeBase). + Run(ctx, &gitcmd.RunOpts{ + Dir: repoPath(repo), + }) +} diff --git a/services/pull/conflicts.go b/services/pull/conflicts.go index 423d8431fc22a..a2a9414e3e109 100644 --- a/services/pull/conflicts.go +++ b/services/pull/conflicts.go @@ -29,7 +29,7 @@ func checkPullRequestMergeableAndUpdateStatus(ctx context.Context, pr *issues_mo // checkConflictsMergeTree uses git merge-tree to check for conflicts and if none are found checks if the patch is empty // return true if there is conflicts otherwise return false // pr.Status and pr.ConflictedFiles will be updated as necessary -func checkConflictsMergeTree(ctx context.Context, repoPath string, pr *issues_model.PullRequest, baseCommitID string) (bool, error) { +func checkConflictsMergeTree(ctx context.Context, pr *issues_model.PullRequest, baseCommitID string) (bool, error) { treeHash, conflict, conflictFiles, err := gitrepo.MergeTree(ctx, pr.BaseRepo, pr.MergeBase, baseCommitID, pr.HeadCommitID) if err != nil { return false, fmt.Errorf("MergeTree: %w", err) @@ -47,10 +47,7 @@ func checkConflictsMergeTree(ctx context.Context, repoPath string, pr *issues_mo // will return exit code 0 if there's no diff and exit code 1 if there's // a diff. isEmpty := true - if err = gitcmd.NewCommand("diff-tree", "--quiet").AddDynamicArguments(treeHash, pr.MergeBase). - Run(ctx, &gitcmd.RunOpts{ - Dir: repoPath, - }); err != nil { + if err = gitrepo.DiffTree(ctx, pr.BaseRepo, treeHash, pr.MergeBase); err != nil { if !gitcmd.IsErrorExitCode(err, 1) { return false, fmt.Errorf("DiffTree: %w", err) } @@ -120,7 +117,7 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * } // 5. Check for conflicts - conflicted, err := checkConflictsMergeTree(ctx, pr.BaseRepo.RepoPath(), pr, baseCommitID) + conflicted, err := checkConflictsMergeTree(ctx, pr, baseCommitID) if err != nil { log.Error("checkConflictsMergeTree: %v", err) pr.Status = issues_model.PullRequestStatusEmpty // if there is no merge base, then it's empty but we still need to allow the pull request created From 72a154a9b53e933a2448d581219d1d4670910ad8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 29 Sep 2025 14:01:27 -0700 Subject: [PATCH 06/12] fix bug --- services/pull/conflicts.go | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/services/pull/conflicts.go b/services/pull/conflicts.go index a2a9414e3e109..2fa318c51d849 100644 --- a/services/pull/conflicts.go +++ b/services/pull/conflicts.go @@ -18,6 +18,7 @@ import ( // checkPullRequestMergeableAndUpdateStatus checks whether a pull request is mergeable and updates its status accordingly. // It uses 'git merge-tree' if supported by the Git version, otherwise it falls back to using a temporary repository. // This function updates the pr.Status, pr.MergeBase and pr.ConflictedFiles fields as necessary. +// The pull request parameter may not be created yet in the database, so do not assume it has an ID. func checkPullRequestMergeableAndUpdateStatus(ctx context.Context, pr *issues_model.PullRequest) error { if git.DefaultFeatures().SupportGitMergeTree { return checkPullRequestMergeableAndUpdateStatusMergeTree(ctx, pr) @@ -72,15 +73,6 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * } defer headGitRepo.Close() - if pr.Flow == issues_model.PullRequestFlowGithub { - pr.HeadCommitID, err = headGitRepo.GetRefCommitID(git.BranchPrefix + pr.HeadBranch) - if err != nil { - return fmt.Errorf("GetBranchCommitID: can't find commit ID for head: %w", err) - } - } else if pr.HeadCommitID == "" { - return errors.New("head commit ID is empty for pull request Agit flow") - } - // 2. Get base commit id var baseGitRepo *git.Repository if pr.IsSameRepo() { @@ -97,12 +89,30 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * return fmt.Errorf("FetchRemoteCommit: %w", err) } } + + // 3. Get head commit id + if pr.Flow == issues_model.PullRequestFlowGithub { + pr.HeadCommitID, err = headGitRepo.GetRefCommitID(git.BranchPrefix + pr.HeadBranch) + if err != nil { + return fmt.Errorf("GetBranchCommitID: can't find commit ID for head: %w", err) + } + } else { + if pr.ID > 0 { + pr.HeadCommitID, err = baseGitRepo.GetRefCommitID(pr.GetGitHeadRefName()) + if err != nil { + return fmt.Errorf("GetRefCommitID: can't find commit ID for head: %w", err) + } + } else if pr.HeadCommitID == "" { // for new pull request with agit, the head commit id must be provided + return errors.New("head commit ID is empty for pull request Agit flow") + } + } + + // 4. update merge base baseCommitID, err := baseGitRepo.GetRefCommitID(git.BranchPrefix + pr.BaseBranch) if err != nil { return fmt.Errorf("GetBranchCommitID: can't find commit ID for base: %w", err) } - // 3. update merge base pr.MergeBase, err = gitrepo.MergeBase(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID) if err != nil { log.Error("GetMergeBase: %v and can't find commit ID for base: %v", err, baseCommitID) @@ -110,13 +120,13 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * return nil } - // 4. if base == head, then it's an ancestor + // 5. if base == head, then it's an ancestor if pr.HeadCommitID == pr.MergeBase { pr.Status = issues_model.PullRequestStatusAncestor return nil } - // 5. Check for conflicts + // 6. Check for conflicts conflicted, err := checkConflictsMergeTree(ctx, pr, baseCommitID) if err != nil { log.Error("checkConflictsMergeTree: %v", err) @@ -126,7 +136,7 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * return nil } - // 6. Check for protected files changes + // 7. Check for protected files changes if err = checkPullFilesProtection(ctx, pr, pr.BaseRepo.RepoPath()); err != nil { return fmt.Errorf("pr.CheckPullFilesProtection(): %v", err) } From 76da4bf7599e38b96d6ff1469f63f68128a16b2c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 29 Sep 2025 15:54:26 -0700 Subject: [PATCH 07/12] fix bug --- services/pull/conflicts.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/services/pull/conflicts.go b/services/pull/conflicts.go index 2fa318c51d849..f1ddedbd4f26d 100644 --- a/services/pull/conflicts.go +++ b/services/pull/conflicts.go @@ -83,11 +83,6 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * return fmt.Errorf("OpenRepository: %w", err) } defer baseGitRepo.Close() - // 2.1. fetch head commit id into the current repository - // it will be checked in 2 weeks by default from git if the pull request created failure. - if err := gitrepo.FetchRemoteCommit(ctx, pr.BaseRepo, pr.HeadRepo, pr.HeadCommitID); err != nil { - return fmt.Errorf("FetchRemoteCommit: %w", err) - } } // 3. Get head commit id @@ -107,7 +102,15 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * } } - // 4. update merge base + // 4. fetch head commit id into the current repository + // it will be checked in 2 weeks by default from git if the pull request created failure. + if !pr.IsSameRepo() { + if err := gitrepo.FetchRemoteCommit(ctx, pr.BaseRepo, pr.HeadRepo, pr.HeadCommitID); err != nil { + return fmt.Errorf("FetchRemoteCommit: %w", err) + } + } + + // 5. update merge base baseCommitID, err := baseGitRepo.GetRefCommitID(git.BranchPrefix + pr.BaseBranch) if err != nil { return fmt.Errorf("GetBranchCommitID: can't find commit ID for base: %w", err) @@ -120,13 +123,13 @@ func checkPullRequestMergeableAndUpdateStatusMergeTree(ctx context.Context, pr * return nil } - // 5. if base == head, then it's an ancestor + // 6. if base == head, then it's an ancestor if pr.HeadCommitID == pr.MergeBase { pr.Status = issues_model.PullRequestStatusAncestor return nil } - // 6. Check for conflicts + // 7. Check for conflicts conflicted, err := checkConflictsMergeTree(ctx, pr, baseCommitID) if err != nil { log.Error("checkConflictsMergeTree: %v", err) From af799927342d2c761aae0353131a4e4a1696a74d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 3 Oct 2025 18:02:03 -0700 Subject: [PATCH 08/12] add tests for mergeable tmprepo checking --- tests/integration/pull_status_test.go | 878 ++++++++++++++------------ 1 file changed, 482 insertions(+), 396 deletions(-) diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 49d8de8c33729..3cb76f4a4e912 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -150,441 +150,527 @@ func TestPullStatusDelayCheck(t *testing.T) { }) } -func Test_PullRequestStatusChecking_Mergeable(t *testing.T) { +func Test_PullRequestStatusChecking_Mergeable_MergeTree(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create new clean repo to test conflict checking. - baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ - Name: "conflict-checking", - Description: "Tempo repo", - AutoInit: true, - Readme: "Default", - DefaultBranch: "main", - }) - assert.NoError(t, err) - assert.NotEmpty(t, baseRepo) - - // create a commit on new branch. - _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "important_file", - ContentReader: strings.NewReader("Just a non-important file"), - }, + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() + testPullRequestStatusCheckingMergeable(t) + }) +} + +func Test_PullRequestStatusChecking_Mergeable_TmpRepo(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + testPullRequestStatusCheckingMergeable(t) + }) +} + +func testPullRequestStatusCheckingMergeable(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // create a commit on new branch. + _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Just a non-important file"), }, - Message: "Add a important file", - OldBranch: "main", - NewBranch: "important-secrets", - }) - assert.NoError(t, err) - - // create Pull to merge the important-secrets branch into main branch. - pullIssue := &issues_model.Issue{ - RepoID: baseRepo.ID, - Title: "PR with no conflict", - PosterID: user.ID, - Poster: user, - IsPull: true, - } + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "important-secrets", + }) + assert.NoError(t, err) + + // create Pull to merge the important-secrets branch into main branch. + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "PR with no conflict", + PosterID: user.ID, + Poster: user, + IsPull: true, + } - pullRequest := &issues_model.PullRequest{ - HeadRepoID: baseRepo.ID, - BaseRepoID: baseRepo.ID, - HeadBranch: "important-secrets", - BaseBranch: "main", - HeadRepo: baseRepo, - BaseRepo: baseRepo, - Type: issues_model.PullRequestGitea, - } - prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} - err = pull_service.NewPullRequest(t.Context(), prOpts) - assert.NoError(t, err) - - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with no conflict"}) - assert.NoError(t, issue.LoadPullRequest(t.Context())) - conflictingPR := issue.PullRequest - - // Ensure conflictedFiles is populated. - assert.Empty(t, conflictingPR.ConflictedFiles) - // Check if status is correct. - assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) - // Ensure that mergeable returns true - assert.True(t, conflictingPR.Mergeable(t.Context())) + pullRequest := &issues_model.PullRequest{ + HeadRepoID: baseRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "important-secrets", + BaseBranch: "main", + HeadRepo: baseRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(t.Context(), prOpts) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with no conflict"}) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Empty(t, conflictingPR.ConflictedFiles) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) + // Ensure that mergeable returns true + assert.True(t, conflictingPR.Mergeable(t.Context())) +} + +func Test_PullRequestStatusChecking_Conflicted_MergeTree(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() + testPullRequestStatusCheckingConflicted(t) }) } -func Test_PullRequestStatusChecking_Conflicted(t *testing.T) { +func Test_PullRequestStatusChecking_Conflicted_TmpRepo(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create new clean repo to test conflict checking. - baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ - Name: "conflict-checking", - Description: "Tempo repo", - AutoInit: true, - Readme: "Default", - DefaultBranch: "main", - }) - assert.NoError(t, err) - assert.NotEmpty(t, baseRepo) - - // create a commit on new branch. - _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "important_file", - ContentReader: strings.NewReader("Just a non-important file"), - }, + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + testPullRequestStatusCheckingConflicted(t) + }) +} + +func testPullRequestStatusCheckingConflicted(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // create a commit on new branch. + _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Just a non-important file"), }, - Message: "Add a important file", - OldBranch: "main", - NewBranch: "important-secrets", - }) - assert.NoError(t, err) - - // create a commit on main branch. - _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "important_file", - ContentReader: strings.NewReader("Not the same content :P"), - }, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "important-secrets", + }) + assert.NoError(t, err) + + // create a commit on main branch. + _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Not the same content :P"), }, - Message: "Add a important file", - OldBranch: "main", - NewBranch: "main", - }) - assert.NoError(t, err) - - // create Pull to merge the important-secrets branch into main branch. - pullIssue := &issues_model.Issue{ - RepoID: baseRepo.ID, - Title: "PR with conflict!", - PosterID: user.ID, - Poster: user, - IsPull: true, - } + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "main", + }) + assert.NoError(t, err) + + // create Pull to merge the important-secrets branch into main branch. + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "PR with conflict!", + PosterID: user.ID, + Poster: user, + IsPull: true, + } - pullRequest := &issues_model.PullRequest{ - HeadRepoID: baseRepo.ID, - BaseRepoID: baseRepo.ID, - HeadBranch: "important-secrets", - BaseBranch: "main", - HeadRepo: baseRepo, - BaseRepo: baseRepo, - Type: issues_model.PullRequestGitea, - } - prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} - err = pull_service.NewPullRequest(t.Context(), prOpts) - assert.NoError(t, err) - - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) - assert.NoError(t, issue.LoadPullRequest(t.Context())) - conflictingPR := issue.PullRequest - - // Ensure conflictedFiles is populated. - assert.Len(t, conflictingPR.ConflictedFiles, 1) - // Check if status is correct. - assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) - // Ensure that mergeable returns false - assert.False(t, conflictingPR.Mergeable(t.Context())) + pullRequest := &issues_model.PullRequest{ + HeadRepoID: baseRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "important-secrets", + BaseBranch: "main", + HeadRepo: baseRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(t.Context(), prOpts) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Len(t, conflictingPR.ConflictedFiles, 1) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) + // Ensure that mergeable returns false + assert.False(t, conflictingPR.Mergeable(t.Context())) +} + +func Test_PullRequestStatusCheckingCrossRepo_Mergeable_MergeTree(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() + testPullRequestStatusCheckingCrossRepoMergeable(t, giteaURL) }) } -func Test_PullRequestStatusCheckingCrossRepo_Mergeable(t *testing.T) { +func Test_PullRequestStatusCheckingCrossRepo_Mergeable_TmpRepo(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - session := loginUser(t, user.Name) - - // Create new clean repo to test conflict checking. - baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ - Name: "conflict-checking", - Description: "Tempo repo", - AutoInit: true, - Readme: "Default", - DefaultBranch: "main", - }) - assert.NoError(t, err) - assert.NotEmpty(t, baseRepo) + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + testPullRequestStatusCheckingCrossRepoMergeable(t, giteaURL) + }) +} - testRepoFork(t, session, baseRepo.OwnerName, baseRepo.Name, "org3", "conflict-checking", "main") +func testPullRequestStatusCheckingCrossRepoMergeable(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user.Name) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + testRepoFork(t, session, baseRepo.OwnerName, baseRepo.Name, "org3", "conflict-checking", "main") - forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "org3", Name: "conflict-checking"}) + forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "org3", Name: "conflict-checking"}) - // create a commit on new branch of forked repository - _, err = files_service.ChangeRepoFiles(t.Context(), forkRepo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "important_file", - ContentReader: strings.NewReader("Just a non-important file"), - }, + // create a commit on new branch of forked repository + _, err = files_service.ChangeRepoFiles(t.Context(), forkRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Just a non-important file"), }, - Message: "Add a important file", - OldBranch: "main", - NewBranch: "important-secrets", - }) - assert.NoError(t, err) - - // create Pull to merge the important-secrets branch into main branch. - pullIssue := &issues_model.Issue{ - RepoID: baseRepo.ID, - Title: "PR with no conflict", - PosterID: user.ID, - Poster: user, - IsPull: true, - } + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "important-secrets", + }) + assert.NoError(t, err) + + // create Pull to merge the important-secrets branch into main branch. + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "PR with no conflict", + PosterID: user.ID, + Poster: user, + IsPull: true, + } - pullRequest := &issues_model.PullRequest{ - HeadRepoID: forkRepo.ID, - BaseRepoID: baseRepo.ID, - HeadBranch: "important-secrets", - BaseBranch: "main", - HeadRepo: forkRepo, - BaseRepo: baseRepo, - Type: issues_model.PullRequestGitea, - } - prOpts := &pull_service.NewPullRequestOptions{ - Repo: baseRepo, - Issue: pullIssue, - PullRequest: pullRequest, - } - err = pull_service.NewPullRequest(t.Context(), prOpts) - assert.NoError(t, err) - - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with no conflict"}) - assert.NoError(t, issue.LoadPullRequest(t.Context())) - conflictingPR := issue.PullRequest - - // Ensure conflictedFiles is populated. - assert.Empty(t, conflictingPR.ConflictedFiles) - // Check if status is correct. - assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) - // Ensure that mergeable returns true - assert.True(t, conflictingPR.Mergeable(t.Context())) + pullRequest := &issues_model.PullRequest{ + HeadRepoID: forkRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "important-secrets", + BaseBranch: "main", + HeadRepo: forkRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + prOpts := &pull_service.NewPullRequestOptions{ + Repo: baseRepo, + Issue: pullIssue, + PullRequest: pullRequest, + } + err = pull_service.NewPullRequest(t.Context(), prOpts) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with no conflict"}) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Empty(t, conflictingPR.ConflictedFiles) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) + // Ensure that mergeable returns true + assert.True(t, conflictingPR.Mergeable(t.Context())) +} + +func Test_PullRequestStatusCheckingCrossRepo_Conflicted_MergeTree(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() + testPullRequestStatusCheckingCrossRepoConflicted(t, giteaURL) }) } -func Test_PullRequestStatusCheckingCrossRepo_Conflicted(t *testing.T) { +func Test_PullRequestStatusCheckingCrossRepo_Conflicted_TmpRepo(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - session := loginUser(t, user.Name) - - // Create new clean repo to test conflict checking. - baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ - Name: "conflict-checking", - Description: "Tempo repo", - AutoInit: true, - Readme: "Default", - DefaultBranch: "main", - }) - assert.NoError(t, err) - assert.NotEmpty(t, baseRepo) + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + testPullRequestStatusCheckingCrossRepoConflicted(t, giteaURL) + }) +} + +func testPullRequestStatusCheckingCrossRepoConflicted(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user.Name) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) - testRepoFork(t, session, baseRepo.OwnerName, baseRepo.Name, "org3", "conflict-checking", "main") + testRepoFork(t, session, baseRepo.OwnerName, baseRepo.Name, "org3", "conflict-checking", "main") - forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "org3", Name: "conflict-checking"}) + forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "org3", Name: "conflict-checking"}) - // create a commit on new branch of forked repository - _, err = files_service.ChangeRepoFiles(t.Context(), forkRepo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "important_file", - ContentReader: strings.NewReader("Just a non-important file"), - }, + // create a commit on new branch of forked repository + _, err = files_service.ChangeRepoFiles(t.Context(), forkRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Just a non-important file"), }, - Message: "Add a important file", - OldBranch: "main", - NewBranch: "important-secrets", - }) - assert.NoError(t, err) - - // create a commit on main branch of base repository. - _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ - Files: []*files_service.ChangeRepoFile{ - { - Operation: "create", - TreePath: "important_file", - ContentReader: strings.NewReader("Not the same content :P"), - }, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "important-secrets", + }) + assert.NoError(t, err) + + // create a commit on main branch of base repository. + _, err = files_service.ChangeRepoFiles(t.Context(), baseRepo, user, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: "important_file", + ContentReader: strings.NewReader("Not the same content :P"), }, - Message: "Add a important file", - OldBranch: "main", - NewBranch: "main", - }) - assert.NoError(t, err) - - // create Pull to merge the important-secrets branch into main branch. - pullIssue := &issues_model.Issue{ - RepoID: baseRepo.ID, - Title: "PR with conflict!", - PosterID: user.ID, - Poster: user, - IsPull: true, + }, + Message: "Add a important file", + OldBranch: "main", + NewBranch: "main", + }) + assert.NoError(t, err) + + // create Pull to merge the important-secrets branch into main branch. + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "PR with conflict!", + PosterID: user.ID, + Poster: user, + IsPull: true, + } + + pullRequest := &issues_model.PullRequest{ + HeadRepoID: forkRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "important-secrets", + BaseBranch: "main", + HeadRepo: forkRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} + err = pull_service.NewPullRequest(t.Context(), prOpts) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Len(t, conflictingPR.ConflictedFiles, 1) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) + // Ensure that mergeable returns false + assert.False(t, conflictingPR.Mergeable(t.Context())) +} + +func Test_PullRequest_AGit_StatusChecking_Mergeable_MergeTree(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + // skip this test if git version is low + if !git.DefaultFeatures().SupportProcReceive { + return } - pullRequest := &issues_model.PullRequest{ - HeadRepoID: forkRepo.ID, - BaseRepoID: baseRepo.ID, - HeadBranch: "important-secrets", - BaseBranch: "main", - HeadRepo: forkRepo, - BaseRepo: baseRepo, - Type: issues_model.PullRequestGitea, + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() + + testPullRequestAGitStatusCheckingMergeable(t, giteaURL) + }) +} + +func Test_PullRequest_AGit_StatusChecking_Mergeable_TmpRepo(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + // skip this test if git version is low + if !git.DefaultFeatures().SupportProcReceive { + return } - prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest} - err = pull_service.NewPullRequest(t.Context(), prOpts) - assert.NoError(t, err) - - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) - assert.NoError(t, issue.LoadPullRequest(t.Context())) - conflictingPR := issue.PullRequest - - // Ensure conflictedFiles is populated. - assert.Len(t, conflictingPR.ConflictedFiles, 1) - // Check if status is correct. - assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) - // Ensure that mergeable returns false - assert.False(t, conflictingPR.Mergeable(t.Context())) + + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + + testPullRequestAGitStatusCheckingMergeable(t, giteaURL) }) } -func Test_PullRequest_AGit_StatusChecking_Mergeable(t *testing.T) { - // skip this test if git version is low - if !git.DefaultFeatures().SupportProcReceive { - return - } +func testPullRequestAGitStatusCheckingMergeable(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create new clean repo to test conflict checking. - baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ - Name: "conflict-checking", - Description: "Tempo repo", - AutoInit: true, - Readme: "Default", - DefaultBranch: "main", - }) - assert.NoError(t, err) - assert.NotEmpty(t, baseRepo) - - // add something in local repository and push it to remote - dstPath := t.TempDir() - repoURL := *giteaURL - repoURL.Path = baseRepo.FullName() + ".git" - repoURL.User = url.UserPassword("user2", userPassword) - doGitClone(dstPath, &repoURL)(t) - - gitRepo, err := git.OpenRepository(t.Context(), dstPath) - assert.NoError(t, err) - defer gitRepo.Close() - - doGitCreateBranch(dstPath, "test-agit-push")(t) - - _, err = generateCommitWithNewData(t.Context(), testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") - assert.NoError(t, err) - - // push to create an agit pull request - err = gitcmd.NewCommand("push", "origin", - "-o", "title=agit-test-title", "-o", "description=agit-test-description", - "-o", "topic=head-branch-name", - "HEAD:refs/for/main", - ).Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ - RepoID: baseRepo.ID, - Title: "agit-test-title", - }) - assert.NoError(t, issue.LoadPullRequest(t.Context())) - conflictingPR := issue.PullRequest - - // Ensure conflictedFiles is populated. - assert.Empty(t, conflictingPR.ConflictedFiles) - // Check if status is correct. - assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) - // Ensure that mergeable returns true - assert.True(t, conflictingPR.Mergeable(t.Context())) + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // add something in local repository and push it to remote + dstPath := t.TempDir() + repoURL := *giteaURL + repoURL.Path = baseRepo.FullName() + ".git" + repoURL.User = url.UserPassword("user2", userPassword) + doGitClone(dstPath, &repoURL)(t) + + gitRepo, err := git.OpenRepository(t.Context(), dstPath) + assert.NoError(t, err) + defer gitRepo.Close() + + doGitCreateBranch(dstPath, "test-agit-push")(t) + + _, err = generateCommitWithNewData(t.Context(), testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-") + assert.NoError(t, err) + + // push to create an agit pull request + err = gitcmd.NewCommand("push", "origin", + "-o", "title=agit-test-title", "-o", "description=agit-test-description", + "-o", "topic=head-branch-name", + "HEAD:refs/for/main", + ).Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "agit-test-title", + }) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Empty(t, conflictingPR.ConflictedFiles) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusMergeable, conflictingPR.Status) + // Ensure that mergeable returns true + assert.True(t, conflictingPR.Mergeable(t.Context())) } -func Test_PullRequest_AGit_StatusChecking_Conflicted(t *testing.T) { - // skip this test if git version is low - if !git.DefaultFeatures().SupportProcReceive { - return - } +func Test_PullRequest_AGit_StatusChecking_Conflicted_MergeTree(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + // skip this test if git version is low + if !git.DefaultFeatures().SupportProcReceive { + return + } + + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() + + testPullRequestAGitStatusCheckingConflicted(t, giteaURL) + }) +} +func Test_PullRequest_AGit_StatusChecking_Conflicted_TmpRepo(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - // Create new clean repo to test conflict checking. - baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ - Name: "conflict-checking", - Description: "Tempo repo", - AutoInit: true, - Readme: "Default", - DefaultBranch: "main", - }) - assert.NoError(t, err) - assert.NotEmpty(t, baseRepo) - - // add something in local repository and push it to remote - dstPath := t.TempDir() - repoURL := *giteaURL - repoURL.Path = baseRepo.FullName() + ".git" - repoURL.User = url.UserPassword("user2", userPassword) - doGitClone(dstPath, &repoURL)(t) - - gitRepo, err := git.OpenRepository(t.Context(), dstPath) - assert.NoError(t, err) - defer gitRepo.Close() - - // create agit branch from current commit - doGitCreateBranch(dstPath, "test-agit-push")(t) - - // add something on the same file of main branch so that it causes conflict - doGitCheckoutBranch(dstPath, "main")(t) - - assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte("Some changes to README file to main cause conflict"), 0o644)) - assert.NoError(t, git.AddChanges(t.Context(), dstPath, true)) - doGitCommit(dstPath, "add something to main branch")(t) - - err = gitcmd.NewCommand("push", "origin", "main").Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - - // check out back to agit branch and change the same file - doGitCheckoutBranch(dstPath, "test-agit-push")(t) - - assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte("Some changes to README file for agit branch"), 0o644)) - assert.NoError(t, git.AddChanges(t.Context(), dstPath, true)) - doGitCommit(dstPath, "add something to agit branch")(t) - - // push to create an agit pull request - err = gitcmd.NewCommand("push", "origin", - "-o", "title=agit-test-title", "-o", "description=agit-test-description", - "-o", "topic=head-branch-name", - "HEAD:refs/for/main", - ).Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) - assert.NoError(t, err) - - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ - RepoID: baseRepo.ID, - Title: "agit-test-title", - }) - assert.NoError(t, issue.LoadPullRequest(t.Context())) - conflictingPR := issue.PullRequest - - // Ensure conflictedFiles is populated. - assert.Equal(t, []string{"README.md"}, conflictingPR.ConflictedFiles) - // Check if status is correct. - assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) - // Ensure that mergeable returns false - assert.False(t, conflictingPR.Mergeable(t.Context())) + // skip this test if git version is low + if !git.DefaultFeatures().SupportProcReceive { + return + } + + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + + testPullRequestAGitStatusCheckingConflicted(t, giteaURL) + }) +} + +func testPullRequestAGitStatusCheckingConflicted(t *testing.T, giteaURL *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // Create new clean repo to test conflict checking. + baseRepo, err := repo_service.CreateRepository(t.Context(), user, user, repo_service.CreateRepoOptions{ + Name: "conflict-checking", + Description: "Tempo repo", + AutoInit: true, + Readme: "Default", + DefaultBranch: "main", + }) + assert.NoError(t, err) + assert.NotEmpty(t, baseRepo) + + // add something in local repository and push it to remote + dstPath := t.TempDir() + repoURL := *giteaURL + repoURL.Path = baseRepo.FullName() + ".git" + repoURL.User = url.UserPassword("user2", userPassword) + doGitClone(dstPath, &repoURL)(t) + + gitRepo, err := git.OpenRepository(t.Context(), dstPath) + assert.NoError(t, err) + defer gitRepo.Close() + + // create agit branch from current commit + doGitCreateBranch(dstPath, "test-agit-push")(t) + + // add something on the same file of main branch so that it causes conflict + doGitCheckoutBranch(dstPath, "main")(t) + + assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte("Some changes to README file to main cause conflict"), 0o644)) + assert.NoError(t, git.AddChanges(t.Context(), dstPath, true)) + doGitCommit(dstPath, "add something to main branch")(t) + + err = gitcmd.NewCommand("push", "origin", "main").Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + // check out back to agit branch and change the same file + doGitCheckoutBranch(dstPath, "test-agit-push")(t) + + assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte("Some changes to README file for agit branch"), 0o644)) + assert.NoError(t, git.AddChanges(t.Context(), dstPath, true)) + doGitCommit(dstPath, "add something to agit branch")(t) + + // push to create an agit pull request + err = gitcmd.NewCommand("push", "origin", + "-o", "title=agit-test-title", "-o", "description=agit-test-description", + "-o", "topic=head-branch-name", + "HEAD:refs/for/main", + ).Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) + assert.NoError(t, err) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "agit-test-title", }) + assert.NoError(t, issue.LoadPullRequest(t.Context())) + conflictingPR := issue.PullRequest + + // Ensure conflictedFiles is populated. + assert.Equal(t, []string{"README.md"}, conflictingPR.ConflictedFiles) + // Check if status is correct. + assert.Equal(t, issues_model.PullRequestStatusConflict, conflictingPR.Status) + // Ensure that mergeable returns false + assert.False(t, conflictingPR.Mergeable(t.Context())) } From e8636b7a4ed4dd75440a048270cb6397551a7b9b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 3 Oct 2025 18:05:48 -0700 Subject: [PATCH 09/12] Add both mergetree and tmprepo for rebase and retarget tests --- tests/integration/pull_comment_test.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go index abab65247ba74..9a27e3692dd5d 100644 --- a/tests/integration/pull_comment_test.go +++ b/tests/integration/pull_comment_test.go @@ -13,6 +13,8 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/test" issues_service "code.gitea.io/gitea/services/issue" "github.com/stretchr/testify/assert" @@ -103,7 +105,22 @@ func TestPullComment(t *testing.T) { testCreateBranch(t, session, "user2", "repo1", "branch/master", "test-branch/retarget", http.StatusSeeOther) testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") - t.Run("RebaseComment", func(t *testing.T) { testPullCommentRebase(t, u, session) }) - t.Run("RetargetComment", func(t *testing.T) { testPullCommentRetarget(t, u, session) }) + t.Run("RebaseComment_MergeTree", func(t *testing.T) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() + testPullCommentRebase(t, u, session) + }) + t.Run("RebaseComment_TmpRepo", func(t *testing.T) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + testPullCommentRebase(t, u, session) + }) + + t.Run("RetargetComment_MergeTree", func(t *testing.T) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() + testPullCommentRetarget(t, u, session) + }) + t.Run("RetargetComment_TmpRepo", func(t *testing.T) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + testPullCommentRetarget(t, u, session) + }) }) } From 4b8c0471e3c5a0f6151d5bc0ee37c4d235ace98d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 6 Oct 2025 14:00:32 -0700 Subject: [PATCH 10/12] make test happy --- tests/integration/pull_comment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go index 9a27e3692dd5d..72de2103d810f 100644 --- a/tests/integration/pull_comment_test.go +++ b/tests/integration/pull_comment_test.go @@ -27,7 +27,7 @@ func testWaitForPullRequestStatus(t *testing.T, prIssue *issues_model.Issue, exp retIssue = unittest.AssertExistsAndLoadBean(t, &prIssueCond) require.NoError(t, retIssue.LoadPullRequest(t.Context())) return retIssue.PullRequest.Status == expectedStatus - }, 5*time.Second, 20*time.Millisecond) + }, 10*time.Second, 20*time.Millisecond) return retIssue } From 6e96a4afc1ee7834ff5683fe692a4ef5497f1e45 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 6 Oct 2025 15:38:20 -0700 Subject: [PATCH 11/12] remove unnecessary check --- tests/integration/pull_status_test.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 3cb76f4a4e912..f39db53ce81fb 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -517,11 +517,6 @@ func Test_PullRequest_AGit_StatusChecking_Mergeable_MergeTree(t *testing.T) { func Test_PullRequest_AGit_StatusChecking_Mergeable_TmpRepo(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - // skip this test if git version is low - if !git.DefaultFeatures().SupportProcReceive { - return - } - defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() testPullRequestAGitStatusCheckingMergeable(t, giteaURL) @@ -583,11 +578,6 @@ func testPullRequestAGitStatusCheckingMergeable(t *testing.T, giteaURL *url.URL) func Test_PullRequest_AGit_StatusChecking_Conflicted_MergeTree(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - // skip this test if git version is low - if !git.DefaultFeatures().SupportProcReceive { - return - } - defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() testPullRequestAGitStatusCheckingConflicted(t, giteaURL) @@ -596,11 +586,6 @@ func Test_PullRequest_AGit_StatusChecking_Conflicted_MergeTree(t *testing.T) { func Test_PullRequest_AGit_StatusChecking_Conflicted_TmpRepo(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - // skip this test if git version is low - if !git.DefaultFeatures().SupportProcReceive { - return - } - defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() testPullRequestAGitStatusCheckingConflicted(t, giteaURL) From 22f0aa25b7ae7198f9e84d48e4f6e49b3c355bb5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 6 Oct 2025 18:40:22 -0700 Subject: [PATCH 12/12] Fix test --- tests/integration/pull_comment_test.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go index 72de2103d810f..5323b1cb72b2d 100644 --- a/tests/integration/pull_comment_test.go +++ b/tests/integration/pull_comment_test.go @@ -98,7 +98,7 @@ func testPullCommentRetarget(t *testing.T, u *url.URL, session *TestSession) { testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusMergeable) } -func TestPullComment(t *testing.T) { +func TestPullComment_MergeTree(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) @@ -109,15 +109,26 @@ func TestPullComment(t *testing.T) { defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() testPullCommentRebase(t, u, session) }) - t.Run("RebaseComment_TmpRepo", func(t *testing.T) { - defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() - testPullCommentRebase(t, u, session) - }) t.Run("RetargetComment_MergeTree", func(t *testing.T) { defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, true)() testPullCommentRetarget(t, u, session) }) + }) +} + +func TestPullComment_TmpRepo(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", "") + + t.Run("RebaseComment_TmpRepo", func(t *testing.T) { + defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() + testPullCommentRebase(t, u, session) + }) + t.Run("RetargetComment_TmpRepo", func(t *testing.T) { defer test.MockVariableValue(&git.DefaultFeatures().SupportGitMergeTree, false)() testPullCommentRetarget(t, u, session)