jest
jest copied to clipboard
[bug] duplicate manual mock found in separate directories
Do you want to request a feature or report a bug? Bug
What is the current behavior?
Given a file tree:
src/app/modules
├── module1
│ ├── index.js
│ ├── __tests__/
├── module2
│ ├── index.js
│ ├── __tests__/
I use the modules outside of the modules
directory by importing them by directory name:
import Module1 from '../modules/module1';
import Module2 from '../modules/module2';
I'd like to be able to mock module1
and module2
. However, if I create src/app/modules/module1/__mocks__/index.js
and src/app/modules/module2/__mocks__/index.js
, I'm given the duplicate manual mock found
error from jest-haste-map
.
If, however, I try to create src/app/modules/__mocks__/{module1.js,module2.js}
, the mocked files are not used.
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal repository on GitHub that we can npm install
and npm test
.
See above behavior.
What is the expected behavior?
I would expect either approach to work, given that the first case uses different paths and the second case uses the pathname of the module.
Run Jest again with --debug
and provide the full configuration it prints. Please mention your node and npm version and operating system.
node v6.2.0 npm v3.8.9 OS X 10.11.6
> NODE_ENV=test jest --env jsdom "--debug" "src/app/redux/modules/devices"
jest version = 17.0.0
test framework = jasmine2
config = {
"moduleFileExtensions": [
"js",
"json"
],
"moduleDirectories": [
"node_modules"
],
"moduleNameMapper": [
[
"^.+\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$",
"/Users/paul/dev/tools/jest/mock-assets.js"
],
[
"^.+\\.css$",
"identity-obj-proxy"
]
],
"name": "dev",
"setupTestFrameworkScriptFile": "/Users/paul/dev/tools/jest/setup-framework.js",
"testPathDirs": [
"/Users/paul/dev/src"
],
"testRegex": "/__tests__/.*\\.test\\.js$",
"timers": "fake",
"rootDir": "/Users/paul/dev",
"setupFiles": [],
"testRunner": "/Users/paul/dev/node_modules/jest-jasmine2/build/index.js",
"testEnvironment": "/Users/paul/dev/node_modules/jest-environment-jsdom/build/index.js",
"transform": [
[
"^.+\\.jsx?$",
"/Users/paul/dev/node_modules/babel-jest/build/index.js"
]
],
"usesBabelJest": true,
"automock": false,
"bail": false,
"browser": false,
"cacheDirectory": "/var/folders/dm/vt920lmd6tzdq_709zkykwx40000gn/T/jest",
"coveragePathIgnorePatterns": [
"/node_modules/"
],
"coverageReporters": [
"json",
"text",
"lcov",
"clover"
],
"expand": false,
"globals": {},
"haste": {
"providesModuleNodeModules": []
},
"mocksPattern": "__mocks__",
"modulePathIgnorePatterns": [],
"noStackTrace": false,
"notify": false,
"preset": null,
"resetMocks": false,
"resetModules": false,
"snapshotSerializers": [],
"testPathIgnorePatterns": [
"/node_modules/"
],
"testURL": "about:blank",
"transformIgnorePatterns": [
"/node_modules/"
],
"useStderr": false,
"verbose": null,
"watch": false,
"cache": true,
"watchman": true,
"testcheckOptions": {
"times": 100,
"maxSize": 200
}
}
jest-haste-map: duplicate manual mock found:
Module name: index
Duplicate Mock path: /Users/paul/dev/src/app/modules/push-notification-manager/__mocks__/index.js
This warning is caused by two manual mock files with the same file name.
Jest will use the mock file found in:
/Users/paul/dev/src/app/modules/push-notification-manager/__mocks__/index.js
Please delete one of the following two files:
/Users/paul/dev/src/app/modules/image-file/__mocks__/index.js
/Users/paul/dev/src/app/modules/push-notification-manager/__mocks__/index.js
No tests found
1 file checked.
testPathDirs: /Users/paul/dev/src - 1 match
testRegex: /__tests__/.*\.test\.js$ - 0 matches
testPathIgnorePatterns: /node_modules/ - 1 match
+1
In my case, after clear the cacheDirectory
/var/folders/dm/vt920lmd6tzdq_709zkykwx40000gn/T/jest and re-install npm dependencies, these messages have been disappeared.
Here's the offending code:
https://github.com/facebook/jest/blob/cd8976ec50dbed79cfe07f275052cdd80d466e73/packages/jest-haste-map/src/index.js#L98
But it looks like the behavior might be explicitly wanted as there is a test confirming it:
https://github.com/facebook/jest/blob/8de90b320c87a0a36d68f6bd8177620a985df269/packages/jest-haste-map/src/tests/snapshots/index-test.js.snap#L15
Which was added in:
https://github.com/facebook/jest/commit/cfade282fbbe2737b6dd2cee1cf3da3ee8624512
I wonder why we are using basename
rather than the whole path as the key?
/cc @flarnie
This means that basename
s for modules need to be globally unique in a project when using manual mocks. For my use case, it means I can't do something like:
import { MyWhatever } from 'models/MyWhatever/schema';
import { MyOtherWhatever } from 'models/MyOtherWhatever/schema';
and use manual mocks at the same time. Jest will currently see them both as mocking schema
and complain.
Though the workaround is trivial (s/schema/MyWhateverSchema/), it feels like a bug to rename and restructure non-test code to make jest happy 🐞 .
Yes this does suck indeed. The manual mocking system is really not good and I'm happy to accept PRs that will improve the situation, assuming we can make sure we don't break all of FB (but I can take care of that :) )
Cool. I might find some time tomorrow to cook up a patch, but no promises though 😅
@cpojer is there a particular reason for this behaviour?
Could it have anything to do with the fact that fb uses unique filenames for modules? I don't see otherwise how not allowing two mocks with the same name makes any sense...
Yes, mocks are "global" as well. This is a terrible design that we have to live with, unfortunately. At FB we have 4000+ mock files in the wrong location (and often there isn't even a proper location). It is likely we will fix this early next half, so this should improve in Jest. I'm happy to support PRs that improve the behavior in Jest for open source – if we can retain the old behavior for Jest at FB for now.
@cpojer how about a flag? Would you accept a pr that enables/disables this with a flag?
Yeah, it should be a config option. But I'm not just talking about the warning, I'm also talking about the feature in general.
@cpojer right - which parts of jest does this touch?
The resolution code is called from jest-runtime: https://github.com/facebook/jest/blob/master/packages/jest-runtime/src/index.js and is somewhere in jest-resolve and jest-resolve-dependencies.
@cpojer thanks for the pointers :+1:
@cpojer what about some global override, like JEST_USE_BASENAME_FOR_CACHING
, to switch this behaviour?
At least, we can enjoy non-unique filenames, and it will not break anything in FB.
Of course, it's a temporary solution.
I mean, this is in some /etc/profile
or ~/.bashrc
export JEST_USE_BASENAME_FOR_CACHING="true"
(or some file with env) and then
$ jest
or this one, without env file modifications:
$ JEST_USE_BASENAME_FOR_CACHING="true" jest
What you think? It's a sort of hack or it's ok? :wink:
I just tried with a new repo, using two versions of jest (^15.0.0
and ^17.0.0
) and, although the latter gives the warning, the test behaves as expected.
@ColCh I don't think the issue here is with the cache, probably a more suitable name could be JEST_USE_BASENAME_FOR_MOCKING
.
@cpojer if FB code has the restriction on the uniqueness of the names, using the full path as a key for the map of mocks shouldn't bring problems.
Am I right or there is something I'm not seeing?
The two solutions I see are:
- modify
getMockName
to accomodate the option to use the basename or the full path - remove that function altogether
it would be nice to see @cpojer answer on it
Hey everyone, sorry for the delay, I'm pretty backed up with a ton of stuff right now.
I think I'm fine if you guys decide to do whatever breaking change in Jest that is necessary to improve this system. Manual mocking is really messed up and doesn't work well. One thing we kind of want to do is limit the scope of "haste modules" (internal FB module system) using a config option, like "haste_modules": ['path/a', 'path/b']"
and then only look at haste modules in those folders, including this weird mocking behavior. If anybody wants to make this change, that would be amazing.
One thing to be figured out then, is this: If all manual mocks are local, like __mocks__/a.js
maps to a.js
, what do we do with node_module mocks? For this there are a couple of ways:
- Introduce a new
__node_modules_mocks__
folder but that is ugly. - The top level
__mocks__
folder as seen fromrootDir
(project root) could act as a global folder.
So to summarize:
- Let's fix manual mocks in Jest!
- Let's namespace haste modules and limit them to certain folders/regex whatever.
- Make sure that the current mocking behavior still works for FB, even if that means we have to whitelist certain folders to make haste work (it would just look like
["<rootDir>"]
for us for now, I guess) - Figure out how to still be able to mock node modules.
What do you think?
In order:
- Let's fix manual mocks in Jest!
HURRAY :smile: :tada:
- Let's namespace haste modules and limit them to certain folders/regex whatever.
- Make sure that the current mocking behavior still works for FB, even if that means we have to whitelist certain folders to make haste work (it would just look like ["<rootDir>"] for us for now, I guess)
I'm not sure I understand the needs with haste.
What you mean is to give the possibility to say "those modules are haste modules"?
If we have four modules: /path_1/a
, /path_1/b
, /path_2/a
, /path_2/c
, and the setting is
"haste_modules:" ["/path_1/a", "/path_2/c"]
only /path_1/a
and /path_1/b
are restricted to exist only in /path_1
, so /path_2/c
is valid, and /path_2/a
raises an error/warning.
I'd say, targets could easily be specific files and entire directories, even with single/double *
.
- Figure out how to still be able to mock node modules.
I'd maintain the current behavior:
If the module you are mocking is a node module (eg: fs), the mock should be placed in the same parent directory as the node_modules folder.
My thoughts:
haste modules:
I think , haste_modules
is nice, it fits just like collectCoverageFrom
and other options: array of globs
In case, if you have all src
as haste modules, and just one directory is non-haste:
haste_modules: [
"src",
"!src/foo"
]
node_modules
@EnoahNetzach what if someone have same names for app module and module from node_modules?
To make it work without haste... hm, I think it can be described like:
given node module
project/node_modules/react
, mock will be inside ofproject/__node_modules_mocks__/react.js
if you have a fileproject/react.js
, then useproject/__mocks__/react.js
(of course, react.js
is an example. here can be any filename among of all modules, which can be installed from npm)
Really, mocking node_modules module is a rare case by my experience, so... may be rareness may compensate ugliness in particular case of mocking node_modules ?
Anyone mocking modules inside of node_modules often?
what about to think further
as I noticed, for react-native projects, we often have to mock application modules
and leave modules from node_modules unmocked (e.g. lodash
)
this means, we have:
- manual created mock component for each dumb component (dumb component are layout components)
- a long list of
jest.mock
call in every test file
what I want to say: it would be very nice to have ability to auto mock modules on some paths.
It can be implemented around well-known data structure array-of-jest-globs in config, and filtering modules upon it.
I will describe that step by step
Given this config entry
"autoMockingPaths": [
"src/components/dumb/**/*.js",
]
and this code at src/screens/app.js
:
import _ from 'lodash';
import Button from '../../components/dumb/button.js';
// blah blah AppScreen implementation skipped
and this test code for screen at src/screens/__tests__/app-test.js
:
import AppScreen from '../app.js';
describe('AppScreen', () => /* testing app screen */);
We come to this situation, in context of app-test.js
:
-
AppScreen
is not mocked -
lodash
is not mocked -
Button
, which is required byAppScreen
, is mocked
... You can answer, how it will play with automock
config entry?
simply saying, automock: true
is equivalent to:
"autoMockingPaths": [
"<rootDir>"
]
auto mocking... wait!
may be just introduce special value for automock
? at least it will not break people's configs
for example, with this config entry:
automock: "app"
jest will auto mock all application modules, and leave actual versions for modules from node_modules
what do you think about app level modules automocking, @cpojer ? I find it very efficient for my particular case.
I fully agree with "haste_modules"
.
We personally don't use automocking that much, so I can't say what's better, my wild guess is that the "autoMockingPaths"
var could be useful and elastic enough.
On the contrary I find "automock": "app"
too stiff (jest already disabled automocking by default).
The __node_modules_mocks__
could be an option, I agree that rareness compensate for ugliness (in my particular case, we rarely mock node_modules
, and when we have to do it, we use jest.mock(...)
).
The only caveat is: what happens when you have a nested node_modules
folder (e.g. src/node_modules
), do you have to mock its modules from the global __node_modules_mocks__
, a nested version of it, or normally with __mocks__
co-located?
may be just throw, if someone has same module name in node_modules
and app modules
e.g.
app/express.js
as app module (may be I'm doing train game) andapp/node_modules/express
as web server from npmthrow new Error("can't mock express.js file - it duplicates one from node_modules")
in this case, __mocks__
can be used for node_modules
, but dev have to rename own modules on such collisions
nah... this is more ugly than __node_modules_mocks__
, isn't it?
What I meant is: what if you have an npm install
ed module x
and later, deeper in your codebase define a module x
in a nested node_modules
folder?
The naming collision is usually handled in node by preferring the nearest, but I don't know how this works in haste.
I'm bringing this up because projects like Create React App are using it, or will do in the near future.
On a side note, @cpojer is this issue somehow related?
Let's keep this focused on changing how haste works (whitelist/blacklist rather than on by default). I do think I'd prefer to maintain that <rootDir>/__mocks__
should be the default for node module mocks. We could make this a configuration option as well: "globalMocks" that defaults to <rootDir>/__mocks__
. Is anybody willing to work on this?
I should be able to work on this no sooner than the next weekend.
I can work on PR this sunday, I'm kinda free
@cpojer just to recap - create globalMocks
config entry with default value of <rootDir>/__mocks__
. This option regulates use of node-haste
within jest by specifying path? Or it will be array of paths?
I think these are some larger changes on the way to get this resolved but I think we need both the singular globalMocks option (could be a string or array of strings) and the hasteModules option which would be an array of paths of haste modules. Most of this code lives in jest-haste-map and jest-resolve. I'm not 100% sure what the right solution will look like yet.
From: Max Sysoev [email protected] Sent: Friday, December 9, 2016 8:18:44 AM To: facebook/jest Cc: Christoph Pojer; Mention Subject: Re: [facebook/jest] [bug] duplicate manual mock found in separate directories (#2070)
I can work on PR this sunday, I'm kinda free
@cpojerhttps://github.com/cpojer just to recap - create globalMocks config entry with default value of <rootDir>/mocks. This option regulates use of node-haste within jest by specifying path? Or it will be array of paths?
You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/facebook/jest/issues/2070#issuecomment-265958606, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAA0KAMFc34iKqBDLHZzgaGHqyc3WkAzks5rGQ7kgaJpZM4Kt2DW.
Sorry, my workstation became broken, and apparently no way to get it working in couple of months (1-2 months). I even lost my project on this PR :( So, sorry for taking responsibilities.
~~What about just a config option to change the behavior of getMockName
?~~
~~Not too familiar with the internals of jest, but it looks like that is the simplest solution to fix the issue without breaking jest for FB.~~
This is going to be more complicated than I originally thought. To me, manual mocks should replace the file they're closest too, ie. something like this:
{ 'aws-sdk': '/Users/project/__mocks__/aws-sdk.js',
'slack': '/Users/project/__mocks__/slack.js',
'/Users/project/db/index': '/Users/project/db/__mocks__/index.js',
'/Users/project/slack/index': '/Users/projects/slack/__mocks__/index.js' }
require('aws-sdk')
should resolve to the /Users/project/__mocks__/aws-sdk.js
this is a mock for a node_module
.
require('./db')
(or any path to db
) should resolve to: /Users/project/db/__mocks__/index.js
.
My understanding of the way to setup jest manual mocks (and there should probably be more documentation if something like the above is used) was that they should be as close to the file being mocked as possible within a __mocks__
directory.
Manual mocks are defined by writing a module in a mocks/ subdirectory immediately adjacent to the module. (https://facebook.github.io/jest/docs/manual-mocks.html)
Given that, the above behavior makes the most sense to me.
Thoughts?