Skip to content

feat: port test_dataview to CTS#39

Open
bavulapati wants to merge 2 commits intonodejs:mainfrom
bavulapati:feat/port-test-dataview
Open

feat: port test_dataview to CTS#39
bavulapati wants to merge 2 commits intonodejs:mainfrom
bavulapati:feat/port-test-dataview

Conversation

@bavulapati
Copy link
Copy Markdown

Ports
https://github.com/nodejs/node/tree/main/test/js-native-api/test_dataview from Node.js test suite to the CTS.
This test contains an experimental feature SharedArrayBuffer, resulting in usage of add_node_api_cts_experimental_addon().

Ports
[https://github.com/nodejs/node/tree/main/test/js-native-api/test_dataview](test_dataview)
from Node.js test suite to the CTS.
This test contains an experimental feature `SharedArrayBuffer`,
resulting in usage of `add_node_api_cts_experimental_addon()`.

Signed-off-by: Balakrishna Avulapati <ba@bavulapati.com>
function(add_node_api_cts_experimental_addon ADDON_NAME)
cmake_parse_arguments(PARSE_ARGV 1 ARG "" "" "SOURCES")
add_node_api_cts_addon(${ADDON_NAME} ${ARG_SOURCES})
add_node_api_cts_addon(${ADDON_NAME} ${ARGN})
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PARSE_ARGV usage isn't picking the sources correctly. We can just do what we do in the add_node_cts_addon.

@kraenhansen
Copy link
Copy Markdown
Contributor

Reviewed: C file is a verbatim copy of upstream, add_node_api_cts_experimental_addon is correct (matching the upstream NAPI_EXPERIMENTAL define), loadAddon is correctly guarded behind experimentalFeatures.sharedArrayBuffer, all upstream test cases preserved. The CMake refactor (dropping cmake_parse_arguments/SOURCES in favour of ${ARGN}) is safe — this PR introduces the first call site. "use strict" tracked in #41.

node_api_is_sharedarraybuffer is added in v24.9.0
https://nodejs.org/api/n-api.html#node-api-is-sharedarraybuffer

Signed-off-by: Balakrishna Avulapati <ba@bavulapati.com>
@bavulapati
Copy link
Copy Markdown
Author

@legendecas Fixed the CI issue. Can we run the tests again?

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

Labels

None yet

Projects

Status: Need Triage

Development

Successfully merging this pull request may close these issues.

2 participants