Jest: Jest fails when git branch name looks like a json file

[amclin]

🐛 Bug Report

Jest hard-fails with an exit code if the git branch is named package.json

To Reproduce

Steps to reproduce the behavior:

• Create a git branch named fix/package.json
• Push the branch to a remote
• Run jest jest --coverage

Expected behavior

Jest runs as normal

Actual behavior

Jest errors because it perceives the git reference marker file as a JSON file (because of the filename) and attempts to parse that reference marker as JSON (which it isn't, and never would be)

> jest --coverage

Error: Cannot parse /Users/c050004/Documents/git/logger/.git/refs/heads/fix/package.json as JSON: Unexpected token e in JSON at position 0
    at Object.<anonymous> (/Users/c050004/Documents/git/logger/node_modules/jest-haste-map/build/worker.js:167:15)
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/Users/c050004/Documents/git/logger/node_modules/jest-haste-map/build/worker.js:78:24)
    at _next (/Users/c050004/Documents/git/logger/node_modules/jest-haste-map/build/worker.js:98:9)
    at /Users/c050004/Documents/git/logger/node_modules/jest-haste-map/build/worker.js:103:7
    at new Promise (<anonymous>)
    at Object.<anonymous> (/Users/c050004/Documents/git/logger/node_modules/jest-haste-map/build/worker.js:95:12)
    at Object.worker (/Users/c050004/Documents/git/logger/node_modules/jest-haste-map/build/worker.js:120:18)
    at execFunction (/Users/c050004/Documents/git/logger/node_modules/jest-worker/build/workers/processChild.js:155:17)
    at execHelper (/Users/c050004/Documents/git/logger/node_modules/jest-worker/build/workers/processChild.js:139:5)
npm ERR! Test failed.  See above for more details.

Link to repl or repo (highly encouraged)

• https://github.com/amclin/react-project-boilerplate/pull/329
• https://travis-ci.com/github/amclin/react-project-boilerplate/jobs/320222180#L245

envinfo

  System:
    OS: macOS 10.15.3
    CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
  Binaries:
    Node: 10.16.3 - ~/.nvm/versions/node/v10.16.3/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 6.14.4 - ~/Documents/git/logger/node_modules/.bin/npm
  npmPackages:
    jest: ^25.3.0 => 25.3.0 

[SimenB]

jest-haste-map tries to crawl .git/, that seems... suboptimal. We should have some sort of sane default ignore list

[SimenB]

Also jest-haste-map shouldn't be trying to parse a directory as a file

It doesn't, .git/refs/heads/fix/package.json is a file containing the commitish

[SimenB]

diff --git i/packages/jest-haste-map/src/crawlers/node.ts w/packages/jest-haste-map/src/crawlers/node.ts
index c8254bd36..fe4d62385 100644
--- i/packages/jest-haste-map/src/crawlers/node.ts
+++ w/packages/jest-haste-map/src/crawlers/node.ts
@@ -47,6 +47,15 @@ function find(
   let activeCalls = 0;

   function search(directory: string): void {
+    const justDir = directory.split(path.sep).pop();
+
+    if (justDir?.startsWith('.')) {
+      if (activeCalls === 0) {
+        callback(result);
+      }
+      return;
+    }
+
     activeCalls++;
     fs.readdir(directory, {withFileTypes: true}, (err, entries) => {
       activeCalls--;
@@ -121,7 +130,7 @@ function findNative(
   callback: Callback,
 ): void {
   const args = Array.from(roots);
-  args.push('-type', 'f');
+  args.push('-type', 'f', '-not', '-path', '*/\\.*');
   if (extensions.length) {
     args.push('(');
   }

Seems to fix it. Are there use cases for not ignoring hidden directories?

We could also add '<rootDir>/\\.', or something to default modulePathIgnorePatterns, that also fixes it.

@jeysal @thymikee ideas? Watchman crawler skips it, not sure if watchman behavior or if we query it asking it to skip it

[thymikee]

I think it would be a sane default to skip dotfiles in modulePathIgnorePatterns and document (add a note?) how to enable this scenario (people put their tests in node_modules/ directories, so I can imagine someone is using a dotfile for the same thing :P)

[leonardovillela]

👋 Hey folks, I just created a PR following the suggestion of @thymikee.

[SimenB]

My main issue with fixing this issue via an ignore pattern is that if people override that pattern, watchman would still ignore it, but other crawlers would not. Which is why I think maybe fixing the crawlers is a better solution?

Oh, thanks for the PR @leonardovillela, very much appreciated 😃

[leonardovillela]

@SimenB I agree with you. But I think even in crawlers solution it should be configurable to cover the case when people put tests and stuff in hidden files.

Does this make sense to you?

[SimenB]

Right, but that will only work for those not using watchman

[leonardovillela]

Yes, I'm trying to maintain the fix with ignore pattern and change the watchman crawler to consider hidden files on fs operations, do you think this is a good approach?

[leonardovillela]

@SimenB I noticed some strange thing, I've watchman installed on my machine and I tested my PR with modulePathIgnorePatterns as the default value(ignoring hidden files) and without ignoring hidden files (e.g: modulePathIgnorePatterns: []) and in both scenarios watchman handle it correctly. So looks like the watchman is not ignoring hidden files.

Did I notice as well that all tests passed in my PR, the CI job is executed in a machine with a watchman installed?

Can you check out my PR locally and test it too?

My machine env info is:

$ npx envinfo --preset jest
  System:
    OS: macOS 10.15.4
    CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz
  Binaries:
    Node: 12.16.2 - ~/.asdf/installs/nodejs/12.16.2/bin/node
    Yarn: 1.22.4 - ~/.asdf/installs/nodejs/12.16.2/.npm/bin/yarn
    npm: 6.14.4 - ~/.asdf/installs/nodejs/12.16.2/bin/npm
  npmPackages:
    jest: ^25.4.0 => 25.4.0 

$ watchman --version
4.9.0

[SimenB]

To reproduce the error, I did the following locally (in the jest repo).

$ git checkout -b package.json
$ ./jest
# tests run fine
$ ./jest --no-watchman
# haste map explodes during boot

Might be that watchman just ignores .git directory and not hidden files in general?

[leonardovillela]

Yes, I took a look in this page on watchman docs and looks like it ignores .git by default.

[SimenB]

Ah, good find! Let's try to emulate that in the other crawlers then, and not mess with hidden files in general. So just skip the VCS roots?

[leonardovillela]

Yes, but we want this skip to be configurable? Other crawlers you say are only jest-haste-map/src/crawlers/node.ts? The VCS roots to be skipped can be the same as watchman(.git, .svn, and .hg)?

[SimenB]

I don't think we need to make it configurable for now. Somebody can complain if they really need it, but I don't really see a use case for tests in a VCS directory (while I can imagine use cases for tests in other hidden directories).

And yes, we only need to update the node crawler (which also has find in it)

[leonardovillela]

In the next days, I'll update my PR to follow this approach.