test_runner: add exports option to mock.module#61727
test_runner: add exports option to mock.module#61727nodejs-github-bot merged 9 commits intonodejs:mainfrom
Conversation
|
Review requested:
|
3d95c0c to
fa99ada
Compare
|
Related: #58443 (I haven't read through this PR yet though) |
I was working on it with that in mind. |
73c4cf1 to
715c12c
Compare
|
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 🙂 |
|
Thanks for the feedback. I will scope this PR to Step 2 of the plan: introduce |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61727 +/- ##
==========================================
- Coverage 91.60% 89.69% -1.91%
==========================================
Files 337 676 +339
Lines 140745 206758 +66013
Branches 21802 39595 +17793
==========================================
+ Hits 128925 185453 +56528
- Misses 11595 13456 +1861
- Partials 225 7849 +7624
🚀 New features to boost your workflow:
|
424675e to
0498de3
Compare
| if ('exports' in options) { | ||
| validateObject(options.exports, 'options.exports'); | ||
| copyOwnProperties(options.exports, moduleExports); | ||
| } |
There was a problem hiding this comment.
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?
JakobJingleheimer
left a comment
There was a problem hiding this comment.
I think this is looking pretty good :)
|
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? |
Heck yes! And yes, that is where to go. @brunocroh may have started. Could you sync with him? I think this would be a great one to get started with 🙂 |
Its almost done, https://github.com/nodejs/userland-migrations/pull/390/changes, I just need add more edge cases and docs for it |
JakobJingleheimer
left a comment
There was a problem hiding this comment.
LGTM aside from the item I'd like some others' opinions on. TC39 and some other conferences are this week, so I'll poke people next week after things are calmer.
Bold the `mock.module()` option compatibility notes in the test API docs. This makes the mutual exclusion between `exports`, `defaultExport`, and `namedExports` easier to notice when reading the module mocking options.
06ba0aa to
2466ad5
Compare
|
The
notable-change
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section. |
Notable change
A An automated migration is available to update user code: https://github.com/nodejs/userland-migrations/tree/main/recipes/mock-module-exports npx codemod @nodejs/mock-module-exports |
|
For the curious, CI is consistently failing on
For win11-arm64-COMPILED_BY-vs2022_clang-arm64 |
|
Landed in 7d1f1b4 |
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: #58443 PR-URL: #61727 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
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: #58443 PR-URL: #61727 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
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: #58443 PR-URL: #61727 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
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 PR-URL: nodejs#61727 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Summary
mock.module(..., { exports })and normalize option shapes through a shared exports pathdefaultExportandnamedExportsas aliases, and emit runtimeDeprecationWarningwhen legacy options are usedLegacy Option Mixing Behavior
options.exportswith legacy options is currently allowed.Scope
defaultExport+namedExportsintoexports#58443 plan.Testing
make lint-jspython3 tools/test.py test/parallel/test-runner-module-mocking.jspython3 tools/test.py test/test-runner/test-output-coverage-with-mock.mjs test/test-runner/test-output-typescript-coverage.mjsRefs: #58443