Jest fails when git branch name looks like a json file #9829
Comments
|
|
It doesn't, |
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 @jeysal @thymikee ideas? Watchman crawler skips it, not sure if watchman behavior or if we query it asking it to skip it |
|
I think it would be a sane default to skip dotfiles in |
|
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 😃 |
|
@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? |
|
Right, but that will only work for those not using watchman |
|
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? |
|
@SimenB I noticed some strange thing, I've 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 |
|
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 bootMight be that watchman just ignores |
|
Yes, I took a look in this page on watchman docs and looks like it ignores |
|
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? |
|
Yes, but we want this skip to be configurable? Other crawlers you say are only |
|
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 |
|
In the next days, I'll update my PR to follow this approach. |
|
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. |
🐛 Bug Report
Jest hard-fails with an exit code if the git branch is named
package.jsonTo Reproduce
Steps to reproduce the behavior:
fix/package.jsonjest --coverageExpected 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)
Link to repl or repo (highly encouraged)
envinfo
The text was updated successfully, but these errors were encountered: