From f9a2ca4b51e2d8557072d01468184c1a923c2fac Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 29 Sep 2022 07:17:41 +0100 Subject: [PATCH 01/13] Adding RefreshState test step (#1069) --- helper/resource/testing.go | 16 ++++ helper/resource/testing_new.go | 39 ++++++++ helper/resource/testing_new_refresh_state.go | 97 ++++++++++++++++++++ helper/resource/teststep_providers_test.go | 96 +++++++++++++++++++ 4 files changed, 248 insertions(+) create mode 100644 helper/resource/testing_new_refresh_state.go diff --git a/helper/resource/testing.go b/helper/resource/testing.go index e509586e350..b423a1383e6 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -292,6 +292,9 @@ type ImportStateCheckFunc func([]*terraform.InstanceState) error // generation for ImportState tests. type ImportStateIdFunc func(*terraform.State) (string, error) +// RefreshStateCheckFunc is the check function for RefreshState tests +type RefreshStateCheckFunc func([]*terraform.InstanceState) error + // ErrorCheckFunc is a function providers can use to handle errors. type ErrorCheckFunc func(error) error @@ -570,6 +573,19 @@ type TestStep struct { // at the end of the test step that is verifying import behavior. ImportStatePersist bool + //--------------------------------------------------------------- + // RefreshState testing + //--------------------------------------------------------------- + + // RefreshState, if true, will test the functionality of `terraform + // refresh` by refreshing the state. + RefreshState bool + + // RefreshStateCheck checks state following `terraform refresh`. It + // should be used to verify that the state has the expected resources, + // IDs, and attributes. + RefreshStateCheck RefreshStateCheckFunc + // ProviderFactories can be specified for the providers that are valid for // this TestStep. When providers are specified at the TestStep level, all // TestStep within a TestCase must declare providers. diff --git a/helper/resource/testing_new.go b/helper/resource/testing_new.go index 81fc59652ba..64d11430a4e 100644 --- a/helper/resource/testing_new.go +++ b/helper/resource/testing_new.go @@ -241,6 +241,45 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest continue } + if step.RefreshState { + logging.HelperResourceTrace(ctx, "TestStep is RefreshState mode") + + err := testStepNewRefreshState(ctx, t, wd, step, appliedCfg, providers) + if step.ExpectError != nil { + logging.HelperResourceDebug(ctx, "Checking TestStep ExpectError") + if err == nil { + logging.HelperResourceError(ctx, + "Error running refresh: expected an error but got none", + ) + t.Fatalf("Step %d/%d error running refresh: expected an error but got none", stepNumber, len(c.Steps)) + } + if !step.ExpectError.MatchString(err.Error()) { + logging.HelperResourceError(ctx, + fmt.Sprintf("Error running refresh: expected an error with pattern (%s)", step.ExpectError.String()), + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("Step %d/%d error running refresh, expected an error with pattern (%s), no match on: %s", stepNumber, len(c.Steps), step.ExpectError.String(), err) + } + } else { + if err != nil && c.ErrorCheck != nil { + logging.HelperResourceDebug(ctx, "Calling TestCase ErrorCheck") + err = c.ErrorCheck(err) + logging.HelperResourceDebug(ctx, "Called TestCase ErrorCheck") + } + if err != nil { + logging.HelperResourceError(ctx, + "Error running refresh", + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("Step %d/%d error running refresh: %s", stepNumber, len(c.Steps), err) + } + } + + logging.HelperResourceDebug(ctx, "Finished TestStep") + + continue + } + if step.Config != "" { logging.HelperResourceTrace(ctx, "TestStep is Config mode") diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go new file mode 100644 index 00000000000..af4c9767f08 --- /dev/null +++ b/helper/resource/testing_new_refresh_state.go @@ -0,0 +1,97 @@ +package resource + +import ( + "context" + + "github.com/davecgh/go-spew/spew" + "github.com/mitchellh/go-testing-interface" + + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/plugintest" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" +) + +func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.WorkingDir, step TestStep, cfg string, providers *providerFactories) error { + t.Helper() + + spewConf := spew.NewDefaultConfig() + spewConf.SortKeys = true + + var err error + err = runProviderCommand(ctx, t, func() error { + _, err = getState(ctx, t, wd) + if err != nil { + return err + } + return nil + }, wd, providers) + if err != nil { + t.Fatalf("Error getting state: %s", err) + } + + if step.Config == "" { + logging.HelperResourceTrace(ctx, "Using prior TestStep Config for refresh") + + step.Config = cfg + if step.Config == "" { + t.Fatal("Cannot refresh state with no specified config") + } + } + + err = wd.SetConfig(ctx, step.Config) + if err != nil { + t.Fatalf("Error setting test config: %s", err) + } + + logging.HelperResourceDebug(ctx, "Running Terraform CLI init and refresh") + + err = runProviderCommand(ctx, t, func() error { + return wd.Init(ctx) + }, wd, providers) + if err != nil { + t.Fatalf("Error running init: %s", err) + } + + err = runProviderCommand(ctx, t, func() error { + return wd.Refresh(ctx) + }, wd, providers) + if err != nil { + return err + } + + var refreshState *terraform.State + err = runProviderCommand(ctx, t, func() error { + refreshState, err = getState(ctx, t, wd) + if err != nil { + return err + } + return nil + }, wd, providers) + if err != nil { + t.Fatalf("Error getting state: %s", err) + } + + // Go through the refreshed state and verify + if step.RefreshStateCheck != nil { + logging.HelperResourceTrace(ctx, "Using TestStep RefreshStateCheck") + + var states []*terraform.InstanceState + for _, r := range refreshState.RootModule().Resources { + if r.Primary != nil { + is := r.Primary.DeepCopy() + is.Ephemeral.Type = r.Type // otherwise the check function cannot see the type + states = append(states, is) + } + } + + logging.HelperResourceDebug(ctx, "Calling TestStep RefreshStateCheck") + + if err := step.RefreshStateCheck(states); err != nil { + t.Fatal(err) + } + + logging.HelperResourceDebug(ctx, "Called TestStep RefreshStateCheck") + } + + return nil +} diff --git a/helper/resource/teststep_providers_test.go b/helper/resource/teststep_providers_test.go index bce1c19d1a8..45128b86944 100644 --- a/helper/resource/teststep_providers_test.go +++ b/helper/resource/teststep_providers_test.go @@ -1574,6 +1574,69 @@ func TestTest_TestStep_ProviderFactories_Import_External_WithoutPersistNonMatch( }) } +func TestTest_TestStep_ProviderFactories_Refresh_Inline(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "random": func() (*schema.Provider, error) { //nolint:unparam // required signature + return &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "random_password": { + CreateContext: func(ctx context.Context, d *schema.ResourceData, i interface{}) diag.Diagnostics { + d.SetId("id") + err := d.Set("min_special", 10) + if err != nil { + panic(err) + } + return nil + }, + DeleteContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := d.Set("min_special", 2) + if err != nil { + panic(err) + } + return nil + }, + Schema: map[string]*schema.Schema{ + "min_special": { + Computed: true, + Type: schema.TypeInt, + }, + + "id": { + Computed: true, + Type: schema.TypeString, + }, + }, + }, + }, + }, nil + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_password" "test" { }`, + Check: TestCheckResourceAttr("random_password.test", "min_special", "10"), + }, + { + Config: `resource "random_password" "test" { }`, + RefreshState: true, + RefreshStateCheck: composeRefreshStateCheck( + testCheckResourceAttrInstanceStateRefresh("min_special", "2"), + ), + }, + { + Config: `resource "random_password" "test" { }`, + Check: TestCheckResourceAttr("random_password.test", "min_special", "2"), + }, + }, + }) +} + func composeImportStateCheck(fs ...ImportStateCheckFunc) ImportStateCheckFunc { return func(s []*terraform.InstanceState) error { for i, f := range fs { @@ -1626,6 +1689,39 @@ func testCheckResourceAttrInstanceState(attributeName, attributeValue string) Im } } +func composeRefreshStateCheck(fs ...RefreshStateCheckFunc) RefreshStateCheckFunc { + return func(s []*terraform.InstanceState) error { + for i, f := range fs { + if err := f(s); err != nil { + return fmt.Errorf("check %d/%d error: %s", i+1, len(fs), err) + } + } + + return nil + } +} + +func testCheckResourceAttrInstanceStateRefresh(attributeName, attributeValue string) RefreshStateCheckFunc { + return func(is []*terraform.InstanceState) error { + if len(is) != 1 { + return fmt.Errorf("unexpected number of instance states: %d", len(is)) + } + + s := is[0] + + attrVal, ok := s.Attributes[attributeName] + if !ok { + return fmt.Errorf("attribute %s found in instance state", attributeName) + } + + if attrVal != attributeValue { + return fmt.Errorf("expected: %s got: %s", attributeValue, attrVal) + } + + return nil + } +} + func testExtractResourceAttr(resourceName string, attributeName string, attributeValue *string) TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] From 204d99ea014394a3721f7d0d13ee3f168885115b Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 29 Sep 2022 07:22:56 +0100 Subject: [PATCH 02/13] Adding CHANGELOG entry (#1069) --- .changelog/1070.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/1070.txt diff --git a/.changelog/1070.txt b/.changelog/1070.txt new file mode 100644 index 00000000000..b830de704af --- /dev/null +++ b/.changelog/1070.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +helper/resource: Add RefreshState and RefreshStateCheck to allow testing of refresh in isolation +``` From 468b72da315af72d7b4e3f91a391d01526df56a8 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Mon, 3 Oct 2022 02:41:05 +0100 Subject: [PATCH 03/13] Running TestCheckFunc with refreshed state (#1069) --- .changelog/1070.txt | 2 +- helper/resource/testing.go | 8 ----- helper/resource/testing_new_refresh_state.go | 19 +++------- helper/resource/teststep_providers_test.go | 37 +------------------- 4 files changed, 6 insertions(+), 60 deletions(-) diff --git a/.changelog/1070.txt b/.changelog/1070.txt index b830de704af..6b95d995327 100644 --- a/.changelog/1070.txt +++ b/.changelog/1070.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -helper/resource: Add RefreshState and RefreshStateCheck to allow testing of refresh in isolation +helper/resource: Add RefreshState to allow testing of refresh in isolation ``` diff --git a/helper/resource/testing.go b/helper/resource/testing.go index b423a1383e6..5fadf24760d 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -292,9 +292,6 @@ type ImportStateCheckFunc func([]*terraform.InstanceState) error // generation for ImportState tests. type ImportStateIdFunc func(*terraform.State) (string, error) -// RefreshStateCheckFunc is the check function for RefreshState tests -type RefreshStateCheckFunc func([]*terraform.InstanceState) error - // ErrorCheckFunc is a function providers can use to handle errors. type ErrorCheckFunc func(error) error @@ -581,11 +578,6 @@ type TestStep struct { // refresh` by refreshing the state. RefreshState bool - // RefreshStateCheck checks state following `terraform refresh`. It - // should be used to verify that the state has the expected resources, - // IDs, and attributes. - RefreshStateCheck RefreshStateCheckFunc - // ProviderFactories can be specified for the providers that are valid for // this TestStep. When providers are specified at the TestStep level, all // TestStep within a TestCase must declare providers. diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go index af4c9767f08..5cbba57a10a 100644 --- a/helper/resource/testing_new_refresh_state.go +++ b/helper/resource/testing_new_refresh_state.go @@ -72,25 +72,14 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo } // Go through the refreshed state and verify - if step.RefreshStateCheck != nil { - logging.HelperResourceTrace(ctx, "Using TestStep RefreshStateCheck") - - var states []*terraform.InstanceState - for _, r := range refreshState.RootModule().Resources { - if r.Primary != nil { - is := r.Primary.DeepCopy() - is.Ephemeral.Type = r.Type // otherwise the check function cannot see the type - states = append(states, is) - } - } - - logging.HelperResourceDebug(ctx, "Calling TestStep RefreshStateCheck") + if step.Check != nil { + logging.HelperResourceDebug(ctx, "Calling TestStep Check for RefreshState") - if err := step.RefreshStateCheck(states); err != nil { + if err := step.Check(refreshState); err != nil { t.Fatal(err) } - logging.HelperResourceDebug(ctx, "Called TestStep RefreshStateCheck") + logging.HelperResourceDebug(ctx, "Called TestStep Check for RefreshState") } return nil diff --git a/helper/resource/teststep_providers_test.go b/helper/resource/teststep_providers_test.go index 45128b86944..01cc247b541 100644 --- a/helper/resource/teststep_providers_test.go +++ b/helper/resource/teststep_providers_test.go @@ -1625,9 +1625,7 @@ func TestTest_TestStep_ProviderFactories_Refresh_Inline(t *testing.T) { { Config: `resource "random_password" "test" { }`, RefreshState: true, - RefreshStateCheck: composeRefreshStateCheck( - testCheckResourceAttrInstanceStateRefresh("min_special", "2"), - ), + Check: TestCheckResourceAttr("random_password.test", "min_special", "2"), }, { Config: `resource "random_password" "test" { }`, @@ -1689,39 +1687,6 @@ func testCheckResourceAttrInstanceState(attributeName, attributeValue string) Im } } -func composeRefreshStateCheck(fs ...RefreshStateCheckFunc) RefreshStateCheckFunc { - return func(s []*terraform.InstanceState) error { - for i, f := range fs { - if err := f(s); err != nil { - return fmt.Errorf("check %d/%d error: %s", i+1, len(fs), err) - } - } - - return nil - } -} - -func testCheckResourceAttrInstanceStateRefresh(attributeName, attributeValue string) RefreshStateCheckFunc { - return func(is []*terraform.InstanceState) error { - if len(is) != 1 { - return fmt.Errorf("unexpected number of instance states: %d", len(is)) - } - - s := is[0] - - attrVal, ok := s.Attributes[attributeName] - if !ok { - return fmt.Errorf("attribute %s found in instance state", attributeName) - } - - if attrVal != attributeValue { - return fmt.Errorf("expected: %s got: %s", attributeValue, attrVal) - } - - return nil - } -} - func testExtractResourceAttr(resourceName string, attributeName string, attributeValue *string) TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] From a5fad91a9b37c6b709eca086286a9ea15bd2b025 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 6 Oct 2022 10:09:58 +0100 Subject: [PATCH 04/13] Adding validation to check conditions for RefreshState (#1069) --- helper/resource/teststep_validate.go | 19 +++++++++++++++--- helper/resource/teststep_validate_test.go | 24 +++++++++++++++++++---- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/helper/resource/teststep_validate.go b/helper/resource/teststep_validate.go index 95b36e7b97c..cb531b928a0 100644 --- a/helper/resource/teststep_validate.go +++ b/helper/resource/teststep_validate.go @@ -43,7 +43,8 @@ func (s TestStep) hasProviders(_ context.Context) bool { // validate ensures the TestStep is valid based on the following criteria: // -// - Config or ImportState is set. +// - Config or ImportState or RefreshState is set. +// - RefreshState is not the first TestStep. // - Providers are not specified (ExternalProviders, // ProtoV5ProviderFactories, ProtoV6ProviderFactories, ProviderFactories) // if specified at the TestCase level. @@ -58,8 +59,20 @@ func (s TestStep) validate(ctx context.Context, req testStepValidateRequest) err logging.HelperResourceTrace(ctx, "Validating TestStep") - if s.Config == "" && !s.ImportState { - err := fmt.Errorf("TestStep missing Config or ImportState") + if s.Config == "" && !s.ImportState && !s.RefreshState { + err := fmt.Errorf("TestStep missing Config or ImportState or RefreshState") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if s.RefreshState && req.StepNumber == 1 { + err := fmt.Errorf("TestStep cannot have RefreshState as first step") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if s.ImportState && s.RefreshState { + err := fmt.Errorf("TestStep cannot have ImportState and RefreshState in same step") logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) return err } diff --git a/helper/resource/teststep_validate_test.go b/helper/resource/teststep_validate_test.go index 8caf5c23323..2a348f9442a 100644 --- a/helper/resource/teststep_validate_test.go +++ b/helper/resource/teststep_validate_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -79,12 +80,27 @@ func TestTestStepValidate(t *testing.T) { testStepValidateRequest testStepValidateRequest expectedError error }{ - "config-and-importstate-missing": { - testStep: TestStep{}, + "config-and-importstate-and-refreshstate-missing": { + testStep: TestStep{}, + testStepValidateRequest: testStepValidateRequest{}, + expectedError: fmt.Errorf("TestStep missing Config or ImportState or RefreshState"), + }, + "refreshstate-first-step": { + testStep: TestStep{ + RefreshState: true, + }, testStepValidateRequest: testStepValidateRequest{ - TestCaseHasProviders: true, + StepNumber: 1, }, - expectedError: fmt.Errorf("TestStep missing Config or ImportState"), + expectedError: fmt.Errorf("TestStep cannot have RefreshState as first step"), + }, + "importstate-and-refreshstate-both-true": { + testStep: TestStep{ + ImportState: true, + RefreshState: true, + }, + testStepValidateRequest: testStepValidateRequest{}, + expectedError: fmt.Errorf("TestStep cannot have ImportState and RefreshState in same step"), }, "externalproviders-overlapping-providerfactories": { testStep: TestStep{ From e17526aebb4b58d8f4109dcfec365a99f0fc1610 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 6 Oct 2022 10:10:20 +0100 Subject: [PATCH 05/13] Expanding comments on RefreshState (#1069) --- helper/resource/testing.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 5fadf24760d..d651a86dcc6 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -575,7 +575,17 @@ type TestStep struct { //--------------------------------------------------------------- // RefreshState, if true, will test the functionality of `terraform - // refresh` by refreshing the state. + // refresh` by refreshing the state, running a plan and optionally + // running checks. + // + // If the refresh is expected to result in a non-empty plan + // ExpectNonEmptyPlan should be set to true in the same TestStep. + // + // If RefreshState is true the Config from the previous TestStep is used + // and Config, if defined, is ignored. + // + // RefreshState cannot be the first TestStep and, it is mutually exclusive + // with ImportState. RefreshState bool // ProviderFactories can be specified for the providers that are valid for From 51342f5ae77313bfc532492a463e3d41820fd24e Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 6 Oct 2022 10:11:37 +0100 Subject: [PATCH 06/13] Removing option to override config during refresh testing, removing re-init and adding plan following refresh (#1069) --- helper/resource/testing_new.go | 4 +- helper/resource/testing_new_refresh_state.go | 61 ++++++++++++-------- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/helper/resource/testing_new.go b/helper/resource/testing_new.go index 64d11430a4e..6fa78c23ec7 100644 --- a/helper/resource/testing_new.go +++ b/helper/resource/testing_new.go @@ -114,7 +114,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest logging.HelperResourceDebug(ctx, "Starting TestSteps") - // use this to track last step succesfully applied + // use this to track last step successfully applied // acts as default for import tests var appliedCfg string @@ -244,7 +244,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest if step.RefreshState { logging.HelperResourceTrace(ctx, "TestStep is RefreshState mode") - err := testStepNewRefreshState(ctx, t, wd, step, appliedCfg, providers) + err := testStepNewRefreshState(ctx, t, wd, step, providers) if step.ExpectError != nil { logging.HelperResourceDebug(ctx, "Checking TestStep ExpectError") if err == nil { diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go index 5cbba57a10a..bbab249649f 100644 --- a/helper/resource/testing_new_refresh_state.go +++ b/helper/resource/testing_new_refresh_state.go @@ -2,8 +2,10 @@ package resource import ( "context" + "fmt" "github.com/davecgh/go-spew/spew" + tfjson "github.com/hashicorp/terraform-json" "github.com/mitchellh/go-testing-interface" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" @@ -11,7 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) -func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.WorkingDir, step TestStep, cfg string, providers *providerFactories) error { +func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.WorkingDir, step TestStep, providers *providerFactories) error { t.Helper() spewConf := spew.NewDefaultConfig() @@ -29,29 +31,6 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo t.Fatalf("Error getting state: %s", err) } - if step.Config == "" { - logging.HelperResourceTrace(ctx, "Using prior TestStep Config for refresh") - - step.Config = cfg - if step.Config == "" { - t.Fatal("Cannot refresh state with no specified config") - } - } - - err = wd.SetConfig(ctx, step.Config) - if err != nil { - t.Fatalf("Error setting test config: %s", err) - } - - logging.HelperResourceDebug(ctx, "Running Terraform CLI init and refresh") - - err = runProviderCommand(ctx, t, func() error { - return wd.Init(ctx) - }, wd, providers) - if err != nil { - t.Fatalf("Error running init: %s", err) - } - err = runProviderCommand(ctx, t, func() error { return wd.Refresh(ctx) }, wd, providers) @@ -82,5 +61,39 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo logging.HelperResourceDebug(ctx, "Called TestStep Check for RefreshState") } + // do a plan + err = runProviderCommand(ctx, t, func() error { + if step.Destroy { + return wd.CreateDestroyPlan(ctx) + } + return wd.CreatePlan(ctx) + }, wd, providers) + if err != nil { + return fmt.Errorf("Error running post-apply plan: %w", err) + } + + var plan *tfjson.Plan + err = runProviderCommand(ctx, t, func() error { + var err error + plan, err = wd.SavedPlan(ctx) + return err + }, wd, providers) + if err != nil { + return fmt.Errorf("Error retrieving post-apply plan: %w", err) + } + + if !planIsEmpty(plan) && !step.ExpectNonEmptyPlan { + var stdout string + err = runProviderCommand(ctx, t, func() error { + var err error + stdout, err = wd.SavedPlanRawStdout(ctx) + return err + }, wd, providers) + if err != nil { + return fmt.Errorf("Error retrieving formatted plan output: %w", err) + } + return fmt.Errorf("After applying this test step, the plan was not empty.\nstdout:\n\n%s", stdout) + } + return nil } From 2f726cb3160410c9eda124044f34593a7134db20 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 6 Oct 2022 10:13:43 +0100 Subject: [PATCH 07/13] Adding test coverage for to verify expect non-empty plan following refresh (#1069) --- helper/resource/teststep_providers_test.go | 92 ++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/helper/resource/teststep_providers_test.go b/helper/resource/teststep_providers_test.go index 01cc247b541..e234b028607 100644 --- a/helper/resource/teststep_providers_test.go +++ b/helper/resource/teststep_providers_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) @@ -1635,6 +1636,97 @@ func TestTest_TestStep_ProviderFactories_Refresh_Inline(t *testing.T) { }) } +func TestTest_TestStep_ProviderFactories_RefreshWithPlanModifier_Inline(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "random": func() (*schema.Provider, error) { //nolint:unparam // required signature + return &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "random_password": { + CustomizeDiff: customdiff.All( + func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error { + special := d.Get("special").(bool) + if special == true { + err := d.SetNew("special", false) + if err != nil { + panic(err) + } + } + return nil + }, + ), + CreateContext: func(ctx context.Context, d *schema.ResourceData, i interface{}) diag.Diagnostics { + d.SetId("id") + err := d.Set("special", false) + if err != nil { + panic(err) + } + return nil + }, + DeleteContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + t := getTimeForTest() + if t.After(time.Now().Add(time.Hour * 1)) { + err := d.Set("special", true) + if err != nil { + panic(err) + } + } + return nil + }, + Schema: map[string]*schema.Schema{ + "special": { + Computed: true, + Type: schema.TypeBool, + ForceNew: true, + }, + + "id": { + Computed: true, + Type: schema.TypeString, + }, + }, + }, + }, + }, nil + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_password" "test" { }`, + Check: TestCheckResourceAttr("random_password.test", "special", "false"), + }, + { + PreConfig: setTimeForTest(time.Now().Add(time.Hour * 2)), + RefreshState: true, + ExpectNonEmptyPlan: true, + Check: TestCheckResourceAttr("random_password.test", "special", "true"), + }, + { + Config: `resource "random_password" "test" { }`, + Check: TestCheckResourceAttr("random_password.test", "special", "false"), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func setTimeForTest(t time.Time) func() { + return func() { + getTimeForTest = func() time.Time { + return t + } + } +} + +var getTimeForTest = func() time.Time { + return time.Now() +} + func composeImportStateCheck(fs ...ImportStateCheckFunc) ImportStateCheckFunc { return func(s []*terraform.InstanceState) error { for i, f := range fs { From 0f220c81755ca47b4c49df4eeed32fc4f865a0df Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 11 Oct 2022 07:51:53 +0100 Subject: [PATCH 08/13] Apply suggestions from code review Co-authored-by: Brian Flad --- .changelog/1070.txt | 2 +- helper/resource/testing.go | 8 +++----- helper/resource/testing_new_refresh_state.go | 6 ++---- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/.changelog/1070.txt b/.changelog/1070.txt index 6b95d995327..53070197d3d 100644 --- a/.changelog/1070.txt +++ b/.changelog/1070.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -helper/resource: Add RefreshState to allow testing of refresh in isolation +helper/resource: Added `TestStep` type `RefreshState` field, which enables a step that refreshes state without an explicit apply or configuration changes ``` diff --git a/helper/resource/testing.go b/helper/resource/testing.go index d651a86dcc6..c77f0629ac6 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -575,15 +575,13 @@ type TestStep struct { //--------------------------------------------------------------- // RefreshState, if true, will test the functionality of `terraform - // refresh` by refreshing the state, running a plan and optionally - // running checks. + // refresh` by refreshing the state, running any checks against the + // refreshed state, and running a plan to verify against unexpected plan + // differences. // // If the refresh is expected to result in a non-empty plan // ExpectNonEmptyPlan should be set to true in the same TestStep. // - // If RefreshState is true the Config from the previous TestStep is used - // and Config, if defined, is ignored. - // // RefreshState cannot be the first TestStep and, it is mutually exclusive // with ImportState. RefreshState bool diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go index bbab249649f..16112934743 100644 --- a/helper/resource/testing_new_refresh_state.go +++ b/helper/resource/testing_new_refresh_state.go @@ -20,6 +20,7 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo spewConf.SortKeys = true var err error + // Explicitly ensure prior state exists before refresh. err = runProviderCommand(ctx, t, func() error { _, err = getState(ctx, t, wd) if err != nil { @@ -63,9 +64,6 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo // do a plan err = runProviderCommand(ctx, t, func() error { - if step.Destroy { - return wd.CreateDestroyPlan(ctx) - } return wd.CreatePlan(ctx) }, wd, providers) if err != nil { @@ -92,7 +90,7 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo if err != nil { return fmt.Errorf("Error retrieving formatted plan output: %w", err) } - return fmt.Errorf("After applying this test step, the plan was not empty.\nstdout:\n\n%s", stdout) + return fmt.Errorf("After refreshing state during this test step, a followup plan was not empty.\nstdout:\n\n%s", stdout) } return nil From 4be309c37976aa8ae06da205e57145a0dac57755 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 11 Oct 2022 08:22:10 +0100 Subject: [PATCH 09/13] Adding validation to verify that refresh state is not present with config or destroy in a test step (#1069) --- helper/resource/teststep_validate.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/helper/resource/teststep_validate.go b/helper/resource/teststep_validate.go index cb531b928a0..0ba655168c8 100644 --- a/helper/resource/teststep_validate.go +++ b/helper/resource/teststep_validate.go @@ -44,6 +44,8 @@ func (s TestStep) hasProviders(_ context.Context) bool { // validate ensures the TestStep is valid based on the following criteria: // // - Config or ImportState or RefreshState is set. +// - Config and RefreshState are not both set. +// - RefreshState and Destroy are not both set. // - RefreshState is not the first TestStep. // - Providers are not specified (ExternalProviders, // ProtoV5ProviderFactories, ProtoV6ProviderFactories, ProviderFactories) @@ -65,6 +67,18 @@ func (s TestStep) validate(ctx context.Context, req testStepValidateRequest) err return err } + if s.Config != "" && s.RefreshState { + err := fmt.Errorf("TestStep cannot have Config and RefreshState") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if s.RefreshState && s.Destroy { + err := fmt.Errorf("TestStep cannot have RefreshState and Destroy") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + if s.RefreshState && req.StepNumber == 1 { err := fmt.Errorf("TestStep cannot have RefreshState as first step") logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) From 034a8b75e66dfb0a94a73f650fc983d7cc9bc692 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 11 Oct 2022 08:42:55 +0100 Subject: [PATCH 10/13] Reset time during test step so that ReadContext does not mutate state and result in a diff (#1069) --- helper/resource/teststep_providers_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helper/resource/teststep_providers_test.go b/helper/resource/teststep_providers_test.go index e234b028607..f9c13d69342 100644 --- a/helper/resource/teststep_providers_test.go +++ b/helper/resource/teststep_providers_test.go @@ -1707,9 +1707,9 @@ func TestTest_TestStep_ProviderFactories_RefreshWithPlanModifier_Inline(t *testi Check: TestCheckResourceAttr("random_password.test", "special", "true"), }, { - Config: `resource "random_password" "test" { }`, - Check: TestCheckResourceAttr("random_password.test", "special", "false"), - ExpectNonEmptyPlan: true, + PreConfig: setTimeForTest(time.Now()), + Config: `resource "random_password" "test" { }`, + Check: TestCheckResourceAttr("random_password.test", "special", "false"), }, }, }) From bc221f73769b67bc39cfa5bbf1bc3e4fbafc42c9 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 11 Oct 2022 08:53:03 +0100 Subject: [PATCH 11/13] Updating website docs (#1069) --- helper/resource/teststep_providers_test.go | 1 - .../plugin/sdkv2/testing/acceptance-tests/teststep.mdx | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/helper/resource/teststep_providers_test.go b/helper/resource/teststep_providers_test.go index f9c13d69342..92ddbe0875e 100644 --- a/helper/resource/teststep_providers_test.go +++ b/helper/resource/teststep_providers_test.go @@ -1624,7 +1624,6 @@ func TestTest_TestStep_ProviderFactories_Refresh_Inline(t *testing.T) { Check: TestCheckResourceAttr("random_password.test", "min_special", "10"), }, { - Config: `resource "random_password" "test" { }`, RefreshState: true, Check: TestCheckResourceAttr("random_password.test", "min_special", "2"), }, diff --git a/website/docs/plugin/sdkv2/testing/acceptance-tests/teststep.mdx b/website/docs/plugin/sdkv2/testing/acceptance-tests/teststep.mdx index 5d0259e777b..b0fe8b08a5f 100644 --- a/website/docs/plugin/sdkv2/testing/acceptance-tests/teststep.mdx +++ b/website/docs/plugin/sdkv2/testing/acceptance-tests/teststep.mdx @@ -14,8 +14,8 @@ under test. ## Test Modes -Terraform’s test framework facilitates two distinct modes of acceptance tests, -_Lifecycle_ and _Import_. +Terraform’s test framework facilitates three distinct modes of acceptance tests, +_Lifecycle_, _Import_ and _Refresh_. _Lifecycle_ mode is the most common mode, and is used for testing plugins by providing one or more configuration files with the same logic as would be used @@ -25,6 +25,10 @@ _Import_ mode is used for testing resource functionality to import existing infrastructure into a Terraform statefile, using the same logic as would be used when running `terraform import`. +_Refresh_ mode is used for testing resource functionality to refresh existing +infrastructure, using the same logic as would be used when running +`terraform refresh`. + An acceptance test’s mode is implicitly determined by the fields provided in the `TestStep` definition. The applicable fields are defined in the [TestStep Reference API](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/helper/resource#TestStep). From 9ec2d62a30a410489bc078307e69d0012b8e48be Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Wed, 12 Oct 2022 07:58:29 +0100 Subject: [PATCH 12/13] Apply suggestions from code review Co-authored-by: Brian Flad --- helper/resource/teststep_validate_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/helper/resource/teststep_validate_test.go b/helper/resource/teststep_validate_test.go index 2a348f9442a..ecdcad33707 100644 --- a/helper/resource/teststep_validate_test.go +++ b/helper/resource/teststep_validate_test.go @@ -102,6 +102,14 @@ func TestTestStepValidate(t *testing.T) { testStepValidateRequest: testStepValidateRequest{}, expectedError: fmt.Errorf("TestStep cannot have ImportState and RefreshState in same step"), }, + "destroy-and-refreshstate-both-true": { + testStep: TestStep{ + Destroy: true, + RefreshState: true, + }, + testStepValidateRequest: testStepValidateRequest{}, + expectedError: fmt.Errorf("TestStep cannot have RefreshState and Destroy"), + }, "externalproviders-overlapping-providerfactories": { testStep: TestStep{ Config: "# not empty", From 5be338016e997e17917b7ca8c638bde989c4dc5a Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Wed, 12 Oct 2022 08:06:34 +0100 Subject: [PATCH 13/13] Test to verify that setting config and refresh state together is not valid (#1069) --- helper/resource/teststep_validate_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/helper/resource/teststep_validate_test.go b/helper/resource/teststep_validate_test.go index ecdcad33707..15567a07888 100644 --- a/helper/resource/teststep_validate_test.go +++ b/helper/resource/teststep_validate_test.go @@ -85,6 +85,13 @@ func TestTestStepValidate(t *testing.T) { testStepValidateRequest: testStepValidateRequest{}, expectedError: fmt.Errorf("TestStep missing Config or ImportState or RefreshState"), }, + "config-and-refreshstate-both-set": { + testStep: TestStep{ + Config: "# not empty", + RefreshState: true, + }, + expectedError: fmt.Errorf("TestStep cannot have Config and RefreshState"), + }, "refreshstate-first-step": { testStep: TestStep{ RefreshState: true,