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

Temp fix: move lint & test workflow to use node 16.10 to prevent Windows out of memory #4544

Merged
merged 4 commits into from
Feb 23, 2022

Conversation

dac09
Copy link
Collaborator

@dac09 dac09 commented Feb 21, 2022

Jest runs out of memory on (internal) redwood tests, on Windows and node 16.

I'm still investigating how to improve this, but while I'm finding a solution this is a temporary work around (see here: jestjs/jest#11956)

@dac09 dac09 changed the title Temp fix: move workflow to use node 16.10 Temp fix: move lint & test workflow to use node 16.10 to prevent Windows out of memory Feb 21, 2022
@dac09 dac09 added the release:chore This PR is a chore (means nothing for users) label Feb 21, 2022
Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this idea buys us anything other than appeasing my aversion to reversion, but I think we could run a matrix for node-version: ['14', '16.10', '16'] and then put an if before steps that would kill the workflow when (os == windows && node.js == 16) || (os == ubuntu && node.js == 16.10)

¯_(ツ)_/¯

Or feel free to ignore me and carry on as is.

Copy link
Collaborator Author

dac09 commented Feb 22, 2022

Soooo many if statements yooo! I will do it if you really insist though, I don't think pinning to 16.10 is a bad thing - remember this is for redwood's internal tests not a user's project (which we should be more rigorous about using lts versions,etc.)

@thedavidprice
Copy link
Contributor

I don't insist. Just thinking through options but definitely not convinced myself.

Just bummed about the Jest mem issue most of all. Following that Issue you linked. Would be good to have an open Issue on our end following it as well as a reminder to revert someday (maybe hopefully).

@dac09
Copy link
Collaborator Author

dac09 commented Feb 23, 2022

Done #4557

That's going to take a while to diagnose I think!

@thedavidprice thedavidprice merged commit 39fdf71 into redwoodjs:main Feb 23, 2022
@jtoar jtoar added this to the next-release milestone Feb 23, 2022
@thedavidprice thedavidprice modified the milestones: next-release, v0.47.0 Feb 24, 2022
@dac09 dac09 deleted the fix/cli-test-oom branch March 6, 2022 07:01
@dac09 dac09 restored the fix/cli-test-oom branch March 6, 2022 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
No open projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

None yet

3 participants