diff --git a/.ci/magician/cmd/generate_comment.go b/.ci/magician/cmd/generate_comment.go index d11cc81962ae..3c957a622584 100644 --- a/.ci/magician/cmd/generate_comment.go +++ b/.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 } diff --git a/.ci/magician/cmd/interfaces.go b/.ci/magician/cmd/interfaces.go index e63f5b1bcd6f..e2037c72eef9 100644 --- a/.ci/magician/cmd/interfaces.go +++ b/.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 diff --git a/.ci/magician/cmd/mock_github_test.go b/.ci/magician/cmd/mock_github_test.go index 5024dddb661d..49edd0327123 100644 --- a/.ci/magician/cmd/mock_github_test.go +++ b/.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 { diff --git a/.ci/magician/cmd/reassign_reviewer.go b/.ci/magician/cmd/reassign_reviewer.go index 33ddbfd4e620..a33e6cf59bcf 100644 --- a/.ci/magician/cmd/reassign_reviewer.go +++ b/.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 } diff --git a/.ci/magician/cmd/request_reviewer.go b/.ci/magician/cmd/request_reviewer.go index f019644bc888..d28ec2d91efb 100644 --- a/.ci/magician/cmd/request_reviewer.go +++ b/.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 } diff --git a/.ci/magician/cmd/templates/vcr/post_replay.tmpl b/.ci/magician/cmd/templates/vcr/post_replay.tmpl index b914d0286884..abb4d279e39e 100644 --- a/.ci/magician/cmd/templates/vcr/post_replay.tmpl +++ b/.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}} |
-Click here to see the affected service packages -
+Affected Service Packages + {{if .RunFullVCR}} -All service packages are affected +* All service packages are affected {{else if gt (len .AffectedServices) 0}} - +{{range .AffectedServices}}{{. | printf "* %s\n"}}{{end}} {{else}} -None +* None {{end}} -
-{{ 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
-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 - -
- -
-
+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 -[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}} + +{{ 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}} diff --git a/.ci/magician/cmd/templates/vcr/record_replay.tmpl b/.ci/magician/cmd/templates/vcr/record_replay.tmpl index e618d2d63b4a..be5f2a6fe7f6 100644 --- a/.ci/magician/cmd/templates/vcr/record_replay.tmpl +++ b/.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. diff --git a/.ci/magician/cmd/test_terraform_vcr.go b/.ci/magician/cmd/test_terraform_vcr.go index b0c9237aaa0d..b312d050afcf 100644 --- a/.ci/magician/cmd/test_terraform_vcr.go +++ b/.ci/magician/cmd/test_terraform_vcr.go @@ -10,6 +10,8 @@ import ( "strings" "text/template" + "strconv" + "github.com/spf13/cobra" "magician/exec" @@ -69,6 +71,7 @@ type postReplay struct { } type recordReplay struct { + AttemptedTests []string RecordingResult vcr.Result ReplayingAfterRecordingResult vcr.Result HasTerminatedTests bool @@ -231,7 +234,7 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, return fmt.Errorf("error uploading replaying logs: %w", err) } - if hasPanics, err := handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha, replayingResult, vcr.Replaying, gh); err != nil { + if hasPanics, err := handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha, replayingResult, vcr.Replaying, gh, rnr); err != nil { return fmt.Errorf("error handling panics: %w", err) } else if hasPanics { return nil @@ -260,8 +263,14 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, if err != nil { return fmt.Errorf("error formatting post replay comment: %w", err) } - if err := gh.PostComment(prNumber, comment); err != nil { - return fmt.Errorf("error posting comment: %w", err) + if len(replayingResult.FailedTests) == 0 { + mentionStr := getMentions(prNumber, gh) + if mentionStr != "" { + comment = fmt.Sprintf("%s\n\n%s VCR tests complete for %s!", comment, mentionStr, mmCommitSha) + } + } + if err := appendVCRResultToDiffComment(prNumber, comment, gh, rnr); err != nil { + return fmt.Errorf("error appending comment: %w", err) } if len(replayingResult.FailedTests) > 0 { recordingResult, recordingErr := vt.RunParallel(vcr.RunOptions{ @@ -291,7 +300,7 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, return fmt.Errorf("error uploading recording logs: %w", err) } - if hasPanics, err := handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha, recordingResult, vcr.Recording, gh); err != nil { + if hasPanics, err := handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha, recordingResult, vcr.Recording, gh, rnr); err != nil { return fmt.Errorf("error handling panics: %w", err) } else if hasPanics { return nil @@ -327,6 +336,7 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, allRecordingPassed := len(recordingResult.FailedTests) == 0 && !hasTerminatedTests && recordingErr == nil recordReplayData := recordReplay{ + AttemptedTests: replayingResult.FailedTests, RecordingResult: subtestResult(recordingResult), ReplayingAfterRecordingResult: subtestResult(replayingAfterRecordingResult), RecordingErr: recordingErr, @@ -341,8 +351,12 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, if err != nil { return fmt.Errorf("error formatting record replay comment: %w", err) } - if err := gh.PostComment(prNumber, recordReplayComment); err != nil { - return fmt.Errorf("error posting comment: %w", err) + mentionStr := getMentions(prNumber, gh) + if mentionStr != "" { + recordReplayComment = fmt.Sprintf("%s\n\n%s VCR tests complete for %s!", recordReplayComment, mentionStr, mmCommitSha) + } + if err := appendVCRResultToDiffComment(prNumber, recordReplayComment, gh, rnr); err != nil { + return fmt.Errorf("error appending comment: %w", err) } } @@ -492,13 +506,17 @@ func runReplaying(runFullVCR bool, version provider.Version, services map[string return result, testDirs, replayingErr } -func handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha string, result vcr.Result, mode vcr.Mode, gh GithubClient) (bool, error) { +func handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha string, result vcr.Result, mode vcr.Mode, gh GithubClient, rnr ExecRunner) (bool, error) { if len(result.Panics) > 0 { comment := color("red", fmt.Sprintf("The provider crashed while running the VCR tests in %s mode\n", mode.Upper())) comment += fmt.Sprintf(`Please fix it to complete your PR. View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/build-log/%s_test.log)`, prNumber, buildID, mode.Lower()) - if err := gh.PostComment(prNumber, comment); err != nil { - return true, fmt.Errorf("error posting comment: %v", err) + mentionStr := getMentions(prNumber, gh) + if mentionStr != "" { + comment = fmt.Sprintf("%s\n\n%s VCR tests complete for %s!", comment, mentionStr, mmCommitSha) + } + if err := appendVCRResultToDiffComment(prNumber, comment, gh, rnr); err != nil { + return true, fmt.Errorf("error appending comment: %v", err) } if err := gh.PostBuildStatus(prNumber, "VCR-test", "failure", buildStatusTargetURL, mmCommitSha); err != nil { return true, fmt.Errorf("error posting failure status: %v", err) @@ -508,6 +526,62 @@ View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/head return false, nil } +func getMentions(prNumber string, gh GithubClient) string { + author := "" + reviewerMentions := "" + if authorName, err := gh.GetPullRequestAuthor(prNumber); err == nil { + author = "@" + authorName + } + if reviewers, err := gh.GetPullRequestRequestedReviewers(prNumber); err == nil { + var mentions []string + for _, r := range reviewers { + mentions = append(mentions, "@"+r.Login) + } + if len(mentions) > 0 { + reviewerMentions = strings.Join(mentions, ", ") + } + } + + mentionStr := "" + if author != "" { + mentionStr += author + } + if reviewerMentions != "" { + if mentionStr != "" { + mentionStr += ", " + } + mentionStr += reviewerMentions + } + return mentionStr +} + +// appendVCRResultToDiffComment appends content to the existing diff report comment +// identified by the ID in /workspace/diff_comment_id.txt. +// If the file is missing or the comment cannot be fetched, it falls back to posting a new comment. +func appendVCRResultToDiffComment(prNumber string, content string, gh GithubClient, rnr ExecRunner) error { + var diffComment *github.PullRequestComment + + // Try to find by ID from file + if idStr, err := rnr.ReadFile("/workspace/diff_comment_id.txt"); err == nil { + if id, err := strconv.Atoi(strings.TrimSpace(idStr)); err == nil { + if comment, err := gh.GetPullRequestComment(id); err == nil { + diffComment = &comment + } else { + fmt.Printf("Warning: failed to fetch comment %d by ID: %v\n", id, err) + } + } + } + + if diffComment != nil { + newBody := diffComment.Body + "\n\n" + content + return gh.UpdateComment(prNumber, newBody, diffComment.ID) + } + + // Fallback to posting a new comment if diff report not found + _, err := gh.PostComment(prNumber, content) + return err +} + func init() { rootCmd.AddCommand(testTerraformVCRCmd) } @@ -518,6 +592,14 @@ func formatComment(fileName string, tmplText string, data any) (string, error) { "add": func(i, j int) int { return i + j }, "color": color, "compoundTest": compoundTest, + "contains": func(slice []string, item string) bool { + for _, s := range slice { + if s == item { + return true + } + } + return false + }, } tmpl, err := template.New(fileName).Funcs(funcs).Parse(tmplText) if err != nil { diff --git a/.ci/magician/cmd/test_terraform_vcr_test.go b/.ci/magician/cmd/test_terraform_vcr_test.go index 835d9d302d32..8ea0df694dd3 100644 --- a/.ci/magician/cmd/test_terraform_vcr_test.go +++ b/.ci/magician/cmd/test_terraform_vcr_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" + "magician/github" "magician/provider" "magician/vcr" ) @@ -235,19 +236,12 @@ func TestAnalyticsComment(t *testing.T) { AffectedServices: []string{}, }, wantContains: []string{ - "#### Tests analytics", - "Total tests: 6", - "Passed tests: 3", - "Skipped tests: 2", - "Affected tests: 1", - "", + "#### Analytics", + "| Total Tests | Passed | Skipped | Affected |", + "| 6 | 3 | 2 | 1 |", "
", - "Click here to see the affected service packages", - "
", - "", - "None", - "", - "
", + "Affected Service Packages", + "* None", "
", }, }, @@ -263,23 +257,13 @@ func TestAnalyticsComment(t *testing.T) { AffectedServices: []string{"svc-a", "svc-b"}, }, wantContains: []string{ - "#### Tests analytics", - "Total tests: 6", - "Passed tests: 3", - "Skipped tests: 2", - "Affected tests: 1", - "", + "#### Analytics", + "| Total Tests | Passed | Skipped | Affected |", + "| 6 | 3 | 2 | 1 |", "
", - "Click here to see the affected service packages", - "
", - "", - "", - "", - "
", + "Affected Service Packages", + "* svc-a", + "* svc-b", "
", }, }, @@ -295,19 +279,12 @@ func TestAnalyticsComment(t *testing.T) { AffectedServices: []string{}, }, wantContains: []string{ - "#### Tests analytics", - "Total tests: 6", - "Passed tests: 3", - "Skipped tests: 2", - "Affected tests: 1", - "", + "#### Analytics", + "| Total Tests | Passed | Skipped | Affected |", + "| 6 | 3 | 2 | 1 |", "
", - "Click here to see the affected service packages", - "
", - "", - "All service packages are affected", - "", - "
", + "Affected Service Packages", + "* All service packages are affected", "
", }, }, @@ -413,18 +390,11 @@ func TestWithReplayFailedTests(t *testing.T) { wantContains: []string{ "#### Action taken", "
", - "Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests", - "", - "
", - "", - "
", + "Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests", + "* a", + "* b", "
", - "", - "[Get to know how VCR tests work](https://googlecloudplatform.github.io/magic-modules/develop/test/test/)", + "[Learn how VCR tests work](https://googlecloudplatform.github.io/magic-modules/develop/test/test/)", }, }, } @@ -459,7 +429,8 @@ func TestWithoutReplayFailedTests(t *testing.T) { Version: provider.Beta.String(), }, wantContains: []string{ - color("red", "Errors occurred during REPLAYING mode. Please fix them to complete your PR."), + "> [!CAUTION]", + "🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.", "View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/replaying_test.log)", }, }, @@ -472,7 +443,7 @@ func TestWithoutReplayFailedTests(t *testing.T) { Version: provider.Beta.String(), }, wantContains: []string{ - color("green", "All tests passed!"), + "🟢 **All tests passed in Replaying mode! No Recording was needed.**", "View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/replaying_test.log)", }, }, @@ -501,6 +472,7 @@ func TestRecordReplay(t *testing.T) { { name: "ReplayingAfterRecordingResult has failed tests", data: recordReplay{ + AttemptedTests: []string{"a", "b", "c", "d", "e"}, RecordingResult: vcr.Result{ PassedTests: []string{"a", "b", "c"}, FailedTests: []string{"d", "e"}, @@ -517,27 +489,27 @@ func TestRecordReplay(t *testing.T) { Head: "auto-pr-123", }, wantContains: []string{ - color("green", "Tests passed during RECORDING mode:"), - "`a` [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/a.log)]", - "`b` [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/b.log)]", - "`c` [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/c.log)]", - color("red", "Tests failed when rerunning REPLAYING mode:"), - "`b` [[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/replaying_build_after_recording/b_replaying_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/replaying_after_recording/b.log)]", - "`c` [[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/replaying_build_after_recording/c_replaying_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/replaying_after_recording/c.log)]", + "| Test Name | Recording Mode | Replaying Rerun |", + "| `a` | 🟢 Passed [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/a.log)] | 🟢 Passed |", + "| `b` | 🟢 Passed [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/b.log)] | 🔴 Failed [[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/replaying_build_after_recording/b_replaying_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/replaying_after_recording/b.log)] |", + "| `c` | 🟢 Passed [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/c.log)] | 🔴 Failed [[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/replaying_build_after_recording/c_replaying_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/replaying_after_recording/c.log)] |", + "| `d` | 🔴 Failed [[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/recording_build/d_recording_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/d.log)] | - |", + "| `e` | 🔴 Failed [[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/recording_build/e_recording_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/e.log)] | - |", + "> [!WARNING]", + "🔴 **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.", - color("red", "Tests failed during RECORDING mode:"), - "`d` [[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/recording_build/d_recording_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/d.log)]", - "`e` [[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/recording_build/e_recording_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/e.log)]", - color("red", "Several tests terminated during RECORDING mode."), - "Errors occurred during RECORDING mode. Please fix them to complete your PR.", - "[build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/recording_test.log)", - "[debug log](https://console.cloud.google.com/storage/browser/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording)", + "> [!WARNING]", + "🔴 Several tests terminated during RECORDING mode.", + "> [!CAUTION]", + "🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.", + "View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/recording_test.log) or the [debug logs folder](https://console.cloud.google.com/storage/browser/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording) for detailed results.", }, }, { name: "ReplayingAfterRecordingResult does not have failed tests", data: recordReplay{ + AttemptedTests: []string{"a", "b", "c"}, RecordingResult: vcr.Result{ PassedTests: []string{"a", "b", "c"}, }, @@ -551,17 +523,13 @@ func TestRecordReplay(t *testing.T) { LogBucket: "ci-vcr-logs", }, wantContains: []string{ - color("green", "Tests passed during RECORDING mode:"), - "`a`", - "[[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/a.log)]", - "`b`", - "[[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/b.log)]", - "`c`", - "[[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/c.log)]", - color("green", "No issues found for passed tests after REPLAYING rerun."), - color("green", "All tests passed!"), - "[build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/recording_test.log)", - "[debug log](https://console.cloud.google.com/storage/browser/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording)", + "| Test Name | Recording Mode | Replaying Rerun |", + "| `a` | 🟢 Passed [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/a.log)] | 🟢 Passed |", + "| `b` | 🟢 Passed [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/b.log)] | 🟢 Passed |", + "| `c` | 🟢 Passed [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording/c.log)] | 🟢 Passed |", + "🟢 No issues found for passed tests after REPLAYING rerun.", + "🟢 **All tests passed!**", + "View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/build-log/recording_test.log) or the [debug logs folder](https://console.cloud.google.com/storage/browser/ci-vcr-logs/beta/refs/heads/auto-pr-123/artifacts/build-123/recording) for detailed results.", }, }, } @@ -579,3 +547,62 @@ func TestRecordReplay(t *testing.T) { }) } } + +func TestAppendVCRResultToDiffComment_NotExists(t *testing.T) { + gh := &mockGithub{ + calledMethods: make(map[string][][]any), + pullRequest: github.PullRequest{ + User: github.User{Login: "author1"}, + }, + requestedReviewers: []github.User{ + {Login: "reviewer1"}, + {Login: "reviewer2"}, + }, + pullRequestComments: []github.PullRequestComment{ + { + ID: 456, + Body: "Some other comment", + }, + }, + } + + rnr := &mockRunner{} + err := appendVCRResultToDiffComment("123", "VCR Results", gh, rnr) + + assert.NoError(t, err) + assert.Len(t, gh.calledMethods["PostComment"], 1) + assert.Equal(t, "123", gh.calledMethods["PostComment"][0][0]) + assert.Equal(t, "VCR Results", gh.calledMethods["PostComment"][0][1]) +} + +func TestAppendVCRResultToDiffComment_UseFileID(t *testing.T) { + gh := &mockGithub{ + calledMethods: make(map[string][][]any), + pullRequest: github.PullRequest{ + User: github.User{Login: "author1"}, + }, + requestedReviewers: []github.User{ + {Login: "reviewer1"}, + {Login: "reviewer2"}, + }, + pullRequestComments: []github.PullRequestComment{ + { + ID: 456, + Body: "Some comment", + }, + }, + } + rnr := &mockRunner{ + fileContents: map[string]string{ + "/workspace/diff_comment_id.txt": "456", + }, + } + + err := appendVCRResultToDiffComment("123", "VCR Results", gh, rnr) + + assert.NoError(t, err) + assert.Len(t, gh.calledMethods["UpdateComment"], 1) + assert.Equal(t, "123", gh.calledMethods["UpdateComment"][0][0]) + assert.Contains(t, gh.calledMethods["UpdateComment"][0][1].(string), "VCR Results") + assert.Equal(t, 456, gh.calledMethods["UpdateComment"][0][2]) +} diff --git a/.ci/magician/github/get.go b/.ci/magician/github/get.go index 8df94827a5f9..06a6c74b14c7 100644 --- a/.ci/magician/github/get.go +++ b/.ci/magician/github/get.go @@ -70,6 +70,15 @@ func (c *Client) GetPullRequest(prNumber string) (PullRequest, error) { return convertGHPullRequest(pr), nil } +// GetPullRequestAuthor fetches the author of a pull request +func (c *Client) GetPullRequestAuthor(prNumber string) (string, error) { + pr, err := c.GetPullRequest(prNumber) + if err != nil { + return "", err + } + return pr.User.Login, nil +} + // GetPullRequests fetches multiple pull requests func (c *Client) GetPullRequests(state, base, sort, direction string) ([]PullRequest, error) { opts := &gh.PullRequestListOptions{ @@ -184,6 +193,15 @@ func (c *Client) GetPullRequestComments(prNumber string) ([]PullRequestComment, return convertGHComments(allComments), nil } +// GetPullRequestComment fetches a single comment by ID +func (c *Client) GetPullRequestComment(commentID int) (PullRequestComment, error) { + comment, _, err := c.gh.Issues.GetComment(c.ctx, defaultOwner, defaultRepo, int64(commentID)) + if err != nil { + return PullRequestComment{}, err + } + return convertGHComment(comment), nil +} + // GetTeamMembers gets all members of a team, handling pagination func (c *Client) GetTeamMembers(organization, team string) ([]User, error) { var allMembers []*gh.User diff --git a/.ci/magician/github/set.go b/.ci/magician/github/set.go index c04e7daf936b..9873d249c49b 100644 --- a/.ci/magician/github/set.go +++ b/.ci/magician/github/set.go @@ -43,24 +43,24 @@ func (c *Client) PostBuildStatus(prNumber, title, state, targetURL, commitSha st return nil } -// PostComment adds a comment to a pull request -func (c *Client) PostComment(prNumber, comment string) error { +// PostComment adds a comment to a pull request and returns the comment ID +func (c *Client) PostComment(prNumber, comment string) (int, error) { num, err := strconv.Atoi(prNumber) if err != nil { - return err + return 0, err } issueComment := &gh.IssueComment{ Body: gh.Ptr(comment), } - _, _, err = c.gh.Issues.CreateComment(c.ctx, defaultOwner, defaultRepo, num, issueComment) + respComment, _, err := c.gh.Issues.CreateComment(c.ctx, defaultOwner, defaultRepo, num, issueComment) if err != nil { - return err + return 0, err } fmt.Printf("Successfully posted comment to pull request %s\n", prNumber) - return nil + return int(respComment.GetID()), nil } // UpdateComment updates an existing comment diff --git a/mmv1/products/cloudrun/DomainMapping.yaml b/mmv1/products/cloudrun/DomainMapping.yaml index 4bdff79c5e6b..221fc08366f9 100644 --- a/mmv1/products/cloudrun/DomainMapping.yaml +++ b/mmv1/products/cloudrun/DomainMapping.yaml @@ -58,6 +58,9 @@ parameters: description: The location of the cloud run instance. eg us-central1 url_param_only: true properties: + - name: dummy + type: String + description: field to trigger missing test report for comment testing - name: name type: String required: true diff --git a/mmv1/third_party/terraform/services/cloudbilling/resource_google_billing_project_info_test.go b/mmv1/third_party/terraform/services/cloudbilling/resource_google_billing_project_info_test.go index 17638d20f3a9..9cd178cfad8f 100644 --- a/mmv1/third_party/terraform/services/cloudbilling/resource_google_billing_project_info_test.go +++ b/mmv1/third_party/terraform/services/cloudbilling/resource_google_billing_project_info_test.go @@ -10,6 +10,8 @@ import ( "github.com/hashicorp/terraform-provider-google/google/envvar" ) +// trigger tests + func TestAccBillingProjectInfo_update(t *testing.T) { t.Parallel() diff --git a/mmv1/third_party/terraform/services/cloudrun/resource_cloud_run_service_test.go.tmpl b/mmv1/third_party/terraform/services/cloudrun/resource_cloud_run_service_test.go.tmpl index 0982e5effaa8..207ea0942883 100644 --- a/mmv1/third_party/terraform/services/cloudrun/resource_cloud_run_service_test.go.tmpl +++ b/mmv1/third_party/terraform/services/cloudrun/resource_cloud_run_service_test.go.tmpl @@ -393,7 +393,7 @@ resource "google_cloud_run_service" "default" { func TestAccCloudRunService_withProviderDefaultLabels(t *testing.T) { // The test failed if VCR testing is enabled, because the cached provider config is used. // With the cached provider config, any changes in the provider default labels will not be applied. - acctest.SkipIfVcr(t) + // acctest.SkipIfVcr(t) t.Parallel() context := map[string]interface{}{ @@ -1308,7 +1308,7 @@ func TestAccCloudRunService_withCreationOnlyAttribution(t *testing.T) { func TestAccCloudRunService_withProactiveAttribution(t *testing.T) { // VCR tests cache provider configuration between steps, this test changes provider configuration and fails under VCR. - acctest.SkipIfVcr(t) + // acctest.SkipIfVcr(t) t.Parallel() suffix := acctest.RandString(t, 10) diff --git a/mmv1/third_party/terraform/services/dataproc/resource_dataproc_cluster_test.go b/mmv1/third_party/terraform/services/dataproc/resource_dataproc_cluster_test.go index c8bc394da7ac..f1fe31195b04 100644 --- a/mmv1/third_party/terraform/services/dataproc/resource_dataproc_cluster_test.go +++ b/mmv1/third_party/terraform/services/dataproc/resource_dataproc_cluster_test.go @@ -1038,7 +1038,7 @@ func TestAccDataprocCluster_withLifecycleConfigIdleDeleteTtl(t *testing.T) { func TestAccDataprocCluster_withLifecycleConfigAutoDeletion(t *testing.T) { // Uses time.Now - acctest.SkipIfVcr(t) + // acctest.SkipIfVcr(t) t.Parallel() rnd := acctest.RandString(t, 10) @@ -1102,7 +1102,7 @@ func TestAccDataprocCluster_withLifecycleConfigIdleStopTtl(t *testing.T) { func TestAccDataprocCluster_withLifecycleConfigAutoStop(t *testing.T) { // Uses time.Now - acctest.SkipIfVcr(t) + // acctest.SkipIfVcr(t) t.Parallel() rnd := acctest.RandString(t, 10)