diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 554964142a..7628c6a677 100644 --- a/internal/command/test_test.go +++ b/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{ diff --git a/internal/command/testdata/test/mocking-invalid/child/main.tf b/internal/command/testdata/test/mocking-invalid/child/main.tf new file mode 100644 index 0000000000..2ef4e4d979 --- /dev/null +++ b/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 +} diff --git a/internal/command/testdata/test/mocking-invalid/main.tf b/internal/command/testdata/test/mocking-invalid/main.tf new file mode 100644 index 0000000000..49506e06c3 --- /dev/null +++ b/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 +} diff --git a/internal/command/testdata/test/mocking-invalid/tests/module_mocked_invalid_type.tftest.hcl b/internal/command/testdata/test/mocking-invalid/tests/module_mocked_invalid_type.tftest.hcl new file mode 100644 index 0000000000..ee4730f89b --- /dev/null +++ b/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. +} diff --git a/internal/command/testdata/test/mocking/tests/module_empty_outputs.tftest.hcl b/internal/command/testdata/test/mocking/tests/module_empty_outputs.tftest.hcl new file mode 100644 index 0000000000..6553ee3f37 --- /dev/null +++ b/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. +} diff --git a/internal/configs/mock_provider.go b/internal/configs/mock_provider.go index 5eef3d3927..f367e00516 100644 --- a/internal/configs/mock_provider.go +++ b/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 diff --git a/internal/terraform/graph.go b/internal/terraform/graph.go index 4991c9a3b5..559c449277 100644 --- a/internal/terraform/graph.go +++ b/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