helper/resource: Only call final Terraform CLI show command each Test…

…Step when TestCase.IDRefreshName is set (#892)

Reference: #730

Also clarifies the Go documentation for TestCase type IDRefreshName and IDRefreshIgnore fields.

.changelog/892.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+helper/resource: Removed extraneous Terraform CLI `show` command each `TestStep` unless using `TestCase.IDRefreshName`
+```

helper/resource/testing.go

@@ -376,16 +376,18 @@ type TestCase struct {
 	// same state. Each step can have its own check to verify correctness.
 	Steps []TestStep
 
-	// The settings below control the "ID-only refresh test." This is
-	// an enabled-by-default test that tests that a refresh can be
-	// refreshed with only an ID to result in the same attributes.
-	// This validates completeness of Refresh.
+	// IDRefreshName is the name of the resource to check during ID-only
+	// refresh testing, which ensures that a resource can be refreshed solely
+	// by its identifier. This will default to the first non-nil primary
+	// resource in the state. It runs every TestStep.
 	//
-	// IDRefreshName is the name of the resource to check. This will
-	// default to the first non-nil primary resource in the state.
-	//
-	// IDRefreshIgnore is a list of configuration keys that will be ignored.
-	IDRefreshName   string
+	// While not deprecated, most resource tests should instead prefer using
+	// TestStep.ImportState based testing as it works with multiple attribute
+	// identifiers and also verifies resource import functionality.
+	IDRefreshName string
+
+	// IDRefreshIgnore is a list of configuration keys that will be ignored
+	// during ID-only refresh testing.
 	IDRefreshIgnore []string
 }
 

helper/resource/testing_new_config.go

@@ -16,9 +16,6 @@ import (
 func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugintest.WorkingDir, step TestStep) error {
 	t.Helper()
 
-	var idRefreshCheck *terraform.ResourceState
-	idRefresh := c.IDRefreshName != ""
-
 	if !step.Destroy {
 		var state *terraform.State
 		var err error
@@ -249,22 +246,31 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint
 	// ID-ONLY REFRESH
 	// If we've never checked an id-only refresh and our state isn't
 	// empty, find the first resource and test it.
-	var state *terraform.State
-	err = runProviderCommand(ctx, t, func() error {
-		state, err = getState(ctx, t, wd)
+	if c.IDRefreshName != "" {
+		logging.HelperResourceTrace(ctx, "Using TestCase IDRefreshName")
+
+		var state *terraform.State
+
+		err = runProviderCommand(ctx, t, func() error {
+			state, err = getState(ctx, t, wd)
+			if err != nil {
+				return err
+			}
+			return nil
+		}, wd, providerFactories{
+			legacy:  c.ProviderFactories,
+			protov5: c.ProtoV5ProviderFactories,
+			protov6: c.ProtoV6ProviderFactories})
+
 		if err != nil {
 			return err
 		}
-		return nil
-	}, wd, providerFactories{
-		legacy:  c.ProviderFactories,
-		protov5: c.ProtoV5ProviderFactories,
-		protov6: c.ProtoV6ProviderFactories})
-	if err != nil {
-		return err
-	}
-	if idRefresh && idRefreshCheck == nil && !state.Empty() {
-		logging.HelperResourceTrace(ctx, "Using TestCase IDRefreshName")
+
+		if state.Empty() {
+			return nil
+		}
+
+		var idRefreshCheck *terraform.ResourceState
 
 		// Find the first non-nil resource in the state
 		for _, m := range state.Modules {