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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Native file crawling doesn't work with POSIX find #10263

Closed
richardlau opened this issue Jul 10, 2020 · 7 comments 路 Fixed by #10308
Closed

Native file crawling doesn't work with POSIX find #10263

richardlau opened this issue Jul 10, 2020 · 7 comments 路 Fixed by #10308

Comments

@richardlau
Copy link
Contributor

richardlau commented Jul 10, 2020

馃悰 Bug Report

I've been investigating module failures with CITGM that we're seeing on AIX in the Node.js CI that are using Jest to test and are all reporting "No tests found". As an example, Jest itself (using @SimenB 's nodejs/citgm#728): https://ci.nodejs.org/job/citgm-smoker-nobuild/876/nodes=aix71-ppc64/console

I tracked the problem down to the file crawler in jest-haste-map passing a non-POSIX option -iname to find:
https://github.com/facebook/jest/blob/d79de34e964dc405b379d2f59583748f8dcc99e3/packages/jest-haste-map/src/crawlers/node.ts#L132

This works with, e.g. the GNU version of find but on systems where the default find only supports the POSIX options (e.g. AIX) the subsequent spawn:
https://github.com/facebook/jest/blob/d79de34e964dc405b379d2f59583748f8dcc99e3/packages/jest-haste-map/src/crawlers/node.ts#L139
exits with a non-zero exit code (which isn't checked) and results in no files being found.

POSIX find: https://pubs.opengroup.org/onlinepubs/009695399/utilities/find.html
AIX find: https://www.ibm.com/support/knowledgecenter/ssw_aix_72/f_commands/find.html
Linux find: https://man7.org/linux/man-pages/man1/find.1.html
(note https://man7.org/linux/man-pages/man1/find.1.html#STANDARDS_CONFORMANCE on which options are POSIX compliant. https://man7.org/linux/man-pages/man1/find.1.html#HISTORY suggests -iname was added in findutils 3.8 but I'm guessing it's very unlikely to find a version of findutils older than that out in the wild?)

FWIW this is what happens when you try to run find . -iname '' on AIX:

bash-4.4$ find . -iname ''
find: bad option -iname
bash-4.4$ echo $?
1
bash-4.4$

I can see there's a forceNodeFilesystemAPI internal option that if I force to true fixes the behavior (as it bypasses the native find):
https://github.com/facebook/jest/blob/d79de34e964dc405b379d2f59583748f8dcc99e3/packages/jest-haste-map/src/crawlers/node.ts#L25-L30
There doesn't seem to be a way to set that option externally (which could be a workaround)? #9329 is semi-related but in this case find exists but the options that are being passed to it are incompatible.

Possible solutions I can see:

  • Provide a way to set forceNodeFilesystemAPI externally -- Jest crashes on systems without find聽#9329 (comment) appears to discourage this.
  • Check the return from the child.spawn and fall back to the non-native crawl if it is non-zero.
  • Be a bit smarter in hasNativeFindSupport -- maybe try running find . -iname '' and check for a non-zero exit code? That could potentially also replace the process.platform === 'win32' check in case someone on Windows has the GNU version of find (e.g. from git) on their PATH ahead of the default Windows find.

To Reproduce

Steps to reproduce the behavior:
Run jest on any module on AIX (or system with a pure POSIX version of find. I recreated it with a minimal module following the Getting Started - Jest guide.

The following is the output from running jest:

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
No files found in /home/riclau/sandbox/github/tmp/jesttest.
Make sure Jest's configuration does not exclude this directory.
To set up Jest, make sure a package.json file exists.
Jest Documentation: facebook.github.io/jest/docs/configuration.html
Pattern:  - 0 matches

See the repo in the "Link to repl or repo" section below for an reproduction using docker where find is wrapped to reject the -iname option.

Expected behavior

Tests are found and run.

Link to repl or repo (highly encouraged)

https://github.com/richardlau/jesttest

-bash-4.2$ docker build -t jesttest .
Sending build context to Docker daemon 40.55 MB
Step 1/6 : FROM node:14
 ---> 37ad18cd8bd1
Step 2/6 : WORKDIR /home/nodejs/app/
 ---> 6f9ef342deee
Removing intermediate container cb99dd9a5500
Step 3/6 : COPY find *.js package.json ./
 ---> 3f4aa2d37525
Removing intermediate container 337b320ada2d
Step 4/6 : RUN yarn install
 ---> Running in 4c8e76a26f84

yarn install v1.22.4
info No lockfile found.
[1/4] Resolving packages...
warning jest > @jest/core > jest-config > jest-environment-jsdom > jsdom > request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
warning jest > @jest/core > jest-haste-map > sane > micromatch > snapdragon > source-map-resolve > resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated
warning jest > @jest/core > jest-haste-map > sane > micromatch > snapdragon > source-map-resolve > urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
[2/4] Fetching packages...
info fsevents@2.1.3: The platform "linux" is incompatible with this module.
info "fsevents@2.1.3" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
Done in 12.47s.
 ---> 3428888974a5
Removing intermediate container 4c8e76a26f84
Step 5/6 : RUN chmod u+x find
 ---> Running in c64c47dce267

 ---> ccae1b64bbdd
Removing intermediate container c64c47dce267
Step 6/6 : RUN export PATH=`pwd`:$PATH && yarn test
 ---> Running in 6c9216db4c6a

yarn run v1.22.4
$ jest
No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
No files found in /home/nodejs/app.
Make sure Jest's configuration does not exclude this directory.
To set up Jest, make sure a package.json file exists.
Jest Documentation: facebook.github.io/jest/docs/configuration.html
Pattern:  - 0 matches
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
The command '/bin/sh -c export PATH=`pwd`:$PATH && yarn test' returned a non-zero code: 1
-bash-4.2$

envinfo

npx: installed 1 in 2.096s

  System:
    OS: aix
    CPU: (32) ppc64 PowerPC_POWER7
  Binaries:
    Node: 15.0.0 - ~/sandbox/bin/node-v15.0.0-pre/bin/node
    Yarn: 1.22.4 - ~/sandbox/bin/node-v15.0.0-pre/bin/yarn
    npm: 6.14.5 - ~/sandbox/bin/node-v15.0.0-pre/bin/npm
  npmPackages:
    jest: ^26.0.1 => 26.1.0

(I've been testing with the current master branch from https://github.com/nodejs/node. The CITGM run from https://ci.nodejs.org/job/citgm-smoker-nobuild/876/nodes=aix71-ppc64/console was using Node.js v14.5.0.)

@richardlau
Copy link
Contributor Author

richardlau commented Jul 10, 2020

I've added a dockerfile to https://github.com/richardlau/jesttest where I've substituted a "bad" find, i.e. one that returns a non-zero exit if -iname is passed.

-bash-4.2$ docker build -t jesttest .
Sending build context to Docker daemon 40.55 MB
Step 1/6 : FROM node:14
 ---> 37ad18cd8bd1
Step 2/6 : WORKDIR /home/nodejs/app/
 ---> 6f9ef342deee
Removing intermediate container cb99dd9a5500
Step 3/6 : COPY find *.js package.json ./
 ---> 3f4aa2d37525
Removing intermediate container 337b320ada2d
Step 4/6 : RUN yarn install
 ---> Running in 4c8e76a26f84

yarn install v1.22.4
info No lockfile found.
[1/4] Resolving packages...
warning jest > @jest/core > jest-config > jest-environment-jsdom > jsdom > request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
warning jest > @jest/core > jest-haste-map > sane > micromatch > snapdragon > source-map-resolve > resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated
warning jest > @jest/core > jest-haste-map > sane > micromatch > snapdragon > source-map-resolve > urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
[2/4] Fetching packages...
info fsevents@2.1.3: The platform "linux" is incompatible with this module.
info "fsevents@2.1.3" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
Done in 12.47s.
 ---> 3428888974a5
Removing intermediate container 4c8e76a26f84
Step 5/6 : RUN chmod u+x find
 ---> Running in c64c47dce267

 ---> ccae1b64bbdd
Removing intermediate container c64c47dce267
Step 6/6 : RUN export PATH=`pwd`:$PATH && yarn test
 ---> Running in 6c9216db4c6a

yarn run v1.22.4
$ jest
No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
No files found in /home/nodejs/app.
Make sure Jest's configuration does not exclude this directory.
To set up Jest, make sure a package.json file exists.
Jest Documentation: facebook.github.io/jest/docs/configuration.html
Pattern:  - 0 matches
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
The command '/bin/sh -c export PATH=`pwd`:$PATH && yarn test' returned a non-zero code: 1
-bash-4.2$

@SimenB
Copy link
Member

SimenB commented Jul 14, 2020

Thanks for the detailed issue @richardlau! Your third option sounds like the best one to me as long as it doesn't cause regressions for Windows users. I'm not sure why we never attempt find (that part of the code base predates my involvement in the project by quite some time), but missing-iname sounds plausible.

Would you be up for a PR? If not, I can get to it in a couple of weeks (currently on holiday)

@richardlau
Copy link
Contributor Author

Would you be up for a PR? If not, I can get to it in a couple of weeks (currently on holiday)

Yes, I'll open a PR.

@SimenB
Copy link
Member

SimenB commented Jul 14, 2020

Awesome! We have which call as well, can probably be replaced with an actual call to find checking if we can use it or not

@richardlau
Copy link
Contributor Author

richardlau commented Jul 23, 2020

Opened #10308.

@SimenB
Copy link
Member

SimenB commented Jul 30, 2020

Released in 26.2.0, thanks for the help!

@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 11, 2021
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.

2 participants