Skip to content

test_runner: add exports option to mock.module#61727

Open
Han5991 wants to merge 4 commits intonodejs:mainfrom
Han5991:test-runner-mock-exports
Open

test_runner: add exports option to mock.module#61727
Han5991 wants to merge 4 commits intonodejs:mainfrom
Han5991:test-runner-mock-exports

Conversation

@Han5991
Copy link
Contributor

@Han5991 Han5991 commented Feb 8, 2026 •
edited
Loading

Summary

  • add mock.module(..., { exports }) and normalize option shapes through a shared exports path
  • keep defaultExport and namedExports as aliases, and emit runtime DeprecationWarning when legacy options are used
  • update docs, tests, and output fixtures/snapshots to reflect deprecation signaling and the new preferred option shape

Legacy Option Mixing Behavior

  • Mixed usage of options.exports with legacy options is currently allowed.
  • Values are normalized/merged into the same internal exports structure.
  • Legacy options are deprecated, but still supported for compatibility during migration.

Scope

Testing

  • make lint-js
  • python3 tools/test.py test/parallel/test-runner-module-mocking.js
  • python3 tools/test.py test/test-runner/test-output-coverage-with-mock.mjs test/test-runner/test-output-typescript-coverage.mjs

Refs: #58443

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 8, 2026

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 Feb 8, 2026
@Han5991 Han5991 marked this pull request as draft February 8, 2026 00:27
@Han5991 Han5991 force-pushed the test-runner-mock-exports branch from 3d95c0c to fa99ada Compare February 8, 2026 00:35
@JakobJingleheimer
Copy link
Member

JakobJingleheimer commented Feb 8, 2026 •
edited
Loading

Related: #58443

(I haven't read through this PR yet though)

@Han5991
Copy link
Contributor Author

Han5991 commented Feb 8, 2026

Related: #58443

(I haven't read through this PR yet though)

@JakobJingleheimer

I was working on it with that in mind.
I'm planning to proceed only up to Step 2 of the plan. Is that okay?

@Han5991 Han5991 force-pushed the test-runner-mock-exports branch from 73c4cf1 to 715c12c Compare February 8, 2026 01:11
@JakobJingleheimer
Copy link
Member

JakobJingleheimer commented Feb 8, 2026 •
edited
Loading

Yes, sounds good go me!

It would be ideal (but not required or anything) to land this with a userland migration ready to go. I think it could be done fairly easily.

Cc @Ceres6 this should make your Jest → node:test migration easier 🙂

@Han5991
Copy link
Contributor Author

Han5991 commented Feb 8, 2026

Thanks for the feedback. I will scope this PR to Step 2 of the plan: introduce options.exports, keep defaultExport/namedExports as aliases (with deprecation signaling), and defer userland migration + removal to follow-up work.

@Han5991 Han5991 marked this pull request as ready for review February 8, 2026 01:33
@codecov
Copy link

codecov bot commented Feb 8, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.74%. Comparing base (0777dcc) to head (60f2d68).
⚠️ Report is 130 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #61727      +/-   ##
==========================================
+ Coverage   89.73%   89.74%   +0.01%     
==========================================
  Files         675      676       +1     
  Lines      204525   206112    +1587     
  Branches    39310    39525     +215     
==========================================
+ Hits       183529   184974    +1445     
- Misses      13287    13305      +18     
- Partials     7709     7833     +124
Files with missing lines Coverage Δ
lib/internal/test_runner/mock/loader.js 99.35% <100.00%> (ø)
lib/internal/test_runner/mock/mock.js 98.78% <100.00%> (+0.05%) ⬆️

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

@Han5991 Han5991 force-pushed the test-runner-mock-exports branch from 715c12c to 424675e Compare February 8, 2026 02:47
@Han5991
test_runner: add exports option for module mocks
0498de3 
Add options.exports support in mock.module() and normalize option

shapes through a shared exports path.

Keep defaultExport and namedExports as aliases, emit runtime

deprecation warnings for legacy options, and update docs and tests,

including output fixtures and coverage snapshots.

Refs: nodejs#58443
@Han5991 Han5991 force-pushed the test-runner-mock-exports branch from 424675e to 0498de3 Compare February 8, 2026 07:24
JakobJingleheimer
JakobJingleheimer reviewed Feb 25, 2026
View reviewed changes
Copy link
Member

@JakobJingleheimer JakobJingleheimer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for picking this up!

lib/internal/test_runner/mock/mock.js
Comment on lines +826 to +829
if ('exports' in options) {
validateObject(options.exports, 'options.exports');
copyOwnProperties(options.exports, moduleExports);
}
Copy link
Member

@JakobJingleheimer JakobJingleheimer Feb 25, 2026

Choose a reason for hiding this comment

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

Since last-wins, I think the "new" should win (and be moved below namedExports and defaultExport copying).

Or maybe when exports exists, the others should be ignored? I feel like there's some kind of back-version compatibility thing here. @ljharb thoughts?

@Han5991
doc: improve mock.module example clarity
f956d87 
Rename the named export in the mock.module() example from `fn`
to `foo` for improved readability.
@Han5991
test_runner: simplify mock.module normalization
13b13b8 
- Unify internal export representation into a single `moduleExports`
  object, removing the split into `defaultExport`, `hasDefaultExport`,
  and `namedExports`.
- DRY up `emitLegacyMockOptionWarning` with a lookup map instead of a
  switch statement with separate flag variables.
- Use `ObjectDefineProperty` for `defaultExport` option to be
  consistent with descriptor copying in `namedExports`.
JakobJingleheimer
JakobJingleheimer reviewed Feb 26, 2026
View reviewed changes
Copy link
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.

I think this is looking pretty good :)

avivkeller
avivkeller reviewed Feb 26, 2026
View reviewed changes
avivkeller
avivkeller reviewed Feb 26, 2026
View reviewed changes
@Han5991
test_runner: improve mock.module options handling
60f2d68 
This commit addresses several feedback items for mock.module():

- Fixes a potential prototype pollution vulnerability by using
  ObjectAssign({ __proto__: null }, ...) for property descriptors.
- Migrates legacy option warnings to use the internal
  deprecateProperty() utility for better consistency.
- Updates documentation to use "a later version" instead of "a future
  major release" for deprecation timelines, as the feature is
  experimental.
- Adds comprehensive test cases to ensure getter properties in
  mock options are correctly preserved across both module systems.
@Han5991
Copy link
Contributor Author

Han5991 commented Mar 2, 2026

@JakobJingleheimer,

I'd like to help with the userland-migration tool for mock.module.

If you approve, I can implement the migration script in short order to help users move to the new exports shape. Should I contribute it to https://github.com/nodejs/userland-migrations?

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

Reviewers

@jasnell jasnell jasnell approved these changes

@avivkeller avivkeller Awaiting requested review from avivkeller

@JakobJingleheimer JakobJingleheimer Awaiting requested review from JakobJingleheimer

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Han5991 @nodejs-github-bot @JakobJingleheimer @jasnell @avivkeller