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

erights commented 5 months ago

https://github.com/endojs/endo/issues/2198 was erroneously closed by #2200 because #2198 only manifested on browsers and we manually tested #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 #2198 until those old tests are fixed, in progress at #2229. But currently I can only verify that #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 #2200 do still actually fix the #2198 problem. The only remaining problem, being addressed in #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 commented 5 months ago

@kriskowal , @frazarshad , I co-assigned this to the three of us.

@kriskowal I see you merged https://github.com/endojs/endo/pull/2231 despite the two CI failures https://github.com/endojs/endo/actions/runs/8760680740/job/24046036596 . What’s failing there is exactly what this PR was adding

Unable to find Node version '22.x' for platform linux and architecture x64.

That’s why I tried #2232, #2233, and #2234 . #2234 brings this back to green CI with 21.x, which it finds, rather than 22.x, which it does not.

Also, how did you merge this despite the CI failures? Isn’t that supposed to be what CI protects us from?

Could/should you back out #2231 while we make progress towards getting the state of #2234 reviewed and merged?

erights commented 5 months ago

endojs / endo

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