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

[bug] duplicate manual mock found in separate directories #2070

Open
paularmstrong opened this issue Nov 9, 2016 · 77 comments
Open

[bug] duplicate manual mock found in separate directories #2070

paularmstrong opened this issue Nov 9, 2016 · 77 comments

Comments

@paularmstrong
Copy link

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
@yongdamsh
Copy link

+1

@yongdamsh
Copy link

yongdamsh commented Nov 15, 2016

In my case, after clear the cacheDirectory/var/folders/dm/vt920lmd6tzdq_709zkykwx40000gn/T/jest and re-install npm dependencies, these messages have been disappeared.

@LegNeato
Copy link

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:

cfade28

I wonder why we are using basename rather than the whole path as the key?

/cc @flarnie

@LegNeato
Copy link

LegNeato commented Nov 17, 2016

This means that basenames 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 🐞 .

@cpojer
Copy link
Member

cpojer commented Nov 17, 2016

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 :) )

@LegNeato
Copy link

Cool. I might find some time tomorrow to cook up a patch, but no promises though 😅

@juliankrispel
Copy link

@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...

@cpojer
Copy link
Member

cpojer commented Nov 18, 2016

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.

@juliankrispel
Copy link

@cpojer how about a flag? Would you accept a pr that enables/disables this with a flag?

@cpojer
Copy link
Member

cpojer commented Nov 18, 2016

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.

@juliankrispel
Copy link

@cpojer right - which parts of jest does this touch?

@cpojer
Copy link
Member

cpojer commented Nov 18, 2016

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.

@juliankrispel
Copy link

@cpojer thanks for the pointers 👍

@ColCh
Copy link
Contributor

ColCh commented Dec 4, 2016

@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? 😉

@EnoahNetzach
Copy link
Contributor

EnoahNetzach commented Dec 5, 2016

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.

@EnoahNetzach
Copy link
Contributor

@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?

@EnoahNetzach
Copy link
Contributor

The two solutions I see are:

  • modify getMockName to accomodate the option to use the basename or the full path
  • remove that function altogether

@ColCh
Copy link
Contributor

ColCh commented Dec 5, 2016

it would be nice to see @cpojer answer on it

@cpojer
Copy link
Member

cpojer commented Dec 6, 2016

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 from rootDir (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?

@EnoahNetzach
Copy link
Contributor

In order:

  • Let's fix manual mocks in Jest!

HURRAY 😄 🎉

  • 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 [""] 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.

@ColCh
Copy link
Contributor

ColCh commented Dec 6, 2016

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 of project/__node_modules_mocks__/react.js
if you have a file project/react.js, then use project/__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 by AppScreen, 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.

@EnoahNetzach
Copy link
Contributor

EnoahNetzach commented Dec 6, 2016

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?

@ColCh
Copy link
Contributor

ColCh commented Dec 6, 2016

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)
and app/node_modules/express as web server from npm
throw 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?

@EnoahNetzach
Copy link
Contributor

What I meant is: what if you have an npm installed 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?

@cpojer
Copy link
Member

cpojer commented Dec 8, 2016

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?

@EnoahNetzach
Copy link
Contributor

I should be able to work on this no sooner than the next weekend.

@ColCh
Copy link
Contributor

ColCh commented Dec 9, 2016

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?

@cpojer
Copy link
Member

cpojer commented Dec 9, 2016 via email

@vinodloha
Copy link

vinodloha commented Aug 9, 2019

In my case, the mock module was copied to Dist dir with every build.

Looks like this is a problem with typescript being unable to exclude paths/patterns that belong to deeper dir structure. Its still a problem with "typescript@^3.4.5".

To fix this I started cleaning my Dist dir with "rimraf dist" before every test run.

"test:unit": "npm run clean && stencil test --spec --snapshot",

I know its a hack, but works.

@trackmystories
Copy link

Hey i solved this here is what happened, and maybe it can help you duplicate this.

three solutions or scenarios:

1, I was editing my app on my text editor twice meaning, I was running pod install / update and react-native run-ios from two different windows. and I got this error, I tried searching for the duplicate files in Xcode and on my app but i couldn't find any. so I simply deleted the app from the simulator and re-ran react-native run-ios and it worked, It turned out that two node_modules had been duplicated as such: node_modules and node_modules0 in my scr file.

2, sometimes when I press random keys on my mac the node_modules are duplicated in for example the SRC folder so this also be the case, so it makes sense to have a look at your folders for any node_modules duplications.

3, I could'nt get my app to start until I launched and terminated a different app on the same simulator and then I rebooted the app with the duplication which then fired up without any errors.

@goranefbl
Copy link

Still nothing? Cant use modulePathIgnorePatterns in CRA

@amedley
Copy link

amedley commented Aug 28, 2020

3 years and 10 months

@NickHeiner
Copy link

Alternatively, it would be nice if we could force Jest to throw an error when this issue comes up, rather than just warning. Warnings are easily ignored; on my project, I'd prefer that an error be thrown, so the dev who introduced it has to deal with it.

@ymoran00
Copy link

ymoran00 commented Dec 3, 2020

3 years and 10 months

And one of the messages 3 years ago was "I might find some time tomorrow to cook up a patch, but no promises though"... :-P

@NickHeiner
Copy link

Heh, I've left many such comments myself.

@adbl
Copy link

adbl commented May 7, 2021

Is there no workaround (without changing your implementation). index exports are really useful and being able to mock them throughout...

@adbl
Copy link

adbl commented May 7, 2021

here's what I came up with 💩

// jest.setup.js

jest.mock('src/my/cool/Scope', () => require('src/my/cool/Scope/__mocks__/Scope.index'))

@JeromeS4RB
Copy link

To ask the stupid question... Would using the whole path instead of just the filename not solve this?

@holylander
Copy link
Contributor

"modulePathIgnorePatterns": ["<rootDir>/src/react/.*/__mocks__"],.

Still it's quite weird that mocks are not just unique based on the full path from the root.

this is what worked for me :)

@LKay
Copy link

LKay commented Sep 17, 2021

When this bug will be fixed? I have __mocks__ folders mocking the same module differently in different folders that conflict with each other. The first one that is resolved when test run is executed is used for all mocking fo that module.

@adbl
Copy link

adbl commented Dec 16, 2021

We have found that in spite of the warnings, our actual tests succeed and thus the individual manual index mocks are actually being used. What then does the warnings mean?

@shamilovtim
Copy link

A flag to turn off the warning would be good enough for us. Or its outright removal. There's no added value from this warning. Having two identically named files is impossible on a standard file system. My concern is that if we keep investing in manual mocks we're going to get flooded by these warnings. They make the DX poor and make our project look clunky and broken.

Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Jan 24, 2024
@shamilovtim
Copy link

Valid

@github-actions github-actions bot removed the Stale label Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.