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]: Unable to run a test from whitelisted "node_modules" #11781

Closed
kettanaito opened this issue Aug 24, 2021 · 13 comments · Fixed by #11084
Closed

[Bug]: Unable to run a test from whitelisted "node_modules" #11781

kettanaito opened this issue Aug 24, 2021 · 13 comments · Fixed by #11084

Comments

@kettanaito
Copy link
Contributor

kettanaito commented Aug 24, 2021

Version

27.0.6

Steps to reproduce

  1. Clone the repo at https://github.com/kettanaito/jest-nm-issue
  2. yarn install
  3. yarn test

Expected behavior

Jest detects the node_modules/__test/example.test.js file and runs it.
The file matches the testMatch pattern, which you can confirm by clicking on that pattern in the terminal and being navigated to the correct existing directory (tested in VS Code).

Actual behavior

$ jest
No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/kettanaito/Projects/contrib/jest-nm-issue
  2 files checked.
  testMatch: /Users/kettanaito/Projects/contrib/jest-nm-issue/node_modules/__test/*.test.js - 0 matches
  testPathIgnorePatterns: not_node_modules - 2 matches
  testRegex:  - 0 matches
Pattern:  - 0 matches
  • 0 matches next to the correct pattern pointing to an existing directory under node_modules is not expected.

Additional context

I distribute a test suite as a node module because it acts as a specification for multiple projects. I wish to run that test suite from the node_modules where it's installed.

Regardless of my setup, it's confusing that Jest seems to ignore anything as long as it includes "node_modules" even if I've explicitly whitelisted a certain directory in it.

Environment

System:
  OS: macOS 11.5.1
  CPU: (16) x64 Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
Binaries:
  Node: 12.18.2 - /usr/local/bin/node
  Yarn: 1.22.10 - /usr/local/bin/yarn
  npm: 6.14.5 - /usr/local/bin/npm
npmPackages:
  jest: ^27.0.6 => 27.0.6
@sigveio
Copy link
Contributor

sigveio commented Aug 24, 2021

Hey @kettanaito,

You are probably hitting a limitation/rationalisation around how Jest's internal crawler/cache system jest-haste-map works. Normally (for the vast majority of use cases) running the tests of external libraries within node_modules is not in the scope of what Jest should do. Tracking those files would therefore be unnecessarily expensive. It would involve Jest going in and reading/parsing every single JS file in node_modules and retaining metadata about them.

@kettanaito
Copy link
Contributor Author

Thanks for the quick reply, @sigveio!

I see. I was suspecting some internals at play, it's good to know. What would you recommend to run tests from node_modules still? I don't mind a workaround, given it's not entirely hacky.

@sigveio
Copy link
Contributor

sigveio commented Aug 24, 2021

If I were to fire from the hip, probably forking jest-haste-map and swapping it out with your own flavour

Does that sound about right, @cpojer?

@kettanaito
Copy link
Contributor Author

That's still unexpected, is it not?

I understand that jest-haste-map blacklists node_modules for performance (and probably other) reasons but if I explicitly whitelist a single directory in node_modules what are the implications of Jest running in it? The nested node_modules are still ignored, I'm keeping my testMatch pattern scoped to a certain file pattern in a certain folder.

I'd be willing to submit a pull request to fix this if you indeed find this an issue. A corner case, I agree, but still.

@tomalec
Copy link

tomalec commented Aug 30, 2021

I'd like to add another use case here.

We are running a platform (WordPress > WooCommerce) with an ecosystem of plugins/extensions. As platform developers, we created a package containing a suite of tests (many tests in many files).

Now, we would like plugin developers (within our company and from the community) to consume this package. Run the suite, to make sure their plugin integrates well with the platform and serves a consistent UX.

The problem is how one can consume test suites from the npm package?:

  1. Add the node module to the allow-list

    • Not possible as per this issue.
  2. Import them and call from a local file:

    import { runAllSuites } from `@woocommerce/admin-e2e-tests`;
    runAllSuites();
    • Fails as well. The above will join them to a single suite, then the report will be shown only once the entire file=> all the suites => all the tests are run (see Report progress of individual test cases #6616 (comment). The consumers would have to wait many minutes to notice what's happening. Being stuck with:
    > npm run test
    RUNS  ../../../tests/specs/imported.test.js
    
    Test Suites: 0 passed, 2 total
    Tests:       40 passed, 385 total
    Snapshots:   0 total
    Time:        235.221s
    

    Plus, most probably will hit the timeout too early.

The only fallback I can think of is to ask consumers, to clone the test file structure, and import every single suite separately. What's extremely fragile for the maintenance updates, requires manual labor, and pollutes the consumer's files structure, impairing DevX.

@ashleykolodziej
Copy link

I'll chime in with my use case - I'm a professor using GitHub classroom to automatically provision repositories for my students to work on, and I use Jest to help me with grading to ensure students have met specific requirements in their code. I keep my tests in a separate repository, and make it a dependency in node_modules, so that if a student discovers a bug in my test after the class starts the assignment, I can update it once for the class and not have to update 20+ individual repositories.

Here's an example of how I have my tests listed as a dependency: https://github.com/ProfessorKolodziej/cm523-hello-world/blob/main/package.json#L79

How I configured Jest to run tests from node modules in a hacky way that I know is going to fail next time I update Jest because providesModuleNodeModules is removed in version 26: https://github.com/ProfessorKolodziej/cm523-hello-world/blob/main/package.json#L85-L112

And then the actual test for this assignment that I run across all my class's repositories is here: https://github.com/ProfessorKolodziej/cm523-unit-tests/blob/main/hello-world/index.test.js

Right now, this setup works fine for anyone on a Mac or Ubuntu, but I just had a student report that this is broken on Windows, and I'm not sure how to keep my couple of students on Windows moving now. They run a test command locally before pushing changes to GitHub to more quickly check that their requirements are in place.

Being able to pull a test from a specific repository in node_modules in a more officially supported way would be an ENORMOUS help to me.

@kettanaito
Copy link
Contributor Author

kettanaito commented Sep 14, 2021

Thanks for the suggestion, @sigveio.

By looking at jest-haste-map, I can see two places where it decides how to treat files that are located in node_modules:

  1. When processing a file:
    https://github.com/facebook/jest/blob/d7f097543f8c222b1ec52bc9bddf3a60c40d59ce/packages/jest-haste-map/src/index.ts#L564-L566

  2. When deciding which files to ignore:

https://github.com/facebook/jest/blob/d7f097543f8c222b1ec52bc9bddf3a60c40d59ce/packages/jest-haste-map/src/index.ts#L1089-L1100

While the NODE_MODULES logic is not configurable, it's clearly stated that it respects the retainAllFiles option (which is somehow omitted in the Jest's docs alongside many other supported options for haste-map).

It seems that by setting the retainAllFiles: true in the haste configuration object Jest should process files located in node_modules:

// jest.config.js
module.exports = {
  haste: {
    retainAllFiles: true
  }
}

Unfortunately, Jest seems to be supporting only selective jest-haste-map options in its haste configuration option:

● Validation Warning:

  Unknown option "haste.retainAllFiles" with value true was found.
  This is probably a typing mistake. Fixing it will remove this message.

Please, @sigveio, do you happen to know why not all jest-haste-map options are supported? It seems to be a one-property solution if only Jest supported the haste.retainAllFiles option.

@marcinwieprzkowicz
Copy link

Hey @kettanaito,

I have had exactly the same problem. Your last message really helped me to understand and fix it on my side. Here is how I did it:

I used jest.config.haste.hasteMapModulePath to provide my custom HasteMap implementation:
jest.config.js

const path = require("path");

module.exports = config = {
  ...
  testPathIgnorePatterns: [], // this is also important, by default jest.config ignore `/node_modules/` folder.
  haste: {
    hasteMapModulePath: path.join(__dirname, "./custom-haste-map.js"),
  }
};

custom-haste-map.js

const path = require("path");
const JestHasteMap = require("jest-haste-map");
const NODE_MODULES = path.join(__dirname, "/node_modules/");

class CustomHasteMap extends JestHasteMap {
  _ignore(filePath) {
    const ignorePattern = this._options.ignorePattern;
    const ignoreMatched =
      ignorePattern instanceof RegExp
        ? ignorePattern.test(filePath)
        : ignorePattern && ignorePattern(filePath);

    return (
      ignoreMatched ||
      (!this._options.retainAllFiles && filePath.includes(NODE_MODULES))
    );
  }
}

And that's it. Hope it works on your side too 😉

@robolivable
Copy link

This is an issue that's been around for a while: #2145

There are clear use cases for running jest embedded from within a parent node_modules directory. The only work around to achieve this is short term and fragile since it involves some sort of monkey patch strategy that gets phased out with major version releases.

Is this a feature that will eventually be supported or not? If not it should at least be documented to discourage people from trying to embed jest in the first place (this can save folks quite a bit of time/effort).

@pauldraper
Copy link

pauldraper commented Dec 21, 2021

Normally (for the vast majority of use cases) running the tests of external libraries within node_modules is not in the scope of what Jest should do.

That's exactly why jest has the configuration option testPathIgnorePatterns default to /node_modules/.

The bug, is that no matter what value you set testPathIgnorePatterns to, jest will ignore node_modules anyway!

This is due to complicated multi-level architecture around listing file, and took me hours to figure out.

One wonders what the purpose of even having the testPathIgnorePatterns default, since jest hard codes ignoring anything, ever that has /node_modules/ anywhere in the path.


My hack is:

const  { default: HasteMap } = require("jest-haste-map");

class CustomHasteMap extends HasteMap {
  _ignore(filePath) {
    const ignorePattern = this._options.ignorePattern;
    const ignoreMatched =
      ignorePattern instanceof RegExp
        ? ignorePattern.test(filePath)
        : ignorePattern && ignorePattern(filePath);
    return ignoreMatched;
  }
}

module.exports = CustomHasteMap;

Configure that with haste.hasteMapModulePath and set testPathIgnorePatterns to [].

@pauldraper
Copy link

This should be closed as dup of #2145

pauldraper added a commit to pauldraper/jest that referenced this issue Feb 20, 2022
Resolves jestjs#2145 and jestjs#11781. Prevent haste map from automatically discarding
node_modules files.

By default, node_modules is still excluded via the testPathIgnorePatterns
option. However, users can now use that option to allow node_modules
without hacks.
pauldraper added a commit to pauldraper/jest that referenced this issue Feb 20, 2022
Resolves jestjs#2145 and jestjs#11781. Prevent haste map from automatically discarding
node_modules files.

By default, node_modules is still excluded via the testPathIgnorePatterns
option. However, users can now use that option to allow node_modules
without hacks.
@SimenB SimenB linked a pull request Feb 21, 2022 that will close this issue
@SimenB
Copy link
Member

SimenB commented Feb 24, 2022

@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 Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants