From 7ae7a203d8e8e47bfd988bcbab1b370235d7710c Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Mon, 19 Feb 2024 15:20:11 +0100
Subject: [PATCH 01/14] test: allow using global variables in suite-level
 variable definitions

Closes #34534
---
 internal/backend/local/test.go                | 90 +++++++++++++++++--
 internal/command/test_test.go                 | 64 +++++++------
 .../test/global_var_ref_in_suite_var/main.tf  | 10 +++
 .../run_block_output.tftest.hcl               | 13 +++
 .../terraform.tfvars                          |  1 +
 internal/moduletest/hcl/context.go            |  2 +-
 internal/moduletest/hcl/provider_test.go      |  2 +-
 7 files changed, 148 insertions(+), 34 deletions(-)
 create mode 100644 internal/command/testdata/test/global_var_ref_in_suite_var/main.tf
 create mode 100644 internal/command/testdata/test/global_var_ref_in_suite_var/run_block_output.tftest.hcl
 create mode 100644 internal/command/testdata/test/global_var_ref_in_suite_var/terraform.tfvars

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index 235ea6f610..1c23491996 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -244,7 +244,10 @@ type TestFileRunner struct {
 	// variables within run blocks.
 	PriorOutputs map[addrs.Run]cty.Value
 
+	// globalVariables are globally defined variables, e.g. through tfvars or CLI flags
 	globalVariables map[string]backend.UnparsedVariableValue
+	// fileVariables are defined in the variables section of a test file
+	fileVariables map[string]hcl.Expression
 }
 
 // TestFileState is a helper struct that just maps a run block to the state that
@@ -379,6 +382,8 @@ func (runner *TestFileRunner) run(run *moduletest.Run, file *moduletest.File, st
 	}
 	runner.gatherProviders(key, config)
 
+	// TODO: Merge local variables and suite-level variables
+	// It might be necessary to change the format or add an extra parameter, not sure yet
 	resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, runner.globalVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
 	defer resetConfig()
 
@@ -992,7 +997,21 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 		}
 	}
 
-	// Second, we'll check to see which variables the run block variables
+	// Second, we'll check to see which variables the suites global variables
+	// themselves reference.
+	for _, expr := range runner.fileVariables {
+		for _, variable := range expr.Variables() {
+			reference, referenceDiags := addrs.ParseRefFromTestingScope(variable)
+			diags = diags.Append(referenceDiags)
+			if reference != nil {
+				if addr, ok := reference.Subject.(addrs.InputVariable); ok {
+					relevantVariables[addr.Name] = true
+				}
+			}
+		}
+	}
+
+	// Third, we'll check to see which variables the run block variables
 	// themselves reference. We might be processing variables just for the file
 	// so the run block itself could be nil.
 	for _, expr := range run.Config.Variables {
@@ -1022,6 +1041,8 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 	//   - ConfigVariables variables, defined directly within the config.
 	values := make(terraform.InputValues)
 
+	fmt.Printf("\nrunner.globalVariables: \n\t%#v \n", runner.globalVariables)
+
 	// First, let's look at the global variables.
 	for name, variable := range runner.globalVariables {
 		if !relevantVariables[name] {
@@ -1033,6 +1054,7 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 		parsingMode := configs.VariableParseHCL
 
 		cfg, exists := config.Module.Variables[name]
+
 		if exists {
 			// Unless we have some configuration that can actually tell us
 			// what parsing mode to use.
@@ -1057,29 +1079,82 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 		values[name] = value
 	}
 
-	// Second, we'll check the run level variables.
+	// Second, we'll check the file level variables
+	var exprs []hcl.Expression
+	for _, expr := range runner.fileVariables {
+		exprs = append(exprs, expr)
+	}
+
+	// Preformat the variables we've processed already - these will be made
+	// available to the eval context.
+	variables := make(map[string]cty.Value)
+	for name, value := range values {
+		variables[name] = value.Value
+	}
+
+	fmt.Printf("\nvariables: \n\t%#v \n", variables)
+
+	ctx, ctxDiags := hcltest.EvalContext(hcltest.TargetRunBlock, exprs, variables, runner.PriorOutputs)
+	diags = diags.Append(ctxDiags)
+
+	var failedContext bool
+	if ctxDiags.HasErrors() {
+		// If we couldn't build the context, we won't actually process these
+		// variables. Instead, we'll fill them with an empty value but still
+		// make a note that the user did provide them.
+		failedContext = true
+	}
+
+	for name, expr := range runner.fileVariables {
+		if !relevantVariables[name] {
+			// We'll add a warning for this. Since we're right in the run block
+			// users shouldn't be defining variables that are not relevant.
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagWarning,
+				Summary:  "Value for undeclared variable",
+				Detail:   fmt.Sprintf("The module under test does not declare a variable named %q, but it is declared in run block %q.", name, run.Name),
+				Subject:  expr.Range().Ptr(),
+			})
+			continue
+		}
+
+		value := cty.NilVal
+		if !failedContext {
+			var valueDiags hcl.Diagnostics
+			value, valueDiags = expr.Value(ctx)
+			diags = diags.Append(valueDiags)
+		}
+
+		values[name] = &terraform.InputValue{
+			Value:       value,
+			SourceType:  terraform.ValueFromConfig,
+			SourceRange: tfdiags.SourceRangeFromHCL(expr.Range()),
+		}
+	}
+
+	// Third, we'll check the run level variables.
 
 	// This is a bit more complicated, as the run level variables can reference
 	// previously defined variables.
 
 	// Preload the available expressions, we're going to validate them when we
 	// build the context.
-	var exprs []hcl.Expression
+	exprs = []hcl.Expression{}
 	for _, expr := range run.Config.Variables {
 		exprs = append(exprs, expr)
 	}
 
 	// Preformat the variables we've processed already - these will be made
 	// available to the eval context.
-	variables := make(map[string]cty.Value)
+	variables = make(map[string]cty.Value)
 	for name, value := range values {
 		variables[name] = value.Value
 	}
 
-	ctx, ctxDiags := hcltest.EvalContext(hcltest.TargetRunBlock, exprs, variables, runner.PriorOutputs)
+	ctx, ctxDiags = hcltest.EvalContext(hcltest.TargetRunBlock, exprs, variables, runner.PriorOutputs)
 	diags = diags.Append(ctxDiags)
 
-	var failedContext bool
+	failedContext = false
 	if ctxDiags.HasErrors() {
 		// If we couldn't build the context, we won't actually process these
 		// variables. Instead, we'll fill them with an empty value but still
@@ -1260,8 +1335,9 @@ func (runner *TestFileRunner) initVariables(file *moduletest.File) {
 			runner.globalVariables[name] = value
 		}
 	}
+	runner.fileVariables = make(map[string]hcl.Expression)
 	for name, expr := range file.Config.Variables {
-		runner.globalVariables[name] = unparsedTestVariableValue{expr}
+		runner.fileVariables[name] = expr
 	}
 }
 
diff --git a/internal/command/test_test.go b/internal/command/test_test.go
index 7628c6a677..813b017db6 100644
--- a/internal/command/test_test.go
+++ b/internal/command/test_test.go
@@ -26,7 +26,7 @@ func TestTest_Runs(t *testing.T) {
 		override              string
 		args                  []string
 		expectedOut           string
-		expectedErr           string
+		expectedErr           []string
 		expectedResourceCount int
 		code                  int
 		initCode              int
@@ -59,13 +59,13 @@ func TestTest_Runs(t *testing.T) {
 		"simple_pass_bad_test_directory": {
 			override:    "simple_pass",
 			args:        []string{"-test-directory", "../tests"},
-			expectedErr: "Invalid testing directory",
+			expectedErr: []string{"Invalid testing directory"},
 			code:        1,
 		},
 		"simple_pass_bad_test_directory_abs": {
 			override:    "simple_pass",
 			args:        []string{"-test-directory", "/home/username/config/tests"},
-			expectedErr: "Invalid testing directory",
+			expectedErr: []string{"Invalid testing directory"},
 			code:        1,
 		},
 		"pass_with_locals": {
@@ -118,32 +118,32 @@ func TestTest_Runs(t *testing.T) {
 			override:    "variables",
 			args:        []string{"-var=input=foo"},
 			expectedOut: "1 passed, 1 failed",
-			expectedErr: `invalid value`,
+			expectedErr: []string{`invalid value`},
 			code:        1,
 		},
 		"simple_fail": {
 			expectedOut: "0 passed, 1 failed.",
-			expectedErr: "invalid value",
+			expectedErr: []string{"invalid value"},
 			code:        1,
 		},
 		"custom_condition_checks": {
 			expectedOut: "0 passed, 1 failed.",
-			expectedErr: "this really should fail",
+			expectedErr: []string{"this really should fail"},
 			code:        1,
 		},
 		"custom_condition_inputs": {
 			expectedOut: "0 passed, 1 failed.",
-			expectedErr: "this should definitely fail",
+			expectedErr: []string{"this should definitely fail"},
 			code:        1,
 		},
 		"custom_condition_outputs": {
 			expectedOut: "0 passed, 1 failed.",
-			expectedErr: "this should fail",
+			expectedErr: []string{"this should fail"},
 			code:        1,
 		},
 		"custom_condition_resources": {
 			expectedOut: "0 passed, 1 failed.",
-			expectedErr: "this really should fail",
+			expectedErr: []string{"this really should fail"},
 			code:        1,
 		},
 		"no_providers_in_main": {
@@ -190,7 +190,7 @@ func TestTest_Runs(t *testing.T) {
 		},
 		"destroy_fail": {
 			expectedOut:           "1 passed, 0 failed.",
-			expectedErr:           `Terraform left the following resources in state`,
+			expectedErr:           []string{`Terraform left the following resources in state`},
 			code:                  1,
 			expectedResourceCount: 1,
 		},
@@ -217,7 +217,7 @@ func TestTest_Runs(t *testing.T) {
 			code:        0,
 		},
 		"mocking-invalid": {
-			expectedErr: "Invalid outputs attribute",
+			expectedErr: []string{"Invalid outputs attribute"},
 			initCode:    1,
 		},
 		"dangling_data_block": {
@@ -234,9 +234,13 @@ func TestTest_Runs(t *testing.T) {
 		},
 		"global_var_refs": {
 			expectedOut: "2 failed, 1 skipped.",
-			expectedErr: "Variables may not be used here.",
+			expectedErr: []string{"The input variable \"env_var_input\" is not available to the current context", "The input variable \"setup\" is not available to the current context"},
 			code:        1,
 		},
+		"global_var_ref_in_suite_var": {
+			expectedOut: "1 passed, 0 failed.",
+			code:        0,
+		},
 	}
 	for name, tc := range tcs {
 		t.Run(name, func(t *testing.T) {
@@ -289,9 +293,13 @@ func TestTest_Runs(t *testing.T) {
 					t.Errorf("output didn't contain expected string:\n\n%s", stdout)
 				}
 
-				if !strings.Contains(stderr, tc.expectedErr) {
-					t.Errorf("error didn't contain expected string:\n\n%s", stderr)
-				} else if tc.expectedErr == "" && stderr != "" {
+				if len(tc.expectedErr) > 0 {
+					for _, expectedErr := range tc.expectedErr {
+						if !strings.Contains(stderr, expectedErr) {
+							t.Errorf("error didn't contain expected string:\n\n%s", stderr)
+						}
+					}
+				} else if stderr != "" {
 					t.Errorf("unexpected stderr output\n%s", stderr)
 				}
 
@@ -316,9 +324,13 @@ func TestTest_Runs(t *testing.T) {
 				t.Errorf("output didn't contain expected string:\n\n%s", output.Stdout())
 			}
 
-			if !strings.Contains(output.Stderr(), tc.expectedErr) {
-				t.Errorf("error didn't contain expected string:\n\n%s", output.Stderr())
-			} else if tc.expectedErr == "" && output.Stderr() != "" {
+			if len(tc.expectedErr) > 0 {
+				for _, expectedErr := range tc.expectedErr {
+					if !strings.Contains(output.Stderr(), expectedErr) {
+						t.Errorf("error didn't contain expected string:\n\n%s", output.Stderr())
+					}
+				}
+			} else if output.Stderr() != "" {
 				t.Errorf("unexpected stderr output\n%s", output.Stderr())
 			}
 
@@ -1161,7 +1173,7 @@ requested in the configuration may have been ignored and the output values
 may not be fully updated. Run the following command to verify that no other
 changes are pending:
     terraform plan
-	
+
 Note that the -target option is not suitable for routine use, and is provided
 only for exceptional situations such as recovering from errors or mistakes,
 or when Terraform specifically suggests to use it as part of an error
@@ -1240,9 +1252,10 @@ Error: Reference to unavailable variable
   on main.tftest.hcl line 15, in run "test":
   15:     input_one = var.notreal
 
-The input variable "notreal" is not available to the current run block. You
-can only reference variables defined at the file or global levels when
-populating the variables block within a run block.
+The input variable "notreal" is not available to the current context. Within
+the variables block of a run block you can only reference variables defined
+at the file or global levels; within the variables block of a suite you can
+only reference variables defined at the global levels.
 
 Error: Reference to unavailable run block
 
@@ -1267,9 +1280,10 @@ Error: Reference to unavailable variable
   on providers.tftest.hcl line 3, in provider "test":
    3:   resource_prefix = var.default
 
-The input variable "default" is not available to the current run block. You
-can only reference variables defined at the file or global levels when
-populating the variables block within a run block.
+The input variable "default" is not available to the current context. Within
+the variables block of a run block you can only reference variables defined
+at the file or global levels; within the variables block of a suite you can
+only reference variables defined at the global levels.
 `
 	actualErr := output.Stderr()
 	if diff := cmp.Diff(actualErr, expectedErr); len(diff) > 0 {
diff --git a/internal/command/testdata/test/global_var_ref_in_suite_var/main.tf b/internal/command/testdata/test/global_var_ref_in_suite_var/main.tf
new file mode 100644
index 0000000000..a776ac2eef
--- /dev/null
+++ b/internal/command/testdata/test/global_var_ref_in_suite_var/main.tf
@@ -0,0 +1,10 @@
+variable "input" {
+  default = null
+  type    = object({
+              organization_name = string
+            })
+}
+
+output "value" {
+  value = var.input
+}
diff --git a/internal/command/testdata/test/global_var_ref_in_suite_var/run_block_output.tftest.hcl b/internal/command/testdata/test/global_var_ref_in_suite_var/run_block_output.tftest.hcl
new file mode 100644
index 0000000000..fd43cbda79
--- /dev/null
+++ b/internal/command/testdata/test/global_var_ref_in_suite_var/run_block_output.tftest.hcl
@@ -0,0 +1,13 @@
+
+variables {
+  input = {
+      organization_name = var.org_name
+  }
+}
+
+run "execute" {
+  assert {
+    condition     = output.value.organization_name == "my-org"
+    error_message = "bad output value"
+  }
+}
diff --git a/internal/command/testdata/test/global_var_ref_in_suite_var/terraform.tfvars b/internal/command/testdata/test/global_var_ref_in_suite_var/terraform.tfvars
new file mode 100644
index 0000000000..724a50aa15
--- /dev/null
+++ b/internal/command/testdata/test/global_var_ref_in_suite_var/terraform.tfvars
@@ -0,0 +1 @@
+org_name = "my-org"
diff --git a/internal/moduletest/hcl/context.go b/internal/moduletest/hcl/context.go
index 471dd08f80..a446325c97 100644
--- a/internal/moduletest/hcl/context.go
+++ b/internal/moduletest/hcl/context.go
@@ -132,7 +132,7 @@ func EvalContext(target EvalContextTarget, expressions []hcl.Expression, availab
 				if _, exists := availableVariables[addr.Name]; !exists {
 					// This variable reference doesn't exist.
 
-					detail := fmt.Sprintf("The input variable %q is not available to the current run block. You can only reference variables defined at the file or global levels when populating the variables block within a run block.", addr.Name)
+					detail := fmt.Sprintf("The input variable %q is not available to the current context. Within the variables block of a run block you can only reference variables defined at the file or global levels; within the variables block of a suite you can only reference variables defined at the global levels.", addr.Name)
 					if availableRunOutputs == nil {
 						detail = fmt.Sprintf("The input variable %q is not available to the current provider configuration. You can only reference variables defined at the file or global levels within provider configurations.", addr.Name)
 					}
diff --git a/internal/moduletest/hcl/provider_test.go b/internal/moduletest/hcl/provider_test.go
index 47f7141788..883ec1316f 100644
--- a/internal/moduletest/hcl/provider_test.go
+++ b/internal/moduletest/hcl/provider_test.go
@@ -71,7 +71,7 @@ func TestProviderConfig(t *testing.T) {
 				"input": cty.StringVal("string"),
 			},
 			expectedErrors: []string{
-				"The input variable \"missing\" is not available to the current run block. You can only reference variables defined at the file or global levels when populating the variables block within a run block.",
+				"The input variable \"missing\" is not available to the current context. Within the variables block of a run block you can only reference variables defined at the file or global levels; within the variables block of a suite you can only reference variables defined at the global levels.",
 			},
 			validate: func(t *testing.T, content *hcl.BodyContent) {
 				if len(content.Attributes) > 0 {

From 697e0d1d9e35dc0db1782f3b91cca43b92f93252 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Wed, 21 Feb 2024 12:04:22 +0100
Subject: [PATCH 02/14] refactor variable extraction

---
 internal/backend/local/test.go | 186 +++++++++++++--------------------
 1 file changed, 74 insertions(+), 112 deletions(-)

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index 1c23491996..7afe3b3dbd 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -997,34 +997,30 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 		}
 	}
 
-	// Second, we'll check to see which variables the suites global variables
-	// themselves reference.
-	for _, expr := range runner.fileVariables {
-		for _, variable := range expr.Variables() {
-			reference, referenceDiags := addrs.ParseRefFromTestingScope(variable)
-			diags = diags.Append(referenceDiags)
-			if reference != nil {
-				if addr, ok := reference.Subject.(addrs.InputVariable); ok {
-					relevantVariables[addr.Name] = true
+	getRelevantVariables := func(src map[string]hcl.Expression) tfdiags.Diagnostics {
+		var getVarsDiags tfdiags.Diagnostics
+		for _, expr := range src {
+			for _, variable := range expr.Variables() {
+				reference, referenceDiags := addrs.ParseRefFromTestingScope(variable)
+				getVarsDiags = getVarsDiags.Append(referenceDiags)
+				if reference != nil {
+					if addr, ok := reference.Subject.(addrs.InputVariable); ok {
+						relevantVariables[addr.Name] = true
+					}
 				}
 			}
 		}
+		return getVarsDiags
 	}
 
+	// Second, we'll check to see which variables the suites global variables
+	// themselves reference.
+	diags = diags.Append(getRelevantVariables(runner.fileVariables))
+
 	// Third, we'll check to see which variables the run block variables
 	// themselves reference. We might be processing variables just for the file
 	// so the run block itself could be nil.
-	for _, expr := range run.Config.Variables {
-		for _, variable := range expr.Variables() {
-			reference, referenceDiags := addrs.ParseRefFromTestingScope(variable)
-			diags = diags.Append(referenceDiags)
-			if reference != nil {
-				if addr, ok := reference.Subject.(addrs.InputVariable); ok {
-					relevantVariables[addr.Name] = true
-				}
-			}
-		}
-	}
+	diags = diags.Append(getRelevantVariables(run.Config.Variables))
 
 	// Finally, we'll check to see which variables are actually defined within
 	// the configuration.
@@ -1041,8 +1037,6 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 	//   - ConfigVariables variables, defined directly within the config.
 	values := make(terraform.InputValues)
 
-	fmt.Printf("\nrunner.globalVariables: \n\t%#v \n", runner.globalVariables)
-
 	// First, let's look at the global variables.
 	for name, variable := range runner.globalVariables {
 		if !relevantVariables[name] {
@@ -1080,81 +1074,86 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 	}
 
 	// Second, we'll check the file level variables
-	var exprs []hcl.Expression
-	for _, expr := range runner.fileVariables {
-		exprs = append(exprs, expr)
+	// This is a bit more complicated, as the file and run level variables can reference
+	// previously defined variables.
+	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, relevantVariables, run.Name, runner.fileVariables)
+	diags = diags.Append(fileDiags)
+	for name, value := range fileValues {
+		values[name] = value
 	}
 
-	// Preformat the variables we've processed already - these will be made
-	// available to the eval context.
-	variables := make(map[string]cty.Value)
-	for name, value := range values {
-		variables[name] = value.Value
+	// Third, we'll check the run level variables.
+	runValues, runDiags := runner.getVariablesFromConfiguration(values, relevantVariables, run.Name, run.Config.Variables)
+	diags = diags.Append(runDiags)
+	for name, value := range runValues {
+		values[name] = value
 	}
 
-	fmt.Printf("\nvariables: \n\t%#v \n", variables)
+	// Finally, we check the configuration again. This is where we'll discover
+	// if there's any missing variables and fill in any optional variables that
+	// don't have a value already.
 
-	ctx, ctxDiags := hcltest.EvalContext(hcltest.TargetRunBlock, exprs, variables, runner.PriorOutputs)
-	diags = diags.Append(ctxDiags)
+	for name, variable := range config.Module.Variables {
+		if _, exists := values[name]; exists {
+			// Then we've provided a variable for this. It's all good.
+			continue
+		}
 
-	var failedContext bool
-	if ctxDiags.HasErrors() {
-		// If we couldn't build the context, we won't actually process these
-		// variables. Instead, we'll fill them with an empty value but still
-		// make a note that the user did provide them.
-		failedContext = true
-	}
+		// Otherwise, we're going to give these variables a value. They'll be
+		// processed by the Terraform graph and provided a default value later
+		// if they have one.
 
-	for name, expr := range runner.fileVariables {
-		if !relevantVariables[name] {
-			// We'll add a warning for this. Since we're right in the run block
-			// users shouldn't be defining variables that are not relevant.
+		if variable.Required() {
 			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagWarning,
-				Summary:  "Value for undeclared variable",
-				Detail:   fmt.Sprintf("The module under test does not declare a variable named %q, but it is declared in run block %q.", name, run.Name),
-				Subject:  expr.Range().Ptr(),
+				Severity: hcl.DiagError,
+				Summary:  "No value for required variable",
+				Detail: fmt.Sprintf("The module under test for run block %q has a required variable %q with no set value. Use a -var or -var-file command line argument or add this variable into a \"variables\" block within the test file or run block.",
+					run.Name, variable.Name),
+				Subject: variable.DeclRange.Ptr(),
 			})
-			continue
-		}
 
-		value := cty.NilVal
-		if !failedContext {
-			var valueDiags hcl.Diagnostics
-			value, valueDiags = expr.Value(ctx)
-			diags = diags.Append(valueDiags)
+			values[name] = &terraform.InputValue{
+				Value:       cty.DynamicVal,
+				SourceType:  terraform.ValueFromConfig,
+				SourceRange: tfdiags.SourceRangeFromHCL(variable.DeclRange),
+			}
+		} else {
+			values[name] = &terraform.InputValue{
+				Value:       cty.NilVal,
+				SourceType:  terraform.ValueFromConfig,
+				SourceRange: tfdiags.SourceRangeFromHCL(variable.DeclRange),
+			}
 		}
 
-		values[name] = &terraform.InputValue{
-			Value:       value,
-			SourceType:  terraform.ValueFromConfig,
-			SourceRange: tfdiags.SourceRangeFromHCL(expr.Range()),
-		}
 	}
 
-	// Third, we'll check the run level variables.
+	return values, diags
+}
 
-	// This is a bit more complicated, as the run level variables can reference
-	// previously defined variables.
+// getVariablesFromConfiguration will process the variables from the configuration
+// and return a map of the variables and their values.
+func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terraform.InputValues, relevantVariables map[string]bool, runName string, variableConfig map[string]hcl.Expression) (terraform.InputValues, tfdiags.Diagnostics) {
+	var exprs []hcl.Expression
+	var diags tfdiags.Diagnostics
+	variableValues := make(terraform.InputValues)
 
 	// Preload the available expressions, we're going to validate them when we
 	// build the context.
-	exprs = []hcl.Expression{}
-	for _, expr := range run.Config.Variables {
+	for _, expr := range variableConfig {
 		exprs = append(exprs, expr)
 	}
 
 	// Preformat the variables we've processed already - these will be made
 	// available to the eval context.
-	variables = make(map[string]cty.Value)
-	for name, value := range values {
+	variables := make(map[string]cty.Value)
+	for name, value := range knownVariables {
 		variables[name] = value.Value
 	}
 
-	ctx, ctxDiags = hcltest.EvalContext(hcltest.TargetRunBlock, exprs, variables, runner.PriorOutputs)
+	ctx, ctxDiags := hcltest.EvalContext(hcltest.TargetRunBlock, exprs, variables, runner.PriorOutputs)
 	diags = diags.Append(ctxDiags)
 
-	failedContext = false
+	var failedContext bool
 	if ctxDiags.HasErrors() {
 		// If we couldn't build the context, we won't actually process these
 		// variables. Instead, we'll fill them with an empty value but still
@@ -1162,14 +1161,14 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 		failedContext = true
 	}
 
-	for name, expr := range run.Config.Variables {
-		if !relevantVariables[name] {
+	for name, expr := range variableConfig {
+		if relevantVariables != nil && !relevantVariables[name] {
 			// We'll add a warning for this. Since we're right in the run block
 			// users shouldn't be defining variables that are not relevant.
 			diags = diags.Append(&hcl.Diagnostic{
 				Severity: hcl.DiagWarning,
 				Summary:  "Value for undeclared variable",
-				Detail:   fmt.Sprintf("The module under test does not declare a variable named %q, but it is declared in run block %q.", name, run.Name),
+				Detail:   fmt.Sprintf("The module under test does not declare a variable named %q, but it is declared in run block %q.", name, runName),
 				Subject:  expr.Range().Ptr(),
 			})
 			continue
@@ -1182,52 +1181,14 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 			diags = diags.Append(valueDiags)
 		}
 
-		values[name] = &terraform.InputValue{
+		variableValues[name] = &terraform.InputValue{
 			Value:       value,
 			SourceType:  terraform.ValueFromConfig,
 			SourceRange: tfdiags.SourceRangeFromHCL(expr.Range()),
 		}
 	}
 
-	// Finally, we check the configuration again. This is where we'll discover
-	// if there's any missing variables and fill in any optional variables that
-	// don't have a value already.
-
-	for name, variable := range config.Module.Variables {
-		if _, exists := values[name]; exists {
-			// Then we've provided a variable for this. It's all good.
-			continue
-		}
-
-		// Otherwise, we're going to give these variables a value. They'll be
-		// processed by the Terraform graph and provided a default value later
-		// if they have one.
-
-		if variable.Required() {
-			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "No value for required variable",
-				Detail: fmt.Sprintf("The module under test for run block %q has a required variable %q with no set value. Use a -var or -var-file command line argument or add this variable into a \"variables\" block within the test file or run block.",
-					run.Name, variable.Name),
-				Subject: variable.DeclRange.Ptr(),
-			})
-
-			values[name] = &terraform.InputValue{
-				Value:       cty.DynamicVal,
-				SourceType:  terraform.ValueFromConfig,
-				SourceRange: tfdiags.SourceRangeFromHCL(variable.DeclRange),
-			}
-		} else {
-			values[name] = &terraform.InputValue{
-				Value:       cty.NilVal,
-				SourceType:  terraform.ValueFromConfig,
-				SourceRange: tfdiags.SourceRangeFromHCL(variable.DeclRange),
-			}
-		}
-
-	}
-
-	return values, diags
+	return variableValues, diags
 }
 
 // FilterVariablesToModule splits the provided values into two disjoint maps:
@@ -1335,6 +1296,7 @@ func (runner *TestFileRunner) initVariables(file *moduletest.File) {
 			runner.globalVariables[name] = value
 		}
 	}
+
 	runner.fileVariables = make(map[string]hcl.Expression)
 	for name, expr := range file.Config.Variables {
 		runner.fileVariables[name] = expr

From df3ee21d82d0b1f62bb563693e84220e2837a791 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Wed, 21 Feb 2024 14:19:44 +0100
Subject: [PATCH 03/14] extract global and suite-level variables before
 configuring providers

---
 internal/backend/local/test.go           | 84 ++++++++++++++++--------
 internal/moduletest/config/config.go     |  4 +-
 internal/moduletest/hcl/provider.go      | 16 +----
 internal/moduletest/hcl/provider_test.go |  8 ++-
 4 files changed, 65 insertions(+), 47 deletions(-)

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index 7afe3b3dbd..dfe32fb086 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -382,9 +382,11 @@ func (runner *TestFileRunner) run(run *moduletest.Run, file *moduletest.File, st
 	}
 	runner.gatherProviders(key, config)
 
+	vars, diag := runner.GetGlobalAndSuiteVariables(runner.Suite.Config)
+	run.Diagnostics = run.Diagnostics.Append(diag)
 	// TODO: Merge local variables and suite-level variables
 	// It might be necessary to change the format or add an extra parameter, not sure yet
-	resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, runner.globalVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
+	resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, vars, runner.PriorOutputs, runner.Suite.configProviders[key])
 	defer resetConfig()
 
 	run.Diagnostics = run.Diagnostics.Append(configDiags)
@@ -949,7 +951,10 @@ func (runner *TestFileRunner) cleanup(file *moduletest.File) {
 			key = state.Run.Config.Module.Source.String()
 		}
 
-		reset, configDiags := configtest.TransformConfigForTest(config, state.Run, file, runner.globalVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
+		vars, varsDiags := runner.GetGlobalAndSuiteVariables(config)
+		diags = diags.Append(varsDiags)
+
+		reset, configDiags := configtest.TransformConfigForTest(config, state.Run, file, vars, runner.PriorOutputs, runner.Suite.configProviders[key])
 		diags = diags.Append(configDiags)
 
 		updated := state.State
@@ -1044,33 +1049,7 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 			continue
 		}
 
-		// By default, we parse global variables as HCL inputs.
-		parsingMode := configs.VariableParseHCL
-
-		cfg, exists := config.Module.Variables[name]
-
-		if exists {
-			// Unless we have some configuration that can actually tell us
-			// what parsing mode to use.
-			parsingMode = cfg.ParsingMode
-		}
-
-		value, valueDiags := variable.ParseVariableValue(parsingMode)
-		diags = diags.Append(valueDiags)
-		if diags.HasErrors() {
-			// We still add a value for this variable even though we couldn't
-			// parse it as we don't want to compound errors later. For example,
-			// the system would report this variable didn't have a value which
-			// would confuse the user because it does have a value, it's just
-			// not a valid value. We have added the diagnostics so the user
-			// will be informed about the error, and the test won't run. We'll
-			// just report only the relevant errors.
-			values[name] = &terraform.InputValue{
-				Value: cty.NilVal,
-			}
-			continue
-		}
-		values[name] = value
+		values[name] = runner.getGlobalVariable(name, variable, config)
 	}
 
 	// Second, we'll check the file level variables
@@ -1130,6 +1109,34 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 	return values, diags
 }
 
+func (runner *TestFileRunner) getGlobalVariable(name string, variable backend.UnparsedVariableValue, config *configs.Config) *terraform.InputValue {
+	// By default, we parse global variables as HCL inputs.
+	parsingMode := configs.VariableParseHCL
+
+	cfg, exists := config.Module.Variables[name]
+
+	if exists {
+		// Unless we have some configuration that can actually tell us
+		// what parsing mode to use.
+		parsingMode = cfg.ParsingMode
+	}
+
+	value, diags := variable.ParseVariableValue(parsingMode)
+	if diags.HasErrors() {
+		// We still add a value for this variable even though we couldn't
+		// parse it as we don't want to compound errors later. For example,
+		// the system would report this variable didn't have a value which
+		// would confuse the user because it does have a value, it's just
+		// not a valid value. We have added the diagnostics so the user
+		// will be informed about the error, and the test won't run. We'll
+		// just report only the relevant errors.
+		return &terraform.InputValue{
+			Value: cty.NilVal,
+		}
+	}
+	return value
+}
+
 // getVariablesFromConfiguration will process the variables from the configuration
 // and return a map of the variables and their values.
 func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terraform.InputValues, relevantVariables map[string]bool, runName string, variableConfig map[string]hcl.Expression) (terraform.InputValues, tfdiags.Diagnostics) {
@@ -1191,6 +1198,25 @@ func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terra
 	return variableValues, diags
 }
 
+// GetGlobalAndSuiteVariables returns the global and suite level variables
+func (runner *TestFileRunner) GetGlobalAndSuiteVariables(config *configs.Config) (terraform.InputValues, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	values := make(terraform.InputValues)
+	// TODO: Verify it's not problematic that we don't care about the variables being used
+	// First, let's look at the global variables.
+	for name, variable := range runner.globalVariables {
+		values[name] = runner.getGlobalVariable(name, variable, config)
+	}
+
+	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, nil, "TODO: Rewrite error to either not mention run block or wrap the error", runner.fileVariables)
+	diags = diags.Append(fileDiags)
+	for name, value := range fileValues {
+		values[name] = value
+	}
+
+	return values, diags
+}
+
 // FilterVariablesToModule splits the provided values into two disjoint maps:
 // moduleVars contains the ones that correspond with declarations in the root
 // module of the given configuration, while testOnlyVars contains any others
diff --git a/internal/moduletest/config/config.go b/internal/moduletest/config/config.go
index 710400dd3d..8bcc41d4bb 100644
--- a/internal/moduletest/config/config.go
+++ b/internal/moduletest/config/config.go
@@ -10,10 +10,10 @@ import (
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/internal/addrs"
-	"github.com/hashicorp/terraform/internal/backend"
 	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/moduletest"
 	hcltest "github.com/hashicorp/terraform/internal/moduletest/hcl"
+	"github.com/hashicorp/terraform/internal/terraform"
 )
 
 // TransformConfigForTest transforms the provided configuration ready for the
@@ -27,7 +27,7 @@ import (
 // We also return a reset function that should be called to return the
 // configuration to it's original state before the next run block or test file
 // needs to use it.
-func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *moduletest.File, availableVariables map[string]backend.UnparsedVariableValue, availableRunOutputs map[addrs.Run]cty.Value, requiredProviders map[string]bool) (func(), hcl.Diagnostics) {
+func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *moduletest.File, availableVariables terraform.InputValues, availableRunOutputs map[addrs.Run]cty.Value, requiredProviders map[string]bool) (func(), hcl.Diagnostics) {
 	var diags hcl.Diagnostics
 
 	// Currently, we only need to override the provider settings.
diff --git a/internal/moduletest/hcl/provider.go b/internal/moduletest/hcl/provider.go
index f87544579f..03d1fc7bc8 100644
--- a/internal/moduletest/hcl/provider.go
+++ b/internal/moduletest/hcl/provider.go
@@ -8,9 +8,9 @@ import (
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/internal/addrs"
-	"github.com/hashicorp/terraform/internal/backend"
 	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/lang"
+	"github.com/hashicorp/terraform/internal/terraform"
 )
 
 var _ hcl.Body = (*ProviderConfig)(nil)
@@ -27,7 +27,7 @@ type ProviderConfig struct {
 	Original hcl.Body
 
 	ConfigVariables     map[string]*configs.Variable
-	AvailableVariables  map[string]backend.UnparsedVariableValue
+	AvailableVariables  terraform.InputValues
 	AvailableRunOutputs map[addrs.Run]cty.Value
 }
 
@@ -88,17 +88,7 @@ func (p *ProviderConfig) transformAttributes(originals hcl.Attributes) (hcl.Attr
 					continue
 				}
 
-				if variable, exists := p.AvailableVariables[addr.Name]; exists {
-					// Then we have a value for this variable! So we think we'll
-					// be able to process it - let's parse it now.
-
-					parsingMode := configs.VariableParseHCL
-					if config, exists := p.ConfigVariables[addr.Name]; exists {
-						parsingMode = config.ParsingMode
-					}
-
-					value, valueDiags := variable.ParseVariableValue(parsingMode)
-					diags = append(diags, valueDiags.ToHCL()...)
+				if value, exists := p.AvailableVariables[addr.Name]; exists {
 					if value != nil {
 						availableVariables[addr.Name] = value.Value
 					}
diff --git a/internal/moduletest/hcl/provider_test.go b/internal/moduletest/hcl/provider_test.go
index 883ec1316f..d23cb2f334 100644
--- a/internal/moduletest/hcl/provider_test.go
+++ b/internal/moduletest/hcl/provider_test.go
@@ -183,10 +183,12 @@ func TestProviderConfig(t *testing.T) {
 					}
 					return variables
 				}(),
-				AvailableVariables: func() map[string]backend.UnparsedVariableValue {
-					variables := make(map[string]backend.UnparsedVariableValue)
+				AvailableVariables: func() terraform.InputValues {
+					variables := make(terraform.InputValues)
 					for name, value := range tc.variables {
-						variables[name] = &variable{value}
+						variables[name] = &terraform.InputValue{
+							Value: value,
+						}
 					}
 					return variables
 				}(),

From e57368045c0e2d73eb21f349e83dfd3738dea19d Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Thu, 22 Feb 2024 18:30:03 +0100
Subject: [PATCH 04/14] fix implementation of variable checking

---
 internal/backend/local/test.go | 42 ++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index dfe32fb086..3dd4aef63e 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -384,6 +384,7 @@ func (runner *TestFileRunner) run(run *moduletest.Run, file *moduletest.File, st
 
 	vars, diag := runner.GetGlobalAndSuiteVariables(runner.Suite.Config)
 	run.Diagnostics = run.Diagnostics.Append(diag)
+
 	// TODO: Merge local variables and suite-level variables
 	// It might be necessary to change the format or add an extra parameter, not sure yet
 	resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, vars, runner.PriorOutputs, runner.Suite.configProviders[key])
@@ -1052,17 +1053,37 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 		values[name] = runner.getGlobalVariable(name, variable, config)
 	}
 
+	// We don't care if the file level variables are relevant or not
+	ignoreRelevance := func(name string, expr hcl.Expression) (diags tfdiags.Diagnostics) {
+		return diags
+	}
+
 	// Second, we'll check the file level variables
 	// This is a bit more complicated, as the file and run level variables can reference
 	// previously defined variables.
-	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, relevantVariables, run.Name, runner.fileVariables)
+	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, ignoreRelevance, runner.fileVariables)
 	diags = diags.Append(fileDiags)
 	for name, value := range fileValues {
 		values[name] = value
 	}
 
+	// We want to make sure every variable declared in the run block is actually relevant.
+	validateRelevance := func(name string, expr hcl.Expression) (diags tfdiags.Diagnostics) {
+		if !relevantVariables[name] {
+			// We'll add a warning for this. Since we're right in the run block
+			// users shouldn't be defining variables that are not relevant.
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagWarning,
+				Summary:  "Value for undeclared variable",
+				Detail:   fmt.Sprintf("The module under test does not declare a variable named %q, but it is declared in run block %q.", name, run.Name),
+				Subject:  expr.Range().Ptr(),
+			})
+		}
+		return diags
+	}
+
 	// Third, we'll check the run level variables.
-	runValues, runDiags := runner.getVariablesFromConfiguration(values, relevantVariables, run.Name, run.Config.Variables)
+	runValues, runDiags := runner.getVariablesFromConfiguration(values, validateRelevance, run.Config.Variables)
 	diags = diags.Append(runDiags)
 	for name, value := range runValues {
 		values[name] = value
@@ -1103,7 +1124,6 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 				SourceRange: tfdiags.SourceRangeFromHCL(variable.DeclRange),
 			}
 		}
-
 	}
 
 	return values, diags
@@ -1139,7 +1159,7 @@ func (runner *TestFileRunner) getGlobalVariable(name string, variable backend.Un
 
 // getVariablesFromConfiguration will process the variables from the configuration
 // and return a map of the variables and their values.
-func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terraform.InputValues, relevantVariables map[string]bool, runName string, variableConfig map[string]hcl.Expression) (terraform.InputValues, tfdiags.Diagnostics) {
+func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terraform.InputValues, validateRelevance func(string, hcl.Expression) tfdiags.Diagnostics, variableConfig map[string]hcl.Expression) (terraform.InputValues, tfdiags.Diagnostics) {
 	var exprs []hcl.Expression
 	var diags tfdiags.Diagnostics
 	variableValues := make(terraform.InputValues)
@@ -1169,15 +1189,9 @@ func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terra
 	}
 
 	for name, expr := range variableConfig {
-		if relevantVariables != nil && !relevantVariables[name] {
-			// We'll add a warning for this. Since we're right in the run block
-			// users shouldn't be defining variables that are not relevant.
-			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagWarning,
-				Summary:  "Value for undeclared variable",
-				Detail:   fmt.Sprintf("The module under test does not declare a variable named %q, but it is declared in run block %q.", name, runName),
-				Subject:  expr.Range().Ptr(),
-			})
+		relevanceDiags := validateRelevance(name, expr)
+		if relevanceDiags.HasWarnings() {
+			diags = diags.Append(relevanceDiags)
 			continue
 		}
 
@@ -1208,7 +1222,7 @@ func (runner *TestFileRunner) GetGlobalAndSuiteVariables(config *configs.Config)
 		values[name] = runner.getGlobalVariable(name, variable, config)
 	}
 
-	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, nil, "TODO: Rewrite error to either not mention run block or wrap the error", runner.fileVariables)
+	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, func(s string, e hcl.Expression) tfdiags.Diagnostics { return tfdiags.Diagnostics{} }, runner.fileVariables)
 	diags = diags.Append(fileDiags)
 	for name, value := range fileValues {
 		values[name] = value

From 5bed43c16dafca12a0ff443ae062ba1874c1a772 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Thu, 22 Feb 2024 18:33:42 +0100
Subject: [PATCH 05/14] remove whitespace

---
 internal/terraform/context_apply.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/terraform/context_apply.go b/internal/terraform/context_apply.go
index 801a5b44ea..10686b6f06 100644
--- a/internal/terraform/context_apply.go
+++ b/internal/terraform/context_apply.go
@@ -183,7 +183,7 @@ func (c *Context) ApplyAndEval(plan *plans.Plan, config *configs.Config, opts *A
 			"Applied changes may be incomplete",
 			`The plan was created with the -target option in effect, so some changes requested in the configuration may have been ignored and the output values may not be fully updated. Run the following command to verify that no other changes are pending:
     terraform plan
-	
+
 Note that the -target option is not suitable for routine use, and is provided only for exceptional situations such as recovering from errors or mistakes, or when Terraform specifically suggests to use it as part of an error message.`,
 		))
 	}

From 631293060bba925cad8369441912b3f519810bb4 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Thu, 22 Feb 2024 19:12:04 +0100
Subject: [PATCH 06/14] only extract global and suite level variables once

---
 internal/backend/local/test.go | 88 ++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 40 deletions(-)

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index 3dd4aef63e..151456a958 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -245,9 +245,14 @@ type TestFileRunner struct {
 	PriorOutputs map[addrs.Run]cty.Value
 
 	// globalVariables are globally defined variables, e.g. through tfvars or CLI flags
-	globalVariables map[string]backend.UnparsedVariableValue
+	globalVariables terraform.InputValues
 	// fileVariables are defined in the variables section of a test file
-	fileVariables map[string]hcl.Expression
+	fileVariables terraform.InputValues
+	// fileVariableExpressions are the hcl expressions for the fileVariables
+	fileVariableExpressions map[string]hcl.Expression
+	// GlobalAndSuiteVariables is a combination of globalVariables and fileVariables
+	// created for convenience
+	GlobalAndSuiteVariables terraform.InputValues
 }
 
 // TestFileState is a helper struct that just maps a run block to the state that
@@ -263,6 +268,14 @@ func (runner *TestFileRunner) Test(file *moduletest.File) {
 	// First thing, initialise the global variables for the file
 	runner.initVariables(file)
 
+	vars := make(terraform.InputValues)
+	for name, value := range runner.globalVariables {
+		vars[name] = value
+	}
+	for name, value := range runner.fileVariables {
+		vars[name] = value
+	}
+
 	// The file validation only returns warnings so we'll just add them without
 	// checking anything about them.
 	file.Diagnostics = file.Diagnostics.Append(file.Config.Validate(runner.Suite.Config))
@@ -382,12 +395,7 @@ func (runner *TestFileRunner) run(run *moduletest.Run, file *moduletest.File, st
 	}
 	runner.gatherProviders(key, config)
 
-	vars, diag := runner.GetGlobalAndSuiteVariables(runner.Suite.Config)
-	run.Diagnostics = run.Diagnostics.Append(diag)
-
-	// TODO: Merge local variables and suite-level variables
-	// It might be necessary to change the format or add an extra parameter, not sure yet
-	resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, vars, runner.PriorOutputs, runner.Suite.configProviders[key])
+	resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, runner.GlobalAndSuiteVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
 	defer resetConfig()
 
 	run.Diagnostics = run.Diagnostics.Append(configDiags)
@@ -952,10 +960,7 @@ func (runner *TestFileRunner) cleanup(file *moduletest.File) {
 			key = state.Run.Config.Module.Source.String()
 		}
 
-		vars, varsDiags := runner.GetGlobalAndSuiteVariables(config)
-		diags = diags.Append(varsDiags)
-
-		reset, configDiags := configtest.TransformConfigForTest(config, state.Run, file, vars, runner.PriorOutputs, runner.Suite.configProviders[key])
+		reset, configDiags := configtest.TransformConfigForTest(config, state.Run, file, runner.GlobalAndSuiteVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
 		diags = diags.Append(configDiags)
 
 		updated := state.State
@@ -1021,7 +1026,7 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 
 	// Second, we'll check to see which variables the suites global variables
 	// themselves reference.
-	diags = diags.Append(getRelevantVariables(runner.fileVariables))
+	diags = diags.Append(getRelevantVariables(runner.fileVariableExpressions))
 
 	// Third, we'll check to see which variables the run block variables
 	// themselves reference. We might be processing variables just for the file
@@ -1044,13 +1049,13 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 	values := make(terraform.InputValues)
 
 	// First, let's look at the global variables.
-	for name, variable := range runner.globalVariables {
+	for name, value := range runner.globalVariables {
 		if !relevantVariables[name] {
 			// Then this run block doesn't need this value.
 			continue
 		}
 
-		values[name] = runner.getGlobalVariable(name, variable, config)
+		values[name] = value
 	}
 
 	// We don't care if the file level variables are relevant or not
@@ -1061,7 +1066,7 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 	// Second, we'll check the file level variables
 	// This is a bit more complicated, as the file and run level variables can reference
 	// previously defined variables.
-	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, ignoreRelevance, runner.fileVariables)
+	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, ignoreRelevance, runner.fileVariableExpressions)
 	diags = diags.Append(fileDiags)
 	for name, value := range fileValues {
 		values[name] = value
@@ -1212,25 +1217,6 @@ func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terra
 	return variableValues, diags
 }
 
-// GetGlobalAndSuiteVariables returns the global and suite level variables
-func (runner *TestFileRunner) GetGlobalAndSuiteVariables(config *configs.Config) (terraform.InputValues, tfdiags.Diagnostics) {
-	var diags tfdiags.Diagnostics
-	values := make(terraform.InputValues)
-	// TODO: Verify it's not problematic that we don't care about the variables being used
-	// First, let's look at the global variables.
-	for name, variable := range runner.globalVariables {
-		values[name] = runner.getGlobalVariable(name, variable, config)
-	}
-
-	fileValues, fileDiags := runner.getVariablesFromConfiguration(values, func(s string, e hcl.Expression) tfdiags.Diagnostics { return tfdiags.Diagnostics{} }, runner.fileVariables)
-	diags = diags.Append(fileDiags)
-	for name, value := range fileValues {
-		values[name] = value
-	}
-
-	return values, diags
-}
-
 // FilterVariablesToModule splits the provided values into two disjoint maps:
 // moduleVars contains the ones that correspond with declarations in the root
 // module of the given configuration, while testOnlyVars contains any others
@@ -1324,22 +1310,44 @@ func (runner *TestFileRunner) AddVariablesToConfig(config *configs.Config, varia
 // merging the global variables from the test suite into the variables from
 // the file.
 func (runner *TestFileRunner) initVariables(file *moduletest.File) {
-	runner.globalVariables = make(map[string]backend.UnparsedVariableValue)
+	// First, we get the global variables from the suite and test suite
+	runner.globalVariables = make(terraform.InputValues)
 	for name, value := range runner.Suite.GlobalVariables {
-		runner.globalVariables[name] = value
+		runner.globalVariables[name] = runner.getGlobalVariable(name, value, runner.Suite.Config)
 	}
 	if filepath.Dir(file.Name) == runner.Suite.TestingDirectory {
 		// If the file is in the testing directory, then also include any
 		// variables that are defined within the default variable file also in
 		// the test directory.
 		for name, value := range runner.Suite.GlobalTestVariables {
-			runner.globalVariables[name] = value
+			runner.globalVariables[name] = runner.getGlobalVariable(name, value, runner.Suite.Config)
 		}
 	}
 
-	runner.fileVariables = make(map[string]hcl.Expression)
+	// Second, we collect the variable expressions so they can later be used to
+	// check for references to variables that are also relevant
+	runner.fileVariableExpressions = make(map[string]hcl.Expression)
 	for name, expr := range file.Config.Variables {
-		runner.fileVariables[name] = expr
+		runner.fileVariableExpressions[name] = expr
+	}
+
+	// Third, we get the variables from the file
+	runner.fileVariables = make(terraform.InputValues)
+	fileValues, fileDiags := runner.getVariablesFromConfiguration(runner.globalVariables, func(s string, e hcl.Expression) tfdiags.Diagnostics { return tfdiags.Diagnostics{} }, runner.fileVariableExpressions)
+
+	for name, value := range fileValues {
+		runner.fileVariables[name] = value
+	}
+	file.Diagnostics = file.Diagnostics.Append(fileDiags)
+
+	// Finally, we merge the global and file variables together to get all
+	// available variables outside the run specific ones
+	runner.GlobalAndSuiteVariables = make(terraform.InputValues)
+	for name, value := range runner.globalVariables {
+		runner.GlobalAndSuiteVariables[name] = value
+	}
+	for name, value := range runner.fileVariables {
+		runner.GlobalAndSuiteVariables[name] = value
 	}
 }
 

From ea0856bfe7b99cf9f2d7f99ee09e26af335d37d3 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Thu, 22 Feb 2024 19:18:07 +0100
Subject: [PATCH 07/14] add a line of documentation to clarify

---
 website/docs/language/tests/index.mdx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/website/docs/language/tests/index.mdx b/website/docs/language/tests/index.mdx
index 67f4ffbbc4..a5a8612c5e 100644
--- a/website/docs/language/tests/index.mdx
+++ b/website/docs/language/tests/index.mdx
@@ -8,7 +8,7 @@ description: >-
 
 -> **Note:** This testing framework is available in Terraform v1.6.0 and later.
 
-Terraform tests let authors validate that module configuration updates do not introduce breaking changes. Tests run against test-specific, short-lived resources, preventing any risk to your existing infrastructure or state. 
+Terraform tests let authors validate that module configuration updates do not introduce breaking changes. Tests run against test-specific, short-lived resources, preventing any risk to your existing infrastructure or state.
 
 ## Integration or Unit testing
 
@@ -174,7 +174,7 @@ For tests defined in a test directory, any variable values defined in automatic
 
 ### Variable References
 
-Variables you define within `run` blocks can refer to outputs from modules executed in earlier `run` blocks and variables defined at higher precedence levels.
+Variables you define within `run` blocks can refer to outputs from modules executed in earlier `run` blocks and variables defined at higher precedence levels. Variables in the root `variables` block can only refer to global variables.
 
 For example, the following code block shows how a variable can refer to higher precedence variables and previous run blocks:
 

From 6a4ef8e2b3f22f6036bef3a3ea69eb737a663841 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Fri, 23 Feb 2024 13:46:12 +0100
Subject: [PATCH 08/14] fix comment

Co-authored-by: Liam Cervante <liam.cervante@hashicorp.com>
---
 internal/backend/local/test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index 151456a958..402d249e28 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -1024,7 +1024,7 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
 		return getVarsDiags
 	}
 
-	// Second, we'll check to see which variables the suites global variables
+	// Second, we'll check to see which variables the file variables
 	// themselves reference.
 	diags = diags.Append(getRelevantVariables(runner.fileVariableExpressions))
 

From 4915bdf90c93880cf817847cec583aa09bcfafe8 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Fri, 23 Feb 2024 13:46:31 +0100
Subject: [PATCH 09/14] fix comment

Co-authored-by: Liam Cervante <liam.cervante@hashicorp.com>
---
 website/docs/language/tests/index.mdx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/website/docs/language/tests/index.mdx b/website/docs/language/tests/index.mdx
index a5a8612c5e..f2c9bfe0da 100644
--- a/website/docs/language/tests/index.mdx
+++ b/website/docs/language/tests/index.mdx
@@ -174,7 +174,7 @@ For tests defined in a test directory, any variable values defined in automatic
 
 ### Variable References
 
-Variables you define within `run` blocks can refer to outputs from modules executed in earlier `run` blocks and variables defined at higher precedence levels. Variables in the root `variables` block can only refer to global variables.
+Variables you define within `run` blocks can refer to outputs from modules executed in earlier `run` blocks and variables defined at higher precedence levels. Variables defined within the file level `variables` block can only refer to global variables.
 
 For example, the following code block shows how a variable can refer to higher precedence variables and previous run blocks:
 

From 57f6ce81b5cf2bebf976d2714c1d80e5db426d90 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Fri, 23 Feb 2024 13:49:23 +0100
Subject: [PATCH 10/14] rename suite to file and make var internal

---
 internal/backend/local/test.go | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index 402d249e28..5ebb100077 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -250,9 +250,9 @@ type TestFileRunner struct {
 	fileVariables terraform.InputValues
 	// fileVariableExpressions are the hcl expressions for the fileVariables
 	fileVariableExpressions map[string]hcl.Expression
-	// GlobalAndSuiteVariables is a combination of globalVariables and fileVariables
+	// globalAndFileVariables is a combination of globalVariables and fileVariables
 	// created for convenience
-	GlobalAndSuiteVariables terraform.InputValues
+	globalAndFileVariables terraform.InputValues
 }
 
 // TestFileState is a helper struct that just maps a run block to the state that
@@ -395,7 +395,7 @@ func (runner *TestFileRunner) run(run *moduletest.Run, file *moduletest.File, st
 	}
 	runner.gatherProviders(key, config)
 
-	resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, runner.GlobalAndSuiteVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
+	resetConfig, configDiags := configtest.TransformConfigForTest(config, run, file, runner.globalAndFileVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
 	defer resetConfig()
 
 	run.Diagnostics = run.Diagnostics.Append(configDiags)
@@ -960,7 +960,7 @@ func (runner *TestFileRunner) cleanup(file *moduletest.File) {
 			key = state.Run.Config.Module.Source.String()
 		}
 
-		reset, configDiags := configtest.TransformConfigForTest(config, state.Run, file, runner.GlobalAndSuiteVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
+		reset, configDiags := configtest.TransformConfigForTest(config, state.Run, file, runner.globalAndFileVariables, runner.PriorOutputs, runner.Suite.configProviders[key])
 		diags = diags.Append(configDiags)
 
 		updated := state.State
@@ -1342,12 +1342,12 @@ func (runner *TestFileRunner) initVariables(file *moduletest.File) {
 
 	// Finally, we merge the global and file variables together to get all
 	// available variables outside the run specific ones
-	runner.GlobalAndSuiteVariables = make(terraform.InputValues)
+	runner.globalAndFileVariables = make(terraform.InputValues)
 	for name, value := range runner.globalVariables {
-		runner.GlobalAndSuiteVariables[name] = value
+		runner.globalAndFileVariables[name] = value
 	}
 	for name, value := range runner.fileVariables {
-		runner.GlobalAndSuiteVariables[name] = value
+		runner.globalAndFileVariables[name] = value
 	}
 }
 

From a4651fac4ceaf99c25fb60daa1521e130de04df3 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Fri, 23 Feb 2024 13:49:56 +0100
Subject: [PATCH 11/14] append diags unconditionally

---
 internal/backend/local/test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index 5ebb100077..d8c56f944d 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -1195,8 +1195,8 @@ func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terra
 
 	for name, expr := range variableConfig {
 		relevanceDiags := validateRelevance(name, expr)
+		diags = diags.Append(relevanceDiags)
 		if relevanceDiags.HasWarnings() {
-			diags = diags.Append(relevanceDiags)
 			continue
 		}
 

From 0529299cc32677e769e88623a6fa459e1be95aff Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Fri, 23 Feb 2024 13:50:09 +0100
Subject: [PATCH 12/14] run terraform fmt on test

---
 .../testdata/test/global_var_ref_in_suite_var/main.tf       | 6 +++---
 .../global_var_ref_in_suite_var/run_block_output.tftest.hcl | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/internal/command/testdata/test/global_var_ref_in_suite_var/main.tf b/internal/command/testdata/test/global_var_ref_in_suite_var/main.tf
index a776ac2eef..5bcb244a1b 100644
--- a/internal/command/testdata/test/global_var_ref_in_suite_var/main.tf
+++ b/internal/command/testdata/test/global_var_ref_in_suite_var/main.tf
@@ -1,8 +1,8 @@
 variable "input" {
   default = null
-  type    = object({
-              organization_name = string
-            })
+  type = object({
+    organization_name = string
+  })
 }
 
 output "value" {
diff --git a/internal/command/testdata/test/global_var_ref_in_suite_var/run_block_output.tftest.hcl b/internal/command/testdata/test/global_var_ref_in_suite_var/run_block_output.tftest.hcl
index fd43cbda79..38bf5750e1 100644
--- a/internal/command/testdata/test/global_var_ref_in_suite_var/run_block_output.tftest.hcl
+++ b/internal/command/testdata/test/global_var_ref_in_suite_var/run_block_output.tftest.hcl
@@ -1,7 +1,7 @@
 
 variables {
   input = {
-      organization_name = var.org_name
+    organization_name = var.org_name
   }
 }
 

From 5a72e696ce8c571eb49ec8cdaa65f85a440c7244 Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Fri, 23 Feb 2024 13:55:37 +0100
Subject: [PATCH 13/14] check for presence and not for warnings

---
 internal/backend/local/test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index d8c56f944d..ca1058c59d 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -1196,7 +1196,7 @@ func (runner *TestFileRunner) getVariablesFromConfiguration(knownVariables terra
 	for name, expr := range variableConfig {
 		relevanceDiags := validateRelevance(name, expr)
 		diags = diags.Append(relevanceDiags)
-		if relevanceDiags.HasWarnings() {
+		if len(relevanceDiags) > 0 {
 			continue
 		}
 

From 4f23efc14eaccef48d69adfdc1aa644fcdfaba9f Mon Sep 17 00:00:00 2001
From: Daniel Schmidt <danielmschmidt92@gmail.com>
Date: Fri, 23 Feb 2024 14:00:53 +0100
Subject: [PATCH 14/14] remove unused config

---
 internal/moduletest/config/config.go     | 2 --
 internal/moduletest/hcl/provider.go      | 6 ++----
 internal/moduletest/hcl/provider_test.go | 9 ---------
 3 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/internal/moduletest/config/config.go b/internal/moduletest/config/config.go
index 8bcc41d4bb..a1ea88b289 100644
--- a/internal/moduletest/config/config.go
+++ b/internal/moduletest/config/config.go
@@ -91,7 +91,6 @@ func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *m
 				Version:    testProvider.Version,
 				Config: &hcltest.ProviderConfig{
 					Original:            testProvider.Config,
-					ConfigVariables:     config.Module.Variables,
 					AvailableVariables:  availableVariables,
 					AvailableRunOutputs: availableRunOutputs,
 				},
@@ -119,7 +118,6 @@ func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *m
 				Version:    provider.Version,
 				Config: &hcltest.ProviderConfig{
 					Original:            provider.Config,
-					ConfigVariables:     config.Module.Variables,
 					AvailableVariables:  availableVariables,
 					AvailableRunOutputs: availableRunOutputs,
 				},
diff --git a/internal/moduletest/hcl/provider.go b/internal/moduletest/hcl/provider.go
index 03d1fc7bc8..2b3784eeed 100644
--- a/internal/moduletest/hcl/provider.go
+++ b/internal/moduletest/hcl/provider.go
@@ -8,7 +8,6 @@ import (
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/hashicorp/terraform/internal/addrs"
-	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/lang"
 	"github.com/hashicorp/terraform/internal/terraform"
 )
@@ -26,7 +25,6 @@ var _ hcl.Body = (*ProviderConfig)(nil)
 type ProviderConfig struct {
 	Original hcl.Body
 
-	ConfigVariables     map[string]*configs.Variable
 	AvailableVariables  terraform.InputValues
 	AvailableRunOutputs map[addrs.Run]cty.Value
 }
@@ -52,7 +50,7 @@ func (p *ProviderConfig) PartialContent(schema *hcl.BodySchema) (*hcl.BodyConten
 		Attributes:       attrs,
 		Blocks:           p.transformBlocks(content.Blocks),
 		MissingItemRange: content.MissingItemRange,
-	}, &ProviderConfig{rest, p.ConfigVariables, p.AvailableVariables, p.AvailableRunOutputs}, diags
+	}, &ProviderConfig{rest, p.AvailableVariables, p.AvailableRunOutputs}, diags
 }
 
 func (p *ProviderConfig) JustAttributes() (hcl.Attributes, hcl.Diagnostics) {
@@ -127,7 +125,7 @@ func (p *ProviderConfig) transformBlocks(originals hcl.Blocks) hcl.Blocks {
 		blocks[name] = &hcl.Block{
 			Type:        block.Type,
 			Labels:      block.Labels,
-			Body:        &ProviderConfig{block.Body, p.ConfigVariables, p.AvailableVariables, p.AvailableRunOutputs},
+			Body:        &ProviderConfig{block.Body, p.AvailableVariables, p.AvailableRunOutputs},
 			DefRange:    block.DefRange,
 			TypeRange:   block.TypeRange,
 			LabelRanges: block.LabelRanges,
diff --git a/internal/moduletest/hcl/provider_test.go b/internal/moduletest/hcl/provider_test.go
index d23cb2f334..97bffb9b48 100644
--- a/internal/moduletest/hcl/provider_test.go
+++ b/internal/moduletest/hcl/provider_test.go
@@ -174,15 +174,6 @@ func TestProviderConfig(t *testing.T) {
 
 			config := ProviderConfig{
 				Original: file.Body,
-				ConfigVariables: func() map[string]*configs.Variable {
-					variables := make(map[string]*configs.Variable)
-					for variable := range tc.variables {
-						variables[variable] = &configs.Variable{
-							Name: variable,
-						}
-					}
-					return variables
-				}(),
 				AvailableVariables: func() terraform.InputValues {
 					variables := make(terraform.InputValues)
 					for name, value := range tc.variables {