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

Does not work with requireindex #2017

Closed
xjamundx opened this issue Oct 27, 2016 · 10 comments
Closed

Does not work with requireindex #2017

xjamundx opened this issue Oct 27, 2016 · 10 comments

Comments

@xjamundx
Copy link

Do you want to request a feature or report a bug?

BUG

What is the current behavior?

requireindex does not work with jest

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.

// my.test.js
var requireindex = require('requireindex')
var x = requireindex('./jest')
var len = Object.keys(x).length
var assert = require('assert')
assert.equal(len, 1)
// jest/one.js
module.exports = {}
node my.test.js # no errors
./node_modules/.bin/jest my.test.js # failure

What is the expected behavior?

requireindex should work

Run Jest again with --debug and provide the full configuration it prints. Please mention your node and npm version and operating system.

~/dev/paypal/p2pnodeweb (pr/jest-all-the-things-2) $ ./node_modules/.bin/jest --no-cache --debug --env node my.test.js 
jest version = 16.0.2
test framework = jasmine2
config = {
  "rootDir": "/Users/jamuferguson/dev/paypal/p2pnodeweb",
  "name": "-Users-jamuferguson-dev-paypal-p2pnodeweb",
  "setupFiles": [
    "/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/babel-polyfill/lib/index.js"
  ],
  "testRunner": "/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/jest-jasmine2/build/index.js",
  "testEnvironment": "/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/jest-environment-node/build/index.js",
  "scriptPreprocessor": "/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/babel-jest/build/index.js",
  "usesBabelJest": true,
  "automock": false,
  "bail": false,
  "browser": false,
  "cacheDirectory": "/var/folders/9d/p3qfw0g94yz7qh3z9dr4btwm391xgk/T/jest",
  "clearMocks": false,
  "coveragePathIgnorePatterns": [
    "/node_modules/"
  ],
  "coverageReporters": [
    "json",
    "text",
    "lcov",
    "clover"
  ],
  "globals": {},
  "haste": {
    "providesModuleNodeModules": []
  },
  "mocksPattern": "__mocks__",
  "moduleDirectories": [
    "node_modules"
  ],
  "moduleFileExtensions": [
    "js",
    "json",
    "jsx",
    "node"
  ],
  "moduleNameMapper": {},
  "modulePathIgnorePatterns": [],
  "noStackTrace": false,
  "notify": false,
  "preset": null,
  "preprocessorIgnorePatterns": [
    "/node_modules/"
  ],
  "resetModules": false,
  "testPathDirs": [
    "/Users/jamuferguson/dev/paypal/p2pnodeweb"
  ],
  "testPathIgnorePatterns": [
    "/node_modules/"
  ],
  "testRegex": "(/__tests__/.*|\\.(test|spec))\\.jsx?$",
  "testURL": "about:blank",
  "timers": "real",
  "useStderr": false,
  "verbose": null,
  "watch": false,
  "cache": false,
  "watchman": true,
  "testcheckOptions": {
    "times": 100,
    "maxSize": 200
  }
}
jest-haste-map: duplicate manual mock found:
  Module name: node-uuid
  Duplicate Mock path: /Users/jamuferguson/dev/paypal/p2pnodeweb/public/js/__mocks__/node-uuid.js
This warning is caused by two manual mock files with the same file name.
Jest will use the mock file found in: 
/Users/jamuferguson/dev/paypal/p2pnodeweb/public/js/__mocks__/node-uuid.js
 Please delete one of the following two files: 
 /Users/jamuferguson/dev/paypal/p2pnodeweb/tests/unit/__mocks__/node-uuid.js
/Users/jamuferguson/dev/paypal/p2pnodeweb/public/js/__mocks__/node-uuid.js


 FAIL  ./my.test.js
  ● Test suite failed to run

    AssertionError: 0 == 1

      at Object.<anonymous> (my.test.js:5:8)
      at process._tickCallback (node.js:369:9)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.84s
Ran all test suites matching "my.test.js".
@xjamundx
Copy link
Author

Not much code here. I may be able to debug myself:
https://github.com/stephenhandley/requireindex/blob/master/index.js

@xjamundx
Copy link
Author

Here is the reason:

This line always is false:
https://github.com/stephenhandley/requireindex/blob/master/index.js#L45

Because require.extensions === {} inside JEST-land.

 node -e "console.log(require.extensions)"
{ '.js': [Function], '.json': [Function], '.node': [Function] }

Possibly related to #281

@xjamundx
Copy link
Author

Manually adding require.extensions['.js'] = true inside requireindex/index.js fixes the issue, but adding this to my setup file doesn't seem to help, I assume because this needs to happen deep inside jests VM setup or something.

@cpojer
Copy link
Member

cpojer commented Oct 28, 2016

We don't support this feature consciously in our require implementation. In the applications we create we would like to be able to statically analyze the dependency tree. Custom module resolution algorithms make this more complicated and require custom tooling. It also breaks jest --watch because we cannot reliably find out which tests should be run based on changed files. People have brought this up in the past and the conclusion has usually been that such extensions aren't great patterns.

@IcanDivideBy0
Copy link

@cpojer Most of the time this is used before using a simple require to check against file extension.

How adding the supported extensions to require object can breaks jest ? Should we understand that dynamic require isn't supported by jest ?

The more you use jest with node, the more it get annoying, and your answer is definitly not a valid one for the reported issue

@cpojer
Copy link
Member

cpojer commented Oct 31, 2016

You are right, it isn't supported the way you expect. Jest has its own require implementation and we don't support all kinds of things that node.js supports. If Jest doesn't work for you, please feel free to use a different test runner or send us a PR that adds the feature you need – then we can discuss appropriate implementations :)

@fkrauthan
Copy link

Hmm it feels to me like a huge problem if jest does not even support all features Node.js require supports. So there are no plans to at least support all nodejs require ways?

@cpojer
Copy link
Member

cpojer commented Nov 15, 2016

As said previously, feel free to work on a PR that adds require.extensions support to Jest. The require implementation for Jest is part of the Runtime: https://github.com/facebook/jest/blob/master/packages/jest-runtime/src/index.js

Note that if we add this, it is possible that we'll break jest --watch for those things that use require.extensions but I guess that is an ok trade-off.

@xjamundx
Copy link
Author

Thanks for the heads up @cpojer if I get really ambitious I'll look into this.

Frankly I think the following would unblock many of us:

Here in https://github.com/facebook/jest/blob/master/packages/jest-runtime/src/index.js#L663:

moduleRequire.extensions = Object.create(null);

Replacing that with:

moduleRequire.extensions = Object.create(null);
moduleRequire.extensions['.js'] = function() {}

Not sure what it would break though :)

@github-actions
Copy link

This issue 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 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants