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

Document what means that we must skip VCR for a given test #20059

Open
SarahFrench opened this issue Oct 28, 2024 · 3 comments · May be fixed by GoogleCloudPlatform/magic-modules#12407
Open
Assignees
Milestone

Comments

@SarahFrench
Copy link
Member

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

@SarahFrench SarahFrench added this to the Goals milestone Oct 28, 2024
@SarahFrench SarahFrench self-assigned this Oct 28, 2024
@wyardley
Copy link

wyardley commented Oct 30, 2024

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: .gitkeep an empty directory for VCR recordings, and .gitignore its contents, and default the vcr dir to that in the Makefile - usually these files are not too large, I think? At that point, it would be easier for contributors to switch modes, or to have a make target that runs in recording mode, then, upon success, again in replaying… getting it right also could save time and money.

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, SkipVCR() could take the reason for the skip as a parameter?

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.

@wyardley
Copy link

wyardley commented Nov 1, 2024

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" 😅

@SarahFrench
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants