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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent EPERM windows transform issue from failing tests #11104

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lukeapage
Copy link
Contributor

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.

@github-actions
Copy link

github-actions bot commented Sep 8, 2022

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.

@github-actions github-actions bot added the Stale label Sep 8, 2022
@lukeapage
Copy link
Contributor Author

Don’t close - it’s awaiting review :(

@github-actions github-actions bot removed the Stale label Sep 8, 2022
@SimenB
Copy link
Member

SimenB commented Sep 28, 2022

could you rebase?

return false;
}
if (!fs.existsSync(cachePath)) {
console.warn('Possible problem writing cache if this occurs many times', e);
Copy link
Member

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

Copy link
Contributor Author

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.

@rdeangelis83
Copy link

How to proceed with this PR?

@dave99galloway
Copy link

EasyCLA Expected — Waiting for status to be reported - can someone with write access force this to update so the PR can be merged please?

@mrazauskas
Copy link
Contributor

mrazauskas commented May 4, 2023

By the way, it looks like watchPathIgnorePatterns does not work on Windows and this might be related. See #13869 (comment) and the following conversation.

I think, first it would be good if someone with Windows machine could fix the issue with anymatch and prove that it works as expected. Having watchPathIgnorePatterns fixed, users could ignore problematic files. Also .git folder would not be watched.

EDIT Ignoring errors is fun, but that hides underlying problems instead of fixing them.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 5, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@netlify
Copy link

netlify bot commented May 5, 2023

Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 5ed4145
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/6540a48b84ca45000850424a
😎 Deploy Preview https://deploy-preview-11104--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@lukeapage
Copy link
Contributor Author

EasyCLA Expected — Waiting for status to be reported - can someone with write access force this to update so the PR can be merged please?

merged latest and signed

By the way, it looks like watchPathIgnorePatterns does not work on Windows and this might be related. See #13869 (comment) and the following conversation.

I think, first it would be good if someone with Windows machine could fix the issue with anymatch and prove that it works as expected. Having watchPathIgnorePatterns fixed, users could ignore problematic files. Also .git folder would not be watched.

EDIT Ignoring errors is fun, but that hides underlying problems instead of fixing them.

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.

@WesleyHovick
Copy link

Nearly a year since this was originally reviewed. What is the status here?

@lukeapage
Copy link
Contributor Author

@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:
https://github.com/fluffynuts/patch-jest-cache
#4444 (comment)

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.

Copy link

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.

@github-actions github-actions bot added the Stale label Oct 31, 2024
@lukeapage
Copy link
Contributor Author

Still waiting for a review

@github-actions github-actions bot removed the Stale label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants