-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Document what means that we must skip VCR for a given test #20059
Document what means that we must skip VCR for a given test #20059
Comments
Agree with all this, having dealt with some of these quirks recently. Explaining how the seed is used with random names would be a useful thing to explain - took me a little to see where that actually happens. Not sure if this would work, but one idea in addition to docs: Some public bucket of recordings would also be very useful (for cheaply and quickly running a lot of tests locally to see which ones might need to be checked in recording mode), though I can understand the security risks of doing that (e.g., if a header or token that should be scrubbed isn’t, leaking internal project names, or whatever). Could there be an option to ignore a missing call? For example, an immutable resource like a KMS keyring being created on the first pass is one reason a test couldn’t use VCR (see #20039 and the PR linked to it) Maybe instead of a comment above the line skipping the test, Side note that’s only partially related, but would be nice if detection of which tests are affected by a PR could be a little better at ignoring unrelated failures, though I guess that could also make it easier to ignore failing tests. |
Would also be nice if the "Get to know how VCR tests work" link in the comments that get posted actually pointed to some information on "how VCR tests work" 😅 |
I've added some information related to this issue here in the wiki for now, but I expect it'll be migrated to the contributions website at some point: https://github.com/hashicorp/terraform-provider-google/wiki/FAQ#what-acceptance-tests-should-be-skipped-during-vcr-testing That's a good point about the "Get to know how VCR tests work" link not being helpful - I think a page in the contribution guide about VCR would be useful even if it's something that 80% of contributors won't need to think about. |
What kind of contribution is this issue about?
Other (specify in details)
Details
There are various things we can do to make valid acceptance tests for resources & data sources that are just not compatible with the VCR system currently. In those cases it may be unclear to a contributor why a test works when run locally but is failing in VCR, and some familiarity with the VCR system is needed either on the contributor or reviewer's part to unpick what is happening.
Maybe in some cases the VCR test could be updated to accommodate these specific use cases, but even if that's possible we are unlikely to make large edits to the VCR system soon. For now we should document what things are valid to include in a VCR test but mean that test should 100% be skipped in VCR.
References
The text was updated successfully, but these errors were encountered: