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

Use global mock in adapters #158

Merged
merged 6 commits into from
Oct 5, 2020
Merged

Use global mock in adapters #158

merged 6 commits into from
Oct 5, 2020

Conversation

surik
Copy link
Contributor

@surik surik commented Sep 24, 2020

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.

Instead of call `:mock.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.
@surik surik mentioned this pull request Sep 24, 2020
Co-authored-by: Kanmaniselvan Murugesan <[email protected]>
@coveralls
Copy link

coveralls commented Sep 28, 2020

Coverage Status

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

@parroty
Copy link
Owner

parroty commented Oct 4, 2020

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
Copy link
Contributor Author

surik commented Oct 5, 2020

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
Copy link
Owner

parroty commented Oct 5, 2020

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 parroty merged commit 9f7f520 into parroty:master Oct 5, 2020
@parroty
Copy link
Owner

parroty commented Oct 5, 2020

Thanks!

@parroty
Copy link
Owner

parroty commented Oct 7, 2020

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?

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.

4 participants