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

Upgrade to Node 20 #137

Merged
merged 3 commits into from
Feb 26, 2024
Merged

Upgrade to Node 20 #137

merged 3 commits into from
Feb 26, 2024

Conversation

amydevs
Copy link
Contributor

@amydevs amydevs commented Feb 23, 2024

Description

This PR upgrades the Node version for all development environments to Node 20, and enables building of arm64 macos builds.

Issues Fixed

Tasks

  • 1. Update utils.nix node package to nodejs_20 for buildsp
  • 2. Update flake.nix mkShell inputs node package to nodejs_20 for nix develop environments
  • 3. Update scripts/choco-install.ps1 to use nodev20.5.1
  • 4. Update scripts/brew-install.sh to use node@20.
  • 5. Add macos Arm64 builds to .gitlab-ci.yml
  • 6. Docblock utils.pkStdio to warn usage when the expected process.exitCode is non-zero.

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai
Copy link
Member

fix: reset `flake.nix` pin to older version
@amydevs
Copy link
Contributor Author

amydevs commented Feb 26, 2024

The problem with the jest tests is:

* [JEST tests complete successfully but returns exit status 1 jestjs/jest#9324](https://github.com/jestjs/jest/issues/9324)

* [[Bug]: In Node 20, Jest exits with non-zero exit code if `process.exitCode` is set in the tested code jestjs/jest#14501](https://github.com/jestjs/jest/issues/14501)

We need a workaround for dealing with process.exitCode = .

@CMCDragonkai

The workaround I have opted, is to docblock pkStdio which was mocking a Polykey instance rather than creating a subprocess, with warnings regarding usage when the expected exitCode is non-zero. I have then replaced the usage of pkStdio wherever the exitCode is expected to be non-zero.

An alternative solution could be to just set the exitCode to zero at the end of pkStdio.

@amydevs amydevs merged commit 5855f03 into staging Feb 26, 2024
@CMCDragonkai
Copy link
Member

Did you make sure to indicate in that docblock that the reason for this workaround is solely due a jest bug, and link the jest bug itself when it gets resolved?

@CMCDragonkai
Copy link
Member

An alternative solution could be to just set the exitCode to zero at the end of pkStdio.

Not sure if that would work, after all isn't it the internal command that is executed that does it?

@CMCDragonkai
Copy link
Member

@amydevs @tegefaulkes you must remember to set the assignments - otherwise these won't be tracked later in R&D.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Update to Node20 and switch pkg
2 participants