Skip to content

Conversation

@kraenhansen
Copy link
Collaborator

Stacked on #320.

Merging this PR will:

  • Add a tests directory in the weak-node-api package with two trivial tests using catch2.

@kraenhansen kraenhansen self-assigned this Nov 5, 2025
@kraenhansen kraenhansen added CI Continuous integration weak-node-api labels Nov 5, 2025
@kraenhansen kraenhansen force-pushed the kh/weak-node-api-tests branch 2 times, most recently from 5f097f4 to ec5d060 Compare November 5, 2025 21:34
@kraenhansen kraenhansen marked this pull request as ready for review November 5, 2025 21:57
@kraenhansen kraenhansen requested a review from Copilot November 5, 2025 21:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds C++ unit tests to the weak-node-api package using the Catch2 testing framework. The tests verify the weak API injection mechanism and function call propagation.

Key changes:

  • Introduces Catch2-based C++ tests for the weak Node-API host injection functionality
  • Updates code generation to use C++20 features and improve cross-platform compatibility
  • Adds CI workflow for running C++ tests across multiple platforms

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/weak-node-api/tests/test_inject.cpp Adds two test cases verifying inject_weak_node_api_host functionality
packages/weak-node-api/tests/CMakeLists.txt Configures Catch2 test executable with CTest integration
packages/weak-node-api/scripts/generate-weak-node-api.ts Updates code generation for C++20 compatibility and adds unreachable macro
packages/weak-node-api/CMakeLists.txt Upgrades to C++20 and adds optional test building
packages/weak-node-api/.gitignore Adds clang cache directory to ignore list
.github/workflows/check.yml Adds cross-platform CI job for running weak-node-api tests

- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: lts/jod
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'jod' to 'jodium'.

Copilot uses AI. Check for mistakes.
Comment on lines +72 to 78
returnType === "void" ? "" : "return ",
`g_host.${name}`,
"(",
argumentTypes.map((_, index) => `arg${index}`).join(", "),
");",
noReturn ? "WEAK_NODE_API_UNREACHABLE" : "",
"};",
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The generated code will produce invalid C++ syntax. When noReturn is true, the unreachable macro appears after the semicolon and before the closing brace, creating ); WEAK_NODE_API_UNREACHABLE };. The macro should be placed before the semicolon or on its own line. Consider using ].filter(Boolean).join(" ") and including the semicolon with the return statement, or conditionally building the return line.

Copilot uses AI. Check for mistakes.
@kraenhansen kraenhansen force-pushed the kh/weak-node-api-headers branch from 1aa4d95 to e1b824b Compare November 6, 2025 08:36
@kraenhansen kraenhansen force-pushed the kh/weak-node-api-tests branch from f2f8a02 to 04400fa Compare November 6, 2025 08:39
Base automatically changed from kh/weak-node-api-headers to main November 6, 2025 09:34
@kraenhansen kraenhansen force-pushed the kh/weak-node-api-tests branch from 04400fa to fd171e6 Compare November 6, 2025 09:35
@kraenhansen kraenhansen merged commit 8418503 into main Nov 6, 2025
11 checks passed
@kraenhansen kraenhansen deleted the kh/weak-node-api-tests branch November 6, 2025 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration weak-node-api

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants