Improve tests for SDK-based util functions, add tests to the new plugin framework equivalents

mmv1/third_party/terraform/framework_utils/framework_utils_test.go

@@ -0,0 +1,52 @@
+package google
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-framework/diag"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+)
+
+func TestGetProjectFramework(t *testing.T) {
+	cases := map[string]struct {
+		ResourceProject types.String
+		ProviderProject types.String
+		ExpectedProject types.String
+		ExpectedError   bool
+	}{
+		"project is pulled from the resource config value instead of the provider config value, even if both set": {
+			ResourceProject: types.StringValue("foo"),
+			ProviderProject: types.StringValue("bar"),
+			ExpectedProject: types.StringValue("foo"),
+		},
+		"project is pulled from the provider config value when unset on the resource": {
+			ResourceProject: types.StringNull(),
+			ProviderProject: types.StringValue("bar"),
+			ExpectedProject: types.StringValue("bar"),
+		},
+		"error when project is not set on the provider or the resource": {
+			ExpectedError: true,
+		},
+	}
+	for tn, tc := range cases {
+		t.Run(tn, func(t *testing.T) {
+			// Arrange
+			var diags diag.Diagnostics
+
+			// Act
+			project := getProjectFramework(tc.ResourceProject, tc.ProviderProject, &diags)
+
+			// Assert
+			if diags.HasError() {
+				if tc.ExpectedError {
+					return
+				}
+				t.Fatalf("Got %d unexpected error(s) during test: %s", diags.ErrorsCount(), diags.Errors())
+			}
+
+			if project != tc.ExpectedProject {
+				t.Fatalf("Incorrect project: got %s, want %s", project, tc.ExpectedProject)
+			}
+		})
+	}
+}

mmv1/third_party/terraform/utils/provider_test.go.erb

@@ -166,14 +166,6 @@ func testAccPreCheck(t *testing.T) {
 	}
 }
 
-func TestProvider_getRegionFromZone(t *testing.T) {
-	expected := "us-central1"
-	actual := getRegionFromZone("us-central1-f")
-	if expected != actual {
-		t.Fatalf("Region (%s) did not match expected value: %s", actual, expected)
-	}
-}
-
 func TestProvider_loadCredentialsFromFile(t *testing.T) {
 	ws, es := validateCredentials(testFakeCredentialsPath, "")
 	if len(ws) != 0 {

mmv1/third_party/terraform/utils/transport_test.go

@@ -199,30 +199,32 @@ func TestReplaceVars(t *testing.T) {
 	}
 
 	for tn, tc := range cases {
-		d := &ResourceDataMock{
-			FieldsInSchema: tc.SchemaValues,
-		}
+		t.Run(tn, func(t *testing.T) {
+			d := &ResourceDataMock{
+				FieldsInSchema: tc.SchemaValues,
+			}
 
-		config := tc.Config
-		if config == nil {
-			config = &Config{}
-		}
+			config := tc.Config
+			if config == nil {
+				config = &Config{}
+			}
 
-		v, err := replaceVars(d, config, tc.Template)
+			v, err := replaceVars(d, config, tc.Template)
 
-		if err != nil {
-			if !tc.ExpectedError {
-				t.Errorf("bad: %s; unexpected error %s", tn, err)
+			if err != nil {
+				if !tc.ExpectedError {
+					t.Errorf("bad: %s; unexpected error %s", tn, err)
+				}
+				return
 			}
-			continue
-		}
 
-		if tc.ExpectedError {
-			t.Errorf("bad: %s; expected error", tn)
-		}
+			if tc.ExpectedError {
+				t.Errorf("bad: %s; expected error", tn)
+			}
 
-		if v != tc.Expected {
-			t.Errorf("bad: %s; expected %q, got %q", tn, tc.Expected, v)
-		}
+			if v != tc.Expected {
+				t.Errorf("bad: %s; expected %q, got %q", tn, tc.Expected, v)
+			}
+		})
 	}
 }

mmv1/third_party/terraform/utils/utils.go

@@ -49,6 +49,9 @@ type TerraformResourceDiff interface {
 }
 
 // getRegionFromZone returns the region from a zone for Google cloud.
+// This is by removing the last two chars from the zone name to leave the region
+// If there aren't enough characters in the input string, an empty string is returned
+// e.g. southamerica-west1-a => southamerica-west1
 func getRegionFromZone(zone string) string {
 	if zone != "" && len(zone) > 2 {
 		region := zone[:len(zone)-2]

mmv1/third_party/terraform/utils/utils_test.go

@@ -144,55 +144,201 @@ func TestRfc3339TimeDiffSuppress(t *testing.T) {
 	}
 }
 
-func TestGetZone(t *testing.T) {
-	d := schema.TestResourceDataRaw(t, ResourceComputeDisk().Schema, map[string]interface{}{
-		"zone": "foo",
-	})
-	var config Config
-	if err := d.Set("zone", "foo"); err != nil {
-		t.Fatalf("Cannot set zone: %s", err)
-	}
-	if zone, err := getZone(d, &config); err != nil || zone != "foo" {
-		t.Fatalf("Zone '%s' != 'foo', %s", zone, err)
-	}
-	config.Zone = "bar"
-	if zone, err := getZone(d, &config); err != nil || zone != "foo" {
-		t.Fatalf("Zone '%s' != 'foo', %s", zone, err)
+func TestGetProject(t *testing.T) {
+	cases := map[string]struct {
+		ResourceProject string
+		ProviderProject string
+		ExpectedProject string
+		ExpectedError   bool
+	}{
+		"project is pulled from resource config instead of provider config": {
+			ResourceProject: "foo",
+			ProviderProject: "bar",
+			ExpectedProject: "foo",
+		},
+		"project is pulled from provider config when not set on resource": {
+			ProviderProject: "bar",
+			ExpectedProject: "bar",
+		},
+		"error returned when project not set on either provider or resource": {
+			ExpectedError: true,
+		},
 	}
-	if err := d.Set("zone", ""); err != nil {
-		t.Fatalf("Error setting zone: %s", err)
+	for tn, tc := range cases {
+		t.Run(tn, func(t *testing.T) {
+			// Arrange
+
+			// Create provider config
+			var config Config
+			if tc.ProviderProject != "" {
+				config.Project = tc.ProviderProject
+			}
+
+			// Create resource config
+			// Here use ResourceComputeDisk schema as example
+			emptyConfigMap := map[string]interface{}{}
+			d := schema.TestResourceDataRaw(t, ResourceComputeDisk().Schema, emptyConfigMap)
+			if tc.ResourceProject != "" {
+				if err := d.Set("project", tc.ResourceProject); err != nil {
+					t.Fatalf("Cannot set project: %s", err)
+				}
+			}
+
+			// Act
+			project, err := getProject(d, &config)
+
+			// Assert
+			if err != nil {
+				if tc.ExpectedError {
+					return
+				}
+				t.Fatalf("Unexpected error using test: %s", err)
+			}
+
+			if project != tc.ExpectedProject {
+				t.Fatalf("Incorrect project: got %s, want %s", project, tc.ExpectedProject)
+			}
+		})
 	}
-	if zone, err := getZone(d, &config); err != nil || zone != "bar" {
-		t.Fatalf("Zone '%s' != 'bar', %s", zone, err)
+}
+
+func TestGetZone(t *testing.T) {
+	cases := map[string]struct {
+		ResourceZone  string
+		ProviderZone  string
+		ExpectedZone  string
+		ExpectedError bool
+	}{
+		"zone is pulled from resource config instead of provider config": {
+			ResourceZone: "foo",
+			ProviderZone: "bar",
+			ExpectedZone: "foo",
+		},
+		"zone is pulled from provider config when not set on resource": {
+			ProviderZone: "bar",
+			ExpectedZone: "bar",
+		},
+		"zone value from resource is retrieved by splitting on slashes and selecting last element": {
+			// Unclear why this is the case - documenting this behaviour in a test case
+			ResourceZone: "this/is/foo/bar",
+			ExpectedZone: "bar",
+		},
+		"error returned when zone not set on either provider or resource": {
+			ExpectedError: true,
+		},
 	}
-	config.Zone = ""
-	if zone, err := getZone(d, &config); err == nil || zone != "" {
-		t.Fatalf("Zone '%s' != '', err=%s", zone, err)
+	for tn, tc := range cases {
+		t.Run(tn, func(t *testing.T) {
+			// Arrange
+
+			// Create provider config
+			var config Config
+			if tc.ProviderZone != "" {
+				config.Zone = tc.ProviderZone
+			}
+
+			// Create resource config
+			// Here use ResourceComputeDisk schema as example - because it has a zone field in schema
+			emptyConfigMap := map[string]interface{}{}
+			d := schema.TestResourceDataRaw(t, ResourceComputeDisk().Schema, emptyConfigMap)
+			if tc.ResourceZone != "" {
+				if err := d.Set("zone", tc.ResourceZone); err != nil {
+					t.Fatalf("Cannot set zone: %s", err)
+				}
+			}
+
+			// Act
+			zone, err := getZone(d, &config)
+
+			// Assert
+			if err != nil {
+				if tc.ExpectedError {
+					return
+				}
+				t.Fatalf("Unexpected error using test: %s", err)
+			}
+
+			if zone != tc.ExpectedZone {
+				t.Fatalf("Incorrect zone: got %s, want %s", zone, tc.ExpectedZone)
+			}
+		})
 	}
 }
 
 func TestGetRegion(t *testing.T) {
-	d := schema.TestResourceDataRaw(t, ResourceComputeDisk().Schema, map[string]interface{}{
-		"zone": "foo",
-	})
-	var config Config
-	barRegionName := getRegionFromZone("bar")
-	fooRegionName := getRegionFromZone("foo")
-
-	if region, err := getRegion(d, &config); err != nil || region != fooRegionName {
-		t.Fatalf("Zone '%s' != '%s', %s", region, fooRegionName, err)
+	cases := map[string]struct {
+		ResourceRegion string
+		ProviderRegion string
+		ProviderZone   string
+		ExpectedRegion string
+		ExpectedZone   string
+		ExpectedError  bool
+	}{
+		"region is pulled from resource config instead of provider config": {
+			ResourceRegion: "foo",
+			ProviderRegion: "bar",
+			ProviderZone:   "lol-a",
+			ExpectedRegion: "foo",
+		},
+		"region is pulled from region on provider config when region unset in resource config": {
+			ProviderRegion: "bar",
+			ProviderZone:   "lol-a",
+			ExpectedRegion: "bar",
+		},
+		"region is pulled from zone on provider config when region unset in both resource and provider config": {
+			ProviderZone:   "lol-a",
+			ExpectedRegion: "lol",
+		},
+		"error returned when region not set on resource and neither region or zone set on provider": {
+			ExpectedError: true,
+		},
 	}
+	for tn, tc := range cases {
+		t.Run(tn, func(t *testing.T) {
+			// Arrange
 
-	config.Zone = "bar"
-	if err := d.Set("zone", ""); err != nil {
-		t.Fatalf("Error setting zone: %s", err)
-	}
-	if region, err := getRegion(d, &config); err != nil || region != barRegionName {
-		t.Fatalf("Zone '%s' != '%s', %s", region, barRegionName, err)
+			// Create provider config
+			var config Config
+			if tc.ProviderRegion != "" {
+				config.Region = tc.ProviderRegion
+			}
+			if tc.ProviderZone != "" {
+				config.Zone = tc.ProviderZone
+			}
+
+			// Create resource config
+			// Here use ResourceComputeSubnetwork schema as example - because it has a region field in schema
+			emptyConfigMap := map[string]interface{}{}
+			d := schema.TestResourceDataRaw(t, ResourceComputeSubnetwork().Schema, emptyConfigMap)
+			if tc.ResourceRegion != "" {
+				if err := d.Set("region", tc.ResourceRegion); err != nil {
+					t.Fatalf("Cannot set region: %s", err)
+				}
+			}
+
+			// Act
+			region, err := getRegion(d, &config)
+
+			// Assert
+			if err != nil {
+				if tc.ExpectedError {
+					return
+				}
+				t.Fatalf("Unexpected error using test: %s", err)
+			}
+
+			if region != tc.ExpectedRegion {
+				t.Fatalf("Incorrect region: got %s, want %s", region, tc.ExpectedRegion)
+			}
+		})
 	}
-	config.Region = "something-else"
-	if region, err := getRegion(d, &config); err != nil || region != config.Region {
-		t.Fatalf("Zone '%s' != '%s', %s", region, config.Region, err)
+}
+
+func TestGetRegionFromZone(t *testing.T) {
+	expected := "us-central1"
+	actual := getRegionFromZone("us-central1-f")
+	if expected != actual {
+		t.Fatalf("Region (%s) did not match expected value: %s", actual, expected)
 	}
 }