test1

Timeline events will contain Reviews and ReviewRequests and
ReviewDismissed events. We need to handle this at event parsing
time and not to punt this to the query functions later on.

If the last event is an actual review, we use this.
If no review, check if last event associated with the reviewer
is Dismissed or Requested Review but not if a dismissed Review
preceeds it.

common/config.go

@@ -39,10 +39,6 @@ const (
 
 	Permission_ForceMerge = "force-merge"
 	Permission_Group      = "release-engineering"
-
-	MergeModeFF      = "ff-only"
-	MergeModeReplace = "replace"
-	MergeModeDevel   = "devel"
 )
 
 type ConfigFile struct {
@@ -56,9 +52,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
 }
 
@@ -93,8 +89,7 @@ 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
-	MergeMode string            // project merge mode
+	Labels map[string]string // list of tags, if not default, to apply
 
 	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
@@ -189,17 +184,6 @@ 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
 }
 

common/config_test.go

@@ -341,67 +341,3 @@ 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)
-			}
-		})
-	}
-}

common/pr.go

@@ -468,7 +468,7 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 				LogError("Cannot fetch gita reaviews for PR:", err)
 				return false
 			}
-			r.RequestedReviewers = reviewers
+			r.SetRequiredReviewers(reviewers)
 			prjgit.Reviews = r
 			if prjgit.Reviews.IsManualMergeOK() {
 				is_manually_reviewed_ok = true
@@ -489,7 +489,7 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 					LogError("Cannot fetch gita reaviews for PR:", err)
 					return false
 				}
-				r.RequestedReviewers = reviewers
+				r.SetRequiredReviewers(reviewers)
 				pr.Reviews = r
 				if !pr.Reviews.IsManualMergeOK() {
 					LogInfo("Not approved manual merge. PR:", pr.PR.URL)
@@ -530,7 +530,7 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 			LogError("Cannot fetch gitea reaviews for PR:", err)
 			return false
 		}
-		r.RequestedReviewers = reviewers
+		r.SetRequiredReviewers(reviewers)
 
 		is_manually_reviewed_ok = r.IsApproved()
 		LogDebug("PR to", pr.PR.Base.Repo.Name, "reviewed?", is_manually_reviewed_ok)
@@ -554,144 +554,6 @@ 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 {
@@ -856,10 +718,11 @@ 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 rs.Config.MergeMode == MergeModeDevel || isNewRepo {
-			git.GitExecOrPanic(repo.Name, "checkout", "-B", br, 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
 		} else {
-			git.GitExecOrPanic(repo.Name, "fetch", prinfo.RemoteName, head.Sha)
 			git.GitExecOrPanic(repo.Name, "merge", "--ff", head.Sha)
 		}
 	}
@@ -885,15 +748,11 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error {
 		}
 
 		if !IsDryRun {
-			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, "push", "-f", prinfo.RemoteName, prinfo.PR.Head.Sha+":"+prinfo.PR.Base.Name)
+			} else {
+				git.GitExecOrPanic(repo.Name, "push", prinfo.RemoteName)
 			}
-			git.GitExecOrPanic(repo.Name, params...)
 		} else {
 			LogInfo("*** WOULD push", repo.Name, "to", prinfo.RemoteName)
 		}

common/pr_test.go

@@ -2,7 +2,6 @@ package common_test
 
 import (
 	"errors"
-	"fmt"
 	"os"
 	"os/exec"
 	"path"
@@ -808,9 +807,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
 						},
 						Reviews: &common.PRReviews{
-							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "m1"}}},
-							RequestedReviewers: []string{"m1"},
-							FullTimeline: []*models.TimelineComment{
+							Reviews: []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "m1"}}},
+							RequestedReviewers: []*models.TimelineComment{
 								{User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "m1"}, Type: common.TimelineCommentType_ReviewRequested},
 							},
 						},
@@ -920,8 +918,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 						},
 						Reviews: &common.PRReviews{
 							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "reviewer"}}},
-							RequestedReviewers: []string{"reviewer"},
-							FullTimeline:       []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
+							RequestedReviewers: []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
 						},
 					},
 					{
@@ -931,8 +928,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 						},
 						Reviews: &common.PRReviews{
 							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "reviewer"}}},
-							RequestedReviewers: []string{"reviewer"},
-							FullTimeline:       []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
+							RequestedReviewers: []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
 						},
 					},
 				},
@@ -967,8 +963,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStateApproved, User: &models.User{UserName: "pkgmaintainer"}},
 								{State: common.ReviewStatePending, User: &models.User{UserName: "prjmaintainer"}},
 							},
-							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer"},
-							FullTimeline: []*models.TimelineComment{
+							RequestedReviewers: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "user2"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
@@ -986,8 +981,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStateRequestChanges, User: &models.User{UserName: "user1"}},
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "autogits_obs_staging_bot"}},
 							},
-							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot"},
-							FullTimeline: []*models.TimelineComment{
+							RequestedReviewers: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "user1"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
 							},
@@ -1027,8 +1021,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStatePending, User: &models.User{UserName: "prj2"}},
 								{State: common.ReviewStatePending, User: &models.User{UserName: "someother"}},
 							},
-							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer", "pkgm1", "pkgm2", "someother", "prj1", "prj2"},
-							FullTimeline: []*models.TimelineComment{
+							RequestedReviewers: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
@@ -1051,8 +1044,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStatePending, User: &models.User{UserName: "prj1"}},
 								{State: common.ReviewStatePending, User: &models.User{UserName: "prj2"}},
 							},
-							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot", "prj1", "prj2"},
-							FullTimeline: []*models.TimelineComment{
+							RequestedReviewers: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj2"}},
@@ -1091,8 +1083,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "prj1"}},
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "someother"}},
 							},
-							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer", "pkgm1", "someother", "prj1"},
-							FullTimeline: []*models.TimelineComment{
+							RequestedReviewers: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgm1"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
@@ -1113,8 +1104,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "autogits_obs_staging_bot"}},
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "prj1"}},
 							},
-							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot", "prj1"},
-							FullTimeline: []*models.TimelineComment{
+							RequestedReviewers: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "!bot"}, Assignee: &models.User{UserName: "user1"}},
@@ -1200,6 +1190,9 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			test.prset.HasAutoStaging = !test.noAutoStaging
 			for idx, pr := range test.prset.PRs {
+				if pr.Reviews != nil {
+					pr.Reviews.SetRequiredReviewers(test.prset.Config.Reviewers)
+				}
 				missing, extra := test.prset.FindMissingAndExtraReviewers(test.maintainers, idx)
 
 				// avoid nil dereference below, by adding empty array elements
@@ -1274,7 +1267,7 @@ func TestPRMerge(t *testing.T) {
 						Owner: &models.User{
 							UserName: "org",
 						},
-						SSHURL: "ssh://git@src.opensuse.org/org/prj.git",
+						SSHURL: "file://" + path.Join(repoDir, "prjgit"),
 					},
 				},
 				Head: &models.PRBranchInfo{
@@ -1296,7 +1289,7 @@ func TestPRMerge(t *testing.T) {
 						Owner: &models.User{
 							UserName: "org",
 						},
-						SSHURL: "ssh://git@src.opensuse.org/org/prj.git",
+						SSHURL: "file://" + path.Join(cmd.Dir, "prjgit"),
 					},
 				},
 				Head: &models.PRBranchInfo{
@@ -1406,344 +1399,3 @@ 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)
-	}
-}
-

common/reviews.go

@@ -8,12 +8,28 @@ import (
 	"src.opensuse.org/autogits/common/gitea-generated/models"
 )
 
+type ReviewInterface interface {
+	IsManualMergeOK() bool
+	IsApproved() bool
+	MisingReviews() []string
+	FindReviewRequester(reviewer string) *models.TimelineComment
+	HasPendingReviewBy(reviewer string) bool
+	IsReviewedBy(reviewer string) bool
+	IsReviewedByOneOf(reviewers ...string) bool
+
+	SetRequiredReviewers(reviewers []string)
+}
+
 type PRReviews struct {
 	Reviews            []*models.PullReview
-	RequestedReviewers []string
+	RequestedReviewers []*models.TimelineComment
 	Comments           []*models.TimelineComment
 
-	FullTimeline []*models.TimelineComment
+	RequiredReviewers []string
+}
+
+func (r *PRReviews) SetRequiredReviewers(reviewers []string) {
+	r.RequiredReviewers = reviewers
 }
 
 func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64) (*PRReviews, error) {
@@ -28,11 +44,11 @@ func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64
 	}
 
 	reviews := make([]*models.PullReview, 0, 10)
-	needNewReviews := []string{}
 	var comments []*models.TimelineComment
 
+	var foundUsers []string
 	alreadyHaveUserReview := func(user string) bool {
-		if slices.Contains(needNewReviews, user) {
+		if slices.Contains(foundUsers, user) {
 			return true
 		}
 		for _, r := range reviews {
@@ -48,20 +64,24 @@ func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64
 	LogDebug("Number of items in timeline:", len(timeline))
 
 	cutOffIdx := len(timeline)
+	var PendingRequestedReviews []*models.TimelineComment
 	for idx, item := range timeline {
-		if item.Type == TimelineCommentType_Review || item.Type == TimelineCommentType_ReviewRequested {
+		if item.Type == TimelineCommentType_Review {
 			for _, r := range rawReviews {
 				if r.ID == item.ReviewID && r.User != nil {
 					if !alreadyHaveUserReview(r.User.UserName) {
-						if item.Type == TimelineCommentType_Review && idx > cutOffIdx {
-							needNewReviews = append(needNewReviews, r.User.UserName)
-						} else {
+						if idx < cutOffIdx {
 							reviews = append(reviews, r)
 						}
+						foundUsers = append(foundUsers, r.User.UserName)
 					}
 					break
 				}
 			}
+		} else if item.Type == TimelineCommentType_ReviewRequested && item.Assignee != nil && !alreadyHaveUserReview(item.Assignee.UserName) {
+			PendingRequestedReviews = append(PendingRequestedReviews, item)
+		} else if item.Type == TimelineCommentType_DismissReview && item.Assignee != nil && !alreadyHaveUserReview(item.Assignee.UserName) {
+			foundUsers = append(foundUsers, item.Assignee.UserName)
 		} else if item.Type == TimelineCommentType_Comment && cutOffIdx > idx {
 			comments = append(comments, item)
 		} else if item.Type == TimelineCommentType_PushPull && cutOffIdx == len(timeline) {
@@ -74,9 +94,9 @@ func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64
 	LogDebug("num comments:", len(comments), "timeline:", len(reviews))
 
 	return &PRReviews{
-		Reviews:      reviews,
-		Comments:     comments,
-		FullTimeline: timeline,
+		Reviews:            reviews,
+		Comments:           comments,
+		RequestedReviewers: PendingRequestedReviews,
 	}, nil
 }
 
@@ -104,7 +124,7 @@ func (r *PRReviews) IsManualMergeOK() bool {
 			continue
 		}
 		LogDebug("comment:", c.User.UserName, c.Body)
-		if slices.Contains(r.RequestedReviewers, c.User.UserName) {
+		if slices.Contains(r.RequiredReviewers, c.User.UserName) {
 			if bodyCommandManualMergeOK(c.Body) {
 				return true
 			}
@@ -115,7 +135,7 @@ func (r *PRReviews) IsManualMergeOK() bool {
 		if c.Updated != c.Submitted {
 			continue
 		}
-		if slices.Contains(r.RequestedReviewers, c.User.UserName) {
+		if slices.Contains(r.RequiredReviewers, c.User.UserName) {
 			if bodyCommandManualMergeOK(c.Body) {
 				return true
 			}
@@ -131,7 +151,7 @@ func (r *PRReviews) IsApproved() bool {
 	}
 	goodReview := true
 
-	for _, reviewer := range r.RequestedReviewers {
+	for _, reviewer := range r.RequiredReviewers {
 		goodReview = false
 		for _, review := range r.Reviews {
 			if review.User.UserName == reviewer && review.State == ReviewStateApproved && !review.Stale && !review.Dismissed {
@@ -155,7 +175,7 @@ func (r *PRReviews) MissingReviews() []string {
 		return missing
 	}
 
-	for _, reviewer := range r.RequestedReviewers {
+	for _, reviewer := range r.RequiredReviewers {
 		if !r.IsReviewedBy(reviewer) {
 			missing = append(missing, reviewer)
 		}
@@ -168,12 +188,11 @@ func (r *PRReviews) FindReviewRequester(reviewer string) *models.TimelineComment
 		return nil
 	}
 
-	for _, r := range r.FullTimeline {
-		if r.Type == TimelineCommentType_ReviewRequested && r.Assignee.UserName == reviewer {
-			return r
+	for _, t := range r.RequestedReviewers {
+		if t.Assignee.UserName == reviewer {
+			return t
 		}
 	}
-
 	return nil
 }
 
@@ -193,6 +212,13 @@ func (r *PRReviews) HasPendingReviewBy(reviewer string) bool {
 		}
 	}
 
+	// at this point, we do not have actual review by user. Check if we have a pending review
+	for _, t := range r.RequestedReviewers {
+		if t.Assignee != nil && t.Assignee.UserName == reviewer {
+			return true
+		}
+	}
+
 	return false
 }
 

common/reviews_test.go

@@ -145,6 +145,53 @@ func TestReviews(t *testing.T) {
 			reviewers:  []string{"user1"},
 			isApproved: false,
 		},
+		{
+			name:    "ReviewRequested predates PushPull should be seen as pending",
+			reviews: []*models.PullReview{},
+			timeline: []*models.TimelineComment{
+				{Type: common.TimelineCommentType_PushPull},
+				{Type: common.TimelineCommentType_ReviewRequested, Assignee: &models.User{UserName: "user1"}},
+			},
+			reviewers:        []string{"user1"},
+			isPendingByTest1: true,
+		},
+		{
+			name:    "ReviewRequested postdates PushPull but blocked by older dismiss",
+			reviews: []*models.PullReview{},
+			timeline: []*models.TimelineComment{
+				{Type: common.TimelineCommentType_ReviewRequested, Assignee: &models.User{UserName: "user1"}},
+				{Type: common.TimelineCommentType_PushPull},
+				{Type: common.TimelineCommentType_ReviewDismissed, Assignee: &models.User{UserName: "user1"}},
+			},
+			reviewers:        []string{"user1"},
+			isPendingByTest1: true,
+		},
+		{
+			name: "ReviewRequested predates PushPull should be seen as pending",
+			reviews: []*models.PullReview{
+				{ID: 101, State: common.ReviewStateRequestReview, User: &models.User{UserName: "user1"}},
+			},
+			timeline: []*models.TimelineComment{
+				{Type: common.TimelineCommentType_PushPull},
+				{Type: common.TimelineCommentType_ReviewRequested, Assignee: &models.User{UserName: "user1"}},
+			},
+			reviewers:        []string{"user1"},
+			isPendingByTest1: true,
+		},
+		{
+			name: "Review requested, review, then push needs re-requesting",
+			reviews: []*models.PullReview{
+				{ID: 100, State: common.ReviewStateRequestChanges, User: &models.User{UserName: "user1"}},
+			},
+			timeline: []*models.TimelineComment{
+				{Type: common.TimelineCommentType_PushPull},
+				{Type: common.TimelineCommentType_Review, ReviewID: 100},
+				{Type: common.TimelineCommentType_ReviewRequested, Assignee: &models.User{UserName: "user1"}},
+			},
+			reviewers:         []string{"user1"},
+			isReviewedByTest1: false, // Should be stale
+			isPendingByTest1:  false, // Should be stale
+		},
 	}
 
 	for _, test := range tests {
@@ -166,7 +213,7 @@ func TestReviews(t *testing.T) {
 				}
 				return
 			}
-			reviews.RequestedReviewers = test.reviewers
+			reviews.SetRequiredReviewers(test.reviewers)
 
 			if r := reviews.IsApproved(); r != test.isApproved {
 				t.Fatal("Unexpected IsReviewed():", r, "vs. expected", test.isApproved)

common/timeline.go

@@ -8,6 +8,7 @@ import (
 )
 
 const (
+	TimelineCommentType_ReviewDismissed = "dismiss_review"
 	TimelineCommentType_ReviewRequested = "review_request"
 	TimelineCommentType_Review          = "review"
 	TimelineCommentType_PushPull        = "pull_push"

integration/test-plan.md

@@ -65,11 +65,11 @@ The testing will be conducted in a dedicated test environment that mimics the pr
 | **TC-REVIEW-001** | P | **Add mandatory reviewers** | 1. Create a new PackageGit PR. | 1. All mandatory reviewers are added to both the PackageGit and ProjectGit PRs. | High |
 | **TC-REVIEW-002** | - | **Add advisory reviewers** | 1. Create a new PackageGit PR with advisory reviewers defined in the configuration. | 1. Advisory reviewers are added to the PR, but their approval is not required for merging. | Medium |
 | **TC-REVIEW-003** | - | **Re-add reviewers** | 1. Push a new commit to a PackageGit PR after it has been approved. | 1. The original reviewers are re-added to the PR. | Medium |
-| **TC-REVIEW-004** | x | **Package PR created by a maintainer** | 1. Create a PackageGit PR from the account of a package maintainer. | 1. No review is requested from other package maintainers. | High |
+| **TC-REVIEW-004** | P | **Package PR created by a maintainer** | 1. Create a PackageGit PR from the account of a package maintainer. | 1. No review is requested from other package maintainers. | High |
 | **TC-REVIEW-005** | P | **Package PR created by an external user (approve)** | 1. Create a PackageGit PR from the account of a user who is not a package maintainer.<br>2. One of the package maintainers approves the PR. | 1. All package maintainers are added as reviewers.<br>2. Once one maintainer approves the PR, the other maintainers are removed as reviewers. | High |
 | **TC-REVIEW-006** | P | **Package PR created by an external user (reject)** | 1. Create a PackageGit PR from the account of a user who is not a package maintainer.<br>2. One of the package maintainers rejects the PR. | 1. All package maintainers are added as reviewers.<br>2. Once one maintainer rejects the PR, the other maintainers are removed as reviewers. | High |
 | **TC-REVIEW-007** | P | **Package PR created by a maintainer with ReviewRequired=true** | 1. Set `ReviewRequired = true` in `workflow.config`.<br>2. Create a PackageGit PR from the account of a package maintainer. | 1. A review is requested from other package maintainers if available. | High |
-| **TC-MERGE-001** | x | **Automatic Merge** | 1. Create a PackageGit PR.<br>2. Ensure all mandatory reviews are completed on both project and package PRs. | 1. The PR is automatically merged. | High |
+| **TC-MERGE-001** | P | **Automatic Merge** | 1. Create a PackageGit PR.<br>2. Ensure all mandatory reviews are completed on both project and package PRs. | 1. The PR is automatically merged. | High |
 | **TC-MERGE-002** | - | **ManualMergeOnly with Package Maintainer** | 1. Create a PackageGit PR with `ManualMergeOnly` set to `true`.<br>2. Ensure all mandatory reviews are completed on both project and package PRs.<br>3. Comment "merge ok" on the package PR from the account of a package maintainer for that package. | 1. The PR is merged. | High |
 | **TC-MERGE-003** | - | **ManualMergeOnly with unauthorized user** | 1. Create a PackageGit PR with `ManualMergeOnly` set to `true`.<br>2. Ensure all mandatory reviews are completed on both project and package PRs.<br>3. Comment "merge ok" on the package PR from the account of a user who is not a maintainer for that package. | 1. The PR is not merged. | High |
 | **TC-MERGE-004** | - | **ManualMergeOnly with multiple packages** | 1. Create a ProjectGit PR that references multiple PackageGit PRs with `ManualMergeOnly` set to `true`.<br>2. Ensure all mandatory reviews are completed on both project and package PRs.<br>3. Comment "merge ok" on each package PR from the account of a package maintainer. | 1. The PR is merged only after "merge ok" is commented on all associated PackageGit PRs. | High |

integration/tests/workflow_pr_merge_test.py

@@ -5,7 +5,6 @@ from pathlib import Path
 from tests.lib.common_test_utils import GiteaAPIClient
 
 @pytest.mark.t001
-@pytest.mark.xfail(reason="The bot sometimes re-request reviews despite having all the approvals")
 def test_001_automerge(automerge_env, test_user_client):
     """
     Test scenario:

integration/tests/workflow_pr_review_test.py

@@ -43,7 +43,6 @@ index 0000000..e69de29
 
 
 @pytest.mark.t004
-@pytest.mark.xfail(reason="the bot sometimes re-requests review from autogits_obs_staging_bot despite having the approval")
 def test_004_maintainer(maintainer_env, ownerA_client):
     """
     Test scenario:
@@ -152,7 +151,6 @@ index 0000000..e69de29
 
 
 @pytest.mark.t005
-# @pytest.mark.xfail(reason="TBD troubleshoot")
 def test_005_any_maintainer_approval_sufficient(maintainer_env, ownerA_client, ownerBB_client):
     """
     Test scenario:

workflow-pr/README.md

@@ -96,19 +96,6 @@ 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
 ------
 

workflow-pr/pr_processor.go

@@ -413,12 +413,6 @@ 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 {