build: use default node test runner with ecmascript modules#148
Conversation
This PR resolves #143 and #144 by updating the dependencies to their ESM-only versions. Changes include: - Add "type": "module" to package.json for ESM support - Convert all source files from CommonJS to ESM imports/exports - Convert all test files to use node:test instead of mocha/sinon/chai - Remove unused mocha, sinon, and chai devDependencies - Remove obsolete .mocharc.json and test/.eslintrc.js configs The @actions/core and @actions/github packages are now ESM-only as of versions 3.0.0 and 9.0.0 respectively. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Restore full test coverage for coverage.js and find-problematic-comments.js using esmock to mock ESM module dependencies (node:child_process, node:fs, @api/codecov). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Use mocks.grep and mocks.coverage directly instead of separate exports. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #148 +/- ##
==========================================
+ Coverage 94.57% 95.65% +1.07%
==========================================
Files 6 12 +6
Lines 369 1610 +1241
==========================================
+ Hits 349 1540 +1191
- Misses 20 70 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WilliamBergamin
left a comment
There was a problem hiding this comment.
LGTM 💯 left some non blocking comments
| mockCore.getInput.mock.mockImplementation((key) => { | ||
| const inputs = { | ||
| codecov_token: 'abcd1234', | ||
| codecov_max_attempts: '1', | ||
| codecov_retry_delay: '10', | ||
| codecov_treat_timeout_as_error: 'true', | ||
| }; | ||
| return inputs[key] ?? ''; | ||
| }); |
| const cov = require('../../src/score_components/coverage'); | ||
| import assert from 'node:assert'; | ||
| import { beforeEach, describe, it, mock } from 'node:test'; | ||
| import esmock from 'esmock'; |
There was a problem hiding this comment.
I'm not sure I'm the biggest fan of this dependency 🤔 Could be nice to find a way to do this natively
There was a problem hiding this comment.
@WilliamBergamin I agree and plan to follow up with an investigation into this. At the moment these changes keep esmock to the following:
node:child_processnode:fs@api/codecov
And we might adjust the src directories to inject these as dependencies for testing instead of direct imports? Will share findings!
There was a problem hiding this comment.
Sorry for the drive by comment, but node:test does have experimental module mocking. It might get the job done for you.
There was a problem hiding this comment.
@cjihrig Ahha no apologies! How else do we learn new things? It's so helpful to know but I'm wanting to merge this at this time!
I'll share changes soon 🔮
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Summary
This PR updates
@actions/coreto v3.0.0 and@actions/githubto v9.0.0, which are now ESM-only packages. To support these dependencies, the codebase has been migrated to ES modules and tests now use thenode:testbuilt-in runner withesmockfor module mocking.Changes:
"type": "module"to package.json for ESM supportnode:testinstead of mocha/sinon/chaiesmockfor ESM module mocking in tests.mocharc.jsonandtest/.eslintrc.jsconfigsCloses #108
Closes #143
Closes #144
Requirements