-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Added module mock example #4199
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4199 +/- ##
=======================================
Coverage 60.27% 60.27%
=======================================
Files 198 198
Lines 6779 6779
Branches 6 6
=======================================
Hits 4086 4086
Misses 2690 2690
Partials 3 3 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet, love it! Just some minor fixes and we're good to merge it 🙂
@@ -0,0 +1,30 @@ | |||
// Copyright 2004-present Facebook. All Rights Reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the copyright header is slightly different, can you fix it? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What copyright header should I use? I got this header from the other example projects:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't realize that. Let's keep it this way then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
|
||
const mockedModule = jest.genMockFromModule('../fruit'); | ||
|
||
const module = Object.assign(mockedModule, originalModule); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this whitespace, pls. Also we're mutating mockedModule here :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, fixed it!
// Module the default export and named export 'apple' | ||
module.default = jest.fn(() => 'mocked fruit'); | ||
module.apple = 'mocked apple'; | ||
return module; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can make it simpler. Usually what you want to do is to have original module and replace just single APIs (like default and apple here). How about:
const originalModule = require.requireActual('../fruit');
return Object.assign({}, orignalModule, {
default: jest.fn(() => 'mocked fruit'),
apple: 'mocked apple'
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why atm (no time dig into it today), but @aaronabramov may help with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks! Hopefully he knows:) If I find some time today I will also investigate further.
|
||
describe('test', () => { | ||
it('calls', () => { | ||
expect(DefaultExport()).toBe('mocked fruit'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function call is easy to miss. How about extract it to a constant?
const defaultExportResult = DefaultExport(); // now it's easier to see that the function was called
expect(defaultExportResult).toBe('mocked fruit');
expect(DefaultExport).toHaveBeenCalled();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, fixed it!
Thanks for the review @thymikee! I left some comments in response to your review. |
I was thinking that it may also be beneficial to add an example of using |
Simplified some of the test code and improved descriptions per test.
I added a commit that illustrates how to define a custom mock per test. Hopefully it is not a problem that I extended the PR:) |
You're more than welcome to extend it! Thank you for doing this. |
Thanks for adding an example! This is great. |
Thanks for your help and your awesome work maintaining this library guys!!! I really appreciate it |
* Added module mock example * Simplified the code in the partial mock example. * Added file illustrating how to define mock behaviour per test Simplified some of the test code and improved descriptions per test.
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
I thought it would be useful to add an example outlining how to do module mocking both for a full mock and partial mock.
Partial mocks is something that I have found useful when writing tests but it is not well documented unfortunately. Hopefully this can help other people use this feature as well.
Let me know if you think this is a worthy addition.