search

parroty / exvcr

HTTP request/response recording library for elixir, inspired by VCR.
MIT License
720 stars 131 forks source link

Use global mock in adapters #158

Closed surik closed 3 years ago

surik commented 3 years ago

Instead of call :meck.expect whenever we use cassette, we can implement a global mocking on application startup and global storage for the recorder. Whenever we need to use a cassette, we can pass the recorder though CurrentRecorder GenServer.

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.1%) to 93.282% when pulling 0d4439dd89afd5aa6ca32c2eca340a7ab6a6cce4 on surik:global-mock into d0ffc47a56ff3922c53616566025ec405f407476 on parroty:master.

parroty commented 3 years ago

Thanks for addressing this item (I could confirm the test completes faster with this change). Do you have any insights on whether if it has negative impact (compatibility, etc.), which requires some note in the documentation?

surik commented 3 years ago

Hi @parroty, thank you for checking it.

I'm currently using this branch with an application that has quite extensive use of cassettes. Switching between upstream and my branch is only about a dependency reference change in mix.exs, so I would say it is 100% compatible. The only impact that I see here is a global mock. We pass HTTP client calls to :meck.passthough/1 outside of use_cassette. That technically makes everything being mocked during the tests.

Another potential thing that may happen if someone includes exvcr to production build, but since README.md clearly says to use it only in the test environment, I wouldn't consider this as a serious problem.

Shall I add a note to README.md about :meck.passthough/1 outside of use_cassette?

parroty commented 3 years ago

Thanks for the detailed description.

Shall I add a note to README.md about :meck.passthough/1 outside of use_cassette?

Can I request a note to the readme? (maybe just a simple note would be great). Then I'll be merging and push to hex.

parroty commented 3 years ago

Thanks!

parroty commented 3 years ago

It seems some tests are failing. I'll be looking to it too, but do you have insights on whether if it can have optional flag to partial disable the global mock?