node
node copied to clipboard
test_runner: support module detection in module mocks
- Resolves #53634.
The tests in test-runner-module-mocking.js
fail when module detection is enabled; you can see by running the following on current main
:
./node --experimental-test-module-mocks --experimental-require-module --experimental-detect-module \
./test/parallel/test-runner-module-mocking.js
(The first two flags are present already at the top of test-runner-module-mocking.js
.)
I think that this means that the test runner module mocking feature is already broken for anyone trying to use it along with --experimental-detect-module
; and of course this becomes all the more urgent to fix as we hopefully unflag --experimental-detect-module
(https://github.com/nodejs/node/pull/53619).
The issue stems from the module mocking using resolution to determine the format to use for what kind of source to generate for the mock, CommonJS or ESM. The format
property returned from the resolve
hook is optional, so this approach is an unsafe one; it breaks not only when detection is enabled, but also if the user registers a custom resolve
hook that doesn’t return a format
property.
I’ve updated the logic to instead use the format
property returned by the next load
hook in the chain, which is usually Node’s internal defaultLoad
. The format
property is required to be returned from load
hooks, so this should be a safe approach. The only potential complication is that this requires the module to be mocked to actually exist on disk, so that nextLoad
succeeds; it’s unclear to me whether this was an expected requirement previously. (There were no tests for mocking nonexistent modules.) I think it should be acceptable, because generally users would be mocking existing things? If mocking nonexistent modules is a requirement, we will need to take another approach or add a fallback approach for that condition.
cc @cjihrig @nodejs/test_runner