Skip to content

Conversation

radditude
Copy link
Member

@radditude radditude commented Feb 8, 2023

When running e2e tests against a locally built Terraform binary set with TFEXEC_E2ETEST_TERRAFORM_PATH, the runTestWithVersions helper (introduced in #354) has some surprising behavior: it throws away the list of versions that was passed in and runs the test against the local binary instead. This seems fairly undesirable and was likely not an intended effect of that prior PR, since the tests using that helper are testing version-specific behavior (and thus are prone to fail when a local bin path is set!).

This PR proposes moving that version override behavior into runTest, where it was previously, and combining runTestWithVersions and runTestVersions into one test helper that runs the given test function against only the versions passed in.

Closes hashicorp/terraform#32639

Potential future enhancement:

If it would be valuable to have the ability to run some tests against a specified list of versions as well as the local binary if it exists, probably a clearer way to do that would be to add something like a runTestWithVersionsAndLatest helper that reads from TFEXEC_E2ETEST_TERRAFORM_PATH and appends the local binary's version instead of overwriting the others.

When running e2e tests against a locally built Terraform
binary, the runTestWithVersions helper has some surprising
behavior: it throws away the selected versions and runs
the test against the local binary instead. This seems fairly
undesirable, since the tests using that helper are testing
version-specific behavior (and thus fail when a local
bin path is set).

This commit clarifies some naming and ensures that version
override behavior is confined to `runTest`, which makes no
assumptions about which versions it's using.
@radditude radditude changed the title fix issues with version-specifc tests when run against a local binary fix issues with version-specific tests when run against a local binary Feb 8, 2023
@kmoe kmoe self-assigned this Feb 9, 2023
Copy link
Member

@kmoe kmoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @radditude, this is much clearer!

I'm going to merge this now as I think the e2e failures against Terraform main/1.4 are resolved by #361.

@kmoe kmoe merged commit 40ccea4 into main Feb 9, 2023
@kmoe kmoe deleted the radditude/fix-e2e-test branch February 9, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

terraform-exec e2etest JSON plan regression (TestPlanJSON_TF014AndEarlier)
2 participants