Merge branch 'GoogleCloudPlatform:main' into main

wj-chen · Apr 11, 2024 · feab221 · feab221
2 parents 1f6f2c3 + 6bbb3ef
commit feab221
Show file tree

Hide file tree

Showing 32 changed files with 551 additions and 878 deletions.
diff --git a/.ci/magician/cmd/generate_comment.go b/.ci/magician/cmd/generate_comment.go
@@ -26,12 +26,13 @@ import (
 	"strings"
 	"text/template"
 
-	"github.com/GoogleCloudPlatform/magic-modules/tools/issue-labeler/labeler"
 	"magician/exec"
 	"magician/github"
 	"magician/provider"
 	"magician/source"
 
+	"github.com/GoogleCloudPlatform/magic-modules/tools/issue-labeler/labeler"
+
 	"github.com/spf13/cobra"
 	"golang.org/x/exp/maps"
 
@@ -265,6 +266,16 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep,
 			uniqueBreakingChanges[breakingChange] = struct{}{}
 		}
 
+		if repo.Name == "terraform-provider-google-beta" {
+			// Run missing test detector (currently only for beta)
+			missingTests, err := detectMissingTests(diffProcessorPath, repo.Path, rnr)
+			if err != nil {
+				fmt.Println("Error running missing test detector: ", err)
+				errors[repo.Title] = append(errors[repo.Title], "The missing test detector failed to run.")
+			}
+			data.MissingTests = missingTests
+		}
+
 		affectedResources, err := changedSchemaResources(diffProcessorPath, rnr)
 		if err != nil {
 			fmt.Println("computing changed resource schemas: ", err)
@@ -348,38 +359,6 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep,
 		errors["Other"] = append(errors["Other"], "Failed to update breaking-change status check with state: "+breakingState)
 	}
 
-	// Run missing test detector (currently only for beta)
-	missingTestsPath := mmLocalPath
-	for _, repo := range []source.Repo{tpgbRepo} {
-		if !repo.Cloned {
-			fmt.Println("Skipping missing tests; repo failed to clone: ", repo.Name)
-			continue
-		}
-		missingTests, err := detectMissingTests(missingTestsPath, repo.Path, oldBranch, rnr)
-		if err != nil {
-			fmt.Println("Error running missing test detector: ", err)
-			errors[repo.Title] = append(errors[repo.Title], "The missing test detector failed to run.")
-		}
-		data.MissingTests = missingTests
-	}
-
-	// Run unit tests for missing test detector (currently only for beta)
-	if pathChanged("tools/missing-test-detector", tpgbRepo.ChangedFiles) {
-		fmt.Printf("Found diffs in missing test detector:\n%s\nRunning tests.\n", diffs)
-		if err = runMissingTestUnitTests(
-			mmLocalPath,
-			tpgbRepo.Path,
-			targetURL,
-			commitSha,
-			prNumber,
-			gh,
-			rnr,
-		); err != nil {
-			fmt.Println("Error running missing test detector unit tests: ", err)
-			errors["Other"] = append(errors["Other"], "Missing test detector unit tests failed to run.")
-		}
-	}
-
 	// Add errors to data as an ordered list
 	errorsList := []Errors{}
 	for _, repo := range []source.Repo{tpgRepo, tpgbRepo, tgcRepo, tfoicsRepo} {
@@ -475,97 +454,17 @@ 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(mmLocalPath, tpgbLocalPath, oldBranch string, rnr ExecRunner) (string, error) {
-	tpgbLocalPathOld := tpgbLocalPath + "old"
-
-	if err := rnr.Copy(tpgbLocalPath, tpgbLocalPathOld); err != nil {
-		return "", err
-	}
-
-	if err := rnr.PushDir(tpgbLocalPathOld); err != nil {
-		return "", err
-	}
-	if _, err := rnr.Run("git", []string{"checkout", "origin/" + oldBranch}, nil); err != nil {
-		return "", err
-	}
-
-	if err := updatePackageName("old", tpgbLocalPathOld, rnr); err != nil {
-		return "", err
-	}
-	if err := updatePackageName("new", tpgbLocalPath, rnr); err != nil {
-		return "", err
-	}
-	if err := rnr.PopDir(); err != nil {
+func detectMissingTests(diffProcessorPath, tpgbLocalPath string, rnr ExecRunner) (string, error) {
+	if err := rnr.PushDir(diffProcessorPath); err != nil {
 		return "", err
 	}
 
-	missingTestDetectorPath := filepath.Join(mmLocalPath, "tools", "missing-test-detector")
-	if err := rnr.PushDir(missingTestDetectorPath); err != nil {
-		return "", err
-	}
-	if _, err := rnr.Run("go", []string{"mod", "edit", "-replace", fmt.Sprintf("google/provider/%s=%s", "new", tpgbLocalPath)}, nil); err != nil {
-		fmt.Printf("Error running go mod edit: %v\n", err)
-	}
-	if _, err := rnr.Run("go", []string{"mod", "edit", "-replace", fmt.Sprintf("google/provider/%s=%s", "old", tpgbLocalPathOld)}, nil); err != nil {
-		fmt.Printf("Error running go mod edit: %v\n", err)
-	}
-	if _, err := rnr.Run("go", []string{"mod", "tidy"}, nil); err != nil {
-		fmt.Printf("Error running go mod tidy: %v\n", err)
-	}
-	missingTests, err := rnr.Run("go", []string{"run", ".", fmt.Sprintf("-services-dir=%s/google-beta/services", tpgbLocalPath)}, nil)
+	output, err := rnr.Run("bin/diff-processor", []string{"detect-missing-tests", fmt.Sprintf("%s/google-beta/services", tpgbLocalPath)}, nil)
 	if err != nil {
-		fmt.Printf("Error running missing test detector: %v\n", err)
-		missingTests = ""
-	} else {
-		fmt.Printf("Successfully ran missing test detector:\n%s\n", missingTests)
-	}
-	return missingTests, rnr.PopDir()
-}
-
-// Update the provider package name to the given name in the given path.
-// name should be either "old" or "new".
-func updatePackageName(name, path string, rnr ExecRunner) error {
-	oldPackageName := "github.com/hashicorp/terraform-provider-google-beta"
-	newPackageName := "google/provider/" + name
-	fmt.Printf("Updating package name in %s from %s to %s\n", path, oldPackageName, newPackageName)
-	if err := rnr.PushDir(path); err != nil {
-		return err
-	}
-	if _, err := rnr.Run("find", []string{".", "-type", "f", "-name", "*.go", "-exec", "sed", "-i.bak", fmt.Sprintf("s~%s~%s~g", oldPackageName, newPackageName), "{}", "+"}, nil); err != nil {
-		return fmt.Errorf("error running find: %v\n", err)
-	}
-	if _, err := rnr.Run("sed", []string{"-i.bak", fmt.Sprintf("s|%s|%s|g", oldPackageName, newPackageName), "go.mod"}, nil); err != nil {
-		return fmt.Errorf("error running sed: %v\n", err)
-	}
-	if _, err := rnr.Run("sed", []string{"-i.bak", fmt.Sprintf("s|%s|%s|g", oldPackageName, newPackageName), "go.sum"}, nil); err != nil {
-		return fmt.Errorf("error running sed: %v\n", err)
+		return "", err
 	}
-	return rnr.PopDir()
-}
 
-// Run unit tests for the missing test detector.
-// Report results using Github API.
-func runMissingTestUnitTests(mmLocalPath, tpgbLocalPath, targetURL, commitSha string, prNumber int, gh GithubClient, rnr ExecRunner) error {
-	missingTestDetectorPath := filepath.Join(mmLocalPath, "tools", "missing-test-detector")
-	rnr.PushDir(missingTestDetectorPath)
-	if _, err := rnr.Run("go", []string{"mod", "tidy"}, nil); err != nil {
-		fmt.Printf("error running go mod tidy in %s: %v\n", missingTestDetectorPath, err)
-	}
-	servicesDir := filepath.Join(tpgbLocalPath, "google-beta", "services")
-	state := "success"
-	if _, err := rnr.Run("go", []string{"test"}, map[string]string{
-		"SERVICES_DIR": servicesDir,
-		// Passthrough vars required for a valid build environment.
-		"GOPATH": os.Getenv("GOPATH"),
-		"HOME":   os.Getenv("HOME"),
-	}); err != nil {
-		fmt.Printf("error from running go test in %s: %v\n", missingTestDetectorPath, err)
-		state = "failure"
-	}
-	if err := gh.PostBuildStatus(strconv.Itoa(prNumber), "unit-tests-missing-test-detector", state, targetURL, commitSha); err != nil {
-		return err
-	}
-	return rnr.PopDir()
+	return output, rnr.PopDir()
 }
 
 func formatDiffComment(data diffCommentData) (string, error) {

diff --git a/.ci/magician/cmd/generate_comment_test.go b/.ci/magician/cmd/generate_comment_test.go
@@ -20,8 +20,9 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"magician/source"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestExecGenerateComment(t *testing.T) {
@@ -55,7 +56,6 @@ func TestExecGenerateComment(t *testing.T) {
 			{"/mock/dir/tpg", "/mock/dir/magic-modules/tools/diff-processor/new"},
 			{"/mock/dir/tpgb", "/mock/dir/magic-modules/tools/diff-processor/old"},
 			{"/mock/dir/tpgb", "/mock/dir/magic-modules/tools/diff-processor/new"},
-			{"/mock/dir/tpgb", "/mock/dir/tpgbold"},
 		},
 		"RemoveAll": {
 			{"/mock/dir/magic-modules/tools/diff-processor/old"},
@@ -86,18 +86,8 @@ func TestExecGenerateComment(t *testing.T) {
 			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"changed-schema-resources"}, map[string]string(nil)},
 			{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, diffProcessorEnv},
 			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, map[string]string(nil)},
+			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"detect-missing-tests", "/mock/dir/tpgb/google-beta/services"}, map[string]string(nil)},
 			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"changed-schema-resources"}, map[string]string(nil)},
-			{"/mock/dir/tpgbold", "git", []string{"checkout", "origin/auto-pr-123456-old"}, map[string]string(nil)},
-			{"/mock/dir/tpgbold", "find", []string{".", "-type", "f", "-name", "*.go", "-exec", "sed", "-i.bak", "s~github.com/hashicorp/terraform-provider-google-beta~google/provider/old~g", "{}", "+"}, map[string]string(nil)},
-			{"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.mod"}, map[string]string(nil)},
-			{"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.sum"}, map[string]string(nil)},
-			{"/mock/dir/tpgb", "find", []string{".", "-type", "f", "-name", "*.go", "-exec", "sed", "-i.bak", "s~github.com/hashicorp/terraform-provider-google-beta~google/provider/new~g", "{}", "+"}, map[string]string(nil)},
-			{"/mock/dir/tpgb", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g", "go.mod"}, map[string]string(nil)},
-			{"/mock/dir/tpgb", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g", "go.sum"}, map[string]string(nil)},
-			{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/new=/mock/dir/tpgb"}, map[string]string(nil)},
-			{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/old=/mock/dir/tpgbold"}, map[string]string(nil)},
-			{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "tidy"}, map[string]string(nil)},
-			{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"run", ".", "-services-dir=/mock/dir/tpgb/google-beta/services"}, map[string]string(nil)},
 		},
 	} {
 		if actualCalls, ok := mr.Calls(method); !ok {

diff --git a/.ci/magician/cmd/mock_runner_test.go b/.ci/magician/cmd/mock_runner_test.go
@@ -67,14 +67,10 @@ func NewMockRunner() MockRunner {
 			"/mock/dir/magic-modules/.ci/magician git [clone -b auto-pr-123456 https://modular-magician:*******@github.com/modular-magician/terraform-google-conversion /mock/dir/tgc] map[]":     "",
 			"/mock/dir/magic-modules/.ci/magician git [clone -b auto-pr-123456 https://modular-magician:*******@github.com/modular-magician/terraform-provider-google /mock/dir/tpg] map[]":       "",
 			"/mock/dir/magic-modules/.ci/magician git [clone -b auto-pr-123456 https://modular-magician:*******@github.com/modular-magician/terraform-provider-google-beta /mock/dir/tpgb] map[]": "",
-			"/mock/dir/magic-modules git [diff HEAD origin/main tools/missing-test-detector] map[]":                                                                                               "",
 			"/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/missing-test-detector go [mod edit -replace google/provider/new=/mock/dir/tpgb] map[]":                                                                 "",
-			"/mock/dir/magic-modules/tools/missing-test-detector go [mod edit -replace google/provider/old=/mock/dir/tpgbold] map[]":                                                              "",
-			"/mock/dir/magic-modules/tools/missing-test-detector go [mod tidy] map[]":                                                                                                             "",
-			"/mock/dir/magic-modules/tools/missing-test-detector go [run . -services-dir=/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[]":                                                    "## 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/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[]":                                                                                       "",

diff --git a/.ci/magician/github/membership.go b/.ci/magician/github/membership.go
@@ -61,6 +61,11 @@ var (
       startDate: newDate(2024, 4, 5, pdtLoc),
       endDate: newDate(2024, 4, 10, pdtLoc),
     },
+	{
+	  id: "hao-nan-li",
+	  startDate: newDate(2024, 4, 15, pdtLoc),
+	  endDate: newDate(2024, 6, 14, pdtLoc),
+	},
 	}
 )
 

diff --git a/.github/workflows/mmv1-check-templates.yml b/.github/workflows/mmv1-check-templates.yml
@@ -27,4 +27,4 @@ jobs:
       - name: Check for invalid version guards
         run: |
           cd repo/tools/template-check
-          git diff --name-only origin/${{ github.base_ref }} ../../*.erb | sed 's=^=../../=g' | go run main.go
+          git diff --name-only --diff-filter=d origin/${{ github.base_ref }} ../../*.erb | sed 's=^=../../=g' | go run main.go
diff --git a/.github/workflows/unit-tests-diff-processor.yml b/.github/workflows/unit-tests-diff-processor.yml
@@ -29,6 +29,8 @@ jobs:
         run: |
           cd tools/diff-processor
           go test -v ./...
+        env:
+          SERVICES_DIR: tools/diff-processor/new/google/services
 
       - name: Build with TPGB
         run: |
@@ -40,3 +42,5 @@ jobs:
         run: |
           cd tools/diff-processor
           go test -v ./...
+        env:
+          SERVICES_DIR: tools/diff-processor/new/google/services
diff --git a/mmv1/api/resource.go b/mmv1/api/resource.go
@@ -780,6 +780,15 @@ func (r Resource) ClientNamePascal() string {
 	return google.Camelize(clientName, "upper")
 }
 
+func (r Resource) PackageName() string {
+	clientName := r.ProductMetadata.ClientName
+	if clientName == "" {
+		clientName = r.ProductMetadata.Name
+	}
+
+	return strings.ToLower(clientName)
+}
+
 // In order of preference, use TF override,
 // general defined timeouts, or default Timeouts
 
@@ -901,3 +910,20 @@ func ImportIdFormats(importFormat, identity []string, baseUrl string) []string {
 	// TODO Q2:  id_formats.uniq.reject(&:empty?).sort_by { |i| [i.count('/'), i.count('{{')] }.reverse
 	return idFormats
 }
+
+func (r Resource) IgnoreReadPropertiesToString(e resource.Examples) string {
+	var props []string
+	for _, tp := range r.AllUserProperties() {
+		if tp.UrlParamOnly || tp.IgnoreRead || tp.IsA("ResourceRef") {
+			props = append(props, fmt.Sprintf("\"%s\"", google.Underscore(tp.Name)))
+		}
+	}
+	for _, tp := range e.IgnoreReadExtra {
+		props = append(props, fmt.Sprintf("\"%s\"", google.Underscore(tp)))
+	}
+	for _, tp := range r.IgnoreReadLabelsFields(r.PropertiesWithExcluded()) {
+		props = append(props, fmt.Sprintf("\"%s\"", google.Underscore(tp)))
+	}
+
+	return fmt.Sprintf("[]string{%s}", strings.Join(props, ", "))
+}
diff --git a/mmv1/api/resource/examples.go b/mmv1/api/resource/examples.go
@@ -20,6 +20,7 @@ import (
 	"path/filepath"
 	"text/template"
 
+	"github.com/GoogleCloudPlatform/magic-modules/mmv1/google"
 	"github.com/golang/glog"
 	"gopkg.in/yaml.v3"
 )
@@ -323,6 +324,18 @@ func (e *Examples) OiCSLink() string {
 	return u.String()
 }
 
+func (e *Examples) TestSlug(productName, resourceName string) string {
+	ret := fmt.Sprintf("%s%s_%sExample", productName, resourceName, google.Camelize(e.Name, "upper"))
+	return ret
+}
+
+func (e *Examples) ResourceType(terraformName string) string {
+	if e.PrimaryResourceType != "" {
+		return e.PrimaryResourceType
+	}
+	return terraformName
+}
+
 // rubocop:disable Layout/LineLength
 // func (e *Examples) substitute_test_paths(config) {
 // config.gsub!('../static/img/header-logo.png', 'test-fixtures/header-logo.png')