From 3f01fb31d95b62248dfbe78daba42de68db9ae5c Mon Sep 17 00:00:00 2001 From: Tom Neuber Date: Wed, 23 Oct 2024 21:31:52 +0200 Subject: [PATCH 1/3] create "shared" package to workaround import loop issues --- services/automerge/automerge.go | 105 +-------------- .../repository/commitstatus/commitstatus.go | 4 +- services/shared/automerge/automerge.go | 120 ++++++++++++++++++ 3 files changed, 128 insertions(+), 101 deletions(-) create mode 100644 services/shared/automerge/automerge.go diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index a1ee204882..fcc013348c 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -7,8 +7,6 @@ import ( "context" "errors" "fmt" - "strconv" - "strings" "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" @@ -24,20 +22,18 @@ import ( "code.gitea.io/gitea/modules/queue" notify_service "code.gitea.io/gitea/services/notify" pull_service "code.gitea.io/gitea/services/pull" + shared_automerge "code.gitea.io/gitea/services/shared/automerge" ) -// prAutoMergeQueue represents a queue to handle update pull request tests -var prAutoMergeQueue *queue.WorkerPoolQueue[string] - // Init runs the task queue to that handles auto merges func Init() error { notify_service.RegisterNotifier(NewNotifier()) - prAutoMergeQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_auto_merge", handler) - if prAutoMergeQueue == nil { + shared_automerge.PRAutoMergeQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_auto_merge", handler) + if shared_automerge.PRAutoMergeQueue == nil { return fmt.Errorf("unable to create pr_auto_merge queue") } - go graceful.GetManager().RunWithCancel(prAutoMergeQueue) + go graceful.GetManager().RunWithCancel(shared_automerge.PRAutoMergeQueue) return nil } @@ -55,13 +51,6 @@ func handler(items ...string) []string { return nil } -func addToQueue(pr *issues_model.PullRequest, sha string) { - log.Trace("Adding pullID: %d to the pull requests patch checking queue with sha %s", pr.ID, sha) - if err := prAutoMergeQueue.Push(fmt.Sprintf("%d_%s", pr.ID, sha)); err != nil { - log.Error("Error adding pullID: %d to the pull requests patch checking queue %v", pr.ID, err) - } -} - // ScheduleAutoMerge if schedule is false and no error, pull can be merged directly func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string) (scheduled bool, err error) { err = db.WithTx(ctx, func(ctx context.Context) error { @@ -90,94 +79,12 @@ func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull * // StartPRCheckAndAutoMergeBySHA start an automerge check and auto merge task for all pull requests of repository and SHA func StartPRCheckAndAutoMergeBySHA(ctx context.Context, sha string, repo *repo_model.Repository) error { - pulls, err := getPullRequestsByHeadSHA(ctx, sha, repo, func(pr *issues_model.PullRequest) bool { - return !pr.HasMerged && pr.CanAutoMerge() - }) - if err != nil { - return err - } - - for _, pr := range pulls { - addToQueue(pr, sha) - } - - return nil + return shared_automerge.StartPRCheckAndAutoMergeBySHA(ctx, sha, repo) } // StartPRCheckAndAutoMerge start an automerge check and auto merge task for a pull request func StartPRCheckAndAutoMerge(ctx context.Context, pull *issues_model.PullRequest) { - if pull == nil || pull.HasMerged || !pull.CanAutoMerge() { - return - } - - if err := pull.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) - return - } - - gitRepo, err := gitrepo.OpenRepository(ctx, pull.BaseRepo) - if err != nil { - log.Error("OpenRepository: %v", err) - return - } - defer gitRepo.Close() - commitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName()) - if err != nil { - log.Error("GetRefCommitID: %v", err) - return - } - - addToQueue(pull, commitID) -} - -func getPullRequestsByHeadSHA(ctx context.Context, sha string, repo *repo_model.Repository, filter func(*issues_model.PullRequest) bool) (map[int64]*issues_model.PullRequest, error) { - gitRepo, err := gitrepo.OpenRepository(ctx, repo) - if err != nil { - return nil, err - } - defer gitRepo.Close() - - refs, err := gitRepo.GetRefsBySha(sha, "") - if err != nil { - return nil, err - } - - pulls := make(map[int64]*issues_model.PullRequest) - - for _, ref := range refs { - // Each pull branch starts with refs/pull/ we then go from there to find the index of the pr and then - // use that to get the pr. - if strings.HasPrefix(ref, git.PullPrefix) { - parts := strings.Split(ref[len(git.PullPrefix):], "/") - - // e.g. 'refs/pull/1/head' would be []string{"1", "head"} - if len(parts) != 2 { - log.Error("getPullRequestsByHeadSHA found broken pull ref [%s] on repo [%-v]", ref, repo) - continue - } - - prIndex, err := strconv.ParseInt(parts[0], 10, 64) - if err != nil { - log.Error("getPullRequestsByHeadSHA found broken pull ref [%s] on repo [%-v]", ref, repo) - continue - } - - p, err := issues_model.GetPullRequestByIndex(ctx, repo.ID, prIndex) - if err != nil { - // If there is no pull request for this branch, we don't try to merge it. - if issues_model.IsErrPullRequestNotExist(err) { - continue - } - return nil, err - } - - if filter(p) { - pulls[p.ID] = p - } - } - } - - return pulls, nil + shared_automerge.StartPRCheckAndAutoMerge(ctx, pull) } // handlePullRequestAutoMerge merge the pull request if all checks are successful diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index 5c630201d2..635b0b108e 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -19,7 +19,7 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/services/automerge" + shared_automerge "code.gitea.io/gitea/services/shared/automerge" ) func getCacheKey(repoID int64, brancheName string) string { @@ -117,7 +117,7 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato } if status.State.IsSuccess() { - if err := automerge.StartPRCheckAndAutoMergeBySHA(ctx, sha, repo); err != nil { + if err := shared_automerge.StartPRCheckAndAutoMergeBySHA(ctx, sha, repo); err != nil { return fmt.Errorf("MergeScheduledPullRequest[repo_id: %d, user_id: %d, sha: %s]: %w", repo.ID, creator.ID, sha, err) } } diff --git a/services/shared/automerge/automerge.go b/services/shared/automerge/automerge.go new file mode 100644 index 0000000000..8f38cf260a --- /dev/null +++ b/services/shared/automerge/automerge.go @@ -0,0 +1,120 @@ +// Copyright 2021 Gitea. All rights reserved. +// SPDX-License-Identifier: MIT + +package automerge + +import ( + "context" + "fmt" + "strconv" + "strings" + + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/queue" +) + +// PRAutoMergeQueue represents a queue to handle update pull request tests +var PRAutoMergeQueue *queue.WorkerPoolQueue[string] + +func addToQueue(pr *issues_model.PullRequest, sha string) { + log.Trace("Adding pullID: %d to the pull requests patch checking queue with sha %s", pr.ID, sha) + if err := PRAutoMergeQueue.Push(fmt.Sprintf("%d_%s", pr.ID, sha)); err != nil { + log.Error("Error adding pullID: %d to the pull requests patch checking queue %v", pr.ID, err) + } +} + +// StartPRCheckAndAutoMergeBySHA start an automerge check and auto merge task for all pull requests of repository and SHA +func StartPRCheckAndAutoMergeBySHA(ctx context.Context, sha string, repo *repo_model.Repository) error { + pulls, err := getPullRequestsByHeadSHA(ctx, sha, repo, func(pr *issues_model.PullRequest) bool { + return !pr.HasMerged && pr.CanAutoMerge() + }) + if err != nil { + return err + } + + for _, pr := range pulls { + addToQueue(pr, sha) + } + + return nil +} + +// StartPRCheckAndAutoMerge start an automerge check and auto merge task for a pull request +func StartPRCheckAndAutoMerge(ctx context.Context, pull *issues_model.PullRequest) { + if pull == nil || pull.HasMerged || !pull.CanAutoMerge() { + return + } + + if err := pull.LoadBaseRepo(ctx); err != nil { + log.Error("LoadBaseRepo: %v", err) + return + } + + gitRepo, err := gitrepo.OpenRepository(ctx, pull.BaseRepo) + if err != nil { + log.Error("OpenRepository: %v", err) + return + } + defer gitRepo.Close() + commitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName()) + if err != nil { + log.Error("GetRefCommitID: %v", err) + return + } + + addToQueue(pull, commitID) +} + +func getPullRequestsByHeadSHA(ctx context.Context, sha string, repo *repo_model.Repository, filter func(*issues_model.PullRequest) bool) (map[int64]*issues_model.PullRequest, error) { + gitRepo, err := gitrepo.OpenRepository(ctx, repo) + if err != nil { + return nil, err + } + defer gitRepo.Close() + + refs, err := gitRepo.GetRefsBySha(sha, "") + if err != nil { + return nil, err + } + + pulls := make(map[int64]*issues_model.PullRequest) + + for _, ref := range refs { + // Each pull branch starts with refs/pull/ we then go from there to find the index of the pr and then + // use that to get the pr. + if strings.HasPrefix(ref, git.PullPrefix) { + parts := strings.Split(ref[len(git.PullPrefix):], "/") + + // e.g. 'refs/pull/1/head' would be []string{"1", "head"} + if len(parts) != 2 { + log.Error("getPullRequestsByHeadSHA found broken pull ref [%s] on repo [%-v]", ref, repo) + continue + } + + prIndex, err := strconv.ParseInt(parts[0], 10, 64) + if err != nil { + log.Error("getPullRequestsByHeadSHA found broken pull ref [%s] on repo [%-v]", ref, repo) + continue + } + + p, err := issues_model.GetPullRequestByIndex(ctx, repo.ID, prIndex) + if err != nil { + // If there is no pull request for this branch, we don't try to merge it. + if issues_model.IsErrPullRequestNotExist(err) { + continue + } + return nil, err + } + + if filter(p) { + pulls[p.ID] = p + } + } + } + + return pulls, nil +} From 68d5cf0e92e945c064c28bf26f24699d261becc2 Mon Sep 17 00:00:00 2001 From: Tom Neuber Date: Mon, 21 Oct 2024 21:21:50 +0200 Subject: [PATCH 2/3] Add branch auto deletion for scheduled PRs --- models/forgejo_migrations/migrate.go | 2 ++ models/forgejo_migrations/v23.go | 16 ++++++++++++ models/pull/automerge.go | 26 ++++++++++--------- routers/api/v1/repo/pull.go | 2 +- routers/private/hook_post_receive_test.go | 2 +- routers/web/repo/pull.go | 2 +- services/automerge/automerge.go | 12 +++++++-- .../integration/actions_commit_status_test.go | 2 +- tests/integration/pull_merge_test.go | 12 ++++----- .../js/components/PullRequestMergeForm.vue | 2 +- 10 files changed, 53 insertions(+), 25 deletions(-) create mode 100644 models/forgejo_migrations/v23.go diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index cca83d6b4d..116579eeff 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -82,6 +82,8 @@ var migrations = []*Migration{ NewMigration("Add SSH keypair to `pull_mirror` table", AddSSHKeypairToPushMirror), // v22 -> v23 NewMigration("Add `legacy` to `web_authn_credential` table", AddLegacyToWebAuthnCredential), + // v23 -> v24 + NewMigration("Add `delete_branch_after_merge` to `auto_merge` table", AddDeleteBranchAfterMergeToAutoMerge), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v23.go b/models/forgejo_migrations/v23.go new file mode 100644 index 0000000000..20a916a716 --- /dev/null +++ b/models/forgejo_migrations/v23.go @@ -0,0 +1,16 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package forgejo_migrations //nolint:revive + +import "xorm.io/xorm" + +// AddDeleteBranchAfterMergeToAutoMerge: add DeleteBranchAfterMerge column, setting existing rows to false +func AddDeleteBranchAfterMergeToAutoMerge(x *xorm.Engine) error { + type AutoMerge struct { + ID int64 `xorm:"pk autoincr"` + DeleteBranchAfterMerge bool `xorm:"NOT NULL DEFAULT false"` + } + + return x.Sync(&AutoMerge{}) +} diff --git a/models/pull/automerge.go b/models/pull/automerge.go index f31159a8d8..d739d5a145 100644 --- a/models/pull/automerge.go +++ b/models/pull/automerge.go @@ -15,13 +15,14 @@ import ( // AutoMerge represents a pull request scheduled for merging when checks succeed type AutoMerge struct { - ID int64 `xorm:"pk autoincr"` - PullID int64 `xorm:"UNIQUE"` - DoerID int64 `xorm:"INDEX NOT NULL"` - Doer *user_model.User `xorm:"-"` - MergeStyle repo_model.MergeStyle `xorm:"varchar(30)"` - Message string `xorm:"LONGTEXT"` - CreatedUnix timeutil.TimeStamp `xorm:"created"` + ID int64 `xorm:"pk autoincr"` + PullID int64 `xorm:"UNIQUE"` + DoerID int64 `xorm:"INDEX NOT NULL"` + Doer *user_model.User `xorm:"-"` + MergeStyle repo_model.MergeStyle `xorm:"varchar(30)"` + Message string `xorm:"LONGTEXT"` + DeleteBranchAfterMerge bool `xorm:"NOT NULL DEFAULT false"` + CreatedUnix timeutil.TimeStamp `xorm:"created"` } // TableName return database table name for xorm @@ -49,7 +50,7 @@ func IsErrAlreadyScheduledToAutoMerge(err error) bool { } // ScheduleAutoMerge schedules a pull request to be merged when all checks succeed -func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, style repo_model.MergeStyle, message string) error { +func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, style repo_model.MergeStyle, message string, deleteBranch bool) error { // Check if we already have a merge scheduled for that pull request if exists, _, err := GetScheduledMergeByPullID(ctx, pullID); err != nil { return err @@ -58,10 +59,11 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, } _, err := db.GetEngine(ctx).Insert(&AutoMerge{ - DoerID: doer.ID, - PullID: pullID, - MergeStyle: style, - Message: message, + DoerID: doer.ID, + PullID: pullID, + MergeStyle: style, + Message: message, + DeleteBranchAfterMerge: deleteBranch, }) return err } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index ace107b289..b4749cac37 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -989,7 +989,7 @@ func MergePullRequest(ctx *context.APIContext) { } if form.MergeWhenChecksSucceed { - scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message) + scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, form.DeleteBranchAfterMerge) if err != nil { if pull_model.IsErrAlreadyScheduledToAutoMerge(err) { ctx.Error(http.StatusConflict, "ScheduleAutoMerge", err) diff --git a/routers/private/hook_post_receive_test.go b/routers/private/hook_post_receive_test.go index bfd647e365..28f1a7d0be 100644 --- a/routers/private/hook_post_receive_test.go +++ b/routers/private/hook_post_receive_test.go @@ -28,7 +28,7 @@ func TestHandlePullRequestMerging(t *testing.T) { user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - err = pull_model.ScheduleAutoMerge(db.DefaultContext, user1, pr.ID, repo_model.MergeStyleSquash, "squash merge a pr") + err = pull_model.ScheduleAutoMerge(db.DefaultContext, user1, pr.ID, repo_model.MergeStyleSquash, "squash merge a pr", false) require.NoError(t, err) autoMerge := unittest.AssertExistsAndLoadBean(t, &pull_model.AutoMerge{PullID: pr.ID}) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 14b03b9fa4..9d0dcad61e 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1302,7 +1302,7 @@ func MergePullRequest(ctx *context.Context) { // delete all scheduled auto merges _ = pull_model.DeleteScheduledAutoMerge(ctx, pr.ID) // schedule auto merge - scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message) + scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, form.DeleteBranchAfterMerge) if err != nil { ctx.ServerError("ScheduleAutoMerge", err) return diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index fcc013348c..d3cc4c6fb1 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/queue" notify_service "code.gitea.io/gitea/services/notify" pull_service "code.gitea.io/gitea/services/pull" + repo_service "code.gitea.io/gitea/services/repository" shared_automerge "code.gitea.io/gitea/services/shared/automerge" ) @@ -52,9 +53,9 @@ func handler(items ...string) []string { } // ScheduleAutoMerge if schedule is false and no error, pull can be merged directly -func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string) (scheduled bool, err error) { +func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string, deleteBranch bool) (scheduled bool, err error) { err = db.WithTx(ctx, func(ctx context.Context) error { - if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message); err != nil { + if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message, deleteBranch); err != nil { return err } scheduled = true @@ -210,4 +211,11 @@ func handlePullRequestAutoMerge(pullID int64, sha string) { // on the pull request page. But this should not be finished in a bug fix PR which will be backport to release branch. return } + + if scheduledPRM.DeleteBranchAfterMerge { + err := repo_service.DeleteBranchAfterMerge(ctx, doer, pr, headGitRepo) + if err != nil { + log.Error("%d repo_service.DeleteBranchIfUnused: %v", pr.ID, err) + } + } } diff --git a/tests/integration/actions_commit_status_test.go b/tests/integration/actions_commit_status_test.go index ace0fbd323..7be40eb0fa 100644 --- a/tests/integration/actions_commit_status_test.go +++ b/tests/integration/actions_commit_status_test.go @@ -34,7 +34,7 @@ func TestActionsAutomerge(t *testing.T) { assert.False(t, pr.HasMerged, "PR should not be merged") assert.Equal(t, issues_model.PullRequestStatusMergeable, pr.Status, "PR should be mergeable") - scheduled, err := automerge.ScheduleAutoMerge(ctx, user, pr, repo_model.MergeStyleMerge, "Dummy") + scheduled, err := automerge.ScheduleAutoMerge(ctx, user, pr, repo_model.MergeStyleMerge, "Dummy", false) require.NoError(t, err, "PR should be scheduled for automerge") assert.True(t, scheduled, "PR should be scheduled for automerge") diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index caf100144d..012e8c1a8f 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -881,12 +881,12 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) { session.MakeRequest(t, req, http.StatusSeeOther) // first time insert automerge record, return true - scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) require.NoError(t, err) assert.True(t, scheduled) // second time insert automerge record, return false because it does exist - scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) require.Error(t, err) assert.False(t, scheduled) @@ -965,12 +965,12 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) { session.MakeRequest(t, req, http.StatusSeeOther) // first time insert automerge record, return true - scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) require.NoError(t, err) assert.True(t, scheduled) // second time insert automerge record, return false because it does exist - scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) require.Error(t, err) assert.False(t, scheduled) @@ -1094,12 +1094,12 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing. user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) // first time insert automerge record, return true - scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) require.NoError(t, err) assert.True(t, scheduled) // second time insert automerge record, return false because it does exist - scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) require.Error(t, err) assert.False(t, scheduled) diff --git a/web_src/js/components/PullRequestMergeForm.vue b/web_src/js/components/PullRequestMergeForm.vue index bd0901a7b5..46bda3a025 100644 --- a/web_src/js/components/PullRequestMergeForm.vue +++ b/web_src/js/components/PullRequestMergeForm.vue @@ -130,7 +130,7 @@ export default { {{ mergeForm.textCancel }} -
+
From fc06763371ecc3937a2563c60ea9c4ec3687de67 Mon Sep 17 00:00:00 2001 From: Tom Neuber Date: Fri, 25 Oct 2024 21:14:14 +0200 Subject: [PATCH 3/3] tests/integration: add integration tests for automerge pull requests --- tests/integration/pull_merge_test.go | 315 ++++++++++++++------------- 1 file changed, 163 insertions(+), 152 deletions(-) diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 012e8c1a8f..175f8a48de 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -846,180 +846,191 @@ func TestPullMergeBranchProtect(t *testing.T) { }) } -func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) { - onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - // create a pull request - session := loginUser(t, "user1") - user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - forkedName := "repo1-1" - testRepoFork(t, session, "user2", "repo1", "user1", forkedName) - defer func() { - testDeleteRepository(t, session, "user1", forkedName) - }() - testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n") - testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull") +func testPullAutoMergeAfterCommitStatusSucceed(t *testing.T, ctx APITestContext, forkName string, approval, deleteBranch, withAPI bool) { + // prepare environment (fork repo, create user) + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + testRepoFork(t, ctx.Session, "user2", "repo1", "user1", forkName) + defer func() { + testDeleteRepository(t, ctx.Session, "user1", forkName) + }() - baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) - forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: forkedName}) - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ - BaseRepoID: baseRepo.ID, - BaseBranch: "master", - HeadRepoID: forkedRepo.ID, - HeadBranch: "master", - }) + // create a pull request with some changes + branchName := "master" + if deleteBranch { + branchName = "new_branch_1" + testEditFileToNewBranch(t, ctx.Session, "user1", forkName, "master", branchName, "README.md", "Hello, World (Edited)\n") + } else { + testEditFile(t, ctx.Session, "user1", forkName, "master", "README.md", "Hello, World (Edited)\n") + } + testPullCreate(t, ctx.Session, "user1", forkName, false, "master", branchName, "Indexer notifier test pull") - // add protected branch for commit status - csrf := GetCSRF(t, session, "/user2/repo1/settings/branches") - // Change master branch to protected - req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ - "_csrf": csrf, - "rule_name": "master", - "enable_push": "true", - "enable_status_check": "true", - "status_check_contexts": "gitea/actions", - }) - session.MakeRequest(t, req, http.StatusSeeOther) - - // first time insert automerge record, return true - scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) - require.NoError(t, err) - assert.True(t, scheduled) - - // second time insert automerge record, return false because it does exist - scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) - require.Error(t, err) - assert.False(t, scheduled) - - // reload pr again - pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) - assert.False(t, pr.HasMerged) - assert.Empty(t, pr.MergedCommitID) - - // update commit status to success, then it should be merged automatically - baseGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, baseRepo) - require.NoError(t, err) - sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) - require.NoError(t, err) - masterCommitID, err := baseGitRepo.GetBranchCommitID("master") - require.NoError(t, err) - - branches, _, err := baseGitRepo.GetBranchNames(0, 100) - require.NoError(t, err) - assert.ElementsMatch(t, []string{"sub-home-md-img-check", "home-md-img-check", "pr-to-update", "branch2", "DefaultBranch", "develop", "feature/1", "master"}, branches) - baseGitRepo.Close() - defer func() { - testResetRepo(t, baseRepo.RepoPath(), "master", masterCommitID) - }() - - err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{ - State: api.CommitStatusSuccess, - TargetURL: "https://gitea.com", - Context: "gitea/actions", - }) - require.NoError(t, err) - - time.Sleep(2 * time.Second) - - // realod pr again - pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) - assert.True(t, pr.HasMerged) - assert.NotEmpty(t, pr.MergedCommitID) - - unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{PullID: pr.ID}) + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: forkName}) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ + BaseRepoID: baseRepo.ID, + BaseBranch: "master", + HeadRepoID: forkedRepo.ID, + HeadBranch: branchName, }) -} -func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) { - onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { - // create a pull request - session := loginUser(t, "user1") - user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - forkedName := "repo1-2" - testRepoFork(t, session, "user2", "repo1", "user1", forkedName) - defer func() { - testDeleteRepository(t, session, "user1", forkedName) - }() - testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n") - testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull") - - baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) - forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: forkedName}) - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ - BaseRepoID: baseRepo.ID, - BaseBranch: "master", - HeadRepoID: forkedRepo.ID, - HeadBranch: "master", - }) - - // add protected branch for commit status - csrf := GetCSRF(t, session, "/user2/repo1/settings/branches") - // Change master branch to protected - req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ - "_csrf": csrf, - "rule_name": "master", - "enable_push": "true", - "enable_status_check": "true", - "status_check_contexts": "gitea/actions", - "required_approvals": "1", - }) - session.MakeRequest(t, req, http.StatusSeeOther) - - // first time insert automerge record, return true - scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) + if deleteBranch { + // check if new branch exists + forkedGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, forkedRepo) require.NoError(t, err) - assert.True(t, scheduled) - - // second time insert automerge record, return false because it does exist - scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false) - require.Error(t, err) - assert.False(t, scheduled) - - // reload pr again - pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) - assert.False(t, pr.HasMerged) - assert.Empty(t, pr.MergedCommitID) - - // update commit status to success, then it should be merged automatically - baseGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, baseRepo) + newBranch, err := forkedGitRepo.GetBranch(branchName) require.NoError(t, err) - sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) - require.NoError(t, err) - masterCommitID, err := baseGitRepo.GetBranchCommitID("master") - require.NoError(t, err) - baseGitRepo.Close() - defer func() { - testResetRepo(t, baseRepo.RepoPath(), "master", masterCommitID) - }() + assert.NotNil(t, newBranch) + forkedGitRepo.Close() + } - err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{ - State: api.CommitStatusSuccess, - TargetURL: "https://gitea.com", - Context: "gitea/actions", - }) - require.NoError(t, err) + // schedule pull request for automerge + if withAPI { + mergePullRequestForm := forms.MergePullRequestForm{ + MergeMessageField: "auto merge test", + Do: string(repo_model.MergeStyleMerge), + MergeWhenChecksSucceed: true, + DeleteBranchAfterMerge: deleteBranch, + } - time.Sleep(2 * time.Second) + // first time scheduling an automerge pull request, should return a 201 + ctx.ExpectedCode = http.StatusCreated + doAPIMergePullRequestForm(t, ctx, "user2", "repo1", pr.Index, &mergePullRequestForm) - // reload pr again + // second time scheduling an automerge pull request, should return a 409 + ctx.ExpectedCode = http.StatusConflict + doAPIMergePullRequestForm(t, ctx, "user2", "repo1", pr.Index, &mergePullRequestForm) + } else { + mergePullRequestForm := map[string]string{ + "merge_message_field": "auto merge test", + "do": string(repo_model.MergeStyleMerge), + "merge_when_checks_succeed": "true", + "delete_branch_after_merge": strconv.FormatBool(deleteBranch), + } + + // first time scheduling an automerge pull request, should return a 200 + testPullMergeForm(t, ctx.Session, http.StatusOK, "user2", "repo1", strconv.FormatInt(pr.Index, 10), mergePullRequestForm) + + // second time scheduling an automerge pull request, should delete the previous scheduled automerge and return a 200 again + testPullMergeForm(t, ctx.Session, http.StatusOK, "user2", "repo1", strconv.FormatInt(pr.Index, 10), mergePullRequestForm) + } + + // reload PR again + pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) + assert.False(t, pr.HasMerged) + assert.Empty(t, pr.MergedCommitID) + + // update commit status to success, then it should be merged automatically + baseGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, baseRepo) + require.NoError(t, err) + sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) + require.NoError(t, err) + masterCommitID, err := baseGitRepo.GetBranchCommitID("master") + require.NoError(t, err) + + branches, _, err := baseGitRepo.GetBranchNames(0, 100) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"sub-home-md-img-check", "home-md-img-check", "pr-to-update", "branch2", "DefaultBranch", "develop", "feature/1", "master"}, branches) + baseGitRepo.Close() + defer func() { + testResetRepo(t, baseRepo.RepoPath(), "master", masterCommitID) + }() + + err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{ + State: api.CommitStatusSuccess, + TargetURL: "https://gitea.com", + Context: "gitea/actions", + }) + require.NoError(t, err) + + time.Sleep(2 * time.Second) + + // approve PR if necessary + if approval { + // reload PR again pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) assert.False(t, pr.HasMerged) assert.Empty(t, pr.MergedCommitID) // approve the PR from non-author approveSession := loginUser(t, "user2") - req = NewRequest(t, "GET", fmt.Sprintf("/user2/repo1/pulls/%d", pr.Index)) + req := NewRequest(t, "GET", fmt.Sprintf("/user2/repo1/pulls/%d", pr.Index)) resp := approveSession.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) testSubmitReview(t, approveSession, htmlDoc.GetCSRF(), "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK) time.Sleep(2 * time.Second) + } - // realod pr again - pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) - assert.True(t, pr.HasMerged) - assert.NotEmpty(t, pr.MergedCommitID) + // reload PR again + pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) + assert.True(t, pr.HasMerged) + assert.NotEmpty(t, pr.MergedCommitID) - unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{PullID: pr.ID}) + unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{PullID: pr.ID}) + + if deleteBranch { + // check if new branch got removed + forkedGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, forkedRepo) + require.NoError(t, err) + _, err = forkedGitRepo.GetBranch(branchName) + require.Error(t, err) + assert.True(t, git.IsErrBranchNotExist(err)) + forkedGitRepo.Close() + } +} + +func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + for _, testCase := range []struct { + name string + forkName string + approval bool + deleteBranch bool + }{ + { + name: "TestPullAutoMergeAfterCommitStatusSucceed", + forkName: "repo1-1", + approval: false, + deleteBranch: false, + }, + { + name: "TestPullAutoMergeAfterCommitStatusSucceedWithBranchDeletion", + forkName: "repo1-2", + approval: false, + deleteBranch: true, + }, + { + name: "TestPullAutoMergeAfterCommitStatusSucceedAndApproval", + forkName: "repo1-3", + approval: true, + deleteBranch: false, + }, + { + name: "TestPullAutoMergeAfterCommitStatusSucceedAndApprovalWithBranchDeletion", + forkName: "repo1-4", + approval: true, + deleteBranch: true, + }, + } { + // perform all tests with API and web routes + for _, withAPI := range []bool{false, true} { + t.Run(testCase.name, func(t *testing.T) { + protectedBranch := parameterProtectBranch{ + "enable_push": "true", + "enable_status_check": "true", + "status_check_contexts": "gitea/actions", + } + if testCase.approval { + protectedBranch["required_approvals"] = "1" + } + ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + doProtectBranch(ctx, "master", protectedBranch)(t) + + ctx = NewAPITestContext(t, "user1", "repo1", auth_model.AccessTokenScopeWriteRepository) + testPullAutoMergeAfterCommitStatusSucceed(t, ctx, testCase.forkName, testCase.approval, testCase.deleteBranch, withAPI) + }) + } + } }) }