diff --git a/common/config.go b/common/config.go index 602d916..7a4a53f 100644 --- a/common/config.go +++ b/common/config.go @@ -39,6 +39,10 @@ const ( Permission_ForceMerge = "force-merge" Permission_Group = "release-engineering" + + MergeModeFF = "ff-only" + MergeModeReplace = "replace" + MergeModeDevel = "devel" ) type ConfigFile struct { @@ -52,9 +56,9 @@ type ReviewGroup struct { } type QAConfig struct { - Name string - Origin string - Label string // requires this gitea lable to be set or skipped + Name string + Origin string + Label string // requires this gitea lable to be set or skipped BuildDisableRepos []string // which repos to build disable in the new project } @@ -89,7 +93,8 @@ type AutogitConfig struct { Committers []string // group in addition to Reviewers and Maintainers that can order the bot around, mostly as helper for factory-maintainers Subdirs []string // list of directories to sort submodules into. Needed b/c _manifest cannot list non-existent directories - Labels map[string]string // list of tags, if not default, to apply + Labels map[string]string // list of tags, if not default, to apply + MergeMode string // project merge mode NoProjectGitPR bool // do not automatically create project git PRs, just assign reviewers and assume somethign else creates the ProjectGit PR ManualMergeOnly bool // only merge with "Merge OK" comment by Project Maintainers and/or Package Maintainers and/or reviewers @@ -184,6 +189,17 @@ func ReadWorkflowConfig(gitea GiteaFileContentAndRepoFetcher, git_project string } } config.GitProjectName = config.GitProjectName + "#" + branch + + // verify merge modes + switch config.MergeMode { + case MergeModeFF, MergeModeDevel, MergeModeReplace: + break // good results + case "": + config.MergeMode = MergeModeFF + default: + return nil, fmt.Errorf("Unsupported merge mode in %s: %s", git_project, config.MergeMode) + } + return config, nil } diff --git a/common/config_test.go b/common/config_test.go index 2398cda..e223f8c 100644 --- a/common/config_test.go +++ b/common/config_test.go @@ -341,3 +341,67 @@ func TestConfigPermissions(t *testing.T) { }) } } + +func TestConfigMergeModeParser(t *testing.T) { + tests := []struct { + name string + json string + mergeMode string + wantErr bool + }{ + { + name: "empty", + json: "{}", + mergeMode: common.MergeModeFF, + }, + { + name: "ff-only", + json: `{"MergeMode": "ff-only"}`, + mergeMode: common.MergeModeFF, + }, + { + name: "replace", + json: `{"MergeMode": "replace"}`, + mergeMode: common.MergeModeReplace, + }, + { + name: "devel", + json: `{"MergeMode": "devel"}`, + mergeMode: common.MergeModeDevel, + }, + { + name: "unsupported", + json: `{"MergeMode": "invalid"}`, + wantErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + repo := models.Repository{ + DefaultBranch: "master", + } + + ctl := gomock.NewController(t) + gitea := mock_common.NewMockGiteaFileContentAndRepoFetcher(ctl) + gitea.EXPECT().GetRepositoryFileContent("foo", "bar", "", "workflow.config").Return([]byte(test.json), "abc", nil) + gitea.EXPECT().GetRepository("foo", "bar").Return(&repo, nil) + + config, err := common.ReadWorkflowConfig(gitea, "foo/bar") + if test.wantErr { + if err == nil { + t.Fatal("Expected error, got nil") + } + return + } + + if err != nil { + t.Fatal(err) + } + + if config.MergeMode != test.mergeMode { + t.Errorf("Expected MergeMode %s, got %s", test.mergeMode, config.MergeMode) + } + }) + } +} diff --git a/common/pr.go b/common/pr.go index 9ccf31c..6e93b78 100644 --- a/common/pr.go +++ b/common/pr.go @@ -554,6 +554,144 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData return is_manually_reviewed_ok } +func (rs *PRSet) AddMergeCommit(git Git, remote string, pr int) bool { + prinfo := rs.PRs[pr] + + LogDebug("Adding merge commit for %s", PRtoString(prinfo.PR)) + if !prinfo.PR.AllowMaintainerEdit { + LogError(" PR is not editable by maintainer") + return false + } + + repo := prinfo.PR.Base.Repo + head := prinfo.PR.Head + br := rs.Config.Branch + if len(br) == 0 { + br = prinfo.PR.Base.Name + } + + msg := fmt.Sprintf("Merge branch '%s' into %s", br, head.Name) + if err := git.GitExec(repo.Name, "merge", "--no-ff", "--no-commit", "-X", "theirs", head.Sha); err != nil { + if err := git.GitExec(repo.Name, "merge", "--no-ff", "--no-commit", "--allow-unrelated-histories", "-X", "theirs", head.Sha); err != nil { + return false + } + LogError("WARNING: Merging unrelated histories") + } + + // ensure only files that are in head.Sha are kept + git.GitExecOrPanic(repo.Name, "read-tree", "--reset", "-u", head.Sha) + git.GitExecOrPanic(repo.Name, "commit", "-m", msg) + + if !IsDryRun { + git.GitExecOrPanic(repo.Name, "push", remote, "HEAD:"+head.Name) + prinfo.PR.Head.Sha = strings.TrimSpace(git.GitExecWithOutputOrPanic(repo.Name, "rev-list", "-1", "HEAD")) // need to update as it's pushed but pr not refetched + } + + return true +} + +func (rs *PRSet) HasMerge(git Git, pr int) bool { + prinfo := rs.PRs[pr] + + repo := prinfo.PR.Base.Repo + head := prinfo.PR.Head + br := rs.Config.Branch + if len(br) == 0 { + br = prinfo.PR.Base.Name + } + + parents, err := git.GitExecWithOutput(repo.Name, "show", "-s", "--format=%P", head.Sha) + if err == nil { + p := strings.Fields(strings.TrimSpace(parents)) + if len(p) == 2 { + targetHead, _ := git.GitExecWithOutput(repo.Name, "rev-parse", "HEAD") + targetHead = strings.TrimSpace(targetHead) + if p[0] == targetHead || p[1] == targetHead { + return true + } + } + } + return false +} + +func (rs *PRSet) PrepareForMerge(git Git) bool { + // verify that package can merge here. Checkout current target branch of each PRSet, make a temporary branch + // PR_#_mergetest and perform the merge based + + if rs.Config.MergeMode == MergeModeDevel { + return true // always can merge as we set branch here, not merge anything + } else { + // make sure that all the package PRs are in mergeable state + for idx, prinfo := range rs.PRs { + if rs.IsPrjGitPR(prinfo.PR) { + continue + } + + repo := prinfo.PR.Base.Repo + head := prinfo.PR.Head + br := rs.Config.Branch + if len(br) == 0 { + br = prinfo.PR.Base.Name + } + + remote, err := git.GitClone(repo.Name, br, repo.SSHURL) + if err != nil { + return false + } + git.GitExecOrPanic(repo.Name, "fetch", remote, head.Sha) + switch rs.Config.MergeMode { + case MergeModeFF: + if err := git.GitExec(repo.Name, "merge-base", "--is-ancestor", "HEAD", head.Sha); err != nil { + return false + } + case MergeModeReplace: + Verify: + if err := git.GitExec(repo.Name, "merge-base", "--is-ancestor", "HEAD", head.Sha); err != nil { + if !rs.HasMerge(git, idx) { + forkRemote, err := git.GitClone(repo.Name, head.Name, head.Repo.SSHURL) + if err != nil { + LogError("Failed to clone head repo:", head.Name, head.Repo.SSHURL) + return false + } + LogDebug("Merge commit is missing and this is not FF merge possibility") + git.GitExecOrPanic(repo.Name, "checkout", remote+"/"+br) + if !rs.AddMergeCommit(git, forkRemote, idx) { + return false + } + if !IsDryRun { + goto Verify + } + } + } + } + } + } + + // now we check project git if mergeable + prjgit_info, err := rs.GetPrjGitPR() + if err != nil { + return false + } + prjgit := prjgit_info.PR + + _, _, prjgitBranch := rs.Config.GetPrjGit() + remote, err := git.GitClone(DefaultGitPrj, prjgitBranch, prjgit.Base.Repo.SSHURL) + if err != nil { + return false + } + + testBranch := fmt.Sprintf("PR_%d_mergetest", prjgit.Index) + git.GitExecOrPanic(DefaultGitPrj, "fetch", remote, prjgit.Head.Sha) + if err := git.GitExec(DefaultGitPrj, "checkout", "-B", testBranch, prjgit.Base.Sha); err != nil { + return false + } + if err := git.GitExec(DefaultGitPrj, "merge", "--no-ff", "--no-commit", prjgit.Head.Sha); err != nil { + return false + } + + return true +} + func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error { prjgit_info, err := rs.GetPrjGitPR() if err != nil { @@ -718,11 +856,10 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error { prinfo.RemoteName, err = git.GitClone(repo.Name, br, repo.SSHURL) PanicOnError(err) git.GitExecOrPanic(repo.Name, "fetch", prinfo.RemoteName, head.Sha) - - if isNewRepo { - LogInfo("Force-pushing new repository branch", br, "to", head.Sha) - // we don't merge, we just set the branch to this commit + if rs.Config.MergeMode == MergeModeDevel || isNewRepo { + git.GitExecOrPanic(repo.Name, "checkout", "-B", br, head.Sha) } else { + git.GitExecOrPanic(repo.Name, "fetch", prinfo.RemoteName, head.Sha) git.GitExecOrPanic(repo.Name, "merge", "--ff", head.Sha) } } @@ -748,11 +885,15 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error { } if !IsDryRun { - if isNewRepo { - git.GitExecOrPanic(repo.Name, "push", "-f", prinfo.RemoteName, prinfo.PR.Head.Sha+":"+prinfo.PR.Base.Name) - } else { - git.GitExecOrPanic(repo.Name, "push", prinfo.RemoteName) + params := []string{"push"} + if rs.Config.MergeMode == MergeModeDevel || isNewRepo { + params = append(params, "-f") } + params = append(params, prinfo.RemoteName) + if isNewRepo { + params = append(params, prinfo.PR.Head.Sha+":"+prinfo.PR.Base.Name) + } + git.GitExecOrPanic(repo.Name, params...) } else { LogInfo("*** WOULD push", repo.Name, "to", prinfo.RemoteName) } diff --git a/common/pr_test.go b/common/pr_test.go index 90059a9..d4574ab 100644 --- a/common/pr_test.go +++ b/common/pr_test.go @@ -2,6 +2,7 @@ package common_test import ( "errors" + "fmt" "os" "os/exec" "path" @@ -1267,7 +1268,7 @@ func TestPRMerge(t *testing.T) { Owner: &models.User{ UserName: "org", }, - SSHURL: "file://" + path.Join(repoDir, "prjgit"), + SSHURL: "ssh://git@src.opensuse.org/org/prj.git", }, }, Head: &models.PRBranchInfo{ @@ -1289,7 +1290,7 @@ func TestPRMerge(t *testing.T) { Owner: &models.User{ UserName: "org", }, - SSHURL: "file://" + path.Join(cmd.Dir, "prjgit"), + SSHURL: "ssh://git@src.opensuse.org/org/prj.git", }, }, Head: &models.PRBranchInfo{ @@ -1399,3 +1400,344 @@ func TestPRChanges(t *testing.T) { }) } } + +func TestPRPrepareForMerge(t *testing.T) { + tests := []struct { + name string + setup func(*mock_common.MockGit, *models.PullRequest, *models.PullRequest) + config *common.AutogitConfig + expected bool + editable bool + }{ + { + name: "Success Devel", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeDevel, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {}, + expected: true, + }, + { + name: "Success FF", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeFF, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(nil) + + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil) + }, + expected: true, + }, + { + name: "Success Replace MergeCommit", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeReplace, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + // merge-base fails initially + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor")) + // HasMerge returns true + m.EXPECT().GitExecWithOutput("pkg", "show", "-s", "--format=%P", pkgPR.Head.Sha).Return("parent1 target_head", nil) + m.EXPECT().GitExecWithOutput("pkg", "rev-parse", "HEAD").Return("target_head", nil) + + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil) + }, + expected: true, + }, + { + name: "Merge Conflict in PrjGit", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeFF, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(nil) + + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(fmt.Errorf("conflict")) + }, + expected: false, + }, + { + name: "Not FF in PkgGit", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeFF, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor")) + }, + expected: false, + }, + { + name: "Success Replace with AddMergeCommit", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeReplace, + }, + editable: true, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + // First merge-base fails + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor")) + // HasMerge returns false + m.EXPECT().GitExecWithOutput("pkg", "show", "-s", "--format=%P", pkgPR.Head.Sha).Return("parent1", nil) + m.EXPECT().GitClone("pkg", pkgPR.Head.Name, pkgPR.Base.Repo.SSHURL).Return("origin_fork", nil) + // AddMergeCommit is called + m.EXPECT().GitExecOrPanic("pkg", "checkout", "origin/master") + m.EXPECT().GitExec("pkg", "merge", "--no-ff", "--no-commit", "-X", "theirs", pkgPR.Head.Sha).Return(nil) + m.EXPECT().GitExecOrPanic("pkg", "read-tree", "--reset", "-u", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "commit", "-m", gomock.Any()) + m.EXPECT().GitExecOrPanic("pkg", "push", "origin_fork", "HEAD:"+pkgPR.Head.Name) + m.EXPECT().GitExecWithOutputOrPanic("pkg", "rev-list", "-1", "HEAD").Return("new_pkg_head_sha") + // Second merge-base succeeds (after goto Verify) + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", "new_pkg_head_sha").Return(nil) + + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil) + }, + expected: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + prjPR := &models.PullRequest{ + Index: 1, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "_ObsPrj", + SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git", + }, + }, + Head: &models.PRBranchInfo{ + Sha: "head_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "_ObsPrj", + SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git", + }, + }, + } + + pkgPR := &models.PullRequest{ + Index: 2, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "pkg_base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "pkg", + SSHURL: "ssh://git@src.opensuse.org/org/pkg.git", + }, + }, + Head: &models.PRBranchInfo{ + Name: "branch_name", + Sha: "pkg_head_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "pkg", + SSHURL: "ssh://git@src.opensuse.org/org/pkg.git", + }, + }, + AllowMaintainerEdit: test.editable, + } + + ctl := gomock.NewController(t) + git := mock_common.NewMockGit(ctl) + test.setup(git, prjPR, pkgPR) + + prset := &common.PRSet{ + Config: test.config, + PRs: []*common.PRInfo{ + {PR: prjPR}, + {PR: pkgPR}, + }, + } + + if res := prset.PrepareForMerge(git); res != test.expected { + t.Errorf("Expected %v, got %v", test.expected, res) + } + }) + } +} + +func TestPRMergeMock(t *testing.T) { + tests := []struct { + name string + setup func(*mock_common.MockGit, *models.PullRequest, *models.PullRequest) + config *common.AutogitConfig + }{ + { + name: "Success FF", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeFF, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "-m", gomock.Any(), prjPR.Head.Sha).Return(nil) + + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin_pkg", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin_pkg", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "merge", "--ff", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "push", "origin_pkg") + m.EXPECT().GitExecOrPanic("_ObsPrj", "push", "origin") + }, + }, + { + name: "Success Devel", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeDevel, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "-m", gomock.Any(), prjPR.Head.Sha).Return(nil) + + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin_pkg", nil) + m.EXPECT().GitExecOrPanic("pkg", "checkout", "-B", "master", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "push", "-f", "origin_pkg") + m.EXPECT().GitExecOrPanic("_ObsPrj", "push", "origin") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + prjPR := &models.PullRequest{ + Index: 1, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "prj_base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "_ObsPrj", + SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git", + }, + }, + Head: &models.PRBranchInfo{ + Sha: "prj_head_sha", + }, + } + pkgPR := &models.PullRequest{ + Index: 2, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "pkg_base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "pkg", + SSHURL: "ssh://git@src.opensuse.org/org/pkg.git", + }, + }, + Head: &models.PRBranchInfo{ + Sha: "pkg_head_sha", + }, + } + + ctl := gomock.NewController(t) + git := mock_common.NewMockGit(ctl) + reviewUnrequestMock := mock_common.NewMockGiteaReviewUnrequester(ctl) + reviewUnrequestMock.EXPECT().UnrequestReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil) + + test.setup(git, prjPR, pkgPR) + + prset := &common.PRSet{ + Config: test.config, + PRs: []*common.PRInfo{ + {PR: prjPR}, + {PR: pkgPR}, + }, + } + + if err := prset.Merge(reviewUnrequestMock, git); err != nil { + t.Errorf("Unexpected error: %v", err) + } + }) + } +} + +func TestPRAddMergeCommit(t *testing.T) { + pkgPR := &models.PullRequest{ + Index: 2, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "pkg_base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "pkg", + SSHURL: "ssh://git@src.opensuse.org/org/pkg.git", + }, + }, + Head: &models.PRBranchInfo{ + Name: "branch_name", + Sha: "pkg_head_sha", + }, + AllowMaintainerEdit: true, + } + + config := &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeReplace, + } + + ctl := gomock.NewController(t) + git := mock_common.NewMockGit(ctl) + + git.EXPECT().GitExec("pkg", "merge", "--no-ff", "--no-commit", "-X", "theirs", pkgPR.Head.Sha).Return(nil) + git.EXPECT().GitExecOrPanic("pkg", "read-tree", "--reset", "-u", pkgPR.Head.Sha) + git.EXPECT().GitExecOrPanic("pkg", "commit", "-m", gomock.Any()) + git.EXPECT().GitExecOrPanic("pkg", "push", "origin", "HEAD:branch_name") + git.EXPECT().GitExecWithOutputOrPanic("pkg", "rev-list", "-1", "HEAD").Return("new_head_sha") + + prset := &common.PRSet{ + Config: config, + PRs: []*common.PRInfo{ + {PR: &models.PullRequest{}}, // prjgit at index 0 + {PR: pkgPR}, // pkg at index 1 + }, + } + + if res := prset.AddMergeCommit(git, "origin", 1); !res { + t.Errorf("Expected true, got %v", res) + } +} + diff --git a/gitea-events-rabbitmq-publisher/rabbitmq.go b/gitea-events-rabbitmq-publisher/rabbitmq.go index 4a6c8d9..9c553b2 100644 --- a/gitea-events-rabbitmq-publisher/rabbitmq.go +++ b/gitea-events-rabbitmq-publisher/rabbitmq.go @@ -92,10 +92,13 @@ func ConnectToExchangeForPublish(host, username, password string) { auth = username + ":" + password + "@" } - connection, err := rabbitmq.DialTLS("amqps://"+auth+host, &tls.Config{ - ServerName: host, + connection, err := rabbitmq.DialConfig("amqps://"+auth+host, rabbitmq.Config{ + Dial: rabbitmq.DefaultDial(10 * time.Second), + TLSClientConfig: &tls.Config{ + ServerName: host, + }, }) - failOnError(err, "Cannot connect to rabbit.opensuse.org") + failOnError(err, "Cannot connect to "+host) defer connection.Close() ch, err := connection.Channel() diff --git a/integration/pytest.ini b/integration/pytest.ini index ab6f8f7..dc2f4a5 100644 --- a/integration/pytest.ini +++ b/integration/pytest.ini @@ -7,4 +7,10 @@ markers = t005: Test case 005 t006: Test case 006 t007: Test case 007 + t008: Test case 008 + t009: Test case 009 + t010: Test case 010 + t011: Test case 011 + t012: Test case 012 + t013: Test case 013 dependency: pytest-dependency marker diff --git a/integration/test-plan.md b/integration/test-plan.md index 97b40bf..63842e4 100644 --- a/integration/test-plan.md +++ b/integration/test-plan.md @@ -76,6 +76,12 @@ The testing will be conducted in a dedicated test environment that mimics the pr | **TC-MERGE-005** | - | **ManualMergeOnly with Project Maintainer** | 1. Create a PackageGit PR with `ManualMergeOnly` set to `true`.
2. Ensure all mandatory reviews are completed on both project and package PRs.
3. Comment "merge ok" on the package PR from the account of a project maintainer. | 1. The PR is merged. | High | | **TC-MERGE-006** | - | **ManualMergeProject with Project Maintainer** | 1. Create a PackageGit PR with `ManualMergeProject` set to `true`.
2. Ensure all mandatory reviews are completed on both project and package PRs.
3. Comment "merge ok" on the project PR from the account of a project maintainer. | 1. The PR is merged. | High | | **TC-MERGE-007** | - | **ManualMergeProject with unauthorized user** | 1. Create a PackageGit PR with `ManualMergeProject` set to `true`.
2. Ensure all mandatory reviews are completed on both project and package PRs.
3. Comment "merge ok" on the project PR from the account of a package maintainer. | 1. The PR is not merged. | High | +| **TC-MERGE-008** | P | **MergeMode: ff-only (Success)** | 1. Set `MergeMode = "ff-only"`.
2. Create a FF-mergeable PackageGit PR.
3. Approve reviews on both PRs. | 1. Both PRs are automatically merged successfully. | High | +| **TC-MERGE-009** | P | **MergeMode: ff-only (Failure)** | 1. Set `MergeMode = "ff-only"`.
2. Create a PackageGit PR that adds a new file.
3. Commit the same file with different content to the base branch to create a content conflict.
4. Approve reviews and trigger a sync by pushing another change. | 1. The bot detects it is not FF-mergeable.
2. The PR is NOT merged. | High | +| **TC-MERGE-010** | P | **MergeMode: devel (Force-push)** | 1. Set `MergeMode = "devel"`.
2. Create a PackageGit PR that adds a new file.
3. Commit the same file with different content to the base branch to create a content conflict.
4. Approve reviews. | 1. Both PRs are merged.
2. The `pkgA` submodule points to the PR's head SHA. | High | +| **TC-MERGE-011** | P | **MergeMode: replace (Merge-commit)** | 1. Set `MergeMode = "replace"`.
2. Create a PackageGit PR that adds a new file.
3. Enable "Allow edits from maintainers" on the PR.
4. Commit the same file with different content to the base branch to create a content conflict.
5. Approve reviews. | 1. Both PRs are merged.
2. The project branch HEAD is a merge commit (has >1 parent).
3. The `pkgA` submodule points to the PR's head SHA. | High | +| **TC-MERGE-012** | P | **MergeMode: devel (No Conflict, Fast-forward)** | 1. Set `MergeMode = "devel"`.
2. Create a FF-mergeable PackageGit PR.
3. Approve reviews. | 1. Both PRs are merged.
2. The package branch HEAD matches the PR head (FF). | High | +| **TC-MERGE-013** | P | **MergeMode: replace (No Conflict, Fast-forward)** | 1. Set `MergeMode = "replace"`.
2. Create a FF-mergeable PackageGit PR.
3. Approve reviews. | 1. Both PRs are merged.
2. The package branch HEAD matches the PR head (FF). | High | | **TC-CONFIG-001** | - | **Invalid Configuration** | 1. Provide an invalid `workflow.config` file. | 1. The bot reports an error and does not process any PRs. | High | | **TC-LABEL-001** | P | **Apply `staging/Auto` label** | 1. Create a new PackageGit PR. | 1. The `staging/Auto` label is applied to the ProjectGit PR. | High | | **TC-LABEL-002** | x | **Apply `review/Pending` label** | 1. Create a new PackageGit PR. | 1. The `review/Pending` label is applied to the ProjectGit PR when there are pending reviews. | Medium | diff --git a/integration/tests/conftest.py b/integration/tests/conftest.py index 0718bf2..80d5104 100644 --- a/integration/tests/conftest.py +++ b/integration/tests/conftest.py @@ -71,6 +71,21 @@ BRANCH_CONFIG_CUSTOM = { "ReviewPending": "review/Pending" } } + }, + "merge-ff": { + "workflow.config": { + "MergeMode": "ff-only" + } + }, + "merge-replace": { + "workflow.config": { + "MergeMode": "replace" + } + }, + "merge-devel": { + "workflow.config": { + "MergeMode": "devel" + } } } @@ -275,6 +290,18 @@ def no_project_git_pr_env(gitea_env): def label_env(gitea_env): return gitea_env, "myproducts/mySLFO", "label-test" +@pytest.fixture(scope="session") +def merge_ff_env(gitea_env): + return gitea_env, "myproducts/mySLFO", "merge-ff" + +@pytest.fixture(scope="session") +def merge_replace_env(gitea_env): + return gitea_env, "myproducts/mySLFO", "merge-replace" + +@pytest.fixture(scope="session") +def merge_devel_env(gitea_env): + return gitea_env, "myproducts/mySLFO", "merge-devel" + @pytest.fixture(scope="session") def usera_client(gitea_env): return GiteaAPIClient(base_url=gitea_env.base_url, token=gitea_env.headers["Authorization"].split(" ")[1], sudo="usera") diff --git a/integration/tests/lib/common_test_utils.py b/integration/tests/lib/common_test_utils.py index 54007d3..477584a 100644 --- a/integration/tests/lib/common_test_utils.py +++ b/integration/tests/lib/common_test_utils.py @@ -3,6 +3,7 @@ import time import pytest import requests import json +import re import xml.etree.ElementTree as ET from pathlib import Path import base64 @@ -272,12 +273,12 @@ index 0000000..{pkg_b_sha} owner, repo = repo_full_name.split("/") head_owner, head_repo = owner, repo - + new_branch_name = f"pr-branch-{int(time.time()*1000)}" + if use_fork: sudo_user = self.headers.get("Sudo") head_owner = sudo_user head_repo = repo - new_branch_name = f"pr-branch-{int(time.time()*1000)}" print(f"--- Forking {repo_full_name} ---") try: @@ -290,29 +291,11 @@ index 0000000..{pkg_b_sha} else: raise - # Create a unique branch in the FORK - base_commit_sha = self._request("GET", f"repos/{owner}/{repo}/branches/{base_branch}").json()["commit"]["id"] - print(f"--- Creating branch {new_branch_name} in {head_owner}/{head_repo} from {base_branch} ({base_commit_sha}) ---") - self._request("POST", f"repos/{head_owner}/{head_repo}/branches", json={ - "new_branch_name": new_branch_name, - "old_ref": base_commit_sha - }) - else: - new_branch_name = f"pr-branch-{int(time.time()*1000)}" - # Get the latest commit SHA of the base branch from the ORIGINAL repo - base_commit_sha = self._request("GET", f"repos/{owner}/{repo}/branches/{base_branch}").json()["commit"]["id"] - - # Try to create the branch in the ORIGINAL repo - print(f"--- Creating branch {new_branch_name} in {repo_full_name} ---") - self._request("POST", f"repos/{owner}/{repo}/branches", json={ - "new_branch_name": new_branch_name, - "old_ref": base_commit_sha - }) - - # Apply the diff using diffpatch in the branch (wherever it is) - print(f"--- Applying diff to {head_owner}/{head_repo} branch {new_branch_name} ---") + # Apply the diff using diffpatch and create the new branch automatically + print(f"--- Applying diff to {head_owner}/{head_repo} from {base_branch} to new branch {new_branch_name} ---") self._request("POST", f"repos/{head_owner}/{head_repo}/diffpatch", json={ - "branch": new_branch_name, + "branch": base_branch, + "new_branch": new_branch_name, "content": diff_content, "message": title }) @@ -530,3 +513,45 @@ index 0000000..{pkg_b_sha} print(f"Error restarting service {service_name}: {e}") raise + def wait_for_project_pr(self, package_pr_repo, package_pr_number, project_pr_repo="myproducts/mySLFO", timeout=60): + print(f"Polling {package_pr_repo} PR #{package_pr_number} timeline for forwarded PR event in {project_pr_repo}...") + for _ in range(timeout): + time.sleep(1) + timeline_events = self.get_timeline_events(package_pr_repo, package_pr_number) + for event in timeline_events: + if event.get("type") == "pull_ref": + if not (ref_issue := event.get("ref_issue")): + continue + url_to_check = ref_issue.get("html_url", "") + match = re.search(fr"{project_pr_repo}/pulls/(\d+)", url_to_check) + if match: + return int(match.group(1)) + return None + + def approve_and_wait_merge(self, package_pr_repo, package_pr_number, project_pr_number, project_pr_repo="myproducts/mySLFO", timeout=30): + print(f"Approving reviews and verifying both PRs are merged ({package_pr_repo}#{package_pr_number} and {project_pr_repo}#{project_pr_number})...") + package_merged = False + project_merged = False + + for i in range(timeout): + self.approve_requested_reviews(package_pr_repo, package_pr_number) + self.approve_requested_reviews(project_pr_repo, project_pr_number) + + if not package_merged: + pkg_details = self.get_pr_details(package_pr_repo, package_pr_number) + if pkg_details.get("merged"): + package_merged = True + print(f"Package PR {package_pr_repo}#{package_pr_number} merged.") + + if not project_merged: + prj_details = self.get_pr_details(project_pr_repo, project_pr_number) + if prj_details.get("merged"): + project_merged = True + print(f"Project PR {project_pr_repo}#{project_pr_number} merged.") + + if package_merged and project_merged: + return True, True + + time.sleep(1) + return package_merged, project_merged + diff --git a/integration/tests/test_pr_workflow.py b/integration/tests/test_pr_workflow.py index 416b159..9264866 100755 --- a/integration/tests/test_pr_workflow.py +++ b/integration/tests/test_pr_workflow.py @@ -23,27 +23,10 @@ def test_pr_workflow_succeeded(staging_main_env, mock_build_result): compose_dir = Path(__file__).parent.parent - forwarded_pr_number = None - print( - f"Polling mypool/pkgA PR #{initial_pr_number} timeline for forwarded PR event..." - ) - for _ in range(20): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", initial_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - forwarded_pr_number = match.group(1) - break - if forwarded_pr_number: - break + forwarded_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", initial_pr_number) assert ( forwarded_pr_number is not None - ), "Workflow bot did not create a pull_ref event on the timeline." + ), "Workflow bot did not create a project PR." print(f"Found forwarded PR: myproducts/mySLFO #{forwarded_pr_number}") print(f"Polling myproducts/mySLFO PR #{forwarded_pr_number} for reviewer assignment...") @@ -94,27 +77,10 @@ def test_pr_workflow_failed(staging_main_env, mock_build_result): compose_dir = Path(__file__).parent.parent - forwarded_pr_number = None - print( - f"Polling mypool/pkgA PR #{initial_pr_number} timeline for forwarded PR event..." - ) - for _ in range(20): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", initial_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - forwarded_pr_number = match.group(1) - break - if forwarded_pr_number: - break + forwarded_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", initial_pr_number) assert ( forwarded_pr_number is not None - ), "Workflow bot did not create a pull_ref event on the timeline." + ), "Workflow bot did not create a project PR." print(f"Found forwarded PR: myproducts/mySLFO #{forwarded_pr_number}") print(f"Polling myproducts/mySLFO PR #{forwarded_pr_number} for reviewer assignment...") diff --git a/integration/tests/workflow_pr_label_test.py b/integration/tests/workflow_pr_label_test.py index 6fa8469..a95da9c 100644 --- a/integration/tests/workflow_pr_label_test.py +++ b/integration/tests/workflow_pr_label_test.py @@ -35,23 +35,7 @@ index 0000000..e69de29 print(f"Created package PR mypool/pkgA#{package_pr_number}") # 2. Make sure the workflow-pr service created related project PR - project_pr_number = None - print(f"Polling mypool/pkgA PR #{package_pr_number} timeline for forwarded PR event...") - for _ in range(40): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", package_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - project_pr_number = int(match.group(1)) - break - if project_pr_number: - break - + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) assert project_pr_number is not None, "Workflow bot did not create a project PR." print(f"Found project PR: myproducts/mySLFO#{project_pr_number}") diff --git a/integration/tests/workflow_pr_merge_test.py b/integration/tests/workflow_pr_merge_test.py index f01a178..6e0050f 100644 --- a/integration/tests/workflow_pr_merge_test.py +++ b/integration/tests/workflow_pr_merge_test.py @@ -25,55 +25,14 @@ index 0000000..e69de29 print(f"Created package PR mypool/pkgA#{package_pr_number}") # 2. Make sure the workflow-pr service created related project PR - project_pr_number = None - print(f"Polling mypool/pkgA PR #{package_pr_number} timeline for forwarded PR event...") - for _ in range(40): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", package_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - project_pr_number = int(match.group(1)) - break - if project_pr_number: - break - + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) assert project_pr_number is not None, "Workflow bot did not create a project PR." print(f"Found project PR: myproducts/mySLFO#{project_pr_number}") # 3. Approve reviews and verify merged - print("Approving reviews and verifying both PRs are merged...") - package_merged = False - project_merged = False - - for i in range(20): # Poll for up to 20 seconds - gitea_env.approve_requested_reviews("mypool/pkgA", package_pr_number) - gitea_env.approve_requested_reviews("myproducts/mySLFO", project_pr_number) - - if not package_merged: - pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number) - if pkg_details.get("merged"): - package_merged = True - print(f"Package PR mypool/pkgA#{package_pr_number} merged.") - - # Project PR - if not project_merged: - prj_details = gitea_env.get_pr_details("myproducts/mySLFO", project_pr_number) - if prj_details.get("merged"): - project_merged = True - print(f"Project PR myproducts/mySLFO#{project_pr_number} merged.") - - if package_merged and project_merged: - break - - time.sleep(1) - - assert package_merged, f"Package PR mypool/pkgA#{package_pr_number} was not merged automatically." - assert project_merged, f"Project PR myproducts/mySLFO#{project_pr_number} was not merged automatically." + pkg_merged, prj_merged = gitea_env.approve_and_wait_merge("mypool/pkgA", package_pr_number, project_pr_number) + assert pkg_merged, f"Package PR mypool/pkgA#{package_pr_number} was not merged automatically." + assert prj_merged, f"Project PR myproducts/mySLFO#{project_pr_number} was not merged automatically." print("Both PRs merged successfully.") @pytest.mark.t002 @@ -98,23 +57,7 @@ index 0000000..e69de29 print(f"Created package PR mypool/pkgA#{package_pr_number}") # 2. Make sure the workflow-pr service created related project PR - project_pr_number = None - print(f"Polling mypool/pkgA PR #{package_pr_number} timeline for forwarded PR event...") - for _ in range(40): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", package_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - project_pr_number = int(match.group(1)) - break - if project_pr_number: - break - + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) assert project_pr_number is not None, "Workflow bot did not create a project PR." print(f"Found project PR: myproducts/mySLFO#{project_pr_number}") @@ -193,3 +136,253 @@ index 0000000..e69de29 assert package_merged, f"Package PR mypool/pkgA#{package_pr_number} was not merged after 'merge ok'." assert project_merged, f"Project PR myproducts/mySLFO#{project_pr_number} was not merged after 'merge ok'." print("Both PRs merged successfully after 'merge ok'.") + +@pytest.mark.t008 +def test_008_merge_mode_ff_only_success(merge_ff_env, test_user_client): + """ + Test MergeMode "ff-only" - Success case (FF-mergeable) + """ + gitea_env, test_full_repo_name, merge_branch_name = merge_ff_env + + # 1. Create a package PR (this will be FF-mergeable by default) + diff = """diff --git a/ff_test.txt b/ff_test.txt +new file mode 100644 +index 0000000..e69de29 +""" + package_pr = test_user_client.create_gitea_pr("mypool/pkgA", diff, "Test FF Merge", False, base_branch=merge_branch_name) + package_pr_number = package_pr["number"] + + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) + assert project_pr_number is not None + + pkg_merged, prj_merged = gitea_env.approve_and_wait_merge("mypool/pkgA", package_pr_number, project_pr_number) + assert pkg_merged and prj_merged + +@pytest.mark.t009 +def test_009_merge_mode_ff_only_failure(merge_ff_env, ownerA_client): + """ + Test MergeMode "ff-only" - Failure case (Content Conflict, should NOT merge) + """ + gitea_env, test_full_repo_name, merge_branch_name = merge_ff_env + + ts = time.strftime("%H%M%S") + filename = f"ff_fail_test_{ts}.txt" + + # 1. Create a package PR that adds a file + diff = f"""diff --git a/{filename} b/{filename} +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/{filename} +@@ -0,0 +1 @@ ++PR content +""" + package_pr = ownerA_client.create_gitea_pr("mypool/pkgA", diff, "Test FF Merge Failure (Conflict)", False, base_branch=merge_branch_name) + package_pr_number = package_pr["number"] + + # 2. Wait for project PR to be created + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) + assert project_pr_number is not None + + print("Making PR non-FF by creating a content conflict in the base branch...") + gitea_env.create_file("mypool", "pkgA", filename, "Conflicting base content\n", branch=merge_branch_name) + + print("Approving reviews initially...") + gitea_env.approve_requested_reviews("mypool/pkgA", package_pr_number) + gitea_env.approve_requested_reviews("myproducts/mySLFO", project_pr_number) + + print("Pushing another change to PR branch to trigger sync...") + gitea_env.modify_gitea_pr("mypool/pkgA", package_pr_number, + "diff --git a/sync_test.txt b/sync_test.txt\nnew file mode 100644\nindex 0000000..e69de29\n", + "Trigger Sync") + + # The bot should detect it's not FF and NOT merge, and re-request reviews because of the new commit + print("Waiting for reviews to be re-requested and approving again...") + time.sleep(10) # Wait for bot to process sync + + # Approve again and verify it is NOT merged + print("Approving again and verifying PR is NOT merged (because it's not FF)...") + for i in range(15): + gitea_env.approve_requested_reviews("mypool/pkgA", package_pr_number) + gitea_env.approve_requested_reviews("myproducts/mySLFO", project_pr_number) + time.sleep(1) + + pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number) + assert not pkg_details.get("merged"), "Package PR merged despite NOT being FF-mergeable!" + + print("FF-only failure (not merged after sync) verified.") + +@pytest.mark.t010 +def test_010_merge_mode_devel_success(merge_devel_env, ownerA_client): + """ + Test MergeMode "devel" - Success case (Content Conflict, should still merge via force-push) + """ + gitea_env, test_full_repo_name, merge_branch_name = merge_devel_env + + ts = time.strftime("%H%M%S") + filename = f"devel_test_{ts}.txt" + + # 1. Create a package PR that adds a file + diff = f"""diff --git a/{filename} b/{filename} +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/{filename} +@@ -0,0 +1 @@ ++PR content +""" + package_pr = ownerA_client.create_gitea_pr("mypool/pkgA", diff, "Test Devel Merge (Conflict)", False, base_branch=merge_branch_name) + package_pr_number = package_pr["number"] + + # 2. Create a content conflict by committing the same file to the base branch + gitea_env.create_file("mypool", "pkgA", filename, "Conflicting base content\n", branch=merge_branch_name) + + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) + assert project_pr_number is not None + + # Before merge, get the head sha of the package pr and project pr + pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number) + pkg_head_sha = pkg_details["head"]["sha"] + prj_details = gitea_env.get_pr_details("myproducts/mySLFO", project_pr_number) + prj_head_sha = prj_details["head"]["sha"] + + pkg_merged, prj_merged = gitea_env.approve_and_wait_merge("mypool/pkgA", package_pr_number, project_pr_number) + assert pkg_merged and prj_merged + print("Devel merge (force-push) successful.") + + # Verify that pkgA submodule points to the correct SHA + pkgA_submodule_info = gitea_env.get_file_info("myproducts", "mySLFO", "pkgA", branch=merge_branch_name) + assert pkgA_submodule_info["sha"] == pkg_head_sha, f"Submodule pkgA should point to {pkg_head_sha} but points to {pkgA_submodule_info['sha']}" + +@pytest.mark.t011 +def test_011_merge_mode_replace_success(merge_replace_env, ownerA_client): + """ + Test MergeMode "replace" - Success case (Content Conflict, bot should add merge commit) + """ + gitea_env, test_full_repo_name, merge_branch_name = merge_replace_env + + ts = time.strftime("%H%M%S") + filename = f"replace_test_{ts}.txt" + + # 1. Create a package PR that adds a file + diff = f"""diff --git a/{filename} b/{filename} +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/{filename} +@@ -0,0 +1 @@ ++PR content +""" + package_pr = ownerA_client.create_gitea_pr("mypool/pkgA", diff, "Test Replace Merge (Conflict)", False, base_branch=merge_branch_name) + package_pr_number = package_pr["number"] + + # Enable "Allow edits from maintainers" + ownerA_client.update_gitea_pr_properties("mypool/pkgA", package_pr_number, allow_maintainer_edit=True) + + # 2. Create a content conflict by committing the same file to the base branch + gitea_env.create_file("mypool", "pkgA", filename, "Conflicting base content\n", branch=merge_branch_name) + + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) + assert project_pr_number is not None + + # Before merge, get the head sha of the package pr and project pr + pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number) + pkg_head_sha = pkg_details["head"]["sha"] + prj_details = gitea_env.get_pr_details("myproducts/mySLFO", project_pr_number) + prj_head_sha = prj_details["head"]["sha"] + + pkg_merged, prj_merged = gitea_env.approve_and_wait_merge("mypool/pkgA", package_pr_number, project_pr_number, timeout=60) + assert pkg_merged and prj_merged + print("Replace merge successful.") + + # Verify that the project branch HEAD is a merge commit + branch_info = gitea_env._request("GET", f"repos/myproducts/mySLFO/branches/{merge_branch_name}").json() + new_head_sha = branch_info["commit"]["id"] + + commit_details = gitea_env._request("GET", f"repos/myproducts/mySLFO/git/commits/{new_head_sha}").json() + assert len(commit_details["parents"]) > 1, f"Project branch {merge_branch_name} HEAD should be a merge commit but has {len(commit_details['parents'])} parents" + + # Verify that pkgA submodule points to the correct SHA + pkgA_submodule_info = gitea_env.get_file_info("myproducts", "mySLFO", "pkgA", branch=merge_branch_name) + assert pkgA_submodule_info["sha"] == pkg_head_sha, f"Submodule pkgA should point to {pkg_head_sha} but points to {pkgA_submodule_info['sha']}" + +@pytest.mark.t012 +def test_012_merge_mode_devel_ff_success(merge_devel_env, ownerA_client): + """ + Test MergeMode "devel" - Success case (No Conflict, should fast-forward) + """ + gitea_env, test_full_repo_name, merge_branch_name = merge_devel_env + + ts = time.strftime("%H%M%S") + filename = f"devel_ff_test_{ts}.txt" + + # 1. Create a package PR (this will be FF-mergeable by default) + diff = f"""diff --git a/{filename} b/{filename} +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/{filename} +@@ -0,0 +1 @@ ++PR content +""" + package_pr = ownerA_client.create_gitea_pr("mypool/pkgA", diff, "Test Devel FF Merge", False, base_branch=merge_branch_name) + package_pr_number = package_pr["number"] + + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) + assert project_pr_number is not None + + pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number) + pkg_head_sha = pkg_details["head"]["sha"] + + pkg_merged, prj_merged = gitea_env.approve_and_wait_merge("mypool/pkgA", package_pr_number, project_pr_number) + assert pkg_merged and prj_merged + print("Devel FF merge successful.") + + # Verify that the package base branch HEAD is the same as the PR head (FF) + branch_info = gitea_env._request("GET", f"repos/mypool/pkgA/branches/{merge_branch_name}").json() + new_head_sha = branch_info["commit"]["id"] + assert new_head_sha == pkg_head_sha, f"Package branch {merge_branch_name} HEAD should be {pkg_head_sha} but is {new_head_sha}" + + commit_details = gitea_env._request("GET", f"repos/mypool/pkgA/git/commits/{new_head_sha}").json() + assert len(commit_details["parents"]) == 1, f"Package branch {merge_branch_name} HEAD should have 1 parent but has {len(commit_details['parents'])}" + +@pytest.mark.t013 +def test_013_merge_mode_replace_ff_success(merge_replace_env, ownerA_client): + """ + Test MergeMode "replace" - Success case (No Conflict, should fast-forward) + """ + gitea_env, test_full_repo_name, merge_branch_name = merge_replace_env + + ts = time.strftime("%H%M%S") + filename = f"replace_ff_test_{ts}.txt" + + # 1. Create a package PR (this will be FF-mergeable by default) + diff = f"""diff --git a/{filename} b/{filename} +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/{filename} +@@ -0,0 +1 @@ ++PR content +""" + package_pr = ownerA_client.create_gitea_pr("mypool/pkgA", diff, "Test Replace FF Merge", False, base_branch=merge_branch_name) + package_pr_number = package_pr["number"] + + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) + assert project_pr_number is not None + + pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number) + pkg_head_sha = pkg_details["head"]["sha"] + + pkg_merged, prj_merged = gitea_env.approve_and_wait_merge("mypool/pkgA", package_pr_number, project_pr_number) + assert pkg_merged and prj_merged + print("Replace FF merge successful.") + + # Verify that the package base branch HEAD is the same as the PR head (FF) + branch_info = gitea_env._request("GET", f"repos/mypool/pkgA/branches/{merge_branch_name}").json() + new_head_sha = branch_info["commit"]["id"] + assert new_head_sha == pkg_head_sha, f"Package branch {merge_branch_name} HEAD should be {pkg_head_sha} but is {new_head_sha}" + + commit_details = gitea_env._request("GET", f"repos/mypool/pkgA/git/commits/{new_head_sha}").json() + assert len(commit_details["parents"]) == 1, f"Package branch {merge_branch_name} HEAD should have 1 parent but has {len(commit_details['parents'])}" + diff --git a/integration/tests/workflow_pr_review_test.py b/integration/tests/workflow_pr_review_test.py index f3985c8..834ac35 100644 --- a/integration/tests/workflow_pr_review_test.py +++ b/integration/tests/workflow_pr_review_test.py @@ -94,23 +94,7 @@ index 0000000..e69de29 print(f"Created package PR mypool/pkgA#{package_pr_number}") # 2. Make sure the workflow-pr service created related project PR - project_pr_number = None - print(f"Polling mypool/pkgA PR #{package_pr_number} timeline for forwarded PR event...") - for _ in range(40): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", package_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - project_pr_number = int(match.group(1)) - break - if project_pr_number: - break - + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number) assert project_pr_number is not None, "Workflow bot did not create a project PR." print(f"Found project PR: myproducts/mySLFO#{project_pr_number}") @@ -179,23 +163,7 @@ index 0000000..e69de29 print(f"Created package PR mypool/pkgB#{package_pr_number}") # 2. Make sure the workflow-pr service created related project PR - project_pr_number = None - print(f"Polling mypool/pkgB PR #{package_pr_number} timeline for forwarded PR event...") - for _ in range(40): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgB", package_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - project_pr_number = int(match.group(1)) - break - if project_pr_number: - break - + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgB", package_pr_number) assert project_pr_number is not None, "Workflow bot did not create a project PR." print(f"Found project PR: myproducts/mySLFO#{project_pr_number}") @@ -317,23 +285,7 @@ index 0000000..e69de29 print(f"Created package PR mypool/pkgB#{package_pr_number}") # 2. Make sure the workflow-pr service created related project PR - project_pr_number = None - print(f"Polling mypool/pkgB PR #{package_pr_number} timeline for forwarded PR event...") - for _ in range(40): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgB", package_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - project_pr_number = int(match.group(1)) - break - if project_pr_number: - break - + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgB", package_pr_number) assert project_pr_number is not None, "Workflow bot did not create a project PR." print(f"Found project PR: myproducts/mySLFO#{project_pr_number}") diff --git a/integration/tests/workflow_pr_sync_test.py b/integration/tests/workflow_pr_sync_test.py index b9590c5..dd8dc17 100755 --- a/integration/tests/workflow_pr_sync_test.py +++ b/integration/tests/workflow_pr_sync_test.py @@ -27,27 +27,8 @@ def test_001_project_pr(gitea_env): pytest.initial_pr_number = pytest.pr["number"] time.sleep(5) # Give Gitea some time to process the PR and make the timeline available - compose_dir = Path(__file__).parent.parent - - pytest.forwarded_pr_number = None - print( - f"Polling mypool/pkgA PR #{pytest.initial_pr_number} timeline for forwarded PR event..." - ) - # Instead of polling timeline, check if forwarded PR exists directly - for _ in range(20): - time.sleep(1) - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", pytest.initial_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - pytest.forwarded_pr_number = match.group(1) - break - if pytest.forwarded_pr_number: - break + pytest.forwarded_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", pytest.initial_pr_number) + assert ( pytest.forwarded_pr_number is not None ), "Workflow bot did not create a forwarded PR." @@ -144,23 +125,9 @@ index 0000000..e69de29 print(f"Created Package PR #{package_pr_number}") # 2. Verify that the workflow-pr bot did not create a Project PR - project_pr_created = False - for i in range(10): # Poll for some time - time.sleep(2) - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", package_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - project_pr_created = True - break - if project_pr_created: - break + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number, timeout=10) - assert not project_pr_created, "Workflow bot unexpectedly created a Project PR in myproducts/mySLFO." + assert project_pr_number is None, "Workflow bot unexpectedly created a Project PR in myproducts/mySLFO." print("Verification complete: No Project PR was created by the bot.") # 3. Manually create the Project PR @@ -252,24 +219,9 @@ index 0000000..e69de29 pkgA_pr_head_sha = package_pr_details["head"]["sha"] # 3. Assert that the workflow-pr bot did not create a Project PR in the myproducts/mySLFO repository - project_pr_created = False - for i in range(20): # Poll for a reasonable time - time.sleep(2) # Wait a bit longer to be sure - timeline_events = gitea_env.get_timeline_events("mypool/pkgA", package_pr_number) - for event in timeline_events: - if event.get("type") == "pull_ref": - if not (ref_issue := event.get("ref_issue")): - continue - url_to_check = ref_issue.get("html_url", "") - # Regex now searches for myproducts/mySLFO/pulls/(\d+) - match = re.search(r"myproducts/mySLFO/pulls/(\d+)", url_to_check) - if match: - project_pr_created = True - break - if project_pr_created: - break + project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number, timeout=10) - assert not project_pr_created, "Workflow bot unexpectedly created a Project PR in myproducts/mySLFO." + assert project_pr_number is None, "Workflow bot unexpectedly created a Project PR in myproducts/mySLFO." print("Verification complete: No Project PR was created in myproducts/mySLFO as expected.") # 1. Create that Project PR from the test code. @@ -322,5 +274,3 @@ index 0000000..f587a12 assert project_pr_updated, "Manually created Project PR was not updated by the bot." print("Verification complete: Manually created Project PR was updated by the bot as expected.") - - diff --git a/integration/workflow-pr/workflow-pr.json b/integration/workflow-pr/workflow-pr.json index 15401a6..479fe91 100644 --- a/integration/workflow-pr/workflow-pr.json +++ b/integration/workflow-pr/workflow-pr.json @@ -6,5 +6,8 @@ "myproducts/mySLFO#maintainer-merge", "myproducts/mySLFO#review-required", "myproducts/mySLFO#label-test", - "myproducts/mySLFO#manual-merge" + "myproducts/mySLFO#manual-merge", + "myproducts/mySLFO#merge-ff", + "myproducts/mySLFO#merge-replace", + "myproducts/mySLFO#merge-devel" ] diff --git a/workflow-pr/README.md b/workflow-pr/README.md index 62bb463..3854a36 100644 --- a/workflow-pr/README.md +++ b/workflow-pr/README.md @@ -96,6 +96,19 @@ Package Deletion Requests If you wish to re-add a package, create a new PrjGit PR which adds again the submodule on the branch that has the "-removed" suffix. The bot will automatically remove this suffix from the project branch in the pool. +Merge Modes +----------- + +| Merge Mode | Description +|------------|-------------------------------------------------------------------------------- +| ff-only | Only allow --ff-only merges in the package branch. This is best suited for +| | devel projects and openSUSE Tumbleweed development, where history should be linear +| replace | Merge is done via `-X theirs` strategy and old files are removed in the merge. +| | This works well for downstream codestreams, like Leap, that would update their branch +| | using latest version. +| devel | No merge, just set the project branch to PR HEAD. This is suitable for downstream +| | projects like Leap during development cycle, where keeping maintenance history is not important + Labels ------ diff --git a/workflow-pr/pr_processor.go b/workflow-pr/pr_processor.go index 6d82369..9f1686d 100644 --- a/workflow-pr/pr_processor.go +++ b/workflow-pr/pr_processor.go @@ -417,6 +417,12 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error { } common.LogInfo("fetched PRSet of size:", len(prset.PRs)) + if !prset.PrepareForMerge(git) { + common.LogError("PRs are NOT mergeable.") + } else { + common.LogInfo("PRs are in mergeable state.") + } + prjGitPRbranch := prGitBranchNameForPR(prRepo, prNo) prjGitPR, err := prset.GetPrjGitPR() if err == common.PRSet_PrjGitMissing {