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

Switch VCR record mode to :none #4822

Merged
merged 2 commits into from Apr 10, 2024
Merged

Switch VCR record mode to :none #4822

merged 2 commits into from Apr 10, 2024

Conversation

Nishnha
Copy link
Member

@Nishnha Nishnha commented Mar 9, 2022

This avoids recording cassettes when we run our tests in CI in case the PR author didn't already record them. This also helps us catch instances where the cassette itself might be poorly recorded; a test with the :vcr annotation can have its cassette change in subsequent runs if the test doesn't contain a description

This may reveal that we have other poorly recorded tests in our codebase

@Nishnha Nishnha requested a review from a team as a code owner March 9, 2022 21:44
@Nishnha
Copy link
Member Author

Nishnha commented Mar 15, 2022

Running VCR=all rspec spec in common seems to make more recordings fail.
We might want to set up a cron job to try running this once a week so we can keep our tests up-to-date after this merges

@mctofu
Copy link
Contributor

mctofu commented Apr 6, 2022

I cleaned up a few more unpinned cassettes in #4953

@Nishnha Nishnha marked this pull request as draft August 1, 2022 21:38
Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable to me. Curious what others think though?

We might want to set up a cron job to try running this once a week so we can keep our tests up-to-date after this merges

IMO we should definitely do that.

@deivid-rodriguez
Copy link
Contributor

Maybe :once is still useful locally to get cassettes automatically generated when writing new specs. Then CI can have a more strict mode.

Periodically regenerating cassettes also sounds good.

Finally, we could also add a CI check that rejects unused cassettes. For example, when you write a spec, generate cassette, then rename the spec, a differently named cassette is generated, and you commit everything, including an empty cassettes. Here's a blog post on that: https://www.jdeen.com/blog/detecting-unused-vcr-cassettes.

@Nishnha
Copy link
Member Author

Nishnha commented May 12, 2023

Maybe :once is still useful locally to get cassettes automatically generated when writing new specs.

The :once VCR mode can still be set locally with an environment variable!

When running a spec you can do something like VCR=once bundle exec rspec spec/... and it will capture a recording.

Periodically regenerating cassettes also sounds good.

Finally, we could also add a CI check that rejects unused cassettes. For example, when you write a spec, generate cassette, then rename the spec, a differently named cassette is generated, and you commit everything, including an empty cassettes. Here's a blog post on that: https://www.jdeen.com/blog/detecting-unused-vcr-cassettes.

I like these ideas, but I think we should do them as a follow up PR?

@deivid-rodriguez
Copy link
Contributor

Yep, that sounds great!

jeffwidman pushed a commit that referenced this pull request Jul 15, 2023
Stubs out a registry request that's not captured by a VCR. The test fails when the VCR mode is set to `:none` as part of #4822

See https://github.com/dependabot/dependabot-core/actions/runs/4957979462/jobs/8870303817?pr=4822#step:5:223 for the error
@Nishnha Nishnha marked this pull request as ready for review August 1, 2023 17:44
@Nishnha Nishnha force-pushed the switch-to-vcr-none branch 2 times, most recently from c39d7ab to 9e9cbc1 Compare August 2, 2023 16:18
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
Stubs out a registry request that's not captured by a VCR. The test fails when the VCR mode is set to `:none` as part of dependabot#4822

See https://github.com/dependabot/dependabot-core/actions/runs/4957979462/jobs/8870303817?pr=4822#step:5:223 for the error
This avoids recording cassettes when we run our tests in CI in case the PR author didn't already record them. This also helps us catch instances where the cassette itself might be poorly recorded; a test with the :vcr annotation can have its cassette change in subsequent runs if the test doesn't contain a description

This may reveal that we have other poorly recorded tests in our codebase
@Nishnha Nishnha merged commit c7bc960 into main Apr 10, 2024
53 of 54 checks passed
@Nishnha Nishnha deleted the switch-to-vcr-none branch April 10, 2024 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants