-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
ID-only tests for all resources #6262
Conversation
LGTM! |
Thanks! Fixing some test failures then will merge. |
What are we to do about write-only attributes on resources? There are plenty of these sprinkled across various providers. I don't see any way this could ever be automatically supported... user intervention will always be required in order to support import for these. |
@apparentlymart I'll add a flag to disable the test for those. |
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. |
This modifies the acceptance test framework to do what is called an ID-only refresh test on resources. This test modifies the state to only have the ID of the resource, performs a refresh, and verifies it results in equivalent attribute state.
This is going to result in a lot of test failures. Therefore, I've guarded it with an env var for now while I run it manually and fix things up over time. We can open the floodgates hopefully shortly.
No tests have to change to enable this, it'll automatically be available for all existing acceptance tests. There isn't a flag to disable yet per-resource but I have a feeling that'll be needed. I'll do that when it comes since for now its feature flagged off anyways.
Why? This verifies that
Refresh
on a resource is complete. This is a pre-req to us supporting importing at some point in the future.Example of a failure: