From 09a236af429125fdbce1ce06f672aa72efb3063d Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 31 May 2022 10:17:25 -0400 Subject: [PATCH] helper/resource: Support TestStep provider handling (#972) Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/253 Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/628 Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/779 Provider developers can now select whether to configure providers for acceptance testing at the `TestCase` or `TestStep` level. Only one level may be used in this current implementation, however it may be possible to allow merged `TestCase` and `TestStep` configuration with additional validation logic to ensure a single provider is not specified multiple times across the merge result of all fields. This change also introduces some upfront `TestCase` and `TestStep` configuration validation when calling any of the `Test` functions, failing the test early if a problem is detected. There are other validations that are possible, however these are considered out of scope. --- .changelog/972.txt | 11 + helper/resource/plugin.go | 99 +++- helper/resource/plugin_test.go | 236 +++++++++ helper/resource/testcase_providers.go | 56 +++ helper/resource/testcase_providers_test.go | 298 ++++++++++++ helper/resource/testcase_validate.go | 85 ++++ helper/resource/testcase_validate_test.go | 170 +++++++ helper/resource/testing.go | 154 ++++-- helper/resource/testing_new.go | 184 +++++--- helper/resource/testing_new_config.go | 90 +--- helper/resource/testing_new_import_state.go | 22 +- helper/resource/testing_test.go | 76 +-- helper/resource/teststep_providers.go | 48 ++ helper/resource/teststep_providers_test.go | 499 ++++++++++++++++++++ helper/resource/teststep_validate.go | 99 ++++ helper/resource/teststep_validate_test.go | 184 ++++++++ internal/plugintest/working_dir.go | 4 +- 17 files changed, 2077 insertions(+), 238 deletions(-) create mode 100644 .changelog/972.txt create mode 100644 helper/resource/plugin_test.go create mode 100644 helper/resource/testcase_providers.go create mode 100644 helper/resource/testcase_providers_test.go create mode 100644 helper/resource/testcase_validate.go create mode 100644 helper/resource/testcase_validate_test.go create mode 100644 helper/resource/teststep_providers.go create mode 100644 helper/resource/teststep_providers_test.go create mode 100644 helper/resource/teststep_validate.go create mode 100644 helper/resource/teststep_validate_test.go diff --git a/.changelog/972.txt b/.changelog/972.txt new file mode 100644 index 00000000000..e5660e47a82 --- /dev/null +++ b/.changelog/972.txt @@ -0,0 +1,11 @@ +```release-note:note +helper/resource: Provider references or external installation can now be handled at either the `TestCase` or `TestStep` level. Using the `TestStep` handling, advanced use cases are now enabled such as state upgrade acceptance testing. +``` + +```release-note:enhancement +helper/resource: Added `TestStep` type `ExternalProviders`, `ProtoV5ProviderFactories`, `ProtoV6ProviderFactories`, and `ProviderFactories` fields +``` + +```release-note:bug +helper/resource: Removed extraneous `terraform state show` command when not using the `TestStep` type `Taint` field +``` diff --git a/helper/resource/plugin.go b/helper/resource/plugin.go index d9bc172ea9a..9e52348d69d 100644 --- a/helper/resource/plugin.go +++ b/helper/resource/plugin.go @@ -19,17 +19,108 @@ import ( testing "github.com/mitchellh/go-testing-interface" ) +// protov5ProviderFactory is a function which is called to start a protocol +// version 5 provider server. +type protov5ProviderFactory func() (tfprotov5.ProviderServer, error) + +// protov5ProviderFactories is a mapping of provider addresses to provider +// factory for protocol version 5 provider servers. +type protov5ProviderFactories map[string]func() (tfprotov5.ProviderServer, error) + +// merge combines provider factories. +// +// In case of an overlapping entry, the later entry will overwrite the previous +// value. +func (pf protov5ProviderFactories) merge(otherPfs ...protov5ProviderFactories) protov5ProviderFactories { + result := make(protov5ProviderFactories) + + for name, providerFactory := range pf { + result[name] = providerFactory + } + + for _, otherPf := range otherPfs { + for name, providerFactory := range otherPf { + result[name] = providerFactory + } + } + + return result +} + +// protov6ProviderFactory is a function which is called to start a protocol +// version 6 provider server. +type protov6ProviderFactory func() (tfprotov6.ProviderServer, error) + +// protov6ProviderFactories is a mapping of provider addresses to provider +// factory for protocol version 6 provider servers. +type protov6ProviderFactories map[string]func() (tfprotov6.ProviderServer, error) + +// merge combines provider factories. +// +// In case of an overlapping entry, the later entry will overwrite the previous +// value. +func (pf protov6ProviderFactories) merge(otherPfs ...protov6ProviderFactories) protov6ProviderFactories { + result := make(protov6ProviderFactories) + + for name, providerFactory := range pf { + result[name] = providerFactory + } + + for _, otherPf := range otherPfs { + for name, providerFactory := range otherPf { + result[name] = providerFactory + } + } + + return result +} + +// sdkProviderFactory is a function which is called to start a SDK provider +// server. +type sdkProviderFactory func() (*schema.Provider, error) + +// protov6ProviderFactories is a mapping of provider addresses to provider +// factory for protocol version 6 provider servers. +type sdkProviderFactories map[string]func() (*schema.Provider, error) + +// merge combines provider factories. +// +// In case of an overlapping entry, the later entry will overwrite the previous +// value. +func (pf sdkProviderFactories) merge(otherPfs ...sdkProviderFactories) sdkProviderFactories { + result := make(sdkProviderFactories) + + for name, providerFactory := range pf { + result[name] = providerFactory + } + + for _, otherPf := range otherPfs { + for name, providerFactory := range otherPf { + result[name] = providerFactory + } + } + + return result +} + type providerFactories struct { - legacy map[string]func() (*schema.Provider, error) - protov5 map[string]func() (tfprotov5.ProviderServer, error) - protov6 map[string]func() (tfprotov6.ProviderServer, error) + legacy sdkProviderFactories + protov5 protov5ProviderFactories + protov6 protov6ProviderFactories } -func runProviderCommand(ctx context.Context, t testing.T, f func() error, wd *plugintest.WorkingDir, factories providerFactories) error { +func runProviderCommand(ctx context.Context, t testing.T, f func() error, wd *plugintest.WorkingDir, factories *providerFactories) error { // don't point to this as a test failure location // point to whatever called it t.Helper() + // This should not happen, but prevent panics just in case. + if factories == nil { + err := fmt.Errorf("Provider factories are missing to run Terraform command. Please report this bug in the testing framework.") + logging.HelperResourceError(ctx, err.Error()) + return err + } + // Run the providers in the same process as the test runner using the // reattach behavior in Terraform. This ensures we get test coverage // and enables the use of delve as a debugger. diff --git a/helper/resource/plugin_test.go b/helper/resource/plugin_test.go new file mode 100644 index 00000000000..c7389bdcb68 --- /dev/null +++ b/helper/resource/plugin_test.go @@ -0,0 +1,236 @@ +package resource + +import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestProtoV5ProviderFactoriesMerge(t *testing.T) { + t.Parallel() + + testProviderFactory1 := func() (tfprotov5.ProviderServer, error) { + return nil, nil + } + testProviderFactory2 := func() (tfprotov5.ProviderServer, error) { + return nil, nil + } + + // Function pointers do not play well with go-cmp, so convert these + // into their stringified address for comparison. + transformer := cmp.Transformer( + "protov5ProviderFactory", + func(pf protov5ProviderFactory) string { + return fmt.Sprintf("%v", pf) + }, + ) + + testCases := map[string]struct { + pf protov5ProviderFactories + others []protov5ProviderFactories + expected protov5ProviderFactories + }{ + "no-overlap": { + pf: protov5ProviderFactories{ + "test1": testProviderFactory1, + }, + others: []protov5ProviderFactories{ + { + "test2": testProviderFactory1, + }, + { + "test3": testProviderFactory1, + }, + }, + expected: protov5ProviderFactories{ + "test1": testProviderFactory1, + "test2": testProviderFactory1, + "test3": testProviderFactory1, + }, + }, + "overlap": { + pf: protov5ProviderFactories{ + "test": testProviderFactory1, + }, + others: []protov5ProviderFactories{ + { + "test": testProviderFactory1, + }, + { + "test": testProviderFactory2, + }, + }, + expected: protov5ProviderFactories{ + "test": testProviderFactory2, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := testCase.pf.merge(testCase.others...) + + if diff := cmp.Diff(got, testCase.expected, transformer); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + +func TestProtoV6ProviderFactoriesMerge(t *testing.T) { + t.Parallel() + + testProviderFactory1 := func() (tfprotov6.ProviderServer, error) { + return nil, nil + } + testProviderFactory2 := func() (tfprotov6.ProviderServer, error) { + return nil, nil + } + + // Function pointers do not play well with go-cmp, so convert these + // into their stringified address for comparison. + transformer := cmp.Transformer( + "protov6ProviderFactory", + func(pf protov6ProviderFactory) string { + return fmt.Sprintf("%v", pf) + }, + ) + + testCases := map[string]struct { + pf protov6ProviderFactories + others []protov6ProviderFactories + expected protov6ProviderFactories + }{ + "no-overlap": { + pf: protov6ProviderFactories{ + "test1": testProviderFactory1, + }, + others: []protov6ProviderFactories{ + { + "test2": testProviderFactory1, + }, + { + "test3": testProviderFactory1, + }, + }, + expected: protov6ProviderFactories{ + "test1": testProviderFactory1, + "test2": testProviderFactory1, + "test3": testProviderFactory1, + }, + }, + "overlap": { + pf: protov6ProviderFactories{ + "test": testProviderFactory1, + }, + others: []protov6ProviderFactories{ + { + "test": testProviderFactory1, + }, + { + "test": testProviderFactory2, + }, + }, + expected: protov6ProviderFactories{ + "test": testProviderFactory2, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := testCase.pf.merge(testCase.others...) + + if diff := cmp.Diff(got, testCase.expected, transformer); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + +func TestSdkProviderFactoriesMerge(t *testing.T) { + t.Parallel() + + testProviderFactory1 := func() (*schema.Provider, error) { + return nil, nil + } + testProviderFactory2 := func() (*schema.Provider, error) { + return nil, nil + } + + // Function pointers do not play well with go-cmp, so convert these + // into their stringified address for comparison. + transformer := cmp.Transformer( + "sdkProviderFactory", + func(pf sdkProviderFactory) string { + return fmt.Sprintf("%v", pf) + }, + ) + + testCases := map[string]struct { + pf sdkProviderFactories + others []sdkProviderFactories + expected sdkProviderFactories + }{ + "no-overlap": { + pf: sdkProviderFactories{ + "test1": testProviderFactory1, + }, + others: []sdkProviderFactories{ + { + "test2": testProviderFactory1, + }, + { + "test3": testProviderFactory1, + }, + }, + expected: sdkProviderFactories{ + "test1": testProviderFactory1, + "test2": testProviderFactory1, + "test3": testProviderFactory1, + }, + }, + "overlap": { + pf: sdkProviderFactories{ + "test": testProviderFactory1, + }, + others: []sdkProviderFactories{ + { + "test": testProviderFactory1, + }, + { + "test": testProviderFactory2, + }, + }, + expected: sdkProviderFactories{ + "test": testProviderFactory2, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := testCase.pf.merge(testCase.others...) + + if diff := cmp.Diff(got, testCase.expected, transformer); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} diff --git a/helper/resource/testcase_providers.go b/helper/resource/testcase_providers.go new file mode 100644 index 00000000000..c09e4657cc2 --- /dev/null +++ b/helper/resource/testcase_providers.go @@ -0,0 +1,56 @@ +package resource + +import ( + "context" + "fmt" + "strings" +) + +// providerConfig takes the list of providers in a TestCase and returns a +// config with only empty provider blocks. This is useful for Import, where no +// config is provided, but the providers must be defined. +func (c TestCase) providerConfig(_ context.Context) string { + var providerBlocks, requiredProviderBlocks strings.Builder + + // [BF] The Providers field handling predates the logic being moved to this + // method. It's not entirely clear to me at this time why this field + // is being used and not the others, but leaving it here just in case + // it does have a special purpose that wasn't being unit tested prior. + for name := range c.Providers { + providerBlocks.WriteString(fmt.Sprintf("provider %q {}\n", name)) + } + + for name, externalProvider := range c.ExternalProviders { + providerBlocks.WriteString(fmt.Sprintf("provider %q {}\n", name)) + + if externalProvider.Source == "" && externalProvider.VersionConstraint == "" { + continue + } + + requiredProviderBlocks.WriteString(fmt.Sprintf(" %s = {\n", name)) + + if externalProvider.Source != "" { + requiredProviderBlocks.WriteString(fmt.Sprintf(" source = %q\n", externalProvider.Source)) + } + + if externalProvider.VersionConstraint != "" { + requiredProviderBlocks.WriteString(fmt.Sprintf(" version = %q\n", externalProvider.VersionConstraint)) + } + + requiredProviderBlocks.WriteString(" }\n") + } + + if requiredProviderBlocks.Len() > 0 { + return fmt.Sprintf(` +terraform { + required_providers { +%[1]s + } +} + +%[2]s +`, strings.TrimSuffix(requiredProviderBlocks.String(), "\n"), providerBlocks.String()) + } + + return providerBlocks.String() +} diff --git a/helper/resource/testcase_providers_test.go b/helper/resource/testcase_providers_test.go new file mode 100644 index 00000000000..706aba522d6 --- /dev/null +++ b/helper/resource/testcase_providers_test.go @@ -0,0 +1,298 @@ +package resource + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestTestCaseProviderConfig(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + testCase TestCase + expected string + }{ + "externalproviders-missing-source-and-versionconstraint": { + testCase: TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "test": {}, + }, + }, + expected: `provider "test" {}`, + }, + "externalproviders-source-and-versionconstraint": { + testCase: TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "test": { + Source: "registry.terraform.io/hashicorp/test", + VersionConstraint: "1.2.3", + }, + }, + }, + expected: ` +terraform { + required_providers { + test = { + source = "registry.terraform.io/hashicorp/test" + version = "1.2.3" + } + } +} + +provider "test" {} +`, + }, + "externalproviders-source": { + testCase: TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "test": { + Source: "registry.terraform.io/hashicorp/test", + }, + }, + }, + expected: ` +terraform { + required_providers { + test = { + source = "registry.terraform.io/hashicorp/test" + } + } +} + +provider "test" {} +`, + }, + "externalproviders-versionconstraint": { + testCase: TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "test": { + VersionConstraint: "1.2.3", + }, + }, + }, + expected: ` +terraform { + required_providers { + test = { + version = "1.2.3" + } + } +} + +provider "test" {} +`, + }, + "protov5providerfactories": { + testCase: TestCase{ + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": nil, + }, + }, + expected: ``, + }, + "protov6providerfactories": { + testCase: TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": nil, + }, + }, + expected: ``, + }, + "providerfactories": { + testCase: TestCase{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": nil, + }, + }, + expected: ``, + }, + "providers": { + testCase: TestCase{ + Providers: map[string]*schema.Provider{ + "test": {}, + }, + }, + expected: `provider "test" {}`, + }, + } + + for name, test := range tests { + name, test := name, test + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := test.testCase.providerConfig(context.Background()) + + if diff := cmp.Diff(strings.TrimSpace(got), strings.TrimSpace(test.expected)); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + +func TestTest_TestCase_ExternalProviders(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "null": { + Source: "registry.terraform.io/hashicorp/null", + }, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) +} + +func TestTest_TestCase_ExternalProviders_Error(t *testing.T) { + t.Parallel() + + testExpectTFatal(t, func() { + Test(&mockT{}, TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "testnonexistent": { + Source: "registry.terraform.io/hashicorp/testnonexistent", + }, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) + }) +} + +func TestTest_TestCase_ProtoV5ProviderFactories(t *testing.T) { + t.Parallel() + + Test(&mockT{}, TestCase{ + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": func() (tfprotov5.ProviderServer, error) { //nolint:unparam // required signature + return nil, nil + }, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) +} + +func TestTest_TestCase_ProtoV5ProviderFactories_Error(t *testing.T) { + t.Parallel() + + testExpectTFatal(t, func() { + Test(&mockT{}, TestCase{ + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": func() (tfprotov5.ProviderServer, error) { //nolint:unparam // required signature + return nil, fmt.Errorf("test") + }, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) + }) +} + +func TestTest_TestCase_ProtoV6ProviderFactories(t *testing.T) { + t.Parallel() + + Test(&mockT{}, TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": func() (tfprotov6.ProviderServer, error) { //nolint:unparam // required signature + return nil, nil + }, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) +} + +func TestTest_TestCase_ProtoV6ProviderFactories_Error(t *testing.T) { + t.Parallel() + + testExpectTFatal(t, func() { + Test(&mockT{}, TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": func() (tfprotov6.ProviderServer, error) { //nolint:unparam // required signature + return nil, fmt.Errorf("test") + }, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) + }) +} + +func TestTest_TestCase_ProviderFactories(t *testing.T) { + t.Parallel() + + Test(&mockT{}, TestCase{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": func() (*schema.Provider, error) { //nolint:unparam // required signature + return nil, nil + }, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) +} + +func TestTest_TestCase_ProviderFactories_Error(t *testing.T) { + t.Parallel() + + testExpectTFatal(t, func() { + Test(&mockT{}, TestCase{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": func() (*schema.Provider, error) { //nolint:unparam // required signature + return nil, fmt.Errorf("test") + }, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) + }) +} + +func TestTest_TestCase_Providers(t *testing.T) { + t.Parallel() + + Test(&mockT{}, TestCase{ + Providers: map[string]*schema.Provider{ + "test": {}, + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) +} diff --git a/helper/resource/testcase_validate.go b/helper/resource/testcase_validate.go new file mode 100644 index 00000000000..39e5da46c9c --- /dev/null +++ b/helper/resource/testcase_validate.go @@ -0,0 +1,85 @@ +package resource + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" +) + +// hasProviders returns true if the TestCase has set any of the +// ExternalProviders, ProtoV5ProviderFactories, ProtoV6ProviderFactories, +// ProviderFactories, or Providers fields. +func (c TestCase) hasProviders(_ context.Context) bool { + if len(c.ExternalProviders) > 0 { + return true + } + + if len(c.ProtoV5ProviderFactories) > 0 { + return true + } + + if len(c.ProtoV6ProviderFactories) > 0 { + return true + } + + if len(c.ProviderFactories) > 0 { + return true + } + + if len(c.Providers) > 0 { + return true + } + + return false +} + +// validate ensures the TestCase is valid based on the following criteria: +// +// - No overlapping ExternalProviders and Providers entries +// - No overlapping ExternalProviders and ProviderFactories entries +// - TestStep validations performed by the (TestStep).validate() method. +// +func (c TestCase) validate(ctx context.Context) error { + logging.HelperResourceTrace(ctx, "Validating TestCase") + + if len(c.Steps) == 0 { + err := fmt.Errorf("TestCase missing Steps") + logging.HelperResourceError(ctx, "TestCase validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + for name := range c.ExternalProviders { + if _, ok := c.Providers[name]; ok { + err := fmt.Errorf("TestCase provider %q set in both ExternalProviders and Providers", name) + logging.HelperResourceError(ctx, "TestCase validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if _, ok := c.ProviderFactories[name]; ok { + err := fmt.Errorf("TestCase provider %q set in both ExternalProviders and ProviderFactories", name) + logging.HelperResourceError(ctx, "TestCase validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + } + + testCaseHasProviders := c.hasProviders(ctx) + + for stepIndex, step := range c.Steps { + stepNumber := stepIndex + 1 // Use 1-based index for humans + stepValidateReq := testStepValidateRequest{ + StepNumber: stepNumber, + TestCaseHasProviders: testCaseHasProviders, + } + + err := step.validate(ctx, stepValidateReq) + + if err != nil { + err := fmt.Errorf("TestStep %d/%d validation error: %w", stepNumber, len(c.Steps), err) + logging.HelperResourceError(ctx, "TestCase validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + } + + return nil +} diff --git a/helper/resource/testcase_validate_test.go b/helper/resource/testcase_validate_test.go new file mode 100644 index 00000000000..97d00ac7c3f --- /dev/null +++ b/helper/resource/testcase_validate_test.go @@ -0,0 +1,170 @@ +package resource + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestTestCaseHasProviders(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + testCase TestCase + expected bool + }{ + "none": { + testCase: TestCase{}, + expected: false, + }, + "externalproviders": { + testCase: TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "test": {}, // does not need to be real + }, + }, + expected: true, + }, + "protov5providerfactories": { + testCase: TestCase{ + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": nil, // does not need to be real + }, + }, + expected: true, + }, + "protov6providerfactories": { + testCase: TestCase{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": nil, // does not need to be real + }, + }, + expected: true, + }, + "providers": { + testCase: TestCase{ + Providers: map[string]*schema.Provider{ + "test": nil, // does not need to be real + }, + }, + expected: true, + }, + "providerfactories": { + testCase: TestCase{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": nil, // does not need to be real + }, + }, + expected: true, + }, + } + + for name, test := range tests { + name, test := name, test + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := test.testCase.hasProviders(context.Background()) + + if got != test.expected { + t.Errorf("expected %t, got %t", test.expected, got) + } + }) + } +} + +func TestTestCaseValidate(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + testCase TestCase + expectedError error + }{ + "valid": { + testCase: TestCase{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": nil, // does not need to be real + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }, + }, + "externalproviders-overlapping-providers": { + testCase: TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "test": {}, // does not need to be real + }, + Providers: map[string]*schema.Provider{ + "test": nil, // does not need to be real + }, + Steps: []TestStep{ + { + Config: "", + }, + }, + }, + expectedError: fmt.Errorf("TestCase provider \"test\" set in both ExternalProviders and Providers"), + }, + "externalproviders-overlapping-providerfactories": { + testCase: TestCase{ + ExternalProviders: map[string]ExternalProvider{ + "test": {}, // does not need to be real + }, + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": nil, // does not need to be real + }, + Steps: []TestStep{ + { + Config: "", + }, + }, + }, + expectedError: fmt.Errorf("TestCase provider \"test\" set in both ExternalProviders and ProviderFactories"), + }, + "steps-missing": { + testCase: TestCase{}, + expectedError: fmt.Errorf("TestCase missing Steps"), + }, + "steps-validate-error": { + testCase: TestCase{ + Steps: []TestStep{ + {}, + }, + }, + expectedError: fmt.Errorf("TestStep 1/1 validation error"), + }, + } + + for name, test := range tests { + name, test := name, test + + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := test.testCase.validate(context.Background()) + + if err != nil { + if test.expectedError == nil { + t.Fatalf("unexpected error: %s", err) + } + + if !strings.Contains(err.Error(), test.expectedError.Error()) { + t.Fatalf("expected error %q, got: %s", test.expectedError, err) + } + } + + if err == nil && test.expectedError != nil { + t.Errorf("expected error: %s", test.expectedError) + } + }) + } +} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 45180d2c779..8047a2cdc85 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -318,6 +318,11 @@ type TestCase struct { // ProviderFactories can be specified for the providers that are valid. // + // This can also be specified at the TestStep level to enable per-step + // differences in providers, however all provider specifications must + // be done either at the TestCase level or TestStep level, otherwise the + // testing framework will raise an error and fail the test. + // // These are the providers that can be referenced within the test. Each key // is an individually addressable provider. Typically you will only pass a // single value here for the provider you are testing. Aliases are not @@ -339,6 +344,11 @@ type TestCase struct { // ProtoV5ProviderFactories serves the same purpose as ProviderFactories, // but for protocol v5 providers defined using the terraform-plugin-go // ProviderServer interface. + // + // This can also be specified at the TestStep level to enable per-step + // differences in providers, however all provider specifications must + // be done either at the TestCase level or TestStep level, otherwise the + // testing framework will raise an error and fail the test. ProtoV5ProviderFactories map[string]func() (tfprotov5.ProviderServer, error) // ProtoV6ProviderFactories serves the same purpose as ProviderFactories, @@ -346,6 +356,11 @@ type TestCase struct { // ProviderServer interface. // The version of Terraform used in acceptance testing must be greater // than or equal to v0.15.4 to use ProtoV6ProviderFactories. + // + // This can also be specified at the TestStep level to enable per-step + // differences in providers, however all provider specifications must + // be done either at the TestCase level or TestStep level, otherwise the + // testing framework will raise an error and fail the test. ProtoV6ProviderFactories map[string]func() (tfprotov6.ProviderServer, error) // Providers is the ResourceProvider that will be under test. @@ -354,11 +369,18 @@ type TestCase struct { Providers map[string]*schema.Provider // ExternalProviders are providers the TestCase relies on that should - // be downloaded from the registry during init. This is only really - // necessary to set if you're using import, as providers in your config - // will be automatically retrieved during init. Import doesn't use a - // config, however, so we allow manually specifying them here to be - // downloaded for import tests. + // be downloaded from the registry during init. + // + // This can also be specified at the TestStep level to enable per-step + // differences in providers, however all provider specifications must + // be done either at the TestCase level or TestStep level, otherwise the + // testing framework will raise an error and fail the test. + // + // This is generally unnecessary to set at the TestCase level, however + // it has existing in the testing framework prior to the introduction of + // TestStep level specification and was only necessary for performing + // import testing where the configuration contained a provider outside the + // one under test. ExternalProviders map[string]ExternalProvider // PreventPostDestroyRefresh can be set to true for cases where data sources @@ -540,6 +562,74 @@ type TestStep struct { // fields that can't be refreshed and don't matter. ImportStateVerify bool ImportStateVerifyIgnore []string + + // ProviderFactories can be specified for the providers that are valid for + // this TestStep. When providers are specified at the TestStep level, all + // TestStep within a TestCase must declare providers. + // + // This can also be specified at the TestCase level for all TestStep, + // however all provider specifications must be done either at the TestCase + // level or TestStep level, otherwise the testing framework will raise an + // error and fail the test. + // + // These are the providers that can be referenced within the test. Each key + // is an individually addressable provider. Typically you will only pass a + // single value here for the provider you are testing. Aliases are not + // supported by the test framework, so to use multiple provider instances, + // you should add additional copies to this map with unique names. To set + // their configuration, you would reference them similar to the following: + // + // provider "my_factory_key" { + // # ... + // } + // + // resource "my_resource" "mr" { + // provider = my_factory_key + // + // # ... + // } + ProviderFactories map[string]func() (*schema.Provider, error) + + // ProtoV5ProviderFactories serves the same purpose as ProviderFactories, + // but for protocol v5 providers defined using the terraform-plugin-go + // ProviderServer interface. When providers are specified at the TestStep + // level, all TestStep within a TestCase must declare providers. + // + // This can also be specified at the TestCase level for all TestStep, + // however all provider specifications must be done either at the TestCase + // level or TestStep level, otherwise the testing framework will raise an + // error and fail the test. + ProtoV5ProviderFactories map[string]func() (tfprotov5.ProviderServer, error) + + // ProtoV6ProviderFactories serves the same purpose as ProviderFactories, + // but for protocol v6 providers defined using the terraform-plugin-go + // ProviderServer interface. + // The version of Terraform used in acceptance testing must be greater + // than or equal to v0.15.4 to use ProtoV6ProviderFactories. When providers + // are specified at the TestStep level, all TestStep within a TestCase must + // declare providers. + // + // This can also be specified at the TestCase level for all TestStep, + // however all provider specifications must be done either at the TestCase + // level or TestStep level, otherwise the testing framework will raise an + // error and fail the test. + ProtoV6ProviderFactories map[string]func() (tfprotov6.ProviderServer, error) + + // ExternalProviders are providers the TestStep relies on that should + // be downloaded from the registry during init. When providers are + // specified at the TestStep level, all TestStep within a TestCase must + // declare providers. + // + // This can also be specified at the TestCase level for all TestStep, + // however all provider specifications must be done either at the TestCase + // level or TestStep level, otherwise the testing framework will raise an + // error and fail the test. + // + // Outside specifying an earlier version of the provider under test, + // typically for state upgrader testing, this is generally only necessary + // for performing import testing where the prior TestStep configuration + // contained a provider outside the one under test. + ExternalProviders map[string]ExternalProvider } // ParallelTest performs an acceptance test on a resource, allowing concurrency @@ -593,6 +683,16 @@ func Test(t testing.T, c TestCase) { ctx := context.Background() ctx = logging.InitTestContext(ctx, t) + err := c.validate(ctx) + + if err != nil { + logging.HelperResourceError(ctx, + "Test validation error", + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("Test validation error: %s", err) + } + // We only run acceptance tests if an env var is set because they're // slow and generally require some outside configuration. You can opt out // of this with OverrideEnvVar on individual TestCases. @@ -608,9 +708,6 @@ func Test(t testing.T, c TestCase) { c.ProviderFactories = map[string]func() (*schema.Provider, error){} for name, p := range c.Providers { - if _, ok := c.ProviderFactories[name]; ok { - t.Fatalf("ProviderFactory for %q already exists, cannot overwrite with Provider", name) - } prov := p c.ProviderFactories[name] = func() (*schema.Provider, error) { //nolint:unparam // required signature return prov, nil @@ -648,43 +745,6 @@ func Test(t testing.T, c TestCase) { logging.HelperResourceDebug(ctx, "Finished TestCase") } -// testProviderConfig takes the list of Providers in a TestCase and returns a -// config with only empty provider blocks. This is useful for Import, where no -// config is provided, but the providers must be defined. -func testProviderConfig(c TestCase) (string, error) { - var lines []string - var requiredProviders []string - for p := range c.Providers { - lines = append(lines, fmt.Sprintf("provider %q {}\n", p)) - } - for p, v := range c.ExternalProviders { - if _, ok := c.Providers[p]; ok { - return "", fmt.Errorf("Provider %q set in both Providers and ExternalProviders for TestCase. Must be set in only one.", p) - } - if _, ok := c.ProviderFactories[p]; ok { - return "", fmt.Errorf("Provider %q set in both ProviderFactories and ExternalProviders for TestCase. Must be set in only one.", p) - } - lines = append(lines, fmt.Sprintf("provider %q {}\n", p)) - var providerBlock string - if v.VersionConstraint != "" { - providerBlock = fmt.Sprintf("%s\nversion = %q", providerBlock, v.VersionConstraint) - } - if v.Source != "" { - providerBlock = fmt.Sprintf("%s\nsource = %q", providerBlock, v.Source) - } - if providerBlock != "" { - providerBlock = fmt.Sprintf("%s = {%s\n}\n", p, providerBlock) - } - requiredProviders = append(requiredProviders, providerBlock) - } - - if len(requiredProviders) > 0 { - lines = append([]string{fmt.Sprintf("terraform {\nrequired_providers {\n%s}\n}\n\n", strings.Join(requiredProviders, ""))}, lines...) - } - - return strings.Join(lines, ""), nil -} - // UnitTest is a helper to force the acceptance testing harness to run in the // normal unit test suite. This should only be used for resource that don't // have any external dependencies. @@ -698,10 +758,6 @@ func UnitTest(t testing.T, c TestCase) { } func testResource(c TestStep, state *terraform.State) (*terraform.ResourceState, error) { - if c.ResourceName == "" { - return nil, fmt.Errorf("ResourceName must be set in TestStep") - } - for _, m := range state.Modules { if len(m.Resources) > 0 { if v, ok := m.Resources[c.ResourceName]; ok { diff --git a/helper/resource/testing_new.go b/helper/resource/testing_new.go index 163bd3d668f..f1e607f8342 100644 --- a/helper/resource/testing_new.go +++ b/helper/resource/testing_new.go @@ -10,23 +10,17 @@ import ( tfjson "github.com/hashicorp/terraform-json" testing "github.com/mitchellh/go-testing-interface" - "github.com/hashicorp/terraform-plugin-go/tfprotov5" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" "github.com/hashicorp/terraform-plugin-sdk/v2/internal/plugintest" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) -func runPostTestDestroy(ctx context.Context, t testing.T, c TestCase, wd *plugintest.WorkingDir, factories map[string]func() (*schema.Provider, error), v5factories map[string]func() (tfprotov5.ProviderServer, error), v6factories map[string]func() (tfprotov6.ProviderServer, error), statePreDestroy *terraform.State) error { +func runPostTestDestroy(ctx context.Context, t testing.T, c TestCase, wd *plugintest.WorkingDir, providers *providerFactories, statePreDestroy *terraform.State) error { t.Helper() err := runProviderCommand(ctx, t, func() error { return wd.Destroy(ctx) - }, wd, providerFactories{ - legacy: factories, - protov5: v5factories, - protov6: v6factories}) + }, wd, providers) if err != nil { return err } @@ -55,6 +49,12 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest ctx = logging.TestTerraformPathContext(ctx, wd.GetHelper().TerraformExecPath()) ctx = logging.TestWorkingDirectoryContext(ctx, wd.GetHelper().WorkingDirectory()) + providers := &providerFactories{ + legacy: c.ProviderFactories, + protov5: c.ProtoV5ProviderFactories, + protov6: c.ProtoV6ProviderFactories, + } + defer func() { var statePreDestroy *terraform.State var err error @@ -64,10 +64,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest return err } return nil - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { logging.HelperResourceError(ctx, "Error retrieving state, there may be dangling resources", @@ -78,7 +75,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest } if !stateIsEmpty(statePreDestroy) { - err := runPostTestDestroy(ctx, t, c, wd, c.ProviderFactories, c.ProtoV5ProviderFactories, c.ProtoV6ProviderFactories, statePreDestroy) + err := runPostTestDestroy(ctx, t, c, wd, providers, statePreDestroy) if err != nil { logging.HelperResourceError(ctx, "Error running post-test destroy, there may be dangling resources", @@ -91,36 +88,28 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest wd.Close() }() - providerCfg, err := testProviderConfig(c) - if err != nil { - logging.HelperResourceError(ctx, - "Error creating test provider configuration", - map[string]interface{}{logging.KeyError: err}, - ) - t.Fatalf("Error creating test provider configuration: %s", err.Error()) - } + if c.hasProviders(ctx) { + err := wd.SetConfig(ctx, c.providerConfig(ctx)) - err = wd.SetConfig(ctx, providerCfg) - if err != nil { - logging.HelperResourceError(ctx, - "Error setting test provider configuration", - map[string]interface{}{logging.KeyError: err}, - ) - t.Fatalf("Error setting test provider configuration: %s", err) - } - err = runProviderCommand(ctx, t, func() error { - return wd.Init(ctx) - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) - if err != nil { - logging.HelperResourceError(ctx, - "Error running init", - map[string]interface{}{logging.KeyError: err}, - ) - t.Fatalf("Error running init: %s", err.Error()) - return + if err != nil { + logging.HelperResourceError(ctx, + "TestCase error setting provider configuration", + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("TestCase error setting provider configuration: %s", err) + } + + err = runProviderCommand(ctx, t, func() error { + return wd.Init(ctx) + }, wd, providers) + + if err != nil { + logging.HelperResourceError(ctx, + "TestCase error running init", + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("TestCase error running init: %s", err.Error()) + } } logging.HelperResourceDebug(ctx, "Starting TestSteps") @@ -129,8 +118,9 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest // acts as default for import tests var appliedCfg string - for i, step := range c.Steps { - ctx = logging.TestStepNumberContext(ctx, i+1) + for stepIndex, step := range c.Steps { + stepNumber := stepIndex + 1 // 1-based indexing for humans + ctx = logging.TestStepNumberContext(ctx, stepNumber) logging.HelperResourceDebug(ctx, "Starting TestStep") @@ -155,30 +145,103 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest logging.HelperResourceDebug(ctx, "Called TestStep SkipFunc") if skip { - t.Logf("Skipping step %d/%d due to SkipFunc", i+1, len(c.Steps)) + t.Logf("Skipping step %d/%d due to SkipFunc", stepNumber, len(c.Steps)) logging.HelperResourceWarn(ctx, "Skipping TestStep due to SkipFunc") continue } } + if step.Config != "" && !step.Destroy && len(step.Taint) > 0 { + var state *terraform.State + + err := runProviderCommand(ctx, t, func() error { + var err error + + state, err = getState(ctx, t, wd) + + if err != nil { + return err + } + + return nil + }, wd, providers) + + if err != nil { + logging.HelperResourceError(ctx, + "TestStep error reading prior state before tainting resources", + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("TestStep %d/%d error reading prior state before tainting resources: %s", stepNumber, len(c.Steps), err) + } + + err = testStepTaint(ctx, state, step) + + if err != nil { + logging.HelperResourceError(ctx, + "TestStep error tainting resources", + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("TestStep %d/%d error tainting resources: %s", stepNumber, len(c.Steps), err) + } + } + + if step.hasProviders(ctx) { + providers = &providerFactories{ + legacy: sdkProviderFactories(c.ProviderFactories).merge(step.ProviderFactories), + protov5: protov5ProviderFactories(c.ProtoV5ProviderFactories).merge(step.ProtoV5ProviderFactories), + protov6: protov6ProviderFactories(c.ProtoV6ProviderFactories).merge(step.ProtoV6ProviderFactories), + } + + providerCfg := step.providerConfig(ctx) + + err := wd.SetConfig(ctx, providerCfg) + + if err != nil { + logging.HelperResourceError(ctx, + "TestStep error setting provider configuration", + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("TestStep %d/%d error setting test provider configuration: %s", stepNumber, len(c.Steps), err) + } + + err = runProviderCommand( + ctx, + t, + func() error { + return wd.Init(ctx) + }, + wd, + providers, + ) + + if err != nil { + logging.HelperResourceError(ctx, + "TestStep error running init", + map[string]interface{}{logging.KeyError: err}, + ) + t.Fatalf("TestStep %d/%d running init: %s", stepNumber, len(c.Steps), err.Error()) + return + } + } + if step.ImportState { logging.HelperResourceTrace(ctx, "TestStep is ImportState mode") - err := testStepNewImportState(ctx, t, c, helper, wd, step, appliedCfg) + err := testStepNewImportState(ctx, t, helper, wd, step, appliedCfg, providers) if step.ExpectError != nil { logging.HelperResourceDebug(ctx, "Checking TestStep ExpectError") if err == nil { logging.HelperResourceError(ctx, "Error running import: expected an error but got none", ) - t.Fatalf("Step %d/%d error running import: expected an error but got none", i+1, len(c.Steps)) + t.Fatalf("Step %d/%d error running import: expected an error but got none", stepNumber, len(c.Steps)) } if !step.ExpectError.MatchString(err.Error()) { logging.HelperResourceError(ctx, fmt.Sprintf("Error running import: expected an error with pattern (%s)", step.ExpectError.String()), map[string]interface{}{logging.KeyError: err}, ) - t.Fatalf("Step %d/%d error running import, expected an error with pattern (%s), no match on: %s", i+1, len(c.Steps), step.ExpectError.String(), err) + t.Fatalf("Step %d/%d error running import, expected an error with pattern (%s), no match on: %s", stepNumber, len(c.Steps), step.ExpectError.String(), err) } } else { if err != nil && c.ErrorCheck != nil { @@ -191,7 +254,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest "Error running import", map[string]interface{}{logging.KeyError: err}, ) - t.Fatalf("Step %d/%d error running import: %s", i+1, len(c.Steps), err) + t.Fatalf("Step %d/%d error running import: %s", stepNumber, len(c.Steps), err) } } @@ -203,7 +266,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest if step.Config != "" { logging.HelperResourceTrace(ctx, "TestStep is Config mode") - err := testStepNewConfig(ctx, t, c, wd, step) + err := testStepNewConfig(ctx, t, c, wd, step, providers) if step.ExpectError != nil { logging.HelperResourceDebug(ctx, "Checking TestStep ExpectError") @@ -211,14 +274,14 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest logging.HelperResourceError(ctx, "Expected an error but got none", ) - t.Fatalf("Step %d/%d, expected an error but got none", i+1, len(c.Steps)) + t.Fatalf("Step %d/%d, expected an error but got none", stepNumber, len(c.Steps)) } if !step.ExpectError.MatchString(err.Error()) { logging.HelperResourceError(ctx, fmt.Sprintf("Expected an error with pattern (%s)", step.ExpectError.String()), map[string]interface{}{logging.KeyError: err}, ) - t.Fatalf("Step %d/%d, expected an error with pattern, no match on: %s", i+1, len(c.Steps), err) + t.Fatalf("Step %d/%d, expected an error with pattern, no match on: %s", stepNumber, len(c.Steps), err) } } else { if err != nil && c.ErrorCheck != nil { @@ -233,7 +296,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest "Unexpected error", map[string]interface{}{logging.KeyError: err}, ) - t.Fatalf("Step %d/%d error: %s", i+1, len(c.Steps), err) + t.Fatalf("Step %d/%d error: %s", stepNumber, len(c.Steps), err) } } @@ -244,7 +307,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest continue } - t.Fatalf("Step %d/%d, unsupported test mode", i+1, len(c.Steps)) + t.Fatalf("Step %d/%d, unsupported test mode", stepNumber, len(c.Steps)) } } @@ -277,7 +340,7 @@ func planIsEmpty(plan *tfjson.Plan) bool { return true } -func testIDRefresh(ctx context.Context, t testing.T, c TestCase, wd *plugintest.WorkingDir, step TestStep, r *terraform.ResourceState) error { +func testIDRefresh(ctx context.Context, t testing.T, c TestCase, wd *plugintest.WorkingDir, step TestStep, r *terraform.ResourceState, providers *providerFactories) error { t.Helper() spewConf := spew.NewDefaultConfig() @@ -291,11 +354,7 @@ func testIDRefresh(ctx context.Context, t testing.T, c TestCase, wd *plugintest. // Temporarily set the config to a minimal provider config for the refresh // test. After the refresh we can reset it. - cfg, err := testProviderConfig(c) - if err != nil { - return err - } - err = wd.SetConfig(ctx, cfg) + err := wd.SetConfig(ctx, c.providerConfig(ctx)) if err != nil { t.Fatalf("Error setting import test config: %s", err) } @@ -317,10 +376,7 @@ func testIDRefresh(ctx context.Context, t testing.T, c TestCase, wd *plugintest. return err } return nil - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return err } diff --git a/helper/resource/testing_new_config.go b/helper/resource/testing_new_config.go index 5a9df2bf5f1..09d18c36452 100644 --- a/helper/resource/testing_new_config.go +++ b/helper/resource/testing_new_config.go @@ -13,30 +13,9 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) -func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugintest.WorkingDir, step TestStep) error { +func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugintest.WorkingDir, step TestStep, providers *providerFactories) error { t.Helper() - if !step.Destroy { - var state *terraform.State - var err error - err = runProviderCommand(ctx, t, func() error { - state, err = getState(ctx, t, wd) - if err != nil { - return err - } - return nil - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) - if err != nil { - return err - } - if err := testStepTaint(ctx, state, step); err != nil { - return fmt.Errorf("Error when tainting resources: %s", err) - } - } - err := wd.SetConfig(ctx, step.Config) if err != nil { return fmt.Errorf("Error setting config: %w", err) @@ -46,10 +25,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint // failing to do this will result in data sources not being updated err = runProviderCommand(ctx, t, func() error { return wd.Refresh(ctx) - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error running pre-apply refresh: %w", err) } @@ -66,10 +42,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return wd.CreateDestroyPlan(ctx) } return wd.CreatePlan(ctx) - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error running pre-apply plan: %w", err) } @@ -84,10 +57,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return err } return nil - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error retrieving pre-apply state: %w", err) } @@ -95,10 +65,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint // Apply the diff, creating real resources err = runProviderCommand(ctx, t, func() error { return wd.Apply(ctx) - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { if step.Destroy { return fmt.Errorf("Error running destroy: %w", err) @@ -114,10 +81,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return err } return nil - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error retrieving state after apply: %w", err) } @@ -148,10 +112,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return wd.CreateDestroyPlan(ctx) } return wd.CreatePlan(ctx) - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error running post-apply plan: %w", err) } @@ -161,10 +122,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint var err error plan, err = wd.SavedPlan(ctx) return err - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error retrieving post-apply plan: %w", err) } @@ -175,10 +133,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint var err error stdout, err = wd.SavedPlanRawStdout(ctx) return err - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error retrieving formatted plan output: %w", err) } @@ -189,10 +144,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint if !step.Destroy || (step.Destroy && !step.PreventPostDestroyRefresh) { err := runProviderCommand(ctx, t, func() error { return wd.Refresh(ctx) - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error running post-apply refresh: %w", err) } @@ -204,10 +156,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return wd.CreateDestroyPlan(ctx) } return wd.CreatePlan(ctx) - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error running second post-apply plan: %w", err) } @@ -216,10 +165,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint var err error plan, err = wd.SavedPlan(ctx) return err - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error retrieving second post-apply plan: %w", err) } @@ -231,10 +177,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint var err error stdout, err = wd.SavedPlanRawStdout(ctx) return err - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return fmt.Errorf("Error retrieving formatted second plan output: %w", err) } @@ -257,10 +200,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint return err } return nil - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { return err @@ -290,7 +230,7 @@ func testStepNewConfig(ctx context.Context, t testing.T, c TestCase, wd *plugint // this fails. If refresh isn't read-only, then this will have // caught a different bug. if idRefreshCheck != nil { - if err := testIDRefresh(ctx, t, c, wd, step, idRefreshCheck); err != nil { + if err := testIDRefresh(ctx, t, c, wd, step, idRefreshCheck, providers); err != nil { return fmt.Errorf( "[ERROR] Test: ID-only test failed: %s", err) } diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go index 1dc98143a64..ec61b055f3a 100644 --- a/helper/resource/testing_new_import_state.go +++ b/helper/resource/testing_new_import_state.go @@ -14,7 +14,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) -func testStepNewImportState(ctx context.Context, t testing.T, c TestCase, helper *plugintest.Helper, wd *plugintest.WorkingDir, step TestStep, cfg string) error { +func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest.Helper, wd *plugintest.WorkingDir, step TestStep, cfg string, providers *providerFactories) error { t.Helper() spewConf := spew.NewDefaultConfig() @@ -33,10 +33,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, c TestCase, helper return err } return nil - }, wd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, wd, providers) if err != nil { t.Fatalf("Error getting state: %s", err) } @@ -100,20 +97,14 @@ func testStepNewImportState(ctx context.Context, t testing.T, c TestCase, helper err = runProviderCommand(ctx, t, func() error { return importWd.Init(ctx) - }, importWd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, importWd, providers) if err != nil { t.Fatalf("Error running init: %s", err) } err = runProviderCommand(ctx, t, func() error { return importWd.Import(ctx, step.ResourceName, importId) - }, importWd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, importWd, providers) if err != nil { return err } @@ -125,10 +116,7 @@ func testStepNewImportState(ctx context.Context, t testing.T, c TestCase, helper return err } return nil - }, importWd, providerFactories{ - legacy: c.ProviderFactories, - protov5: c.ProtoV5ProviderFactories, - protov6: c.ProtoV6ProviderFactories}) + }, importWd, providers) if err != nil { t.Fatalf("Error getting state: %s", err) } diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go index 90caae7681d..189ebd03eea 100644 --- a/helper/resource/testing_test.go +++ b/helper/resource/testing_test.go @@ -23,44 +23,64 @@ func init() { } } -func TestParallelTest(t *testing.T) { - mt := new(mockT) +// testExpectTFatal provides a wrapper for logic which should call +// (*testing.T).Fatal() or (*testing.T).Fatalf(). +// +// Since we do not want the wrapping test to fail when an expected test error +// occurs, it is required that the testLogic passed in uses +// github.com/mitchellh/go-testing-interface.RuntimeT instead of the real +// *testing.T. +// +// If Fatal() or Fatalf() is not called in the logic, the real (*testing.T).Fatal() will +// be called to fail the test. +func testExpectTFatal(t *testing.T, testLogic func()) { + t.Helper() + + var recoverIface interface{} - ParallelTest(mt, TestCase{IsUnitTest: true}) + func() { + defer func() { + recoverIface = recover() + }() - if !mt.ParallelCalled { - t.Fatal("Parallel() not called") + testLogic() + }() + + if recoverIface == nil { + t.Fatalf("expected t.Fatal(), got none") } -} -func TestTest_factoryError(t *testing.T) { - resourceFactoryError := fmt.Errorf("resource factory error") + recoverStr, ok := recoverIface.(string) - factory := func() (*schema.Provider, error) { //nolint:unparam // required signature - return nil, resourceFactoryError + if !ok { + t.Fatalf("expected string from recover(), got: %v (%T)", recoverIface, recoverIface) } + + // this string is hardcoded in github.com/mitchellh/go-testing-interface + if !strings.HasPrefix(recoverStr, "testing.T failed, see logs for output") { + t.Fatalf("expected t.Fatal(), got: %s", recoverStr) + } +} + +func TestParallelTest(t *testing.T) { mt := new(mockT) - recovered := false - func() { - defer func() { - r := recover() - // this string is hardcoded in github.com/mitchellh/go-testing-interface - if s, ok := r.(string); !ok || !strings.HasPrefix(s, "testing.T failed, see logs for output") { - panic(r) - } - recovered = true - }() - Test(mt, TestCase{ - ProviderFactories: map[string]func() (*schema.Provider, error){ - "test": factory, + ParallelTest(mt, TestCase{ + IsUnitTest: true, + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": func() (*schema.Provider, error) { //nolint:unparam // required signature + return nil, nil }, - IsUnitTest: true, - }) - }() + }, + Steps: []TestStep{ + { + Config: "# not empty", + }, + }, + }) - if !recovered { - t.Fatalf("test should've failed fatally") + if !mt.ParallelCalled { + t.Fatal("Parallel() not called") } } diff --git a/helper/resource/teststep_providers.go b/helper/resource/teststep_providers.go new file mode 100644 index 00000000000..35d4a9bf57f --- /dev/null +++ b/helper/resource/teststep_providers.go @@ -0,0 +1,48 @@ +package resource + +import ( + "context" + "fmt" + "strings" +) + +// providerConfig takes the list of providers in a TestStep and returns a +// config with only empty provider blocks. This is useful for Import, where no +// config is provided, but the providers must be defined. +func (s TestStep) providerConfig(_ context.Context) string { + var providerBlocks, requiredProviderBlocks strings.Builder + + for name, externalProvider := range s.ExternalProviders { + providerBlocks.WriteString(fmt.Sprintf("provider %q {}\n", name)) + + if externalProvider.Source == "" && externalProvider.VersionConstraint == "" { + continue + } + + requiredProviderBlocks.WriteString(fmt.Sprintf(" %s = {\n", name)) + + if externalProvider.Source != "" { + requiredProviderBlocks.WriteString(fmt.Sprintf(" source = %q\n", externalProvider.Source)) + } + + if externalProvider.VersionConstraint != "" { + requiredProviderBlocks.WriteString(fmt.Sprintf(" version = %q\n", externalProvider.VersionConstraint)) + } + + requiredProviderBlocks.WriteString(" }\n") + } + + if requiredProviderBlocks.Len() > 0 { + return fmt.Sprintf(` +terraform { + required_providers { +%[1]s + } +} + +%[2]s +`, strings.TrimSuffix(requiredProviderBlocks.String(), "\n"), providerBlocks.String()) + } + + return providerBlocks.String() +} diff --git a/helper/resource/teststep_providers_test.go b/helper/resource/teststep_providers_test.go new file mode 100644 index 00000000000..dd4c0d7e7a2 --- /dev/null +++ b/helper/resource/teststep_providers_test.go @@ -0,0 +1,499 @@ +package resource + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestStepProviderConfig(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + testStep TestStep + expected string + }{ + "externalproviders-missing-source-and-versionconstraint": { + testStep: TestStep{ + ExternalProviders: map[string]ExternalProvider{ + "test": {}, + }, + }, + expected: `provider "test" {}`, + }, + "externalproviders-source-and-versionconstraint": { + testStep: TestStep{ + ExternalProviders: map[string]ExternalProvider{ + "test": { + Source: "registry.terraform.io/hashicorp/test", + VersionConstraint: "1.2.3", + }, + }, + }, + expected: ` +terraform { + required_providers { + test = { + source = "registry.terraform.io/hashicorp/test" + version = "1.2.3" + } + } +} + +provider "test" {} +`, + }, + "externalproviders-source": { + testStep: TestStep{ + ExternalProviders: map[string]ExternalProvider{ + "test": { + Source: "registry.terraform.io/hashicorp/test", + }, + }, + }, + expected: ` +terraform { + required_providers { + test = { + source = "registry.terraform.io/hashicorp/test" + } + } +} + +provider "test" {} +`, + }, + "externalproviders-versionconstraint": { + testStep: TestStep{ + ExternalProviders: map[string]ExternalProvider{ + "test": { + VersionConstraint: "1.2.3", + }, + }, + }, + expected: ` +terraform { + required_providers { + test = { + version = "1.2.3" + } + } +} + +provider "test" {} +`, + }, + "protov5providerfactories": { + testStep: TestStep{ + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": nil, + }, + }, + expected: ``, + }, + "protov6providerfactories": { + testStep: TestStep{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": nil, + }, + }, + expected: ``, + }, + "providerfactories": { + testStep: TestStep{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": nil, + }, + }, + expected: ``, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := testCase.testStep.providerConfig(context.Background()) + + if diff := cmp.Diff(strings.TrimSpace(got), strings.TrimSpace(testCase.expected)); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + +func TestTest_TestStep_ExternalProviders(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + Steps: []TestStep{ + { + Config: "# not empty", + ExternalProviders: map[string]ExternalProvider{ + "null": { + Source: "registry.terraform.io/hashicorp/null", + }, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExternalProviders_DifferentProviders(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + Steps: []TestStep{ + { + Config: `resource "null_resource" "test" {}`, + ExternalProviders: map[string]ExternalProvider{ + "null": { + Source: "registry.terraform.io/hashicorp/null", + }, + }, + }, + { + Config: `resource "random_pet" "test" {}`, + ExternalProviders: map[string]ExternalProvider{ + "random": { + Source: "registry.terraform.io/hashicorp/random", + }, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExternalProviders_DifferentVersions(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + Steps: []TestStep{ + { + Config: `resource "null_resource" "test" {}`, + ExternalProviders: map[string]ExternalProvider{ + "null": { + Source: "registry.terraform.io/hashicorp/null", + VersionConstraint: "3.1.0", + }, + }, + }, + { + Config: `resource "null_resource" "test" {}`, + ExternalProviders: map[string]ExternalProvider{ + "null": { + Source: "registry.terraform.io/hashicorp/null", + VersionConstraint: "3.1.1", + }, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExternalProviders_Error(t *testing.T) { + t.Parallel() + + testExpectTFatal(t, func() { + Test(&mockT{}, TestCase{ + Steps: []TestStep{ + { + Config: "# not empty", + ExternalProviders: map[string]ExternalProvider{ + "testnonexistent": { + Source: "registry.terraform.io/hashicorp/testnonexistent", + }, + }, + }, + }, + }) + }) +} + +func TestTest_TestStep_ExternalProviders_To_ProviderFactories(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + Steps: []TestStep{ + { + Config: `resource "null_resource" "test" {}`, + ExternalProviders: map[string]ExternalProvider{ + "null": { + Source: "registry.terraform.io/hashicorp/null", + VersionConstraint: "3.1.1", + }, + }, + }, + { + Config: `resource "null_resource" "test" {}`, + ProviderFactories: map[string]func() (*schema.Provider, error){ + "null": func() (*schema.Provider, error) { //nolint:unparam // required signature + return &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "null_resource": { + CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + d.SetId("test") + return nil + }, + DeleteContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + ReadContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + Schema: map[string]*schema.Schema{ + "triggers": { + Elem: &schema.Schema{Type: schema.TypeString}, + ForceNew: true, + Optional: true, + Type: schema.TypeMap, + }, + }, + }, + }, + }, nil + }, + }, + }, + }, + }) +} + +func TestTest_TestStep_ExternalProviders_To_ProviderFactories_StateUpgraders(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + Steps: []TestStep{ + { + Config: `resource "null_resource" "test" {}`, + ExternalProviders: map[string]ExternalProvider{ + "null": { + Source: "registry.terraform.io/hashicorp/null", + VersionConstraint: "3.1.1", + }, + }, + }, + { + Check: TestCheckResourceAttr("null_resource.test", "id", "test-schema-version-1"), + Config: `resource "null_resource" "test" {}`, + ProviderFactories: map[string]func() (*schema.Provider, error){ + "null": func() (*schema.Provider, error) { //nolint:unparam // required signature + return &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "null_resource": { + CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + d.SetId("test") + return nil + }, + DeleteContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + ReadContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + Schema: map[string]*schema.Schema{ + "triggers": { + Elem: &schema.Schema{Type: schema.TypeString}, + ForceNew: true, + Optional: true, + Type: schema.TypeMap, + }, + }, + SchemaVersion: 1, // null 3.1.3 is version 0 + StateUpgraders: []schema.StateUpgrader{ + { + Type: cty.Object(map[string]cty.Type{ + "id": cty.String, + "triggers": cty.Map(cty.String), + }), + Upgrade: func(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + // null 3.1.3 sets the id attribute to a stringified random integer. + // Double check that our resource wasn't created by this TestStep. + id, ok := rawState["id"].(string) + + if !ok || id == "test" { + return rawState, fmt.Errorf("unexpected rawState: %v", rawState) + } + + rawState["id"] = "test-schema-version-1" + + return rawState, nil + }, + Version: 0, + }, + }, + }, + }, + }, nil + }, + }, + }, + }, + }) +} + +func TestTest_TestStep_ProtoV5ProviderFactories(t *testing.T) { + t.Parallel() + + Test(&mockT{}, TestCase{ + Steps: []TestStep{ + { + Config: "# not empty", + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": func() (tfprotov5.ProviderServer, error) { //nolint:unparam // required signature + return nil, nil + }, + }, + }, + }, + }) +} + +func TestTest_TestStep_ProtoV5ProviderFactories_Error(t *testing.T) { + t.Parallel() + + testExpectTFatal(t, func() { + Test(&mockT{}, TestCase{ + Steps: []TestStep{ + { + Config: "# not empty", + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": func() (tfprotov5.ProviderServer, error) { //nolint:unparam // required signature + return nil, fmt.Errorf("test") + }, + }, + }, + }, + }) + }) +} + +func TestTest_TestStep_ProtoV6ProviderFactories(t *testing.T) { + t.Parallel() + + Test(&mockT{}, TestCase{ + Steps: []TestStep{ + { + Config: "# not empty", + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": func() (tfprotov6.ProviderServer, error) { //nolint:unparam // required signature + return nil, nil + }, + }, + }, + }, + }) +} + +func TestTest_TestStep_ProtoV6ProviderFactories_Error(t *testing.T) { + t.Parallel() + + testExpectTFatal(t, func() { + Test(&mockT{}, TestCase{ + Steps: []TestStep{ + { + Config: "# not empty", + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": func() (tfprotov6.ProviderServer, error) { //nolint:unparam // required signature + return nil, fmt.Errorf("test") + }, + }, + }, + }, + }) + }) +} + +func TestTest_TestStep_ProviderFactories(t *testing.T) { + t.Parallel() + + Test(&mockT{}, TestCase{ + Steps: []TestStep{ + { + Config: "# not empty", + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": func() (*schema.Provider, error) { //nolint:unparam // required signature + return nil, nil + }, + }, + }, + }, + }) +} + +func TestTest_TestStep_ProviderFactories_Error(t *testing.T) { + t.Parallel() + + testExpectTFatal(t, func() { + Test(&mockT{}, TestCase{ + Steps: []TestStep{ + { + Config: "# not empty", + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": func() (*schema.Provider, error) { //nolint:unparam // required signature + return nil, fmt.Errorf("test") + }, + }, + }, + }, + }) + }) +} + +func TestTest_TestStep_ProviderFactories_To_ExternalProviders(t *testing.T) { + t.Parallel() + + Test(t, TestCase{ + Steps: []TestStep{ + { + Config: `resource "null_resource" "test" {}`, + ProviderFactories: map[string]func() (*schema.Provider, error){ + "null": func() (*schema.Provider, error) { //nolint:unparam // required signature + return &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "null_resource": { + CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + d.SetId("test") + return nil + }, + DeleteContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + ReadContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics { + return nil + }, + Schema: map[string]*schema.Schema{ + "triggers": { + Elem: &schema.Schema{Type: schema.TypeString}, + ForceNew: true, + Optional: true, + Type: schema.TypeMap, + }, + }, + }, + }, + }, nil + }, + }, + }, + { + Config: `resource "null_resource" "test" {}`, + ExternalProviders: map[string]ExternalProvider{ + "null": { + Source: "registry.terraform.io/hashicorp/null", + }, + }, + }, + }, + }) +} diff --git a/helper/resource/teststep_validate.go b/helper/resource/teststep_validate.go new file mode 100644 index 00000000000..e9239328c69 --- /dev/null +++ b/helper/resource/teststep_validate.go @@ -0,0 +1,99 @@ +package resource + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" +) + +// testStepValidateRequest contains data for the (TestStep).validate() method. +type testStepValidateRequest struct { + // StepNumber is the index of the TestStep in the TestCase.Steps. + StepNumber int + + // TestCaseHasProviders is enabled if the TestCase has set any of + // ExternalProviders, ProtoV5ProviderFactories, ProtoV6ProviderFactories, + // or ProviderFactories. + TestCaseHasProviders bool +} + +// hasProviders returns true if the TestStep has set any of the +// ExternalProviders, ProtoV5ProviderFactories, ProtoV6ProviderFactories, or +// ProviderFactories fields. +func (s TestStep) hasProviders(_ context.Context) bool { + if len(s.ExternalProviders) > 0 { + return true + } + + if len(s.ProtoV5ProviderFactories) > 0 { + return true + } + + if len(s.ProtoV6ProviderFactories) > 0 { + return true + } + + if len(s.ProviderFactories) > 0 { + return true + } + + return false +} + +// validate ensures the TestStep is valid based on the following criteria: +// +// - Config or ImportState is set. +// - Providers are not specified (ExternalProviders, +// ProtoV5ProviderFactories, ProtoV6ProviderFactories, ProviderFactories) +// if specified at the TestCase level. +// - Providers are specified (ExternalProviders, ProtoV5ProviderFactories, +// ProtoV6ProviderFactories, ProviderFactories) if not specified at the +// TestCase level. +// - No overlapping ExternalProviders and ProviderFactories entries +// - ResourceName is not empty when ImportState is true, ImportStateIdFunc +// is not set, and ImportStateId is not set. +// +func (s TestStep) validate(ctx context.Context, req testStepValidateRequest) error { + ctx = logging.TestStepNumberContext(ctx, req.StepNumber) + + logging.HelperResourceTrace(ctx, "Validating TestStep") + + if s.Config == "" && !s.ImportState { + err := fmt.Errorf("TestStep missing Config or ImportState") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + for name := range s.ExternalProviders { + if _, ok := s.ProviderFactories[name]; ok { + err := fmt.Errorf("TestStep provider %q set in both ExternalProviders and ProviderFactories", name) + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + } + + hasProviders := s.hasProviders(ctx) + + if req.TestCaseHasProviders && hasProviders { + err := fmt.Errorf("Providers must only be specified either at the TestCase or TestStep level") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if !req.TestCaseHasProviders && !hasProviders { + err := fmt.Errorf("Providers must be specified at the TestCase level or in all TestStep") + logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err}) + return err + } + + if s.ImportState { + if s.ImportStateId == "" && s.ImportStateIdFunc == nil && s.ResourceName == "" { + err := fmt.Errorf("TestStep ImportState must be specified with ImportStateId, ImportStateIdFunc, or ResourceName") + 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 new file mode 100644 index 00000000000..8caf5c23323 --- /dev/null +++ b/helper/resource/teststep_validate_test.go @@ -0,0 +1,184 @@ +package resource + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func TestTestStepHasProviders(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + testStep TestStep + expected bool + }{ + "none": { + testStep: TestStep{}, + expected: false, + }, + "externalproviders": { + testStep: TestStep{ + ExternalProviders: map[string]ExternalProvider{ + "test": {}, // does not need to be real + }, + }, + expected: true, + }, + "protov5providerfactories": { + testStep: TestStep{ + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": nil, // does not need to be real + }, + }, + expected: true, + }, + "protov6providerfactories": { + testStep: TestStep{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": nil, // does not need to be real + }, + }, + expected: true, + }, + "providerfactories": { + testStep: TestStep{ + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": nil, // does not need to be real + }, + }, + expected: true, + }, + } + + for name, test := range tests { + name, test := name, test + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := test.testStep.hasProviders(context.Background()) + + if got != test.expected { + t.Errorf("expected %t, got %t", test.expected, got) + } + }) + } +} + +func TestTestStepValidate(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + testStep TestStep + testStepValidateRequest testStepValidateRequest + expectedError error + }{ + "config-and-importstate-missing": { + testStep: TestStep{}, + testStepValidateRequest: testStepValidateRequest{ + TestCaseHasProviders: true, + }, + expectedError: fmt.Errorf("TestStep missing Config or ImportState"), + }, + "externalproviders-overlapping-providerfactories": { + testStep: TestStep{ + Config: "# not empty", + ExternalProviders: map[string]ExternalProvider{ + "test": {}, // does not need to be real + }, + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": nil, // does not need to be real + }, + }, + testStepValidateRequest: testStepValidateRequest{}, + expectedError: fmt.Errorf("TestStep provider \"test\" set in both ExternalProviders and ProviderFactories"), + }, + "externalproviders-testcase-providers": { + testStep: TestStep{ + Config: "# not empty", + ExternalProviders: map[string]ExternalProvider{ + "test": {}, // does not need to be real + }, + }, + testStepValidateRequest: testStepValidateRequest{ + TestCaseHasProviders: true, + }, + expectedError: fmt.Errorf("Providers must only be specified either at the TestCase or TestStep level"), + }, + "importstate-missing-resourcename": { + testStep: TestStep{ + ImportState: true, + }, + testStepValidateRequest: testStepValidateRequest{ + TestCaseHasProviders: true, + }, + expectedError: fmt.Errorf("TestStep ImportState must be specified with ImportStateId, ImportStateIdFunc, or ResourceName"), + }, + "protov5providerfactories-testcase-providers": { + testStep: TestStep{ + Config: "# not empty", + ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){ + "test": nil, // does not need to be real + }, + }, + testStepValidateRequest: testStepValidateRequest{ + TestCaseHasProviders: true, + }, + expectedError: fmt.Errorf("Providers must only be specified either at the TestCase or TestStep level"), + }, + "protov6providerfactories-testcase-providers": { + testStep: TestStep{ + Config: "# not empty", + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "test": nil, // does not need to be real + }, + }, + testStepValidateRequest: testStepValidateRequest{ + TestCaseHasProviders: true, + }, + expectedError: fmt.Errorf("Providers must only be specified either at the TestCase or TestStep level"), + }, + "providerfactories-testcase-providers": { + testStep: TestStep{ + Config: "# not empty", + ProviderFactories: map[string]func() (*schema.Provider, error){ + "test": nil, // does not need to be real + }, + }, + testStepValidateRequest: testStepValidateRequest{ + TestCaseHasProviders: true, + }, + expectedError: fmt.Errorf("Providers must only be specified either at the TestCase or TestStep level"), + }, + } + + for name, test := range tests { + name, test := name, test + + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := test.testStep.validate(context.Background(), test.testStepValidateRequest) + + if err != nil { + if test.expectedError == nil { + t.Fatalf("unexpected error: %s", err) + } + + if !strings.Contains(err.Error(), test.expectedError.Error()) { + t.Fatalf("expected error %q, got: %s", test.expectedError, err) + } + } + + if err == nil && test.expectedError != nil { + t.Errorf("expected error: %s", test.expectedError) + } + }) + } +} diff --git a/internal/plugintest/working_dir.go b/internal/plugintest/working_dir.go index 5309fcfb0a0..1c15e6f5c19 100644 --- a/internal/plugintest/working_dir.go +++ b/internal/plugintest/working_dir.go @@ -155,7 +155,9 @@ func (wd *WorkingDir) Init(ctx context.Context) error { logging.HelperResourceTrace(ctx, "Calling Terraform CLI init command") - err := wd.tf.Init(context.Background(), tfexec.Reattach(wd.reattachInfo)) + // -upgrade=true is required for per-TestStep provider version changes + // e.g. TestTest_TestStep_ExternalProviders_DifferentVersions + err := wd.tf.Init(context.Background(), tfexec.Reattach(wd.reattachInfo), tfexec.Upgrade(true)) logging.HelperResourceTrace(ctx, "Called Terraform CLI init command")