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

Jest tests cause require-dir to break in dependencies #4567

Closed
nickserv opened this issue Sep 30, 2017 · 7 comments · Fixed by #4614
Closed

Jest tests cause require-dir to break in dependencies #4567

nickserv opened this issue Sep 30, 2017 · 7 comments · Fixed by #4614

Comments

@nickserv
Copy link
Contributor

nickserv commented Sep 30, 2017

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

What is the current behavior?
When a Jest test requires a module in node_modules that uses require-dir with a relative path, the path is resolved relatively to the package that has the executed Jest test instead of the package that uses require-dir (what's supposed to happen). This issue does not occur if exactly the same code is executed directly through Node instead of Jest.

If the current behavior is a bug, please provide the steps to reproduce and either a repl.it demo through https://repl.it/languages/jest or a minimal repository on GitHub that we can yarn install and yarn test.

  1. Clone https://github.com/nickmccurdy/jest-require-dir-bug
  2. npm install-test
~/Repos/jest-require-dir-bug → node index.test.js 
~/Repos/jest-require-dir-bug → npm test

> @ test /Users/nick/Repos/ncu-example
> jest

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

    ENOENT: no such file or directory, scandir '/Users/nick/Repos/ncu-example/dir'
      
      at Object.fs.readdirSync (fs.js:911:18)
      at requireDir (node_modules/require-dir/index.js:25:20)
      at Object.<anonymous> (node_modules/example/index.js:1:112)
      at Object.<anonymous> (index.test.js:1:90)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.88s
Ran all test suites.
npm ERR! Test failed.  See above for more details.

This issue can also be observed by requiring code that uses npm-check-updates (which also uses require-dir) via JavaScript in a Jest test.

What is the expected behavior?
Jest's execution environment should allow require-dir to function normally, requiring a relative directory with the requireDir('./example') syntax even in a dependency. Jest should also behave the same as Node itself, or work around this difference in execution environments.

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.

  • Empty configuration
  • index.test.js or __tests__/index.js naming conventions
  • Jest 15-21.2.1
  • require-dir 0.3.2
  • Node 8.6.0
  • npm 5.4.2
  • macOS 10.13
@jvivs
Copy link

jvivs commented Oct 1, 2017

Looks like it's a wontfix: #1657 @cpojer can you confirm?

Currently looking into how and the right place to mock the problematic part of my dependency graph.

@nickserv
Copy link
Contributor Author

nickserv commented Oct 1, 2017

Thanks for the helpful issue link, I am interesting in knowing if module.parent is a wontfix. I also noticed that when I did use __dirname, I got errors suggesting that Jest was marking the directory as empty (as reported in that issue).

While I personally dislike directory require utilities (in favor of index.js files using static dependencies), it's unfortunate that this causes some third party library (including npm-check-updates to fail in Jest tests when the tested project itself doesn't use them. Is there any way around this besides reimplementing module.parent or patching dependencies to not use module.parent? I was also thinking about a mocking solution, I wonder if we could somehow automatically mock module.parent in a way that could allow these utilities to continue working automatically (or at least with some sort of additional opt in mocking API).

@cpojer
Copy link
Member

cpojer commented Oct 1, 2017

At this point I'm ok if somebody adds support for module.parent, granted the PR includes tests.

We do not support require.cache, so it would only work if you also call jest.resetModules(), which I guess is fine for your use-case, right?

@nickserv
Copy link
Contributor Author

nickserv commented Oct 1, 2017

Cool, do you know roughly how involved it would be to add support? Is it a matter of hooking into module loading in the VMs and adding metadata or is it a more invasive refactor?

@cpojer
Copy link
Member

cpojer commented Oct 1, 2017

Shouldn't be too much, check out jest-runtime's main module, which is Jest's require implementation.

@SimenB
Copy link
Member

SimenB commented Oct 6, 2017

@nickmccurdy I've opened a PR for it

@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 13, 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

Successfully merging a pull request may close this issue.

4 participants