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

Feature request: Need to habitually test on pre-release forms of supported target platforms #9265

Open
erights opened this issue Apr 19, 2024 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@erights
Copy link
Member

erights commented Apr 19, 2024

Same issue for agoric-sdk as endojs/endo#2230 describes for endo. Like endo, agoric-sdk CI is not currently run on anything more recent than Node 20. We should at least CI under Node 21, so we have advance warning of problems before Node 22 is released.

endojs/endo#2198 was erroneously closed by endojs/endo#2200 because endojs/endo#2198 only manifested on browsers and we manually tested endojs/endo#2200 on those browsers to "verify" that it was fixed.

However, we are not set up to run our normal yarn test CI tests habitually (or under CI) on those browser engines. We current do our regular yarn test CI testing, where everything was coming up green, but only because the most recent Node we CI is Node 20, and Node 20 does not yet have v8's problematic stack accessor behavior. I separately reopened endojs/endo#2198 until those old tests are fixed, in progress at endojs/endo#2229. But currently I can only verify that endojs/endo#2229 fixes it by running yarn test locally with Node 21.

So, feature request: We should as much as possible run all our normal tests on the pre-release form of all target platforms we support, so we get early warning when we're about to stop working on a release.

Fortunately, in this case, it looks like the src/ fixes from endojs/endo#2200 do still actually fix the endojs/endo#2198 problem. The only remaining problem, being addressed in endojs/endo#2229, is to update the tests. This would indeed have caused CI failures as soon as we would upgrade to support Node 22, but only because the tests are wrong. The code being tested already seems good for Node 21 and thus presumably for Node 22, at least as far as these issues are concerned.

@erights erights added the bug Something isn't working label Apr 19, 2024
mergify bot added a commit that referenced this issue Jul 1, 2024
refs: #9622

## Description
#9623 claimed to enforce Node.js versions ^18.12 or ^20.9, but due to a typo allows arbitrarily high versions to pass the check. This corrects it to enforce that the major version is exactly 18 or 20.

### Security Considerations
None.

### Scaling Considerations
n/a

### Documentation Considerations
Matches the [README](https://github.com/Agoric/agoric-sdk?tab=readme-ov-file#prerequisites).

### Testing Considerations
Checked by integration testing in CI.

### Upgrade Considerations
Will need an update for supporting Node.js v22 when that enters LTS (ref #9265).
gibson042 pushed a commit that referenced this issue Jul 1, 2024
refs: #9622

## Description
#9623 claimed to enforce Node.js versions ^18.12 or ^20.9, but due to a typo allows arbitrarily high versions to pass the check. This corrects it to enforce that the major version is exactly 18 or 20.

### Security Considerations
None.

### Scaling Considerations
n/a

### Documentation Considerations
Matches the [README](https://github.com/Agoric/agoric-sdk?tab=readme-ov-file#prerequisites).

### Testing Considerations
Checked by integration testing in CI.

### Upgrade Considerations
Will need an update for supporting Node.js v22 when that enters LTS (ref #9265).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants