Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of configs: A test for the regression reported in #31081 into v1.2 #31098

Conversation

teamterraform
Copy link
Contributor

Backport

This PR is auto-generated from #31097 to be assessed for backporting due to the inclusion of the label 1.2-backport.

The below text is copied from the body of the original PR.


#31091 addressed a regression in the logic for catching when the newer module meta-arguments are used in conjunction with the legacy practice of including provider configurations inside modules, where the check started incorrectly catching situations where the legacy nested provider configuration was in the same module as the child call using one of those meta-arguments.

This is a regression test to catch if a similar bug arises in the future.

Since it's testing validation rules that apply to an entire configuration tree, it ended up in a rather idiosyncratic location under the configload package, rather than directly in configs. The configs package only knows how to load one module at a time, so it's harder to write a test like this in that context. Due to it being further removed from the code it is testing, I included a test for the correct error too in order to increase the chance that we'll learn if future changes in the "configs" package invalidate this regression test.

I've verified that this new test fails without the change made in the earlier commit.

@teamterraform teamterraform force-pushed the backport/b-module-is-incompatible-with-count-test/gladly-becoming-skink branch from a6a1e94 to c1c4d05 Compare May 20, 2022 18:24
5417975 addressed a regression in the
logic for catching when the newer module meta-arguments are used in
conjunction with the legacy practice of including provider configurations
inside modules, where the check started incorrectly catching situations
where the legacy nested provider configuration was in the same module
as the child call using one of those meta-arguments.

This is a regression test to catch if a similar bug arises in the future.

Since it's testing validation rules that apply to an entire configuration
tree, it ended up in a rather idiosyncratic location under the "configload"
package, rather than directly in "configs". The "configs" package only
knows how to load one module at a time, so it's harder to write a test
like this in that context. Due to it being further removed from the code
it is testing, I included a test for the correct error too in order to
increase the chance that we'll learn if future changes in the "configs"
package invalidate this regression test.

I've verified that this new test fails without the change made in the
earlier commit.
@apparentlymart apparentlymart force-pushed the backport/b-module-is-incompatible-with-count-test/gladly-becoming-skink branch from c1c4d05 to 371472c Compare May 20, 2022 18:26
@apparentlymart apparentlymart merged commit 271023b into v1.2 May 20, 2022
@apparentlymart apparentlymart deleted the backport/b-module-is-incompatible-with-count-test/gladly-becoming-skink branch May 20, 2022 18:32
@github-actions
Copy link

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants