- Notifications
You must be signed in to change notification settings - Fork7
Add test toweak-node-api#321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:kh/weak-node-api-headers
Are you sure you want to change the base?
Conversation
5f097f4 toec5d060CompareThere was a problem hiding this 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 theweak-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 verifyinginject_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 |
CopilotAINov 5, 2025
There was a problem hiding this comment.
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'.
| returnType==="void" ?"" :"return ", | ||
| `g_host.${name}`, | ||
| "(", | ||
| argumentTypes.map((_,index)=>`arg${index}`).join(", "), | ||
| ");", | ||
| noReturn ?"WEAK_NODE_API_UNREACHABLE" :"", | ||
| "};", |
CopilotAINov 5, 2025
There was a problem hiding this comment.
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. WhennoReturn 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 toe1b824bComparef2f8a02 to04400faCompare
Stacked on#320.
Merging this PR will: