-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
chore: use createDirectory from jest-util when creating directories #7610
Conversation
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! Should we add node 10 + experimentalWorker to circle?
Yeah, we can do that. |
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.
Mind also extracting createDirectory
from jest-util
's index.js
to its own file while at it?
That being in there already bugged me when I added specialChars
recently 😄
Yeah, can do that as well |
f74a3ef
to
f431a14
Compare
Added the worker build, hopefully it passes 🤞 |
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.
Well played
It's failing... @rickhanlonii something you could look into? Seems like the coverage doesn't pick up uncovered files, as well as some other stuff |
Moved the worker CI run to #7637 |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
After landing #7408,
jest-worker
will useworker_threads
if they are available (behind a flag in node 10 & 11, unflagged in Node 12). However, running Jest's own integration tests in a worker currently fails withTypeError: process.umask is not a function
If you do not provide a
mode
tomkdir
, it usesprocess.umask
(https://github.com/substack/node-mkdirp/blob/f2003bbcffa80f8c9744579fabab1212fc84545a/index.js#L64) which throws that error.See https://nodejs.org/api/worker_threads.html#worker_threads_class_worker
createDirectory
injest-util
passes777
as mode (same asnode
's ownfs.mkdir
does), so it works perfectly fine in a workerThis is just changing internal stuff, so no changelog.
Test plan
Ran tests locally with workers after the change