importstate: compare plan to previous state

helper/resource/importstate/import_block_as_first_step_test.go

@@ -38,12 +38,10 @@ func Test_ImportBlock_AsFirstStep(t *testing.T) {
 				ImportStateId:   "examplecloud_container.test",
 				ImportState:     true,
 				ImportStateKind: r.ImportBlockWithID,
-				// ImportStateVerify: true,
 				Config: `resource "examplecloud_container" "test" {
 					name = "somevalue"
 					location = "westeurope"
 				}`,
-				ImportStatePersist: true,
 				ImportPlanChecks: r.ImportPlanChecks{
 					PreApply: []plancheck.PlanCheck{
 						plancheck.ExpectResourceAction("examplecloud_container.test", plancheck.ResourceActionNoop),

helper/resource/importstate/import_block_in_config_directory_test.go

@@ -35,11 +35,11 @@ func Test_ImportBlock_InConfigDirectory(t *testing.T) {
 				ConfigDirectory: config.StaticDirectory(`testdata/1`),
 			},
 			{
-				ResourceName:      "examplecloud_container.test",
-				ImportState:       true,
-				ImportStateKind:   r.ImportBlockWithID,
-				ImportStateVerify: true,
-				ConfigDirectory:   config.StaticDirectory(`testdata/2`),
+				ResourceName:     "examplecloud_container.test",
+				ImportState:      true,
+				ImportStateKind:  r.ImportBlockWithID,
+				ImportPlanVerify: true,
+				ConfigDirectory:  config.StaticDirectory(`testdata/2`),
 			},
 		},
 	})

helper/resource/importstate/import_block_in_config_file_test.go

@@ -35,11 +35,11 @@ func Test_ImportBlock_InConfigFile(t *testing.T) {
 				ConfigFile: config.StaticFile(`testdata/1/examplecloud_container.tf`),
 			},
 			{
-				ResourceName:      "examplecloud_container.test",
-				ImportState:       true,
-				ImportStateKind:   r.ImportBlockWithID,
-				ImportStateVerify: true,
-				ConfigFile:        config.StaticFile(`testdata/2/examplecloud_container_import.tf`),
+				ResourceName:     "examplecloud_container.test",
+				ImportState:      true,
+				ImportStateKind:  r.ImportBlockWithID,
+				ImportPlanVerify: true,
+				ConfigFile:       config.StaticFile(`testdata/2/examplecloud_container_import.tf`),
 			},
 		},
 	})

helper/resource/importstate/import_block_verify_plan_test.go

@@ -0,0 +1,48 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package importstate_test
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
+
+	"github.com/hashicorp/terraform-plugin-testing/internal/testing/testprovider"
+	"github.com/hashicorp/terraform-plugin-testing/internal/testing/testsdk/providerserver"
+	"github.com/hashicorp/terraform-plugin-testing/tfversion"
+
+	r "github.com/hashicorp/terraform-plugin-testing/helper/resource"
+)
+
+func Test_ImportBlock_VerifyPlan(t *testing.T) {
+	t.Parallel()
+
+	r.UnitTest(t, r.TestCase{
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.SkipBelow(tfversion.Version1_5_0), // ImportBlockWithID requires Terraform 1.5.0 or later
+		},
+		ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
+			"examplecloud": providerserver.NewProviderServer(testprovider.Provider{
+				Resources: map[string]testprovider.Resource{
+					"examplecloud_container": examplecloudResource(),
+				},
+			}),
+		},
+		Steps: []r.TestStep{
+			{
+				Config: `
+				resource "examplecloud_container" "test" {
+					location = "westeurope"
+					name     = "somevalue"
+				}`,
+			},
+			{
+				ResourceName:     "examplecloud_container.test",
+				ImportState:      true,
+				ImportStateKind:  r.ImportBlockWithID,
+				ImportPlanVerify: true,
+			},
+		},
+	})
+}

helper/resource/importstate/import_block_with_id_test.go

@@ -44,10 +44,10 @@ func Test_TestStep_ImportBlockId(t *testing.T) {
 				}`,
 			},
 			{
-				ResourceName:      "examplecloud_container.test",
-				ImportState:       true,
-				ImportStateKind:   r.ImportBlockWithID,
-				ImportStateVerify: true,
+				ResourceName:     "examplecloud_container.test",
+				ImportState:      true,
+				ImportStateKind:  r.ImportBlockWithID,
+				ImportPlanVerify: true,
 			},
 		},
 	})
@@ -81,11 +81,11 @@ func TestTest_TestStep_ImportBlockId_ExpectError(t *testing.T) {
 					location = "eastus"
 					name     = "somevalue"
 				}`,
-				ResourceName:      "examplecloud_container.test",
-				ImportState:       true,
-				ImportStateKind:   r.ImportBlockWithID,
-				ImportStateVerify: true,
-				ExpectError:       regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got "update" action with plan(.?)`),
+				ResourceName:     "examplecloud_container.test",
+				ImportState:      true,
+				ImportStateKind:  r.ImportBlockWithID,
+				ImportPlanVerify: true,
+				ExpectError:      regexp.MustCompile(`importing resource examplecloud_container.test: expected a no-op resource action, got "update" action with plan(.?)`),
 			},
 		},
 	})
@@ -120,11 +120,11 @@ func TestTest_TestStep_ImportBlockId_FailWhenPlannableImportIsNotSupported(t *te
 					location = "eastus"
 					name     = "somevalue"
 				}`,
-				ResourceName:      "examplecloud_container.test",
-				ImportState:       true,
-				ImportStateKind:   r.ImportBlockWithID,
-				ImportStateVerify: true,
-				ExpectError:       regexp.MustCompile(`Terraform 1.5.0`),
+				ResourceName:     "examplecloud_container.test",
+				ImportState:      true,
+				ImportStateKind:  r.ImportBlockWithID,
+				ImportPlanVerify: true,
+				ExpectError:      regexp.MustCompile(`Terraform 1.5.0`),
 			},
 		},
 	})
@@ -200,6 +200,11 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes
 			I also need to omit the `password` in the import config, otherwise the value in the config is used when importing the
 		    with an import block and the test ends up passing regardless of whether `ImportStateVerifyIgnore` has been specified or not
 	*/
+
+	// In prerelease, we are choosing that ImportBlockWithID will not perform an apply, so it will not produce a new state,
+	// and there is no new state for ImportStateVerify to do anything meaningful with.
+	t.Skip()
+
 	t.Parallel()
 
 	r.UnitTest(t, r.TestCase{
@@ -301,6 +306,10 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes
 }
 
 func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore(t *testing.T) {
+	// In prerelease, we are choosing that ImportBlockWithID will not perform an apply, so it will not produce a new state,
+	// and there is no new state for ImportStateVerify to do anything meaningful with.
+	t.Skip()
+
 	t.Parallel()
 
 	r.UnitTest(t, r.TestCase{

helper/resource/importstate/import_command_with_id_test.go

@@ -20,7 +20,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/tfversion"
 )
 
-func Test_TestStep_ImportStateCheck_SkipDataSourceState(t *testing.T) {
+func Test_ImportCommmandWithId_SkipDataSourceState(t *testing.T) {
 	t.Parallel()
 
 	r.UnitTest(t, r.TestCase{
@@ -123,7 +123,7 @@ func Test_TestStep_ImportStateCheck_SkipDataSourceState(t *testing.T) {
 	})
 }
 
-func Test_TestStep_ImportStateVerify(t *testing.T) {
+func Test_ImportCommandWithId_ImportStateVerify(t *testing.T) {
 	t.Parallel()
 
 	r.UnitTest(t, r.TestCase{

helper/resource/testing.go

@@ -700,6 +700,11 @@ type TestStep struct {
 	// [plancheck]: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck
 	ImportPlanChecks ImportPlanChecks
 
+	// ImportPlanVerify checks that the planned resource state, created via a plannable import
+	// step (ImportBlockWithID or ImportBlockWithResourceIdentity), exactly matches
+	// the resource state of the previous test step.
+	ImportPlanVerify bool
+
 	// ImportStateVerify, if true, will also check that the state values
 	// that are finally put into the state after import match for all the
 	// IDs returned by the Import.  Note that this checks for strict equality

helper/resource/testing_new.go

@@ -68,7 +68,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest
 		var statePreDestroy *terraform.State
 		var err error
 		err = runProviderCommand(ctx, t, func() error {
-			statePreDestroy, err = getState(ctx, t, wd)
+			_, statePreDestroy, err = getState(ctx, t, wd)
 			if err != nil {
 				return err
 			}
@@ -447,18 +447,18 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest
 	}
 }
 
-func getState(ctx context.Context, t testing.T, wd *plugintest.WorkingDir) (*terraform.State, error) {
+func getState(ctx context.Context, t testing.T, wd *plugintest.WorkingDir) (*tfjson.State, *terraform.State, error) {
 	t.Helper()
 
 	jsonState, err := wd.State(ctx)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	state, err := shimStateFromJson(jsonState)
 	if err != nil {
 		t.Fatal(err)
 	}
-	return state, nil
+	return jsonState, state, nil
 }
 
 func stateIsEmpty(state *terraform.State) bool {
@@ -573,7 +573,7 @@ func testIDRefresh(ctx context.Context, t testing.T, c TestCase, wd *plugintest.
 		if err != nil {
 			t.Fatalf("Error running terraform refresh: %s", err)
 		}
-		state, err = getState(ctx, t, wd)
+		_, state, err = getState(ctx, t, wd)
 		if err != nil {
 			return err
 		}

helper/resource/testing_new_config.go

@@ -159,7 +159,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint
 			}
 
 			err = runProviderCommand(ctx, t, func() error {
-				stateBeforeApplication, err = getState(ctx, t, wd)
+				_, stateBeforeApplication, err = getState(ctx, t, wd)
 				if err != nil {
 					return err
 				}
@@ -200,7 +200,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint
 				var state *terraform.State
 
 				err := runProviderCommand(ctx, t, func() error {
-					state, err = getState(ctx, t, wd)
+					_, state, err = getState(ctx, t, wd)
 					if err != nil {
 						return err
 					}
@@ -384,7 +384,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint
 		var state *terraform.State
 
 		err = runProviderCommand(ctx, t, func() error {
-			state, err = getState(ctx, t, wd)
+			_, state, err = getState(ctx, t, wd)
 			if err != nil {
 				return err
 			}

helper/resource/testing_new_import_state.go

@@ -58,10 +58,11 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
 
 	// get state from check sequence
 	var state *terraform.State
+	var stateJSON *tfjson.State
 	var err error
 
 	err = runProviderCommand(ctx, t, func() error {
-		state, err = getState(ctx, t, wd)
+		stateJSON, state, err = getState(ctx, t, wd)
 		if err != nil {
 			return err
 		}
@@ -72,7 +73,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
 	}
 
 	// Determine the ID to import
-	var importId string
+	var importId string //nolint:revive
 	switch {
 	case step.ImportStateIdFunc != nil:
 		logging.HelperResourceTrace(ctx, "Using TestStep ImportStateIdFunc for import identifier")
@@ -185,37 +186,20 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
 			return err
 		}
 
-		if plan.ResourceChanges != nil {
+		if len(plan.ResourceChanges) > 0 {
 			logging.HelperResourceDebug(ctx, fmt.Sprintf("ImportBlockWithId: %d resource changes", len(plan.ResourceChanges)))
 
-			for _, rc := range plan.ResourceChanges {
-				if rc.Address != resourceName {
-					// we're only interested in the changes for the resource being imported
-					continue
-				}
-				if rc.Change != nil && rc.Change.Actions != nil {
-					// should this be length checked and used as a condition, if it's a no-op then there shouldn't be any other changes here
-					for _, action := range rc.Change.Actions {
-						if action != "no-op" {
-							var stdout string
-							err = runProviderCommand(ctx, t, func() error {
-								var err error
-								stdout, err = importWd.SavedPlanRawStdout(ctx)
-								return err
-							}, importWd, providers)
-							if err != nil {
-								return fmt.Errorf("retrieving formatted plan output: %w", err)
-							}
-
-							return fmt.Errorf("importing resource %s: expected a no-op resource action, got %q action with plan \nstdout:\n\n%s", rc.Address, action, stdout)
-						}
-					}
+			if err := requireNoopResourceAction(ctx, t, plan, resourceName, importWd, providers); err != nil {
+				return err
+			}
+
+			if step.ImportPlanVerify {
+				if err := teststep.VerifyImportPlan(plan, stateJSON); err != nil {
+					return err
 				}
 			}
 		}
 
-		// TODO compare plan to state from previous step
-
 		if err := runPlanChecks(ctx, t, plan, step.ImportPlanChecks.PreApply); err != nil {
 			return err
 		}
@@ -230,7 +214,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
 
 	var importState *terraform.State
 	err = runProviderCommand(ctx, t, func() error {
-		importState, err = getState(ctx, t, importWd)
+		_, importState, err = getState(ctx, t, importWd)
 		if err != nil {
 			return err
 		}
@@ -275,6 +259,10 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
 			identifierAttribute = "id"
 		}
 
+		if len(newResources) == 0 {
+			return fmt.Errorf("ImportStateVerify: no new resources imported")
+		}
+
 		for _, r := range newResources {
 			rIdentifier, ok := r.Primary.Attributes[identifierAttribute]
 
@@ -389,6 +377,46 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
 	return nil
 }
 
+func requireNoopResourceAction(ctx context.Context, t testing.T, plan *tfjson.Plan, resourceName string, importWd *plugintest.WorkingDir, providers *providerFactories) error {
+	t.Helper()
+
+	rc := findResourceChangeInPlan(t, plan, resourceName)
+	if rc == nil || rc.Change == nil || rc.Change.Actions == nil {
+		// does this matter?
+		return nil
+	}
+
+	// should this be length checked and used as a condition, if it's a no-op then there shouldn't be any other changes here
+	for _, action := range rc.Change.Actions {
+		if action != tfjson.ActionNoop {
+			var stdout string
+			err := runProviderCommand(ctx, t, func() error {
+				var err error
+				stdout, err = importWd.SavedPlanRawStdout(ctx)
+				return err
+			}, importWd, providers)
+			if err != nil {
+				return fmt.Errorf("retrieving formatted plan output: %w", err)
+			}
+
+			return fmt.Errorf("importing resource %s: expected a no-op resource action, got %q action with plan \nstdout:\n\n%s", rc.Address, action, stdout)
+		}
+	}
+
+	return nil
+}
+
+func findResourceChangeInPlan(t testing.T, plan *tfjson.Plan, resourceName string) *tfjson.ResourceChange {
+	t.Helper()
+
+	for _, rc := range plan.ResourceChanges {
+		if rc.Address == resourceName {
+			return rc
+		}
+	}
+	return nil
+}
+
 func appendImportBlock(config string, resourceName string, importID string) string {
 	return config + fmt.Sprintf(``+"\n"+
 		`import {`+"\n"+