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

Global mock support #143

Merged
merged 3 commits into from
Dec 11, 2017
Merged

Global mock support #143

merged 3 commits into from
Dec 11, 2017

Conversation

teamon
Copy link
Member

@teamon teamon commented Dec 7, 2017

Continuation of #136, also related to #140

I've moved setting up global mock into separate function to be explicit about it. It will also fail fast when used inappropriately with async test cases.

@zorbash, @amatalai please take a look.

@gottfrois, @pawelduda, @Daniel-Xu please try using tesla from global-mock branch and let us know if there are any issues.

{:tesla, github: "teamon/tesla", branch: "global-mock"}

@codecov
Copy link

codecov bot commented Dec 7, 2017

Codecov Report

Merging #143 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #143      +/-   ##
==========================================
+ Coverage    98.3%   98.32%   +0.02%     
==========================================
  Files          21       21              
  Lines         412      418       +6     
==========================================
+ Hits          405      411       +6     
  Misses          7        7
Impacted Files Coverage Δ
lib/tesla/mock.ex 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45de286...423b2a5. Read the comment docs.

@doc """
Setup global mocks.

**WARNING**: This mock will be available to ALL process.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: processes

Copy link
Member Author

Choose a reason for hiding this comment

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

👍


While this style is recommended, there is one drawback:
if Tesla client is called from different process then the
test case it will not get proper mock.
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest a minor rewording to:

If Tesla client is called from a different process then the test case will not be mocked.

Copy link
Member Author

Choose a reason for hiding this comment

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

The test case process itself will be mocked. Either way, both messages would be wrong ;)

It should be something like this "if Tesla client is called from different process it will not use the setup mock"

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good :-)

@teamon teamon merged commit b65ebb8 into master Dec 11, 2017
@teamon teamon deleted the global-mock branch December 11, 2017 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants