Merge pull request #1351 from hongweiliu17/STONEINTG-1390

fix(STONEINTG-1390): update existing GH/GL comment not creating new one

Author: hongweiliu17

git/github/github.go

@@ -110,7 +110,7 @@ type ClientInterface interface {
 	GetAllCommitStatusesForRef(ctx context.Context, owner, repo, sha string) ([]*ghapi.RepoStatus, int, error)
 	GetAllCommentsForPR(ctx context.Context, owner string, repo string, pr int) ([]*ghapi.IssueComment, int, error)
 	CommitStatusExists(res []*ghapi.RepoStatus, commitStatus *CommitStatusAdapter) (bool, error)
-	GetExistingCommentID(comments []*ghapi.IssueComment, snapshotName, scenarioName string) *int64
+	GetExistingCommentID(comments []*ghapi.IssueComment, componentName, scenarioName string) *int64
 	EditComment(ctx context.Context, owner string, repo string, commentID int64, body string) (int64, int, error)
 	GetPullRequest(ctx context.Context, owner string, repo string, prID int) (*ghapi.PullRequest, int, error)
 }
@@ -428,9 +428,9 @@ func (c *Client) GetExistingCheckRun(checkRuns []*ghapi.CheckRun, newCheckRun *C
 }
 
 // GetExistingComment returns existing GitHub comment for the scenario of ref.
-func (c *Client) GetExistingCommentID(comments []*ghapi.IssueComment, snapshotName, scenarioName string) *int64 {
+func (c *Client) GetExistingCommentID(comments []*ghapi.IssueComment, componentName, scenarioName string) *int64 {
 	for _, comment := range comments {
-		if strings.Contains(*comment.Body, snapshotName) && strings.Contains(*comment.Body, scenarioName) {
+		if strings.Contains(*comment.Body, componentName) && strings.Contains(*comment.Body, scenarioName) {
 			c.logger.Info("found comment ID with a matching scenarioName", "scenarioName", scenarioName)
 			return comment.ID
 		}

git/github/github_test.go

@@ -114,7 +114,7 @@ func (MockIssuesService) CreateComment(
 func (MockIssuesService) ListComments(ctx context.Context, owner string, repo string,
 	number int, opts *ghapi.IssueListCommentsOptions) ([]*ghapi.IssueComment, *ghapi.Response, error) {
 	var id int64 = 40
-	var body = "Integration test for snapshot snapshotName and scenario scenarioName"
+	var body = "Integration test for component component-sample snapshot snapshotName and scenario scenarioName"
 	issueComments := []*ghapi.IssueComment{{ID: &id, Body: &body}}
 	return issueComments, nil, nil
 }
@@ -355,7 +355,7 @@ var _ = Describe("Client", func() {
 		Expect(comments).NotTo(BeEmpty())
 		Expect(statusCode).NotTo(BeNil())
 
-		commentID := client.GetExistingCommentID(comments, "snapshotName", "scenarioName")
+		commentID := client.GetExistingCommentID(comments, "component-sample", "scenarioName")
 		Expect(*commentID).To(Equal(int64(40)))
 	})
 

internal/controller/statusreport/statusreport_adapter_test.go

@@ -931,7 +931,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 				SnapshotName:        "snapshot-pr-sample",
 				ComponentName:       "component-sample",
 				Text:                "Test in progress",
-				Summary:             "Integration test for snapshot snapshot-pr-sample and scenario scenario1 is in progress",
+				Summary:             "Integration test for component component-sample snapshot snapshot-pr-sample and scenario scenario1 is in progress",
 				Status:              intgteststat.IntegrationTestStatusInProgress,
 				StartTime:           &t,
 				TestPipelineRunName: "test-pipelinerun",

internal/controller/statusreport/statusreport_controller.go

@@ -96,7 +96,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 // AdapterInterface is an interface defining all the operations that should be defined in an Integration adapter.
 type AdapterInterface interface {
-	EnsureSnapshotTestStatusReportedToGitHub() (controller.OperationResult, error)
+	EnsureSnapshotTestStatusReportedToGitProvider() (controller.OperationResult, error)
 	EnsureGroupSnapshotCreationStatusReportedToGitProvider(controller.OperationResult, error)
 	EnsureSnapshotFinishedAllTests() (controller.OperationResult, error)
 }

status/reporter_github.go

@@ -417,7 +417,7 @@ func (csu *CommitStatusUpdater) updateStatusInComment(ctx context.Context, repor
 		csu.logger.Error(err, fmt.Sprintf("error while getting all comments for pull-request %s", issueNumberStr))
 		return statusCode, fmt.Errorf("error while getting all comments for pull-request %s: %w", issueNumberStr, err)
 	}
-	existingCommentId := csu.ghClient.GetExistingCommentID(allComments, csu.snapshot.Name, report.ScenarioName)
+	existingCommentId := csu.ghClient.GetExistingCommentID(allComments, GenerateComponentNameWithPrefix(report.ComponentName), report.ScenarioName)
 	if existingCommentId == nil {
 		_, statusCode, err = csu.ghClient.CreateComment(ctx, csu.owner, csu.repo, issueNumber, comment)
 		if err != nil {

status/reporter_github_test.go

@@ -153,7 +153,7 @@ func (c *MockGitHubClient) GetAllCommentsForPR(ctx context.Context, owner string
 	return comments, 200, nil
 }
 
-func (c *MockGitHubClient) GetExistingCommentID(comments []*ghapi.IssueComment, snapshotName, scenarioName string) *int64 {
+func (c *MockGitHubClient) GetExistingCommentID(comments []*ghapi.IssueComment, componentName, scenarioName string) *int64 {
 	return nil
 }
 

status/reporter_gitlab.go

@@ -256,7 +256,7 @@ func (r *GitLabReporter) updateStatusInComment(report TestReport) (int, error) {
 		r.logger.Error(err, "error while getting all comments for merge-request", "mergeRequest", r.mergeRequest, "report.SnapshotName", report.SnapshotName)
 		return statusCode, fmt.Errorf("error while getting all comments for merge-request %d: %w", r.mergeRequest, err)
 	}
-	existingCommentId := r.GetExistingNoteID(allNotes, report.ScenarioName, report.SnapshotName)
+	existingCommentId := r.GetExistingNoteID(allNotes, report.ScenarioName, GenerateComponentNameWithPrefix(report.ComponentName))
 	if existingCommentId == nil {
 		noteOptions := gitlab.CreateMergeRequestNoteOptions{Body: &comment}
 		_, response, err := r.client.Notes.CreateMergeRequestNote(r.targetProjectID, r.mergeRequest, &noteOptions)
@@ -294,10 +294,10 @@ func (r *GitLabReporter) GetExistingCommitStatus(commitStatuses []*gitlab.Commit
 }
 
 // GetExistingNoteID returns existing GitLab note for the scenario of ref.
-func (r *GitLabReporter) GetExistingNoteID(notes []*gitlab.Note, scenarioName, snapshotName string) *int {
+func (r *GitLabReporter) GetExistingNoteID(notes []*gitlab.Note, scenarioName, componentName string) *int {
 	for _, note := range notes {
-		if strings.Contains(note.Body, snapshotName) && strings.Contains(note.Body, scenarioName) {
-			r.logger.Info("found note ID with a matching scenarioName", "scenarioName", scenarioName, "noteID", &note.ID)
+		if strings.Contains(note.Body, componentName) && strings.Contains(note.Body, scenarioName) {
+			r.logger.Info("found note ID with a matching componentName and scenarioName", "omponentName", componentName, "scenarioName", scenarioName, "noteID", &note.ID)
 			return &note.ID
 		}
 	}

status/status.go

@@ -239,7 +239,7 @@ func GenerateTestReport(ctx context.Context, client client.Client, detail intgte
 		return nil, fmt.Errorf("failed to generate text message: %w", err)
 	}
 
-	summary, err := GenerateSummary(detail.Status, testedSnapshot.Name, detail.ScenarioName)
+	summary, err := GenerateSummary(detail.Status, testedSnapshot.Name, detail.ScenarioName, componentName)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate summary message: %w", err)
 	}
@@ -315,7 +315,7 @@ func generateText(ctx context.Context, client client.Client, integrationTestStat
 }
 
 // GenerateSummary returns summary for the given state, snapshotName and scenarioName
-func GenerateSummary(state intgteststat.IntegrationTestStatus, snapshotName, scenarioName string) (string, error) {
+func GenerateSummary(state intgteststat.IntegrationTestStatus, snapshotName, scenarioName, componentName string) (string, error) {
 	var summary string
 	var statusDesc string
 
@@ -348,15 +348,28 @@ func GenerateSummary(state intgteststat.IntegrationTestStatus, snapshotName, sce
 		return summary, fmt.Errorf("unknown status")
 	}
 
+	componentName = GenerateComponentNameWithPrefix(componentName)
+
 	if state == intgteststat.BuildPLRInProgress || state == intgteststat.SnapshotCreationFailed || state == intgteststat.BuildPLRFailed || state == intgteststat.GroupSnapshotCreationFailed {
-		summary = fmt.Sprintf("Integration test for scenario %s %s", scenarioName, statusDesc)
+		summary = fmt.Sprintf("Integration test for %s snapshot scenario %s %s", componentName, scenarioName, statusDesc)
 	} else {
-		summary = fmt.Sprintf("Integration test for snapshot %s and scenario %s %s", snapshotName, scenarioName, statusDesc)
+		summary = fmt.Sprintf("Integration test for %s snapshot %s and scenario %s %s", componentName, snapshotName, scenarioName, statusDesc)
 	}
 
 	return summary, nil
 }
 
+// function GenerateComponentNameWithPrefix to generate component name "pr group" or "component component-sample"
+// to help search it in comment correctly to avoid component name is searched in pr group report
+func GenerateComponentNameWithPrefix(componentName string) string {
+	if componentName == gitops.ComponentNameForGroupSnapshot {
+		componentName = gitops.ComponentNameForGroupSnapshot
+	} else {
+		componentName = "component " + componentName
+	}
+	return componentName
+}
+
 func getConsoleName() string {
 	consoleName := os.Getenv("CONSOLE_NAME")
 	if consoleName == "" {

status/status_test.go

@@ -522,19 +522,19 @@ var _ = Describe("Status Adapter", func() {
 
 `
 		expectedTestReport := status.TestReport{
-			FullName:            "Red Hat Konflux / scenario1",
+			FullName:            "Red Hat Konflux / scenario1 / component-sample",
 			ScenarioName:        "scenario1",
 			SnapshotName:        "snapshot-sample",
-			ComponentName:       "",
+			ComponentName:       "component-sample",
 			Text:                text,
-			Summary:             "Integration test for snapshot snapshot-sample and scenario scenario1 has passed",
+			Summary:             "Integration test for component component-sample snapshot snapshot-sample and scenario scenario1 has passed",
 			Status:              integrationteststatus.IntegrationTestStatusTestPassed,
 			StartTime:           &ts,
 			CompletionTime:      &tc,
 			TestPipelineRunName: "test-pipelinerun",
 		}
 
-		testReport, err := status.GenerateTestReport(context.Background(), mockK8sClient, integrationTestStatusDetail, hasSnapshot, "")
+		testReport, err := status.GenerateTestReport(context.Background(), mockK8sClient, integrationTestStatusDetail, hasSnapshot, "component-sample")
 		Expect(err).NotTo(HaveOccurred())
 		Expect(testReport).To(Equal(&expectedTestReport))
 	})
@@ -545,7 +545,7 @@ var _ = Describe("Status Adapter", func() {
 
 			integrationTestStatusDetail := newIntegrationTestStatusDetail(expectedScenarioStatus)
 
-			expectedSummary := fmt.Sprintf("Integration test for snapshot snapshot-sample and scenario scenario1 %s", expectedTextEnding)
+			expectedSummary := fmt.Sprintf("Integration test for component component-sample snapshot snapshot-sample and scenario scenario1 %s", expectedTextEnding)
 			testReport, err := status.GenerateTestReport(context.Background(), mockK8sClient, integrationTestStatusDetail, hasSnapshot, "component-sample")
 			Expect(err).NotTo(HaveOccurred())
 			Expect(testReport.Summary).To(Equal(expectedSummary))
@@ -566,7 +566,7 @@ var _ = Describe("Status Adapter", func() {
 
 			integrationTestStatusDetail := newIntegrationTestStatusDetail(expectedScenarioStatus)
 
-			expectedSummary := fmt.Sprintf("Integration test for scenario scenario1 %s", expectedTextEnding)
+			expectedSummary := fmt.Sprintf("Integration test for component component-sample snapshot scenario scenario1 %s", expectedTextEnding)
 			testReport, err := status.GenerateTestReport(context.Background(), mockK8sClient, integrationTestStatusDetail, hasSnapshot, "component-sample")
 			Expect(err).NotTo(HaveOccurred())
 			Expect(testReport.Summary).To(Equal(expectedSummary))
@@ -578,7 +578,7 @@ var _ = Describe("Status Adapter", func() {
 
 	It("check if GenerateSummary supports all integration test statuses", func() {
 		for _, teststatus := range integrationteststatus.IntegrationTestStatusValues() {
-			_, err := status.GenerateSummary(teststatus, "yolo", "yolo")
+			_, err := status.GenerateSummary(teststatus, "yolo", "yolo", "yoyo")
 			Expect(err).NotTo(HaveOccurred())
 		}
 	})