From 802d49a9cacea4de867cf753932a95430ff8e664 Mon Sep 17 00:00:00 2001
From: Liam Cervante <liam.cervante@hashicorp.com>
Date: Fri, 1 Sep 2023 16:53:23 +0200
Subject: [PATCH] Handle null outputs in Terraform test files (#33781)

---
 internal/backend/local/test.go                | 28 ++++++++++++++++++-
 internal/command/test_test.go                 |  4 +++
 .../testdata/test/null-outputs/main.tf        |  8 ++++++
 .../test/null-outputs/main.tftest.hcl         | 22 +++++++++++++++
 internal/terraform/evaluate.go                | 18 ++++++++----
 5 files changed, 74 insertions(+), 6 deletions(-)
 create mode 100644 internal/command/testdata/test/null-outputs/main.tf
 create mode 100644 internal/command/testdata/test/null-outputs/main.tftest.hcl

diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go
index 76892d18e4..85c38c69ce 100644
--- a/internal/backend/local/test.go
+++ b/internal/backend/local/test.go
@@ -1345,7 +1345,33 @@ func (runner *TestFileRunner) ctx(run *moduletest.Run, file *moduletest.File, av
 						},
 					})
 
-					if value.Sensitive {
+					if value == nil {
+						// Then this output returned null when the configuration
+						// executed. For now, we'll just skip this output.
+						//
+						// There are several things we could try to do, like
+						// figure out the type based on the variable that it
+						// is referencing and wrap it up as cty.Val(...) or we
+						// could not try and work anything out and return it as
+						// a cty.NilVal.
+						//
+						// Both of these mean the error would be raised later
+						// as non-optional variables would say they don't have
+						// a value. By just ignoring it here, we get an error
+						// quicker that says this output doesn't exist. I think
+						// that would prompt users to go look at the output and
+						// realise it might be returning null and make the
+						// connection. With the other approaches they'd look at
+						// their variable definitions and think they are
+						// assigning it a value since we would be telling them
+						// the output does exist.
+						//
+						// Let's do the simple thing now, and see what the
+						// future holds.
+						continue
+					}
+
+					if value.Sensitive || output.Sensitive {
 						outputs[output.Name] = value.Value.Mark(marks.Sensitive)
 						continue
 					}
diff --git a/internal/command/test_test.go b/internal/command/test_test.go
index 3f60099582..e583920a33 100644
--- a/internal/command/test_test.go
+++ b/internal/command/test_test.go
@@ -165,6 +165,10 @@ func TestTest(t *testing.T) {
 			args:     []string{"-var=number_input=0", "-var=string_input=Hello, world!", "-var=list_input=[\"Hello\",\"world\"]"},
 			code:     0,
 		},
+		"null-outputs": {
+			expected: "2 passed, 0 failed.",
+			code:     0,
+		},
 	}
 	for name, tc := range tcs {
 		t.Run(name, func(t *testing.T) {
diff --git a/internal/command/testdata/test/null-outputs/main.tf b/internal/command/testdata/test/null-outputs/main.tf
new file mode 100644
index 0000000000..83bff5c54b
--- /dev/null
+++ b/internal/command/testdata/test/null-outputs/main.tf
@@ -0,0 +1,8 @@
+
+variable "input" {
+  type = number
+}
+
+output "output" {
+  value = var.input > 5 ? var.input : null
+}
diff --git a/internal/command/testdata/test/null-outputs/main.tftest.hcl b/internal/command/testdata/test/null-outputs/main.tftest.hcl
new file mode 100644
index 0000000000..f8b8072438
--- /dev/null
+++ b/internal/command/testdata/test/null-outputs/main.tftest.hcl
@@ -0,0 +1,22 @@
+
+run "first" {
+  variables {
+    input = 2
+  }
+
+  assert {
+    condition = output.output == null
+    error_message = "output should have been null"
+  }
+}
+
+run "second" {
+  variables {
+    input = 8
+  }
+
+  assert {
+    condition = output.output == 8
+    error_message = "output should have been 8"
+  }
+}
diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go
index 0884da2f6b..b9548bd728 100644
--- a/internal/terraform/evaluate.go
+++ b/internal/terraform/evaluate.go
@@ -981,13 +981,21 @@ func (d *evaluationStateData) GetOutput(addr addrs.OutputValue, rng tfdiags.Sour
 	}
 
 	output := d.Evaluator.State.OutputValue(addr.Absolute(d.ModulePath))
-
-	val := output.Value
-	if val == cty.NilVal {
-		// Not evaluated yet?
-		val = cty.DynamicVal
+	if output == nil {
+		// Then the output itself returned null, so we'll package that up and
+		// pass it on.
+		output = &states.OutputValue{
+			Addr:      addr.Absolute(d.ModulePath),
+			Value:     cty.NilVal,
+			Sensitive: config.Sensitive,
+		}
+	} else if output.Value == cty.NilVal {
+		// Then we did get a value but Terraform itself thought it was NilVal
+		// so we treat this as if the value isn't yet known.
+		output.Value = cty.DynamicVal
 	}
 
+	val := output.Value
 	if output.Sensitive {
 		val = val.Mark(marks.Sensitive)
 	}