Conversation
|
This fails on Node v20, we might want to just remove that from the test matrix as active support ended 22 Oct 2024? |
91187a6 to
a0f9aee
Compare
|
CI fails on Node.js v20 with SIGSEGV because Node-API version 10 is not fully supported on v20 — the fix landed in nodejs/node#55676 (v22.14.0+, never backported to v20). Blocked on #37 (dropping Node v20 from CI). |
|
We can use the napiVersion and skipTest primitives from #46 to skip this test when napi version < 10 |
Builds two addons from the same C source with different NAPI_VERSION defines (10 and 9) to verify that finalizers attempting to access JS during shutdown receive napi_cannot_run_js or napi_pending_exception (respectively), or napi_ok if the event loop is still running. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
a0f9aee to
a590d2c
Compare
Signed-off-by: Balakrishna Avulapati <ba@bavulapati.com>
use the napiVersion and skipTest primitives for porting test_cannot_run_js
| if (napiVersion < 10) skipTest(); | ||
| const addon_v8 = loadAddon("test_pending_exception"); | ||
| const addon_new = loadAddon("test_cannot_run_js"); |
There was a problem hiding this comment.
let's split this into two tests. so that we can only skip test_cannot_run_js when napiVersion < 10. We did the split similarly in #44
There was a problem hiding this comment.
I see we did that 🤔 My hope is that we can keep these tests very close (almost verbatim) to the original source as this makes it easier to automatically spot regressions in coverage.
And then have follow-up PRs with refactors, which are easy to revert in isolation.
I suggest that we create an issue to track splitting this or simply follow up with a PR splitting this once it merges. @legendecas would you be okay with that approach?
Summary
Ports
test_cannot_run_jsfromnode/test/js-native-api/to the CTS.test_cannot_run_jswithNAPI_VERSION=10,test_pending_exceptionwithNAPI_VERSION=9) to cover both code paths in the finalizernapi_get_named_propertyreturnsnapi_cannot_run_js(v10+),napi_pending_exception(v9), ornapi_okif the event loop is still runningUpstream source: https://github.com/nodejs/node/tree/main/test/js-native-api/test_cannot_run_js
Test plan
npm run addons:configure && npm run addons:build— both addons compile cleanlynpm run node:test— all 56 tests pass🤖 Generated with Claude Code