Update TeamCity diff checker tool (#12426)

.github/workflows/teamcity-services-diff-check-weekly.yml

@@ -37,11 +37,13 @@ jobs:
           run: echo "GOOGLE_BETA_REPO_PATH=${{ env.OUTPUT_PATH}}" >> $GITHUB_ENV
         - name: Check that new services have been added to the TeamCity configuration code
           run: |
-              # Create lists of service packages in providers
-              ls ${{env.GOOGLE_REPO_PATH}}/google/services > tools/teamcity-diff-check/services_ga.txt
-              ls ${{env.GOOGLE_BETA_REPO_PATH}}/google-beta/services > tools/teamcity-diff-check/services_beta.txt
-    
+              # Create lists of service packages in providers. Need to cd into repos where go.mod is to do this command.
+              cd ${{env.GOOGLE_REPO_PATH}}
+              go list -f '{{.Name}}' ${{env.GOOGLE_REPO_PATH}}/google/services/... > $GITHUB_WORKSPACE/provider_services_ga.txt
+              cd ${{env.GOOGLE_BETA_REPO_PATH}}
+              go list -f '{{.Name}}' ${{env.GOOGLE_BETA_REPO_PATH}}/google-beta/services/... > $GITHUB_WORKSPACE/provider_services_beta.txt
+
               # Run tool to compare service packages in the providers vs those listed in TeamCity config files
-              cd tools/teamcity-diff-check
-              go run main.go -service_file=services_ga
-              go run main.go -service_file=services_beta
+              cd $GITHUB_WORKSPACE
+              go run ./tools/teamcity-diff-check/main.go -version=ga -provider_services=./provider_services_ga.txt -teamcity_services=./mmv1/third_party/terraform/.teamcity/components/inputs/services_ga.kt
+              go run ./tools/teamcity-diff-check/main.go -version=beta -provider_services=./provider_services_beta.txt -teamcity_services=./mmv1/third_party/terraform/.teamcity/components/inputs/services_beta.kt

.github/workflows/teamcity-services-diff-check.yml

@@ -52,11 +52,13 @@ jobs:
       - name: Check that new services have been added to the TeamCity configuration code
         if: steps.generate.outcome == 'success'
         run: |
-          # Create lists of service packages in providers
-          ls ${{env.GOOGLE_REPO_PATH}}/google/services > tools/teamcity-diff-check/services_ga.txt
-          ls ${{env.GOOGLE_BETA_REPO_PATH}}/google-beta/services > tools/teamcity-diff-check/services_beta.txt
+          # Create lists of service packages in providers. Need to cd into repos where go.mod is to do this command.
+          cd ${{env.GOOGLE_REPO_PATH}}
+          go list -f '{{.Name}}' ${{env.GOOGLE_REPO_PATH}}/google/services/... > $GITHUB_WORKSPACE/provider_services_ga.txt
+          cd ${{env.GOOGLE_BETA_REPO_PATH}}
+          go list -f '{{.Name}}' ${{env.GOOGLE_BETA_REPO_PATH}}/google-beta/services/... > $GITHUB_WORKSPACE/provider_services_beta.txt
 
           # Run tool to compare service packages in the providers vs those listed in TeamCity config files
-          cd tools/teamcity-diff-check
-          go run main.go -service_file=services_ga
-          go run main.go -service_file=services_beta
+          cd $GITHUB_WORKSPACE
+          go run ./tools/teamcity-diff-check/main.go -version=ga -provider_services=./provider_services_ga.txt -teamcity_services=./mmv1/third_party/terraform/.teamcity/components/inputs/services_ga.kt
+          go run ./tools/teamcity-diff-check/main.go -version=beta -provider_services=./provider_services_beta.txt -teamcity_services=./mmv1/third_party/terraform/.teamcity/components/inputs/services_beta.kt

tools/teamcity-diff-check/go.mod

@@ -0,0 +1,3 @@
+module github.com/GoogleCloudPlatform/magic-modules/tools/teamcity-diff-check
+
+go 1.23.2

tools/teamcity-diff-check/main.go

@@ -9,31 +9,58 @@ import (
 	"regexp"
 )
 
-var serviceFile = flag.String("service_file", "services_ga", "kotlin service file to be parsed")
+var version = flag.String("version", "", "the provider version under test. Must be `ga` or `beta`")
+var teamcityServiceFile = flag.String("teamcity_services", "", "path to a kotlin service file to be parsed")
+var providerServiceFile = flag.String("provider_services", "", "path to a .txt file listing all service packages in the provider")
 
-func serviceDifference(gS, tS []string) []string {
-	t := make(map[string]struct{}, len(tS))
-	for _, s := range tS {
-		t[s] = struct{}{}
-	}
+// listDifference checks that all the items in list B are present in list A
+func listDifference(listA, listB []string) error {
 
+	a := make(map[string]struct{}, len(listA))
+	for _, s := range listA {
+		a[s] = struct{}{}
+	}
 	var diff []string
-	for _, s := range gS {
-		if _, found := t[s]; !found {
+	for _, s := range listB {
+		if _, found := a[s]; !found {
 			diff = append(diff, s)
 		}
 	}
 
-	return diff
+	if len(diff) > 0 {
+		return fmt.Errorf("%v", diff)
+	}
+
+	return nil
 }
 
 func main() {
 	flag.Parse()
 
-	file, err := os.Open(*serviceFile + ".txt")
+	ga := *version == "ga"
+	beta := *version == "beta"
+	if !ga && !beta {
+		fmt.Fprint(os.Stderr, "the flag `version` must be set to either `ga` or `beta`, and is case sensitive\n")
+		os.Exit(1)
+	}
+
+	err := compareServices(*teamcityServiceFile, *providerServiceFile)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Errors when inspecting the %s version of the Google provider\n", *version)
+		fmt.Fprintf(os.Stderr, "%s\n", err.Error())
+		os.Exit(1)
+	}
+
+	fmt.Fprintf(os.Stdout, "All services present in the %s provider codebase are present in TeamCity config, and vice versa\n", *version)
+}
+
+// compareServices contains most of the logic of the main function, but is separated to make the code more testable
+func compareServices(teamcityServiceFile, providerServiceFile string) error {
+
+	// Get array of services from the provider service list file
+	file, err := os.Open(providerServiceFile)
 	if err != nil {
-		fmt.Println(err)
-		return
+		return fmt.Errorf("error opening provider service list file: %w", err)
 	}
 	defer file.Close()
 
@@ -42,52 +69,62 @@ func main() {
 	for scanner.Scan() {
 		googleServices = append(googleServices, scanner.Text())
 	}
+	if len(googleServices) == 0 {
+		return fmt.Errorf("could not find any services in the provider service list file %s", providerServiceFile)
+	}
 
-	////////////////////////////////////////////////////////////////////////////////
-
-	filePath := fmt.Sprintf("mmv1/third_party/terraform/.teamcity/components/inputs/%s.kt", *serviceFile)
-	f, err := os.Open(fmt.Sprintf("../../%s", filePath)) // Need to make path relative to where the script is called
+	// Get array of services from the TeamCity service list file
+	f, err := os.Open(teamcityServiceFile)
 	if err != nil {
-		panic(err)
+		return fmt.Errorf("error opening TeamCity service list file: %w", err)
 	}
 
-	// Get the file size
 	stat, err := f.Stat()
 	if err != nil {
-		fmt.Println(err)
-		return
+		return fmt.Errorf("error stating TeamCity service list file: %w", err)
 	}
 
-	// Read the file into a byte slice
 	bs := make([]byte, stat.Size())
 	_, err = bufio.NewReader(f).Read(bs)
 	if err != nil && err != io.EOF {
-		fmt.Println(err)
-		return
+		return fmt.Errorf("error processing TeamCity service list file: %w", err)
 	}
 
-	// Regex pattern captures "services" from *serviceFile.
+	// Regex pattern captures "services" from the Kotlin service list file.
 	pattern := regexp.MustCompile(`(?m)"(?P<service>\w+)"\sto\s+mapOf`)
 
 	template := []byte("$service")
 
 	dst := []byte{}
 	teamcityServices := []string{}
 
-	// For each match of the regex in the content.
 	for _, submatches := range pattern.FindAllSubmatchIndex(bs, -1) {
 		service := pattern.Expand(dst, template, bs, submatches)
 		teamcityServices = append(teamcityServices, string(service))
 	}
 	if len(teamcityServices) == 0 {
-		fmt.Fprintf(os.Stderr, "error: script could not find any services listed in the file %s.kt .\n", filePath)
-		os.Exit(1)
+		return fmt.Errorf("could not find any services in the TeamCity service list file %s", teamcityServiceFile)
 	}
 
-	if diff := serviceDifference(googleServices, teamcityServices); len(diff) != 0 {
-		fmt.Fprintf(os.Stderr, "error: missing services detected in %s\n", filePath)
-		fmt.Fprintf(os.Stderr, "Please update file to include these new services: %s\n", diff)
-		os.Exit(1)
+	// Determine diffs
+	errTeamCity := listDifference(teamcityServices, googleServices)
+	errProvider := listDifference(googleServices, teamcityServices)
+
+	switch {
+	case errTeamCity != nil && errProvider != nil:
+		return fmt.Errorf(`mismatches detected:
+TeamCity service file is missing services present in the provider: %s
+Provider codebase is missing services present in the TeamCity service file: %s`,
+			errTeamCity, errProvider)
+	case errTeamCity != nil:
+		return fmt.Errorf(`mismatches detected:
+TeamCity service file is missing services present in the provider: %s`,
+			errTeamCity)
+	case errProvider != nil:
+		return fmt.Errorf(`mismatches detected:
+Provider codebase is missing services present in the TeamCity service file: %s`,
+			errProvider)
 	}
 
+	return nil
 }

tools/teamcity-diff-check/main_test.go

@@ -0,0 +1,159 @@
+package main
+
+import (
+	"regexp"
+	"testing"
+)
+
+func Test_main_happyPaths(t *testing.T) {
+	testCases := map[string]struct {
+		providerServiceFile string
+		teamcityServiceFile string
+		expectError         bool
+		errorRegex          *regexp.Regexp
+		missingServiceRegex *regexp.Regexp
+	}{
+		"everything matches": {
+			providerServiceFile: "./test-fixtures/everything-ok/ga-services.txt",
+			teamcityServiceFile: "./test-fixtures/everything-ok/services_ga.kt",
+		},
+		"something missing in TeamCity config present in provider code": {
+			providerServiceFile: "./test-fixtures/mismatch-teamcity/ga-services.txt",
+			teamcityServiceFile: "./test-fixtures/mismatch-teamcity/services_ga.kt",
+			expectError:         true,
+			errorRegex:          regexp.MustCompile("TeamCity service file is missing services present in the provider"),
+			missingServiceRegex: regexp.MustCompile("[pubsub]"),
+		},
+		"something missing in provider code present in TeamCity config": {
+			providerServiceFile: "./test-fixtures/mismatch-provider/ga-services.txt",
+			teamcityServiceFile: "./test-fixtures/mismatch-provider/services_ga.kt",
+			expectError:         true,
+			errorRegex:          regexp.MustCompile("Provider codebase is missing services present in the TeamCity service file"),
+			missingServiceRegex: regexp.MustCompile("[compute]"),
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			err := compareServices(tc.teamcityServiceFile, tc.providerServiceFile)
+			if err != nil && !tc.expectError {
+				t.Fatalf("saw an unexpected error: %s", err)
+			}
+			if err == nil && tc.expectError {
+				t.Fatalf("expected an error but saw none")
+			}
+
+			if err == nil {
+				// Stop handling of non-error test cases
+				return
+			}
+
+			if !tc.errorRegex.MatchString(err.Error()) {
+				t.Fatalf("expected error to contain a match for regex `%s`, got error string: `%s`", tc.errorRegex.String(), err)
+			}
+			if !tc.missingServiceRegex.MatchString(err.Error()) {
+				t.Fatalf("expected error to contain a match for regex `%s`, got error string: `%s`", tc.errorRegex.String(), err)
+			}
+		})
+	}
+}
+
+func Test_main_unhappyPaths(t *testing.T) {
+	testCases := map[string]struct {
+		providerServiceFile string
+		teamcityServiceFile string
+		expectError         bool
+		errorRegex          *regexp.Regexp
+	}{
+		"cannot find provider service file": {
+			providerServiceFile: "./test-fixtures/doesnt-exist.txt",
+			teamcityServiceFile: "./test-fixtures/everything-ok/services_ga.kt",
+			expectError:         true,
+			errorRegex:          regexp.MustCompile("error opening provider service list file: open ./test-fixtures/doesnt-exist.txt"),
+		},
+		"cannot find TeamCity service file": {
+			providerServiceFile: "./test-fixtures/everything-ok/ga-services.txt",
+			teamcityServiceFile: "./test-fixtures/everything-ok/doesnt-exist.kt",
+			expectError:         true,
+			errorRegex:          regexp.MustCompile("error opening TeamCity service list file: open ./test-fixtures/everything-ok/doesnt-exist.kt"),
+		},
+		"empty TeamCity service file": {
+			providerServiceFile: "./test-fixtures/everything-ok/ga-services.txt",
+			teamcityServiceFile: "./test-fixtures/empty-files/services_ga.kt",
+			expectError:         true,
+			errorRegex:          regexp.MustCompile("could not find any services in the TeamCity service list file ./test-fixtures/empty-files/services_ga.kt"),
+		},
+		"empty provider service file": {
+			providerServiceFile: "./test-fixtures/empty-files/ga-services.txt",
+			teamcityServiceFile: "./test-fixtures/everything-ok/services_ga.kt",
+			expectError:         true,
+			errorRegex:          regexp.MustCompile("could not find any services in the provider service list file ./test-fixtures/empty-files/ga-services.txt"),
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			err := compareServices(tc.teamcityServiceFile, tc.providerServiceFile)
+			if err != nil && !tc.expectError {
+				t.Fatalf("saw an unexpected error: %s", err)
+			}
+			if err == nil && tc.expectError {
+				t.Fatalf("expected an error but saw none")
+			}
+
+			if !tc.errorRegex.MatchString(err.Error()) {
+				t.Fatalf("expected error to contain a match for regex `%s`, got error string: `%s`", tc.errorRegex.String(), err)
+			}
+		})
+	}
+}
+
+func Test_listDifference(t *testing.T) {
+	testCases := map[string]struct {
+		a          []string
+		b          []string
+		expectDiff bool
+		errorRegex *regexp.Regexp
+	}{
+		"detects when lists match": {
+			a: []string{"a", "c", "b"},
+			b: []string{"a", "b", "c"},
+		},
+		"detects when items from list A is missing items present in list B - 1 missing": {
+			a:          []string{"a", "b"},
+			b:          []string{"a", "c", "b"},
+			expectDiff: true,
+			errorRegex: regexp.MustCompile("[c]"),
+		},
+		"detects when items from list A is missing items present in list B - 2 missing": {
+			a:          []string{"b"},
+			b:          []string{"a", "c", "b"},
+			expectDiff: true,
+			errorRegex: regexp.MustCompile("[a, c]"),
+		},
+		"doesn't detect differences if list A is a superset of list B": {
+			a:          []string{"a", "b", "c"},
+			b:          []string{"a", "c"},
+			expectDiff: false,
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			err := listDifference(tc.a, tc.b)
+			if !tc.expectDiff && (err != nil) {
+				t.Fatalf("saw an unexpected diff error: %s", err)
+			}
+			if tc.expectDiff && (err == nil) {
+				t.Fatalf("expected a diff error but saw none")
+			}
+			if !tc.expectDiff && err == nil {
+				// Stop assertions in no error cases
+				return
+			}
+			if !tc.errorRegex.MatchString(err.Error()) {
+				t.Fatalf("expected diff error to contain a match for regex %s, error string: %s", tc.errorRegex.String(), err)
+			}
+		})
+	}
+}

tools/teamcity-diff-check/test-fixtures/everything-ok/ga-services.txt

@@ -0,0 +1,2 @@
+compute
+pubsub

tools/teamcity-diff-check/test-fixtures/everything-ok/services_ga.kt

@@ -0,0 +1,12 @@
+var ServicesListGa = mapOf(
+    "compute" to mapOf(
+        "name" to "compute",
+        "displayName" to "Compute",
+        "path" to "./google/services/compute"
+    ),
+    "pubsub" to mapOf(
+        "name" to "pubsub",
+        "displayName" to "PubSub",
+        "path" to "./google/services/pubsub"
+    ),
+)

tools/teamcity-diff-check/test-fixtures/mismatch-provider/ga-services.txt

@@ -0,0 +1 @@
+pubsub

tools/teamcity-diff-check/test-fixtures/mismatch-provider/services_ga.kt

@@ -0,0 +1,12 @@
+var ServicesListGa = mapOf(
+    "compute" to mapOf(
+        "name" to "compute",
+        "displayName" to "Compute",
+        "path" to "./google/services/compute"
+    ),
+    "pubsub" to mapOf(
+        "name" to "pubsub",
+        "displayName" to "PubSub",
+        "path" to "./google/services/pubsub"
+    ),
+)

tools/teamcity-diff-check/test-fixtures/mismatch-teamcity/ga-services.txt

@@ -0,0 +1,2 @@
+compute
+pubsub