Skip to content

port test_string to CTS#44

Open
bavulapati wants to merge 3 commits intonodejs:mainfrom
bavulapati:feat/port-test-string
Open

port test_string to CTS#44
bavulapati wants to merge 3 commits intonodejs:mainfrom
bavulapati:feat/port-test-string

Conversation

@bavulapati
Copy link
Copy Markdown
Contributor

Ports
test_string from the Node.js test suite to the CTS.

  • Most of the files are identical except for the js files and CMakeLists.txt
  • js files use global assert and loadAddon instead of requiring modules
  • CMakeLists.txt sets NAPI_VERSION >= 10, as API like node_api_create_external_string_latin1 are available only from NAPI_VERSION >= 10. Ref
    docs

Ports
[test_string](https://github.com/nodejs/node/tree/main/test/js-native-api/test_string)
from the Node.js test suite to the CTS.
- Most of the files are identical except for the js files and
CMakeLists.txt
- js files use global assert and loadAddon instead of requiring modules
- CMakeLists.txt sets NAPI_VERSION >= 10, as API like
node_api_create_external_string_latin1 are available only from
NAPI_VERSION >= 10. Ref
[docs](https://nodejs.org/docs/latest/api/n-api.html#node-api-create-external-string-latin1)

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

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

// test_string addon requires Node-API version >= 10
// (node_api_create_external_string_latin1/utf16).
// Node-API 10 is supported in Node.js >= 22.14.0 and >= 23.6.0.
if (Number(napiVersion) < 10) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we split the API tests requiring node_api_create_external_string_latin1/utf16 to a separate test, intead of skipping the whole test?

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