Skip to content

test_runner: mock dual-package with conditional exports#62943

Open
maruthang wants to merge 1 commit intonodejs:mainfrom
maruthang:fix-58231-mock-dual-package
Open

test_runner: mock dual-package with conditional exports#62943
maruthang wants to merge 1 commit intonodejs:mainfrom
maruthang:fix-58231-mock-dual-package

Conversation

@maruthang
Copy link
Copy Markdown

When mock.module() targets a package whose exports field maps
import and require to different files, the ESM resolver and the
CJS resolver disagree on the resolved path. Only the ESM path was
registered in mockMap, so require() of the mocked specifier
bypassed the mock and loaded the real CJS module.

Resolve the specifier through Module._resolveFilename from the
caller's directory in addition to the existing ESM resolution. When
the two paths differ, register the CJS path as a second key in
mockMap and invalidate Module._cache[cjsPath], restoring it on
restore(). Single-resolution packages keep their existing behavior.

Note: I was unable to run the test suite locally (no built out/Release/node available on this Windows host). All four files pass node --check; the resolver-divergence premise was verified empirically against system Node. Looking forward to CI verification.

Fixes: #58231

When `mock.module()` targets a package whose `exports` field maps
`import` and `require` to different files, the ESM resolver and the
CJS resolver disagree on the resolved path. Only the ESM path was
registered in `mockMap`, so `require()` of the mocked specifier
bypassed the mock and loaded the real CJS module.

Resolve the specifier through `Module._resolveFilename` from the
caller's directory in addition to the existing ESM resolution. When
the two paths differ, register the CJS path as a second key in
`mockMap` and invalidate `Module._cache[cjsPath]`, restoring it on
`restore()`. Single-resolution packages keep their existing behavior.

Fixes: nodejs#58231
Signed-off-by: Maruthan G <[email protected]>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Apr 25, 2026
@JakobJingleheimer JakobJingleheimer self-requested a review April 25, 2026 15:49
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2026

Codecov Report

❌ Patch coverage is 82.95455% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.64%. Comparing base (4744070) to head (8c4a8e4).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/test_runner/mock/mock.js 82.95% 13 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62943      +/-   ##
==========================================
- Coverage   89.66%   89.64%   -0.03%     
==========================================
  Files         706      706              
  Lines      219370   219489     +119     
  Branches    42065    42095      +30     
==========================================
+ Hits       196694   196751      +57     
- Misses      14586    14638      +52     
- Partials     8090     8100      +10     
Files with missing lines Coverage Δ
lib/internal/test_runner/mock/mock.js 97.53% <82.95%> (-1.28%) ⬇️

... and 49 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.

Copy link
Copy Markdown
Member

@JakobJingleheimer JakobJingleheimer left a comment

Choose a reason for hiding this comment

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

Thanks for this! The tests is correct, and I see it's passing.

I'll do a pass on the implementation soon.

I can't remember whether http/s specifiers are still supported (I think that was maybe removed?).

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

Labels

needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

test-runner: requireing a mocked dual-package gets the original instead of the mock

3 participants