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
fix: EMFILE errors #18313
fix: EMFILE errors #18313
Conversation
✅ Deploy Preview for docs-eslint canceled.
|
I could use some help understanding what is going on with the lint error. Linting passes locally, so it's difficult for me to debug. |
Looks like a bug in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Leaving open for a couple of days if anyone else would like to review it before merging.
When the EMFILE test runs locally, it will create 100000 files in eslint/tests/lib/eslint/eslint.js Line 120 in 09675e1
eslint/tests/lib/eslint/legacy-eslint.js Line 105 in 09675e1
eslint/tests/lib/cli-engine/cli-engine.js Line 96 in 29c3595
Line 100 in ae8103d
It seems unnecessary to copy those files, even if the EMFILE test will mostly run only in CI. Would it be possible to delete the test files after the EMFILE test runs? Alternatively, we could create the files for the EMFILE test in a different directory in or in a temporary directory and run the test there. |
Moved the file creation to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note about .gitignore, then LGTM.
Co-authored-by: Francesco Trotta <github@fasttime.org>
@fasttime @mdjermanovic can you re-approve please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
File read operations that throw an EMFILE or ENFILE error will be automatically retried.
I also added a test that runs only in CI to save us time locally. There was an error with the abort handling that I had to fix in order to make the test pass.
fixes #18301
Is there anything you'd like reviewers to focus on?