From c7669b494055c55ce7322da35d1773e41d80f4a1 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 27 Feb 2023 16:14:47 -0500 Subject: [PATCH 01/31] initial implementation w/ tests --- helper/resource/diff_change_type.go | 18 + helper/resource/testing.go | 3 + helper/resource/testing_new_config.go | 77 +++ helper/resource/teststep_diffchanges_test.go | 499 +++++++++++++++++++ 4 files changed, 597 insertions(+) create mode 100644 helper/resource/diff_change_type.go create mode 100644 helper/resource/teststep_diffchanges_test.go diff --git a/helper/resource/diff_change_type.go b/helper/resource/diff_change_type.go new file mode 100644 index 000000000..d3978271b --- /dev/null +++ b/helper/resource/diff_change_type.go @@ -0,0 +1,18 @@ +package resource + +// TODO: is this a good name? +// TODO: document type and all aliases below +type DiffChangeType byte + +const ( + DiffInvalid DiffChangeType = iota + DiffNoop + DiffCreate + DiffRead + DiffUpdate + DiffDestroy + DiffDestroyBeforeCreate + DiffCreateBeforeDestroy + // TODO: document, this is more of a helper to detect either of the two above + DiffReplace +) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 0d479d8d6..ecda83dd3 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -510,6 +510,9 @@ type TestStep struct { // test to pass. ExpectError *regexp.Regexp + // TODO: document + ExpectedResourceChanges map[string]DiffChangeType + // PlanOnly can be set to only run `plan` with this configuration, and not // actually apply it. This is useful for ensuring config changes result in // no-op plans diff --git a/helper/resource/testing_new_config.go b/helper/resource/testing_new_config.go index 3ad3f0916..f441039e4 100644 --- a/helper/resource/testing_new_config.go +++ b/helper/resource/testing_new_config.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" + "github.com/hashicorp/go-multierror" tfjson "github.com/hashicorp/terraform-json" "github.com/mitchellh/go-testing-interface" @@ -51,6 +52,24 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error running pre-apply plan: %w", err) } + // If there are expectations for the planned diffs of resources, we assert those with the saved plan file + if len(step.ExpectedResourceChanges) > 0 { + 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 pre-apply plan: %w", err) + } + + err = assertExpectedResourceChanges(step, plan) + if err != nil { + return fmt.Errorf("Error asserting ExpectedResourceChanges: %w", err) + } + } + // We need to keep a copy of the state prior to destroying such // that the destroy steps can verify their behavior in the // check function @@ -243,3 +262,61 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return nil } + +// TODO: refactor this logic? +func assertExpectedResourceChanges(step TestStep, plan *tfjson.Plan) error { + var result *multierror.Error + for resource, expectedChange := range step.ExpectedResourceChanges { + foundResource := false + + for _, rc := range plan.ResourceChanges { + if resource == rc.Address { + switch expectedChange { + case DiffNoop: + if !rc.Change.Actions.NoOp() { + result = multierror.Append(result, fmt.Errorf("'%s' - expected NoOp, got action(s): %v", rc.Address, rc.Change.Actions)) + } + case DiffCreate: + if !rc.Change.Actions.Create() { + result = multierror.Append(result, fmt.Errorf("'%s' - expected Create, got action(s): %v", rc.Address, rc.Change.Actions)) + } + case DiffRead: + if !rc.Change.Actions.Read() { + result = multierror.Append(result, fmt.Errorf("'%s' - expected Read, got action(s): %v", rc.Address, rc.Change.Actions)) + } + case DiffUpdate: + if !rc.Change.Actions.Update() { + result = multierror.Append(result, fmt.Errorf("'%s' - expected Update, got action(s): %v", rc.Address, rc.Change.Actions)) + } + case DiffDestroy: + if !rc.Change.Actions.Delete() { + result = multierror.Append(result, fmt.Errorf("'%s' - expected Destroy, got action(s): %v", rc.Address, rc.Change.Actions)) + } + case DiffDestroyBeforeCreate: + if !rc.Change.Actions.DestroyBeforeCreate() { + result = multierror.Append(result, fmt.Errorf("'%s' - expected DestroyBeforeCreate, got action(s): %v", rc.Address, rc.Change.Actions)) + } + case DiffCreateBeforeDestroy: + if !rc.Change.Actions.CreateBeforeDestroy() { + result = multierror.Append(result, fmt.Errorf("'%s' - expected CreateBeforeDestroy, got action(s): %v", rc.Address, rc.Change.Actions)) + } + case DiffReplace: + if !rc.Change.Actions.Replace() { + result = multierror.Append(result, fmt.Errorf("%s - expected Replace, got action(s): %v", rc.Address, rc.Change.Actions)) + } + default: + result = multierror.Append(result, fmt.Errorf("%s - unexpected DiffChangeType byte: %d", rc.Address, expectedChange)) + } + + foundResource = true + break + } + } + + if !foundResource { + result = multierror.Append(result, fmt.Errorf("%s - Resource not found in planned ResourceChanges", resource)) + } + } + + return result.ErrorOrNil() +} diff --git a/helper/resource/teststep_diffchanges_test.go b/helper/resource/teststep_diffchanges_test.go new file mode 100644 index 000000000..ad82ea3a1 --- /dev/null +++ b/helper/resource/teststep_diffchanges_test.go @@ -0,0 +1,499 @@ +package resource + +import ( + "regexp" + "testing" +) + +func TestTest_TestStep_ExpectedResourceChanges_NoOp(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffNoop, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_NoOp_NoMatch(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffNoop, + }, + ExpectError: regexp.MustCompile(`expected NoOp, got action\(s\): \[create\]`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Create(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffCreate, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Create_NoMatch(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffCreate, + }, + ExpectError: regexp.MustCompile(`expected Create, got action\(s\): \[delete create\]`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Read(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + "null": { + Source: "registry.terraform.io/hashicorp/null", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 15 + } + + data "null_data_source" "two" { + inputs = { + unknown_val = random_string.one.result + } + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "data.null_data_source.two": DiffRead, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Read_NoMatch(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + "null": { + Source: "registry.terraform.io/hashicorp/null", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 15 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffRead, + }, + ExpectError: regexp.MustCompile(`expected Read, got action\(s\): \[create\]`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Update(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "time": { + Source: "registry.terraform.io/hashicorp/time", + }, + }, + Steps: []TestStep{ + { + Config: `resource "time_offset" "one" { + offset_days = 1 + }`, + }, + { + Config: `resource "time_offset" "one" { + offset_days = 2 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "time_offset.one": DiffUpdate, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Update_NoMatch(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "time": { + Source: "registry.terraform.io/hashicorp/time", + }, + }, + Steps: []TestStep{ + { + Config: `resource "time_offset" "one" { + offset_days = 1 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "time_offset.one": DiffUpdate, + }, + ExpectError: regexp.MustCompile(`expected Update, got action\(s\): \[create\]`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Destroy(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: ` `, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffDestroy, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Destroy_NoMatch(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffDestroy, + }, + ExpectError: regexp.MustCompile(`expected Destroy, got action\(s\): \[create\]`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_DestroyBeforeCreate(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffDestroyBeforeCreate, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_DestroyBeforeCreate_NoMatch(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffDestroyBeforeCreate, + }, + ExpectError: regexp.MustCompile(`expected DestroyBeforeCreate, got action\(s\): \[create\]`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_CreateBeforeDestroy(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + lifecycle { + create_before_destroy = true + } + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + lifecycle { + create_before_destroy = true + } + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffCreateBeforeDestroy, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_CreateBeforeDestroy_NoMatch(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffCreateBeforeDestroy, + }, + ExpectError: regexp.MustCompile(`expected CreateBeforeDestroy, got action\(s\): \[create\]`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Replace(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + } + + resource "random_string" "two" { + length = 16 + lifecycle { + create_before_destroy = true + } + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + } + + resource "random_string" "two" { + length = 15 + lifecycle { + create_before_destroy = true + } + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffReplace, + "random_string.two": DiffReplace, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_Replace_NoMatch(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": DiffReplace, + }, + ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), + }, + { + Config: `resource "random_string" "two" { + length = 16 + lifecycle { + create_before_destroy = true + } + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.two": DiffReplace, + }, + ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_NoResourceFound(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.doesntexist": DiffCreate, + }, + ExpectError: regexp.MustCompile(`random_string.doesntexist - Resource not found in planned ResourceChanges`), + }, + }, + }) +} + +func TestTest_TestStep_ExpectedResourceChanges_InvalidDiffChangeType(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": 0, + }, + ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 0`), + }, + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ExpectedResourceChanges: map[string]DiffChangeType{ + "random_string.one": 9, + }, + ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 9`), + }, + }, + }) +} From 10de5f07b4baa375872f9c3c841245e7230c97d1 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 28 Feb 2023 11:38:47 -0500 Subject: [PATCH 02/31] initial pre-apply assertions for RFC --- helper/planassert/expect_resource_action.go | 79 +++ .../planassert/expect_resource_action_test.go | 533 ++++++++++++++++++ helper/planassert/resource_action.go | 18 + helper/resource/diff_change_type.go | 18 - helper/resource/planassert.go | 8 + helper/resource/planassert_test.go | 86 +++ helper/resource/testing.go | 2 +- helper/resource/testing_new_config.go | 63 +-- helper/resource/teststep_diffchanges_test.go | 499 ---------------- 9 files changed, 734 insertions(+), 572 deletions(-) create mode 100644 helper/planassert/expect_resource_action.go create mode 100644 helper/planassert/expect_resource_action_test.go create mode 100644 helper/planassert/resource_action.go delete mode 100644 helper/resource/diff_change_type.go create mode 100644 helper/resource/planassert.go create mode 100644 helper/resource/planassert_test.go delete mode 100644 helper/resource/teststep_diffchanges_test.go diff --git a/helper/planassert/expect_resource_action.go b/helper/planassert/expect_resource_action.go new file mode 100644 index 000000000..0e9d35c41 --- /dev/null +++ b/helper/planassert/expect_resource_action.go @@ -0,0 +1,79 @@ +package planassert + +import ( + "fmt" + + tfjson "github.com/hashicorp/terraform-json" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +var _ resource.PlanAssert = expectResourceAction{} + +type expectResourceAction struct { + resourceName string + actionType ResourceActionType +} + +// RunAssert implements PlanAssert +// TODO: document +// TODO: refactor logic +func (e expectResourceAction) RunAssert(plan *tfjson.Plan) error { + foundResource := false + + for _, rc := range plan.ResourceChanges { + if e.resourceName == rc.Address { + switch e.actionType { + case ResourceActionNoop: + if !rc.Change.Actions.NoOp() { + return fmt.Errorf("'%s' - expected NoOp, got action(s): %v", rc.Address, rc.Change.Actions) + } + case ResourceActionCreate: + if !rc.Change.Actions.Create() { + return fmt.Errorf("'%s' - expected Create, got action(s): %v", rc.Address, rc.Change.Actions) + } + case ResourceActionRead: + if !rc.Change.Actions.Read() { + return fmt.Errorf("'%s' - expected Read, got action(s): %v", rc.Address, rc.Change.Actions) + } + case ResourceActionUpdate: + if !rc.Change.Actions.Update() { + return fmt.Errorf("'%s' - expected Update, got action(s): %v", rc.Address, rc.Change.Actions) + } + case ResourceActionDestroy: + if !rc.Change.Actions.Delete() { + return fmt.Errorf("'%s' - expected Destroy, got action(s): %v", rc.Address, rc.Change.Actions) + } + case ResourceActionDestroyBeforeCreate: + if !rc.Change.Actions.DestroyBeforeCreate() { + return fmt.Errorf("'%s' - expected DestroyBeforeCreate, got action(s): %v", rc.Address, rc.Change.Actions) + } + case ResourceActionCreateBeforeDestroy: + if !rc.Change.Actions.CreateBeforeDestroy() { + return fmt.Errorf("'%s' - expected CreateBeforeDestroy, got action(s): %v", rc.Address, rc.Change.Actions) + } + case ResourceActionReplace: + if !rc.Change.Actions.Replace() { + return fmt.Errorf("%s - expected Replace, got action(s): %v", rc.Address, rc.Change.Actions) + } + default: + return fmt.Errorf("%s - unexpected DiffChangeType byte: %d", rc.Address, e.actionType) + } + + foundResource = true + break + } + } + + if !foundResource { + return fmt.Errorf("%s - Resource not found in planned ResourceChanges", e.resourceName) + } + + return nil +} + +func ExpectResourceAction(resourceName string, actionType ResourceActionType) resource.PlanAssert { + return expectResourceAction{ + resourceName: resourceName, + actionType: actionType, + } +} diff --git a/helper/planassert/expect_resource_action_test.go b/helper/planassert/expect_resource_action_test.go new file mode 100644 index 000000000..88a92acdc --- /dev/null +++ b/helper/planassert/expect_resource_action_test.go @@ -0,0 +1,533 @@ +package planassert + +import ( + "regexp" + "testing" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +func Test_ExpectedResourceAction_NoOp(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionNoop), + }, + }, + }, + }) +} + +func Test_ExpectedResourceAction_NoOp_NoMatch(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionNoop), + }, + ExpectError: regexp.MustCompile(`expected NoOp, got action\(s\): \[create\]`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_Create(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionCreate), + }, + }, + }, + }) +} + +func Test_ExpectedResourceAction_Create_NoMatch(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionCreate), + }, + ExpectError: regexp.MustCompile(`expected Create, got action\(s\): \[delete create\]`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_Read(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + "null": { + Source: "registry.terraform.io/hashicorp/null", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 15 + } + + data "null_data_source" "two" { + inputs = { + unknown_val = random_string.one.result + } + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("data.null_data_source.two", ResourceActionRead), + }, + }, + }, + }) +} + +func Test_ExpectedResourceAction_Read_NoMatch(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + "null": { + Source: "registry.terraform.io/hashicorp/null", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 15 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionRead), + }, + ExpectError: regexp.MustCompile(`expected Read, got action\(s\): \[create\]`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_Update(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "time": { + Source: "registry.terraform.io/hashicorp/time", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "time_offset" "one" { + offset_days = 1 + }`, + }, + { + Config: `resource "time_offset" "one" { + offset_days = 2 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("time_offset.one", ResourceActionUpdate), + }, + }, + }, + }) +} + +func Test_ExpectedResourceAction_Update_NoMatch(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "time": { + Source: "registry.terraform.io/hashicorp/time", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "time_offset" "one" { + offset_days = 1 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("time_offset.one", ResourceActionUpdate), + }, + ExpectError: regexp.MustCompile(`expected Update, got action\(s\): \[create\]`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_Destroy(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: ` `, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionDestroy), + }, + }, + }, + }) +} + +func Test_ExpectedResourceAction_Destroy_NoMatch(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionDestroy), + }, + ExpectError: regexp.MustCompile(`expected Destroy, got action\(s\): \[create\]`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_DestroyBeforeCreate(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), + }, + }, + }, + }) +} + +func Test_ExpectedResourceAction_DestroyBeforeCreate_NoMatch(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), + }, + ExpectError: regexp.MustCompile(`expected DestroyBeforeCreate, got action\(s\): \[create\]`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_CreateBeforeDestroy(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + lifecycle { + create_before_destroy = true + } + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + lifecycle { + create_before_destroy = true + } + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), + }, + }, + }, + }) +} + +func Test_ExpectedResourceAction_CreateBeforeDestroy_NoMatch(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), + }, + ExpectError: regexp.MustCompile(`expected CreateBeforeDestroy, got action\(s\): \[create\]`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_Replace(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + } + + resource "random_string" "two" { + length = 16 + lifecycle { + create_before_destroy = true + } + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + } + + resource "random_string" "two" { + length = 15 + lifecycle { + create_before_destroy = true + } + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionReplace), + ExpectResourceAction("random_string.two", ResourceActionReplace), + }, + }, + }, + }) +} + +func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionReplace), + }, + ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), + }, + { + Config: `resource "random_string" "two" { + length = 16 + lifecycle { + create_before_destroy = true + } + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.two", ResourceActionReplace), + }, + ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_NoResourceFound(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.doesntexist", ResourceActionCreate), + }, + ExpectError: regexp.MustCompile(`random_string.doesntexist - Resource not found in planned ResourceChanges`), + }, + }, + }) +} + +func Test_ExpectedResourceAction_InvalidDiffChangeType(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", 0), + }, + ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 0`), + }, + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []r.PlanAssert{ + ExpectResourceAction("random_string.one", 9), + }, + ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 9`), + }, + }, + }) +} + +// TODO: for RFC +// PreApplyPlanAsserts: []r.PlanAssert{ +// planassert.ExpectResourceAction("random_string.one", ResourceActionReplace), +// planassert.ExpectResourceAction("random_string.two", ResourceActionReplace), +// }, +// PostApplyPlanAsserts: []PlanAsserts{ +// // planasserts -> new package? +// // Types could be functions that accept *tfjson.plan and return error +// // All asserts would run and aggregate into an error message +// // Could have PrePlanAsserts + PostPlanAsserts + SecondPostApplyPlanAsserts, all taking in an array of PlanAsserts +// planassert.ExpectResourceAction("random_string.one", planassert.ResourceActionReplace), +// planassert.ExpectResourceReplaceReason("random_string.one", tfjson.ReplaceBecauseCannotUpdate), +// planassert.ExpectResourceReplacePaths("random_string.one", "length"), +// planassert.ExpectResourceAction("random_string.two", planassert.ResourceActionReplace), +// planassert.ExpectEmptyPlan(), +// planassert.ExpectDrift("random_string.one"), +// planassert.ExpectNoDrift("random_string.two"), +// }, +// SecondPostApplyPlanAsserts: []PlanAsserts{ +// // planassert -> new package? +// // Types could be functions that accept *tfjson.plan and return error +// // All asserts would run and aggregate into an error message +// // Could have PrePlanAsserts + PostPlanAsserts, both taking in an array of PlanAsserts +// planassert.ExpectResourceAction("random_string.one", planassert.ResourceActionReplace), +// planassert.ExpectResourceReplaceReason("random_string.one", tfjson.ReplaceBecauseCannotUpdate), +// planassert.ExpectResourceReplacePaths("random_string.one", "length"), +// planassert.ExpectResourceAction("random_string.two", planassert.ResourceActionReplace), +// planassert.ExpectEmptyPlan(), +// planassert.ExpectDrift("random_string.one"), +// planassert.ExpectNoDrift("random_string.two"), +// }, diff --git a/helper/planassert/resource_action.go b/helper/planassert/resource_action.go new file mode 100644 index 000000000..83c9ff41d --- /dev/null +++ b/helper/planassert/resource_action.go @@ -0,0 +1,18 @@ +package planassert + +// TODO: is this a good name? +// TODO: document type and all aliases below +type ResourceActionType byte + +const ( + ResourceActionInvalid ResourceActionType = iota + ResourceActionNoop + ResourceActionCreate + ResourceActionRead + ResourceActionUpdate + ResourceActionDestroy + ResourceActionDestroyBeforeCreate + ResourceActionCreateBeforeDestroy + // TODO: document, this is more of a helper to detect either of the two above + ResourceActionReplace +) diff --git a/helper/resource/diff_change_type.go b/helper/resource/diff_change_type.go deleted file mode 100644 index d3978271b..000000000 --- a/helper/resource/diff_change_type.go +++ /dev/null @@ -1,18 +0,0 @@ -package resource - -// TODO: is this a good name? -// TODO: document type and all aliases below -type DiffChangeType byte - -const ( - DiffInvalid DiffChangeType = iota - DiffNoop - DiffCreate - DiffRead - DiffUpdate - DiffDestroy - DiffDestroyBeforeCreate - DiffCreateBeforeDestroy - // TODO: document, this is more of a helper to detect either of the two above - DiffReplace -) diff --git a/helper/resource/planassert.go b/helper/resource/planassert.go new file mode 100644 index 000000000..37254e80e --- /dev/null +++ b/helper/resource/planassert.go @@ -0,0 +1,8 @@ +package resource + +import tfjson "github.com/hashicorp/terraform-json" + +// TODO: document +type PlanAssert interface { + RunAssert(*tfjson.Plan) error +} diff --git a/helper/resource/planassert_test.go b/helper/resource/planassert_test.go new file mode 100644 index 000000000..e7fd3bc30 --- /dev/null +++ b/helper/resource/planassert_test.go @@ -0,0 +1,86 @@ +package resource + +import ( + "errors" + "regexp" + "testing" + + tfjson "github.com/hashicorp/terraform-json" +) + +var _ PlanAssert = &planAssertSpy{} + +type planAssertSpy struct { + err error + called bool +} + +func (f *planAssertSpy) RunAssert(_ *tfjson.Plan) error { + f.called = true + return f.err +} + +func Test_PreApplyPlanAssert_Called(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{} + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []PlanAssert{ + spy1, + spy2, + }, + }, + }, + }) + + if !spy1.called { + t.Error("expected PreApplyPlanAssert spy1 to be called at least once") + } + + if !spy2.called { + t.Error("expected PreApplyPlanAssert spy2 to be called at least once") + } +} + +func Test_PreApplyPlanAssert_Errors(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{ + err: errors.New("spy2 assert failed"), + } + spy3 := &planAssertSpy{ + err: errors.New("spy3 assert failed"), + } + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + PreApplyPlanAsserts: []PlanAssert{ + spy1, + spy2, + spy3, + }, + ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + }, + }, + }) +} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index ecda83dd3..8194a6e3e 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -511,7 +511,7 @@ type TestStep struct { ExpectError *regexp.Regexp // TODO: document - ExpectedResourceChanges map[string]DiffChangeType + PreApplyPlanAsserts []PlanAssert // PlanOnly can be set to only run `plan` with this configuration, and not // actually apply it. This is useful for ensuring config changes result in diff --git a/helper/resource/testing_new_config.go b/helper/resource/testing_new_config.go index f441039e4..6604c7348 100644 --- a/helper/resource/testing_new_config.go +++ b/helper/resource/testing_new_config.go @@ -52,8 +52,8 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error running pre-apply plan: %w", err) } - // If there are expectations for the planned diffs of resources, we assert those with the saved plan file - if len(step.ExpectedResourceChanges) > 0 { + // Run pre-apply plan assertions + if len(step.PreApplyPlanAsserts) > 0 { var plan *tfjson.Plan err = runProviderCommand(ctx, t, func() error { var err error @@ -64,9 +64,9 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error retrieving pre-apply plan: %w", err) } - err = assertExpectedResourceChanges(step, plan) + err = runPlanAssertions(plan, step.PreApplyPlanAsserts) if err != nil { - return fmt.Errorf("Error asserting ExpectedResourceChanges: %w", err) + return fmt.Errorf("Pre-apply Plan Assertion(s) failed: %w", err) } } @@ -263,58 +263,13 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return nil } -// TODO: refactor this logic? -func assertExpectedResourceChanges(step TestStep, plan *tfjson.Plan) error { +func runPlanAssertions(plan *tfjson.Plan, planAsserts []PlanAssert) error { var result *multierror.Error - for resource, expectedChange := range step.ExpectedResourceChanges { - foundResource := false - - for _, rc := range plan.ResourceChanges { - if resource == rc.Address { - switch expectedChange { - case DiffNoop: - if !rc.Change.Actions.NoOp() { - result = multierror.Append(result, fmt.Errorf("'%s' - expected NoOp, got action(s): %v", rc.Address, rc.Change.Actions)) - } - case DiffCreate: - if !rc.Change.Actions.Create() { - result = multierror.Append(result, fmt.Errorf("'%s' - expected Create, got action(s): %v", rc.Address, rc.Change.Actions)) - } - case DiffRead: - if !rc.Change.Actions.Read() { - result = multierror.Append(result, fmt.Errorf("'%s' - expected Read, got action(s): %v", rc.Address, rc.Change.Actions)) - } - case DiffUpdate: - if !rc.Change.Actions.Update() { - result = multierror.Append(result, fmt.Errorf("'%s' - expected Update, got action(s): %v", rc.Address, rc.Change.Actions)) - } - case DiffDestroy: - if !rc.Change.Actions.Delete() { - result = multierror.Append(result, fmt.Errorf("'%s' - expected Destroy, got action(s): %v", rc.Address, rc.Change.Actions)) - } - case DiffDestroyBeforeCreate: - if !rc.Change.Actions.DestroyBeforeCreate() { - result = multierror.Append(result, fmt.Errorf("'%s' - expected DestroyBeforeCreate, got action(s): %v", rc.Address, rc.Change.Actions)) - } - case DiffCreateBeforeDestroy: - if !rc.Change.Actions.CreateBeforeDestroy() { - result = multierror.Append(result, fmt.Errorf("'%s' - expected CreateBeforeDestroy, got action(s): %v", rc.Address, rc.Change.Actions)) - } - case DiffReplace: - if !rc.Change.Actions.Replace() { - result = multierror.Append(result, fmt.Errorf("%s - expected Replace, got action(s): %v", rc.Address, rc.Change.Actions)) - } - default: - result = multierror.Append(result, fmt.Errorf("%s - unexpected DiffChangeType byte: %d", rc.Address, expectedChange)) - } - - foundResource = true - break - } - } - if !foundResource { - result = multierror.Append(result, fmt.Errorf("%s - Resource not found in planned ResourceChanges", resource)) + for _, planAssert := range planAsserts { + err := planAssert.RunAssert(plan) + if err != nil { + result = multierror.Append(result, err) } } diff --git a/helper/resource/teststep_diffchanges_test.go b/helper/resource/teststep_diffchanges_test.go deleted file mode 100644 index ad82ea3a1..000000000 --- a/helper/resource/teststep_diffchanges_test.go +++ /dev/null @@ -1,499 +0,0 @@ -package resource - -import ( - "regexp" - "testing" -) - -func TestTest_TestStep_ExpectedResourceChanges_NoOp(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - }, - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffNoop, - }, - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_NoOp_NoMatch(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffNoop, - }, - ExpectError: regexp.MustCompile(`expected NoOp, got action\(s\): \[create\]`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Create(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffCreate, - }, - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Create_NoMatch(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - }, - { - Config: `resource "random_string" "one" { - length = 15 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffCreate, - }, - ExpectError: regexp.MustCompile(`expected Create, got action\(s\): \[delete create\]`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Read(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - "null": { - Source: "registry.terraform.io/hashicorp/null", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 15 - } - - data "null_data_source" "two" { - inputs = { - unknown_val = random_string.one.result - } - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "data.null_data_source.two": DiffRead, - }, - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Read_NoMatch(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - "null": { - Source: "registry.terraform.io/hashicorp/null", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 15 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffRead, - }, - ExpectError: regexp.MustCompile(`expected Read, got action\(s\): \[create\]`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Update(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "time": { - Source: "registry.terraform.io/hashicorp/time", - }, - }, - Steps: []TestStep{ - { - Config: `resource "time_offset" "one" { - offset_days = 1 - }`, - }, - { - Config: `resource "time_offset" "one" { - offset_days = 2 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "time_offset.one": DiffUpdate, - }, - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Update_NoMatch(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "time": { - Source: "registry.terraform.io/hashicorp/time", - }, - }, - Steps: []TestStep{ - { - Config: `resource "time_offset" "one" { - offset_days = 1 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "time_offset.one": DiffUpdate, - }, - ExpectError: regexp.MustCompile(`expected Update, got action\(s\): \[create\]`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Destroy(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - }, - { - Config: ` `, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffDestroy, - }, - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Destroy_NoMatch(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffDestroy, - }, - ExpectError: regexp.MustCompile(`expected Destroy, got action\(s\): \[create\]`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_DestroyBeforeCreate(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - }, - { - Config: `resource "random_string" "one" { - length = 15 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffDestroyBeforeCreate, - }, - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_DestroyBeforeCreate_NoMatch(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffDestroyBeforeCreate, - }, - ExpectError: regexp.MustCompile(`expected DestroyBeforeCreate, got action\(s\): \[create\]`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_CreateBeforeDestroy(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - lifecycle { - create_before_destroy = true - } - }`, - }, - { - Config: `resource "random_string" "one" { - length = 15 - lifecycle { - create_before_destroy = true - } - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffCreateBeforeDestroy, - }, - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_CreateBeforeDestroy_NoMatch(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffCreateBeforeDestroy, - }, - ExpectError: regexp.MustCompile(`expected CreateBeforeDestroy, got action\(s\): \[create\]`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Replace(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - } - - resource "random_string" "two" { - length = 16 - lifecycle { - create_before_destroy = true - } - }`, - }, - { - Config: `resource "random_string" "one" { - length = 15 - } - - resource "random_string" "two" { - length = 15 - lifecycle { - create_before_destroy = true - } - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffReplace, - "random_string.two": DiffReplace, - }, - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_Replace_NoMatch(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": DiffReplace, - }, - ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), - }, - { - Config: `resource "random_string" "two" { - length = 16 - lifecycle { - create_before_destroy = true - } - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.two": DiffReplace, - }, - ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_NoResourceFound(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.doesntexist": DiffCreate, - }, - ExpectError: regexp.MustCompile(`random_string.doesntexist - Resource not found in planned ResourceChanges`), - }, - }, - }) -} - -func TestTest_TestStep_ExpectedResourceChanges_InvalidDiffChangeType(t *testing.T) { - t.Parallel() - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": 0, - }, - ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 0`), - }, - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ExpectedResourceChanges: map[string]DiffChangeType{ - "random_string.one": 9, - }, - ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 9`), - }, - }, - }) -} From c096c30e5a678f12b35eee3a7231af25fb04b24a Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 1 Mar 2023 07:29:49 -0500 Subject: [PATCH 03/31] comment save --- helper/resource/testing.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 8194a6e3e..8f1f6ea24 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -513,6 +513,8 @@ type TestStep struct { // TODO: document PreApplyPlanAsserts []PlanAssert + // TODO: Add PostApplyPlanAsserts, SecondPostApplyPlanAsserts? + // PlanOnly can be set to only run `plan` with this configuration, and not // actually apply it. This is useful for ensuring config changes result in // no-op plans From 973cd57fcdb555da8a03faedca7fa250dd61d6a4 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 6 Mar 2023 17:54:18 -0500 Subject: [PATCH 04/31] RFC feedback, more impl --- .../planassert/expect_resource_action_test.go | 122 ++++++++++++------ helper/resource/planassert.go | 18 ++- helper/resource/planassert_test.go | 86 ------------ helper/resource/testing.go | 19 ++- helper/resource/testing_new_config.go | 36 +++--- helper/resource/testing_new_config_test.go | 87 +++++++++++++ helper/resource/testing_new_refresh_state.go | 12 +- .../testing_new_refresh_state_test.go | 3 + 8 files changed, 234 insertions(+), 149 deletions(-) delete mode 100644 helper/resource/planassert_test.go create mode 100644 helper/resource/testing_new_refresh_state_test.go diff --git a/helper/planassert/expect_resource_action_test.go b/helper/planassert/expect_resource_action_test.go index 88a92acdc..b82f65a3e 100644 --- a/helper/planassert/expect_resource_action_test.go +++ b/helper/planassert/expect_resource_action_test.go @@ -26,8 +26,10 @@ func Test_ExpectedResourceAction_NoOp(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionNoop), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionNoop), + }, }, }, }, @@ -48,8 +50,10 @@ func Test_ExpectedResourceAction_NoOp_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionNoop), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionNoop), + }, }, ExpectError: regexp.MustCompile(`expected NoOp, got action\(s\): \[create\]`), }, @@ -71,8 +75,10 @@ func Test_ExpectedResourceAction_Create(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionCreate), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionCreate), + }, }, }, }, @@ -98,8 +104,10 @@ func Test_ExpectedResourceAction_Create_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 15 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionCreate), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionCreate), + }, }, ExpectError: regexp.MustCompile(`expected Create, got action\(s\): \[delete create\]`), }, @@ -130,8 +138,10 @@ func Test_ExpectedResourceAction_Read(t *testing.T) { unknown_val = random_string.one.result } }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("data.null_data_source.two", ResourceActionRead), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("data.null_data_source.two", ResourceActionRead), + }, }, }, }, @@ -155,8 +165,10 @@ func Test_ExpectedResourceAction_Read_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 15 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionRead), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionRead), + }, }, ExpectError: regexp.MustCompile(`expected Read, got action\(s\): \[create\]`), }, @@ -183,8 +195,10 @@ func Test_ExpectedResourceAction_Update(t *testing.T) { Config: `resource "time_offset" "one" { offset_days = 2 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("time_offset.one", ResourceActionUpdate), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("time_offset.one", ResourceActionUpdate), + }, }, }, }, @@ -205,8 +219,10 @@ func Test_ExpectedResourceAction_Update_NoMatch(t *testing.T) { Config: `resource "time_offset" "one" { offset_days = 1 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("time_offset.one", ResourceActionUpdate), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("time_offset.one", ResourceActionUpdate), + }, }, ExpectError: regexp.MustCompile(`expected Update, got action\(s\): \[create\]`), }, @@ -231,8 +247,10 @@ func Test_ExpectedResourceAction_Destroy(t *testing.T) { }, { Config: ` `, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionDestroy), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionDestroy), + }, }, }, }, @@ -253,8 +271,10 @@ func Test_ExpectedResourceAction_Destroy_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionDestroy), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionDestroy), + }, }, ExpectError: regexp.MustCompile(`expected Destroy, got action\(s\): \[create\]`), }, @@ -281,8 +301,10 @@ func Test_ExpectedResourceAction_DestroyBeforeCreate(t *testing.T) { Config: `resource "random_string" "one" { length = 15 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), + }, }, }, }, @@ -303,8 +325,10 @@ func Test_ExpectedResourceAction_DestroyBeforeCreate_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), + }, }, ExpectError: regexp.MustCompile(`expected DestroyBeforeCreate, got action\(s\): \[create\]`), }, @@ -337,8 +361,10 @@ func Test_ExpectedResourceAction_CreateBeforeDestroy(t *testing.T) { create_before_destroy = true } }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), + }, }, }, }, @@ -359,8 +385,10 @@ func Test_ExpectedResourceAction_CreateBeforeDestroy_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), + }, }, ExpectError: regexp.MustCompile(`expected CreateBeforeDestroy, got action\(s\): \[create\]`), }, @@ -401,9 +429,11 @@ func Test_ExpectedResourceAction_Replace(t *testing.T) { create_before_destroy = true } }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionReplace), - ExpectResourceAction("random_string.two", ResourceActionReplace), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionReplace), + ExpectResourceAction("random_string.two", ResourceActionReplace), + }, }, }, }, @@ -424,8 +454,10 @@ func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", ResourceActionReplace), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", ResourceActionReplace), + }, }, ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), }, @@ -436,8 +468,10 @@ func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { create_before_destroy = true } }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.two", ResourceActionReplace), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.two", ResourceActionReplace), + }, }, ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), }, @@ -459,8 +493,10 @@ func Test_ExpectedResourceAction_NoResourceFound(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.doesntexist", ResourceActionCreate), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.doesntexist", ResourceActionCreate), + }, }, ExpectError: regexp.MustCompile(`random_string.doesntexist - Resource not found in planned ResourceChanges`), }, @@ -482,8 +518,10 @@ func Test_ExpectedResourceAction_InvalidDiffChangeType(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", 0), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", 0), + }, }, ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 0`), }, @@ -491,8 +529,10 @@ func Test_ExpectedResourceAction_InvalidDiffChangeType(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - PreApplyPlanAsserts: []r.PlanAssert{ - ExpectResourceAction("random_string.one", 9), + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectResourceAction("random_string.one", 9), + }, }, ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 9`), }, diff --git a/helper/resource/planassert.go b/helper/resource/planassert.go index 37254e80e..13c51586c 100644 --- a/helper/resource/planassert.go +++ b/helper/resource/planassert.go @@ -1,8 +1,24 @@ package resource -import tfjson "github.com/hashicorp/terraform-json" +import ( + "github.com/hashicorp/go-multierror" + tfjson "github.com/hashicorp/terraform-json" +) // TODO: document type PlanAssert interface { RunAssert(*tfjson.Plan) error } + +func runPlanAssertions(plan *tfjson.Plan, planAsserts []PlanAssert) error { + var result *multierror.Error + + for _, planAssert := range planAsserts { + err := planAssert.RunAssert(plan) + if err != nil { + result = multierror.Append(result, err) + } + } + + return result.ErrorOrNil() +} diff --git a/helper/resource/planassert_test.go b/helper/resource/planassert_test.go deleted file mode 100644 index e7fd3bc30..000000000 --- a/helper/resource/planassert_test.go +++ /dev/null @@ -1,86 +0,0 @@ -package resource - -import ( - "errors" - "regexp" - "testing" - - tfjson "github.com/hashicorp/terraform-json" -) - -var _ PlanAssert = &planAssertSpy{} - -type planAssertSpy struct { - err error - called bool -} - -func (f *planAssertSpy) RunAssert(_ *tfjson.Plan) error { - f.called = true - return f.err -} - -func Test_PreApplyPlanAssert_Called(t *testing.T) { - t.Parallel() - - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{} - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - PreApplyPlanAsserts: []PlanAssert{ - spy1, - spy2, - }, - }, - }, - }) - - if !spy1.called { - t.Error("expected PreApplyPlanAssert spy1 to be called at least once") - } - - if !spy2.called { - t.Error("expected PreApplyPlanAssert spy2 to be called at least once") - } -} - -func Test_PreApplyPlanAssert_Errors(t *testing.T) { - t.Parallel() - - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{ - err: errors.New("spy2 assert failed"), - } - spy3 := &planAssertSpy{ - err: errors.New("spy3 assert failed"), - } - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - PreApplyPlanAsserts: []PlanAssert{ - spy1, - spy2, - spy3, - }, - ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), - }, - }, - }) -} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 8f1f6ea24..12acc3e01 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -510,8 +510,9 @@ type TestStep struct { // test to pass. ExpectError *regexp.Regexp - // TODO: document - PreApplyPlanAsserts []PlanAssert + ConfigPlanAsserts ConfigPlanAsserts + + RefreshPlanAsserts RefreshPlanAsserts // TODO: Add PostApplyPlanAsserts, SecondPostApplyPlanAsserts? @@ -684,6 +685,20 @@ type TestStep struct { ExternalProviders map[string]ExternalProvider } +// TODO: document all fields / move to a different file/package? +type ConfigPlanAsserts struct { + PreApply []PlanAssert + + PostApplyPreRefresh []PlanAssert + + // TODO: should this be named 2nd post apply? Since refresh is not guaranteed + PostApplyPostRefresh []PlanAssert +} + +type RefreshPlanAsserts struct { + PostRefresh []PlanAssert +} + // ParallelTest performs an acceptance test on a resource, allowing concurrency // with other ParallelTest. The number of concurrent tests is controlled by the // "go test" command -parallel flag. diff --git a/helper/resource/testing_new_config.go b/helper/resource/testing_new_config.go index 6604c7348..4a6155ef7 100644 --- a/helper/resource/testing_new_config.go +++ b/helper/resource/testing_new_config.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" - "github.com/hashicorp/go-multierror" tfjson "github.com/hashicorp/terraform-json" "github.com/mitchellh/go-testing-interface" @@ -53,7 +52,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint } // Run pre-apply plan assertions - if len(step.PreApplyPlanAsserts) > 0 { + if len(step.ConfigPlanAsserts.PreApply) > 0 { var plan *tfjson.Plan err = runProviderCommand(ctx, t, func() error { var err error @@ -64,9 +63,9 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error retrieving pre-apply plan: %w", err) } - err = runPlanAssertions(plan, step.PreApplyPlanAsserts) + err = runPlanAssertions(plan, step.ConfigPlanAsserts.PreApply) if err != nil { - return fmt.Errorf("Pre-apply Plan Assertion(s) failed: %w", err) + return fmt.Errorf("Pre-apply plan assertion(s) failed: %w", err) } } @@ -150,6 +149,14 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error retrieving post-apply plan: %w", err) } + // Run post-apply, pre-refresh plan assertions + if len(step.ConfigPlanAsserts.PostApplyPreRefresh) > 0 { + err = runPlanAssertions(plan, step.ConfigPlanAsserts.PostApplyPreRefresh) + if err != nil { + return fmt.Errorf("Post-apply, pre-refresh plan assertion(s) failed: %w", err) + } + } + if !planIsEmpty(plan) && !step.ExpectNonEmptyPlan { var stdout string err = runProviderCommand(ctx, t, func() error { @@ -193,6 +200,14 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error retrieving second post-apply plan: %w", err) } + // Run post-apply, post-refresh plan assertions + if len(step.ConfigPlanAsserts.PostApplyPostRefresh) > 0 { + err = runPlanAssertions(plan, step.ConfigPlanAsserts.PostApplyPostRefresh) + if err != nil { + return fmt.Errorf("Post-apply, post-refresh plan assertion(s) failed: %w", err) + } + } + // check if plan is empty if !planIsEmpty(plan) && !step.ExpectNonEmptyPlan { var stdout string @@ -262,16 +277,3 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return nil } - -func runPlanAssertions(plan *tfjson.Plan, planAsserts []PlanAssert) error { - var result *multierror.Error - - for _, planAssert := range planAsserts { - err := planAssert.RunAssert(plan) - if err != nil { - result = multierror.Append(result, err) - } - } - - return result.ErrorOrNil() -} diff --git a/helper/resource/testing_new_config_test.go b/helper/resource/testing_new_config_test.go index 278cebf7e..50366b44e 100644 --- a/helper/resource/testing_new_config_test.go +++ b/helper/resource/testing_new_config_test.go @@ -1,10 +1,25 @@ package resource import ( + "errors" "regexp" "testing" + + tfjson "github.com/hashicorp/terraform-json" ) +var _ PlanAssert = &planAssertSpy{} + +type planAssertSpy struct { + err error + called bool +} + +func (f *planAssertSpy) RunAssert(_ *tfjson.Plan) error { + f.called = true + return f.err +} + func TestTest_TestStep_ExpectError_NewConfig(t *testing.T) { t.Parallel() @@ -26,3 +41,75 @@ func TestTest_TestStep_ExpectError_NewConfig(t *testing.T) { }, }) } + +func Test_ConfigPlanAsserts_PreApply_Called(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{} + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanAsserts: ConfigPlanAsserts{ + PreApply: []PlanAssert{ + spy1, + spy2, + }, + }, + }, + }, + }) + + if !spy1.called { + t.Error("expected ConfigPlanAsserts.PreApply spy1 to be called at least once") + } + + if !spy2.called { + t.Error("expected ConfigPlanAsserts.PreApply spy2 to be called at least once") + } +} + +func Test_ConfigPlanAsserts_PreApply_Errors(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{ + err: errors.New("spy2 assert failed"), + } + spy3 := &planAssertSpy{ + err: errors.New("spy3 assert failed"), + } + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanAsserts: ConfigPlanAsserts{ + PreApply: []PlanAssert{ + spy1, + spy2, + spy3, + }, + }, + ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + }, + }, + }) +} + +// TODO: add post apply, pre refresh plan assert tests +// TODO: add post apply, post refresh plan assert tests diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go index fe7b28145..dff348355 100644 --- a/helper/resource/testing_new_refresh_state.go +++ b/helper/resource/testing_new_refresh_state.go @@ -67,7 +67,7 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo return wd.CreatePlan(ctx) }, wd, providers) if err != nil { - return fmt.Errorf("Error running post-apply plan: %w", err) + return fmt.Errorf("Error running post-refresh plan: %w", err) } var plan *tfjson.Plan @@ -77,7 +77,15 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo return err }, wd, providers) if err != nil { - return fmt.Errorf("Error retrieving post-apply plan: %w", err) + return fmt.Errorf("Error retrieving post-refresh plan: %w", err) + } + + // Run post-refresh plan assertions + if len(step.RefreshPlanAsserts.PostRefresh) > 0 { + err = runPlanAssertions(plan, step.RefreshPlanAsserts.PostRefresh) + if err != nil { + return fmt.Errorf("Post-refresh plan assertion(s) failed: %w", err) + } } if !planIsEmpty(plan) && !step.ExpectNonEmptyPlan { diff --git a/helper/resource/testing_new_refresh_state_test.go b/helper/resource/testing_new_refresh_state_test.go new file mode 100644 index 000000000..f787ac9c5 --- /dev/null +++ b/helper/resource/testing_new_refresh_state_test.go @@ -0,0 +1,3 @@ +package resource + +// TODO: add post refresh plan assert tests From 3fbc68c6b733c29eae8f4278e1e5aeba1b4e0c2c Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 7 Mar 2023 16:33:14 -0500 Subject: [PATCH 05/31] added tests for each set of plan asserts --- helper/resource/planassert_test.go | 15 ++ helper/resource/testing_new_config_test.go | 153 ++++++++++++++++-- .../testing_new_refresh_state_test.go | 81 +++++++++- 3 files changed, 232 insertions(+), 17 deletions(-) create mode 100644 helper/resource/planassert_test.go diff --git a/helper/resource/planassert_test.go b/helper/resource/planassert_test.go new file mode 100644 index 000000000..c25c9e7bd --- /dev/null +++ b/helper/resource/planassert_test.go @@ -0,0 +1,15 @@ +package resource + +import tfjson "github.com/hashicorp/terraform-json" + +var _ PlanAssert = &planAssertSpy{} + +type planAssertSpy struct { + err error + called bool +} + +func (f *planAssertSpy) RunAssert(_ *tfjson.Plan) error { + f.called = true + return f.err +} diff --git a/helper/resource/testing_new_config_test.go b/helper/resource/testing_new_config_test.go index 50366b44e..892fa12aa 100644 --- a/helper/resource/testing_new_config_test.go +++ b/helper/resource/testing_new_config_test.go @@ -4,22 +4,8 @@ import ( "errors" "regexp" "testing" - - tfjson "github.com/hashicorp/terraform-json" ) -var _ PlanAssert = &planAssertSpy{} - -type planAssertSpy struct { - err error - called bool -} - -func (f *planAssertSpy) RunAssert(_ *tfjson.Plan) error { - f.called = true - return f.err -} - func TestTest_TestStep_ExpectError_NewConfig(t *testing.T) { t.Parallel() @@ -111,5 +97,140 @@ func Test_ConfigPlanAsserts_PreApply_Errors(t *testing.T) { }) } -// TODO: add post apply, pre refresh plan assert tests -// TODO: add post apply, post refresh plan assert tests +func Test_ConfigPlanAsserts_PostApplyPreRefresh_Called(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{} + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanAsserts: ConfigPlanAsserts{ + PostApplyPreRefresh: []PlanAssert{ + spy1, + spy2, + }, + }, + }, + }, + }) + + if !spy1.called { + t.Error("expected ConfigPlanAsserts.PostApplyPreRefresh spy1 to be called at least once") + } + + if !spy2.called { + t.Error("expected ConfigPlanAsserts.PostApplyPreRefresh spy2 to be called at least once") + } +} + +func Test_ConfigPlanAsserts_PostApplyPreRefresh_Errors(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{ + err: errors.New("spy2 assert failed"), + } + spy3 := &planAssertSpy{ + err: errors.New("spy3 assert failed"), + } + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanAsserts: ConfigPlanAsserts{ + PostApplyPreRefresh: []PlanAssert{ + spy1, + spy2, + spy3, + }, + }, + ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + }, + }, + }) +} + +func Test_ConfigPlanAsserts_PostApplyPostRefresh_Called(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{} + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanAsserts: ConfigPlanAsserts{ + PostApplyPostRefresh: []PlanAssert{ + spy1, + spy2, + }, + }, + }, + }, + }) + + if !spy1.called { + t.Error("expected ConfigPlanAsserts.PostApplyPostRefresh spy1 to be called at least once") + } + + if !spy2.called { + t.Error("expected ConfigPlanAsserts.PostApplyPostRefresh spy2 to be called at least once") + } +} + +func Test_ConfigPlanAsserts_PostApplyPostRefresh_Errors(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{ + err: errors.New("spy2 assert failed"), + } + spy3 := &planAssertSpy{ + err: errors.New("spy3 assert failed"), + } + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanAsserts: ConfigPlanAsserts{ + PostApplyPostRefresh: []PlanAssert{ + spy1, + spy2, + spy3, + }, + }, + ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + }, + }, + }) +} diff --git a/helper/resource/testing_new_refresh_state_test.go b/helper/resource/testing_new_refresh_state_test.go index f787ac9c5..492c677df 100644 --- a/helper/resource/testing_new_refresh_state_test.go +++ b/helper/resource/testing_new_refresh_state_test.go @@ -1,3 +1,82 @@ package resource -// TODO: add post refresh plan assert tests +import ( + "errors" + "regexp" + "testing" +) + +func Test_RefreshPlanAsserts_PostRefresh_Called(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{} + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + RefreshState: true, + RefreshPlanAsserts: RefreshPlanAsserts{ + PostRefresh: []PlanAssert{ + spy1, + spy2, + }, + }, + }, + }, + }) + + if !spy1.called { + t.Error("expected RefreshPlanAsserts.PostRefresh spy1 to be called at least once") + } + + if !spy2.called { + t.Error("expected RefreshPlanAsserts.PostRefresh spy2 to be called at least once") + } +} + +func Test_RefreshPlanAsserts_PostRefresh_Errors(t *testing.T) { + t.Parallel() + + spy1 := &planAssertSpy{} + spy2 := &planAssertSpy{ + err: errors.New("spy2 assert failed"), + } + spy3 := &planAssertSpy{ + err: errors.New("spy3 assert failed"), + } + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + RefreshState: true, + RefreshPlanAsserts: RefreshPlanAsserts{ + PostRefresh: []PlanAssert{ + spy1, + spy2, + spy3, + }, + }, + ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + }, + }, + }) +} From 6168af1769205ee80d1534a034c0e7f691521c2f Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 7 Mar 2023 16:55:30 -0500 Subject: [PATCH 06/31] add skeleton doc pages --- website/data/plugin-testing-nav-data.json | 4 ++++ .../testing/acceptance-tests/plan-assertions.mdx | 10 ++++++++++ 2 files changed, 14 insertions(+) create mode 100644 website/docs/plugin/testing/acceptance-tests/plan-assertions.mdx diff --git a/website/data/plugin-testing-nav-data.json b/website/data/plugin-testing-nav-data.json index f1b263d4c..fad3ef429 100644 --- a/website/data/plugin-testing-nav-data.json +++ b/website/data/plugin-testing-nav-data.json @@ -20,6 +20,10 @@ { "title": "Sweepers", "path": "acceptance-tests/sweepers" + }, + { + "title": "Plan Assertions", + "path": "acceptance-tests/plan-assertions" } ] }, diff --git a/website/docs/plugin/testing/acceptance-tests/plan-assertions.mdx b/website/docs/plugin/testing/acceptance-tests/plan-assertions.mdx new file mode 100644 index 000000000..1e53ce32c --- /dev/null +++ b/website/docs/plugin/testing/acceptance-tests/plan-assertions.mdx @@ -0,0 +1,10 @@ +--- +page_title: 'Plugin Development - Acceptance Testing: Plan Assertions' +description: >- + TBD +--- + +# Plan Assertions + +{/* TODO: Add documentation mentioned in RFC here */} + From 76bb8c21e278ab8e506b00a66f44739137654750 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 7 Mar 2023 17:22:41 -0500 Subject: [PATCH 07/31] small refactoring --- helper/planassert/expect_resource_action.go | 20 +++++----- .../planassert/expect_resource_action_test.go | 40 ++----------------- 2 files changed, 13 insertions(+), 47 deletions(-) diff --git a/helper/planassert/expect_resource_action.go b/helper/planassert/expect_resource_action.go index 0e9d35c41..e2c0ede3c 100644 --- a/helper/planassert/expect_resource_action.go +++ b/helper/planassert/expect_resource_action.go @@ -10,18 +10,15 @@ import ( var _ resource.PlanAssert = expectResourceAction{} type expectResourceAction struct { - resourceName string - actionType ResourceActionType + resourceAddress string + actionType ResourceActionType } -// RunAssert implements PlanAssert -// TODO: document -// TODO: refactor logic func (e expectResourceAction) RunAssert(plan *tfjson.Plan) error { foundResource := false for _, rc := range plan.ResourceChanges { - if e.resourceName == rc.Address { + if e.resourceAddress == rc.Address { switch e.actionType { case ResourceActionNoop: if !rc.Change.Actions.NoOp() { @@ -56,7 +53,7 @@ func (e expectResourceAction) RunAssert(plan *tfjson.Plan) error { return fmt.Errorf("%s - expected Replace, got action(s): %v", rc.Address, rc.Change.Actions) } default: - return fmt.Errorf("%s - unexpected DiffChangeType byte: %d", rc.Address, e.actionType) + return fmt.Errorf("%s - unexpected ResourceActionType byte: %d", rc.Address, e.actionType) } foundResource = true @@ -65,15 +62,16 @@ func (e expectResourceAction) RunAssert(plan *tfjson.Plan) error { } if !foundResource { - return fmt.Errorf("%s - Resource not found in planned ResourceChanges", e.resourceName) + return fmt.Errorf("%s - Resource not found in plan ResourceChanges", e.resourceAddress) } return nil } -func ExpectResourceAction(resourceName string, actionType ResourceActionType) resource.PlanAssert { +// TODO: document +func ExpectResourceAction(resourceAddress string, actionType ResourceActionType) resource.PlanAssert { return expectResourceAction{ - resourceName: resourceName, - actionType: actionType, + resourceAddress: resourceAddress, + actionType: actionType, } } diff --git a/helper/planassert/expect_resource_action_test.go b/helper/planassert/expect_resource_action_test.go index b82f65a3e..95aa76e3c 100644 --- a/helper/planassert/expect_resource_action_test.go +++ b/helper/planassert/expect_resource_action_test.go @@ -498,13 +498,13 @@ func Test_ExpectedResourceAction_NoResourceFound(t *testing.T) { ExpectResourceAction("random_string.doesntexist", ResourceActionCreate), }, }, - ExpectError: regexp.MustCompile(`random_string.doesntexist - Resource not found in planned ResourceChanges`), + ExpectError: regexp.MustCompile(`random_string.doesntexist - Resource not found in plan ResourceChanges`), }, }, }) } -func Test_ExpectedResourceAction_InvalidDiffChangeType(t *testing.T) { +func Test_ExpectedResourceAction_InvalidResourceActionType(t *testing.T) { t.Parallel() r.Test(t, r.TestCase{ @@ -523,7 +523,7 @@ func Test_ExpectedResourceAction_InvalidDiffChangeType(t *testing.T) { ExpectResourceAction("random_string.one", 0), }, }, - ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 0`), + ExpectError: regexp.MustCompile(`random_string.one - unexpected ResourceActionType byte: 0`), }, { Config: `resource "random_string" "one" { @@ -534,40 +534,8 @@ func Test_ExpectedResourceAction_InvalidDiffChangeType(t *testing.T) { ExpectResourceAction("random_string.one", 9), }, }, - ExpectError: regexp.MustCompile(`random_string.one - unexpected DiffChangeType byte: 9`), + ExpectError: regexp.MustCompile(`random_string.one - unexpected ResourceActionType byte: 9`), }, }, }) } - -// TODO: for RFC -// PreApplyPlanAsserts: []r.PlanAssert{ -// planassert.ExpectResourceAction("random_string.one", ResourceActionReplace), -// planassert.ExpectResourceAction("random_string.two", ResourceActionReplace), -// }, -// PostApplyPlanAsserts: []PlanAsserts{ -// // planasserts -> new package? -// // Types could be functions that accept *tfjson.plan and return error -// // All asserts would run and aggregate into an error message -// // Could have PrePlanAsserts + PostPlanAsserts + SecondPostApplyPlanAsserts, all taking in an array of PlanAsserts -// planassert.ExpectResourceAction("random_string.one", planassert.ResourceActionReplace), -// planassert.ExpectResourceReplaceReason("random_string.one", tfjson.ReplaceBecauseCannotUpdate), -// planassert.ExpectResourceReplacePaths("random_string.one", "length"), -// planassert.ExpectResourceAction("random_string.two", planassert.ResourceActionReplace), -// planassert.ExpectEmptyPlan(), -// planassert.ExpectDrift("random_string.one"), -// planassert.ExpectNoDrift("random_string.two"), -// }, -// SecondPostApplyPlanAsserts: []PlanAsserts{ -// // planassert -> new package? -// // Types could be functions that accept *tfjson.plan and return error -// // All asserts would run and aggregate into an error message -// // Could have PrePlanAsserts + PostPlanAsserts, both taking in an array of PlanAsserts -// planassert.ExpectResourceAction("random_string.one", planassert.ResourceActionReplace), -// planassert.ExpectResourceReplaceReason("random_string.one", tfjson.ReplaceBecauseCannotUpdate), -// planassert.ExpectResourceReplacePaths("random_string.one", "length"), -// planassert.ExpectResourceAction("random_string.two", planassert.ResourceActionReplace), -// planassert.ExpectEmptyPlan(), -// planassert.ExpectDrift("random_string.one"), -// planassert.ExpectNoDrift("random_string.two"), -// }, From 5311af5003d2e49cc8234ca1a603f66ee5f03094 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 7 Mar 2023 17:27:33 -0500 Subject: [PATCH 08/31] updated comments --- helper/resource/testing.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 12acc3e01..33819c887 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -510,12 +510,14 @@ type TestStep struct { // test to pass. ExpectError *regexp.Regexp + // TODO: is this naming good? + // TODO: document ConfigPlanAsserts ConfigPlanAsserts + // TODO: is this naming good? + // TODO: document RefreshPlanAsserts RefreshPlanAsserts - // TODO: Add PostApplyPlanAsserts, SecondPostApplyPlanAsserts? - // PlanOnly can be set to only run `plan` with this configuration, and not // actually apply it. This is useful for ensuring config changes result in // no-op plans @@ -695,6 +697,8 @@ type ConfigPlanAsserts struct { PostApplyPostRefresh []PlanAssert } +// TODO: is this naming good? +// TODO: document type RefreshPlanAsserts struct { PostRefresh []PlanAssert } From d0720743b69a2e81bb2d8d7703f3a0e1e7325abd Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 9 Mar 2023 10:42:35 -0500 Subject: [PATCH 09/31] add expect empty + non-empty plan asserts --- helper/planassert/expect_empty_plan.go | 30 +++++++ helper/planassert/expect_empty_plan_test.go | 79 +++++++++++++++++++ helper/planassert/expect_non_empty_plan.go | 27 +++++++ .../planassert/expect_non_empty_plan_test.go | 67 ++++++++++++++++ helper/resource/testing.go | 2 + 5 files changed, 205 insertions(+) create mode 100644 helper/planassert/expect_empty_plan.go create mode 100644 helper/planassert/expect_empty_plan_test.go create mode 100644 helper/planassert/expect_non_empty_plan.go create mode 100644 helper/planassert/expect_non_empty_plan_test.go diff --git a/helper/planassert/expect_empty_plan.go b/helper/planassert/expect_empty_plan.go new file mode 100644 index 000000000..741fb2f6e --- /dev/null +++ b/helper/planassert/expect_empty_plan.go @@ -0,0 +1,30 @@ +package planassert + +import ( + "fmt" + + "github.com/hashicorp/go-multierror" + tfjson "github.com/hashicorp/terraform-json" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +var _ resource.PlanAssert = expectEmptyPlan{} + +type expectEmptyPlan struct{} + +func (e expectEmptyPlan) RunAssert(plan *tfjson.Plan) error { + var result *multierror.Error + + for _, rc := range plan.ResourceChanges { + if !rc.Change.Actions.NoOp() { + result = multierror.Append(result, fmt.Errorf("expected empty plan, but %s has planned action(s): %v", rc.Address, rc.Change.Actions)) + } + } + + return result.ErrorOrNil() +} + +// TODO: document +func ExpectEmptyPlan() resource.PlanAssert { + return expectEmptyPlan{} +} diff --git a/helper/planassert/expect_empty_plan_test.go b/helper/planassert/expect_empty_plan_test.go new file mode 100644 index 000000000..e4d9035eb --- /dev/null +++ b/helper/planassert/expect_empty_plan_test.go @@ -0,0 +1,79 @@ +package planassert + +import ( + "regexp" + "testing" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +func Test_ExpectEmptyPlan(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectEmptyPlan(), + }, + }, + }, + }, + }) +} + +func Test_ExpectEmptyPlan_Error(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + } + resource "random_string" "two" { + length = 16 + } + resource "random_string" "three" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 12 + } + resource "random_string" "two" { + length = 16 + } + resource "random_string" "three" { + length = 12 + }`, + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectEmptyPlan(), + }, + }, + ExpectError: regexp.MustCompile(`.*?(random_string.one has planned action\(s\): \[delete create\])\n.*?(random_string.three has planned action\(s\): \[delete create\])`), + }, + }, + }) +} diff --git a/helper/planassert/expect_non_empty_plan.go b/helper/planassert/expect_non_empty_plan.go new file mode 100644 index 000000000..d80c043eb --- /dev/null +++ b/helper/planassert/expect_non_empty_plan.go @@ -0,0 +1,27 @@ +package planassert + +import ( + "errors" + + tfjson "github.com/hashicorp/terraform-json" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +var _ resource.PlanAssert = expectNonEmptyPlan{} + +type expectNonEmptyPlan struct{} + +func (e expectNonEmptyPlan) RunAssert(plan *tfjson.Plan) error { + for _, rc := range plan.ResourceChanges { + if !rc.Change.Actions.NoOp() { + return nil + } + } + + return errors.New("expected a non-empty plan, but got an empty plan") +} + +// TODO: document +func ExpectNonEmptyPlan() resource.PlanAssert { + return expectNonEmptyPlan{} +} diff --git a/helper/planassert/expect_non_empty_plan_test.go b/helper/planassert/expect_non_empty_plan_test.go new file mode 100644 index 000000000..0b269c0b4 --- /dev/null +++ b/helper/planassert/expect_non_empty_plan_test.go @@ -0,0 +1,67 @@ +package planassert + +import ( + "regexp" + "testing" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +func Test_ExpectNonEmptyPlan(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 12 + }`, + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectNonEmptyPlan(), + }, + }, + }, + }, + }) +} + +func Test_ExpectNonEmptyPlan_Error(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanAsserts: r.ConfigPlanAsserts{ + PreApply: []r.PlanAssert{ + ExpectNonEmptyPlan(), + }, + }, + ExpectError: regexp.MustCompile(`expected a non-empty plan, but got an empty plan`), + }, + }, + }) +} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 33819c887..efc108dcf 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -503,6 +503,8 @@ type TestStep struct { // ExpectNonEmptyPlan can be set to true for specific types of tests that are // looking to verify that a diff occurs + // TODO: deprecate and describe how to replace (ConfigPlanAsserts.PostApplyPostRefresh or RefreshPlanAsserts.PostRefresh) + // TODO: describe implicit empty plan behavior and how to replace? documentation? ExpectNonEmptyPlan bool // ExpectError allows the construction of test cases that we expect to fail From 4c37f36022af8c2335ff4d219da88b1aa44964c0 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 16 Mar 2023 14:53:31 -0400 Subject: [PATCH 10/31] refactored to name PlanCheck + follow req/resp model --- helper/planassert/expect_non_empty_plan.go | 27 ------ helper/planassert/expect_resource_action.go | 77 --------------- .../expect_empty_plan.go | 13 ++- .../expect_empty_plan_test.go | 10 +- helper/plancheck/expect_non_empty_plan.go | 26 +++++ .../expect_non_empty_plan_test.go | 10 +- helper/plancheck/expect_resource_action.go | 84 ++++++++++++++++ .../expect_resource_action_test.go | 82 ++++++++-------- .../resource_action.go | 2 +- helper/resource/plan_check.go | 43 +++++++++ helper/resource/plan_check_test.go | 13 +++ helper/resource/planassert.go | 24 ----- helper/resource/planassert_test.go | 15 --- helper/resource/testing.go | 18 ++-- helper/resource/testing_new_config.go | 24 ++--- helper/resource/testing_new_config_test.go | 96 +++++++++---------- helper/resource/testing_new_refresh_state.go | 8 +- .../testing_new_refresh_state_test.go | 32 +++---- website/data/plugin-testing-nav-data.json | 4 +- .../acceptance-tests/plan-assertions.mdx | 10 -- .../testing/acceptance-tests/plan-checks.mdx | 10 ++ 21 files changed, 325 insertions(+), 303 deletions(-) delete mode 100644 helper/planassert/expect_non_empty_plan.go delete mode 100644 helper/planassert/expect_resource_action.go rename helper/{planassert => plancheck}/expect_empty_plan.go (58%) rename helper/{planassert => plancheck}/expect_empty_plan_test.go (89%) create mode 100644 helper/plancheck/expect_non_empty_plan.go rename helper/{planassert => plancheck}/expect_non_empty_plan_test.go (86%) create mode 100644 helper/plancheck/expect_resource_action.go rename helper/{planassert => plancheck}/expect_resource_action_test.go (87%) rename helper/{planassert => plancheck}/resource_action.go (95%) create mode 100644 helper/resource/plan_check.go create mode 100644 helper/resource/plan_check_test.go delete mode 100644 helper/resource/planassert.go delete mode 100644 helper/resource/planassert_test.go delete mode 100644 website/docs/plugin/testing/acceptance-tests/plan-assertions.mdx create mode 100644 website/docs/plugin/testing/acceptance-tests/plan-checks.mdx diff --git a/helper/planassert/expect_non_empty_plan.go b/helper/planassert/expect_non_empty_plan.go deleted file mode 100644 index d80c043eb..000000000 --- a/helper/planassert/expect_non_empty_plan.go +++ /dev/null @@ -1,27 +0,0 @@ -package planassert - -import ( - "errors" - - tfjson "github.com/hashicorp/terraform-json" - "github.com/hashicorp/terraform-plugin-testing/helper/resource" -) - -var _ resource.PlanAssert = expectNonEmptyPlan{} - -type expectNonEmptyPlan struct{} - -func (e expectNonEmptyPlan) RunAssert(plan *tfjson.Plan) error { - for _, rc := range plan.ResourceChanges { - if !rc.Change.Actions.NoOp() { - return nil - } - } - - return errors.New("expected a non-empty plan, but got an empty plan") -} - -// TODO: document -func ExpectNonEmptyPlan() resource.PlanAssert { - return expectNonEmptyPlan{} -} diff --git a/helper/planassert/expect_resource_action.go b/helper/planassert/expect_resource_action.go deleted file mode 100644 index e2c0ede3c..000000000 --- a/helper/planassert/expect_resource_action.go +++ /dev/null @@ -1,77 +0,0 @@ -package planassert - -import ( - "fmt" - - tfjson "github.com/hashicorp/terraform-json" - "github.com/hashicorp/terraform-plugin-testing/helper/resource" -) - -var _ resource.PlanAssert = expectResourceAction{} - -type expectResourceAction struct { - resourceAddress string - actionType ResourceActionType -} - -func (e expectResourceAction) RunAssert(plan *tfjson.Plan) error { - foundResource := false - - for _, rc := range plan.ResourceChanges { - if e.resourceAddress == rc.Address { - switch e.actionType { - case ResourceActionNoop: - if !rc.Change.Actions.NoOp() { - return fmt.Errorf("'%s' - expected NoOp, got action(s): %v", rc.Address, rc.Change.Actions) - } - case ResourceActionCreate: - if !rc.Change.Actions.Create() { - return fmt.Errorf("'%s' - expected Create, got action(s): %v", rc.Address, rc.Change.Actions) - } - case ResourceActionRead: - if !rc.Change.Actions.Read() { - return fmt.Errorf("'%s' - expected Read, got action(s): %v", rc.Address, rc.Change.Actions) - } - case ResourceActionUpdate: - if !rc.Change.Actions.Update() { - return fmt.Errorf("'%s' - expected Update, got action(s): %v", rc.Address, rc.Change.Actions) - } - case ResourceActionDestroy: - if !rc.Change.Actions.Delete() { - return fmt.Errorf("'%s' - expected Destroy, got action(s): %v", rc.Address, rc.Change.Actions) - } - case ResourceActionDestroyBeforeCreate: - if !rc.Change.Actions.DestroyBeforeCreate() { - return fmt.Errorf("'%s' - expected DestroyBeforeCreate, got action(s): %v", rc.Address, rc.Change.Actions) - } - case ResourceActionCreateBeforeDestroy: - if !rc.Change.Actions.CreateBeforeDestroy() { - return fmt.Errorf("'%s' - expected CreateBeforeDestroy, got action(s): %v", rc.Address, rc.Change.Actions) - } - case ResourceActionReplace: - if !rc.Change.Actions.Replace() { - return fmt.Errorf("%s - expected Replace, got action(s): %v", rc.Address, rc.Change.Actions) - } - default: - return fmt.Errorf("%s - unexpected ResourceActionType byte: %d", rc.Address, e.actionType) - } - - foundResource = true - break - } - } - - if !foundResource { - return fmt.Errorf("%s - Resource not found in plan ResourceChanges", e.resourceAddress) - } - - return nil -} - -// TODO: document -func ExpectResourceAction(resourceAddress string, actionType ResourceActionType) resource.PlanAssert { - return expectResourceAction{ - resourceAddress: resourceAddress, - actionType: actionType, - } -} diff --git a/helper/planassert/expect_empty_plan.go b/helper/plancheck/expect_empty_plan.go similarity index 58% rename from helper/planassert/expect_empty_plan.go rename to helper/plancheck/expect_empty_plan.go index 741fb2f6e..530b59910 100644 --- a/helper/planassert/expect_empty_plan.go +++ b/helper/plancheck/expect_empty_plan.go @@ -1,30 +1,29 @@ -package planassert +package plancheck import ( "fmt" "github.com/hashicorp/go-multierror" - tfjson "github.com/hashicorp/terraform-json" "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) -var _ resource.PlanAssert = expectEmptyPlan{} +var _ resource.PlanCheck = expectEmptyPlan{} type expectEmptyPlan struct{} -func (e expectEmptyPlan) RunAssert(plan *tfjson.Plan) error { +func (e expectEmptyPlan) RunCheck(req resource.PlanCheckRequest, resp *resource.PlanCheckResponse) { var result *multierror.Error - for _, rc := range plan.ResourceChanges { + for _, rc := range req.Plan.ResourceChanges { if !rc.Change.Actions.NoOp() { result = multierror.Append(result, fmt.Errorf("expected empty plan, but %s has planned action(s): %v", rc.Address, rc.Change.Actions)) } } - return result.ErrorOrNil() + resp.Error = result.ErrorOrNil() } // TODO: document -func ExpectEmptyPlan() resource.PlanAssert { +func ExpectEmptyPlan() resource.PlanCheck { return expectEmptyPlan{} } diff --git a/helper/planassert/expect_empty_plan_test.go b/helper/plancheck/expect_empty_plan_test.go similarity index 89% rename from helper/planassert/expect_empty_plan_test.go rename to helper/plancheck/expect_empty_plan_test.go index e4d9035eb..caabcf121 100644 --- a/helper/planassert/expect_empty_plan_test.go +++ b/helper/plancheck/expect_empty_plan_test.go @@ -1,4 +1,4 @@ -package planassert +package plancheck import ( "regexp" @@ -26,8 +26,8 @@ func Test_ExpectEmptyPlan(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectEmptyPlan(), }, }, @@ -67,8 +67,8 @@ func Test_ExpectEmptyPlan_Error(t *testing.T) { resource "random_string" "three" { length = 12 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectEmptyPlan(), }, }, diff --git a/helper/plancheck/expect_non_empty_plan.go b/helper/plancheck/expect_non_empty_plan.go new file mode 100644 index 000000000..3d4961e72 --- /dev/null +++ b/helper/plancheck/expect_non_empty_plan.go @@ -0,0 +1,26 @@ +package plancheck + +import ( + "errors" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +var _ resource.PlanCheck = expectNonEmptyPlan{} + +type expectNonEmptyPlan struct{} + +func (e expectNonEmptyPlan) RunCheck(req resource.PlanCheckRequest, resp *resource.PlanCheckResponse) { + for _, rc := range req.Plan.ResourceChanges { + if !rc.Change.Actions.NoOp() { + return + } + } + + resp.Error = errors.New("expected a non-empty plan, but got an empty plan") +} + +// TODO: document +func ExpectNonEmptyPlan() resource.PlanCheck { + return expectNonEmptyPlan{} +} diff --git a/helper/planassert/expect_non_empty_plan_test.go b/helper/plancheck/expect_non_empty_plan_test.go similarity index 86% rename from helper/planassert/expect_non_empty_plan_test.go rename to helper/plancheck/expect_non_empty_plan_test.go index 0b269c0b4..644bffc54 100644 --- a/helper/planassert/expect_non_empty_plan_test.go +++ b/helper/plancheck/expect_non_empty_plan_test.go @@ -1,4 +1,4 @@ -package planassert +package plancheck import ( "regexp" @@ -26,8 +26,8 @@ func Test_ExpectNonEmptyPlan(t *testing.T) { Config: `resource "random_string" "one" { length = 12 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectNonEmptyPlan(), }, }, @@ -55,8 +55,8 @@ func Test_ExpectNonEmptyPlan_Error(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectNonEmptyPlan(), }, }, diff --git a/helper/plancheck/expect_resource_action.go b/helper/plancheck/expect_resource_action.go new file mode 100644 index 000000000..84a405c69 --- /dev/null +++ b/helper/plancheck/expect_resource_action.go @@ -0,0 +1,84 @@ +package plancheck + +import ( + "fmt" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +var _ resource.PlanCheck = expectResourceAction{} + +type expectResourceAction struct { + resourceAddress string + actionType ResourceActionType +} + +func (e expectResourceAction) RunCheck(req resource.PlanCheckRequest, resp *resource.PlanCheckResponse) { + foundResource := false + + for _, rc := range req.Plan.ResourceChanges { + if e.resourceAddress == rc.Address { + switch e.actionType { + case ResourceActionNoop: + if !rc.Change.Actions.NoOp() { + resp.Error = fmt.Errorf("'%s' - expected NoOp, got action(s): %v", rc.Address, rc.Change.Actions) + return + } + case ResourceActionCreate: + if !rc.Change.Actions.Create() { + resp.Error = fmt.Errorf("'%s' - expected Create, got action(s): %v", rc.Address, rc.Change.Actions) + return + } + case ResourceActionRead: + if !rc.Change.Actions.Read() { + resp.Error = fmt.Errorf("'%s' - expected Read, got action(s): %v", rc.Address, rc.Change.Actions) + return + } + case ResourceActionUpdate: + if !rc.Change.Actions.Update() { + resp.Error = fmt.Errorf("'%s' - expected Update, got action(s): %v", rc.Address, rc.Change.Actions) + return + } + case ResourceActionDestroy: + if !rc.Change.Actions.Delete() { + resp.Error = fmt.Errorf("'%s' - expected Destroy, got action(s): %v", rc.Address, rc.Change.Actions) + return + } + case ResourceActionDestroyBeforeCreate: + if !rc.Change.Actions.DestroyBeforeCreate() { + resp.Error = fmt.Errorf("'%s' - expected DestroyBeforeCreate, got action(s): %v", rc.Address, rc.Change.Actions) + return + } + case ResourceActionCreateBeforeDestroy: + if !rc.Change.Actions.CreateBeforeDestroy() { + resp.Error = fmt.Errorf("'%s' - expected CreateBeforeDestroy, got action(s): %v", rc.Address, rc.Change.Actions) + return + } + case ResourceActionReplace: + if !rc.Change.Actions.Replace() { + resp.Error = fmt.Errorf("%s - expected Replace, got action(s): %v", rc.Address, rc.Change.Actions) + return + } + default: + resp.Error = fmt.Errorf("%s - unexpected ResourceActionType byte: %d", rc.Address, e.actionType) + return + } + + foundResource = true + break + } + } + + if !foundResource { + resp.Error = fmt.Errorf("%s - Resource not found in plan ResourceChanges", e.resourceAddress) + return + } +} + +// TODO: document +func ExpectResourceAction(resourceAddress string, actionType ResourceActionType) resource.PlanCheck { + return expectResourceAction{ + resourceAddress: resourceAddress, + actionType: actionType, + } +} diff --git a/helper/planassert/expect_resource_action_test.go b/helper/plancheck/expect_resource_action_test.go similarity index 87% rename from helper/planassert/expect_resource_action_test.go rename to helper/plancheck/expect_resource_action_test.go index 95aa76e3c..e501ea578 100644 --- a/helper/planassert/expect_resource_action_test.go +++ b/helper/plancheck/expect_resource_action_test.go @@ -1,4 +1,4 @@ -package planassert +package plancheck import ( "regexp" @@ -26,8 +26,8 @@ func Test_ExpectedResourceAction_NoOp(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionNoop), }, }, @@ -50,8 +50,8 @@ func Test_ExpectedResourceAction_NoOp_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionNoop), }, }, @@ -75,8 +75,8 @@ func Test_ExpectedResourceAction_Create(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionCreate), }, }, @@ -104,8 +104,8 @@ func Test_ExpectedResourceAction_Create_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 15 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionCreate), }, }, @@ -138,8 +138,8 @@ func Test_ExpectedResourceAction_Read(t *testing.T) { unknown_val = random_string.one.result } }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("data.null_data_source.two", ResourceActionRead), }, }, @@ -165,8 +165,8 @@ func Test_ExpectedResourceAction_Read_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 15 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionRead), }, }, @@ -195,8 +195,8 @@ func Test_ExpectedResourceAction_Update(t *testing.T) { Config: `resource "time_offset" "one" { offset_days = 2 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("time_offset.one", ResourceActionUpdate), }, }, @@ -219,8 +219,8 @@ func Test_ExpectedResourceAction_Update_NoMatch(t *testing.T) { Config: `resource "time_offset" "one" { offset_days = 1 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("time_offset.one", ResourceActionUpdate), }, }, @@ -247,8 +247,8 @@ func Test_ExpectedResourceAction_Destroy(t *testing.T) { }, { Config: ` `, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionDestroy), }, }, @@ -271,8 +271,8 @@ func Test_ExpectedResourceAction_Destroy_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionDestroy), }, }, @@ -301,8 +301,8 @@ func Test_ExpectedResourceAction_DestroyBeforeCreate(t *testing.T) { Config: `resource "random_string" "one" { length = 15 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), }, }, @@ -325,8 +325,8 @@ func Test_ExpectedResourceAction_DestroyBeforeCreate_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), }, }, @@ -361,8 +361,8 @@ func Test_ExpectedResourceAction_CreateBeforeDestroy(t *testing.T) { create_before_destroy = true } }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), }, }, @@ -385,8 +385,8 @@ func Test_ExpectedResourceAction_CreateBeforeDestroy_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), }, }, @@ -429,8 +429,8 @@ func Test_ExpectedResourceAction_Replace(t *testing.T) { create_before_destroy = true } }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionReplace), ExpectResourceAction("random_string.two", ResourceActionReplace), }, @@ -454,8 +454,8 @@ func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", ResourceActionReplace), }, }, @@ -468,8 +468,8 @@ func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { create_before_destroy = true } }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.two", ResourceActionReplace), }, }, @@ -493,8 +493,8 @@ func Test_ExpectedResourceAction_NoResourceFound(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.doesntexist", ResourceActionCreate), }, }, @@ -518,8 +518,8 @@ func Test_ExpectedResourceAction_InvalidResourceActionType(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", 0), }, }, @@ -529,8 +529,8 @@ func Test_ExpectedResourceAction_InvalidResourceActionType(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: r.ConfigPlanAsserts{ - PreApply: []r.PlanAssert{ + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []r.PlanCheck{ ExpectResourceAction("random_string.one", 9), }, }, diff --git a/helper/planassert/resource_action.go b/helper/plancheck/resource_action.go similarity index 95% rename from helper/planassert/resource_action.go rename to helper/plancheck/resource_action.go index 83c9ff41d..f77c7fef4 100644 --- a/helper/planassert/resource_action.go +++ b/helper/plancheck/resource_action.go @@ -1,4 +1,4 @@ -package planassert +package plancheck // TODO: is this a good name? // TODO: document type and all aliases below diff --git a/helper/resource/plan_check.go b/helper/resource/plan_check.go new file mode 100644 index 000000000..afecadb4f --- /dev/null +++ b/helper/resource/plan_check.go @@ -0,0 +1,43 @@ +package resource + +import ( + "github.com/hashicorp/go-multierror" + tfjson "github.com/hashicorp/terraform-json" + "github.com/mitchellh/go-testing-interface" +) + +// TODO: document +type PlanCheck interface { + RunCheck(req PlanCheckRequest, resp *PlanCheckResponse) +} + +// TODO: document +type PlanCheckRequest struct { + Plan *tfjson.Plan +} + +// TODO: document +type PlanCheckResponse struct { + Error error + SkipTest bool +} + +func runPlanChecks(t testing.T, plan *tfjson.Plan, planChecks []PlanCheck) error { + t.Helper() + + var result *multierror.Error + + for _, planCheck := range planChecks { + resp := PlanCheckResponse{} + planCheck.RunCheck(PlanCheckRequest{Plan: plan}, &resp) + + if resp.SkipTest { + t.Skip("skipping test caused by plan check") + } + if resp.Error != nil { + result = multierror.Append(result, resp.Error) + } + } + + return result.ErrorOrNil() +} diff --git a/helper/resource/plan_check_test.go b/helper/resource/plan_check_test.go new file mode 100644 index 000000000..c96a51f9a --- /dev/null +++ b/helper/resource/plan_check_test.go @@ -0,0 +1,13 @@ +package resource + +var _ PlanCheck = &planCheckSpy{} + +type planCheckSpy struct { + err error + called bool +} + +func (s *planCheckSpy) RunCheck(req PlanCheckRequest, resp *PlanCheckResponse) { + s.called = true + resp.Error = s.err +} diff --git a/helper/resource/planassert.go b/helper/resource/planassert.go deleted file mode 100644 index 13c51586c..000000000 --- a/helper/resource/planassert.go +++ /dev/null @@ -1,24 +0,0 @@ -package resource - -import ( - "github.com/hashicorp/go-multierror" - tfjson "github.com/hashicorp/terraform-json" -) - -// TODO: document -type PlanAssert interface { - RunAssert(*tfjson.Plan) error -} - -func runPlanAssertions(plan *tfjson.Plan, planAsserts []PlanAssert) error { - var result *multierror.Error - - for _, planAssert := range planAsserts { - err := planAssert.RunAssert(plan) - if err != nil { - result = multierror.Append(result, err) - } - } - - return result.ErrorOrNil() -} diff --git a/helper/resource/planassert_test.go b/helper/resource/planassert_test.go deleted file mode 100644 index c25c9e7bd..000000000 --- a/helper/resource/planassert_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package resource - -import tfjson "github.com/hashicorp/terraform-json" - -var _ PlanAssert = &planAssertSpy{} - -type planAssertSpy struct { - err error - called bool -} - -func (f *planAssertSpy) RunAssert(_ *tfjson.Plan) error { - f.called = true - return f.err -} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index efc108dcf..d07e35e5e 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -503,7 +503,7 @@ type TestStep struct { // ExpectNonEmptyPlan can be set to true for specific types of tests that are // looking to verify that a diff occurs - // TODO: deprecate and describe how to replace (ConfigPlanAsserts.PostApplyPostRefresh or RefreshPlanAsserts.PostRefresh) + // TODO: deprecate and describe how to replace (ConfigPlanChecks.PostApplyPostRefresh or RefreshPlanChecks.PostRefresh) // TODO: describe implicit empty plan behavior and how to replace? documentation? ExpectNonEmptyPlan bool @@ -514,11 +514,11 @@ type TestStep struct { // TODO: is this naming good? // TODO: document - ConfigPlanAsserts ConfigPlanAsserts + ConfigPlanChecks ConfigPlanChecks // TODO: is this naming good? // TODO: document - RefreshPlanAsserts RefreshPlanAsserts + RefreshPlanChecks RefreshPlanChecks // PlanOnly can be set to only run `plan` with this configuration, and not // actually apply it. This is useful for ensuring config changes result in @@ -690,19 +690,19 @@ type TestStep struct { } // TODO: document all fields / move to a different file/package? -type ConfigPlanAsserts struct { - PreApply []PlanAssert +type ConfigPlanChecks struct { + PreApply []PlanCheck - PostApplyPreRefresh []PlanAssert + PostApplyPreRefresh []PlanCheck // TODO: should this be named 2nd post apply? Since refresh is not guaranteed - PostApplyPostRefresh []PlanAssert + PostApplyPostRefresh []PlanCheck } // TODO: is this naming good? // TODO: document -type RefreshPlanAsserts struct { - PostRefresh []PlanAssert +type RefreshPlanChecks struct { + PostRefresh []PlanCheck } // ParallelTest performs an acceptance test on a resource, allowing concurrency diff --git a/helper/resource/testing_new_config.go b/helper/resource/testing_new_config.go index 4a6155ef7..30dff134f 100644 --- a/helper/resource/testing_new_config.go +++ b/helper/resource/testing_new_config.go @@ -51,8 +51,8 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error running pre-apply plan: %w", err) } - // Run pre-apply plan assertions - if len(step.ConfigPlanAsserts.PreApply) > 0 { + // Run pre-apply plan checks + if len(step.ConfigPlanChecks.PreApply) > 0 { var plan *tfjson.Plan err = runProviderCommand(ctx, t, func() error { var err error @@ -63,9 +63,9 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error retrieving pre-apply plan: %w", err) } - err = runPlanAssertions(plan, step.ConfigPlanAsserts.PreApply) + err = runPlanChecks(t, plan, step.ConfigPlanChecks.PreApply) if err != nil { - return fmt.Errorf("Pre-apply plan assertion(s) failed: %w", err) + return fmt.Errorf("Pre-apply plan check(s) failed: %w", err) } } @@ -149,11 +149,11 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error retrieving post-apply plan: %w", err) } - // Run post-apply, pre-refresh plan assertions - if len(step.ConfigPlanAsserts.PostApplyPreRefresh) > 0 { - err = runPlanAssertions(plan, step.ConfigPlanAsserts.PostApplyPreRefresh) + // Run post-apply, pre-refresh plan checks + if len(step.ConfigPlanChecks.PostApplyPreRefresh) > 0 { + err = runPlanChecks(t, plan, step.ConfigPlanChecks.PostApplyPreRefresh) if err != nil { - return fmt.Errorf("Post-apply, pre-refresh plan assertion(s) failed: %w", err) + return fmt.Errorf("Post-apply, pre-refresh plan check(s) failed: %w", err) } } @@ -200,11 +200,11 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error retrieving second post-apply plan: %w", err) } - // Run post-apply, post-refresh plan assertions - if len(step.ConfigPlanAsserts.PostApplyPostRefresh) > 0 { - err = runPlanAssertions(plan, step.ConfigPlanAsserts.PostApplyPostRefresh) + // Run post-apply, post-refresh plan checks + if len(step.ConfigPlanChecks.PostApplyPostRefresh) > 0 { + err = runPlanChecks(t, plan, step.ConfigPlanChecks.PostApplyPostRefresh) if err != nil { - return fmt.Errorf("Post-apply, post-refresh plan assertion(s) failed: %w", err) + return fmt.Errorf("Post-apply, post-refresh plan check(s) failed: %w", err) } } diff --git a/helper/resource/testing_new_config_test.go b/helper/resource/testing_new_config_test.go index 892fa12aa..f4601f9f6 100644 --- a/helper/resource/testing_new_config_test.go +++ b/helper/resource/testing_new_config_test.go @@ -28,11 +28,11 @@ func TestTest_TestStep_ExpectError_NewConfig(t *testing.T) { }) } -func Test_ConfigPlanAsserts_PreApply_Called(t *testing.T) { +func Test_ConfigPlanChecks_PreApply_Called(t *testing.T) { t.Parallel() - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{} + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{} Test(t, TestCase{ ExternalProviders: map[string]ExternalProvider{ "random": { @@ -44,8 +44,8 @@ func Test_ConfigPlanAsserts_PreApply_Called(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: ConfigPlanAsserts{ - PreApply: []PlanAssert{ + ConfigPlanChecks: ConfigPlanChecks{ + PreApply: []PlanCheck{ spy1, spy2, }, @@ -55,23 +55,23 @@ func Test_ConfigPlanAsserts_PreApply_Called(t *testing.T) { }) if !spy1.called { - t.Error("expected ConfigPlanAsserts.PreApply spy1 to be called at least once") + t.Error("expected ConfigPlanChecks.PreApply spy1 to be called at least once") } if !spy2.called { - t.Error("expected ConfigPlanAsserts.PreApply spy2 to be called at least once") + t.Error("expected ConfigPlanChecks.PreApply spy2 to be called at least once") } } -func Test_ConfigPlanAsserts_PreApply_Errors(t *testing.T) { +func Test_ConfigPlanChecks_PreApply_Errors(t *testing.T) { t.Parallel() - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{ - err: errors.New("spy2 assert failed"), + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{ + err: errors.New("spy2 check failed"), } - spy3 := &planAssertSpy{ - err: errors.New("spy3 assert failed"), + spy3 := &planCheckSpy{ + err: errors.New("spy3 check failed"), } Test(t, TestCase{ ExternalProviders: map[string]ExternalProvider{ @@ -84,24 +84,24 @@ func Test_ConfigPlanAsserts_PreApply_Errors(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: ConfigPlanAsserts{ - PreApply: []PlanAssert{ + ConfigPlanChecks: ConfigPlanChecks{ + PreApply: []PlanCheck{ spy1, spy2, spy3, }, }, - ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + ExpectError: regexp.MustCompile(`.*?(spy2 check failed)\n.*?(spy3 check failed)`), }, }, }) } -func Test_ConfigPlanAsserts_PostApplyPreRefresh_Called(t *testing.T) { +func Test_ConfigPlanChecks_PostApplyPreRefresh_Called(t *testing.T) { t.Parallel() - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{} + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{} Test(t, TestCase{ ExternalProviders: map[string]ExternalProvider{ "random": { @@ -113,8 +113,8 @@ func Test_ConfigPlanAsserts_PostApplyPreRefresh_Called(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: ConfigPlanAsserts{ - PostApplyPreRefresh: []PlanAssert{ + ConfigPlanChecks: ConfigPlanChecks{ + PostApplyPreRefresh: []PlanCheck{ spy1, spy2, }, @@ -124,23 +124,23 @@ func Test_ConfigPlanAsserts_PostApplyPreRefresh_Called(t *testing.T) { }) if !spy1.called { - t.Error("expected ConfigPlanAsserts.PostApplyPreRefresh spy1 to be called at least once") + t.Error("expected ConfigPlanChecks.PostApplyPreRefresh spy1 to be called at least once") } if !spy2.called { - t.Error("expected ConfigPlanAsserts.PostApplyPreRefresh spy2 to be called at least once") + t.Error("expected ConfigPlanChecks.PostApplyPreRefresh spy2 to be called at least once") } } -func Test_ConfigPlanAsserts_PostApplyPreRefresh_Errors(t *testing.T) { +func Test_ConfigPlanChecks_PostApplyPreRefresh_Errors(t *testing.T) { t.Parallel() - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{ - err: errors.New("spy2 assert failed"), + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{ + err: errors.New("spy2 check failed"), } - spy3 := &planAssertSpy{ - err: errors.New("spy3 assert failed"), + spy3 := &planCheckSpy{ + err: errors.New("spy3 check failed"), } Test(t, TestCase{ ExternalProviders: map[string]ExternalProvider{ @@ -153,24 +153,24 @@ func Test_ConfigPlanAsserts_PostApplyPreRefresh_Errors(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: ConfigPlanAsserts{ - PostApplyPreRefresh: []PlanAssert{ + ConfigPlanChecks: ConfigPlanChecks{ + PostApplyPreRefresh: []PlanCheck{ spy1, spy2, spy3, }, }, - ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + ExpectError: regexp.MustCompile(`.*?(spy2 check failed)\n.*?(spy3 check failed)`), }, }, }) } -func Test_ConfigPlanAsserts_PostApplyPostRefresh_Called(t *testing.T) { +func Test_ConfigPlanChecks_PostApplyPostRefresh_Called(t *testing.T) { t.Parallel() - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{} + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{} Test(t, TestCase{ ExternalProviders: map[string]ExternalProvider{ "random": { @@ -182,8 +182,8 @@ func Test_ConfigPlanAsserts_PostApplyPostRefresh_Called(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: ConfigPlanAsserts{ - PostApplyPostRefresh: []PlanAssert{ + ConfigPlanChecks: ConfigPlanChecks{ + PostApplyPostRefresh: []PlanCheck{ spy1, spy2, }, @@ -193,23 +193,23 @@ func Test_ConfigPlanAsserts_PostApplyPostRefresh_Called(t *testing.T) { }) if !spy1.called { - t.Error("expected ConfigPlanAsserts.PostApplyPostRefresh spy1 to be called at least once") + t.Error("expected ConfigPlanChecks.PostApplyPostRefresh spy1 to be called at least once") } if !spy2.called { - t.Error("expected ConfigPlanAsserts.PostApplyPostRefresh spy2 to be called at least once") + t.Error("expected ConfigPlanChecks.PostApplyPostRefresh spy2 to be called at least once") } } -func Test_ConfigPlanAsserts_PostApplyPostRefresh_Errors(t *testing.T) { +func Test_ConfigPlanChecks_PostApplyPostRefresh_Errors(t *testing.T) { t.Parallel() - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{ - err: errors.New("spy2 assert failed"), + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{ + err: errors.New("spy2 check failed"), } - spy3 := &planAssertSpy{ - err: errors.New("spy3 assert failed"), + spy3 := &planCheckSpy{ + err: errors.New("spy3 check failed"), } Test(t, TestCase{ ExternalProviders: map[string]ExternalProvider{ @@ -222,14 +222,14 @@ func Test_ConfigPlanAsserts_PostApplyPostRefresh_Errors(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanAsserts: ConfigPlanAsserts{ - PostApplyPostRefresh: []PlanAssert{ + ConfigPlanChecks: ConfigPlanChecks{ + PostApplyPostRefresh: []PlanCheck{ spy1, spy2, spy3, }, }, - ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + ExpectError: regexp.MustCompile(`.*?(spy2 check failed)\n.*?(spy3 check failed)`), }, }, }) diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go index dff348355..134b51c31 100644 --- a/helper/resource/testing_new_refresh_state.go +++ b/helper/resource/testing_new_refresh_state.go @@ -80,11 +80,11 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo return fmt.Errorf("Error retrieving post-refresh plan: %w", err) } - // Run post-refresh plan assertions - if len(step.RefreshPlanAsserts.PostRefresh) > 0 { - err = runPlanAssertions(plan, step.RefreshPlanAsserts.PostRefresh) + // Run post-refresh plan checks + if len(step.RefreshPlanChecks.PostRefresh) > 0 { + err = runPlanChecks(t, plan, step.RefreshPlanChecks.PostRefresh) if err != nil { - return fmt.Errorf("Post-refresh plan assertion(s) failed: %w", err) + return fmt.Errorf("Post-refresh plan check(s) failed: %w", err) } } diff --git a/helper/resource/testing_new_refresh_state_test.go b/helper/resource/testing_new_refresh_state_test.go index 492c677df..3ff5b5190 100644 --- a/helper/resource/testing_new_refresh_state_test.go +++ b/helper/resource/testing_new_refresh_state_test.go @@ -6,11 +6,11 @@ import ( "testing" ) -func Test_RefreshPlanAsserts_PostRefresh_Called(t *testing.T) { +func Test_RefreshPlanChecks_PostRefresh_Called(t *testing.T) { t.Parallel() - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{} + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{} Test(t, TestCase{ ExternalProviders: map[string]ExternalProvider{ "random": { @@ -25,8 +25,8 @@ func Test_RefreshPlanAsserts_PostRefresh_Called(t *testing.T) { }, { RefreshState: true, - RefreshPlanAsserts: RefreshPlanAsserts{ - PostRefresh: []PlanAssert{ + RefreshPlanChecks: RefreshPlanChecks{ + PostRefresh: []PlanCheck{ spy1, spy2, }, @@ -36,23 +36,23 @@ func Test_RefreshPlanAsserts_PostRefresh_Called(t *testing.T) { }) if !spy1.called { - t.Error("expected RefreshPlanAsserts.PostRefresh spy1 to be called at least once") + t.Error("expected RefreshPlanChecks.PostRefresh spy1 to be called at least once") } if !spy2.called { - t.Error("expected RefreshPlanAsserts.PostRefresh spy2 to be called at least once") + t.Error("expected RefreshPlanChecks.PostRefresh spy2 to be called at least once") } } -func Test_RefreshPlanAsserts_PostRefresh_Errors(t *testing.T) { +func Test_RefreshPlanChecks_PostRefresh_Errors(t *testing.T) { t.Parallel() - spy1 := &planAssertSpy{} - spy2 := &planAssertSpy{ - err: errors.New("spy2 assert failed"), + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{ + err: errors.New("spy2 check failed"), } - spy3 := &planAssertSpy{ - err: errors.New("spy3 assert failed"), + spy3 := &planCheckSpy{ + err: errors.New("spy3 check failed"), } Test(t, TestCase{ ExternalProviders: map[string]ExternalProvider{ @@ -68,14 +68,14 @@ func Test_RefreshPlanAsserts_PostRefresh_Errors(t *testing.T) { }, { RefreshState: true, - RefreshPlanAsserts: RefreshPlanAsserts{ - PostRefresh: []PlanAssert{ + RefreshPlanChecks: RefreshPlanChecks{ + PostRefresh: []PlanCheck{ spy1, spy2, spy3, }, }, - ExpectError: regexp.MustCompile(`.*?(spy2 assert failed)\n.*?(spy3 assert failed)`), + ExpectError: regexp.MustCompile(`.*?(spy2 check failed)\n.*?(spy3 check failed)`), }, }, }) diff --git a/website/data/plugin-testing-nav-data.json b/website/data/plugin-testing-nav-data.json index fad3ef429..b425ef400 100644 --- a/website/data/plugin-testing-nav-data.json +++ b/website/data/plugin-testing-nav-data.json @@ -22,8 +22,8 @@ "path": "acceptance-tests/sweepers" }, { - "title": "Plan Assertions", - "path": "acceptance-tests/plan-assertions" + "title": "Plan Checks", + "path": "acceptance-tests/plan-checks" } ] }, diff --git a/website/docs/plugin/testing/acceptance-tests/plan-assertions.mdx b/website/docs/plugin/testing/acceptance-tests/plan-assertions.mdx deleted file mode 100644 index 1e53ce32c..000000000 --- a/website/docs/plugin/testing/acceptance-tests/plan-assertions.mdx +++ /dev/null @@ -1,10 +0,0 @@ ---- -page_title: 'Plugin Development - Acceptance Testing: Plan Assertions' -description: >- - TBD ---- - -# Plan Assertions - -{/* TODO: Add documentation mentioned in RFC here */} - diff --git a/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx b/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx new file mode 100644 index 000000000..7f3f46ef7 --- /dev/null +++ b/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx @@ -0,0 +1,10 @@ +--- +page_title: 'Plugin Development - Acceptance Testing: Plan Checks' +description: >- + TBD +--- + +# Plan Checks + +{/* TODO: Add documentation mentioned in RFC here */} + From 525012ef357efd824f965a070e585d4b5cae86cf Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Thu, 16 Mar 2023 15:54:41 -0400 Subject: [PATCH 11/31] added tests for skip functionality --- helper/resource/plan_check.go | 1 + helper/resource/plan_check_test.go | 2 + helper/resource/testing_new_config_test.go | 102 ++++++++++++++++++ .../testing_new_refresh_state_test.go | 36 +++++++ 4 files changed, 141 insertions(+) diff --git a/helper/resource/plan_check.go b/helper/resource/plan_check.go index afecadb4f..855dfb3c2 100644 --- a/helper/resource/plan_check.go +++ b/helper/resource/plan_check.go @@ -32,6 +32,7 @@ func runPlanChecks(t testing.T, plan *tfjson.Plan, planChecks []PlanCheck) error planCheck.RunCheck(PlanCheckRequest{Plan: plan}, &resp) if resp.SkipTest { + // TODO: better msg t.Skip("skipping test caused by plan check") } if resp.Error != nil { diff --git a/helper/resource/plan_check_test.go b/helper/resource/plan_check_test.go index c96a51f9a..0771ec2ea 100644 --- a/helper/resource/plan_check_test.go +++ b/helper/resource/plan_check_test.go @@ -4,10 +4,12 @@ var _ PlanCheck = &planCheckSpy{} type planCheckSpy struct { err error + skip bool called bool } func (s *planCheckSpy) RunCheck(req PlanCheckRequest, resp *PlanCheckResponse) { s.called = true + resp.SkipTest = s.skip resp.Error = s.err } diff --git a/helper/resource/testing_new_config_test.go b/helper/resource/testing_new_config_test.go index f4601f9f6..3a0bb7a30 100644 --- a/helper/resource/testing_new_config_test.go +++ b/helper/resource/testing_new_config_test.go @@ -97,6 +97,40 @@ func Test_ConfigPlanChecks_PreApply_Errors(t *testing.T) { }) } +func Test_ConfigPlanChecks_PreApply_Skipped(t *testing.T) { + t.Parallel() + + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{skip: true} + spy3 := &planCheckSpy{ + err: errors.New("spy3 check failed"), + } + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanChecks: ConfigPlanChecks{ + PreApply: []PlanCheck{ + spy1, + spy2, + spy3, + }, + }, + }, + }, + }) + + t.Fatal("expected spy2 check to skip test") +} + func Test_ConfigPlanChecks_PostApplyPreRefresh_Called(t *testing.T) { t.Parallel() @@ -166,6 +200,40 @@ func Test_ConfigPlanChecks_PostApplyPreRefresh_Errors(t *testing.T) { }) } +func Test_ConfigPlanChecks_PostApplyPreRefresh_Skipped(t *testing.T) { + t.Parallel() + + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{skip: true} + spy3 := &planCheckSpy{ + err: errors.New("spy3 check failed"), + } + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanChecks: ConfigPlanChecks{ + PostApplyPreRefresh: []PlanCheck{ + spy1, + spy2, + spy3, + }, + }, + }, + }, + }) + + t.Fatal("expected spy2 check to skip test") +} + func Test_ConfigPlanChecks_PostApplyPostRefresh_Called(t *testing.T) { t.Parallel() @@ -234,3 +302,37 @@ func Test_ConfigPlanChecks_PostApplyPostRefresh_Errors(t *testing.T) { }, }) } + +func Test_ConfigPlanChecks_PostApplyPostRefresh_Skipped(t *testing.T) { + t.Parallel() + + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{skip: true} + spy3 := &planCheckSpy{ + err: errors.New("spy3 check failed"), + } + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanChecks: ConfigPlanChecks{ + PostApplyPostRefresh: []PlanCheck{ + spy1, + spy2, + spy3, + }, + }, + }, + }, + }) + + t.Fatal("expected spy2 check to skip test") +} diff --git a/helper/resource/testing_new_refresh_state_test.go b/helper/resource/testing_new_refresh_state_test.go index 3ff5b5190..4e84b14fb 100644 --- a/helper/resource/testing_new_refresh_state_test.go +++ b/helper/resource/testing_new_refresh_state_test.go @@ -80,3 +80,39 @@ func Test_RefreshPlanChecks_PostRefresh_Errors(t *testing.T) { }, }) } + +func Test_RefreshPlanChecks_PostRefresh_Skipped(t *testing.T) { + t.Parallel() + + spy1 := &planCheckSpy{} + spy2 := &planCheckSpy{skip: true} + spy3 := &planCheckSpy{ + err: errors.New("spy3 check failed"), + } + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + RefreshState: true, + RefreshPlanChecks: RefreshPlanChecks{ + PostRefresh: []PlanCheck{ + spy1, + spy2, + spy3, + }, + }, + }, + }, + }) + + t.Fatal("expected spy2 check to skip test") +} From ebfe82569ccc0f1d82570b4a6af0b9c6f17ef25c Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 17 Mar 2023 15:53:04 -0400 Subject: [PATCH 12/31] moved plancheck package to root --- {helper/plancheck => plancheck}/expect_empty_plan.go | 0 {helper/plancheck => plancheck}/expect_empty_plan_test.go | 0 {helper/plancheck => plancheck}/expect_non_empty_plan.go | 0 {helper/plancheck => plancheck}/expect_non_empty_plan_test.go | 0 {helper/plancheck => plancheck}/expect_resource_action.go | 0 {helper/plancheck => plancheck}/expect_resource_action_test.go | 0 {helper/plancheck => plancheck}/resource_action.go | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename {helper/plancheck => plancheck}/expect_empty_plan.go (100%) rename {helper/plancheck => plancheck}/expect_empty_plan_test.go (100%) rename {helper/plancheck => plancheck}/expect_non_empty_plan.go (100%) rename {helper/plancheck => plancheck}/expect_non_empty_plan_test.go (100%) rename {helper/plancheck => plancheck}/expect_resource_action.go (100%) rename {helper/plancheck => plancheck}/expect_resource_action_test.go (100%) rename {helper/plancheck => plancheck}/resource_action.go (100%) diff --git a/helper/plancheck/expect_empty_plan.go b/plancheck/expect_empty_plan.go similarity index 100% rename from helper/plancheck/expect_empty_plan.go rename to plancheck/expect_empty_plan.go diff --git a/helper/plancheck/expect_empty_plan_test.go b/plancheck/expect_empty_plan_test.go similarity index 100% rename from helper/plancheck/expect_empty_plan_test.go rename to plancheck/expect_empty_plan_test.go diff --git a/helper/plancheck/expect_non_empty_plan.go b/plancheck/expect_non_empty_plan.go similarity index 100% rename from helper/plancheck/expect_non_empty_plan.go rename to plancheck/expect_non_empty_plan.go diff --git a/helper/plancheck/expect_non_empty_plan_test.go b/plancheck/expect_non_empty_plan_test.go similarity index 100% rename from helper/plancheck/expect_non_empty_plan_test.go rename to plancheck/expect_non_empty_plan_test.go diff --git a/helper/plancheck/expect_resource_action.go b/plancheck/expect_resource_action.go similarity index 100% rename from helper/plancheck/expect_resource_action.go rename to plancheck/expect_resource_action.go diff --git a/helper/plancheck/expect_resource_action_test.go b/plancheck/expect_resource_action_test.go similarity index 100% rename from helper/plancheck/expect_resource_action_test.go rename to plancheck/expect_resource_action_test.go diff --git a/helper/plancheck/resource_action.go b/plancheck/resource_action.go similarity index 100% rename from helper/plancheck/resource_action.go rename to plancheck/resource_action.go From 67a06f5cfa0b05e9a893d4e48a92dcce52525918 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 17 Mar 2023 15:56:49 -0400 Subject: [PATCH 13/31] moved plancheck tests to new testing pkg --- plancheck/expect_empty_plan_test.go | 7 ++-- plancheck/expect_non_empty_plan_test.go | 7 ++-- plancheck/expect_resource_action_test.go | 45 ++++++++++++------------ 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/plancheck/expect_empty_plan_test.go b/plancheck/expect_empty_plan_test.go index caabcf121..e547d1cf9 100644 --- a/plancheck/expect_empty_plan_test.go +++ b/plancheck/expect_empty_plan_test.go @@ -1,10 +1,11 @@ -package plancheck +package plancheck_test import ( "regexp" "testing" r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func Test_ExpectEmptyPlan(t *testing.T) { @@ -28,7 +29,7 @@ func Test_ExpectEmptyPlan(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectEmptyPlan(), + plancheck.ExpectEmptyPlan(), }, }, }, @@ -69,7 +70,7 @@ func Test_ExpectEmptyPlan_Error(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectEmptyPlan(), + plancheck.ExpectEmptyPlan(), }, }, ExpectError: regexp.MustCompile(`.*?(random_string.one has planned action\(s\): \[delete create\])\n.*?(random_string.three has planned action\(s\): \[delete create\])`), diff --git a/plancheck/expect_non_empty_plan_test.go b/plancheck/expect_non_empty_plan_test.go index 644bffc54..70b3738fd 100644 --- a/plancheck/expect_non_empty_plan_test.go +++ b/plancheck/expect_non_empty_plan_test.go @@ -1,10 +1,11 @@ -package plancheck +package plancheck_test import ( "regexp" "testing" r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func Test_ExpectNonEmptyPlan(t *testing.T) { @@ -28,7 +29,7 @@ func Test_ExpectNonEmptyPlan(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectNonEmptyPlan(), + plancheck.ExpectNonEmptyPlan(), }, }, }, @@ -57,7 +58,7 @@ func Test_ExpectNonEmptyPlan_Error(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectNonEmptyPlan(), + plancheck.ExpectNonEmptyPlan(), }, }, ExpectError: regexp.MustCompile(`expected a non-empty plan, but got an empty plan`), diff --git a/plancheck/expect_resource_action_test.go b/plancheck/expect_resource_action_test.go index e501ea578..e495ad0da 100644 --- a/plancheck/expect_resource_action_test.go +++ b/plancheck/expect_resource_action_test.go @@ -1,10 +1,11 @@ -package plancheck +package plancheck_test import ( "regexp" "testing" r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func Test_ExpectedResourceAction_NoOp(t *testing.T) { @@ -28,7 +29,7 @@ func Test_ExpectedResourceAction_NoOp(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionNoop), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionNoop), }, }, }, @@ -52,7 +53,7 @@ func Test_ExpectedResourceAction_NoOp_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionNoop), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionNoop), }, }, ExpectError: regexp.MustCompile(`expected NoOp, got action\(s\): \[create\]`), @@ -77,7 +78,7 @@ func Test_ExpectedResourceAction_Create(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionCreate), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionCreate), }, }, }, @@ -106,7 +107,7 @@ func Test_ExpectedResourceAction_Create_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionCreate), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionCreate), }, }, ExpectError: regexp.MustCompile(`expected Create, got action\(s\): \[delete create\]`), @@ -140,7 +141,7 @@ func Test_ExpectedResourceAction_Read(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("data.null_data_source.two", ResourceActionRead), + plancheck.ExpectResourceAction("data.null_data_source.two", plancheck.ResourceActionRead), }, }, }, @@ -167,7 +168,7 @@ func Test_ExpectedResourceAction_Read_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionRead), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionRead), }, }, ExpectError: regexp.MustCompile(`expected Read, got action\(s\): \[create\]`), @@ -197,7 +198,7 @@ func Test_ExpectedResourceAction_Update(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("time_offset.one", ResourceActionUpdate), + plancheck.ExpectResourceAction("time_offset.one", plancheck.ResourceActionUpdate), }, }, }, @@ -221,7 +222,7 @@ func Test_ExpectedResourceAction_Update_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("time_offset.one", ResourceActionUpdate), + plancheck.ExpectResourceAction("time_offset.one", plancheck.ResourceActionUpdate), }, }, ExpectError: regexp.MustCompile(`expected Update, got action\(s\): \[create\]`), @@ -249,7 +250,7 @@ func Test_ExpectedResourceAction_Destroy(t *testing.T) { Config: ` `, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionDestroy), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroy), }, }, }, @@ -273,7 +274,7 @@ func Test_ExpectedResourceAction_Destroy_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionDestroy), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroy), }, }, ExpectError: regexp.MustCompile(`expected Destroy, got action\(s\): \[create\]`), @@ -303,7 +304,7 @@ func Test_ExpectedResourceAction_DestroyBeforeCreate(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroyBeforeCreate), }, }, }, @@ -327,7 +328,7 @@ func Test_ExpectedResourceAction_DestroyBeforeCreate_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionDestroyBeforeCreate), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroyBeforeCreate), }, }, ExpectError: regexp.MustCompile(`expected DestroyBeforeCreate, got action\(s\): \[create\]`), @@ -363,7 +364,7 @@ func Test_ExpectedResourceAction_CreateBeforeDestroy(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionCreateBeforeDestroy), }, }, }, @@ -387,7 +388,7 @@ func Test_ExpectedResourceAction_CreateBeforeDestroy_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionCreateBeforeDestroy), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionCreateBeforeDestroy), }, }, ExpectError: regexp.MustCompile(`expected CreateBeforeDestroy, got action\(s\): \[create\]`), @@ -431,8 +432,8 @@ func Test_ExpectedResourceAction_Replace(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionReplace), - ExpectResourceAction("random_string.two", ResourceActionReplace), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionReplace), + plancheck.ExpectResourceAction("random_string.two", plancheck.ResourceActionReplace), }, }, }, @@ -456,7 +457,7 @@ func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", ResourceActionReplace), + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionReplace), }, }, ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), @@ -470,7 +471,7 @@ func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.two", ResourceActionReplace), + plancheck.ExpectResourceAction("random_string.two", plancheck.ResourceActionReplace), }, }, ExpectError: regexp.MustCompile(`expected Replace, got action\(s\): \[create\]`), @@ -495,7 +496,7 @@ func Test_ExpectedResourceAction_NoResourceFound(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.doesntexist", ResourceActionCreate), + plancheck.ExpectResourceAction("random_string.doesntexist", plancheck.ResourceActionCreate), }, }, ExpectError: regexp.MustCompile(`random_string.doesntexist - Resource not found in plan ResourceChanges`), @@ -520,7 +521,7 @@ func Test_ExpectedResourceAction_InvalidResourceActionType(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", 0), + plancheck.ExpectResourceAction("random_string.one", 0), }, }, ExpectError: regexp.MustCompile(`random_string.one - unexpected ResourceActionType byte: 0`), @@ -531,7 +532,7 @@ func Test_ExpectedResourceAction_InvalidResourceActionType(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []r.PlanCheck{ - ExpectResourceAction("random_string.one", 9), + plancheck.ExpectResourceAction("random_string.one", 9), }, }, ExpectError: regexp.MustCompile(`random_string.one - unexpected ResourceActionType byte: 9`), From bcef2c4ca095d0f3f6d68c88e081219621a93241 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 17 Mar 2023 16:57:52 -0400 Subject: [PATCH 14/31] added context to plancheck --- helper/resource/plan_check.go | 14 ++++++++------ helper/resource/plan_check_test.go | 4 +++- helper/resource/testing_new_config.go | 6 +++--- helper/resource/testing_new_refresh_state.go | 2 +- plancheck/expect_empty_plan.go | 3 ++- plancheck/expect_non_empty_plan.go | 3 ++- plancheck/expect_resource_action.go | 3 ++- 7 files changed, 21 insertions(+), 14 deletions(-) diff --git a/helper/resource/plan_check.go b/helper/resource/plan_check.go index 855dfb3c2..58dea9a6a 100644 --- a/helper/resource/plan_check.go +++ b/helper/resource/plan_check.go @@ -1,6 +1,8 @@ package resource import ( + "context" + "github.com/hashicorp/go-multierror" tfjson "github.com/hashicorp/terraform-json" "github.com/mitchellh/go-testing-interface" @@ -8,28 +10,28 @@ import ( // TODO: document type PlanCheck interface { - RunCheck(req PlanCheckRequest, resp *PlanCheckResponse) + CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) } // TODO: document -type PlanCheckRequest struct { +type CheckPlanRequest struct { Plan *tfjson.Plan } // TODO: document -type PlanCheckResponse struct { +type CheckPlanResponse struct { Error error SkipTest bool } -func runPlanChecks(t testing.T, plan *tfjson.Plan, planChecks []PlanCheck) error { +func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChecks []PlanCheck) error { t.Helper() var result *multierror.Error for _, planCheck := range planChecks { - resp := PlanCheckResponse{} - planCheck.RunCheck(PlanCheckRequest{Plan: plan}, &resp) + resp := CheckPlanResponse{} + planCheck.CheckPlan(ctx, CheckPlanRequest{Plan: plan}, &resp) if resp.SkipTest { // TODO: better msg diff --git a/helper/resource/plan_check_test.go b/helper/resource/plan_check_test.go index 0771ec2ea..8622ba4e2 100644 --- a/helper/resource/plan_check_test.go +++ b/helper/resource/plan_check_test.go @@ -1,5 +1,7 @@ package resource +import "context" + var _ PlanCheck = &planCheckSpy{} type planCheckSpy struct { @@ -8,7 +10,7 @@ type planCheckSpy struct { called bool } -func (s *planCheckSpy) RunCheck(req PlanCheckRequest, resp *PlanCheckResponse) { +func (s *planCheckSpy) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { s.called = true resp.SkipTest = s.skip resp.Error = s.err diff --git a/helper/resource/testing_new_config.go b/helper/resource/testing_new_config.go index 30dff134f..8aa29e4b2 100644 --- a/helper/resource/testing_new_config.go +++ b/helper/resource/testing_new_config.go @@ -63,7 +63,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return fmt.Errorf("Error retrieving pre-apply plan: %w", err) } - err = runPlanChecks(t, plan, step.ConfigPlanChecks.PreApply) + err = runPlanChecks(ctx, t, plan, step.ConfigPlanChecks.PreApply) if err != nil { return fmt.Errorf("Pre-apply plan check(s) failed: %w", err) } @@ -151,7 +151,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint // Run post-apply, pre-refresh plan checks if len(step.ConfigPlanChecks.PostApplyPreRefresh) > 0 { - err = runPlanChecks(t, plan, step.ConfigPlanChecks.PostApplyPreRefresh) + err = runPlanChecks(ctx, t, plan, step.ConfigPlanChecks.PostApplyPreRefresh) if err != nil { return fmt.Errorf("Post-apply, pre-refresh plan check(s) failed: %w", err) } @@ -202,7 +202,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint // Run post-apply, post-refresh plan checks if len(step.ConfigPlanChecks.PostApplyPostRefresh) > 0 { - err = runPlanChecks(t, plan, step.ConfigPlanChecks.PostApplyPostRefresh) + err = runPlanChecks(ctx, t, plan, step.ConfigPlanChecks.PostApplyPostRefresh) if err != nil { return fmt.Errorf("Post-apply, post-refresh plan check(s) failed: %w", err) } diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go index 134b51c31..e80051d2a 100644 --- a/helper/resource/testing_new_refresh_state.go +++ b/helper/resource/testing_new_refresh_state.go @@ -82,7 +82,7 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo // Run post-refresh plan checks if len(step.RefreshPlanChecks.PostRefresh) > 0 { - err = runPlanChecks(t, plan, step.RefreshPlanChecks.PostRefresh) + err = runPlanChecks(ctx, t, plan, step.RefreshPlanChecks.PostRefresh) if err != nil { return fmt.Errorf("Post-refresh plan check(s) failed: %w", err) } diff --git a/plancheck/expect_empty_plan.go b/plancheck/expect_empty_plan.go index 530b59910..23c22bdb8 100644 --- a/plancheck/expect_empty_plan.go +++ b/plancheck/expect_empty_plan.go @@ -1,6 +1,7 @@ package plancheck import ( + "context" "fmt" "github.com/hashicorp/go-multierror" @@ -11,7 +12,7 @@ var _ resource.PlanCheck = expectEmptyPlan{} type expectEmptyPlan struct{} -func (e expectEmptyPlan) RunCheck(req resource.PlanCheckRequest, resp *resource.PlanCheckResponse) { +func (e expectEmptyPlan) CheckPlan(ctx context.Context, req resource.CheckPlanRequest, resp *resource.CheckPlanResponse) { var result *multierror.Error for _, rc := range req.Plan.ResourceChanges { diff --git a/plancheck/expect_non_empty_plan.go b/plancheck/expect_non_empty_plan.go index 3d4961e72..e0ffcc53a 100644 --- a/plancheck/expect_non_empty_plan.go +++ b/plancheck/expect_non_empty_plan.go @@ -1,6 +1,7 @@ package plancheck import ( + "context" "errors" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -10,7 +11,7 @@ var _ resource.PlanCheck = expectNonEmptyPlan{} type expectNonEmptyPlan struct{} -func (e expectNonEmptyPlan) RunCheck(req resource.PlanCheckRequest, resp *resource.PlanCheckResponse) { +func (e expectNonEmptyPlan) CheckPlan(ctx context.Context, req resource.CheckPlanRequest, resp *resource.CheckPlanResponse) { for _, rc := range req.Plan.ResourceChanges { if !rc.Change.Actions.NoOp() { return diff --git a/plancheck/expect_resource_action.go b/plancheck/expect_resource_action.go index 84a405c69..8eb586a5a 100644 --- a/plancheck/expect_resource_action.go +++ b/plancheck/expect_resource_action.go @@ -1,6 +1,7 @@ package plancheck import ( + "context" "fmt" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -13,7 +14,7 @@ type expectResourceAction struct { actionType ResourceActionType } -func (e expectResourceAction) RunCheck(req resource.PlanCheckRequest, resp *resource.PlanCheckResponse) { +func (e expectResourceAction) CheckPlan(ctx context.Context, req resource.CheckPlanRequest, resp *resource.CheckPlanResponse) { foundResource := false for _, rc := range req.Plan.ResourceChanges { From 9246bb37accf5841ee70f02ee8a94a2752909f13 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Fri, 17 Mar 2023 17:47:37 -0400 Subject: [PATCH 15/31] replaced go-multierror with err func --- helper/resource/plan_check.go | 10 +++-- helper/resource/testing_new_config.go | 6 +-- helper/resource/testing_new_refresh_state.go | 2 +- internal/errorshim/error_join_shim.go | 44 ++++++++++++++++++++ plancheck/expect_empty_plan.go | 10 +++-- 5 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 internal/errorshim/error_join_shim.go diff --git a/helper/resource/plan_check.go b/helper/resource/plan_check.go index 58dea9a6a..4ce396fe1 100644 --- a/helper/resource/plan_check.go +++ b/helper/resource/plan_check.go @@ -3,8 +3,8 @@ package resource import ( "context" - "github.com/hashicorp/go-multierror" tfjson "github.com/hashicorp/terraform-json" + "github.com/hashicorp/terraform-plugin-testing/internal/errorshim" "github.com/mitchellh/go-testing-interface" ) @@ -27,7 +27,7 @@ type CheckPlanResponse struct { func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChecks []PlanCheck) error { t.Helper() - var result *multierror.Error + var result error for _, planCheck := range planChecks { resp := CheckPlanResponse{} @@ -38,9 +38,11 @@ func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChec t.Skip("skipping test caused by plan check") } if resp.Error != nil { - result = multierror.Append(result, resp.Error) + // TODO: Once Go 1.20 is the minimum supported version for this module, replace with `errors.Join` function + // - https://github.com/hashicorp/terraform-plugin-testing/issues/99 + result = errorshim.Join(result, resp.Error) } } - return result.ErrorOrNil() + return result } diff --git a/helper/resource/testing_new_config.go b/helper/resource/testing_new_config.go index 8aa29e4b2..add09bcc7 100644 --- a/helper/resource/testing_new_config.go +++ b/helper/resource/testing_new_config.go @@ -65,7 +65,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint err = runPlanChecks(ctx, t, plan, step.ConfigPlanChecks.PreApply) if err != nil { - return fmt.Errorf("Pre-apply plan check(s) failed: %w", err) + return fmt.Errorf("Pre-apply plan check(s) failed:\n%w", err) } } @@ -153,7 +153,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint if len(step.ConfigPlanChecks.PostApplyPreRefresh) > 0 { err = runPlanChecks(ctx, t, plan, step.ConfigPlanChecks.PostApplyPreRefresh) if err != nil { - return fmt.Errorf("Post-apply, pre-refresh plan check(s) failed: %w", err) + return fmt.Errorf("Post-apply, pre-refresh plan check(s) failed:\n%w", err) } } @@ -204,7 +204,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint if len(step.ConfigPlanChecks.PostApplyPostRefresh) > 0 { err = runPlanChecks(ctx, t, plan, step.ConfigPlanChecks.PostApplyPostRefresh) if err != nil { - return fmt.Errorf("Post-apply, post-refresh plan check(s) failed: %w", err) + return fmt.Errorf("Post-apply, post-refresh plan check(s) failed:\n%w", err) } } diff --git a/helper/resource/testing_new_refresh_state.go b/helper/resource/testing_new_refresh_state.go index e80051d2a..86073b165 100644 --- a/helper/resource/testing_new_refresh_state.go +++ b/helper/resource/testing_new_refresh_state.go @@ -84,7 +84,7 @@ func testStepNewRefreshState(ctx context.Context, t testing.T, wd *plugintest.Wo if len(step.RefreshPlanChecks.PostRefresh) > 0 { err = runPlanChecks(ctx, t, plan, step.RefreshPlanChecks.PostRefresh) if err != nil { - return fmt.Errorf("Post-refresh plan check(s) failed: %w", err) + return fmt.Errorf("Post-refresh plan check(s) failed:\n%w", err) } } diff --git a/internal/errorshim/error_join_shim.go b/internal/errorshim/error_join_shim.go new file mode 100644 index 000000000..06a6314fc --- /dev/null +++ b/internal/errorshim/error_join_shim.go @@ -0,0 +1,44 @@ +// TODO: Once Go 1.20 is the minimum supported version delete this package, replace all usages with `errors` package +// - https://github.com/hashicorp/terraform-plugin-testing/issues/99 +package errorshim + +// Copied from -> https://cs.opensource.google/go/go/+/refs/tags/go1.20.2:src/errors/join.go +func Join(errs ...error) error { + n := 0 + for _, err := range errs { + if err != nil { + n++ + } + } + if n == 0 { + return nil + } + e := &joinError{ + errs: make([]error, 0, n), + } + for _, err := range errs { + if err != nil { + e.errs = append(e.errs, err) + } + } + return e +} + +type joinError struct { + errs []error +} + +func (e *joinError) Error() string { + var b []byte + for i, err := range e.errs { + if i > 0 { + b = append(b, '\n') + } + b = append(b, err.Error()...) + } + return string(b) +} + +func (e *joinError) Unwrap() []error { + return e.errs +} diff --git a/plancheck/expect_empty_plan.go b/plancheck/expect_empty_plan.go index 23c22bdb8..22455c54d 100644 --- a/plancheck/expect_empty_plan.go +++ b/plancheck/expect_empty_plan.go @@ -4,8 +4,8 @@ import ( "context" "fmt" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/internal/errorshim" ) var _ resource.PlanCheck = expectEmptyPlan{} @@ -13,15 +13,17 @@ var _ resource.PlanCheck = expectEmptyPlan{} type expectEmptyPlan struct{} func (e expectEmptyPlan) CheckPlan(ctx context.Context, req resource.CheckPlanRequest, resp *resource.CheckPlanResponse) { - var result *multierror.Error + var result error for _, rc := range req.Plan.ResourceChanges { if !rc.Change.Actions.NoOp() { - result = multierror.Append(result, fmt.Errorf("expected empty plan, but %s has planned action(s): %v", rc.Address, rc.Change.Actions)) + // TODO: Once Go 1.20 is the minimum supported version for this module, replace with `errors.Join` function + // - https://github.com/hashicorp/terraform-plugin-testing/issues/99 + result = errorshim.Join(result, fmt.Errorf("expected empty plan, but %s has planned action(s): %v", rc.Address, rc.Change.Actions)) } } - resp.Error = result.ErrorOrNil() + resp.Error = result } // TODO: document From f1dd07cb04525a1a7c7d80ff57870f32f691eb78 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 20 Mar 2023 09:18:55 -0400 Subject: [PATCH 16/31] moved skip to string --- helper/resource/plan_check.go | 16 +++++++++++----- helper/resource/plan_check_test.go | 4 ++-- helper/resource/testing_new_config_test.go | 6 +++--- .../resource/testing_new_refresh_state_test.go | 2 +- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/helper/resource/plan_check.go b/helper/resource/plan_check.go index 4ce396fe1..a777cbd84 100644 --- a/helper/resource/plan_check.go +++ b/helper/resource/plan_check.go @@ -2,6 +2,7 @@ package resource import ( "context" + "fmt" tfjson "github.com/hashicorp/terraform-json" "github.com/hashicorp/terraform-plugin-testing/internal/errorshim" @@ -20,8 +21,14 @@ type CheckPlanRequest struct { // TODO: document type CheckPlanResponse struct { - Error error - SkipTest bool + Error error + + // Skip, if non-empty, immediately skips further TestStep and defines a message + // to be included with a call to (*testing.T).Skip(), which is visible in the test output. + // + // If a state has been applied via this TestStep or a prior TestStep, the testing will still + // invoke Terraform to destroy that state before finalizing the skipped test result. + Skip string } func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChecks []PlanCheck) error { @@ -33,9 +40,8 @@ func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChec resp := CheckPlanResponse{} planCheck.CheckPlan(ctx, CheckPlanRequest{Plan: plan}, &resp) - if resp.SkipTest { - // TODO: better msg - t.Skip("skipping test caused by plan check") + if resp.Skip != "" { + t.Skip(fmt.Sprintf("plan check forced test skip: %s", resp.Skip)) } if resp.Error != nil { // TODO: Once Go 1.20 is the minimum supported version for this module, replace with `errors.Join` function diff --git a/helper/resource/plan_check_test.go b/helper/resource/plan_check_test.go index 8622ba4e2..ea9c00834 100644 --- a/helper/resource/plan_check_test.go +++ b/helper/resource/plan_check_test.go @@ -6,12 +6,12 @@ var _ PlanCheck = &planCheckSpy{} type planCheckSpy struct { err error - skip bool + skip string called bool } func (s *planCheckSpy) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { s.called = true - resp.SkipTest = s.skip + resp.Skip = s.skip resp.Error = s.err } diff --git a/helper/resource/testing_new_config_test.go b/helper/resource/testing_new_config_test.go index 3a0bb7a30..059796ba4 100644 --- a/helper/resource/testing_new_config_test.go +++ b/helper/resource/testing_new_config_test.go @@ -101,7 +101,7 @@ func Test_ConfigPlanChecks_PreApply_Skipped(t *testing.T) { t.Parallel() spy1 := &planCheckSpy{} - spy2 := &planCheckSpy{skip: true} + spy2 := &planCheckSpy{skip: "skipping on purpose!"} spy3 := &planCheckSpy{ err: errors.New("spy3 check failed"), } @@ -204,7 +204,7 @@ func Test_ConfigPlanChecks_PostApplyPreRefresh_Skipped(t *testing.T) { t.Parallel() spy1 := &planCheckSpy{} - spy2 := &planCheckSpy{skip: true} + spy2 := &planCheckSpy{skip: "skipping on purpose!"} spy3 := &planCheckSpy{ err: errors.New("spy3 check failed"), } @@ -307,7 +307,7 @@ func Test_ConfigPlanChecks_PostApplyPostRefresh_Skipped(t *testing.T) { t.Parallel() spy1 := &planCheckSpy{} - spy2 := &planCheckSpy{skip: true} + spy2 := &planCheckSpy{skip: "skipping on purpose!"} spy3 := &planCheckSpy{ err: errors.New("spy3 check failed"), } diff --git a/helper/resource/testing_new_refresh_state_test.go b/helper/resource/testing_new_refresh_state_test.go index 4e84b14fb..826971f0d 100644 --- a/helper/resource/testing_new_refresh_state_test.go +++ b/helper/resource/testing_new_refresh_state_test.go @@ -85,7 +85,7 @@ func Test_RefreshPlanChecks_PostRefresh_Skipped(t *testing.T) { t.Parallel() spy1 := &planCheckSpy{} - spy2 := &planCheckSpy{skip: true} + spy2 := &planCheckSpy{skip: "skipping on purpose!"} spy3 := &planCheckSpy{ err: errors.New("spy3 check failed"), } From b671d414d1af280254e0756cf7bc03eb98f9079f Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 20 Mar 2023 09:30:02 -0400 Subject: [PATCH 17/31] remove skip message --- helper/resource/plan_check.go | 11 -- helper/resource/plan_check_test.go | 2 - helper/resource/testing_new_config_test.go | 102 ------------------ .../testing_new_refresh_state_test.go | 36 ------- 4 files changed, 151 deletions(-) diff --git a/helper/resource/plan_check.go b/helper/resource/plan_check.go index a777cbd84..8490651cd 100644 --- a/helper/resource/plan_check.go +++ b/helper/resource/plan_check.go @@ -2,7 +2,6 @@ package resource import ( "context" - "fmt" tfjson "github.com/hashicorp/terraform-json" "github.com/hashicorp/terraform-plugin-testing/internal/errorshim" @@ -22,13 +21,6 @@ type CheckPlanRequest struct { // TODO: document type CheckPlanResponse struct { Error error - - // Skip, if non-empty, immediately skips further TestStep and defines a message - // to be included with a call to (*testing.T).Skip(), which is visible in the test output. - // - // If a state has been applied via this TestStep or a prior TestStep, the testing will still - // invoke Terraform to destroy that state before finalizing the skipped test result. - Skip string } func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChecks []PlanCheck) error { @@ -40,9 +32,6 @@ func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChec resp := CheckPlanResponse{} planCheck.CheckPlan(ctx, CheckPlanRequest{Plan: plan}, &resp) - if resp.Skip != "" { - t.Skip(fmt.Sprintf("plan check forced test skip: %s", resp.Skip)) - } if resp.Error != nil { // TODO: Once Go 1.20 is the minimum supported version for this module, replace with `errors.Join` function // - https://github.com/hashicorp/terraform-plugin-testing/issues/99 diff --git a/helper/resource/plan_check_test.go b/helper/resource/plan_check_test.go index ea9c00834..f878cc6a2 100644 --- a/helper/resource/plan_check_test.go +++ b/helper/resource/plan_check_test.go @@ -6,12 +6,10 @@ var _ PlanCheck = &planCheckSpy{} type planCheckSpy struct { err error - skip string called bool } func (s *planCheckSpy) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { s.called = true - resp.Skip = s.skip resp.Error = s.err } diff --git a/helper/resource/testing_new_config_test.go b/helper/resource/testing_new_config_test.go index 059796ba4..f4601f9f6 100644 --- a/helper/resource/testing_new_config_test.go +++ b/helper/resource/testing_new_config_test.go @@ -97,40 +97,6 @@ func Test_ConfigPlanChecks_PreApply_Errors(t *testing.T) { }) } -func Test_ConfigPlanChecks_PreApply_Skipped(t *testing.T) { - t.Parallel() - - spy1 := &planCheckSpy{} - spy2 := &planCheckSpy{skip: "skipping on purpose!"} - spy3 := &planCheckSpy{ - err: errors.New("spy3 check failed"), - } - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ConfigPlanChecks: ConfigPlanChecks{ - PreApply: []PlanCheck{ - spy1, - spy2, - spy3, - }, - }, - }, - }, - }) - - t.Fatal("expected spy2 check to skip test") -} - func Test_ConfigPlanChecks_PostApplyPreRefresh_Called(t *testing.T) { t.Parallel() @@ -200,40 +166,6 @@ func Test_ConfigPlanChecks_PostApplyPreRefresh_Errors(t *testing.T) { }) } -func Test_ConfigPlanChecks_PostApplyPreRefresh_Skipped(t *testing.T) { - t.Parallel() - - spy1 := &planCheckSpy{} - spy2 := &planCheckSpy{skip: "skipping on purpose!"} - spy3 := &planCheckSpy{ - err: errors.New("spy3 check failed"), - } - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ConfigPlanChecks: ConfigPlanChecks{ - PostApplyPreRefresh: []PlanCheck{ - spy1, - spy2, - spy3, - }, - }, - }, - }, - }) - - t.Fatal("expected spy2 check to skip test") -} - func Test_ConfigPlanChecks_PostApplyPostRefresh_Called(t *testing.T) { t.Parallel() @@ -302,37 +234,3 @@ func Test_ConfigPlanChecks_PostApplyPostRefresh_Errors(t *testing.T) { }, }) } - -func Test_ConfigPlanChecks_PostApplyPostRefresh_Skipped(t *testing.T) { - t.Parallel() - - spy1 := &planCheckSpy{} - spy2 := &planCheckSpy{skip: "skipping on purpose!"} - spy3 := &planCheckSpy{ - err: errors.New("spy3 check failed"), - } - - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ConfigPlanChecks: ConfigPlanChecks{ - PostApplyPostRefresh: []PlanCheck{ - spy1, - spy2, - spy3, - }, - }, - }, - }, - }) - - t.Fatal("expected spy2 check to skip test") -} diff --git a/helper/resource/testing_new_refresh_state_test.go b/helper/resource/testing_new_refresh_state_test.go index 826971f0d..3ff5b5190 100644 --- a/helper/resource/testing_new_refresh_state_test.go +++ b/helper/resource/testing_new_refresh_state_test.go @@ -80,39 +80,3 @@ func Test_RefreshPlanChecks_PostRefresh_Errors(t *testing.T) { }, }) } - -func Test_RefreshPlanChecks_PostRefresh_Skipped(t *testing.T) { - t.Parallel() - - spy1 := &planCheckSpy{} - spy2 := &planCheckSpy{skip: "skipping on purpose!"} - spy3 := &planCheckSpy{ - err: errors.New("spy3 check failed"), - } - Test(t, TestCase{ - ExternalProviders: map[string]ExternalProvider{ - "random": { - Source: "registry.terraform.io/hashicorp/random", - }, - }, - Steps: []TestStep{ - { - Config: `resource "random_string" "one" { - length = 16 - }`, - }, - { - RefreshState: true, - RefreshPlanChecks: RefreshPlanChecks{ - PostRefresh: []PlanCheck{ - spy1, - spy2, - spy3, - }, - }, - }, - }, - }) - - t.Fatal("expected spy2 check to skip test") -} From d1be48ceffa1886b8a062e4158a4a4ee683e2862 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 20 Mar 2023 10:18:36 -0400 Subject: [PATCH 18/31] removed name params from interface --- helper/resource/plan_check.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper/resource/plan_check.go b/helper/resource/plan_check.go index 8490651cd..e84bd02f6 100644 --- a/helper/resource/plan_check.go +++ b/helper/resource/plan_check.go @@ -10,7 +10,7 @@ import ( // TODO: document type PlanCheck interface { - CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) + CheckPlan(context.Context, CheckPlanRequest, *CheckPlanResponse) } // TODO: document From 9959f286677e09cff28b195f9770f1171f1f42d5 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 20 Mar 2023 11:16:47 -0400 Subject: [PATCH 19/31] added validation steps --- helper/resource/teststep_validate.go | 35 +++++++++++++++ helper/resource/teststep_validate_test.go | 52 +++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/helper/resource/teststep_validate.go b/helper/resource/teststep_validate.go index 082d9e6dd..b1da44ad4 100644 --- a/helper/resource/teststep_validate.go +++ b/helper/resource/teststep_validate.go @@ -59,6 +59,9 @@ func (s TestStep) hasProviders(_ context.Context) bool { // - No overlapping ExternalProviders and ProviderFactories entries // - ResourceName is not empty when ImportState is true, ImportStateIdFunc // is not set, and ImportStateId is not set. +// - ConfigPlanChecks (PreApply, PostApplyPreRefresh, PostApplyPostRefresh) are only set when Config is set. +// - ConfigPlanChecks.PreApply are only set when PlanOnly is false. +// - RefreshPlanChecks (PostRefresh) are only set when RefreshState is set. func (s TestStep) validate(ctx context.Context, req testStepValidateRequest) error { ctx = logging.TestStepNumberContext(ctx, req.StepNumber) @@ -124,5 +127,37 @@ func (s TestStep) validate(ctx context.Context, req testStepValidateRequest) err } } + if len(s.ConfigPlanChecks.PreApply) > 0 { + if s.Config == "" { + err := fmt.Errorf("TestStep ConfigPlanChecks.PreApply must only be specified with Config") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if s.PlanOnly { + err := fmt.Errorf("TestStep ConfigPlanChecks.PreApply cannot be run with PlanOnly") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + } + + if len(s.ConfigPlanChecks.PostApplyPreRefresh) > 0 && s.Config == "" { + err := fmt.Errorf("TestStep ConfigPlanChecks.PostApplyPreRefresh must only be specified with Config") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if len(s.ConfigPlanChecks.PostApplyPostRefresh) > 0 && s.Config == "" { + err := fmt.Errorf("TestStep ConfigPlanChecks.PostApplyPostRefresh must only be specified with Config") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if len(s.RefreshPlanChecks.PostRefresh) > 0 && !s.RefreshState { + err := fmt.Errorf("TestStep RefreshPlanChecks.PostRefresh must only be specified with RefreshState") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + return nil } diff --git a/helper/resource/teststep_validate_test.go b/helper/resource/teststep_validate_test.go index 051da1d21..a807e7790 100644 --- a/helper/resource/teststep_validate_test.go +++ b/helper/resource/teststep_validate_test.go @@ -5,6 +5,7 @@ package resource import ( "context" + "errors" "fmt" "strings" "testing" @@ -190,6 +191,57 @@ func TestTestStepValidate(t *testing.T) { }, expectedError: fmt.Errorf("Providers must only be specified either at the TestCase or TestStep level"), }, + "configplanchecks-preapply-not-config-mode": { + testStep: TestStep{ + ConfigPlanChecks: ConfigPlanChecks{ + PreApply: []PlanCheck{&planCheckSpy{}}, + }, + RefreshState: true, + }, + testStepValidateRequest: testStepValidateRequest{TestCaseHasProviders: true}, + expectedError: errors.New("TestStep ConfigPlanChecks.PreApply must only be specified with Config"), + }, + "configplanchecks-preapply-not-planonly": { + testStep: TestStep{ + ConfigPlanChecks: ConfigPlanChecks{ + PreApply: []PlanCheck{&planCheckSpy{}}, + }, + Config: "# not empty", + PlanOnly: true, + }, + testStepValidateRequest: testStepValidateRequest{TestCaseHasProviders: true}, + expectedError: errors.New("TestStep ConfigPlanChecks.PreApply cannot be run with PlanOnly"), + }, + "configplanchecks-postapplyprerefresh-not-config-mode": { + testStep: TestStep{ + ConfigPlanChecks: ConfigPlanChecks{ + PostApplyPreRefresh: []PlanCheck{&planCheckSpy{}}, + }, + RefreshState: true, + }, + testStepValidateRequest: testStepValidateRequest{TestCaseHasProviders: true}, + expectedError: errors.New("TestStep ConfigPlanChecks.PostApplyPreRefresh must only be specified with Config"), + }, + "configplanchecks-postapplypostrefresh-not-config-mode": { + testStep: TestStep{ + ConfigPlanChecks: ConfigPlanChecks{ + PostApplyPostRefresh: []PlanCheck{&planCheckSpy{}}, + }, + RefreshState: true, + }, + testStepValidateRequest: testStepValidateRequest{TestCaseHasProviders: true}, + expectedError: errors.New("TestStep ConfigPlanChecks.PostApplyPostRefresh must only be specified with Config"), + }, + "refreshplanchecks-postrefresh-not-refresh-mode": { + testStep: TestStep{ + RefreshPlanChecks: RefreshPlanChecks{ + PostRefresh: []PlanCheck{&planCheckSpy{}}, + }, + Config: "# not empty", + }, + testStepValidateRequest: testStepValidateRequest{TestCaseHasProviders: true}, + expectedError: errors.New("TestStep RefreshPlanChecks.PostRefresh must only be specified with RefreshState"), + }, } for name, test := range tests { From db10c90f2f38a423ac839c2594c434efc3709c9b Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 20 Mar 2023 11:44:42 -0400 Subject: [PATCH 20/31] add doc for package --- plancheck/doc.go | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 plancheck/doc.go diff --git a/plancheck/doc.go b/plancheck/doc.go new file mode 100644 index 000000000..8f5f62e51 --- /dev/null +++ b/plancheck/doc.go @@ -0,0 +1,2 @@ +// TODO: write package documentation +package plancheck From 6c7287825291b32a8f1040de6eb606cc11f7bc80 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 20 Mar 2023 11:57:09 -0400 Subject: [PATCH 21/31] moved plancheck interface under plancheck --- helper/resource/plan_check_test.go | 15 ------- .../{plan_check.go => plan_checks.go} | 22 ++-------- helper/resource/plan_checks_test.go | 19 +++++++++ helper/resource/testing.go | 9 +++-- helper/resource/testing_new_config_test.go | 14 ++++--- .../testing_new_refresh_state_test.go | 6 ++- helper/resource/teststep_validate_test.go | 11 ++--- plancheck/expect_empty_plan.go | 7 ++-- plancheck/expect_empty_plan_test.go | 4 +- plancheck/expect_non_empty_plan.go | 8 ++-- plancheck/expect_non_empty_plan_test.go | 4 +- plancheck/expect_resource_action.go | 8 ++-- plancheck/expect_resource_action_test.go | 40 +++++++++---------- plancheck/plan_check.go | 22 ++++++++++ 14 files changed, 101 insertions(+), 88 deletions(-) delete mode 100644 helper/resource/plan_check_test.go rename helper/resource/{plan_check.go => plan_checks.go} (61%) create mode 100644 helper/resource/plan_checks_test.go create mode 100644 plancheck/plan_check.go diff --git a/helper/resource/plan_check_test.go b/helper/resource/plan_check_test.go deleted file mode 100644 index f878cc6a2..000000000 --- a/helper/resource/plan_check_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package resource - -import "context" - -var _ PlanCheck = &planCheckSpy{} - -type planCheckSpy struct { - err error - called bool -} - -func (s *planCheckSpy) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { - s.called = true - resp.Error = s.err -} diff --git a/helper/resource/plan_check.go b/helper/resource/plan_checks.go similarity index 61% rename from helper/resource/plan_check.go rename to helper/resource/plan_checks.go index e84bd02f6..0a74b0955 100644 --- a/helper/resource/plan_check.go +++ b/helper/resource/plan_checks.go @@ -5,32 +5,18 @@ import ( tfjson "github.com/hashicorp/terraform-json" "github.com/hashicorp/terraform-plugin-testing/internal/errorshim" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/mitchellh/go-testing-interface" ) -// TODO: document -type PlanCheck interface { - CheckPlan(context.Context, CheckPlanRequest, *CheckPlanResponse) -} - -// TODO: document -type CheckPlanRequest struct { - Plan *tfjson.Plan -} - -// TODO: document -type CheckPlanResponse struct { - Error error -} - -func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChecks []PlanCheck) error { +func runPlanChecks(ctx context.Context, t testing.T, plan *tfjson.Plan, planChecks []plancheck.PlanCheck) error { t.Helper() var result error for _, planCheck := range planChecks { - resp := CheckPlanResponse{} - planCheck.CheckPlan(ctx, CheckPlanRequest{Plan: plan}, &resp) + resp := plancheck.CheckPlanResponse{} + planCheck.CheckPlan(ctx, plancheck.CheckPlanRequest{Plan: plan}, &resp) if resp.Error != nil { // TODO: Once Go 1.20 is the minimum supported version for this module, replace with `errors.Join` function diff --git a/helper/resource/plan_checks_test.go b/helper/resource/plan_checks_test.go new file mode 100644 index 000000000..9e74e213f --- /dev/null +++ b/helper/resource/plan_checks_test.go @@ -0,0 +1,19 @@ +package resource + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-testing/plancheck" +) + +var _ plancheck.PlanCheck = &planCheckSpy{} + +type planCheckSpy struct { + err error + called bool +} + +func (s *planCheckSpy) CheckPlan(ctx context.Context, req plancheck.CheckPlanRequest, resp *plancheck.CheckPlanResponse) { + s.called = true + resp.Error = s.err +} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index d07e35e5e..a1c3df882 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -23,6 +23,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-plugin-testing/internal/addrs" @@ -691,18 +692,18 @@ type TestStep struct { // TODO: document all fields / move to a different file/package? type ConfigPlanChecks struct { - PreApply []PlanCheck + PreApply []plancheck.PlanCheck - PostApplyPreRefresh []PlanCheck + PostApplyPreRefresh []plancheck.PlanCheck // TODO: should this be named 2nd post apply? Since refresh is not guaranteed - PostApplyPostRefresh []PlanCheck + PostApplyPostRefresh []plancheck.PlanCheck } // TODO: is this naming good? // TODO: document type RefreshPlanChecks struct { - PostRefresh []PlanCheck + PostRefresh []plancheck.PlanCheck } // ParallelTest performs an acceptance test on a resource, allowing concurrency diff --git a/helper/resource/testing_new_config_test.go b/helper/resource/testing_new_config_test.go index f4601f9f6..3276b444b 100644 --- a/helper/resource/testing_new_config_test.go +++ b/helper/resource/testing_new_config_test.go @@ -4,6 +4,8 @@ import ( "errors" "regexp" "testing" + + "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func TestTest_TestStep_ExpectError_NewConfig(t *testing.T) { @@ -45,7 +47,7 @@ func Test_ConfigPlanChecks_PreApply_Called(t *testing.T) { length = 16 }`, ConfigPlanChecks: ConfigPlanChecks{ - PreApply: []PlanCheck{ + PreApply: []plancheck.PlanCheck{ spy1, spy2, }, @@ -85,7 +87,7 @@ func Test_ConfigPlanChecks_PreApply_Errors(t *testing.T) { length = 16 }`, ConfigPlanChecks: ConfigPlanChecks{ - PreApply: []PlanCheck{ + PreApply: []plancheck.PlanCheck{ spy1, spy2, spy3, @@ -114,7 +116,7 @@ func Test_ConfigPlanChecks_PostApplyPreRefresh_Called(t *testing.T) { length = 16 }`, ConfigPlanChecks: ConfigPlanChecks{ - PostApplyPreRefresh: []PlanCheck{ + PostApplyPreRefresh: []plancheck.PlanCheck{ spy1, spy2, }, @@ -154,7 +156,7 @@ func Test_ConfigPlanChecks_PostApplyPreRefresh_Errors(t *testing.T) { length = 16 }`, ConfigPlanChecks: ConfigPlanChecks{ - PostApplyPreRefresh: []PlanCheck{ + PostApplyPreRefresh: []plancheck.PlanCheck{ spy1, spy2, spy3, @@ -183,7 +185,7 @@ func Test_ConfigPlanChecks_PostApplyPostRefresh_Called(t *testing.T) { length = 16 }`, ConfigPlanChecks: ConfigPlanChecks{ - PostApplyPostRefresh: []PlanCheck{ + PostApplyPostRefresh: []plancheck.PlanCheck{ spy1, spy2, }, @@ -223,7 +225,7 @@ func Test_ConfigPlanChecks_PostApplyPostRefresh_Errors(t *testing.T) { length = 16 }`, ConfigPlanChecks: ConfigPlanChecks{ - PostApplyPostRefresh: []PlanCheck{ + PostApplyPostRefresh: []plancheck.PlanCheck{ spy1, spy2, spy3, diff --git a/helper/resource/testing_new_refresh_state_test.go b/helper/resource/testing_new_refresh_state_test.go index 3ff5b5190..afc34c9f1 100644 --- a/helper/resource/testing_new_refresh_state_test.go +++ b/helper/resource/testing_new_refresh_state_test.go @@ -4,6 +4,8 @@ import ( "errors" "regexp" "testing" + + "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func Test_RefreshPlanChecks_PostRefresh_Called(t *testing.T) { @@ -26,7 +28,7 @@ func Test_RefreshPlanChecks_PostRefresh_Called(t *testing.T) { { RefreshState: true, RefreshPlanChecks: RefreshPlanChecks{ - PostRefresh: []PlanCheck{ + PostRefresh: []plancheck.PlanCheck{ spy1, spy2, }, @@ -69,7 +71,7 @@ func Test_RefreshPlanChecks_PostRefresh_Errors(t *testing.T) { { RefreshState: true, RefreshPlanChecks: RefreshPlanChecks{ - PostRefresh: []PlanCheck{ + PostRefresh: []plancheck.PlanCheck{ spy1, spy2, spy3, diff --git a/helper/resource/teststep_validate_test.go b/helper/resource/teststep_validate_test.go index a807e7790..cf910c576 100644 --- a/helper/resource/teststep_validate_test.go +++ b/helper/resource/teststep_validate_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -194,7 +195,7 @@ func TestTestStepValidate(t *testing.T) { "configplanchecks-preapply-not-config-mode": { testStep: TestStep{ ConfigPlanChecks: ConfigPlanChecks{ - PreApply: []PlanCheck{&planCheckSpy{}}, + PreApply: []plancheck.PlanCheck{&planCheckSpy{}}, }, RefreshState: true, }, @@ -204,7 +205,7 @@ func TestTestStepValidate(t *testing.T) { "configplanchecks-preapply-not-planonly": { testStep: TestStep{ ConfigPlanChecks: ConfigPlanChecks{ - PreApply: []PlanCheck{&planCheckSpy{}}, + PreApply: []plancheck.PlanCheck{&planCheckSpy{}}, }, Config: "# not empty", PlanOnly: true, @@ -215,7 +216,7 @@ func TestTestStepValidate(t *testing.T) { "configplanchecks-postapplyprerefresh-not-config-mode": { testStep: TestStep{ ConfigPlanChecks: ConfigPlanChecks{ - PostApplyPreRefresh: []PlanCheck{&planCheckSpy{}}, + PostApplyPreRefresh: []plancheck.PlanCheck{&planCheckSpy{}}, }, RefreshState: true, }, @@ -225,7 +226,7 @@ func TestTestStepValidate(t *testing.T) { "configplanchecks-postapplypostrefresh-not-config-mode": { testStep: TestStep{ ConfigPlanChecks: ConfigPlanChecks{ - PostApplyPostRefresh: []PlanCheck{&planCheckSpy{}}, + PostApplyPostRefresh: []plancheck.PlanCheck{&planCheckSpy{}}, }, RefreshState: true, }, @@ -235,7 +236,7 @@ func TestTestStepValidate(t *testing.T) { "refreshplanchecks-postrefresh-not-refresh-mode": { testStep: TestStep{ RefreshPlanChecks: RefreshPlanChecks{ - PostRefresh: []PlanCheck{&planCheckSpy{}}, + PostRefresh: []plancheck.PlanCheck{&planCheckSpy{}}, }, Config: "# not empty", }, diff --git a/plancheck/expect_empty_plan.go b/plancheck/expect_empty_plan.go index 22455c54d..8c3012b6d 100644 --- a/plancheck/expect_empty_plan.go +++ b/plancheck/expect_empty_plan.go @@ -4,15 +4,14 @@ import ( "context" "fmt" - "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/internal/errorshim" ) -var _ resource.PlanCheck = expectEmptyPlan{} +var _ PlanCheck = expectEmptyPlan{} type expectEmptyPlan struct{} -func (e expectEmptyPlan) CheckPlan(ctx context.Context, req resource.CheckPlanRequest, resp *resource.CheckPlanResponse) { +func (e expectEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { var result error for _, rc := range req.Plan.ResourceChanges { @@ -27,6 +26,6 @@ func (e expectEmptyPlan) CheckPlan(ctx context.Context, req resource.CheckPlanRe } // TODO: document -func ExpectEmptyPlan() resource.PlanCheck { +func ExpectEmptyPlan() PlanCheck { return expectEmptyPlan{} } diff --git a/plancheck/expect_empty_plan_test.go b/plancheck/expect_empty_plan_test.go index e547d1cf9..ddc1a4b25 100644 --- a/plancheck/expect_empty_plan_test.go +++ b/plancheck/expect_empty_plan_test.go @@ -28,7 +28,7 @@ func Test_ExpectEmptyPlan(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectEmptyPlan(), }, }, @@ -69,7 +69,7 @@ func Test_ExpectEmptyPlan_Error(t *testing.T) { length = 12 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectEmptyPlan(), }, }, diff --git a/plancheck/expect_non_empty_plan.go b/plancheck/expect_non_empty_plan.go index e0ffcc53a..a325a507a 100644 --- a/plancheck/expect_non_empty_plan.go +++ b/plancheck/expect_non_empty_plan.go @@ -3,15 +3,13 @@ package plancheck import ( "context" "errors" - - "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) -var _ resource.PlanCheck = expectNonEmptyPlan{} +var _ PlanCheck = expectNonEmptyPlan{} type expectNonEmptyPlan struct{} -func (e expectNonEmptyPlan) CheckPlan(ctx context.Context, req resource.CheckPlanRequest, resp *resource.CheckPlanResponse) { +func (e expectNonEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { for _, rc := range req.Plan.ResourceChanges { if !rc.Change.Actions.NoOp() { return @@ -22,6 +20,6 @@ func (e expectNonEmptyPlan) CheckPlan(ctx context.Context, req resource.CheckPla } // TODO: document -func ExpectNonEmptyPlan() resource.PlanCheck { +func ExpectNonEmptyPlan() PlanCheck { return expectNonEmptyPlan{} } diff --git a/plancheck/expect_non_empty_plan_test.go b/plancheck/expect_non_empty_plan_test.go index 70b3738fd..5277591ea 100644 --- a/plancheck/expect_non_empty_plan_test.go +++ b/plancheck/expect_non_empty_plan_test.go @@ -28,7 +28,7 @@ func Test_ExpectNonEmptyPlan(t *testing.T) { length = 12 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectNonEmptyPlan(), }, }, @@ -57,7 +57,7 @@ func Test_ExpectNonEmptyPlan_Error(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectNonEmptyPlan(), }, }, diff --git a/plancheck/expect_resource_action.go b/plancheck/expect_resource_action.go index 8eb586a5a..8c20252a6 100644 --- a/plancheck/expect_resource_action.go +++ b/plancheck/expect_resource_action.go @@ -3,18 +3,16 @@ package plancheck import ( "context" "fmt" - - "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) -var _ resource.PlanCheck = expectResourceAction{} +var _ PlanCheck = expectResourceAction{} type expectResourceAction struct { resourceAddress string actionType ResourceActionType } -func (e expectResourceAction) CheckPlan(ctx context.Context, req resource.CheckPlanRequest, resp *resource.CheckPlanResponse) { +func (e expectResourceAction) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { foundResource := false for _, rc := range req.Plan.ResourceChanges { @@ -77,7 +75,7 @@ func (e expectResourceAction) CheckPlan(ctx context.Context, req resource.CheckP } // TODO: document -func ExpectResourceAction(resourceAddress string, actionType ResourceActionType) resource.PlanCheck { +func ExpectResourceAction(resourceAddress string, actionType ResourceActionType) PlanCheck { return expectResourceAction{ resourceAddress: resourceAddress, actionType: actionType, diff --git a/plancheck/expect_resource_action_test.go b/plancheck/expect_resource_action_test.go index e495ad0da..c0e01c242 100644 --- a/plancheck/expect_resource_action_test.go +++ b/plancheck/expect_resource_action_test.go @@ -28,7 +28,7 @@ func Test_ExpectedResourceAction_NoOp(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionNoop), }, }, @@ -52,7 +52,7 @@ func Test_ExpectedResourceAction_NoOp_NoMatch(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionNoop), }, }, @@ -77,7 +77,7 @@ func Test_ExpectedResourceAction_Create(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionCreate), }, }, @@ -106,7 +106,7 @@ func Test_ExpectedResourceAction_Create_NoMatch(t *testing.T) { length = 15 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionCreate), }, }, @@ -140,7 +140,7 @@ func Test_ExpectedResourceAction_Read(t *testing.T) { } }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("data.null_data_source.two", plancheck.ResourceActionRead), }, }, @@ -167,7 +167,7 @@ func Test_ExpectedResourceAction_Read_NoMatch(t *testing.T) { length = 15 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionRead), }, }, @@ -197,7 +197,7 @@ func Test_ExpectedResourceAction_Update(t *testing.T) { offset_days = 2 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("time_offset.one", plancheck.ResourceActionUpdate), }, }, @@ -221,7 +221,7 @@ func Test_ExpectedResourceAction_Update_NoMatch(t *testing.T) { offset_days = 1 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("time_offset.one", plancheck.ResourceActionUpdate), }, }, @@ -249,7 +249,7 @@ func Test_ExpectedResourceAction_Destroy(t *testing.T) { { Config: ` `, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroy), }, }, @@ -273,7 +273,7 @@ func Test_ExpectedResourceAction_Destroy_NoMatch(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroy), }, }, @@ -303,7 +303,7 @@ func Test_ExpectedResourceAction_DestroyBeforeCreate(t *testing.T) { length = 15 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroyBeforeCreate), }, }, @@ -327,7 +327,7 @@ func Test_ExpectedResourceAction_DestroyBeforeCreate_NoMatch(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroyBeforeCreate), }, }, @@ -363,7 +363,7 @@ func Test_ExpectedResourceAction_CreateBeforeDestroy(t *testing.T) { } }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionCreateBeforeDestroy), }, }, @@ -387,7 +387,7 @@ func Test_ExpectedResourceAction_CreateBeforeDestroy_NoMatch(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionCreateBeforeDestroy), }, }, @@ -431,7 +431,7 @@ func Test_ExpectedResourceAction_Replace(t *testing.T) { } }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionReplace), plancheck.ExpectResourceAction("random_string.two", plancheck.ResourceActionReplace), }, @@ -456,7 +456,7 @@ func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionReplace), }, }, @@ -470,7 +470,7 @@ func Test_ExpectedResourceAction_Replace_NoMatch(t *testing.T) { } }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.two", plancheck.ResourceActionReplace), }, }, @@ -495,7 +495,7 @@ func Test_ExpectedResourceAction_NoResourceFound(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.doesntexist", plancheck.ResourceActionCreate), }, }, @@ -520,7 +520,7 @@ func Test_ExpectedResourceAction_InvalidResourceActionType(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", 0), }, }, @@ -531,7 +531,7 @@ func Test_ExpectedResourceAction_InvalidResourceActionType(t *testing.T) { length = 16 }`, ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []r.PlanCheck{ + PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", 9), }, }, diff --git a/plancheck/plan_check.go b/plancheck/plan_check.go new file mode 100644 index 000000000..c057bb19a --- /dev/null +++ b/plancheck/plan_check.go @@ -0,0 +1,22 @@ +package plancheck + +import ( + "context" + + tfjson "github.com/hashicorp/terraform-json" +) + +// TODO: document +type PlanCheck interface { + CheckPlan(context.Context, CheckPlanRequest, *CheckPlanResponse) +} + +// TODO: document +type CheckPlanRequest struct { + Plan *tfjson.Plan +} + +// TODO: document +type CheckPlanResponse struct { + Error error +} From 42a63a955fd2f94ce6c715f270256f15e7d794ea Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 20 Mar 2023 14:54:08 -0400 Subject: [PATCH 22/31] add initial docs for plancheck package --- plancheck/doc.go | 2 +- plancheck/expect_empty_plan.go | 4 ++- plancheck/expect_non_empty_plan.go | 3 +- plancheck/expect_resource_action.go | 22 +++++++------ plancheck/expect_resource_action_test.go | 15 ++------- plancheck/plan_check.go | 11 +++++-- plancheck/resource_action.go | 39 ++++++++++++++++-------- 7 files changed, 54 insertions(+), 42 deletions(-) diff --git a/plancheck/doc.go b/plancheck/doc.go index 8f5f62e51..5f50334cf 100644 --- a/plancheck/doc.go +++ b/plancheck/doc.go @@ -1,2 +1,2 @@ -// TODO: write package documentation +// Package plancheck contains the plan check interface, request/response structs, and common plan check implementations. package plancheck diff --git a/plancheck/expect_empty_plan.go b/plancheck/expect_empty_plan.go index 8c3012b6d..23f03e826 100644 --- a/plancheck/expect_empty_plan.go +++ b/plancheck/expect_empty_plan.go @@ -11,6 +11,7 @@ var _ PlanCheck = expectEmptyPlan{} type expectEmptyPlan struct{} +// CheckPlan implements the plan check logic. func (e expectEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { var result error @@ -25,7 +26,8 @@ func (e expectEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, re resp.Error = result } -// TODO: document +// ExpectEmptyPlan returns a plan check that asserts that there are no resource changes in the plan. +// All resource changes found will be aggregated and returned in a plan check error. func ExpectEmptyPlan() PlanCheck { return expectEmptyPlan{} } diff --git a/plancheck/expect_non_empty_plan.go b/plancheck/expect_non_empty_plan.go index a325a507a..1a6206b65 100644 --- a/plancheck/expect_non_empty_plan.go +++ b/plancheck/expect_non_empty_plan.go @@ -9,6 +9,7 @@ var _ PlanCheck = expectNonEmptyPlan{} type expectNonEmptyPlan struct{} +// CheckPlan implements the plan check logic. func (e expectNonEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { for _, rc := range req.Plan.ResourceChanges { if !rc.Change.Actions.NoOp() { @@ -19,7 +20,7 @@ func (e expectNonEmptyPlan) CheckPlan(ctx context.Context, req CheckPlanRequest, resp.Error = errors.New("expected a non-empty plan, but got an empty plan") } -// TODO: document +// ExpectNonEmptyPlan returns a plan check that asserts there is at least one resource change in the plan. func ExpectNonEmptyPlan() PlanCheck { return expectNonEmptyPlan{} } diff --git a/plancheck/expect_resource_action.go b/plancheck/expect_resource_action.go index 8c20252a6..bd0690c3e 100644 --- a/plancheck/expect_resource_action.go +++ b/plancheck/expect_resource_action.go @@ -12,6 +12,7 @@ type expectResourceAction struct { actionType ResourceActionType } +// CheckPlan implements the plan check logic. func (e expectResourceAction) CheckPlan(ctx context.Context, req CheckPlanRequest, resp *CheckPlanResponse) { foundResource := false @@ -20,46 +21,46 @@ func (e expectResourceAction) CheckPlan(ctx context.Context, req CheckPlanReques switch e.actionType { case ResourceActionNoop: if !rc.Change.Actions.NoOp() { - resp.Error = fmt.Errorf("'%s' - expected NoOp, got action(s): %v", rc.Address, rc.Change.Actions) + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } case ResourceActionCreate: if !rc.Change.Actions.Create() { - resp.Error = fmt.Errorf("'%s' - expected Create, got action(s): %v", rc.Address, rc.Change.Actions) + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } case ResourceActionRead: if !rc.Change.Actions.Read() { - resp.Error = fmt.Errorf("'%s' - expected Read, got action(s): %v", rc.Address, rc.Change.Actions) + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } case ResourceActionUpdate: if !rc.Change.Actions.Update() { - resp.Error = fmt.Errorf("'%s' - expected Update, got action(s): %v", rc.Address, rc.Change.Actions) + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } case ResourceActionDestroy: if !rc.Change.Actions.Delete() { - resp.Error = fmt.Errorf("'%s' - expected Destroy, got action(s): %v", rc.Address, rc.Change.Actions) + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } case ResourceActionDestroyBeforeCreate: if !rc.Change.Actions.DestroyBeforeCreate() { - resp.Error = fmt.Errorf("'%s' - expected DestroyBeforeCreate, got action(s): %v", rc.Address, rc.Change.Actions) + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } case ResourceActionCreateBeforeDestroy: if !rc.Change.Actions.CreateBeforeDestroy() { - resp.Error = fmt.Errorf("'%s' - expected CreateBeforeDestroy, got action(s): %v", rc.Address, rc.Change.Actions) + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } case ResourceActionReplace: if !rc.Change.Actions.Replace() { - resp.Error = fmt.Errorf("%s - expected Replace, got action(s): %v", rc.Address, rc.Change.Actions) + resp.Error = fmt.Errorf("%s - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } default: - resp.Error = fmt.Errorf("%s - unexpected ResourceActionType byte: %d", rc.Address, e.actionType) + resp.Error = fmt.Errorf("%s - unexpected ResourceActionType: %s", rc.Address, e.actionType) return } @@ -74,7 +75,8 @@ func (e expectResourceAction) CheckPlan(ctx context.Context, req CheckPlanReques } } -// TODO: document +// ExpectResourceAction returns a plan check that asserts that a given resource will have a specific resource change type in the plan. +// Valid actionType are an enum of type plancheck.ResourceActionType, examples: NoOp, DestroyBeforeCreate, Update (in-place), etc. func ExpectResourceAction(resourceAddress string, actionType ResourceActionType) PlanCheck { return expectResourceAction{ resourceAddress: resourceAddress, diff --git a/plancheck/expect_resource_action_test.go b/plancheck/expect_resource_action_test.go index c0e01c242..abc7b6333 100644 --- a/plancheck/expect_resource_action_test.go +++ b/plancheck/expect_resource_action_test.go @@ -521,21 +521,10 @@ func Test_ExpectedResourceAction_InvalidResourceActionType(t *testing.T) { }`, ConfigPlanChecks: r.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ - plancheck.ExpectResourceAction("random_string.one", 0), + plancheck.ExpectResourceAction("random_string.one", "Invalid"), }, }, - ExpectError: regexp.MustCompile(`random_string.one - unexpected ResourceActionType byte: 0`), - }, - { - Config: `resource "random_string" "one" { - length = 16 - }`, - ConfigPlanChecks: r.ConfigPlanChecks{ - PreApply: []plancheck.PlanCheck{ - plancheck.ExpectResourceAction("random_string.one", 9), - }, - }, - ExpectError: regexp.MustCompile(`random_string.one - unexpected ResourceActionType byte: 9`), + ExpectError: regexp.MustCompile(`random_string.one - unexpected ResourceActionType: Invalid`), }, }, }) diff --git a/plancheck/plan_check.go b/plancheck/plan_check.go index c057bb19a..4de90e17c 100644 --- a/plancheck/plan_check.go +++ b/plancheck/plan_check.go @@ -6,17 +6,22 @@ import ( tfjson "github.com/hashicorp/terraform-json" ) -// TODO: document +// PlanCheck defines an interface for implementing test logic that checks a plan file and then returns an error +// if the plan file does not match what is expected. type PlanCheck interface { + // CheckPlan should perform the plan check. CheckPlan(context.Context, CheckPlanRequest, *CheckPlanResponse) } -// TODO: document +// CheckPlanRequest is a request for an invoke of the CheckPlan function. type CheckPlanRequest struct { + // Plan represents a parsed plan file, retrieved via the `terraform show -json` command. Plan *tfjson.Plan } -// TODO: document +// CheckPlanResponse is a response to an invoke of the CheckPlan function. type CheckPlanResponse struct { + // Error is used to report the failure of a plan check assertion and is combined with other PlanCheck errors + // to be reported as a test failure. Error error } diff --git a/plancheck/resource_action.go b/plancheck/resource_action.go index f77c7fef4..1e0bbc84e 100644 --- a/plancheck/resource_action.go +++ b/plancheck/resource_action.go @@ -1,18 +1,31 @@ package plancheck -// TODO: is this a good name? -// TODO: document type and all aliases below -type ResourceActionType byte +// ResourceActionType is a string enum type that routes to a specific terraform-json.Actions function for asserting resource changes. +// https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions +type ResourceActionType string const ( - ResourceActionInvalid ResourceActionType = iota - ResourceActionNoop - ResourceActionCreate - ResourceActionRead - ResourceActionUpdate - ResourceActionDestroy - ResourceActionDestroyBeforeCreate - ResourceActionCreateBeforeDestroy - // TODO: document, this is more of a helper to detect either of the two above - ResourceActionReplace + // ResourceActionNoop is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.NoOp + ResourceActionNoop ResourceActionType = "NoOp" + + // ResourceActionCreate is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Create + ResourceActionCreate ResourceActionType = "Create" + + // ResourceActionRead is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Read + ResourceActionRead ResourceActionType = "Read" + + // ResourceActionUpdate is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Update + ResourceActionUpdate ResourceActionType = "Update" + + // ResourceActionDestroy is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Delete + ResourceActionDestroy ResourceActionType = "Destroy" + + // ResourceActionDestroyBeforeCreate is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.DestroyBeforeCreate + ResourceActionDestroyBeforeCreate ResourceActionType = "DestroyBeforeCreate" + + // ResourceActionCreateBeforeDestroy is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.CreateBeforeDestroy + ResourceActionCreateBeforeDestroy ResourceActionType = "CreateBeforeDestroy" + + // ResourceActionReplace is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Replace + ResourceActionReplace ResourceActionType = "Replace" ) From e2060b6dbe79ab79e86861b7fafa93620c0a2043 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Mon, 20 Mar 2023 15:50:14 -0400 Subject: [PATCH 23/31] add initial documentation --- helper/resource/testing.go | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index a1c3df882..abd9dc0df 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -504,8 +504,12 @@ type TestStep struct { // ExpectNonEmptyPlan can be set to true for specific types of tests that are // looking to verify that a diff occurs - // TODO: deprecate and describe how to replace (ConfigPlanChecks.PostApplyPostRefresh or RefreshPlanChecks.PostRefresh) - // TODO: describe implicit empty plan behavior and how to replace? documentation? + // + // Deprecated: Please replace by using the plan check provided in the testing plancheck package: [plancheck.ExpectNonEmptyPlan] + // - For Config (plan/apply) testing, add the above plan check to the slice: TestStep.ConfigPlanChecks.PostApplyPostRefresh + // - For Refresh testing, add the above plan check to the slice: TestStep.RefreshPlanChecks.PostRefresh + // + // [plancheck.ExpectNonEmptyPlan]: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#ExpectNonEmptyPlan ExpectNonEmptyPlan bool // ExpectError allows the construction of test cases that we expect to fail @@ -513,12 +517,18 @@ type TestStep struct { // test to pass. ExpectError *regexp.Regexp - // TODO: is this naming good? - // TODO: document + // ConfigPlanChecks allows assertions to be made against the plan file at different points of a Config (apply) test using a plan check. + // Custom plan checks can be created by implementing the [PlanCheck] interface, or by using a PlanCheck implementation from the provided [plancheck] package + // + // [PlanCheck]: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#PlanCheck + // [plancheck]: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck ConfigPlanChecks ConfigPlanChecks - // TODO: is this naming good? - // TODO: document + // RefreshPlanChecks allows assertions to be made against the plan file at different points of a Refresh test using a plan check. + // Custom plan checks can be created by implementing the [PlanCheck] interface, or by using a PlanCheck implementation from the provided [plancheck] package + // + // [PlanCheck]: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#PlanCheck + // [plancheck]: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck RefreshPlanChecks RefreshPlanChecks // PlanOnly can be set to only run `plan` with this configuration, and not @@ -690,19 +700,25 @@ type TestStep struct { ExternalProviders map[string]ExternalProvider } -// TODO: document all fields / move to a different file/package? +// ConfigPlanChecks defines the different points in a Config TestStep when plan checks can be run. type ConfigPlanChecks struct { + // PreApply runs all plan checks in the slice. This occurs before the apply of a Config test is run. This slice cannot be populated + // with TestStep.PlanOnly, as there is no PreApply plan run with that flag set. All errors by plan checks in this slice are aggregated, reported, and will result in a test failure. PreApply []plancheck.PlanCheck + // PostApplyPreRefresh runs all plan checks in the slice. This occurs after the apply and before the refresh of a Config test is run. + // All errors by plan checks in this slice are aggregated, reported, and will result in a test failure. PostApplyPreRefresh []plancheck.PlanCheck - // TODO: should this be named 2nd post apply? Since refresh is not guaranteed + // PostApplyPostRefresh runs all plan checks in the slice. This occurs after the apply and refresh of a Config test are run. + // All errors by plan checks in this slice are aggregated, reported, and will result in a test failure. PostApplyPostRefresh []plancheck.PlanCheck } -// TODO: is this naming good? -// TODO: document +// RefreshPlanChecks defines the different points in a Refresh TestStep when plan checks can be run. type RefreshPlanChecks struct { + // PostRefresh runs all plan checks in the slice. This occurs after the refresh of the Refresh test is run. + // All errors by plan checks in this slice are aggregated, reported, and will result in a test failure. PostRefresh []plancheck.PlanCheck } From 6b8d872abe093072f9210c49d8d87c89ec9842db Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 21 Mar 2023 14:47:37 -0400 Subject: [PATCH 24/31] remove deprecation message --- helper/resource/testing.go | 6 ------ website/docs/plugin/testing/acceptance-tests/teststep.mdx | 8 ++++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index abd9dc0df..55aee5a53 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -504,12 +504,6 @@ type TestStep struct { // ExpectNonEmptyPlan can be set to true for specific types of tests that are // looking to verify that a diff occurs - // - // Deprecated: Please replace by using the plan check provided in the testing plancheck package: [plancheck.ExpectNonEmptyPlan] - // - For Config (plan/apply) testing, add the above plan check to the slice: TestStep.ConfigPlanChecks.PostApplyPostRefresh - // - For Refresh testing, add the above plan check to the slice: TestStep.RefreshPlanChecks.PostRefresh - // - // [plancheck.ExpectNonEmptyPlan]: https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#ExpectNonEmptyPlan ExpectNonEmptyPlan bool // ExpectError allows the construction of test cases that we expect to fail diff --git a/website/docs/plugin/testing/acceptance-tests/teststep.mdx b/website/docs/plugin/testing/acceptance-tests/teststep.mdx index ae23aa446..6df9179e3 100644 --- a/website/docs/plugin/testing/acceptance-tests/teststep.mdx +++ b/website/docs/plugin/testing/acceptance-tests/teststep.mdx @@ -14,10 +14,10 @@ under test. ## Test Modes -Terraform’s test framework facilitates three distinct modes of acceptance tests, -_Lifecycle_, _Import_ and _Refresh_. +Terraform's test framework facilitates three distinct modes of acceptance tests, +_Lifecycle (config)_, _Import_ and _Refresh_. -_Lifecycle_ mode is the most common mode, and is used for testing plugins by +_Lifecycle (config)_ 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 when running `terraform apply`. @@ -29,7 +29,7 @@ _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 +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-testing/helper/resource#TestStep). From c3aefc4450e59570c719b3d7f696f15cae10fc71 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 21 Mar 2023 15:35:55 -0400 Subject: [PATCH 25/31] add detailed docs to actions --- plancheck/resource_action.go | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/plancheck/resource_action.go b/plancheck/resource_action.go index 1e0bbc84e..95494ead3 100644 --- a/plancheck/resource_action.go +++ b/plancheck/resource_action.go @@ -1,31 +1,47 @@ package plancheck // ResourceActionType is a string enum type that routes to a specific terraform-json.Actions function for asserting resource changes. -// https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions +// - https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions +// +// More information about expected resource behavior can be found at: https://developer.hashicorp.com/terraform/language/resources/behavior type ResourceActionType string const ( - // ResourceActionNoop is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.NoOp + // ResourceActionNoop occurs when a resource is not planned to change (no-op). + // - Routes to: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.NoOp ResourceActionNoop ResourceActionType = "NoOp" - // ResourceActionCreate is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Create + // ResourceActionCreate occurs when a resource is planned to be created. + // - Routes to: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Create ResourceActionCreate ResourceActionType = "Create" - // ResourceActionRead is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Read + // ResourceActionRead occurs when a data source is planned to be read during the apply stage (data sources are read during plan stage when possible). + // See the data source documentation for more information on this behavior: https://developer.hashicorp.com/terraform/language/data-sources#data-resource-behavior + // - Routes to: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Read ResourceActionRead ResourceActionType = "Read" - // ResourceActionUpdate is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Update + // ResourceActionUpdate occurs when a resource is planned to be updated in-place. + // - Routes to: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Update ResourceActionUpdate ResourceActionType = "Update" - // ResourceActionDestroy is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Delete + // ResourceActionDestroy occurs when a resource is planned to be deleted. + // - Routes to: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Delete ResourceActionDestroy ResourceActionType = "Destroy" - // ResourceActionDestroyBeforeCreate is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.DestroyBeforeCreate + // ResourceActionDestroyBeforeCreate occurs when a resource is planned to be deleted and then re-created. This is the default + // behavior when terraform must change a resource argument that cannot be updated in-place due to remote API limitations. + // - Routes to: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.DestroyBeforeCreate ResourceActionDestroyBeforeCreate ResourceActionType = "DestroyBeforeCreate" - // ResourceActionCreateBeforeDestroy is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.CreateBeforeDestroy + // ResourceActionCreateBeforeDestroy occurs when a resource is planned to be created and then deleted. This is opt-in behavior that + // is enabled with the [create_before_destroy] meta-argument. + // - Routes to: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.CreateBeforeDestroy + // + // [create_before_destroy]: https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#create_before_destroy ResourceActionCreateBeforeDestroy ResourceActionType = "CreateBeforeDestroy" - // ResourceActionReplace is a string enum that represents: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Replace + // ResourceActionReplace can be used to verify a resource is planned to be deleted and re-created (where the order of delete and create actions are not important). + // This action matches both ResourceActionDestroyBeforeCreate and ResourceActionCreateBeforeDestroy. + // - Routes to: https://pkg.go.dev/github.com/hashicorp/terraform-json#Actions.Replace ResourceActionReplace ResourceActionType = "Replace" ) From e511999683692f5e060f342d8a8a5ad622b717fb Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 21 Mar 2023 17:29:12 -0400 Subject: [PATCH 26/31] added plan check doc page --- .../testing/acceptance-tests/plan-checks.mdx | 234 +++++++++++++++++- 1 file changed, 232 insertions(+), 2 deletions(-) diff --git a/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx b/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx index 7f3f46ef7..68bf7f828 100644 --- a/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx +++ b/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx @@ -1,10 +1,240 @@ --- page_title: 'Plugin Development - Acceptance Testing: Plan Checks' description: >- - TBD + Plan Checks are test assertions that can inspect a plan at different phases in a TestStep. The testing module + provides built-in Plan Checks for common use-cases, but custom Plan Checks can also be implemented. --- # Plan Checks -{/* TODO: Add documentation mentioned in RFC here */} +During the **Lifecycle (config)** and **Refresh** [modes](/terraform/plugin/testing/acceptance-tests/teststep#test-modes) of a `TestStep`, the testing framework will run `terraform plan` before and after certain operations. For example, the **Lifecycle (config)** mode will run a plan before the `terraform apply` phase, as well as a plan before and after the `terraform refresh` phase. +These `terraform plan` operations results in a [plan file](/terraform/cli/commands/plan#out-filename) and can be represented by this [JSON format](/terraform/internals/json-format#plan-representation). + +A **plan check** is a test assertion that inspects the plan file at a specific phase during the current testing mode. Multiple plan checks can be run at each defined phase, all assertion errors returned are aggregated, reported as a test failure, and all test cleanup logic is executed. + +- Available plan phases for **Lifecycle (config)** mode are defined in the [`TestStep.ConfigPlanChecks`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/helper/resource#TestStep) struct +- Available plan phases for **Refresh** mode are defined in the [`TestStep.RefreshPlanChecks`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/helper/resource#TestStep) struct +- **Import** mode currently does not run any plan operations, and therefore does not support plan checks. + +## Built-in Plan Checks + +The `terraform-plugin-testing` module provides a package [`plancheck`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck) with built-in plan checks for common use-cases. + +### Examples using `plancheck.ExpectResourceAction` + +One of the built-in plan checks, [`plancheck.ExpectResourceAction`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#ExpectResourceAction), is useful for determining the exact action type a resource will under-go during, say, the `terraform apply` phase. + +Given the following example with the [random provider](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/string), we have written a test that asserts that `random_string.one` will be destroyed and re-created when the `length` attribute is changed: + +```go +package example_test + +import ( + "testing" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" +) + +func Test_Random_ForcesRecreate(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 15 + }`, + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroyBeforeCreate), + }, + }, + }, + }, + }) +} +``` + +Another example with the [time provider](https://registry.terraform.io/providers/hashicorp/time/latest/docs/resources/offset) asserts that `time_offset.one` will be updated in-place when the `offset_days` attribute is changed: + +```go +package example_test + +import ( + "testing" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" +) + +func Test_Time_UpdateInPlace(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "time": { + Source: "registry.terraform.io/hashicorp/time", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "time_offset" "one" { + offset_days = 1 + }`, + }, + { + Config: `resource "time_offset" "one" { + offset_days = 2 + }`, + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("time_offset.one", plancheck.ResourceActionUpdate), + }, + }, + }, + }, + }) +} +``` + +Multiple plan checks can be combined if you want to assert multiple resource actions: +```go +package example_test + +import ( + "testing" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" +) + +func Test_Time_UpdateInPlace_and_NoOp(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "time": { + Source: "registry.terraform.io/hashicorp/time", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "time_offset" "one" { + offset_days = 1 + } + resource "time_offset" "two" { + offset_days = 1 + }`, + }, + { + Config: `resource "time_offset" "one" { + offset_days = 2 + } + resource "time_offset" "two" { + offset_days = 1 + }`, + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("time_offset.one", plancheck.ResourceActionUpdate), + plancheck.ExpectResourceAction("time_offset.two", plancheck.ResourceActionNoop), + }, + }, + }, + }, + }) +} +``` + +## Custom Plan Checks + +The package [`plancheck`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck) also provides the [`PlanCheck`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#PlanCheck) interface, which can be implemented for a custom plan check. + +The [`plancheck.CheckPlanRequest`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#CheckPlanRequest) contains the current plan file, parsed by the [terraform-json package](https://pkg.go.dev/github.com/hashicorp/terraform-json#Plan). + +Here is an example implementation of a plan check that asserts that every resource change is a no-op, aka, an empty plan: +```go +package example_test + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-testing/plancheck" +) + +var _ plancheck.PlanCheck = expectEmptyPlan{} + +type expectEmptyPlan struct{} + +func (e expectEmptyPlan) CheckPlan(ctx context.Context, req plancheck.CheckPlanRequest, resp *plancheck.CheckPlanResponse) { + var result error + + for _, rc := range req.Plan.ResourceChanges { + if !rc.Change.Actions.NoOp() { + result = errors.Join(result, fmt.Errorf("expected empty plan, but %s has planned action(s): %v", rc.Address, rc.Change.Actions)) + } + } + + resp.Error = result +} + +func ExpectEmptyPlan() plancheck.PlanCheck { + return expectEmptyPlan{} +} +``` + +And example usage: +```go +package example_test + +import ( + "context" + "errors" + "fmt" + "testing" + + r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" +) + +func Test_CustomPlanCheck_ExpectEmptyPlan(t *testing.T) { + t.Parallel() + + r.Test(t, r.TestCase{ + ExternalProviders: map[string]r.ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + Steps: []r.TestStep{ + { + Config: `resource "random_string" "one" { + length = 16 + }`, + }, + { + Config: `resource "random_string" "one" { + length = 16 + }`, + ConfigPlanChecks: r.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + ExpectEmptyPlan(), + }, + }, + }, + }, + }) +} +``` From ebbafa86242c9fc680805f3323c5362961a494de Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Tue, 21 Mar 2023 17:41:08 -0400 Subject: [PATCH 27/31] added changelogs --- .changes/unreleased/ENHANCEMENTS-20230321-174020.yaml | 6 ++++++ .changes/unreleased/FEATURES-20230321-173506.yaml | 6 ++++++ .changes/unreleased/FEATURES-20230321-173639.yaml | 6 ++++++ .changes/unreleased/FEATURES-20230321-173726.yaml | 6 ++++++ .changes/unreleased/FEATURES-20230321-173805.yaml | 6 ++++++ 5 files changed, 30 insertions(+) create mode 100644 .changes/unreleased/ENHANCEMENTS-20230321-174020.yaml create mode 100644 .changes/unreleased/FEATURES-20230321-173506.yaml create mode 100644 .changes/unreleased/FEATURES-20230321-173639.yaml create mode 100644 .changes/unreleased/FEATURES-20230321-173726.yaml create mode 100644 .changes/unreleased/FEATURES-20230321-173805.yaml diff --git a/.changes/unreleased/ENHANCEMENTS-20230321-174020.yaml b/.changes/unreleased/ENHANCEMENTS-20230321-174020.yaml new file mode 100644 index 000000000..b8411d38c --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-20230321-174020.yaml @@ -0,0 +1,6 @@ +kind: ENHANCEMENTS +body: 'helper/resource: Added plan check functionality to config and refresh modes + with new fields `TestStep.ConfigPlanChecks` and `TestStep.RefreshPlanChecks`' +time: 2023-03-21T17:40:20.521786-04:00 +custom: + Issue: "63" diff --git a/.changes/unreleased/FEATURES-20230321-173506.yaml b/.changes/unreleased/FEATURES-20230321-173506.yaml new file mode 100644 index 000000000..7f2dd9683 --- /dev/null +++ b/.changes/unreleased/FEATURES-20230321-173506.yaml @@ -0,0 +1,6 @@ +kind: FEATURES +body: 'plancheck: Introduced new `plancheck` package with interface and built-in plan + check functionality' +time: 2023-03-21T17:35:06.650327-04:00 +custom: + Issue: "63" diff --git a/.changes/unreleased/FEATURES-20230321-173639.yaml b/.changes/unreleased/FEATURES-20230321-173639.yaml new file mode 100644 index 000000000..7d897d92d --- /dev/null +++ b/.changes/unreleased/FEATURES-20230321-173639.yaml @@ -0,0 +1,6 @@ +kind: FEATURES +body: 'plancheck: Added `ExpectResourceAction` built-in plan check, which asserts + that a given resource will have a specific resource change type in the plan' +time: 2023-03-21T17:36:39.391477-04:00 +custom: + Issue: "63" diff --git a/.changes/unreleased/FEATURES-20230321-173726.yaml b/.changes/unreleased/FEATURES-20230321-173726.yaml new file mode 100644 index 000000000..e85622f80 --- /dev/null +++ b/.changes/unreleased/FEATURES-20230321-173726.yaml @@ -0,0 +1,6 @@ +kind: FEATURES +body: 'plancheck: Added `ExpectEmptyPlan` built-in plan check, which asserts that + there are no resource changes in the plan' +time: 2023-03-21T17:37:26.076041-04:00 +custom: + Issue: "63" diff --git a/.changes/unreleased/FEATURES-20230321-173805.yaml b/.changes/unreleased/FEATURES-20230321-173805.yaml new file mode 100644 index 000000000..2650209fc --- /dev/null +++ b/.changes/unreleased/FEATURES-20230321-173805.yaml @@ -0,0 +1,6 @@ +kind: FEATURES +body: 'plancheck: Added `ExpectNonEmptyPlan` built-in plan check, which asserts that + there is at least one resource change in the plan' +time: 2023-03-21T17:38:05.815307-04:00 +custom: + Issue: "63" From dbacdcf2124d212e1fac8d9b2f12b945dae77d13 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 22 Mar 2023 10:14:55 -0400 Subject: [PATCH 28/31] remove nest --- plancheck/expect_resource_action.go | 96 +++++++++++++++-------------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/plancheck/expect_resource_action.go b/plancheck/expect_resource_action.go index bd0690c3e..6d8d16607 100644 --- a/plancheck/expect_resource_action.go +++ b/plancheck/expect_resource_action.go @@ -17,56 +17,58 @@ func (e expectResourceAction) CheckPlan(ctx context.Context, req CheckPlanReques foundResource := false for _, rc := range req.Plan.ResourceChanges { - if e.resourceAddress == rc.Address { - switch e.actionType { - case ResourceActionNoop: - if !rc.Change.Actions.NoOp() { - resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) - return - } - case ResourceActionCreate: - if !rc.Change.Actions.Create() { - resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) - return - } - case ResourceActionRead: - if !rc.Change.Actions.Read() { - resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) - return - } - case ResourceActionUpdate: - if !rc.Change.Actions.Update() { - resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) - return - } - case ResourceActionDestroy: - if !rc.Change.Actions.Delete() { - resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) - return - } - case ResourceActionDestroyBeforeCreate: - if !rc.Change.Actions.DestroyBeforeCreate() { - resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) - return - } - case ResourceActionCreateBeforeDestroy: - if !rc.Change.Actions.CreateBeforeDestroy() { - resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) - return - } - case ResourceActionReplace: - if !rc.Change.Actions.Replace() { - resp.Error = fmt.Errorf("%s - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) - return - } - default: - resp.Error = fmt.Errorf("%s - unexpected ResourceActionType: %s", rc.Address, e.actionType) + if e.resourceAddress != rc.Address { + continue + } + + switch e.actionType { + case ResourceActionNoop: + if !rc.Change.Actions.NoOp() { + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) return } - - foundResource = true - break + case ResourceActionCreate: + if !rc.Change.Actions.Create() { + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) + return + } + case ResourceActionRead: + if !rc.Change.Actions.Read() { + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) + return + } + case ResourceActionUpdate: + if !rc.Change.Actions.Update() { + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) + return + } + case ResourceActionDestroy: + if !rc.Change.Actions.Delete() { + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) + return + } + case ResourceActionDestroyBeforeCreate: + if !rc.Change.Actions.DestroyBeforeCreate() { + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) + return + } + case ResourceActionCreateBeforeDestroy: + if !rc.Change.Actions.CreateBeforeDestroy() { + resp.Error = fmt.Errorf("'%s' - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) + return + } + case ResourceActionReplace: + if !rc.Change.Actions.Replace() { + resp.Error = fmt.Errorf("%s - expected %s, got action(s): %v", rc.Address, e.actionType, rc.Change.Actions) + return + } + default: + resp.Error = fmt.Errorf("%s - unexpected ResourceActionType: %s", rc.Address, e.actionType) + return } + + foundResource = true + break } if !foundResource { From 10b852996ad3a6ddd1b5785cc6776d082ff83e52 Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 22 Mar 2023 10:21:32 -0400 Subject: [PATCH 29/31] remove import aliases from docs --- .../testing/acceptance-tests/plan-checks.mdx | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx b/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx index 68bf7f828..165c071fb 100644 --- a/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx +++ b/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx @@ -33,20 +33,20 @@ package example_test import ( "testing" - r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func Test_Random_ForcesRecreate(t *testing.T) { t.Parallel() - r.Test(t, r.TestCase{ - ExternalProviders: map[string]r.ExternalProvider{ + resource.Test(t, resource.TestCase{ + ExternalProviders: map[string]resource.ExternalProvider{ "random": { Source: "registry.terraform.io/hashicorp/random", }, }, - Steps: []r.TestStep{ + Steps: []resource.TestStep{ { Config: `resource "random_string" "one" { length = 16 @@ -56,7 +56,7 @@ func Test_Random_ForcesRecreate(t *testing.T) { Config: `resource "random_string" "one" { length = 15 }`, - ConfigPlanChecks: r.ConfigPlanChecks{ + ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("random_string.one", plancheck.ResourceActionDestroyBeforeCreate), }, @@ -75,20 +75,20 @@ package example_test import ( "testing" - r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func Test_Time_UpdateInPlace(t *testing.T) { t.Parallel() - r.Test(t, r.TestCase{ - ExternalProviders: map[string]r.ExternalProvider{ + resource.Test(t, resource.TestCase{ + ExternalProviders: map[string]resource.ExternalProvider{ "time": { Source: "registry.terraform.io/hashicorp/time", }, }, - Steps: []r.TestStep{ + Steps: []resource.TestStep{ { Config: `resource "time_offset" "one" { offset_days = 1 @@ -98,7 +98,7 @@ func Test_Time_UpdateInPlace(t *testing.T) { Config: `resource "time_offset" "one" { offset_days = 2 }`, - ConfigPlanChecks: r.ConfigPlanChecks{ + ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("time_offset.one", plancheck.ResourceActionUpdate), }, @@ -116,20 +116,20 @@ package example_test import ( "testing" - r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func Test_Time_UpdateInPlace_and_NoOp(t *testing.T) { t.Parallel() - r.Test(t, r.TestCase{ - ExternalProviders: map[string]r.ExternalProvider{ + resource.Test(t, resource.TestCase{ + ExternalProviders: map[string]resource.ExternalProvider{ "time": { Source: "registry.terraform.io/hashicorp/time", }, }, - Steps: []r.TestStep{ + Steps: []resource.TestStep{ { Config: `resource "time_offset" "one" { offset_days = 1 @@ -145,7 +145,7 @@ func Test_Time_UpdateInPlace_and_NoOp(t *testing.T) { resource "time_offset" "two" { offset_days = 1 }`, - ConfigPlanChecks: r.ConfigPlanChecks{ + ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction("time_offset.one", plancheck.ResourceActionUpdate), plancheck.ExpectResourceAction("time_offset.two", plancheck.ResourceActionNoop), @@ -205,20 +205,20 @@ import ( "fmt" "testing" - r "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/plancheck" ) func Test_CustomPlanCheck_ExpectEmptyPlan(t *testing.T) { t.Parallel() - r.Test(t, r.TestCase{ - ExternalProviders: map[string]r.ExternalProvider{ + resource.Test(t, resource.TestCase{ + ExternalProviders: map[string]resource.ExternalProvider{ "random": { Source: "registry.terraform.io/hashicorp/random", }, }, - Steps: []r.TestStep{ + Steps: []resource.TestStep{ { Config: `resource "random_string" "one" { length = 16 @@ -228,7 +228,7 @@ func Test_CustomPlanCheck_ExpectEmptyPlan(t *testing.T) { Config: `resource "random_string" "one" { length = 16 }`, - ConfigPlanChecks: r.ConfigPlanChecks{ + ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ ExpectEmptyPlan(), }, From 6ea297a0535be723e5d200cac09f7f8c3ba8236a Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 22 Mar 2023 11:25:02 -0400 Subject: [PATCH 30/31] added table with built-ins --- .../docs/plugin/testing/acceptance-tests/plan-checks.mdx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx b/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx index 165c071fb..fa91a402f 100644 --- a/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx +++ b/website/docs/plugin/testing/acceptance-tests/plan-checks.mdx @@ -19,7 +19,13 @@ A **plan check** is a test assertion that inspects the plan file at a specific p ## Built-in Plan Checks -The `terraform-plugin-testing` module provides a package [`plancheck`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck) with built-in plan checks for common use-cases. +The `terraform-plugin-testing` module provides a package [`plancheck`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck) with built-in plan checks for common use-cases: + +| Check | Description | +|---|---| +| [`plancheck.ExpectEmptyPlan()`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#ExpectEmptyPlan) | Asserts the entire plan has no operations for apply. | +| [`plancheck.ExpectNonEmptyPlan()`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#ExpectNonEmptyPlan) | Asserts the entire plan contains at least one operation for apply. | +| [`plancheck.ExpectResourceAction(address, operation)`](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing/plancheck#ExpectResourceAction) | Asserts the given resource has the specified operation for apply. | ### Examples using `plancheck.ExpectResourceAction` From 5dc04ce8cbdb01a635eae29e7d5c45b5db51266d Mon Sep 17 00:00:00 2001 From: Austin Valle Date: Wed, 22 Mar 2023 11:36:14 -0400 Subject: [PATCH 31/31] swap nav + add section to point to plan checks --- website/data/plugin-testing-nav-data.json | 8 ++++---- website/docs/plugin/testing/acceptance-tests/teststep.mdx | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/website/data/plugin-testing-nav-data.json b/website/data/plugin-testing-nav-data.json index b425ef400..a3699b093 100644 --- a/website/data/plugin-testing-nav-data.json +++ b/website/data/plugin-testing-nav-data.json @@ -17,13 +17,13 @@ "title": "Test Steps", "path": "acceptance-tests/teststep" }, - { - "title": "Sweepers", - "path": "acceptance-tests/sweepers" - }, { "title": "Plan Checks", "path": "acceptance-tests/plan-checks" + }, + { + "title": "Sweepers", + "path": "acceptance-tests/sweepers" } ] }, diff --git a/website/docs/plugin/testing/acceptance-tests/teststep.mdx b/website/docs/plugin/testing/acceptance-tests/teststep.mdx index 6df9179e3..0c66a9ff7 100644 --- a/website/docs/plugin/testing/acceptance-tests/teststep.mdx +++ b/website/docs/plugin/testing/acceptance-tests/teststep.mdx @@ -82,7 +82,7 @@ applying and checking a basic configuration, followed by applying a modified configuration with updated or additional checks is a common pattern used to test update functionality. -## Check Functions +## State Check Functions After the configuration for a `TestStep` is applied, Terraform’s testing framework provides developers an opportunity to check the results by providing a @@ -294,6 +294,9 @@ func testAccCheckExampleWidgetExists(resourceName string, widget *example.Widget } ``` -## Next Steps +## Plan Checks +Before and after the configuration for a `TestStep` is applied, Terraform's testing framework provides developers an opportunity to make test assertions against `terraform plan` results via the plan file. This is provided via [Plan Checks](/terraform/plugin/testing/acceptance-tests/plan-checks), which provide both built-in plan checks and an interface to implement custom plan checks. + +## Sweepers Acceptance Testing is an essential approach to validating the implementation of a Terraform Provider. Using actual APIs to provision resources for testing can leave behind real infrastructure that costs money between tests. The reasons for these leaks can vary, regardless Terraform provides a mechanism known as [Sweepers](/terraform/plugin/testing/acceptance-tests/sweepers) to help keep the testing account clean.