Uh oh!
There was an error while loading. Please reload this page.
test_runner: support mocking json modules - #58007
Conversation
Review requested:
|
acca0d8 to
8961496Compare| This function is used to mock the exports of ECMAScript modules, CommonJS modules, JSON modules, and | ||
| Node.js builtin modules. Any references to the original module prior to mocking are not impacted. In | ||
| order to enable module mocking, Node.js must be started with the | ||
| [`--experimental-test-module-mocks`][] command-line flag. |
There was a problem hiding this comment.
The only change here was to add JSON modules,.
There was a problem hiding this comment.
Let's minimize the diff
| This function is used to mock the exports of ECMAScript modules, CommonJS modules, JSON modules, and | |
| Node.js builtin modules. Any references to the original module prior to mocking are not impacted. In | |
| order to enable module mocking, Node.js must be started with the | |
| [`--experimental-test-module-mocks`][] command-line flag. | |
| This function is used to mock the exports of ECMAScript modules, CommonJS modules, JSON | |
| modules, and Node.js builtin modules. Any references to the original module | |
| prior to mocking are not impacted. In order to enable module mocking, Node.js must | |
| be started with the [`--experimental-test-module-mocks`][] command-line flag. |
| case 'json': | ||
| format = 'module'; |
There was a problem hiding this comment.
This is the main change. Creating a nested ternary seemed messy, so I changed this to a switch.
| 'builtin', | ||
| 'commonjs-typescript', | ||
| 'commonjs', | ||
| 'json', |
There was a problem hiding this comment.
Adding 'json', was the only real change here.
Uh oh!
There was an error while loading. Please reload this page.
pmarchini
commented
Apr 24, 2025
Except for the linting issue: LGTM |
JakobJingleheimer
commented
Apr 24, 2025
Got to it a split-second after you. Could you re-approve since now there has to be at least approval on the most recent commit. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #58007 +/- ##
==========================================
+ Coverage 90.27% 90.28% +0.01%
==========================================
Files 630 630 Lines 186158 186172 +14 Branches 36472 36474 +2 ==========================================
+ Hits 168047 168092 +45 + Misses 10976 10964 -12 + Partials 7135 7116 -19
🚀 New features to boost your workflow:
|
nodejs-github-bot
commented
Apr 24, 2025
nodejs-github-bot
commented
Apr 25, 2025
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
| - version: | ||
| - REPLACEME | ||
| pr-url: https://github.com/nodejs/node/pull/58007 | ||
| description: Support JSON modules. |
There was a problem hiding this comment.
nit
| description: Support JSON modules. | |
| description: Add support for JSON modules mocking. |
| This function is used to mock the exports of ECMAScript modules, CommonJS modules, JSON modules, and | ||
| Node.js builtin modules. Any references to the original module prior to mocking are not impacted. In | ||
| order to enable module mocking, Node.js must be started with the | ||
| [`--experimental-test-module-mocks`][] command-line flag. |
There was a problem hiding this comment.
Let's minimize the diff
| This function is used to mock the exports of ECMAScript modules, CommonJS modules, JSON modules, and | |
| Node.js builtin modules. Any references to the original module prior to mocking are not impacted. In | |
| order to enable module mocking, Node.js must be started with the | |
| [`--experimental-test-module-mocks`][] command-line flag. | |
| This function is used to mock the exports of ECMAScript modules, CommonJS modules, JSON | |
| modules, and Node.js builtin modules. Any references to the original module | |
| prior to mocking are not impacted. In order to enable module mocking, Node.js must | |
| be started with the [`--experimental-test-module-mocks`][] command-line flag. |
There was a problem hiding this comment.
nit: we could use an existing fixtures JSON file (e.g. test/fixtures/experimental.json), or at least move/rename it to test/fixtures/simple.json so it can be used by other tests
nodejs-github-bot
commented
Apr 25, 2025
nodejs-github-bot
commented
Apr 26, 2025
nodejs-github-bot
commented
Apr 26, 2025
nodejs-github-bot
commented
Apr 26, 2025
Landed in cf896c3 |
PR-URL: #58007 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #58007 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #58007 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
No description provided.