GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — test_runner: mock dual-package with conditional exports by maruthang · Pull Request #62943 · nodejs/node

Conversation

maruthang commented Apr 25, 2026

Copy link

Copy Markdown

Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          test_runner: mock dual-package with conditional exports

8c4a8e4

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 commented Apr 25, 2026

Copy link

Copy Markdown

Collaborator

Review requested:

@nodejs/test_runner

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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 self-requested a review

April 25, 2026 15:49

codecov Bot commented Apr 25, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JakobJingleheimer reviewed

Apr 25, 2026

View reviewed changes

JakobJingleheimer left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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.

Thanks for this! The test 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?).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

JakobJingleheimer added this to Test Runner Board

Apr 25, 2026

ljharb commented Apr 26, 2026

Copy link

Copy Markdown

Member

It used to be experimental, but i believe it was removed a few majors ago.

JakobJingleheimer reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JakobJingleheimer reviewed

Apr 26, 2026

View reviewed changes

Comment thread

lib/internal/test_runner/mock/mock.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

maruthang commented Apr 30, 2026

Copy link

Copy Markdown

Contributor Author

Confirming what @ljharb said — --experimental-network-imports (and with it http: / https: ESM specifiers) was dropped in PR #53822 (commit 15a94e67b1c, landed in v22.6.0, July 2024). Network specifiers are no longer resolvable.

For this PR specifically, that means the http: / https: branches in resolveAsCJS (lib/internal/test_runner/mock/mock.js:1011-1012) are dead code in practice: the upstream sharedState.moduleLoader.resolveSync(...) call at line 681 errors out for any network specifier well before resolveAsCJS runs at line 708. They were defensive against a case that can't reach the function.

Happy to drop those two checks as a follow-up cleanup commit, or keep them as belt-and-braces defense — your call. The other branches (isBuiltin, node:, data:) all still guard live cases, so I'd leave those.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JakobJingleheimer commented May 2, 2026

Copy link

Copy Markdown

Member

your call

My call is #62943 (comment)

please remove them from

🙂

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          test_runner: drop unsupported http(s) specifier handling in resolveAsCJS

c45255a

avivkeller approved these changes

Jul 31, 2026

View reviewed changes

MoLow approved these changes

Aug 2, 2026

View reviewed changes

JakobJingleheimer approved these changes

Aug 8, 2026

View reviewed changes

JakobJingleheimer left a comment

Copy link

Copy Markdown

Member

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.

🙌 Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

JakobJingleheimer added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 8, 2026

JakobJingleheimer moved this to In Progress in Test Runner Board

Aug 8, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 8, 2026

This comment was marked as outdated.

aduh95 added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label

Aug 17, 2026

This comment was marked as outdated.

nodejs-github-bot commented Aug 20, 2026

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/76084/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

trivikr pushed a commit that referenced this pull request

Aug 21, 2026


          test_runner: mock dual-package with conditional exports

9d94b65

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: #58231
Signed-off-by: Maruthan G <[email protected]>
PR-URL: #62943
Reviewed-By: Aviv Keller <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>

trivikr commented Aug 21, 2026

Copy link

Copy Markdown

Member

Landed in 9d94b65

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

trivikr closed this

Aug 21, 2026

github-project-automation Bot moved this from In Progress to Done in Test Runner Board

Aug 21, 2026

aduh95 pushed a commit that referenced this pull request

Aug 25, 2026


          test_runner: mock dual-package with conditional exports

d384603

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: #58231
Signed-off-by: Maruthan G <[email protected]>
PR-URL: #62943
Reviewed-By: Aviv Keller <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>

aduh95 pushed a commit that referenced this pull request

Aug 25, 2026


          test_runner: mock dual-package with conditional exports

797ef40

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: #58231
Signed-off-by: Maruthan G <[email protected]>
PR-URL: #62943
Reviewed-By: Aviv Keller <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>

github-actions Bot mentioned this pull request

Aug 25, 2026

2026-08-26, Version 26.8.0 (Current) #65551

Merged

aduh95 pushed a commit that referenced this pull request

Aug 27, 2026


          test_runner: mock dual-package with conditional exports

a59febb

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: #58231
Signed-off-by: Maruthan G <[email protected]>
PR-URL: #62943
Reviewed-By: Aviv Keller <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>

github-actions Bot mentioned this pull request

Sep 7, 2026

2026-09-08, Version 24.21.0 'Krypton' (LTS) #65873

Merged

aduh95 pushed a commit that referenced this pull request

Sep 7, 2026


          test_runner: mock dual-package with conditional exports

6f91d09

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: #58231
Signed-off-by: Maruthan G <[email protected]>
PR-URL: #62943
Reviewed-By: Aviv Keller <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>

aduh95 pushed a commit that referenced this pull request

Sep 7, 2026


          test_runner: mock dual-package with conditional exports

339acf4

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: #58231
Signed-off-by: Maruthan G <[email protected]>
PR-URL: #62943
Reviewed-By: Aviv Keller <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>

sloops77 mentioned this pull request

Oct 16, 2026

test(base-contract-io): pass real ethers exports through module mock LFDT-Verii/core#971

Merged

1 of 2 tasks

sloops77 added a commit to LFDT-Verii/core that referenced this pull request

Oct 16, 2026


          test(base-contract-io): pass real ethers exports through module mock (#…

6395aee

…971)

Node 24.21.0 (nodejs/node#62943) makes mock.module() also intercept
require() of dual CJS/ESM packages. The skipped mock-contract suite
replaced ethers with only { Wallet, Contract }, so loading
resyncing-nonce-manager.js threw 'Class extends value undefined'
when extending ethers.NonceManager, failing main's nightly CI since
the runners picked up 24.21.

Signed-off-by: Andres Olave <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>

robobun mentioned this pull request

Nov 10, 2026

bun test: register a module mock under the import path and the require() path of its name oven-sh/bun#48467

Draft

Reviewers

JakobJingleheimer

JakobJingleheimer approved these changes

MoLow

MoLow approved these changes

avivkeller

avivkeller approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

needs-ci

PRs that need a full CI run.

test_runner

Issues and PRs related to the test runner subsystem.

Projects

Test Runner Board

Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

Uh oh!

There was an error while loading. Please reload this page.

8 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

maruthang commented Apr 25, 2026

Uh oh!

nodejs-github-bot commented Apr 25, 2026

Uh oh!

codecov Bot commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JakobJingleheimer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ljharb commented Apr 26, 2026

Uh oh!

Uh oh!

maruthang commented Apr 30, 2026

Uh oh!

JakobJingleheimer commented May 2, 2026

Uh oh!

JakobJingleheimer left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Aug 20, 2026

Uh oh!

trivikr commented Aug 21, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecov Bot commented Apr 25, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

JakobJingleheimer left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.