diff --git a/group-review/main.go b/group-review/main.go index 0f7831f..07d8f3b 100644 --- a/group-review/main.go +++ b/group-review/main.go @@ -18,20 +18,23 @@ import ( "src.opensuse.org/autogits/common/gitea-generated/models" ) -var configs common.AutogitConfigs -var acceptRx *regexp.Regexp -var rejectRx *regexp.Regexp -var groupName string - -func InitRegex(newGroupName string) { - groupName = newGroupName - acceptRx = regexp.MustCompile("^:\\s*(LGTM|approved?)") - rejectRx = regexp.MustCompile("^:\\s*") +type ReviewBot struct { + configs common.AutogitConfigs + acceptRx *regexp.Regexp + rejectRx *regexp.Regexp + groupName string + gitea common.Gitea } -func ParseReviewLine(reviewText string) (bool, string) { +func (bot *ReviewBot) InitRegex(newGroupName string) { + bot.groupName = newGroupName + bot.acceptRx = regexp.MustCompile("^:\\s*(LGTM|approved?)") + bot.rejectRx = regexp.MustCompile("^:\\s*") +} + +func (bot *ReviewBot) ParseReviewLine(reviewText string) (bool, string) { line := strings.TrimSpace(reviewText) - groupTextName := "@" + groupName + groupTextName := "@" + bot.groupName glen := len(groupTextName) if len(line) < glen || line[0:glen] != groupTextName { return false, line @@ -51,20 +54,20 @@ func ParseReviewLine(reviewText string) (bool, string) { return false, line } -func ReviewAccepted(reviewText string) bool { +func (bot *ReviewBot) ReviewAccepted(reviewText string) bool { for _, line := range common.SplitStringNoEmpty(reviewText, "\n") { - if matched, reviewLine := ParseReviewLine(line); matched { - return acceptRx.MatchString(reviewLine) + if matched, reviewLine := bot.ParseReviewLine(line); matched { + return bot.acceptRx.MatchString(reviewLine) } } return false } -func ReviewRejected(reviewText string) bool { +func (bot *ReviewBot) ReviewRejected(reviewText string) bool { for _, line := range common.SplitStringNoEmpty(reviewText, "\n") { - if matched, reviewLine := ParseReviewLine(line); matched { - if rejectRx.MatchString(reviewLine) { - return !acceptRx.MatchString(reviewLine) + if matched, reviewLine := bot.ParseReviewLine(line); matched { + if bot.rejectRx.MatchString(reviewLine) { + return !bot.acceptRx.MatchString(reviewLine) } } } @@ -114,10 +117,10 @@ var commentStrings = []string{ "change_time_estimate", }*/ -func FindAcceptableReviewInTimeline(user string, timeline []*models.TimelineComment, reviews []*models.PullReview) *models.TimelineComment { +func (bot *ReviewBot) FindAcceptableReviewInTimeline(user string, timeline []*models.TimelineComment, reviews []*models.PullReview) *models.TimelineComment { for _, t := range timeline { if t.Type == common.TimelineCommentType_Comment && t.User.UserName == user && t.Created == t.Updated { - if ReviewAccepted(t.Body) || ReviewRejected(t.Body) { + if bot.ReviewAccepted(t.Body) || bot.ReviewRejected(t.Body) { return t } } @@ -126,9 +129,9 @@ func FindAcceptableReviewInTimeline(user string, timeline []*models.TimelineComm return nil } -func FindOurLastReviewInTimeline(timeline []*models.TimelineComment) *models.TimelineComment { +func (bot *ReviewBot) FindOurLastReviewInTimeline(timeline []*models.TimelineComment) *models.TimelineComment { for _, t := range timeline { - if t.Type == common.TimelineCommentType_Review && t.User.UserName == groupName && t.Created == t.Updated { + if t.Type == common.TimelineCommentType_Review && t.User.UserName == bot.groupName && t.Created == t.Updated { return t } } @@ -136,13 +139,13 @@ func FindOurLastReviewInTimeline(timeline []*models.TimelineComment) *models.Tim return nil } -func UnrequestReviews(gitea common.Gitea, org, repo string, id int64, users []string) { - if err := gitea.UnrequestReview(org, repo, id, users...); err != nil { +func (bot *ReviewBot) UnrequestReviews(org, repo string, id int64, users []string) { + if err := bot.gitea.UnrequestReview(org, repo, id, users...); err != nil { common.LogError("Can't remove reviewrs after a review:", err) } } -func ProcessNotifications(notification *models.NotificationThread, gitea common.Gitea) { +func (bot *ReviewBot) ProcessNotifications(notification *models.NotificationThread) { defer func() { if r := recover(); r != nil { common.LogInfo("panic cought --- recovered") @@ -169,14 +172,14 @@ func ProcessNotifications(notification *models.NotificationThread, gitea common. id, _ := strconv.ParseInt(match[3], 10, 64) common.LogInfo("processing:", fmt.Sprintf("%s/%s!%d", org, repo, id)) - pr, err := gitea.GetPullRequest(org, repo, id) + pr, err := bot.gitea.GetPullRequest(org, repo, id) if err != nil { common.LogError(" ** Cannot fetch PR associated with review:", subject.URL, "Error:", err) return } - if err := ProcessPR(pr); err == nil && !common.IsDryRun { - if err := gitea.SetNotificationRead(notification.ID); err != nil { + if err := bot.ProcessPR(pr); err == nil && !common.IsDryRun { + if err := bot.gitea.SetNotificationRead(notification.ID); err != nil { common.LogDebug(" Cannot set notification as read", err) } } else if err != nil && err != ReviewNotFinished { @@ -186,24 +189,24 @@ func ProcessNotifications(notification *models.NotificationThread, gitea common. var ReviewNotFinished = fmt.Errorf("Review is not finished") -func ProcessPR(pr *models.PullRequest) error { +func (bot *ReviewBot) ProcessPR(pr *models.PullRequest) error { org := pr.Base.Repo.Owner.UserName repo := pr.Base.Repo.Name id := pr.Index found := false for _, reviewer := range pr.RequestedReviewers { - if reviewer != nil && reviewer.UserName == groupName { + if reviewer != nil && reviewer.UserName == bot.groupName { found = true break } } if !found { - common.LogInfo(" review is not requested for", groupName) + common.LogInfo(" review is not requested for", bot.groupName) return nil } - config := configs.GetPrjGitConfig(org, repo, pr.Base.Name) + config := bot.configs.GetPrjGitConfig(org, repo, pr.Base.Name) if config == nil { return fmt.Errorf("Cannot find config for: %s", pr.URL) } @@ -213,17 +216,17 @@ func ProcessPR(pr *models.PullRequest) error { return nil } - reviews, err := gitea.GetPullRequestReviews(org, repo, id) + reviews, err := bot.gitea.GetPullRequestReviews(org, repo, id) if err != nil { return fmt.Errorf("Failed to fetch reviews for: %v: %w", pr.URL, err) } - timeline, err := common.FetchTimelineSinceReviewRequestOrPush(gitea, groupName, pr.Head.Sha, org, repo, id) + timeline, err := common.FetchTimelineSinceReviewRequestOrPush(bot.gitea, bot.groupName, pr.Head.Sha, org, repo, id) if err != nil { return fmt.Errorf("Failed to fetch timeline to review. %w", err) } - groupConfig, err := config.GetReviewGroup(groupName) + groupConfig, err := config.GetReviewGroup(bot.groupName) if err != nil { return fmt.Errorf("Failed to fetch review group. %w", err) } @@ -234,30 +237,30 @@ func ProcessPR(pr *models.PullRequest) error { // pr.Head.Sha for _, reviewer := range requestReviewers { - if review := FindAcceptableReviewInTimeline(reviewer, timeline, reviews); review != nil { - if ReviewAccepted(review.Body) { + if review := bot.FindAcceptableReviewInTimeline(reviewer, timeline, reviews); review != nil { + if bot.ReviewAccepted(review.Body) { if !common.IsDryRun { - text := reviewer + " approved a review on behalf of " + groupName - if review := FindOurLastReviewInTimeline(timeline); review == nil || review.Body != text { - _, err := gitea.AddReviewComment(pr, common.ReviewStateApproved, text) + text := reviewer + " approved a review on behalf of " + bot.groupName + if review := bot.FindOurLastReviewInTimeline(timeline); review == nil || review.Body != text { + _, err := bot.gitea.AddReviewComment(pr, common.ReviewStateApproved, text) if err != nil { common.LogError(" -> failed to write approval comment", err) } - UnrequestReviews(gitea, org, repo, id, requestReviewers) + bot.UnrequestReviews(org, repo, id, requestReviewers) } } common.LogInfo(" -> approved by", reviewer) common.LogInfo(" review at", review.Created) return nil - } else if ReviewRejected(review.Body) { + } else if bot.ReviewRejected(review.Body) { if !common.IsDryRun { - text := reviewer + " requested changes on behalf of " + groupName + ". See " + review.HTMLURL - if review := FindOurLastReviewInTimeline(timeline); review == nil || review.Body != text { - _, err := gitea.AddReviewComment(pr, common.ReviewStateRequestChanges, text) + text := reviewer + " requested changes on behalf of " + bot.groupName + ". See " + review.HTMLURL + if review := bot.FindOurLastReviewInTimeline(timeline); review == nil || review.Body != text { + _, err := bot.gitea.AddReviewComment(pr, common.ReviewStateRequestChanges, text) if err != nil { common.LogError(" -> failed to write rejecting comment", err) } - UnrequestReviews(gitea, org, repo, id, requestReviewers) + bot.UnrequestReviews(org, repo, id, requestReviewers) } } common.LogInfo(" -> declined by", reviewer) @@ -271,7 +274,7 @@ func ProcessPR(pr *models.PullRequest) error { if !groupConfig.Silent && len(requestReviewers) > 0 { common.LogDebug(" Requesting reviews for:", requestReviewers) if !common.IsDryRun { - if _, err := gitea.RequestReviews(pr, requestReviewers...); err != nil { + if _, err := bot.gitea.RequestReviews(pr, requestReviewers...); err != nil { common.LogDebug(" -> err:", err) } } else { @@ -284,42 +287,40 @@ func ProcessPR(pr *models.PullRequest) error { // add a helpful comment, if not yet added found_help_comment := false for _, t := range timeline { - if t.Type == common.TimelineCommentType_Comment && t.User != nil && t.User.UserName == groupName { + if t.Type == common.TimelineCommentType_Comment && t.User != nil && t.User.UserName == bot.groupName { found_help_comment = true break } } if !found_help_comment && !common.IsDryRun { - helpComment := fmt.Sprintln("Review by", groupName, "represents a group of reviewers:", strings.Join(requestReviewers, ", "), ".\n\n"+ + helpComment := fmt.Sprintln("Review by", bot.groupName, "represents a group of reviewers:", strings.Join(requestReviewers, ", "), ".\n\n"+ "Do **not** use standard review interface to review on behalf of the group.\n"+ - "To accept the review on behalf of the group, create the following comment: `@"+groupName+": approve`.\n"+ - "To request changes on behalf of the group, create the following comment: `@"+groupName+": decline` followed with lines justifying the decision.\n"+ + "To accept the review on behalf of the group, create the following comment: `@"+bot.groupName+": approve`.\n"+ + "To request changes on behalf of the group, create the following comment: `@"+bot.groupName+": decline` followed with lines justifying the decision.\n"+ "Future edits of the comments are ignored, a new comment is required to change the review state.") if slices.Contains(groupConfig.Reviewers, pr.User.UserName) { helpComment = helpComment + "\n\n" + "Submitter is member of this review group, hence they are excluded from being one of the reviewers here" } - gitea.AddComment(pr, helpComment) + bot.gitea.AddComment(pr, helpComment) } return ReviewNotFinished } -func PeriodReviewCheck() { - notifications, err := gitea.GetNotifications(common.GiteaNotificationType_Pull, nil) +func (bot *ReviewBot) PeriodReviewCheck() { + notifications, err := bot.gitea.GetNotifications(common.GiteaNotificationType_Pull, nil) if err != nil { common.LogError(" Error fetching unread notifications: %w", err) return } for _, notification := range notifications { - ProcessNotifications(notification, gitea) + bot.ProcessNotifications(notification) } } -var gitea common.Gitea - func main() { giteaUrl := flag.String("gitea-url", "https://src.opensuse.org", "Gitea instance used for reviews") rabbitMqHost := flag.String("rabbit-url", "amqps://rabbit.opensuse.org", "RabbitMQ instance where Gitea webhook notifications are sent") @@ -355,7 +356,7 @@ func main() { flag.Usage() return } - groupName = args[0] + targetGroupName := args[0] if *configFile == "" { common.LogError("Missing config file") @@ -378,14 +379,14 @@ func main() { return } - gitea = common.AllocateGiteaTransport(*giteaUrl) - configs, err = common.ResolveWorkflowConfigs(gitea, configData) + giteaTransport := common.AllocateGiteaTransport(*giteaUrl) + configs, err := common.ResolveWorkflowConfigs(giteaTransport, configData) if err != nil { common.LogError("Cannot parse workflow configs:", err) return } - reviewer, err := gitea.GetCurrentUser() + reviewer, err := giteaTransport.GetCurrentUser() if err != nil { common.LogError("Cannot fetch review user:", err) return @@ -395,14 +396,18 @@ func main() { *interval = 1 } - InitRegex(groupName) + bot := &ReviewBot{ + gitea: giteaTransport, + configs: configs, + } + bot.InitRegex(targetGroupName) - common.LogInfo(" ** processing group reviews for group:", groupName) + common.LogInfo(" ** processing group reviews for group:", bot.groupName) common.LogInfo(" ** username in Gitea:", reviewer.UserName) common.LogInfo(" ** polling interval:", *interval, "min") common.LogInfo(" ** connecting to RabbitMQ:", *rabbitMqHost) - if groupName != reviewer.UserName { + if bot.groupName != reviewer.UserName { common.LogError(" ***** Reviewer does not match group name. Aborting. *****") return } @@ -414,10 +419,13 @@ func main() { } config_update := ConfigUpdatePush{ + bot: bot, config_modified: make(chan *common.AutogitConfig), } - process_issue_pr := IssueCommentProcessor{} + process_issue_pr := IssueCommentProcessor{ + bot: bot, + } configUpdates := &common.RabbitMQGiteaEventsProcessor{ Orgs: []string{}, @@ -427,7 +435,7 @@ func main() { }, } configUpdates.Connection().RabbitURL = u - for _, c := range configs { + for _, c := range bot.configs { if org, _, _ := c.GetPrjGit(); !slices.Contains(configUpdates.Orgs, org) { configUpdates.Orgs = append(configUpdates.Orgs, org) } @@ -440,17 +448,17 @@ func main() { select { case configTouched, ok := <-config_update.config_modified: if ok { - for idx, c := range configs { + for idx, c := range bot.configs { if c == configTouched { org, repo, branch := c.GetPrjGit() prj := fmt.Sprintf("%s/%s#%s", org, repo, branch) common.LogInfo("Detected config update for", prj) - new_config, err := common.ReadWorkflowConfig(gitea, prj) + new_config, err := common.ReadWorkflowConfig(bot.gitea, prj) if err != nil { common.LogError("Failed parsing Project config for", prj, err) } else { - configs[idx] = new_config + bot.configs[idx] = new_config } } } @@ -460,7 +468,7 @@ func main() { } } - PeriodReviewCheck() + bot.PeriodReviewCheck() time.Sleep(time.Duration(*interval * int64(time.Minute))) } } diff --git a/group-review/main_test.go b/group-review/main_test.go index f18c7e7..6b3e091 100644 --- a/group-review/main_test.go +++ b/group-review/main_test.go @@ -1,6 +1,359 @@ package main -import "testing" +import ( + "fmt" + "testing" + + "go.uber.org/mock/gomock" + "src.opensuse.org/autogits/common" + "src.opensuse.org/autogits/common/gitea-generated/models" + mock_common "src.opensuse.org/autogits/common/mock" +) + +func TestProcessPR(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockGitea := mock_common.NewMockGitea(ctrl) + groupName := "testgroup" + + bot := &ReviewBot{ + gitea: mockGitea, + groupName: groupName, + } + bot.InitRegex(groupName) + + org := "myorg" + repo := "myrepo" + prIndex := int64(1) + headSha := "abcdef123456" + + pr := &models.PullRequest{ + Index: prIndex, + URL: "http://gitea/pr/1", + State: "open", + Base: &models.PRBranchInfo{ + Name: "main", + Repo: &models.Repository{ + Name: repo, + Owner: &models.User{ + UserName: org, + }, + }, + }, + Head: &models.PRBranchInfo{ + Sha: headSha, + }, + User: &models.User{ + UserName: "submitter", + }, + RequestedReviewers: []*models.User{ + {UserName: groupName}, + }, + } + + prjConfig := &common.AutogitConfig{ + GitProjectName: org + "/" + repo + "#main", + ReviewGroups: []*common.ReviewGroup{ + { + Name: groupName, + Reviewers: []string{"reviewer1", "reviewer2"}, + }, + }, + } + bot.configs = common.AutogitConfigs{prjConfig} + + t.Run("Review not requested for group", func(t *testing.T) { + prNoRequest := *pr + prNoRequest.RequestedReviewers = nil + err := bot.ProcessPR(&prNoRequest) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + }) + + t.Run("PR is closed", func(t *testing.T) { + prClosed := *pr + prClosed.State = "closed" + err := bot.ProcessPR(&prClosed) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + }) + + t.Run("Successful Approval", func(t *testing.T) { + common.IsDryRun = false + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil) + // reviewer1 approved in timeline + timeline := []*models.TimelineComment{ + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: "reviewer1"}, + Body: "@" + groupName + ": approve", + }, + } + + mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil) + + expectedText := "reviewer1 approved a review on behalf of " + groupName + mockGitea.EXPECT().AddReviewComment(pr, common.ReviewStateApproved, expectedText).Return(nil, nil) + mockGitea.EXPECT().UnrequestReview(org, repo, prIndex, gomock.Any()).Return(nil) + + err := bot.ProcessPR(pr) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) + + t.Run("Dry Run - No actions taken", func(t *testing.T) { + common.IsDryRun = true + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil) + timeline := []*models.TimelineComment{ + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: "reviewer1"}, + Body: "@" + groupName + ": approve", + }, + } + mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil) + + // No AddReviewComment or UnrequestReview should be called + err := bot.ProcessPR(pr) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) + + t.Run("Approval already exists - No new comment", func(t *testing.T) { + common.IsDryRun = false + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil) + + approvalText := "reviewer1 approved a review on behalf of " + groupName + timeline := []*models.TimelineComment{ + { + Type: common.TimelineCommentType_Review, + User: &models.User{UserName: groupName}, + Body: approvalText, + }, + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: "reviewer1"}, + Body: "@" + groupName + ": approve", + }, + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: groupName}, + Body: "Help comment", + }, + } + + mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil) + + // No AddReviewComment, UnrequestReview, or AddComment should be called + err := bot.ProcessPR(pr) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) + + t.Run("Rejection already exists - No new comment", func(t *testing.T) { + common.IsDryRun = false + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil) + + rejectionText := "reviewer1 requested changes on behalf of " + groupName + ". See http://gitea/comment/123" + timeline := []*models.TimelineComment{ + { + Type: common.TimelineCommentType_Review, + User: &models.User{UserName: groupName}, + Body: rejectionText, + }, + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: "reviewer1"}, + Body: "@" + groupName + ": decline", + HTMLURL: "http://gitea/comment/123", + }, + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: groupName}, + Body: "Help comment", + }, + } + + mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil) + + err := bot.ProcessPR(pr) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) + + t.Run("Pending review - Help comment already exists", func(t *testing.T) { + common.IsDryRun = false + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil) + + timeline := []*models.TimelineComment{ + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: groupName}, + Body: "Some help comment", + }, + } + + mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil) + + // It will try to request reviews + mockGitea.EXPECT().RequestReviews(pr, "reviewer1", "reviewer2").Return(nil, nil) + + // AddComment should NOT be called because bot already has a comment in timeline + err := bot.ProcessPR(pr) + if err != ReviewNotFinished { + t.Errorf("Expected ReviewNotFinished error, got %v", err) + } + }) + + t.Run("Submitter is group member - Excluded from review request", func(t *testing.T) { + common.IsDryRun = false + prSubmitterMember := *pr + prSubmitterMember.User = &models.User{UserName: "reviewer1"} + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil) + mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(nil, nil) + mockGitea.EXPECT().RequestReviews(&prSubmitterMember, "reviewer2").Return(nil, nil) + mockGitea.EXPECT().AddComment(&prSubmitterMember, gomock.Any()).Return(nil) + err := bot.ProcessPR(&prSubmitterMember) + if err != ReviewNotFinished { + t.Errorf("Expected ReviewNotFinished error, got %v", err) + } + }) + + t.Run("Successful Rejection", func(t *testing.T) { + common.IsDryRun = false + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil) + timeline := []*models.TimelineComment{ + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: "reviewer2"}, + Body: "@" + groupName + ": decline", + HTMLURL: "http://gitea/comment/999", + }, + } + mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil) + expectedText := "reviewer2 requested changes on behalf of " + groupName + ". See http://gitea/comment/999" + mockGitea.EXPECT().AddReviewComment(pr, common.ReviewStateRequestChanges, expectedText).Return(nil, nil) + mockGitea.EXPECT().UnrequestReview(org, repo, prIndex, gomock.Any()).Return(nil) + err := bot.ProcessPR(pr) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) + + t.Run("Config not found", func(t *testing.T) { + bot.configs = common.AutogitConfigs{} + err := bot.ProcessPR(pr) + if err == nil { + t.Error("Expected error when config is missing, got nil") + } + }) + + t.Run("Gitea error in GetPullRequestReviews", func(t *testing.T) { + bot.configs = common.AutogitConfigs{prjConfig} + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, fmt.Errorf("gitea error")) + err := bot.ProcessPR(pr) + if err == nil { + t.Error("Expected error from gitea, got nil") + } + }) +} + +func TestProcessNotifications(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockGitea := mock_common.NewMockGitea(ctrl) + groupName := "testgroup" + + bot := &ReviewBot{ + gitea: mockGitea, + groupName: groupName, + } + bot.InitRegex(groupName) + + org := "myorg" + repo := "myrepo" + prIndex := int64(123) + notificationID := int64(456) + + notification := &models.NotificationThread{ + ID: notificationID, + Subject: &models.NotificationSubject{ + URL: fmt.Sprintf("http://gitea/api/v1/repos/%s/%s/pulls/%d", org, repo, prIndex), + }, + } + + t.Run("Notification Success", func(t *testing.T) { + common.IsDryRun = false + pr := &models.PullRequest{ + Index: prIndex, + Base: &models.PRBranchInfo{ + Name: "main", + Repo: &models.Repository{ + Name: repo, + Owner: &models.User{UserName: org}, + }, + }, + + Head: &models.PRBranchInfo{ + Sha: "headsha", + Repo: &models.Repository{ + Name: repo, + Owner: &models.User{UserName: org}, + }, + }, + + User: &models.User{UserName: "submitter"}, + RequestedReviewers: []*models.User{{UserName: groupName}}, + } + + mockGitea.EXPECT().GetPullRequest(org, repo, prIndex).Return(pr, nil) + + prjConfig := &common.AutogitConfig{ + GitProjectName: org + "/" + repo + "#main", + ReviewGroups: []*common.ReviewGroup{{Name: groupName, Reviewers: []string{"r1"}}}, + } + bot.configs = common.AutogitConfigs{prjConfig} + mockGitea.EXPECT().GetPullRequestReviews(org, repo, prIndex).Return(nil, nil) + timeline := []*models.TimelineComment{ + { + Type: common.TimelineCommentType_Comment, + User: &models.User{UserName: "r1"}, + Body: "@" + groupName + ": approve", + }, + } + mockGitea.EXPECT().GetTimeline(org, repo, prIndex).Return(timeline, nil) + expectedText := "r1 approved a review on behalf of " + groupName + mockGitea.EXPECT().AddReviewComment(pr, common.ReviewStateApproved, expectedText).Return(nil, nil) + mockGitea.EXPECT().UnrequestReview(org, repo, prIndex, gomock.Any()).Return(nil) + + mockGitea.EXPECT().SetNotificationRead(notificationID).Return(nil) + + bot.ProcessNotifications(notification) + + }) + + t.Run("Invalid Notification URL", func(t *testing.T) { + badNotification := &models.NotificationThread{ + Subject: &models.NotificationSubject{ + URL: "http://gitea/invalid/url", + }, + } + bot.ProcessNotifications(badNotification) + }) + + t.Run("Gitea error in GetPullRequest", func(t *testing.T) { + mockGitea.EXPECT().GetPullRequest(org, repo, prIndex).Return(nil, fmt.Errorf("gitea error")) + bot.ProcessNotifications(notification) + }) +} func TestReviewApprovalCheck(t *testing.T) { tests := []struct { @@ -60,16 +413,78 @@ func TestReviewApprovalCheck(t *testing.T) { InString: "@group2: disapprove", Rejected: true, }, + { + Name: "Whitespace before colon", + GroupName: "group", + InString: "@group : LGTM", + Approved: true, + }, + { + Name: "No whitespace after colon", + GroupName: "group", + InString: "@group:LGTM", + Approved: true, + }, + { + Name: "Leading and trailing whitespace on line", + GroupName: "group", + InString: " @group: LGTM ", + Approved: true, + }, + { + Name: "Multiline: Approved on second line", + GroupName: "group", + InString: "Random noise\n@group: approved", + Approved: true, + }, + { + Name: "Multiline: Multiple group mentions, first wins", + GroupName: "group", + InString: "@group: decline\n@group: approve", + Rejected: true, + }, + { + Name: "Multiline: Multiple group mentions, noise in between", + GroupName: "group", + InString: "noise\n@group: approve\nmore noise", + Approved: true, + }, + { + Name: "Not at start of line (even with whitespace)", + GroupName: "group", + InString: "Hello @group: approve", + Approved: false, + }, + { + Name: "Rejecting with reason", + GroupName: "group", + InString: "@group: decline because of X, Y and Z", + Rejected: true, + }, + { + Name: "No colon after group", + GroupName: "group", + InString: "@group LGTM", + Approved: false, + Rejected: false, + }, + { + Name: "Invalid char after group", + GroupName: "group", + InString: "@group! LGTM", + Approved: false, + Rejected: false, + }, } - for _, test := range tests { t.Run(test.Name, func(t *testing.T) { - InitRegex(test.GroupName) + bot := &ReviewBot{} + bot.InitRegex(test.GroupName) - if r := ReviewAccepted(test.InString); r != test.Approved { + if r := bot.ReviewAccepted(test.InString); r != test.Approved { t.Error("ReviewAccepted() returned", r, "expecting", test.Approved) } - if r := ReviewRejected(test.InString); r != test.Rejected { + if r := bot.ReviewRejected(test.InString); r != test.Rejected { t.Error("ReviewRejected() returned", r, "expecting", test.Rejected) } }) diff --git a/group-review/rabbit.go b/group-review/rabbit.go index 940c4c3..3a255c1 100644 --- a/group-review/rabbit.go +++ b/group-review/rabbit.go @@ -7,7 +7,9 @@ import ( "src.opensuse.org/autogits/common" ) -type IssueCommentProcessor struct{} +type IssueCommentProcessor struct { + bot *ReviewBot +} func (s *IssueCommentProcessor) ProcessFunc(req *common.Request) error { if req.Type != common.RequestType_IssueComment { @@ -19,14 +21,15 @@ func (s *IssueCommentProcessor) ProcessFunc(req *common.Request) error { repo := data.Repository.Name index := int64(data.Issue.Number) - pr, err := gitea.GetPullRequest(org, repo, index) + pr, err := s.bot.gitea.GetPullRequest(org, repo, index) if err != nil { return fmt.Errorf("Failed to fetch PullRequest from event: %s/%s!%d Error: %w", org, repo, index, err) } - return ProcessPR(pr) + return s.bot.ProcessPR(pr) } type ConfigUpdatePush struct { + bot *ReviewBot config_modified chan *common.AutogitConfig } @@ -46,7 +49,7 @@ func (s *ConfigUpdatePush) ProcessFunc(req *common.Request) error { } branch := data.Ref[len(branch_ref):] - c := configs.GetPrjGitConfig(org, repo, branch) + c := s.bot.configs.GetPrjGitConfig(org, repo, branch) if c == nil { return nil } @@ -64,7 +67,7 @@ func (s *ConfigUpdatePush) ProcessFunc(req *common.Request) error { } if modified_config { - for _, config := range configs { + for _, config := range s.bot.configs { if o, r, _ := config.GetPrjGit(); o == org && r == repo { s.config_modified <- config } diff --git a/group-review/rabbit_test.go b/group-review/rabbit_test.go new file mode 100644 index 0000000..ed46c50 --- /dev/null +++ b/group-review/rabbit_test.go @@ -0,0 +1,203 @@ +package main + +import ( + "fmt" + "testing" + + "go.uber.org/mock/gomock" + "src.opensuse.org/autogits/common" + "src.opensuse.org/autogits/common/gitea-generated/models" + mock_common "src.opensuse.org/autogits/common/mock" +) + +func TestIssueCommentProcessor(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockGitea := mock_common.NewMockGitea(ctrl) + groupName := "testgroup" + bot := &ReviewBot{ + gitea: mockGitea, + groupName: groupName, + } + bot.InitRegex(groupName) + + processor := &IssueCommentProcessor{bot: bot} + + org := "myorg" + repo := "myrepo" + index := 123 + + event := &common.IssueCommentWebhookEvent{ + Repository: &common.Repository{ + Name: repo, + Owner: &common.Organization{ + Username: org, + }, + }, + Issue: &common.IssueDetail{ + Number: index, + }, + } + + req := &common.Request{ + Type: common.RequestType_IssueComment, + Data: event, + } + + t.Run("Successful Processing", func(t *testing.T) { + pr := &models.PullRequest{ + Index: int64(index), + Base: &models.PRBranchInfo{ + Name: "main", + Repo: &models.Repository{ + Name: repo, + Owner: &models.User{UserName: org}, + }, + }, + Head: &models.PRBranchInfo{ + Sha: "headsha", + Repo: &models.Repository{ + Name: repo, + Owner: &models.User{UserName: org}, + }, + }, + User: &models.User{UserName: "submitter"}, + RequestedReviewers: []*models.User{{UserName: groupName}}, + } + + mockGitea.EXPECT().GetPullRequest(org, repo, int64(index)).Return(pr, nil) + + prjConfig := &common.AutogitConfig{ + GitProjectName: org + "/" + repo + "#main", + ReviewGroups: []*common.ReviewGroup{{Name: groupName, Reviewers: []string{"r1"}}}, + } + bot.configs = common.AutogitConfigs{prjConfig} + mockGitea.EXPECT().GetPullRequestReviews(org, repo, int64(index)).Return(nil, nil) + mockGitea.EXPECT().GetTimeline(org, repo, int64(index)).Return(nil, nil) + mockGitea.EXPECT().RequestReviews(pr, "r1").Return(nil, nil) + mockGitea.EXPECT().AddComment(pr, gomock.Any()).Return(nil) + + err := processor.ProcessFunc(req) + if err != ReviewNotFinished { + t.Errorf("Expected ReviewNotFinished, got %v", err) + } + }) + + t.Run("Gitea error in GetPullRequest", func(t *testing.T) { + mockGitea.EXPECT().GetPullRequest(org, repo, int64(index)).Return(nil, fmt.Errorf("gitea error")) + err := processor.ProcessFunc(req) + if err == nil { + t.Error("Expected error, got nil") + } + }) + + t.Run("Wrong Request Type", func(t *testing.T) { + wrongReq := &common.Request{Type: common.RequestType_Push} + err := processor.ProcessFunc(wrongReq) + if err == nil { + t.Error("Expected error for wrong request type, got nil") + } + }) +} + +func TestConfigUpdatePush(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + groupName := "testgroup" + bot := &ReviewBot{ + groupName: groupName, + } + bot.InitRegex(groupName) + + configChan := make(chan *common.AutogitConfig, 1) + processor := &ConfigUpdatePush{ + bot: bot, + config_modified: configChan, + } + + org := "myorg" + repo := "myrepo" + branch := "main" + + prjConfig := &common.AutogitConfig{ + GitProjectName: org + "/" + repo + "#" + branch, + Organization: org, + Branch: branch, + } + bot.configs = common.AutogitConfigs{prjConfig} + + event := &common.PushWebhookEvent{ + Ref: "refs/heads/" + branch, + Repository: &common.Repository{ + Name: repo, + Owner: &common.Organization{ + Username: org, + }, + }, + Commits: []common.Commit{ + { + Modified: []string{common.ProjectConfigFile}, + }, + }, + } + + req := &common.Request{ + Type: common.RequestType_Push, + Data: event, + } + + t.Run("Config Modified", func(t *testing.T) { + err := processor.ProcessFunc(req) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + + select { + case modified := <-configChan: + if modified != prjConfig { + t.Errorf("Expected modified config to be %v, got %v", prjConfig, modified) + } + default: + t.Error("Expected config modification signal, but none received") + } + }) + + t.Run("No Config Modified", func(t *testing.T) { + noConfigEvent := *event + noConfigEvent.Commits = []common.Commit{{Modified: []string{"README.md"}}} + noConfigReq := &common.Request{Type: common.RequestType_Push, Data: &noConfigEvent} + + err := processor.ProcessFunc(noConfigReq) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + + select { + case <-configChan: + t.Error("Did not expect config modification signal") + default: + // Success + } + }) + + t.Run("Wrong Branch Ref", func(t *testing.T) { + wrongBranchEvent := *event + wrongBranchEvent.Ref = "refs/tags/v1.0" + wrongBranchReq := &common.Request{Type: common.RequestType_Push, Data: &wrongBranchEvent} + + err := processor.ProcessFunc(wrongBranchReq) + if err == nil { + t.Error("Expected error for wrong branch ref, got nil") + } + }) + + t.Run("Config Not Found", func(t *testing.T) { + bot.configs = common.AutogitConfigs{} + err := processor.ProcessFunc(req) + if err != nil { + t.Errorf("Expected nil error even if config not found, got %v", err) + } + }) +} \ No newline at end of file