Skip to content

module: fix coverage of mocked CJS modules imported from ESM#62133

Open
marcopiraccini wants to merge 1 commit intonodejs:mainfrom
marcopiraccini:fix/coverage-mocked-cjs-from-esm
Open

module: fix coverage of mocked CJS modules imported from ESM#62133
marcopiraccini wants to merge 1 commit intonodejs:mainfrom
marcopiraccini:fix/coverage-mocked-cjs-from-esm

Conversation

@marcopiraccini
Copy link
Contributor

When a CommonJS module is mocked via mock.module() and imported from an ESM module, the mocked module incorrectly appears in the coverage report with partial coverage, even though the original source was never executed.

The root cause is in loadCJSModule. It passes filename to compileFunctionForCJSLoader as the V8 resource name.
When the mock loader resolves a mocked module, it appends a ?node-test-mock search param to the URL. However, urlToFilename() strips this param. The coverage filter in shouldSkipFileCoverage then fails to recognize it as a mocked module.

The fix passes the full url (which preserves the ?node-test-mock search param) to compileFunctionForCJSLoader instead of the stripped filename. For non-mocked CJS modules loaded via ESM, the url is file:///path/to/file.cjs which produces the same V8 behavior as passing the bare path

Fixes: #61709

Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Mar 6, 2026
@marcopiraccini marcopiraccini marked this pull request as ready for review March 6, 2026 15:11
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 6, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 6, 2026
@nodejs-github-bot
Copy link
Collaborator

@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.65%. Comparing base (35d3bc8) to head (ec9a9d5).
⚠️ Report is 58 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62133      +/-   ##
==========================================
- Coverage   89.65%   89.65%   -0.01%     
==========================================
  Files         676      676              
  Lines      206231   206410     +179     
  Branches    39505    39530      +25     
==========================================
+ Hits       184898   185049     +151     
- Misses      13463    13475      +12     
- Partials     7870     7886      +16     
Files with missing lines Coverage Δ
lib/internal/modules/esm/translators.js 97.66% <100.00%> (+0.02%) ⬆️

... and 45 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node:test coverage attempts to cover mocked modules

6 participants