Adding test coverage for changes to ignore data sources in imported state (#1077)

* Adding test coverage for changes in #1074 (#1066)

* Skips data sources from both original state and imported state

* Adding changelog entry (#1066)

Co-authored-by: Graham Davison <[email protected]>

Author: bendbennett

Diff for: .changelog/1077.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+helper/resource: Fixed `TestStep` type `ImportStateVerify` field so that it only matches against resources following a change in behaviour in Terraform 1.3 that imports both resources and their dependent data sources
+```

Diff for: helper/resource/testing_new_import_state.go

@@ -158,20 +158,27 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
 	if step.ImportStateVerify {
 		logging.HelperResourceTrace(ctx, "Using TestStep ImportStateVerify")
 
-		newResources := importState.RootModule().Resources
-		oldResources := state.RootModule().Resources
+		// Ensure that we do not match against data sources as they
+		// cannot be imported and are not what we want to verify.
+		// Mode is not present in ResourceState so we use the
+		// stringified ResourceStateKey for comparison.
+		newResources := make(map[string]*terraform.ResourceState)
+		for k, v := range importState.RootModule().Resources {
+			if !strings.HasPrefix(k, "data.") {
+				newResources[k] = v
+			}
+		}
+		oldResources := make(map[string]*terraform.ResourceState)
+		for k, v := range state.RootModule().Resources {
+			if !strings.HasPrefix(k, "data.") {
+				oldResources[k] = v
+			}
+		}
 
 		for _, r := range newResources {
 			// Find the existing resource
 			var oldR *terraform.ResourceState
-			for r2Key, r2 := range oldResources {
-				// Ensure that we do not match against data sources as they
-				// cannot be imported and are not what we want to verify.
-				// Mode is not present in ResourceState so we use the
-				// stringified ResourceStateKey for comparison.
-				if strings.HasPrefix(r2Key, "data.") {
-					continue
-				}
+			for _, r2 := range oldResources {
 
 				if r2.Primary != nil && r2.Primary.ID == r.Primary.ID && r2.Type == r.Type && r2.Provider == r.Provider {
 					oldR = r2

Diff for: helper/resource/teststep_providers_test.go

@@ -1076,6 +1076,7 @@ func TestTest_TestStep_Taint(t *testing.T) {
 	}
 }
 
+//nolint:unparam
 func extractResourceAttr(resourceName string, attributeName string, attributeValue *string) TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[resourceName]
@@ -1253,6 +1254,8 @@ func TestTest_TestStep_ProviderFactories_To_ExternalProviders(t *testing.T) {
 }
 
 func TestTest_TestStep_ProviderFactories_Import_Inline(t *testing.T) {
+	id := "none"
+
 	t.Parallel()
 
 	Test(t, TestCase{
@@ -1316,9 +1319,8 @@ func TestTest_TestStep_ProviderFactories_Import_Inline(t *testing.T) {
 				ImportStateId:      "Z=:cbrJE?Ltg",
 				ImportStatePersist: true,
 				ImportStateCheck: composeImportStateCheck(
-					testCheckResourceAttrInstanceState("id", "none"),
-					testCheckResourceAttrInstanceState("result", "Z=:cbrJE?Ltg"),
-					testCheckResourceAttrInstanceState("length", "12"),
+					testCheckResourceAttrInstanceState(&id, "result", "Z=:cbrJE?Ltg"),
+					testCheckResourceAttrInstanceState(&id, "length", "12"),
 				),
 			},
 		},
@@ -1391,7 +1393,7 @@ func TestTest_TestStep_ProviderFactories_Import_Inline_WithPersistMatch(t *testi
 				ImportStateId:      "Z=:cbrJE?Ltg",
 				ImportStatePersist: true,
 				ImportStateCheck: composeImportStateCheck(
-					testExtractResourceAttrInstanceState("result", &result1),
+					testExtractResourceAttrInstanceState("none", "result", &result1),
 				),
 			},
 			{
@@ -1484,6 +1486,8 @@ func TestTest_TestStep_ProviderFactories_Import_Inline_WithoutPersist(t *testing
 }
 
 func TestTest_TestStep_ProviderFactories_Import_External(t *testing.T) {
+	id := "none"
+
 	t.Parallel()
 
 	Test(t, TestCase{
@@ -1500,9 +1504,8 @@ func TestTest_TestStep_ProviderFactories_Import_External(t *testing.T) {
 				ImportStateId:      "Z=:cbrJE?Ltg",
 				ImportStatePersist: true,
 				ImportStateCheck: composeImportStateCheck(
-					testCheckResourceAttrInstanceState("id", "none"),
-					testCheckResourceAttrInstanceState("result", "Z=:cbrJE?Ltg"),
-					testCheckResourceAttrInstanceState("length", "12"),
+					testCheckResourceAttrInstanceState(&id, "result", "Z=:cbrJE?Ltg"),
+					testCheckResourceAttrInstanceState(&id, "length", "12"),
 				),
 			},
 		},
@@ -1528,7 +1531,7 @@ func TestTest_TestStep_ProviderFactories_Import_External_WithPersistMatch(t *tes
 				ImportStateId:      "Z=:cbrJE?Ltg",
 				ImportStatePersist: true,
 				ImportStateCheck: composeImportStateCheck(
-					testExtractResourceAttrInstanceState("result", &result1),
+					testExtractResourceAttrInstanceState("none", "result", &result1),
 				),
 			},
 			{
@@ -1561,7 +1564,7 @@ func TestTest_TestStep_ProviderFactories_Import_External_WithoutPersistNonMatch(
 				ImportStateId:      "Z=:cbrJE?Ltg",
 				ImportStatePersist: false,
 				ImportStateCheck: composeImportStateCheck(
-					testExtractResourceAttrInstanceState("result", &result1),
+					testExtractResourceAttrInstanceState("none", "result", &result1),
 				),
 			},
 			{
@@ -1714,6 +1717,188 @@ func TestTest_TestStep_ProviderFactories_RefreshWithPlanModifier_Inline(t *testi
 	})
 }
 
+func TestTest_TestStep_ProviderFactories_Import_Inline_With_Data_Source(t *testing.T) {
+	var id string
+
+	t.Parallel()
+
+	Test(t, TestCase{
+		ProviderFactories: map[string]func() (*schema.Provider, error){
+			"http": func() (*schema.Provider, error) { //nolint:unparam // required signature
+				return &schema.Provider{
+					DataSourcesMap: map[string]*schema.Resource{
+						"http": {
+							ReadContext: func(ctx context.Context, d *schema.ResourceData, i interface{}) (diags diag.Diagnostics) {
+								url := d.Get("url").(string)
+
+								responseHeaders := map[string]string{
+									"headerOne":   "one",
+									"headerTwo":   "two",
+									"headerThree": "three",
+									"headerFour":  "four",
+								}
+								if err := d.Set("response_headers", responseHeaders); err != nil {
+									return append(diags, diag.Errorf("Error setting HTTP response headers: %s", err)...)
+								}
+
+								d.SetId(url)
+
+								return diags
+							},
+							Schema: map[string]*schema.Schema{
+								"url": {
+									Type:     schema.TypeString,
+									Required: true,
+								},
+								"response_headers": {
+									Type:     schema.TypeMap,
+									Computed: true,
+									Elem: &schema.Schema{
+										Type: schema.TypeString,
+									},
+								},
+							},
+						},
+					},
+				}, nil
+			},
+			"random": func() (*schema.Provider, error) { //nolint:unparam // required signature
+				return &schema.Provider{
+					ResourcesMap: map[string]*schema.Resource{
+						"random_string": {
+							CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics {
+								d.SetId("none")
+								err := d.Set("length", 4)
+								if err != nil {
+									panic(err)
+								}
+								err = d.Set("result", "none")
+								if err != nil {
+									panic(err)
+								}
+								return nil
+							},
+							DeleteContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics {
+								return nil
+							},
+							ReadContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics {
+								return nil
+							},
+							Schema: map[string]*schema.Schema{
+								"length": {
+									Required: true,
+									ForceNew: true,
+									Type:     schema.TypeInt,
+								},
+								"result": {
+									Type:      schema.TypeString,
+									Computed:  true,
+									Sensitive: true,
+								},
+
+								"id": {
+									Computed: true,
+									Type:     schema.TypeString,
+								},
+							},
+							Importer: &schema.ResourceImporter{
+								StateContext: func(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
+									val := d.Id()
+
+									d.SetId(val)
+
+									err := d.Set("result", val)
+									if err != nil {
+										panic(err)
+									}
+
+									err = d.Set("length", len(val))
+									if err != nil {
+										panic(err)
+									}
+
+									return []*schema.ResourceData{d}, nil
+								},
+							},
+						},
+					},
+				}, nil
+			},
+		},
+		Steps: []TestStep{
+			{
+				Config: `data "http" "example" {
+							url = "https://checkpoint-api.hashicorp.com/v1/check/terraform"
+						}
+
+						resource "random_string" "example" {
+							length = length(data.http.example.response_headers)
+						}`,
+				Check: extractResourceAttr("random_string.example", "id", &id),
+			},
+			{
+				Config: `data "http" "example" {
+							url = "https://checkpoint-api.hashicorp.com/v1/check/terraform"
+						}
+
+						resource "random_string" "example" {
+							length = length(data.http.example.response_headers)
+						}`,
+				ResourceName: "random_string.example",
+				ImportState:  true,
+				ImportStateCheck: composeImportStateCheck(
+					testCheckResourceAttrInstanceState(&id, "length", "4"),
+				),
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestTest_TestStep_ProviderFactories_Import_External_With_Data_Source(t *testing.T) {
+	var id string
+
+	t.Parallel()
+
+	Test(t, TestCase{
+		ExternalProviders: map[string]ExternalProvider{
+			"http": {
+				Source: "registry.terraform.io/hashicorp/http",
+			},
+			"random": {
+				Source: "registry.terraform.io/hashicorp/random",
+			},
+		},
+		Steps: []TestStep{
+			{
+				Config: `data "http" "example" {
+							url = "https://checkpoint-api.hashicorp.com/v1/check/terraform"
+						}
+
+						resource "random_string" "example" {
+							length = length(data.http.example.response_headers)
+						}`,
+				Check: extractResourceAttr("random_string.example", "id", &id),
+			},
+			{
+				Config: `data "http" "example" {
+							url = "https://checkpoint-api.hashicorp.com/v1/check/terraform"
+						}
+
+						resource "random_string" "example" {
+							length = length(data.http.example.response_headers)
+						}`,
+				ResourceName: "random_string.example",
+				ImportState:  true,
+				ImportStateCheck: composeImportStateCheck(
+					testCheckResourceAttrInstanceState(&id, "length", "12"),
+				),
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func setTimeForTest(t time.Time) func() {
 	return func() {
 		getTimeForTest = func() time.Time {
@@ -1738,43 +1923,41 @@ func composeImportStateCheck(fs ...ImportStateCheckFunc) ImportStateCheckFunc {
 	}
 }
 
-func testExtractResourceAttrInstanceState(attributeName string, attributeValue *string) ImportStateCheckFunc {
+func testExtractResourceAttrInstanceState(id, attributeName string, attributeValue *string) ImportStateCheckFunc {
 	return func(is []*terraform.InstanceState) error {
-		if len(is) != 1 {
-			return fmt.Errorf("unexpected number of instance states: %d", len(is))
-		}
+		for _, v := range is {
+			if v.ID != id {
+				continue
+			}
 
-		s := is[0]
+			if attrVal, ok := v.Attributes[attributeName]; ok {
+				*attributeValue = attrVal
 
-		attrValue, ok := s.Attributes[attributeName]
-		if !ok {
-			return fmt.Errorf("attribute %s not found in instance state", attributeName)
+				return nil
+			}
 		}
 
-		*attributeValue = attrValue
-
-		return nil
+		return fmt.Errorf("attribute %s not found in instance state", attributeName)
 	}
 }
 
-func testCheckResourceAttrInstanceState(attributeName, attributeValue string) ImportStateCheckFunc {
+func testCheckResourceAttrInstanceState(id *string, attributeName, attributeValue string) ImportStateCheckFunc {
 	return func(is []*terraform.InstanceState) error {
-		if len(is) != 1 {
-			return fmt.Errorf("unexpected number of instance states: %d", len(is))
-		}
-
-		s := is[0]
+		for _, v := range is {
+			if v.ID != *id {
+				continue
+			}
 
-		attrVal, ok := s.Attributes[attributeName]
-		if !ok {
-			return fmt.Errorf("attribute %s found in instance state", attributeName)
-		}
+			if attrVal, ok := v.Attributes[attributeName]; ok {
+				if attrVal != attributeValue {
+					return fmt.Errorf("expected: %s got: %s", attributeValue, attrVal)
+				}
 
-		if attrVal != attributeValue {
-			return fmt.Errorf("expected: %s got: %s", attributeValue, attrVal)
+				return nil
+			}
 		}
 
-		return nil
+		return fmt.Errorf("attribute %s not found in instance state", attributeName)
 	}
 }