Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

New record mode "re_record" #746

Closed
codener opened this issue May 8, 2019 · 6 comments
Closed

New record mode "re_record" #746

codener opened this issue May 8, 2019 · 6 comments

Comments

@codener
Copy link

codener commented May 8, 2019

At first, thanks for the VCR gem! We're using it quite frequently and it's doing a great job.

Is your feature request related to a problem? Please describe.
We're using VCR to stub slow/unstable HTTP communication in tests. Sometimes changed code provokes differend requests, thusly requiring a re-recording of cassettes.

The amount of VCR cassettes (>200) plus the separation of test from cassette location makes re-recording a little tedious. Say I need to re-record all search tests, I need to look and find where the concerned cassettes are stored, rm them manually, and then run all tests that lost their cassette (only those, at best).

Describe the solution you'd like
If VCR had a (default cassette option) record_mode: :re_record, I would turn it on and just run the tests that I want to drop and re-record their cassettes. Afterwards I would switch back to :once mode. The idea is that the developer can re-record just by running tests.

(Probably I'd then funnel the activation of the re_record mode through an ENV var that's read in the VCR config, but that's out of scope. Example: VCR=re_record <run tests>)

Describe alternatives you've considered
While the automatic re-recording feature sounds handy, we cannot afford to have tests "randomly" drop their cassettes. It slows down the test suite run at unexpected times, and if requests fail, the developer faces broken tests he didn't break.

The record_mode: :all is neither a solution. While it updates previous requests and adds new requests, it does not drop unused requests. Together with the allow_unused_http_interactions: false option, this is not a feasible solution.

What do you think?

@krainboltgreene
Copy link
Contributor

Hello @codener (and @mhuntglickman)!

Sorry for not responding in a long time, but I'm not sure VCR needs this option. The fundamental issue you're facing is not essentially a VCR issue in my opinion.

VCR is a method for quickly mocking a complex function call: The HTTP request. Ideally you should be mocking one layer above the HTTP request itself, because that part may change. Maybe you use a new service provider, maybe you stop using HTTP in favor of GraphQL, or maybe you use your own interface.

So when I see "We have 200 HTTP requests stored to disk that we replay and sometimes they're wrong because the HTTP request itself is different (instead of broken)" I feel the need to reply: "Why are your tests care about how something is done instead of what the layer wrapping that action returns?"

I hope this makes sense. @olleolleolle, care to chime in?

@FeminismIsAwesome
Copy link

FeminismIsAwesome commented Sep 15, 2019

I think I get what you’re saying, but let’s talk about an example where I might enjoy this flag. Let’s say I’ve got a suite of tests that make sure my integration with Stripe works end to end, with VCR being part of that process. While I could have mocks end to end and have a go between layer, I like the knowledge that if I bump the Stripe api I can run my suite fully and know that things are ok via blowing away my vcrs. I guess you’re suggesting that the layer between is the only thing VCRd so that’d be the only cassettes to delete (when updating the api), rendering such a feature moot? (Or that I turn on :all temporarily, and don’t care if the vcr has extra requests.)

@krainboltgreene
Copy link
Contributor

"How do I test a version bump in an external HTTP service I use" is a pretty compelling case, but as you point out setting record mode to "all" temporarily gets you the same result, for example mode: ENV["RERECORD"] ? :all : :once.

@codener
Copy link
Author

codener commented Sep 16, 2019

Dear @krainboltgreene,

thanks for your reply. Unfortunately, I'm not fully getting your point. VCR presents itself (the Github description) as a service to:

Record your test suite's HTTP interactions and replay them during future test runs for fast, deterministic, accurate tests.

To me, this is not the same as mocking a function call (where I'd say "skip that, pretend this happened"). It is short-circuiting a real part of an application that is not under its own control: external HTTP requests. VCR does not construct virtual responses, but plays back real responses for "fast, deterministic, accurate tests". We're using VCR for this kind of integrative tests, and it's doing great.

The reasons for updating stored requests are various: sometime I just want to update my recordings to make sure an external service still behaves as the tests expect. Sometime a change to the application slightly modifies how it makes external requests (e.g. a parameter changed), rendering the recordings useless. The affected cassettes need to be re-recorded.

As stated in the initial post, the :all mode is getting close, but it does not work together with allow_unused_http_interactions: false. By just adding new requests, VCR keeps the old interactions around and will complain (as configured). In a way, it is blocking itself here.

Considering this, I believe an option to re-record cassettes whilst dropping previous requests complements VCRs feature set. The approach of @mhuntglickman using a config option instead of a new recording mode looks fine to me.

@codener
Copy link
Author

codener commented Oct 18, 2019

Dear @krainboltgreene, @olleolleolle,

please let me know if I can be of any help here.


/edit: There was a comment below that krainboltgreene is referring to, and that has been deleted.

@krainboltgreene
Copy link
Contributor

I do not care how frustrated you are. I will do this on my own time and I am not beholden to your timeline. Please don't comment like this ever again on open source.

@vcr vcr locked and limited conversation to collaborators Feb 28, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants