feat: Add test to cover when both invalid and ignored comments exists

Author: przechrzta-f3

internal/api/api_test.go

@@ -328,7 +328,7 @@ func TestWhenPRReviewedByNonTeamMemberAndAuthorsArePartOfTeam(t *testing.T) {
 	then.
 		ExpectPendingAnswerReturned().
 		ExpectStatusPendingReported().
-		ExpectCommentCharlieIgnoredAsReviewer().
+		ExpectInvalidCommentCharlieIgnoredAsReviewer().
 		ExpectLabelsUpdated().
 		ExpectedReviewRequestsMadeForFoo()
 }
@@ -378,7 +378,32 @@ func TestGitHubTeamApproverCleansUpOldInvalidReviewsComments(t *testing.T) {
 		ExpectPendingAnswerReturned().
 		ExpectStatusPendingReported().
 		ExpectPreviousIgnoredReviewCommentsDeleted().
-		ExpectCommentCharlieIgnoredAsReviewer().
+		ExpectInvalidCommentCharlieIgnoredAsReviewer().
+		ExpectLabelsUpdated().
+		ExpectedReviewRequestsMadeForFoo()
+}
+
+func TestGitHubTeamApproverCleansUpOldInvalidAndRetainsIgnoredReviewsComments(t *testing.T) {
+	given, when, then := stages.ApiTest(t)
+
+	given.
+		EncryptionKeyExists().
+		GitHubWebHookTokenExists().
+		FakeGHRunning().
+		OrganisationWithTeamFoo().
+		RepoWithNoContributorReviewEnabledAndFooAsApprovingTeam().
+		PullRequestExists().
+		InvalidAndIgnoredReviewCommentsExist().
+		CommitsWithAliceAsContributor().
+		CharlieApprovesPullRequest().
+		GitHubTeamApproverRunning()
+	when.
+		SendingApprovedPRReviewSubmittedEvent()
+	then.
+		ExpectPendingAnswerReturned().
+		ExpectStatusPendingReported().
+		ExpectPreviousAliceIgnoredReviewCommentsRetained().
+		ExpectInvalidCommentCharlieIgnoredAsReviewer().
 		ExpectLabelsUpdated().
 		ExpectedReviewRequestsMadeForFoo()
 }

internal/api/stages/api_stage.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
+	"strings"
 	"testing"
 
 	approverCfg "github.com/form3tech-oss/github-team-approver-commons/v2/pkg/configuration"
@@ -525,6 +526,24 @@ func (s *ApiStage) InvalidReviewCommentsExist() *ApiStage {
 	return s
 }
 
+func (s *ApiStage) InvalidAndIgnoredReviewCommentsExist() *ApiStage {
+	msgIgnored := fmt.Sprintf("%s\n- @%s\n", ignoredReviewerMsg, "alice")
+	msgInvalid := fmt.Sprintf("%s\n- @%s\n", invalidReviewerMsg, "some other user")
+	comments := []*github.IssueComment{
+		{
+			ID:   github.Int64(1),
+			Body: github.String(msgIgnored),
+		},
+		{
+			ID:   github.Int64(2),
+			Body: github.String(msgInvalid),
+		},
+	}
+	s.fakeGitHub.SetIssueComments(comments)
+
+	return s
+}
+
 func (s *ApiStage) AliceApprovesPullRequest() *ApiStage {
 	reviews := []*github.PullRequestReview{
 		{
@@ -762,33 +781,49 @@ func (s *ApiStage) ExpectedReviewRequestsMadeForFoo() *ApiStage {
 }
 
 func (s *ApiStage) ExpectCommentAliceIgnoredAsReviewer() *ApiStage {
-	comment := s.fakeGitHub.ReportedComment()
-	require.NotNil(s.t, comment)
-	require.NotEmpty(s.t, comment.Body)
-	require.Contains(s.t, *comment.Body, ignoredReviewerMsg)
-	require.Contains(s.t, *comment.Body, "alice")
+
+	var comments []*github.IssueComment
+	for _, c := range s.fakeGitHub.ReportedComments() {
+		if strings.Contains(*c.Body, ignoredReviewerMsg) {
+			comments = append(comments, c)
+		}
+	}
+
+	require.Len(s.t, comments, 1)
+	require.NotNil(s.t, comments[0])
+	require.NotEmpty(s.t, comments[0].Body)
+	require.Contains(s.t, *comments[0].Body, ignoredReviewerMsg)
+	require.Contains(s.t, *comments[0].Body, "alice")
 
 	return s
 }
 
-func (s *ApiStage) ExpectCommentCharlieIgnoredAsReviewer() *ApiStage {
-	comment := s.fakeGitHub.ReportedComment()
-	require.NotNil(s.t, comment)
-	require.NotEmpty(s.t, comment.Body)
-	require.Contains(s.t, *comment.Body, invalidReviewerMsg)
-	require.Contains(s.t, *comment.Body, "charlie")
+func (s *ApiStage) ExpectInvalidCommentCharlieIgnoredAsReviewer() *ApiStage {
+
+	var comments []*github.IssueComment
+	for _, c := range s.fakeGitHub.ReportedComments() {
+		if strings.Contains(*c.Body, invalidReviewerMsg) {
+			comments = append(comments, c)
+		}
+	}
+
+	require.Len(s.t, comments, 1)
+	require.NotNil(s.t, comments[0])
+	require.NotEmpty(s.t, comments[0].Body)
+	require.Contains(s.t, *comments[0].Body, invalidReviewerMsg)
+	require.Contains(s.t, *comments[0].Body, "charlie")
 
 	return s
 }
 
 func (s *ApiStage) ExpectNoCommentsMade() *ApiStage {
-	require.Nil(s.t, s.fakeGitHub.ReportedComment())
+	require.Empty(s.t, s.fakeGitHub.ReportedComments())
 
 	return s
 }
 
 func (s *ApiStage) ExpectNoReviewRequestsMade() *ApiStage {
-	require.Nil(s.t, s.fakeGitHub.ReportedComment())
+	require.Empty(s.t, s.fakeGitHub.ReportedComments())
 
 	return s
 }
@@ -799,6 +834,16 @@ func (s *ApiStage) ExpectPreviousIgnoredReviewCommentsDeleted() *ApiStage {
 	return s
 }
 
+func (s *ApiStage) ExpectPreviousAliceIgnoredReviewCommentsRetained() *ApiStage {
+	require.Len(s.t, s.fakeGitHub.Comments(), 1)
+	comment := s.fakeGitHub.Comments()[0]
+	require.NotNil(s.t, comment)
+	require.NotEmpty(s.t, comment.Body)
+	require.Contains(s.t, *comment.Body, ignoredReviewerMsg)
+	require.Contains(s.t, *comment.Body, "alice")
+	return s
+}
+
 func (s *ApiStage) IgnoreRepositoryExists() *ApiStage {
 	ignoredRepo := fmt.Sprintf(
 		"%s/%s",

internal/api/stages/fakegithub/comments.go

@@ -44,7 +44,7 @@ func (f *FakeGitHub) postCommentHandler(w http.ResponseWriter, r *http.Request)
 	err = json.Unmarshal(payload, &comment)
 	require.NoError(f.t, err)
 
-	f.reportedComment = comment
+	f.reportedComments = append(f.reportedComments, comment)
 
 	w.Header().Set("Content-Type", "application/json")
 

internal/api/stages/fakegithub/github.go

@@ -54,7 +54,7 @@ type FakeGitHub struct {
 	events        []*github.IssueEvent
 
 	reportedStatus         *github.RepoStatus
-	reportedComment        *github.IssueComment
+	reportedComments       []*github.IssueComment
 	reportedLabels         []string
 	requestedTeamReviewers []string
 }
@@ -143,11 +143,11 @@ func (f *FakeGitHub) Org() *Org   { return f.org }
 func (f *FakeGitHub) Repo() *Repo { return f.repo }
 func (f *FakeGitHub) PR() *PR     { return f.pr }
 
-func (f *FakeGitHub) ReportedLabels() []string              { return f.reportedLabels }
-func (f *FakeGitHub) ReportedStatus() *github.RepoStatus    { return f.reportedStatus }
-func (f *FakeGitHub) ReportedComment() *github.IssueComment { return f.reportedComment }
-func (f *FakeGitHub) RequestedTeamReviews() []string        { return f.requestedTeamReviewers }
-func (f *FakeGitHub) Comments() []*github.IssueComment      { return f.issueComments }
+func (f *FakeGitHub) ReportedLabels() []string                 { return f.reportedLabels }
+func (f *FakeGitHub) ReportedStatus() *github.RepoStatus       { return f.reportedStatus }
+func (f *FakeGitHub) ReportedComments() []*github.IssueComment { return f.reportedComments }
+func (f *FakeGitHub) RequestedTeamReviews() []string           { return f.requestedTeamReviewers }
+func (f *FakeGitHub) Comments() []*github.IssueComment         { return f.issueComments }
 
 func (f *FakeGitHub) URL() string {
 	return fmt.Sprintf("%s", f.ts.URL)