Merge pull request #11023 from c2thorn/sync-main-FEATURE-BRANCH-6.0.0

Sync main feature branch 6.0.0

Author: c2thorn

.ci/magician/cmd/test_terraform_vcr.go

@@ -7,6 +7,7 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+	"text/template"
 
 	"github.com/spf13/cobra"
 
@@ -15,6 +16,21 @@ import (
 	"magician/provider"
 	"magician/source"
 	"magician/vcr"
+
+	_ "embed"
+)
+
+var (
+	//go:embed test_terraform_vcr_test_analytics.tmpl
+	testsAnalyticsTmplText string
+	//go:embed test_terraform_vcr_non_exercised_tests.tmpl
+	nonExercisedTestsTmplText string
+	//go:embed test_terraform_vcr_with_replay_failed_tests.tmpl
+	withReplayFailedTestsTmplText string
+	//go:embed test_terraform_vcr_without_replay_failed_tests.tmpl
+	withoutReplayFailedTestsTmplText string
+	//go:embed test_terraform_vcr_record_replay.tmpl
+	recordReplayTmplText string
 )
 
 var ttvEnvironmentVariables = [...]string{
@@ -40,6 +56,37 @@ var ttvEnvironmentVariables = [...]string{
 	"USER",
 }
 
+type analytics struct {
+	ReplayingResult  *vcr.Result
+	RunFullVCR       bool
+	AffectedServices []string
+}
+
+type nonExercisedTests struct {
+	NotRunBetaTests []string
+	NotRunGATests   []string
+}
+
+type withReplayFailedTests struct {
+	ReplayingResult *vcr.Result
+}
+
+type withoutReplayFailedTests struct {
+	ReplayingErr error
+	PRNumber     string
+	BuildID      string
+}
+
+type recordReplay struct {
+	RecordingResult               *vcr.Result
+	ReplayingAfterRecordingResult *vcr.Result
+	HasTerminatedTests            bool
+	RecordingErr                  error
+	AllRecordingPassed            bool
+	PRNumber                      string
+	BuildID                       string
+}
+
 var testTerraformVCRCmd = &cobra.Command{
 	Use:   "test-terraform-vcr",
 	Short: "Run vcr tests for affected packages",
@@ -143,7 +190,7 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep,
 		return fmt.Errorf("error posting pending status: %w", err)
 	}
 
-	replayingResult, affectedServicesComment, testDirs, replayingErr := runReplaying(runFullVCR, services, vt)
+	replayingResult, testDirs, replayingErr := runReplaying(runFullVCR, services, vt)
 	testState := "success"
 	if replayingErr != nil {
 		testState = "failure"
@@ -159,55 +206,41 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep,
 		return nil
 	}
 
-	failedTestsPattern := strings.Join(replayingResult.FailedTests, "|")
-
-	comment := `#### Tests analytics
-Total tests: ` + fmt.Sprintf("`%d`", len(replayingResult.PassedTests)+len(replayingResult.SkippedTests)+len(replayingResult.FailedTests)) + `
-Passed tests: ` + fmt.Sprintf("`%d`", len(replayingResult.PassedTests)) + `
-Skipped tests: ` + fmt.Sprintf("`%d`", len(replayingResult.SkippedTests)) + `
-Affected tests: ` + fmt.Sprintf("`%d`", len(replayingResult.FailedTests)) + `
-
-<details><summary>Click here to see the affected service packages</summary><blockquote>` + affectedServicesComment + `</blockquote></details>`
+	var servicesArr []string
+	for s := range services {
+		servicesArr = append(servicesArr, s)
+	}
+	analyticsData := analytics{
+		ReplayingResult:  replayingResult,
+		RunFullVCR:       runFullVCR,
+		AffectedServices: sort.StringSlice(servicesArr),
+	}
+	testsAnalyticsComment, err := formatTestsAnalytics(analyticsData)
+	if err != nil {
+		return fmt.Errorf("error formatting test_analytics comment: %w", err)
+	}
 
 	notRunBeta, notRunGa := notRunTests(tpgRepo.UnifiedZeroDiff, tpgbRepo.UnifiedZeroDiff, replayingResult)
-	if len(notRunBeta) > 0 || len(notRunGa) > 0 {
-		comment += `
 
-
-#### Non-exercised tests`
-
-		if len(notRunBeta) > 0 {
-			comment += `
-
-Tests were added that are skipped in VCR:
-`
-			for _, t := range notRunBeta {
-				comment += `
-- ` + t
-			}
-		}
-
-		if len(notRunGa) > 0 {
-			comment += `
-
-Tests were added that are GA-only additions and require manual runs:
-`
-			for _, t := range notRunGa {
-				comment += `
-- ` + t
-			}
-		}
+	nonExercisedTestsData := nonExercisedTests{
+		NotRunBetaTests: notRunBeta,
+		NotRunGATests:   notRunGa,
+	}
+	nonExercisedTestsComment, err := formatNonExercisedTests(nonExercisedTestsData)
+	if err != nil {
+		return fmt.Errorf("error formatting non exercised tests comment: %w", err)
 	}
 
 	if len(replayingResult.FailedTests) > 0 {
-		comment += fmt.Sprintf(`
-
-
-#### Action taken
-<details> <summary>Found %d 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>%s </blockquote></details>
-
-[Get to know how VCR tests work](https://googlecloudplatform.github.io/magic-modules/docs/getting-started/contributing/#general-contributing-steps)`, len(replayingResult.FailedTests), failedTestsPattern)
+		withReplayFailedTestsData := withReplayFailedTests{
+			ReplayingResult: replayingResult,
+		}
+		withReplayFailedTestsComment, err := formatWithReplayFailedTests(withReplayFailedTestsData)
+		if err != nil {
+			return fmt.Errorf("error formatting action taken comment: %w", err)
+		}
 
+		comment := strings.Join([]string{testsAnalyticsComment, nonExercisedTestsComment, withReplayFailedTestsComment}, "\n")
 		if err := gh.PostComment(prNumber, comment); err != nil {
 			return fmt.Errorf("error posting comment: %w", err)
 		}
@@ -233,77 +266,54 @@ Tests were added that are GA-only additions and require manual runs:
 			return nil
 		}
 
-		comment = ""
+		var replayingAfterRecordingResult *vcr.Result
+		var replayingAfterRecordingErr error
 		if len(recordingResult.PassedTests) > 0 {
-			comment += "$\\textcolor{green}{\\textsf{Tests passed during RECORDING mode:}}$\n"
-			for _, passedTest := range recordingResult.PassedTests {
-				comment += fmt.Sprintf("`%s`[[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/recording/%s.log)]\n", passedTest, prNumber, buildID, passedTest)
-			}
-			comment += "\n\n"
-
-			replayingAfterRecordingResult, replayingAfterRecordingErr := vt.RunParallel(vcr.Replaying, provider.Beta, testDirs, recordingResult.PassedTests)
+			replayingAfterRecordingResult, replayingAfterRecordingErr = vt.RunParallel(vcr.Replaying, provider.Beta, testDirs, recordingResult.PassedTests)
 			if replayingAfterRecordingErr != nil {
 				testState = "failure"
 			}
 
 			if err := vt.UploadLogs("ci-vcr-logs", prNumber, buildID, true, true, vcr.Replaying, provider.Beta); err != nil {
 				return fmt.Errorf("error uploading recording logs: %w", err)
 			}
-
-			if len(replayingAfterRecordingResult.FailedTests) > 0 {
-				comment += "$\\textcolor{red}{\\textsf{Tests failed when rerunning REPLAYING mode:}}$\n"
-				for _, failedTest := range replayingAfterRecordingResult.FailedTests {
-					comment += fmt.Sprintf("`%s`[[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/build-log/replaying_build_after_recording/%s_replaying_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/replaying_after_recording/%s.log)]\n", failedTest, prNumber, buildID, failedTest, prNumber, buildID, failedTest)
-				}
-				comment += "\n\n"
-				comment += `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 {
-				comment += "$\\textcolor{green}{\\textsf{No issues found for passed tests after REPLAYING rerun.}}$\n"
-			}
-			comment += "\n---\n"
-
 		}
 
-		if len(recordingResult.FailedTests) > 0 {
-			comment += "$\\textcolor{red}{\\textsf{Tests failed during RECORDING mode:}}$\n"
-			for _, failedTest := range recordingResult.FailedTests {
-				comment += fmt.Sprintf("`%s`[[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/build-log/recording_build/%s_recording_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/recording/%s.log)]\n", failedTest, prNumber, buildID, failedTest, prNumber, buildID, failedTest)
-			}
-			comment += "\n\n"
-			if len(recordingResult.PassedTests)+len(recordingResult.FailedTests) < len(replayingResult.FailedTests) {
-				comment += "$\\textcolor{red}{\\textsf{Several tests got terminated during RECORDING mode.}}$\n"
-			}
-			comment += "$\\textcolor{red}{\\textsf{Please fix these to complete your PR.}}$\n"
-		} else {
-			if len(recordingResult.PassedTests)+len(recordingResult.FailedTests) < len(replayingResult.FailedTests) {
-				comment += "$\\textcolor{red}{\\textsf{Several tests got terminated during RECORDING mode.}}$\n"
-			} else if recordingErr != nil {
-				// Check for any uncaught errors in RECORDING mode.
-				comment += "$\\textcolor{red}{\\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$\n"
-			} else {
-				comment += "$\\textcolor{green}{\\textsf{All tests passed!}}$\n"
-			}
+		hasTerminatedTests := (len(recordingResult.PassedTests) + len(recordingResult.FailedTests)) < len(replayingResult.FailedTests)
+		allRecordingPassed := len(recordingResult.FailedTests) == 0 && !hasTerminatedTests && recordingErr == nil
+
+		recordReplayData := recordReplay{
+			RecordingResult:               recordingResult,
+			ReplayingAfterRecordingResult: replayingAfterRecordingResult,
+			RecordingErr:                  recordingErr,
+			HasTerminatedTests:            hasTerminatedTests,
+			AllRecordingPassed:            allRecordingPassed,
+			PRNumber:                      prNumber,
+			BuildID:                       buildID,
+		}
+		recordReplayComment, err := formatRecordReplay(recordReplayData)
+		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)
 		}
 
-		comment += fmt.Sprintf("View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/build-log/recording_test.log) or the [debug log](https://console.cloud.google.com/storage/browser/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/recording) for each test", prNumber, buildID, prNumber, buildID)
-	} else {
-		// Add newlines so that the color formatting will work properly.
-		comment += `
+	} else { //  len(replayingResult.FailedTests) == 0
+		withoutReplayFailedTestsData := withoutReplayFailedTests{
+			ReplayingErr: replayingErr,
+			PRNumber:     prNumber,
+			BuildID:      buildID,
+		}
+		withoutReplayFailedTestsComment, err := formatWithoutReplayFailedTests(withoutReplayFailedTestsData)
+		if err != nil {
+			return fmt.Errorf("error formatting action taken comment: %w", err)
+		}
 
-`
-		if replayingErr != nil {
-			// Check for any uncaught errors in REPLAYING mode.
-			comment += "$\\textcolor{red}{\\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$\n"
-		} else {
-			comment += "$\\textcolor{green}{\\textsf{All tests passed!}}$\n"
+		comment := strings.Join([]string{testsAnalyticsComment, nonExercisedTestsComment, withoutReplayFailedTestsComment}, "\n")
+		if err := gh.PostComment(prNumber, comment); err != nil {
+			return fmt.Errorf("error posting comment: %w", err)
 		}
-		comment += fmt.Sprintf("View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-%s/artifacts/%s/build-log/replaying_test.log)", prNumber, buildID)
-	}
-	if err := gh.PostComment(prNumber, comment); err != nil {
-		return fmt.Errorf("error posting comment: %w", err)
 	}
 
 	if err := gh.PostBuildStatus(prNumber, "VCR-test", testState, buildStatusTargetURL, mmCommitSha); err != nil {
@@ -379,17 +389,14 @@ func modifiedPackages(changedFiles []string) (map[string]struct{}, bool) {
 	return services, runFullVCR
 }
 
-func runReplaying(runFullVCR bool, services map[string]struct{}, vt *vcr.Tester) (*vcr.Result, string, []string, error) {
+func runReplaying(runFullVCR bool, services map[string]struct{}, vt *vcr.Tester) (*vcr.Result, []string, error) {
 	var result *vcr.Result
-	affectedServicesComment := "None"
 	var testDirs []string
 	var replayingErr error
 	if runFullVCR {
 		fmt.Println("run full VCR tests")
-		affectedServicesComment = "all service packages are affected"
 		result, replayingErr = vt.Run(vcr.Replaying, provider.Beta, nil)
 	} else if len(services) > 0 {
-		affectedServicesComment = "<ul>"
 		result = &vcr.Result{}
 		for service := range services {
 			servicePath := "./" + filepath.Join("google-beta", "services", service)
@@ -403,12 +410,10 @@ func runReplaying(runFullVCR bool, services map[string]struct{}, vt *vcr.Tester)
 			result.SkippedTests = append(result.SkippedTests, serviceResult.SkippedTests...)
 			result.FailedTests = append(result.FailedTests, serviceResult.FailedTests...)
 			result.Panics = append(result.Panics, serviceResult.Panics...)
-			affectedServicesComment += fmt.Sprintf("<li>%s</li>", service)
 		}
-		affectedServicesComment += "</ul>"
 	}
 
-	return result, affectedServicesComment, testDirs, replayingErr
+	return result, testDirs, replayingErr
 }
 
 func handlePanics(prNumber, buildID, buildStatusTargetURL, mmCommitSha string, result *vcr.Result, mode vcr.Mode, gh GithubClient) (bool, error) {
@@ -430,3 +435,40 @@ View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/head
 func init() {
 	rootCmd.AddCommand(testTerraformVCRCmd)
 }
+
+func formatComment(fileName string, tmplText string, data any) (string, error) {
+	funcs := template.FuncMap{
+		"join": strings.Join,
+		"add":  func(i, j int) int { return i + j },
+	}
+	tmpl, err := template.New(fileName).Funcs(funcs).Parse(tmplText)
+	if err != nil {
+		panic(fmt.Sprintf("Unable to parse %s: %s", fileName, err))
+	}
+	sb := new(strings.Builder)
+	err = tmpl.Execute(sb, data)
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimSpace(sb.String()), nil
+}
+
+func formatTestsAnalytics(data analytics) (string, error) {
+	return formatComment("test_terraform_vcr_test_analytics.tmpl", testsAnalyticsTmplText, data)
+}
+
+func formatNonExercisedTests(data nonExercisedTests) (string, error) {
+	return formatComment("test_terraform_vcr_recording_mode_results.tmpl", nonExercisedTestsTmplText, data)
+}
+
+func formatWithReplayFailedTests(data withReplayFailedTests) (string, error) {
+	return formatComment("test_terraform_vcr_with_replay_failed_tests.tmpl", withReplayFailedTestsTmplText, data)
+}
+
+func formatWithoutReplayFailedTests(data withoutReplayFailedTests) (string, error) {
+	return formatComment("test_terraform_vcr_without_replay_failed_tests.tmpl", withoutReplayFailedTestsTmplText, data)
+}
+
+func formatRecordReplay(data recordReplay) (string, error) {
+	return formatComment("test_terraform_vcr_record_replay.tmpl", recordReplayTmplText, data)
+}

.ci/magician/cmd/test_terraform_vcr_non_exercised_tests.tmpl

@@ -0,0 +1,13 @@
+{{- if or (gt (len .NotRunBetaTests) 0) (gt (len .NotRunGATests) 0) -}}
+#### Non-exercised tests
+
+{{if gt (len .NotRunBetaTests) 0 -}}
+Tests were added that are skipped in VCR:
+{{range .NotRunBetaTests}}{{. | printf "- %s\n"}}{{end}}
+{{end}}
+
+{{if gt (len .NotRunGATests) 0 -}}
+Tests were added that are GA-only additions and require manual runs:
+{{range .NotRunGATests}}{{. | printf "- %s\n"}}{{end}}
+{{end}}
+{{end}}

.ci/magician/cmd/test_terraform_vcr_record_replay.tmpl

@@ -0,0 +1,31 @@
+{{- if gt (len .RecordingResult.PassedTests) 0 -}}
+$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
+{{range .RecordingResult.PassedTests}}`{{.}}`[[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-{{$.PRNumber}}/artifacts/{{$.BuildID}}/recording/{{.}}.log)]
+{{end}}
+
+{{- if gt (len .ReplayingAfterRecordingResult.FailedTests ) 0 -}}
+$\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$
+{{range .ReplayingAfterRecordingResult.FailedTests}}`{{.}}`[[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-{{$.PRNumber}}/artifacts/{{$.BuildID}}/build-log/replaying_build_after_recording/{{.}}_replaying_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-{{$.PRNumber}}/artifacts/{{$.BuildID}}/replaying_after_recording/{{.}}.log)]
+{{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}}
+$\textcolor{green}{\textsf{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 -}}
+$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
+{{range .RecordingResult.FailedTests}}`{{.}}`[[Error message](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-{{$.PRNumber}}/artifacts/{{$.BuildID}}/build-log/recording_build/{{.}}_recording_test.log)] [[Debug log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-{{$.PRNumber}}/artifacts/{{$.BuildID}}/recording/{{.}}.log)]
+{{end}}
+{{end}} {{- /* end of if gt (len .RecordingResult.FailedTests) 0 */ -}}
+
+{{if .HasTerminatedTests}}$\textcolor{red}{\textsf{Several tests got terminated during RECORDING mode.}}${{end}}
+{{if .RecordingErr}}$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}${{end}}
+{{if .AllRecordingPassed}}$\textcolor{green}{\textsf{All tests passed!}}${{end}}
+
+View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-{{.PRNumber}}/artifacts/{{.BuildID}}/build-log/recording_test.log) or the [debug log](https://console.cloud.google.com/storage/browser/ci-vcr-logs/beta/refs/heads/auto-pr-{{.PRNumber}}/artifacts/{{.BuildID}}/recording) for each test