chore(ci): jsonify missing tests and move template into DIFF_COMMIT.md (

GoogleCloudPlatform#10816)

.ci/magician/cmd/DIFF_COMMENT.md

@@ -24,7 +24,22 @@ If you believe this detection to be incorrect please raise the concern with your
 If you intend to make this change you will need to wait for a [major release](https://www.terraform.io/plugin/sdkv2/best-practices/versioning#example-major-number-increments) window.
 An `override-breaking-change` label can be added to allow merging.
 {{end}}
-{{.MissingTests}}
+
+{{if gt (len .MissingTests) 0}}
+## Missing test report
+Your PR includes resource fields which are not covered by any test.
+{{ range $resourceName, $missingTestInfo := .MissingTests }}
+Resource: `{{ $resourceName }}` ({{ len $missingTestInfo.Tests }} total tests)
+Please add an acceptance test which includes these fields. The test should include the following:
+
+```hcl
+{{ $missingTestInfo.SuggestedTest }}
+
+```
+
+{{- end }}
+{{end}}
+
 {{- $errorsLength := len .Errors}}
 {{- if gt $errorsLength 0}}
 ## Errors

.ci/magician/cmd/generate_comment.go

@@ -55,6 +55,11 @@ type BreakingChange struct {
 	DocumentationReference string
 }
 
+type MissingTestInfo struct {
+	SuggestedTest string
+	Tests         []string
+}
+
 type Errors struct {
 	Title  string
 	Errors []string
@@ -64,7 +69,7 @@ type diffCommentData struct {
 	PrNumber        int
 	Diffs           []Diff
 	BreakingChanges []BreakingChange
-	MissingTests    string
+	MissingTests    map[string]*MissingTestInfo
 	Errors          []Errors
 }
 
@@ -465,17 +470,21 @@ func changedSchemaResources(diffProcessorPath string, rnr ExecRunner) ([]string,
 // Run the missing test detector and return the results.
 // Returns an empty string unless there are missing tests.
 // Error will be nil unless an error occurs during setup.
-func detectMissingTests(diffProcessorPath, tpgbLocalPath string, rnr ExecRunner) (string, error) {
+func detectMissingTests(diffProcessorPath, tpgbLocalPath string, rnr ExecRunner) (map[string]*MissingTestInfo, error) {
 	if err := rnr.PushDir(diffProcessorPath); err != nil {
-		return "", err
+		return nil, err
 	}
 
 	output, err := rnr.Run("bin/diff-processor", []string{"detect-missing-tests", fmt.Sprintf("%s/google-beta/services", tpgbLocalPath)}, nil)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
-	return output, rnr.PopDir()
+	var missingTests map[string]*MissingTestInfo
+	if err = json.Unmarshal([]byte(output), &missingTests); err != nil {
+		return nil, err
+	}
+	return missingTests, rnr.PopDir()
 }
 
 func formatDiffComment(data diffCommentData) (string, error) {

.ci/magician/cmd/generate_comment_test.go

@@ -105,7 +105,7 @@ func TestExecGenerateComment(t *testing.T) {
 
 	for method, expectedCalls := range map[string][][]any{
 		"PostBuildStatus": {{"123456", "terraform-provider-breaking-change-test", "success", "https://console.cloud.google.com/cloud-build/builds;region=global/build1;step=17?project=project1", "sha1"}},
-		"PostComment":     {{"123456", "Hi there, I'm the Modular magician. I've detected the following information about your changes:\n\n## Diff report\n\nYour PR generated some diffs in downstreams - here they are.\n\n`google` provider: [Diff](https://github.com/modular-magician/terraform-provider-google/compare/auto-pr-123456-old..auto-pr-123456) ( 2 files changed, 40 insertions(+))\n`google-beta` provider: [Diff](https://github.com/modular-magician/terraform-provider-google-beta/compare/auto-pr-123456-old..auto-pr-123456) ( 2 files changed, 40 insertions(+))\n`terraform-google-conversion`: [Diff](https://github.com/modular-magician/terraform-google-conversion/compare/auto-pr-123456-old..auto-pr-123456) ( 1 file changed, 10 insertions(+))\n\n## Missing test report\nYour PR includes resource fields which are not covered by any test.\n\nResource: `google_folder_access_approval_settings` (3 total tests)\nPlease add an acceptance test which includes these fields. The test should include the following:\n\n```hcl\nresource \"google_folder_access_approval_settings\" \"primary\" {\n  uncovered_field = # value needed\n}\n\n```\n"}},
+		"PostComment":     {{"123456", "Hi there, I'm the Modular magician. I've detected the following information about your changes:\n\n## Diff report\n\nYour PR generated some diffs in downstreams - here they are.\n\n`google` provider: [Diff](https://github.com/modular-magician/terraform-provider-google/compare/auto-pr-123456-old..auto-pr-123456) ( 2 files changed, 40 insertions(+))\n`google-beta` provider: [Diff](https://github.com/modular-magician/terraform-provider-google-beta/compare/auto-pr-123456-old..auto-pr-123456) ( 2 files changed, 40 insertions(+))\n`terraform-google-conversion`: [Diff](https://github.com/modular-magician/terraform-google-conversion/compare/auto-pr-123456-old..auto-pr-123456) ( 1 file changed, 10 insertions(+))\n\n\n\n## Missing test report\nYour PR includes resource fields which are not covered by any test.\n\nResource: `google_folder_access_approval_settings` (3 total tests)\nPlease add an acceptance test which includes these fields. The test should include the following:\n\n```hcl\nresource \"google_folder_access_approval_settings\" \"primary\" {\n  uncovered_field = # value needed\n}\n\n```\n"}},
 		"AddLabels":       {{"123456", []string{"service/alloydb"}}},
 	} {
 		if actualCalls, ok := gh.calledMethods[method]; !ok {
@@ -214,7 +214,12 @@ func TestFormatDiffComment(t *testing.T) {
 		},
 		"missing tests are displayed": {
 			data: diffCommentData{
-				MissingTests: "## Missing test report",
+				MissingTests: map[string]*MissingTestInfo{
+					"resource": {
+						Tests:         []string{"test-a", "test-b"},
+						SuggestedTest: "x",
+					},
+				},
 			},
 			expectedStrings: []string{
 				"## Diff report",

.ci/magician/cmd/mock_runner_test.go

@@ -71,7 +71,7 @@ func NewMockRunner() MockRunner {
 			"/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [breaking-changes] map[]":                                                                                            "",
 			"/mock/dir/magic-modules/tools/diff-processor make [build] " + sortedEnvString(diffProcessorEnv):                                                                                      "",
 			"/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [changed-schema-resources] map[]":                                                                                    "[\"google_alloydb_instance\"]",
-			"/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [detect-missing-tests /mock/dir/tpgb/google-beta/services] map[]":                                                    "## Missing test report\nYour PR includes resource fields which are not covered by any test.\n\nResource: `google_folder_access_approval_settings` (3 total tests)\nPlease add an acceptance test which includes these fields. The test should include the following:\n\n```hcl\nresource \"google_folder_access_approval_settings\" \"primary\" {\n  uncovered_field = # value needed\n}\n\n```\n",
+			"/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [detect-missing-tests /mock/dir/tpgb/google-beta/services] map[]":                                                    `{"google_folder_access_approval_settings":{"SuggestedTest":"resource \"google_folder_access_approval_settings\" \"primary\" {\n  uncovered_field = # value needed\n}","Tests":["a","b","c"]}}`,
 			"/mock/dir/tgc git [diff origin/auto-pr-123456-old origin/auto-pr-123456 --shortstat] map[]":                                                                                          " 1 file changed, 10 insertions(+)\n",
 			"/mock/dir/tgc git [fetch origin auto-pr-123456-old] map[]":                                                                                                                           "",
 			"/mock/dir/tfoics git [diff origin/auto-pr-123456-old origin/auto-pr-123456 --shortstat] map[]":                                                                                       "",

tools/diff-processor/cmd/detect_missing_tests.go

@@ -1,13 +1,13 @@
 package cmd
 
 import (
+	"encoding/json"
 	newProvider "google/provider/new/google/provider"
 	oldProvider "google/provider/old/google/provider"
+	"io"
 
 	"fmt"
 	"os"
-	"strings"
-	"text/template"
 
 	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/detector"
 	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
@@ -20,11 +20,13 @@ const detectMissingTestsDesc = "Run the missing test detector using the given se
 
 type detectMissingTestsOptions struct {
 	rootOptions *rootOptions
+	stdout      io.Writer
 }
 
 func newDetectMissingTestsCmd(rootOptions *rootOptions) *cobra.Command {
 	o := &detectMissingTestsOptions{
 		rootOptions: rootOptions,
+		stdout:      os.Stdout,
 	}
 	return &cobra.Command{
 		Use:   "detect-missing-tests SERVICES_DIR",
@@ -49,27 +51,8 @@ func (o *detectMissingTestsOptions) run(args []string) error {
 	if err != nil {
 		return fmt.Errorf("error detecting missing tests: %v", err)
 	}
-	if len(missingTests) > 0 {
-		funcs := template.FuncMap{
-			"join": strings.Join,
-			"backTickAll": func(ss []string) []string {
-				rs := make([]string, len(ss))
-				for i, s := range ss {
-					rs[i] = fmt.Sprintf("`%s`", s)
-				}
-				return rs
-			},
-		}
-		outputTemplate, err := template.New("missing_test_output.tmpl").Funcs(funcs).ParseFiles("missing_test_output.tmpl")
-		if err != nil {
-			return fmt.Errorf("Error parsing missing test template file: %s", err)
-		}
-		if err := outputTemplate.Execute(os.Stdout, missingTests); err != nil {
-			return fmt.Errorf("Error executing missing test output template: %s", err)
-		}
-		for resourceName, missingTestInfo := range missingTests {
-			glog.Infof("%s tests parsed: %v", resourceName, missingTestInfo.Tests)
-		}
+	if err := json.NewEncoder(o.stdout).Encode(missingTests); err != nil {
+		return fmt.Errorf("error encoding json: %w", err)
 	}
 	return nil
 }