Adding HasConfigurationFiles() func to Config interface to be able to…

… validate that when ConfigDirectory is defined that ExternalProviders cannot be specified for either TestCase or TestStep (#150)

helper/resource/testcase_validate.go

@@ -11,6 +11,11 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/internal/teststep"
 )
 
+// hasProviders returns true if the TestCase has ExternalProviders set.
+func (c TestCase) hasExternalProviders(_ context.Context) bool {
+	return len(c.ExternalProviders) > 0
+}
+
 // hasProviders returns true if the TestCase has set any of the
 // ExternalProviders, ProtoV5ProviderFactories, ProtoV6ProviderFactories,
 // ProviderFactories, or Providers fields.
@@ -66,6 +71,7 @@ func (c TestCase) validate(ctx context.Context) error {
 		}
 	}
 
+	testCaseHasExternalProviders := c.hasExternalProviders(ctx)
 	testCaseHasProviders := c.hasProviders(ctx)
 
 	for stepIndex, step := range c.Steps {
@@ -82,9 +88,10 @@ func (c TestCase) validate(ctx context.Context) error {
 
 		stepNumber := stepIndex + 1 // Use 1-based index for humans
 		stepValidateReq := testStepValidateRequest{
-			StepConfiguration:    stepConfiguration,
-			StepNumber:           stepNumber,
-			TestCaseHasProviders: testCaseHasProviders,
+			StepConfiguration:            stepConfiguration,
+			StepNumber:                   stepNumber,
+			TestCaseHasExternalProviders: testCaseHasExternalProviders,
+			TestCaseHasProviders:         testCaseHasProviders,
 		}
 
 		err = step.validate(ctx, stepValidateReq)

helper/resource/testcase_validate_test.go

@@ -14,6 +14,42 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
+func TestTestCaseHasExternalProviders(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,
+		},
+	}
+
+	for name, test := range tests {
+		name, test := name, test
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			got := test.testCase.hasExternalProviders(context.Background())
+
+			if got != test.expected {
+				t.Errorf("expected %t, got %t", test.expected, got)
+			}
+		})
+	}
+}
+
 func TestTestCaseHasProviders(t *testing.T) {
 	t.Parallel()
 

helper/resource/teststep_validate.go

@@ -20,12 +20,22 @@ type testStepValidateRequest struct {
 	// StepNumber is the index of the TestStep in the TestCase.Steps.
 	StepNumber int
 
+	// TestCaseHasExternalProviders is enabled if the TestCase has
+	// ExternalProviders.
+	TestCaseHasExternalProviders bool
+
 	// TestCaseHasProviders is enabled if the TestCase has set any of
 	// ExternalProviders, ProtoV5ProviderFactories, ProtoV6ProviderFactories,
 	// or ProviderFactories.
 	TestCaseHasProviders bool
 }
 
+// hasExternalProviders returns true if the TestStep has
+// ExternalProviders set.
+func (s TestStep) hasExternalProviders() bool {
+	return len(s.ExternalProviders) > 0
+}
+
 // hasProviders returns true if the TestStep has set any of the
 // ExternalProviders, ProtoV5ProviderFactories, ProtoV6ProviderFactories, or
 // ProviderFactories fields. It will also return true if ConfigDirectory or
@@ -132,6 +142,18 @@ func (s TestStep) validate(ctx context.Context, req testStepValidateRequest) err
 		}
 	}
 
+	if req.TestCaseHasExternalProviders && req.StepConfiguration.HasConfigurationFiles() {
+		err := fmt.Errorf("Providers must only be specified within the terraform configuration files when using TestStep.ConfigDirectory")
+		logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err})
+		return err
+	}
+
+	if s.hasExternalProviders() && req.StepConfiguration.HasConfigurationFiles() {
+		err := fmt.Errorf("Providers must only be specified within the terraform configuration files when using TestStep.ConfigDirectory")
+		logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err})
+		return err
+	}
+
 	hasProviders, err := s.hasProviders(ctx)
 
 	if err != nil {
@@ -145,8 +167,15 @@ func (s TestStep) validate(ctx context.Context, req testStepValidateRequest) err
 		return err
 	}
 
-	if !req.TestCaseHasProviders && !hasProviders {
-		err := fmt.Errorf("Providers must be specified at the TestCase level or in all TestStep")
+	cfgHasProviderBlock, err := req.StepConfiguration.HasProviderBlock(ctx)
+
+	if err != nil {
+		logging.HelperResourceError(ctx, "TestStep error checking for if configuration has provider block", map[string]interface{}{logging.KeyError: err})
+		return err
+	}
+
+	if !req.TestCaseHasProviders && !hasProviders && !cfgHasProviderBlock {
+		err := fmt.Errorf("Providers must be specified at the TestCase level, or in all TestStep, or in TestStep.ConfigDirectory")
 		logging.HelperResourceError(ctx, "TestStep validation error", map[string]interface{}{logging.KeyError: err})
 		return err
 	}

helper/resource/teststep_validate_test.go

@@ -19,6 +19,42 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
+func TestTestStepHasExternalProviders(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,
+		},
+	}
+
+	for name, test := range tests {
+		name, test := name, test
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			got := test.testStep.hasExternalProviders()
+
+			if got != test.expected {
+				t.Errorf("expected %t, got %t", test.expected, got)
+			}
+		})
+	}
+}
+
 func TestTestStepHasProviders(t *testing.T) {
 	t.Parallel()
 
@@ -163,25 +199,31 @@ func TestTestStepValidate(t *testing.T) {
 			testStepValidateRequest: testStepValidateRequest{},
 			expectedError:           fmt.Errorf("TestStep provider \"test\" set in both ExternalProviders and ProviderFactories"),
 		},
-		"externalproviders-testcase-providers": {
+		"externalproviders-testcase-config-directory": {
+			testStep:                TestStep{},
+			testStepConfigDirectory: "# not empty",
+			testStepValidateRequest: testStepValidateRequest{
+				TestCaseHasExternalProviders: true,
+			},
+			expectedError: fmt.Errorf("Providers must only be specified within the terraform configuration files when using TestStep.ConfigDirectory"),
+		},
+		"externalproviders-teststep-config-directory": {
 			testStep: TestStep{
 				ExternalProviders: map[string]ExternalProvider{
 					"test": {}, // does not need to be real
 				},
 			},
-			testStepConfig: "# not empty",
-			testStepValidateRequest: testStepValidateRequest{
-				TestCaseHasProviders: true,
-			},
-			expectedError: fmt.Errorf("Providers must only be specified either at the TestCase or TestStep level"),
+			testStepConfigDirectory: "# not empty",
+			testStepValidateRequest: testStepValidateRequest{},
+			expectedError:           fmt.Errorf("Providers must only be specified within the terraform configuration files when using TestStep.ConfigDirectory"),
 		},
-		"externalproviders-testcase-providers-config-directory": {
+		"externalproviders-testcase-providers": {
 			testStep: TestStep{
 				ExternalProviders: map[string]ExternalProvider{
 					"test": {}, // does not need to be real
 				},
 			},
-			testStepConfigDirectory: "# not empty",
+			testStepConfig: "# not empty",
 			testStepValidateRequest: testStepValidateRequest{
 				TestCaseHasProviders: true,
 			},
@@ -296,7 +338,7 @@ func TestTestStepValidate(t *testing.T) {
 				},
 				PlanOnly: true,
 			},
-			testStepConfigDirectory: "# not empty",
+			testStepConfigDirectory: "../fixtures/random_id",
 			testStepValidateRequest: testStepValidateRequest{TestCaseHasProviders: true},
 			expectedError:           errors.New("TestStep ConfigPlanChecks.PreApply cannot be run with PlanOnly"),
 		},
@@ -336,7 +378,7 @@ func TestTestStepValidate(t *testing.T) {
 					PostRefresh: []plancheck.PlanCheck{&planCheckSpy{}},
 				},
 			},
-			testStepConfigDirectory: "# not empty",
+			testStepConfigDirectory: "../fixtures/random_id",
 			testStepValidateRequest: testStepValidateRequest{TestCaseHasProviders: true},
 			expectedError:           errors.New("TestStep RefreshPlanChecks.PostRefresh must only be specified with RefreshState"),
 		},

internal/teststep/config.go

@@ -28,6 +28,7 @@ var (
 
 type Config interface {
 	HasConfiguration() bool
+	HasConfigurationFiles() bool
 	HasProviderBlock(context.Context) (bool, error)
 	Write(context.Context, string) error
 }
@@ -91,6 +92,10 @@ func (c configuration) HasConfiguration() bool {
 	return false
 }
 
+func (c configuration) HasConfigurationFiles() bool {
+	return c.directory != ""
+}
+
 // HasProviderBlock returns true if the Config has declared a provider
 // configuration block, e.g. provider "examplecloud" {...}
 func (c configuration) HasProviderBlock(ctx context.Context) (bool, error) {