t: correctly handle multiple reviews from the same user

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/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)

common/pr_test.go

@@ -807,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},
 							},
 						},
@@ -919,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"}}},
 						},
 					},
 					{
@@ -930,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"}}},
 						},
 					},
 				},
@@ -966,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"}},
@@ -985,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"}},
 							},
@@ -1026,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"}},
@@ -1050,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"}},
@@ -1090,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"}},
@@ -1112,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"}},
@@ -1199,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

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

@@ -59,18 +59,17 @@ The testing will be conducted in a dedicated test environment that mimics the pr
 | **TC-SYNC-002** | P | **Update ProjectGit PR from PackageGit PR** | 1. Push a new commit to an existing PackageGit PR. | 1. The corresponding ProjectGit PR's head branch is updated with the new commit. | High |
 | **TC-SYNC-003** | P | **WIP Flag Synchronization** | 1. Mark a PackageGit PR as "Work In Progress".<br>2. Remove the WIP flag from the PackageGit PR. | 1. The corresponding ProjectGit PR is also marked as "Work In Progress".<br>2. The WIP flag on the ProjectGit PR is removed. | Medium |
 | **TC-SYNC-004** | - | **WIP Flag (multiple referenced package PRs)** | 1. Create a ProjectGit PR that references multiple PackageGit PRs.<br>2. Mark one of the PackageGit PRs as "Work In Progress".<br>3. Remove the "Work In Progress" flag from all PackageGit PRs. | 1. The ProjectGit PR is marked as "Work In Progress".<br>2. The "Work In Progress" flag is removed from the ProjectGit PR only after it has been removed from all associated PackageGit PRs. | Medium |
-| **TC-SYNC-005** | m | **NoProjectGitPR = true, edits disabled** | 1. Set `NoProjectGitPR = true` in `workflow.config`.<br>2. Create a PackageGit PR without "Allow edits from maintainers" enabled. <br>3. Push a new commit to the PackageGit PR. | 1. No ProjectGit PR is created.<br>2. The bot adds a warning comment to the PackageGit PR explaining that it cannot update the PR. | High |
-| **TC-SYNC-006** | m | **NoProjectGitPR = true, edits enabled** | 1. Set `NoProjectGitPR = true` in `workflow.config`.<br>2. Create a PackageGit PR with "Allow edits from maintainers" enabled.<br>3. Push a new commit to the PackageGit PR. | 1. No ProjectGit PR is created.<br>2. The submodule commit on the project PR is updated with the new commit from the PackageGit PR. | High |
-| **TC-SYNC-007** | m | **Change target branch** | 1. Create a Package PR targeting 'main'.<br>2. Change the target branch of the Package PR to 'merge'. | 1. The target branch of the corresponding Project PR is updated to 'merge' automatically. | High |
+| **TC-SYNC-005** | x | **NoProjectGitPR = true, edits disabled** | 1. Set `NoProjectGitPR = true` in `workflow.config`.<br>2. Create a PackageGit PR without "Allow edits from maintainers" enabled. <br>3. Push a new commit to the PackageGit PR. | 1. No ProjectGit PR is created.<br>2. The bot adds a warning comment to the PackageGit PR explaining that it cannot update the PR. | High |
+| **TC-SYNC-006** | x | **NoProjectGitPR = true, edits enabled** | 1. Set `NoProjectGitPR = true` in `workflow.config`.<br>2. Create a PackageGit PR with "Allow edits from maintainers" enabled.<br>3. Push a new commit to the PackageGit PR. | 1. No ProjectGit PR is created.<br>2. The submodule commit on the project PR is updated with the new commit from the PackageGit PR. | High |
 | **TC-COMMENT-001** | - | **Detect duplicate comments** | 1. Create a PackageGit PR.<br>2. Wait for the `workflow-pr` bot to act on the PR.<br>3. Edit the body of the PR to trigger the bot a second time. | 1. The bot should not post a duplicate comment. | High |
 | **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 |
@@ -87,5 +86,4 @@ The testing will be conducted in a dedicated test environment that mimics the pr
 * P = implemented and passing;
 * x = likely implemented, but investigation is needed;
 * X = implemented and likely to pass, but someteimes may fail, but troubleshooting is needed;
-* m = bot functionality is missing in the branch, the test is waiting for implementation in the code base
 * - = test is not implemented

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:

integration/tests/workflow_pr_sync_test.py

@@ -324,54 +324,3 @@ index 0000000..f587a12
     print("Verification complete: Manually created Project PR was updated by the bot as expected.")
 
 
-@pytest.mark.t007
-@pytest.mark.xfail(reason="test is ready; bot functionality is missing: it creates a new project PR instead of updating existing")
-def test_007_change_target_branch(gitea_env):
-    """Target branch in project PR is updated when package PR target branch changes"""
-    # 1. Create a Package PR targeting 'main'
-    diff = "diff --git a/target_test.txt b/target_test.txt\nnew file mode 100644\nindex 0000000..e69de29\n"
-    package_pr = gitea_env.create_gitea_pr("pool/pkgA", diff, "Target Branch Test PR", False, base_branch="main")
-    package_pr_number = package_pr["number"]
-
-    # 2. Poll for forwarded Project PR
-    forwarded_pr_number = None
-    print(f"Polling pool/pkgA PR #{package_pr_number} timeline for forwarded PR event...")
-    for _ in range(20):
-        time.sleep(1)
-        timeline_events = gitea_env.get_timeline_events("pool/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"products/SLFO/pulls/(\d+)", url_to_check)
-                if match:
-                    forwarded_pr_number = match.group(1)
-                    break
-        if forwarded_pr_number:
-            break
-
-    assert forwarded_pr_number is not None, "Workflow bot did not create a forwarded PR."
-
-    # Verify initial target branch
-    project_pr_details = gitea_env.get_pr_details("products/SLFO", forwarded_pr_number)
-    assert project_pr_details["base"]["ref"] == "main"
-
-    # 3. User changes the target branch in the package PR to 'merge'
-    print(f"Changing target branch of Package PR #{package_pr_number} to 'merge'")
-    gitea_env.update_gitea_pr_properties("pool/pkgA", package_pr_number, base="merge")
-
-    # 4. Check that target branch in the project PR gets updated by the bots automatically
-    updated_successfully = False
-    print(f"Polling Project PR #{forwarded_pr_number} for target branch update...")
-    for _ in range(20):
-        time.sleep(1)
-        project_pr_details = gitea_env.get_pr_details("products/SLFO", forwarded_pr_number)
-        if project_pr_details["base"]["ref"] == "merge":
-            updated_successfully = True
-            print("Project PR target branch updated to 'merge'")
-            break
-
-    assert updated_successfully, "Target branch in Project PR was not updated to 'merge'"
-
-