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

Fix for mocks not working with module name mapper #7787

Merged

Conversation

grosto
Copy link
Contributor

@grosto grosto commented Feb 3, 2019

Summary

Fixes #4262. Closes #7668

There were two bugs with how module name mapper and mocks interact. First, _generateMock was calling _resolveModule method which does not handle mapped modules, and second requireMock was assuming that mapped module was a manual mock.

Test plan

I have added one additional unit test and copied integration test from #7668

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

This is awesome, thank you! Could you update the changelog as well? 🙂

e2e/module-name-mapper-mock/package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@jeysal jeysal left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!
I find the e2e test file names (Track & TrackExpected) a bit confusing, not sure what they are supposed to mean.
MappedMockOnly & MappedMockAndImport?

@codecov-io
Copy link

Codecov Report

Merging #7787 into master will decrease coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7787      +/-   ##
==========================================
- Coverage   68.34%   68.27%   -0.07%     
==========================================
  Files         252      252              
  Lines        9707     9684      -23     
  Branches        5        6       +1     
==========================================
- Hits         6634     6612      -22     
+ Misses       3071     3070       -1     
  Partials        2        2
Impacted Files Coverage Δ
packages/jest-resolve/src/index.js 57.5% <100%> (ø) ⬆️
packages/jest-runtime/src/index.js 74.56% <100%> (+1.13%) ⬆️
packages/jest-config/src/normalize.js 82.3% <0%> (-6.25%) ⬇️
packages/jest-cli/src/testResultHelpers.js 72.72% <0%> (-1.56%) ⬇️
packages/jest-cli/src/runJest.js 75% <0%> (-0.95%) ⬇️
packages/jest-runtime/src/ScriptTransformer.js 88.67% <0%> (-0.08%) ⬇️
packages/jest-resolve/src/defaultResolver.js 77.41% <0%> (ø) ⬆️
...us/src/legacy-code-todo-rewrite/jestAdapterInit.js 0% <0%> (ø) ⬆️
packages/jest-jasmine2/src/jasmineAsyncInstall.js 0% <0%> (ø) ⬆️
... and 6 more

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 a61ac1d...18503b9. Read the comment docs.

@SimenB
Copy link
Member

SimenB commented Feb 3, 2019

The names come from the reproduction case - I don't think they matter or takes away from the clarity of the tests


test('relative import', () => {
const track = new Track();
expect(track.someRandomFunction).not.toBeCalled();

Choose a reason for hiding this comment

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

@grosto, I'm currently experiencing the problem this PR was supposed to solve; my aliased components aren't being mocked.

Reviewing this expected solution, I suspect it's broken again and was not caught because of this test. I believe this expectation here would always be true event if the mock is not successful. In order for this test to be valuable, someRandomFunction should be called if there is no mock, but I don't see any case where it would be called.

Copy link
Member

Choose a reason for hiding this comment

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

@dbroytman could you open up a new issue with a reproducion? Or even better, a PR fixing it 🙂

@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

moduleNameMapper - mocks do not work for imported modules of mapped modules using "moduleNameMapper"
7 participants