From 58a351c8950206aad546e5767eefc826a56a8d00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 5 Nov 2025 15:42:44 +0100 Subject: [PATCH 1/6] Add scaffold for testing --- packages/weak-node-api/.gitignore | 3 +++ packages/weak-node-api/CMakeLists.txt | 6 +++++ packages/weak-node-api/tests/CMakeLists.txt | 24 ++++++++++++++++++++ packages/weak-node-api/tests/test_inject.cpp | 6 +++++ 4 files changed, 39 insertions(+) create mode 100644 packages/weak-node-api/tests/CMakeLists.txt create mode 100644 packages/weak-node-api/tests/test_inject.cpp diff --git a/packages/weak-node-api/.gitignore b/packages/weak-node-api/.gitignore index 30ea0716..5cc9e939 100644 --- a/packages/weak-node-api/.gitignore +++ b/packages/weak-node-api/.gitignore @@ -9,3 +9,6 @@ # Copied from node-api-headers by scripts/copy-node-api-headers.ts /include/ + +# Clang cache +/.cache/ diff --git a/packages/weak-node-api/CMakeLists.txt b/packages/weak-node-api/CMakeLists.txt index e53c71cd..08f030de 100644 --- a/packages/weak-node-api/CMakeLists.txt +++ b/packages/weak-node-api/CMakeLists.txt @@ -45,3 +45,9 @@ target_compile_options(${PROJECT_NAME} PRIVATE $<$:/W4 /WX> $<$>:-Wall -Wextra -Werror> ) + +option(BUILD_TESTS "Build the tests" OFF) +if(BUILD_TESTS) + enable_testing() + add_subdirectory(tests) +endif() diff --git a/packages/weak-node-api/tests/CMakeLists.txt b/packages/weak-node-api/tests/CMakeLists.txt new file mode 100644 index 00000000..05450434 --- /dev/null +++ b/packages/weak-node-api/tests/CMakeLists.txt @@ -0,0 +1,24 @@ +Include(FetchContent) + +FetchContent_Declare( + Catch2 + GIT_REPOSITORY https://github.com/catchorg/Catch2.git + GIT_TAG v3.11.0 +) + +FetchContent_MakeAvailable(Catch2) + +add_executable(weak-node-api-tests + test_inject.cpp +) +target_link_libraries(weak-node-api-tests + PRIVATE + weak-node-api + Catch2::Catch2WithMain +) + +# As per https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake +list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras) +include(CTest) +include(Catch) +catch_discover_tests(weak-node-api-tests) diff --git a/packages/weak-node-api/tests/test_inject.cpp b/packages/weak-node-api/tests/test_inject.cpp new file mode 100644 index 00000000..f5d6d3d6 --- /dev/null +++ b/packages/weak-node-api/tests/test_inject.cpp @@ -0,0 +1,6 @@ +#include +#include + +SECTION("inject_weak_node_api_host") { + TEST_CASE("is callable") { REQUIRE(true); } +} From 9c5f8c1be7d0ba49f08a546f289cb41c6b9bb4c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 5 Nov 2025 16:56:44 +0100 Subject: [PATCH 2/6] Implemented two simple tests --- packages/weak-node-api/tests/CMakeLists.txt | 4 ++++ packages/weak-node-api/tests/test_inject.cpp | 23 ++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/weak-node-api/tests/CMakeLists.txt b/packages/weak-node-api/tests/CMakeLists.txt index 05450434..352e035a 100644 --- a/packages/weak-node-api/tests/CMakeLists.txt +++ b/packages/weak-node-api/tests/CMakeLists.txt @@ -16,6 +16,10 @@ target_link_libraries(weak-node-api-tests weak-node-api Catch2::Catch2WithMain ) +target_compile_definitions(weak-node-api-tests + PRIVATE + NAPI_VERSION=8 +) # As per https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras) diff --git a/packages/weak-node-api/tests/test_inject.cpp b/packages/weak-node-api/tests/test_inject.cpp index f5d6d3d6..e2101c8c 100644 --- a/packages/weak-node-api/tests/test_inject.cpp +++ b/packages/weak-node-api/tests/test_inject.cpp @@ -1,6 +1,25 @@ #include #include -SECTION("inject_weak_node_api_host") { - TEST_CASE("is callable") { REQUIRE(true); } +TEST_CASE("inject_weak_node_api_host") { + SECTION("is callable") { + WeakNodeApiHost host{}; + inject_weak_node_api_host(host); + } + + SECTION("propagates calls to napi_create_object") { + static bool called = false; + auto my_create_object = [](napi_env env, + napi_value *result) -> napi_status { + called = true; + return napi_status::napi_ok; + }; + WeakNodeApiHost host{.napi_create_object = my_create_object}; + inject_weak_node_api_host(host); + + napi_value result; + napi_create_object({}, &result); + + REQUIRE(called); + } } From ebb7b523733f71774db8037fd44d07edb61eda6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 5 Nov 2025 17:05:55 +0100 Subject: [PATCH 3/6] Add test job in the check workflow --- .github/workflows/check.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index f24d9c04..ecff59bd 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -76,6 +76,32 @@ jobs: - run: npm ci - run: npm run bootstrap - run: npm test + weak-node-api-tests: + if: github.ref == 'refs/heads/main' || contains(github.event.pull_request.labels.*.name, 'weak-node-api') + strategy: + fail-fast: false + matrix: + runner: + - ubuntu-latest + - windows-latest + - macos-latest + runs-on: ${{ matrix.runner }} + name: Weak Node-API tests (${{ matrix.runner }}) + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: lts/jod + - run: npm ci + - run: npm run build + - name: Prepare weak-node-api + run: npm run prepare-weak-node-api --workspace weak-node-api + - name: Build and run weak-node-api C++ tests + run: | + cmake -S . -B build -DBUILD_TESTS=ON + cmake --build build + ctest --test-dir build --output-on-failure + working-directory: packages/weak-node-api test-ios: if: github.ref == 'refs/heads/main' || contains(github.event.pull_request.labels.*.name, 'Apple 🍎') name: Test app (iOS) From 93478d3a4e28d748332bcbb6c230f3e209e9fac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 5 Nov 2025 17:58:18 +0100 Subject: [PATCH 4/6] Using the NAPI_NO_RETURN definition instead of __attribute__((noreturn)) --- .../scripts/generate-weak-node-api.ts | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/weak-node-api/scripts/generate-weak-node-api.ts b/packages/weak-node-api/scripts/generate-weak-node-api.ts index b9a8736c..fd908bbe 100644 --- a/packages/weak-node-api/scripts/generate-weak-node-api.ts +++ b/packages/weak-node-api/scripts/generate-weak-node-api.ts @@ -20,11 +20,14 @@ export function generateHeader(functions: FunctionDecl[]) { "#include ", // abort() // Generate the struct of function pointers "struct WeakNodeApiHost {", - ...functions.map( - ({ returnType, noReturn, name, argumentTypes }) => - `${returnType} ${ - noReturn ? " __attribute__((noreturn))" : "" - }(*${name})(${argumentTypes.join(", ")});`, + ...functions.map(({ returnType, noReturn, name, argumentTypes }) => + [ + returnType, + // Using a define from node_api.h + noReturn ? "NAPI_NO_RETURN" : "", + // Signature + `(*${name})(${argumentTypes.join(", ")});`, + ].join(" "), ), "};", "typedef void(*InjectHostFunction)(const WeakNodeApiHost&);", @@ -48,23 +51,24 @@ export function generateSource(functions: FunctionDecl[]) { // Generate function calling into the host ...functions.flatMap(({ returnType, noReturn, name, argumentTypes }) => { return [ - `extern "C" ${returnType} ${ - noReturn ? " __attribute__((noreturn))" : "" - }${name}(${argumentTypes - .map((type, index) => `${type} arg${index}`) - .join(", ")}) {`, + 'extern "C"', + returnType, + noReturn ? "NAPI_NO_RETURN" : "", + name, + "(", + argumentTypes.map((type, index) => `${type} arg${index}`).join(", "), + ") {", `if (g_host.${name} == nullptr) {`, ` fprintf(stderr, "Node-API function '${name}' called before it was injected!\\n");`, " abort();", "}", - (returnType === "void" ? "" : "return ") + - "g_host." + - name + - "(" + - argumentTypes.map((_, index) => `arg${index}`).join(", ") + - ");", + returnType === "void" ? "" : "return ", + `g_host.${name}`, + "(", + argumentTypes.map((_, index) => `arg${index}`).join(", "), + ");", "};", - ]; + ].join(" "); }), ].join("\n"); } From 6c08e57f53905768e61a90cd482cb14b65e2f581 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 5 Nov 2025 22:34:23 +0100 Subject: [PATCH 5/6] Using unreachable instead of noreturn --- .../scripts/generate-weak-node-api.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/weak-node-api/scripts/generate-weak-node-api.ts b/packages/weak-node-api/scripts/generate-weak-node-api.ts index fd908bbe..b47aed27 100644 --- a/packages/weak-node-api/scripts/generate-weak-node-api.ts +++ b/packages/weak-node-api/scripts/generate-weak-node-api.ts @@ -18,13 +18,21 @@ export function generateHeader(functions: FunctionDecl[]) { "#include ", // Node-API "#include ", // fprintf() "#include ", // abort() + "", + // Ideally we would have just used NAPI_NO_RETURN, but + // __declspec(noreturn) (when building with Microsoft Visual C++) cannot be used on members of a struct + // TODO: If we targeted C++23 we could use std::unreachable() + "#if defined(__GNUC__)", + "#define WEAK_NODE_API_UNREACHABLE __builtin_unreachable();", + "#else", + "#define WEAK_NODE_API_UNREACHABLE __assume(0);", + "#endif", + "", // Generate the struct of function pointers "struct WeakNodeApiHost {", - ...functions.map(({ returnType, noReturn, name, argumentTypes }) => + ...functions.map(({ returnType, name, argumentTypes }) => [ returnType, - // Using a define from node_api.h - noReturn ? "NAPI_NO_RETURN" : "", // Signature `(*${name})(${argumentTypes.join(", ")});`, ].join(" "), @@ -49,11 +57,10 @@ export function generateSource(functions: FunctionDecl[]) { "};", ``, // Generate function calling into the host - ...functions.flatMap(({ returnType, noReturn, name, argumentTypes }) => { + ...functions.flatMap(({ returnType, name, argumentTypes, noReturn }) => { return [ 'extern "C"', returnType, - noReturn ? "NAPI_NO_RETURN" : "", name, "(", argumentTypes.map((type, index) => `${type} arg${index}`).join(", "), @@ -67,6 +74,7 @@ export function generateSource(functions: FunctionDecl[]) { "(", argumentTypes.map((_, index) => `arg${index}`).join(", "), ");", + noReturn ? "WEAK_NODE_API_UNREACHABLE" : "", "};", ].join(" "); }), From fd171e68dfc09c9fddc7dba2c43e22d9093cf8dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 5 Nov 2025 22:53:12 +0100 Subject: [PATCH 6/6] Use C++20 --- packages/weak-node-api/CMakeLists.txt | 3 ++- packages/weak-node-api/tests/CMakeLists.txt | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/weak-node-api/CMakeLists.txt b/packages/weak-node-api/CMakeLists.txt index 08f030de..d61630f2 100644 --- a/packages/weak-node-api/CMakeLists.txt +++ b/packages/weak-node-api/CMakeLists.txt @@ -38,7 +38,8 @@ if(APPLE) ) endif() -target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_17) +# C++20 is needed to use designated initializers +target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_20) target_compile_definitions(${PROJECT_NAME} PRIVATE NAPI_VERSION=8) target_compile_options(${PROJECT_NAME} PRIVATE diff --git a/packages/weak-node-api/tests/CMakeLists.txt b/packages/weak-node-api/tests/CMakeLists.txt index 352e035a..89b19f84 100644 --- a/packages/weak-node-api/tests/CMakeLists.txt +++ b/packages/weak-node-api/tests/CMakeLists.txt @@ -16,10 +16,9 @@ target_link_libraries(weak-node-api-tests weak-node-api Catch2::Catch2WithMain ) -target_compile_definitions(weak-node-api-tests - PRIVATE - NAPI_VERSION=8 -) + +target_compile_features(weak-node-api-tests PRIVATE cxx_std_20) +target_compile_definitions(weak-node-api-tests PRIVATE NAPI_VERSION=8) # As per https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md#catchcmake-and-catchaddtestscmake list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras)