-
-
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
Prevent EPERM windows transform issue from failing tests #11104
base: main
Are you sure you want to change the base?
Conversation
This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
Don’t close - it’s awaiting review :( |
could you rebase? |
return false; | ||
} | ||
if (!fs.existsSync(cachePath)) { | ||
console.warn('Possible problem writing cache if this occurs many times', e); |
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.
how does this logging look in tests when it happens? seems somewhat disruptive
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.
yes, I'm not sure exactly what to do...
There may be people where writing the cache file fails on windows and its down to permissions, so it happens every time.
But there is also the situation where this occasionally fails all the time intermittently.
So my idea was that it would warn constantly on the console for the first situation and not at all for the second situation.
Currently I am using https://github.com/fluffynuts/patch-jest-cache which I believe does not warn - so I'm happy to remove the warning or if it is a concern, make this a option.
How to proceed with this PR? |
|
By the way, it looks like I think, first it would be good if someone with Windows machine could fix the issue with EDIT Ignoring errors is fun, but that hides underlying problems instead of fixing them. |
✅ Deploy Preview for jestjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
merged latest and signed
This is unrelated.. I'd much rather fix the issue, but as per the PR description - there is a link to a comment in the issue where I explain what happens, the problem is around 2 jest agents trying to update/write the same file at once and its because on windows, rename operations are not atomic. |
Nearly a year since this was originally reviewed. What is the status here? |
@SimenB Please could you find some time to review this PR? Theres a whole load of people using a special patch package to patch jest to fix this issue, for years: but now v30 will mean that package needs fixing to work with the new bundled files. I'd much rather get this merged and we can retire the patches and close the issue. I'll do whatever you want to this PR - remove the logging, make it an option, whatever but I'd really like to be able to use jest without is breaking and without having to patch it every install. |
This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
Still waiting for a review |
Summary
We would like to not have jest failing multiple runs a day because of #4444.
See #4444 (comment) for an explanation of what is going on.
Test plan
This is really hard to test... If you think this is an approach you could accept I'd be happy to discuss what else you need.