feat: port 15 easy js-native-api tests to CTS#25
Open
kraenhansen wants to merge 8 commits intonodejs:mainfrom
Open
feat: port 15 easy js-native-api tests to CTS#25kraenhansen wants to merge 8 commits intonodejs:mainfrom
kraenhansen wants to merge 8 commits intonodejs:mainfrom
Conversation
This was referenced Mar 1, 2026
9188967 to
ad36fb5
Compare
Port all "Easy" difficulty engine-specific tests from Node.js: 3_callbacks, 4_object_factory, 5_function_factory, 7_factory_wrap, 8_passing_wrapped, test_array, test_bigint, test_dataview, test_date, test_handle_scope, test_instance_data, test_new_target, test_number, test_promise, test_properties, test_reference_double_free, test_symbol. C/C++ sources are copied from Node.js with minimal changes (entry_point.h macro). JS tests are adapted to use CTS globals (loadAddon, assert, gcUntil) instead of Node.js test harness (common.js, require). test_dataview: removed SharedArrayBuffer tests that depend on experimental node_api_is_sharedarraybuffer API (not in stable node-api-headers). Also updates node-api-headers from 1.7.0 to 1.8.0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mark 17 easy js-native-api tests as Ported (test_dataview as Partial due to missing experimental SharedArrayBuffer APIs in node-api-headers). Add "Experimental Node-API Features" section documenting which tests depend on experimental APIs not yet available in node-api-headers, and the approach needed to support them. References nodejs#26. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ad36fb5 to
d8c7229
Compare
The upstream test verifies finalizer and environment teardown behavior via subprocesses and worker threads, not just increment(). The simplified CTS port lost that coverage, so remove it and reclassify as Medium. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
What happens if the function is never called? Would the awaiting on the promise hang indefinitely? |
Use a regular function wrapper with fn.apply(this, args) to preserve receiver binding, and verify call counts on process exit instead of returning a [wrapper, called] Promise tuple. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
@KevinEady Yes - and I think that's not great. We could extend it to fail on a timeout. I choose this because I expected it to be easier to implement for the other implementors, but thinking more about it, I think this optimization is pre-mature and we should preserve the "fail on exit if function wasn't called" semantics for now. I've pushed 36dfc98 to restore that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports 15 "Easy" difficulty
js-native-apitests to the CTS.Ported tests
3_callbacks,4_object_factory,5_function_factory,7_factory_wrap,8_passing_wrapped,test_array,test_bigint,test_date,test_handle_scope,test_new_target,test_number,test_promise,test_properties,test_reference_double_free,test_symbolC/C++ sources copied from Node.js with minimal changes (
entry_point.hmacro). JS tests adapted to use CTS globals (loadAddon,assert,gcUntil,mustCall,mustNotCall) instead of Node.js test harness (common.js,require()).Coverage verification
All C/C++ files are byte-for-byte identical to upstream. JS test files were compared assertion-by-assertion:
3_callbackscommon.mustCall→mustCall)4_object_factory5_function_factory7_factory_wrap8_passing_wrappedtest_arraytest_biginttest_datetest_handle_scopetest_new_targettest_numbertest_promisecommon.mustCall/common.mustNotCall→mustCall/mustNotCall)test_propertiestest_reference_double_freetest_symbolNew harness helper:
mustCall/mustNotCallNode.js tests use
common.mustCall(fn)to wrap callbacks and assert they are called. The CTS provides equivalent globals that preserve the same call pattern:mustCall(fn)— wrapsfnand asserts it is called exactly once before process exit. Returns the wrapper function directly, so the call pattern is identical to Node.js'scommon.mustCall().mustNotCall(msg)— returns a function that throws immediately if called.Example:
Note: The Node.js implementor uses
process.on("exit")to verify call counts, matching Node.js's owncommon.mustCallimplementation. Other runtimes can implement this verification using their own lifecycle hooks.Notable adaptations
test_dataview: Reclassified from Easy to Medium — depends on experimentalSharedArrayBufferAPIs (node_api_is_sharedarraybuffer) not available innode-api-headers. Tracked in Support experimental Node-API features in the CTS #26.test_instance_data: Reclassified from Easy to Medium — the upstream test verifies finalizer and environment teardown behavior via subprocesses and worker threads, which can't be faithfully ported without harness support.test_promise: UsesmustCall()/mustNotCall()to assert callbacks are invoked, replacingcommon.mustCall().7_factory_wrap/8_passing_wrapped: Useawait gcUntil(...)for finalizer testing.Other changes
node-api-headersfrom 1.7.0 to 1.8.0Test plan
npm run addons:configure && npm run addons:build— all 16 addons compilenpm run node:test— all 45 tests pass (4 harness + 41 test files)🤖 Generated with Claude Code