Conversation
5f097f4 to
ec5d060
Compare
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Corrected spelling of 'jod' to 'jodium'.
| returnType === "void" ? "" : "return ", | ||
| `g_host.${name}`, | ||
| "(", | ||
| argumentTypes.map((_, index) => `arg${index}`).join(", "), | ||
| ");", | ||
| noReturn ? "WEAK_NODE_API_UNREACHABLE" : "", | ||
| "};", |
There was a problem hiding this comment.
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.
1aa4d95 to
e1b824b
Compare
f2f8a02 to
04400fa
Compare
04400fa to
fd171e6
Compare
Stacked on #320.
Merging this PR will: