mocking overrides: default to concrete empty object when values are missing (#34563)

Author: Liam Cervante

internal/command/test_test.go

@@ -29,6 +29,7 @@ func TestTest_Runs(t *testing.T) {
 		expectedErr           string
 		expectedResourceCount int
 		code                  int
+		initCode              int
 		skip                  bool
 	}{
 		"simple_pass": {
@@ -212,9 +213,13 @@ func TestTest_Runs(t *testing.T) {
 			code:        0,
 		},
 		"mocking": {
-			expectedOut: "5 passed, 0 failed.",
+			expectedOut: "6 passed, 0 failed.",
 			code:        0,
 		},
+		"mocking-invalid": {
+			expectedErr: "Invalid outputs attribute",
+			initCode:    1,
+		},
 		"dangling_data_block": {
 			expectedOut: "2 passed, 0 failed.",
 			code:        0,
@@ -270,8 +275,30 @@ func TestTest_Runs(t *testing.T) {
 				Meta: meta,
 			}
 
-			if code := init.Run(nil); code != 0 {
-				t.Fatalf("expected status code 0 but got %d: %s", code, ui.ErrorWriter)
+			if code := init.Run(nil); code != tc.initCode {
+				t.Fatalf("expected status code %d but got %d: %s", tc.initCode, code, ui.ErrorWriter)
+			}
+
+			if tc.initCode > 0 {
+				// Then we don't expect the init step to succeed. So we'll check
+				// the init output for our expected error messages and outputs.
+
+				stdout, stderr := ui.ErrorWriter.String(), ui.ErrorWriter.String()
+
+				if !strings.Contains(stdout, tc.expectedOut) {
+					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 != "" {
+					t.Errorf("unexpected stderr output\n%s", stderr)
+				}
+
+				// If `terraform init` failed, then we don't expect that
+				// `terraform test` will have run at all, so we can just return
+				// here.
+				return
 			}
 
 			c := &TestCommand{

internal/command/testdata/test/mocking-invalid/child/main.tf

@@ -0,0 +1,30 @@
+terraform {
+  required_providers {
+    test = {
+      source = "hashicorp/test"
+      configuration_aliases = [test.primary, test.secondary]
+    }
+  }
+}
+
+variable "instances" {
+  type = number
+}
+
+resource "test_resource" "primary" {
+  provider = test.primary
+  count = var.instances
+}
+
+resource "test_resource" "secondary" {
+  provider = test.secondary
+  count = var.instances
+}
+
+output "primary" {
+  value = test_resource.primary
+}
+
+output "secondary" {
+  value = test_resource.secondary
+}

internal/command/testdata/test/mocking-invalid/main.tf

@@ -0,0 +1,46 @@
+terraform {
+  required_providers {
+    test = {
+      source = "hashicorp/test"
+    }
+  }
+}
+
+provider "test" {
+  alias = "primary"
+}
+
+provider "test" {
+  alias = "secondary"
+}
+
+variable "instances" {
+  type = number
+}
+
+variable "child_instances" {
+  type = number
+}
+
+resource "test_resource" "primary" {
+  provider = test.primary
+  count = var.instances
+}
+
+resource "test_resource" "secondary" {
+  provider = test.secondary
+  count = var.instances
+}
+
+module "child" {
+  count = var.instances
+
+  source = "./child"
+
+  providers = {
+    test.primary = test.primary
+    test.secondary = test.secondary
+  }
+
+  instances = var.child_instances
+}

internal/command/testdata/test/mocking-invalid/tests/module_mocked_invalid_type.tftest.hcl

@@ -0,0 +1,13 @@
+override_module {
+  target  = module.child[1]
+  outputs = "should be an object"
+}
+
+variables {
+  instances = 3
+  child_instances = 1
+}
+
+run "test" {
+  # We won't even execute this, as the configuration isn't valid.
+}

internal/command/testdata/test/mocking/tests/module_empty_outputs.tftest.hcl

@@ -0,0 +1,12 @@
+override_module {
+  target = module.child[1]
+}
+
+variables {
+  instances = 3
+  child_instances = 1
+}
+
+run "test" {
+  # Just want to make sure things don't crash with missing `outputs` attribute.
+}

internal/configs/mock_provider.go

@@ -435,8 +435,27 @@ func decodeOverrideBlock(block *hcl.Block, attributeName string, blockName strin
 		diags = append(diags, valueDiags...)
 	} else {
 		// It's fine if we don't have any values, just means we'll generate
-		// values for everything ourselves.
-		override.Values = cty.NilVal
+		// values for everything ourselves. We set this to an empty object so
+		// it's equivalent to `values = {}` which makes later processing easier.
+		override.Values = cty.EmptyObjectVal
+	}
+
+	if !override.Values.Type().IsObjectType() {
+
+		var attributePreposition string
+		switch attributeName {
+		case "outputs":
+			attributePreposition = "an"
+		default:
+			attributePreposition = "a"
+		}
+
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  fmt.Sprintf("Invalid %s attribute", attributeName),
+			Detail:   fmt.Sprintf("%s blocks must specify %s %s attribute that is an object.", blockName, attributePreposition, attributeName),
+			Subject:  override.ValuesRange.Ptr(),
+		})
 	}
 
 	return override, diags

internal/terraform/graph.go

@@ -196,7 +196,13 @@ func (g *Graph) checkAndApplyOverrides(overrides *mocking.Overrides, target dag.
 
 		setOverride := func(values cty.Value) {
 			key := v.Addr.OutputValue.Name
-			if values.Type().HasAttribute(key) {
+
+			// The values.Type() should be an object type, but it might have
+			// been set to nil by a test or something. We can handle it in the
+			// same way as the attribute just not being specified. It's
+			// functionally the same for us and not something we need to raise
+			// alarms about.
+			if values.Type().IsObjectType() && values.Type().HasAttribute(key) {
 				v.override = values.GetAttr(key)
 			} else {
 				// If we don't have a value provided for an output, then we'll