importstate: compare plan to previous state #467
Conversation
bbasata
force-pushed
the
import-compare-plan-to-previous-state
branch
from
1298559 to
4cf6a87
Compare
There was a problem hiding this comment.
nit: I'd love to see a negative test here as well, like one where the follow-up plan has changes.
Might need to do some awkwardness to create that "inconsistency" without having a remote system, especially since the end of test step 1 will refresh. In the past I've used the provider factories on each test step to inject the differences, which is a potential option: https://github.com/hashicorp/terraform-provider-corner/blob/52e22f4ee878680727ff68a2e44967b496242a76/internal/protocolv6provider/writeonly_datacheck_test.go#L182-L204
| // In prerelease, we are choosing that ImportBlockWithID will not perform an apply, so it will not produce a new state, | ||
| // and there is no new state for ImportStateVerify to do anything meaningful with. |
There was a problem hiding this comment.
nit: Are we planning on revisiting this decision later? If so, maybe a TODO prefix here (I know I'll forget what the original intention was of this comment 😆)
Same question for the other test below
| ImportStateVerify: true, | ||
| ResourceName: "examplecloud_container.test", | ||
| ImportState: true, | ||
| ImportStateKind: r.ImportBlockWithID, |
There was a problem hiding this comment.
nit: Is there a reason we don't want to use ImportStateVerify here?
| if attr != vs { | ||
| return fmt.Errorf("importing resource %s: expected %s in known state to be %q, got %q", rc.Address, k, oldResource.Primary.Attributes[k], vs) | ||
| } |
There was a problem hiding this comment.
Is this meant to work for slices/maps? (collections, nested types, etc.)
| } | ||
| } | ||
|
|
||
| for _, rc := range plan.ResourceChanges { |
There was a problem hiding this comment.
One of the things we will run into here is actually the exact same problem as ImportStateVerify, which is that because we are comparing two raw data objects from Terraform (state or plan, doesn't matter tbh), if we compare them without the schema itself, we can't respect set's unordered nature.
The only real solution I can see to that problem is to build the logic to grab a provider/resource schema from Terraform, then use it to compare two data objects.
Bonus points if the logic is generic enough that we can pass any JSON objects into it. (we could re-use it for ImportStateVerify with two state objects vs. one plan and one state object)
| "github.com/hashicorp/terraform-plugin-testing/terraform" | ||
| ) | ||
|
|
||
| func Test_VerifyImportPlan(t *testing.T) { |
There was a problem hiding this comment.
One nice thing for this test would be to expand it for the other types of data outside of primitives (lists, sets, maps, single nested object/block, list nested objects/blocks, set nested objects/blocks, map nested attribute)
Co-authored-by: Austin Valle <austinvalle@gmail.com>
Co-authored-by: Austin Valle <austinvalle@gmail.com>
…are-plan-to-previous-state
|
Closing in favor of #476. |
To be merged after #465
This change introduces the
TestStep.ImportPlanVerifyoption. With this option, a plannable import test compares a generated plan to the known state of a previous test step.This unlocks verification for plannable import without invoking the apply command.
There is no attempt in this PR to mirror these existing
TestStep.ImportStateVerifyoptions:ImportStateVerifyIdentifierAttributeImportStateVerifyIgnoreIf there's value, we can add these in a follow-up PR.