helper/resource: Refresh during plan rather than separately and remove state shimming when possible #223
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e state shimming when possible Reference: hashicorp/terraform-plugin-sdk#515 Reference: #194 Reference: #222 Reference: https://developer.hashicorp.com/terraform/cli/commands/refresh This change replaces the usage of explicit refresh commands by instead calling the next command with the relevant refresh flags. Separating the refresh from plan was previously done with the idea that the testing logic may introduce further assertion hooks based on that explicit refresh. Over time though, the Terraform recommendations for refreshing and planning have solidified around always recommending to use a plan with behavior changing flags as necessary. Even the Terraform CLI implementation of the refresh command itself has been shifted to call a plan with special flags. This should reduce testing times decently as it means that the testing logic will skip spinning up and tearing down any relevant providers two times per `TestStep` and whatever time would be associated with Terraform CLI loading for those commands. Comparing the testing performed across this Go module: Prior: `TF_ACC=1 go test -count=1 ./... 111.07s user 59.37s system 233% cpu 1:12.99 total` Now: `TF_ACC=1 go test -count=1 ./... 90.81s user 48.25s system 229% cpu 1:00.54 total` The separated refresh also prevented `PreApply` plan checks from catching certain situations because the refreshed state could cause the plan to be missing changes.
|
If they have time, I'm soliciting a |
|
If any bugs are caused by this change, please open a bug report and we will take a look. |
6 tasks
bflad
added a commit
that referenced
this pull request
Dec 26, 2023
Reference: #223 Reference: #237 Reference: #256 Post-release update to the v1.6.0 CHANGELOG to capture now-known situations that would affect existing provider testing. This update ensures developers upgrading to v1.6.0 will receive the latest guidance and the guidance will remain for this version after future releases. If approved and merged, the v1.6.0 GitHub Release description will also be updated with the same additional content.
bflad
added a commit
that referenced
this pull request
Dec 28, 2023
Reference: #223 Reference: #237 Reference: #256 Post-release update to the v1.6.0 CHANGELOG to capture now-known situations that would affect existing provider testing. This update ensures developers upgrading to v1.6.0 will receive the latest guidance and the guidance will remain for this version after future releases. If approved and merged, the v1.6.0 GitHub Release description will also be updated with the same additional content.
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference: hashicorp/terraform-plugin-sdk#515
Reference: #194
Reference: #222
Reference: https://developer.hashicorp.com/terraform/cli/commands/refresh
This change replaces the usage of explicit refresh commands by instead calling the next command with the relevant refresh flags. Separating the refresh from plan was previously done with the idea that the testing logic may introduce further assertion hooks based on that explicit refresh. Over time though, the Terraform recommendations for refreshing and planning have solidified around always recommending to use a plan with behavior changing flags as necessary. Even the Terraform CLI implementation of the refresh command itself has been shifted to call a plan with special flags.
This should reduce testing times decently as it means that the testing logic will skip spinning up and tearing down any relevant providers two times per
TestStepand whatever time would be associated with Terraform CLI loading for those commands. Comparing the testing performed across this Go module:Prior:
TF_ACC=1 go test -count=1 ./... 111.07s user 59.37s system 233% cpu 1:12.99 totalNow:
TF_ACC=1 go test -count=1 ./... 90.81s user 48.25s system 229% cpu 1:00.54 totalThe separated refresh also prevented
PreApplyplan checks from catching certain situations because the refreshed state could cause the plan to be missing changes.