From e70889324d184bbaec6ae4e450133a963ba9bae5 Mon Sep 17 00:00:00 2001
From: Thomas Rodgers <thomasrodgers@google.com>
Date: Fri, 19 Apr 2024 15:12:52 -0700
Subject: [PATCH] Run acceptance and unit tests when test fixtures change
 (#10483)

---
 .ci/magician/cmd/test_terraform_vcr.go      | 14 +++--
 .ci/magician/cmd/test_terraform_vcr_test.go | 66 +++++++++++++++++++++
 .github/workflows/test-tpg.yml              |  2 +-
 3 files changed, 76 insertions(+), 6 deletions(-)
 create mode 100644 .ci/magician/cmd/test_terraform_vcr_test.go

diff --git a/.ci/magician/cmd/test_terraform_vcr.go b/.ci/magician/cmd/test_terraform_vcr.go
index 60fda5577d82..c3e0b54037be 100644
--- a/.ci/magician/cmd/test_terraform_vcr.go
+++ b/.ci/magician/cmd/test_terraform_vcr.go
@@ -117,10 +117,10 @@ func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep,
 	}
 
 	if len(services) == 0 && !runFullVCR {
-		fmt.Println("Skipping tests: No go files changed")
+		fmt.Println("Skipping tests: No go files or test fixtures changed")
 		os.Exit(0)
 	}
-	fmt.Println("Running tests: Go files changed")
+	fmt.Println("Running tests: Go files or test fixtures changed")
 
 	if err := vt.FetchCassettes(provider.Beta, baseBranch, prNumber); err != nil {
 		fmt.Println("Error fetching cassettes: ", err)
@@ -281,7 +281,7 @@ Please fix these to complete your PR. If you believe these test failures to be i
 }
 
 func modifiedPackages(newBranch, oldBranch string, rnr ExecRunner) (map[string]struct{}, bool, error) {
-	fmt.Println("Checking for modified go files")
+	fmt.Println("Checking for modified go files or test fixtures")
 
 	if _, err := rnr.Run("git", []string{"fetch", "origin", fmt.Sprintf("%s:%s", oldBranch, oldBranch), "--depth", "1"}, nil); err != nil {
 
@@ -290,9 +290,13 @@ func modifiedPackages(newBranch, oldBranch string, rnr ExecRunner) (map[string]s
 	if err != nil {
 		return nil, false, err
 	}
+	return modifiedPackagesFromDiffs(strings.Split(diffs, "\n"))
+}
+
+func modifiedPackagesFromDiffs(diffs []string) (map[string]struct{}, bool, error) {
 	var goFiles []string
-	for _, line := range strings.Split(diffs, "\n") {
-		if strings.HasSuffix(line, ".go") || line == "go.mod" || line == "go.sum" {
+	for _, line := range diffs {
+		if strings.HasSuffix(line, ".go") || strings.Contains(line, "test-fixtures") || strings.HasSuffix(line, "go.mod") || strings.HasSuffix(line, "go.sum") {
 			goFiles = append(goFiles, line)
 		}
 	}
diff --git a/.ci/magician/cmd/test_terraform_vcr_test.go b/.ci/magician/cmd/test_terraform_vcr_test.go
new file mode 100644
index 000000000000..9945c3f07e2c
--- /dev/null
+++ b/.ci/magician/cmd/test_terraform_vcr_test.go
@@ -0,0 +1,66 @@
+package cmd
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestModifiedPackagesFromDiffs(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		diffs    []string
+		packages map[string]struct{}
+		all      bool
+	}{
+		{
+			name:     "one-package",
+			diffs:    []string{"google-beta/services/servicename/resource.go"},
+			packages: map[string]struct{}{"servicename": struct{}{}},
+			all:      false,
+		},
+		{
+			name: "multiple-packages",
+			diffs: []string{
+				"google-beta/services/serviceone/resource.go",
+				"google-beta/services/servicetwo/test-fixtures/fixture.txt",
+				"google-beta/services/servicethree/resource_test.go",
+			},
+			packages: map[string]struct{}{
+				"serviceone":   struct{}{},
+				"servicetwo":   struct{}{},
+				"servicethree": struct{}{},
+			},
+			all: false,
+		},
+		{
+			name:     "all-packages",
+			diffs:    []string{"google-beta/provider/provider.go"},
+			packages: map[string]struct{}{},
+			all:      true,
+		},
+		{
+			name:     "all-packages-go-mod",
+			diffs:    []string{"scripts/go.mod"},
+			packages: map[string]struct{}{},
+			all:      true,
+		},
+		{
+			name:     "all-packages-go-sum",
+			diffs:    []string{"go.sum"},
+			packages: map[string]struct{}{},
+			all:      true,
+		},
+		{
+			name:     "no-packages",
+			diffs:    []string{"website/docs/d/notebooks_runtime_iam_policy.html.markdown"},
+			packages: map[string]struct{}{},
+			all:      false,
+		},
+	} {
+		if packages, all, _ := modifiedPackagesFromDiffs(tc.diffs); !reflect.DeepEqual(packages, tc.packages) {
+			t.Errorf("Unexpected packages found for test %s: %v, expected %v", tc.name, packages, tc.packages)
+		} else if all != tc.all {
+			t.Errorf("Unexpected value for all packages for test %s: %v, expected %v", tc.name, all, tc.all)
+		}
+	}
+}
diff --git a/.github/workflows/test-tpg.yml b/.github/workflows/test-tpg.yml
index 9c8900c019f8..e5b534729e7d 100644
--- a/.github/workflows/test-tpg.yml
+++ b/.github/workflows/test-tpg.yml
@@ -60,7 +60,7 @@ jobs:
     - name: Check for Code Changes
       id: pull_request
       run: |
-        gofiles=$(git diff --name-only HEAD~1 | { grep -e "\.go$" -e "go.mod$" -e "go.sum$" || test $? = 1; })
+        gofiles=$(git diff --name-only HEAD~1 | { grep -e "\.go$" -e "test-fixtures" -e "go.mod$" -e "go.sum$" || test $? = 1; })
         if [ -z "$gofiles" ]; then
           echo "has_changes=false" >> $GITHUB_OUTPUT
         else