One comment per presubmit run

.ci/magician/cmd/generate_comment.go

@@ -519,10 +519,15 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep,
 		fmt.Printf("Data: %v\n", data)
 		return fmt.Errorf("error formatting message: %w", err)
 	}
-	if err := gh.PostComment(strconv.Itoa(prNumber), message); err != nil {
+	commentId, err := gh.PostComment(strconv.Itoa(prNumber), message)
+	if err != nil {
 		fmt.Println("Comment: ", message)
 		return fmt.Errorf("error posting comment to PR %d: %w", prNumber, err)
 	}
+
+	if err := rnr.WriteFile("/workspace/diff_comment_id.txt", strconv.Itoa(commentId)); err != nil {
+		fmt.Printf("Warning: failed to save comment ID to file: %v\n", err)
+	}
 	return nil
 }
 

.ci/magician/cmd/interfaces.go

@@ -23,16 +23,18 @@ import (
 
 type GithubClient interface {
 	GetPullRequest(prNumber string) (github.PullRequest, error)
+	GetPullRequestAuthor(prNumber string) (string, error)
 	GetPullRequests(state, base, sort, direction string) ([]github.PullRequest, error)
 	GetPullRequestRequestedReviewers(prNumber string) ([]github.User, error)
 	GetPullRequestPreviousReviewers(prNumber string) ([]github.User, error)
 	GetPullRequestComments(prNumber string) ([]github.PullRequestComment, error)
+	GetPullRequestComment(commentID int) (github.PullRequestComment, error)
 	GetCommitMessage(owner, repo, sha string) (string, error)
 	GetUserType(user string) github.UserType
 	GetTeamMembers(organization, team string) ([]github.User, error)
 	MergePullRequest(owner, repo, prNumber, commitSha string) error
 	PostBuildStatus(prNumber, title, state, targetURL, commitSha string) error
-	PostComment(prNumber, comment string) error
+	PostComment(prNumber, comment string) (int, error)
 	UpdateComment(prNumber, comment string, id int) error
 	RequestPullRequestReviewers(prNumber string, reviewers []string) error
 	RemovePullRequestReviewers(prNumber string, reviewers []string) error

.ci/magician/cmd/mock_github_test.go

@@ -17,6 +17,7 @@ package cmd
 
 import (
 	"errors"
+	"fmt"
 
 	"magician/github"
 )
@@ -37,6 +38,11 @@ func (m *mockGithub) GetPullRequest(prNumber string) (github.PullRequest, error)
 	return m.pullRequest, nil
 }
 
+func (m *mockGithub) GetPullRequestAuthor(prNumber string) (string, error) {
+	m.calledMethods["GetPullRequestAuthor"] = append(m.calledMethods["GetPullRequestAuthor"], []any{prNumber})
+	return m.pullRequest.User.Login, nil
+}
+
 func (m *mockGithub) GetPullRequests(state, base, sort, direction string) ([]github.PullRequest, error) {
 	m.calledMethods["GetPullRequests"] = append(m.calledMethods["GetPullRequests"], []any{state, base, sort, direction})
 	return []github.PullRequest{m.pullRequest}, nil
@@ -62,6 +68,16 @@ func (m *mockGithub) GetPullRequestComments(prNumber string) ([]github.PullReque
 	return m.pullRequestComments, nil
 }
 
+func (m *mockGithub) GetPullRequestComment(commentID int) (github.PullRequestComment, error) {
+	m.calledMethods["GetPullRequestComment"] = append(m.calledMethods["GetPullRequestComment"], []any{commentID})
+	for _, c := range m.pullRequestComments {
+		if c.ID == commentID {
+			return c, nil
+		}
+	}
+	return github.PullRequestComment{}, fmt.Errorf("comment not found")
+}
+
 func (m *mockGithub) GetCommitMessage(owner, repo, sha string) (string, error) {
 	m.calledMethods["GetCommitMessage"] = append(m.calledMethods["GetCommitMessage"], []any{owner, repo, sha})
 	return m.commitMessage, nil
@@ -85,9 +101,9 @@ func (m *mockGithub) RemovePullRequestReviewers(prNumber string, reviewers []str
 	return nil
 }
 
-func (m *mockGithub) PostComment(prNumber string, comment string) error {
+func (m *mockGithub) PostComment(prNumber string, comment string) (int, error) {
 	m.calledMethods["PostComment"] = append(m.calledMethods["PostComment"], []any{prNumber, comment})
-	return nil
+	return 0, nil
 }
 
 func (m *mockGithub) UpdateComment(prNumber, comment string, id int) error {

.ci/magician/cmd/reassign_reviewer.go

@@ -91,7 +91,7 @@ func execReassignReviewer(prNumber, newPrimaryReviewer string, gh GithubClient)
 
 	if currentReviewer == "" {
 		fmt.Println("No reviewer comment found, creating one")
-		err := gh.PostComment(prNumber, comment)
+		_, err = gh.PostComment(prNumber, comment)
 		if err != nil {
 			return err
 		}

.ci/magician/cmd/request_reviewer.go

@@ -83,7 +83,7 @@ func execRequestReviewer(prNumber string, gh GithubClient) error {
 
 		if newPrimaryReviewer != "" {
 			comment := github.FormatReviewerComment(newPrimaryReviewer)
-			err = gh.PostComment(prNumber, comment)
+			_, err = gh.PostComment(prNumber, comment)
 			if err != nil {
 				return err
 			}

.ci/magician/cmd/templates/vcr/post_replay.tmpl

@@ -1,3 +1,5 @@
+## Test report
+
 {{- if or (gt (len .NotRunBetaTests) 0) (gt (len .NotRunGATests) 0)}}
 #### Non-exercised tests
 
@@ -11,46 +13,44 @@
 {{range .NotRunGATests}}{{. | printf "- %s\n"}}{{end}}
 {{end}}
 {{end}}
-#### Tests analytics
-Total tests: {{add (add (len .ReplayingResult.PassedTests) (len .ReplayingResult.SkippedTests)) (len .ReplayingResult.FailedTests) }}
-Passed tests: {{len .ReplayingResult.PassedTests}}
-Skipped tests: {{len .ReplayingResult.SkippedTests}}
-Affected tests: {{len .ReplayingResult.FailedTests}}
+
+#### Analytics
+| Total Tests | Passed | Skipped | Affected |
+| :--- | :--- | :--- | :--- |
+| {{add (add (len .ReplayingResult.PassedTests) (len .ReplayingResult.SkippedTests)) (len .ReplayingResult.FailedTests) }} | {{len .ReplayingResult.PassedTests}} | {{len .ReplayingResult.SkippedTests}} | {{len .ReplayingResult.FailedTests}} |
 
 <details>
-<summary>Click here to see the affected service packages</summary>
-<blockquote>
+<summary><b>Affected Service Packages</b></summary>
+
 {{if .RunFullVCR}}
-All service packages are affected
+* All service packages are affected
 {{else if gt (len .AffectedServices) 0}}
-<ul>
-{{range .AffectedServices}}{{. | printf "<li>%s</li>\n"}}{{end}}
-</ul>
+{{range .AffectedServices}}{{. | printf "* %s\n"}}{{end}}
 {{else}}
-None
+* None
 {{end}}
-</blockquote>
 </details>
 
-{{ if gt (len .ReplayingResult.FailedTests) 0 -}}
+[Learn how VCR tests work](https://googlecloudplatform.github.io/magic-modules/develop/test/test/)
+
+---
+
+**Step 1: Replaying Mode**
+
+{{ if gt (len .ReplayingResult.FailedTests) 0 }}
 #### Action taken
 <details>
-<summary>Found {{len .ReplayingResult.FailedTests}} affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
-</summary>
-<blockquote>
-<ul>
-{{range .ReplayingResult.FailedTests}}{{. | printf "<li>%s</li>\n"}}{{end}}
-</ul>
-</blockquote>
-</details>
+<summary>Found {{len .ReplayingResult.FailedTests}} affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests</summary>
 
-[Get to know how VCR tests work](https://googlecloudplatform.github.io/magic-modules/develop/test/test/)
-{{ else -}}
-{{- if .ReplayingErr -}}
-{{color "red" "Errors occurred during REPLAYING mode. Please fix them to complete your PR."}}
-{{- else -}}
-{{color "green" "All tests passed!"}}
-{{- end}}
+{{range .ReplayingResult.FailedTests}}{{. | printf "* %s\n"}}{{end}}
+</details>
+{{ else }}
+{{ if .ReplayingErr }}
+> [!CAUTION]
+> {{color "red" "Errors occurred during REPLAYING mode. Please fix them to complete your PR."}}
+{{ else }}
+{{color "green" "**All tests passed in Replaying mode! No Recording was needed.**"}}
+{{ end }}
+{{ end }}
 
 View the [build log](https://storage.cloud.google.com/{{.LogBucket}}/{{.Version}}/refs/heads/{{.Head}}/artifacts/{{.BuildID}}/build-log/replaying_test.log)
-{{- end}}

.ci/magician/cmd/templates/vcr/record_replay.tmpl

@@ -1,46 +1,34 @@
-{{- if gt (len .RecordingResult.PassedTests) 0 -}}
-{{color "green" "Tests passed during RECORDING mode:"}}
-{{range .RecordingResult.PassedTests -}}
-`{{.}}` {{/* remove trailing whitespace */ -}}
-  [[Debug log]({{$.LogBaseUrl}}/recording/{{.}}.log)]
-{{/* remove trailing whitespace */ -}}
-{{end}}
+---
 
-{{- if gt (len .ReplayingAfterRecordingResult.FailedTests ) 0 }}
+**Step 2: Recording Mode**
 
-{{color "red" "Tests failed when rerunning REPLAYING mode:"}}
-{{range .ReplayingAfterRecordingResult.FailedTests -}}
-`{{.}}` {{/* remove trailing whitespace */ -}}
-  [[Error message]({{$.LogBaseUrl}}/build-log/replaying_build_after_recording/{{compoundTest .}}_replaying_test.log)] {{/* remove trailing whitespace */ -}}
-  [[Debug log]({{$.LogBaseUrl}}/replaying_after_recording/{{.}}.log)]
-{{/* remove trailing whitespace */ -}}
+| Test Name | Recording Mode | Replaying Rerun |
+| :--- | :--- | :--- |
+{{range .AttemptedTests -}}
+| `{{.}}` | {{if contains $.RecordingResult.PassedTests .}}{{color "green" "Passed"}} [[Debug log]({{$.LogBaseUrl}}/recording/{{.}}.log)]{{else if contains $.RecordingResult.FailedTests .}}{{color "red" "Failed"}} [[Error message]({{$.LogBaseUrl}}/build-log/recording_build/{{compoundTest .}}_recording_test.log)] [[Debug log]({{$.LogBaseUrl}}/recording/{{.}}.log)]{{else}}{{color "red" "Terminated"}}{{end}} | {{if contains $.ReplayingAfterRecordingResult.PassedTests .}}{{color "green" "Passed"}}{{else if contains $.ReplayingAfterRecordingResult.FailedTests .}}{{color "red" "Failed"}} [[Error message]({{$.LogBaseUrl}}/build-log/replaying_build_after_recording/{{compoundTest .}}_replaying_test.log)] [[Debug log]({{$.LogBaseUrl}}/replaying_after_recording/{{.}}.log)]{{else}}-{{end}} |
 {{end}}
 
-Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.
-
-Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.
-
-{{else}}
+{{if gt (len .ReplayingAfterRecordingResult.FailedTests) 0}}
+> [!WARNING]
+> {{color "red" "**Tests failed when rerunning REPLAYING mode after recording!**"}}
+> Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.
+> Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.
+{{else if gt (len .RecordingResult.PassedTests) 0}}
 {{color "green" "No issues found for passed tests after REPLAYING rerun."}}
-{{end}}{{/* end of if gt (len .ReplayingAfterRecordingResult.FailedTests ) 0 */}}
----
-{{end}}{{/* end of if gt (len .RecordingResult.PassedTests) 0 */}}
-
-{{if gt (len .RecordingResult.FailedTests) 0 -}}
-{{color "red" "Tests failed during RECORDING mode:"}}
-{{range .RecordingResult.FailedTests -}}
-`{{.}}` {{/* remove trailing whitespace */ -}}
-  [[Error message]({{$.LogBaseUrl}}/build-log/recording_build/{{compoundTest .}}_recording_test.log)] {{/* remove trailing whitespace */ -}}
-  [[Debug log]({{$.LogBaseUrl}}/recording/{{.}}.log)]
-{{/* remove trailing whitespace */ -}}
 {{end}}
-{{end}} {{- /* end of if gt (len .RecordingResult.FailedTests) 0 */ -}}
 
-{{if .HasTerminatedTests}}{{color "red" "Several tests terminated during RECORDING mode."}}{{end}}
+{{if .HasTerminatedTests}}
+> [!WARNING]
+> {{color "red" "Several tests terminated during RECORDING mode."}}
+{{end}}
 
-{{if .RecordingErr}}{{color "red" "Errors occurred during RECORDING mode. Please fix them to complete your PR."}}{{end}}
+{{if .RecordingErr}}
+> [!CAUTION]
+> {{color "red" "Errors occurred during RECORDING mode. Please fix them to complete your PR."}}
+{{end}}
 
-{{if .AllRecordingPassed}}{{color "green" "All tests passed!"}}{{end}}
+{{if .AllRecordingPassed}}
+{{color "green" "**All tests passed!**"}}
+{{end}}
 
-View the [build log]({{.LogBaseUrl}}/build-log/recording_test.log) {{/* remove trailing whitespace */ -}}
-or the [debug log]({{.BrowseLogBaseUrl}}/recording) for each test
+View the [build log]({{.LogBaseUrl}}/build-log/recording_test.log) or the [debug logs folder]({{.BrowseLogBaseUrl}}/recording) for detailed results.