terraform icon indicating copy to clipboard operation
terraform copied to clipboard

`terraform test`: silently ignores when two test_assertions have the same component, ignoring result of all but one of them

Open drewmullen opened this issue 3 years ago • 10 comments

Terraform Version

using RC due to bug. I have also tested these issues with 1.0.8

$ tf -v
Terraform v1.1.0-rc1
on darwin_amd64

Terraform Configuration Files

Scenario 1: In this branch I expect the terraform test to take about 45m (the resources take a long time to destroy) but it only takes about 3m.

Scenario 2: When i that didnt work as expected i tried purposely getting tests to fail and i was unable to. I distilled a very simple PR to demonstrate what should be an easy failure

resource "test_assertions" "expect_failure" {
  component = "tags"

  check "fail" {
    description = "this test should fail"
    condition   = true == false
  }
  equal "scheme" {
    description = "should fail"
    got         = true
    want        = false 
  }
}

Debug Output

Will post upon request

Expected Behavior

Scenario 1: tests should take about 45m to complete Scenario 2: tests should fail

Actual Behavior

Scenario 1: tests completed within 5m Scenario 2: tests pass

Additional Context

Conversation started on discuss.hashicorp.com

References

drewmullen avatar Dec 10 '21 13:12 drewmullen

Hi @drewmullen, thanks for reporting this.

I'm unable to reproduce the failure with the test configuration you supplied. Here's what I did:

  1. mkdir -p tests/30133
  2. Copied your configuration into tests/30133/main.tf
  3. Ran terraform test

Here's the output:

$ terraform test
╷
│ Warning: The "terraform test" command is experimental
│
│ We'd like to invite adventurous module authors to write integration tests for
│ their modules using this command, but all of the behaviors of this command
│ are currently experimental and may change based on feedback.
│
│ For more information on the testing experiment, including ongoing research
│ goals and avenues for feedback, see:
│     https://www.terraform.io/docs/language/modules/testing-experiment.html
╵
─── Failed: 30133.tags.fail (this test should fail) ───────────────────────────
condition failed
─── Failed: 30133.tags.scheme (should fail) ───────────────────────────────────
wrong value
    got:  true
    want: false

───────────────────────────────────────────────────────────────────────────────

As you can see, both tests failed. Can you suggest what might be missing in this reproduction?

If you can share the output from your tests which are failing like this, that would be helpful too. I took a look at the linked PR but was unable to see any test output from the CI runs. Thanks!

alisdair avatar Dec 10 '21 17:12 alisdair

@alisdair this is what im getting

git clone [email protected]:aws-ia/terraform-aws-label.git
cd terraform-aws-label
git checkout terraform-test
terraform test

╷
│ Warning: The "terraform test" command is experimental
│
│ We'd like to invite adventurous module authors to write integration tests for their modules using this command, but all of the
│ behaviors of this command are currently experimental and may change based on feedback.
│
│ For more information on the testing experiment, including ongoing research goals and avenues for feedback, see:
│     https://www.terraform.io/docs/language/modules/testing-experiment.html
╵
Success! All of the test assertions passed

drewmullen avatar Dec 10 '21 18:12 drewmullen

I see. Looking in more detail at that branch, the issue here is that you have multiple test_assertions resources with the same component argument. This argument must be a unique identifier, and it is invalid to have more than one resource using the same component.

With the following diff from your branch, terraform test fails successfully:

$ git diff
diff --git a/tests/examples_formatted_tags/test.tf b/tests/examples_formatted_tags/test.tf
index 55e9be9..6b9824a 100644
--- a/tests/examples_formatted_tags/test.tf
+++ b/tests/examples_formatted_tags/test.tf
@@ -14,7 +14,7 @@ locals {
 }

 resource "test_assertions" "matching_formatted_labels_awscc" {
-  component = "tags"
+  component = "tags_awscc"

   check "valid_tags_for_awscc" {
     description = "Validate tags formatting for AWSCC Provider usage. expected: {key=\"foo\", value=\"bar\"}"
@@ -23,7 +23,7 @@ resource "test_assertions" "matching_formatted_labels_awscc" {
 }

 resource "test_assertions" "matching_formatted_labels_aws" {
-  component = "tags"
+  component = "tags_aws"

   check "valid_tags_for_aws" {
     description = "Validate tags formatting for AWS Provider usage. expected: {foo = bar}"
@@ -31,7 +31,7 @@ resource "test_assertions" "matching_formatted_labels_aws" {
   }
 }
 resource "test_assertions" "expect_failure" {
-  component = "tags"
+  component = "tags_fail"

   check "fail" {
     description = "this test should fail"
$ terraform test
╷
│ Warning: The "terraform test" command is experimental
│
│ We'd like to invite adventurous module authors to write integration tests for
│ their modules using this command, but all of the behaviors of this command are
│ currently experimental and may change based on feedback.
│
│ For more information on the testing experiment, including ongoing research goals
│ and avenues for feedback, see:
│     https://www.terraform.io/docs/language/modules/testing-experiment.html
╵
─── Failed: examples_formatted_tags.tags_fail.fail (this test should fail) ─────────
condition failed
─── Failed: examples_formatted_tags.tags_fail.scheme (should fail) ─────────────────
wrong value
    got:  true
    want: false

────────────────────────────────────────────────────────────────────────────────────

With regards to your original concern: Terraform is creating and destroying the VPC resources, and you can verify this by reading the debug logs with TF_LOG=trace. I'm not sure why you're expecting them to take 45 minutes to destroy, as that hasn't been my experience in the past.

I don't think there's a bug in Terraform here, so I'm going to close this issue. But if I've misunderstood something please do let me know!

alisdair avatar Dec 10 '21 19:12 alisdair

hey, nice! thanks for pointing that out, @alisdair ! however i dont think this is quite closed yet.

from multiple-component perspective, perhaps its not a bug but from a user experience you should either:

  • error
  • warn the user that tests are being skipped

regarding the time outs (45m vs 3m) - please look a little closer above, theres actually 2 different repos. the one youre referring to (that i called "scenario 2") is only vpcs. but theres also this branch (which i called scenario 1) which doesnt have multiple components and is expected to take a full 45m but only takes 3m

in hindsight this should have been 2 separate issues but at the time of opening they seemed to be related to the same root cause

drewmullen avatar Dec 10 '21 19:12 drewmullen

@alisdair on top of taking around 45m, it should also fail: https://github.com/drewmullen/terraform-aws-vpc/blob/88dce3e1320cd28febeaba63d141c3d928b9c957/tests/examples_ipam_vpc/tests.tf#L19

i tried to force a failure 2 ways, with regex for /29$ (should be 28) and then also just setting false

drewmullen avatar Dec 10 '21 19:12 drewmullen

from multiple-component perspective, perhaps its not a bug but from a user experience you should either:

  • error
  • warn the user that tests are being skipped

Agreed, this is definitely a sharp edge with the current testing experiment. We're aware that it's an issue, but unfortunately it isn't one we'll be able to address until the next iteration of testing. Some details here, in case you're interested: https://github.com/hashicorp/terraform/blob/6530055d188d363bf64ff99b56bb00b350de2cfb/internal/moduletest/provider.go#L270-L277

alisdair avatar Dec 10 '21 19:12 alisdair

Looking at the other repository you linked to, the issue appears to be a configuration which cannot be applied. If you change into the test directory and apply, it fails before reaching the assertions:

aws_vpc_ipam.example: Creating...
module.no_ipam_vpc_example.aws_vpc.this[0]: Creating...
module.no_ipam_vpc_example.aws_vpc.this[0]: Creation complete after 2s [id=vpc-03c65093c91773df1]
aws_vpc_ipam.example: Creation complete after 6s [id=ipam-0de3de0f95a17e2b2]
aws_vpc_ipam_pool.ipv4_example: Creating...
aws_vpc_ipam_pool.ipv4_example: Still creating... [10s elapsed]
aws_vpc_ipam_pool.ipv4_example: Creation complete after 13s [id=ipam-pool-0674aedcb97a12f0a]
aws_vpc_ipam_pool_cidr.ipv4_example: Creating...
module.ipv4_ipam_explicit_cidr_vpc.aws_vpc.this[0]: Creating...
module.ipv4_ipam_default_netmask_vpc.aws_vpc.this[0]: Creating...
module.ipv4_ipam_explicit_netmask_vpc.aws_vpc.this[0]: Creating...
aws_vpc_ipam_pool_cidr.ipv4_example: Still creating... [10s elapsed]
aws_vpc_ipam_pool_cidr.ipv4_example: Creation complete after 14s [id=172.2.0.0/16_ipam-pool-0674aedcb97a12f0a]
╷
│ Warning: The test provider is experimental
│
│   with provider["terraform.io/builtin/test"],
│ The Terraform team is using the test provider (terraform.io/builtin/test) as
│ part of ongoing research about declarative testing of Terraform modules.
│
│ The availability and behavior of this provider is expected to change
│ significantly even in patch releases, so we recommend using this provider
│ only in test configurations and constraining your test configurations to an
│ exact Terraform version.
╵
╷
│ Error: Error creating VPC: InvalidParameterValue: The allocation size is too big for the pool.
│ 	status code: 400, request id: 1e677b02-4056-434a-a40c-e94fe80727a5
│
│   with module.ipv4_ipam_explicit_netmask_vpc.aws_vpc.this[0],
│   on ../../main.tf line 29, in resource "aws_vpc" "this":
│   29: resource "aws_vpc" "this" {
│
╵
╷
│ Error: Error creating VPC: InvalidParameterValue: The allocation size is too big for the pool.
│ 	status code: 400, request id: 773515ec-41c3-4496-9366-536e31598a8d
│
│   with module.ipv4_ipam_explicit_cidr_vpc.aws_vpc.this[0],
│   on ../../main.tf line 29, in resource "aws_vpc" "this":
│   29: resource "aws_vpc" "this" {
│
╵
╷
│ Error: Error creating VPC: InvalidParameterValue: The allocation size is too big for the pool.
│ 	status code: 400, request id: 0b74936c-9b3b-4395-a781-5070bf9f0638
│
│   with module.ipv4_ipam_default_netmask_vpc.aws_vpc.this[0],
│   on ../../main.tf line 29, in resource "aws_vpc" "this":
│   29: resource "aws_vpc" "this" {
│

This is noted subtly in the documentation (see excerpt below), and is certainly another sharp edge. I'll retitle this issue accordingly and we should aim to fix this in the next iteration of terraform test.

The test_assertions resource captures any assertion failures but does not return an error, because that can then potentially allow downstream assertions to also run and thus capture as much context as possible. However, if Terraform encounters any errors while processing the test configuration it will halt processing, which may cause some of the test assertions to be skipped.

alisdair avatar Dec 10 '21 20:12 alisdair

@alisdair thank you for pointing that out! the explicit depends_on for those modules got lost while moving around commits

I may open a feature request that terraform test drops a tfstate file upon failure. i finally got the error to fire appropriately and now i have a ton of dangling resources 🤣 thoughts?

drewmullen avatar Dec 10 '21 20:12 drewmullen

That seems like a good feature request to me, thanks in advance for filing it!

alisdair avatar Dec 10 '21 20:12 alisdair

I'm having trouble understanding from this thread, what the expected behavior should be when resource creation fails. E.g. VPC creation fails, but test assertion is not run, would still test as successful? This is what I'm seeing at least.

If any resource creation fails, for whatever reason, even though test assertions have not taken place, I would like to see that reflected in the test result, as this could be an issue with the module code. Or should we test for such issues in another way?

Dennizz avatar Mar 31 '22 12:03 Dennizz

I think this issue is now obsolete with the release of the finalised test command. If you can reproduce this with the new test configuration then please open a new issue. Thanks!

liamcervante avatar Dec 10 '24 11:12 liamcervante

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Jan 10 '25 02:01 github-actions[bot]