Idea: `dontMock` has option to add recursively #204

Closed
dahjelle opened this Issue Dec 24, 2014 · 24 comments

Projects

None yet
@dahjelle

I was trying to call dontMock on the joi library (a schema validation library), and I had to add a lot of dontMock commands to get it to work—basically one for every file required in the library, recursively. While this makes sense in many situations, it would sure be nice to be able to tell Jest to dontMock a file and and files it requires.

One option could be something like jest.dontMock( moduleName, [recursive] ), where recursive is a boolean that defaults to false, matching the current behavior. If it is set to true, it would apply dontMock recursively.

Another option might be to base it off of what is being required: it seems to me that calling dontMock on a module in node_modules would usually want a recursive dontMock, whereas most other cases would want the non-recursive variety. Perhaps that API is too confusing.

Thoughts?

@jeffmo
Member
jeffmo commented Jan 6, 2015

Yea this has come up a few times before. It's a bit of a tricky subject which mostly boils down to the fact that modules and their closure state are shared between other modules. So, while it may be advantageous to deep-mock one module, that may mean you are unintentionally mocking a deep-recursive dependency module that a 3rd module might expect not to be mocked (or maybe you've even explicitly .mock()'d it).

The above situation, of course, assumes that mocking is a global concept -- as it is today in Jest. But we could also consider the idea that it's not, and that deep mock() and deep dontMock()s are states that only persist while executing that particular module, but not others. The problem here again comes back to the fact that modules persist state, and so it can quickly become confusing when some module state generated by A (which, let's say, was deep mock()ed) isn't being reflected deep down in B's dependency graph somewhere which was, say, deep dontMock()ed.

Ultimately it's a trade-off: Do we deal with a pretty annoying number of explicit dontMock()s? Or do we deal with a pretty annoying, edge-casey situation where modules get implicitly mocked for (probably) unexpected reasons? We decided to go with the first of the two evils because it's at least explicit and the cost of writing all those dontMock()s is one you only pay "once" and up-front (rather than later, when you might not remember all the intricacies of the module under test -- which includes all of their recursive dependencies, etc).

I'd really love to find a good solution here, but between the two options we haven't thought of a good one yet.

@dahjelle
dahjelle commented Jan 6, 2015

Yes, I can see that. I definitely think being explicit about exactly what one is doing with dontMock is a good thing—but I definitely wish there was a better solution.

Feel free to close this ticket if you'd like; I'm going to ramble on a bit more below with some related ideas I've had, if you don't mind.

Better Docs and Examples for Mocking

I've been testing with jest for a while now, but only today figured out how to use manual mocks in the __mocks__ directory. I had a couple hang-ups:

  • For some reason, it wasn't clear that I didn't have to write a mock completely from scratch, but could use dontMock and require.requireActual to build a mock based off of the existing module. I know the docs say it, but I guess it just took a while to sink in. Perhaps an example like this mock for mailcomposer would be useful in the docs? (Presuming this is reasonable jest style.)
jest.dontMock('../../node_modules/mailcomposer/node_modules/dkim-signer');
jest.dontMock('../../node_modules/mailcomposer/node_modules/follow-redirects');
jest.dontMock('../../node_modules/mailcomposer/node_modules/he');
jest.dontMock('../../node_modules/mailcomposer/node_modules/mime/mime.js');
jest.dontMock('../../node_modules/mailcomposer/node_modules/mimelib');
jest.dontMock('fs');
jest.dontMock('path');

module.exports = require.requireActual('mailcomposer');

EDIT: And…I should have worked more before posting. That mock for mailcomposer probably isn't what one wants…but I'll leave it there for now.

  • Similarly, the docs say "Manual mocks are defined by writing a module in a mocks/ subdirectory immediately adjacent to the module." What wasn't clear to me is what this meant for modules in the node_modules directory. When I got around to testing, it appears that "immediately adjacent to the module under test" is actually what is going on—which is great.
  • I know there is a config.unmockedModulePathPatterns configuration parameter, but I haven't figured out how to use it.

If I'm not too far off base, I may be able to make a pull request for some of these documentation items. Let me know!

Generation of mocks

Being new to jest, I've no idea if this is practicable in enough situations to make it worthwhile, but I wonder if jest (or a related tool) couldn't generate a mock of a module and stick it in the __mocks__ directory—perhaps something like the sample code I have above.

Actually, as I think about it, perhaps the generated mock should just do a regular require, and have all the dontMocks commented out. The benefit I see is that it gives a user a template for what might have to be mocked or not. They can requireActual and comment/uncomment dontMocks until their tests are working correctly.

User-contributed mocks

Again, just brainstorming—but perhaps some sort of user-contributed mocks of various modules would make jest more approachable for beginners? Certainly, in my case, the modules I've had the most trouble with were third-party modules that required mocking, not ones that I wrote myself.

Thanks for listening! :-D

@dahjelle
dahjelle commented Jan 6, 2015

I forgot to say…I've really enjoyed using jest to test my code, even with the work to get mocks to work correctly. Thanks for the great work!!!

@jacobstr

A glob-styled syntax for dontMock'ing against a folder hierarchy would accomplish most of what I want here without leading to unexpected dontMockin'ing of dependencies.

@rattrayalex
Contributor

+1
Globs would make sense, though anything require'd from inside node_modules should probably be dontMockd by default, no?

(I am brand new to Jest, so if that's way off the mark don't hesitate to say so).

@rattrayalex
Contributor

Perhaps this isn't the place to ask, but why would you want to mock libraries in node_modules?

@jacobstr

@rattrayalex If it's something like lodash where you've got a library of sugar methods that could very well be built-ins - mocking is often unnecessary.

For something like react-router - you might be testing a custom component that happens to utilize a Link. The rendering / behaviour of the link isn't particularly important to your particular test case - e.g. asserting that "should update the shopping cart total when an item is added" doesn't care that back to shopping points to the correct location. If you use the real react-router you'll have to do extra work to set up the context required by react-router.

So you can't generally assume node_modules should / shouldn't be mocked by default. unmockedModulePathPatterns is a convenient solution for things like lodash.

@rattrayalex
Contributor

Thanks @jacobstr, that makes a lot of sense!

Do you think it might be worthwhile to mention unmokedModulePathPatterns in one of the tutorials? I had about 30 lines of jest.dontMock('../../node_modules...etc...') before cleaning it up to 5 in package.json.

To be honest, though, I don't think Jest is right for me, as a total newcomer to testing. I don't really understand the utility of mocking because I haven't had to do it myself yet. If you think that's an accurate assessment (maybe not?) it may be worth mentioning in the docs. Or just saying more explicitly who Jest is for. I wouldn't say this is a big issue.

@vipin-inbetween

Difference between dontMock and Mock in jest?

@volkanunsal

I am seeing the polar opposite in 0.5.2. The requires of an unmocked module are not mocked at all. I'm using a manual mock, though... as in this:

// __mocks__/NewProjectPage.js
global.$ = require.requireActual('jquery');
module.exports = require.requireActual('pages/NewProjectPage');
@cpojer
Member
cpojer commented Mar 27, 2016

Jest 0.9 should work much better in all of these cases. node_modules are now by default deep-unmocked as well, as that was a frequent source of issues with npm3. We have also updated the documentation to be more clear and we are exploring more ways of improving (un)mocking. I think this task is resolved but I'm happy to reopen if there is anything left to do here for now.

@cpojer cpojer closed this Mar 27, 2016
@ttrmw
ttrmw commented Apr 13, 2016

Is there a difference in behaviours between unmockedModulePathPatterns, unmock/dontMock and disableAutomock/autoMockOff when it comes to mocking a module's dependencies?

We observed that in a test that uses disableAutomock, a dependency's dependency (specifically enzyme's depdency, cheerio) got mocked and appeared to remain mocked for other tests.

Adding cheerio to unmockedModulePathPatterns seemed to solve this, but with enzyme already listed in this config item this feels like it ought to be unnecessary (and without the disableAutomock call, appears to be).

Symptomatically we saw the effect mentioned in this issue:

airbnb/enzyme#309

which the original reporter mentions occurs with a Karma set up, so our problem may not actually relate to this issue. However, since it seems we were able to fix it with a specific configuration of mock disablements, which has led me to believe that in our case it could be a problem with Jest's mocking (or unmocking) behaviour.

@cpojer
Member
cpojer commented Apr 13, 2016

Hey! unmockedModulePathPatterns unmocks everything inside of a specific path. Right now, unmocking a node_module in any way should transitively unmock all of its dependencies. If that doesn't work, then that seems like a bug. Would you mind sharing your specific Jest configuration and what was mocked and what wasn't? :)

@ttrmw
ttrmw commented Apr 13, 2016

@goivemaster could you update this issue with our specifics? Otherwise I will edit this when I can pull the latest version later today. Thanks @cpojer

@goivemaster

Jest configuration :

"jest": {
    "rootDir": "./app/",
    "cache": false,
    "testPathIgnorePatterns": [
      "<rootDir>/__tests__/__helpers__/"
    ],
    "moduleFileExtensions": [
      "js",
      "jsx",
      "json"
    ],
    "unmockedModulePathPatterns": [
      "react",
      "moment",
      "radium",
      "enzyme",
      "cheerio",
      "lodash",
      "<rootDir>/__tests__/__helpers__/"
    ],
    "testFileExtensions": [
      "js",
      "jsx"
    ]
  }

If we put something inside unmockedModulePathPatterns should we expect all of its dependencies to also be unmocked? It appears that this is not the case. After adding enzyme to the unmockedModulePathPatterns we have been seeing errors relating to its dependencies remaining mocked hence the presence of cheerio and lodash. Is this a problem with Jest?

@cpojer
Member
cpojer commented Apr 13, 2016

They should be unmocked. Although I recommend specifying something like node_modules/enzyme to make clear it is a path. Are you using the latest version of Jest?

@goivemaster

this is the latest version of jest on npm: 11.0.0 is the latest of 81 releases,
we are running this version.
we see that the latest version on github is this: v0.10.0

we see no difference when adding the longer path.

we notice enzyme has no node_modules directory of its own

book.json        docs                       LICENSE.md       README.md          src
build            example-test.sh            mount.js         render.js          test
CHANGELOG.md     install-relevant-react.sh  package.json     shallow.js         withDom.js
CONTRIBUTING.md  INTHEWILD.md               ReactWrapper.js  ShallowWrapper.js

would this be causing the problem or does jest just look at the package.json of each package specified under unmockedModulePathPatterns?

@inventive-security

@goivemaster did you have any luck on getting enzyme working? I hit exactly the same problem as you hit. When I use jest.disableAutomock(); the tests works, but when I omit it and rely on the unmockedModulePathPatterns the test fails. I even added dependencies from enzyme in the unmockedModulePathPatterns but now the dependencies of the dependencies are the problem I guess...

@ttrmw
ttrmw commented Apr 26, 2016 edited

just a hunch, but this feels like it's related to Enzyme not having its own node_modules directory - this kind of transitive mocking seems to behave with other dependencies.

For our use case (I work with @goivemaster) we were able to get away with unmocking lodash and cheerio - which were causing the problems - by way of the unmockedModulePathPatterns config item.

Hoping @cpojer might have some insight on this one?

@cpojer
Member
cpojer commented Apr 27, 2016

That's weird, which version of Jest are you using? npm modules should definitely have transitive dependencies unmocked. If that's not the case, then that is a bug (the test for this is passing in Jest right now, though).

@nvdbleek

@cpojer It seems to have something to do with running multiple tests in one run. When I run tests individually they work, but when I run all the test suites together it fails depending on the order in which the test run/finish I have different tests failing.

Can it be that when test A requires module X to be mocked (and doesn't specify anything about module X, because mocking is the default) and a test B wants module X unmocked that test A might get an unmocked version. The opposite also seems true if you have automock-off and explicitly mock a module in a test you might get it mocked in another test that has automock-off.

I should try to create a simple reproducible test case I/we can use to debug the problem. Because in my real project I have way to many tests to make it easily debuggable, but it is hard to create such a test case because I don't know the internals, and have only a hunch about what is causing the problem.

@cpojer
Member
cpojer commented Apr 27, 2016

Oh I see. Can you upgrade to Jest 12 and let me know if this problem still persists? We fixed something around this issue :)

@nvdbleek

thanks, upgrading to Jest 12 now, will revert the workarounds of explicitly calling autmockoff, dontmock and mock, and then report back

@nvdbleek

@cpojer Thanks! Upgrading from Jest 11 to 12 indeed solved the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment