Fix web core testing (local and remote)#956
Conversation
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the test script in renderers/web_core/package.json. The test command has been changed from node --test "dist/**/*.test.js" to node --test dist, which simplifies how tests are discovered and executed. All review comments were filtered out as they either explained/validated the change or contained praise, therefore there is no feedback to provide.
| }, | ||
| "test": { | ||
| "command": "node --test \"dist/**/*.test.js\"", | ||
| "command": "node --test dist", |
There was a problem hiding this comment.
The change to node --test dist addresses the issue of npm run test not working, which is a good fix. However, the test:coverage script (line 44) still uses the explicit glob pattern "dist/**/*.test.js".
For consistency and clarity in defining which files are considered tests, it would be beneficial to align these two commands. Consider either updating test:coverage to also use node --test dist (if c8 supports it and it correctly identifies all tests) or, if the glob pattern is preferred for its explicitness, investigate why node --test "dist/**/*.test.js" was not working previously and potentially re-introduce a working explicit pattern for both.
gspencergoog
left a comment
gspencergoog
left a comment
There was a problem hiding this comment.
Thanks for fixing this!
* Fix web core testing * revert changes to move elsewhere * Rename workflow to be more consistent * Create new workflow for web build and test * Rename workflows to be more consistent and less redundant * Fix copyright year
2 participants
This makes
npm run testactually work for web core, and also runs that in the relevant GitHub check.I also renamed the GitHub actions to be more consistent and drop the "build" or "build and test" from the name, since it is redundant: