Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: support module mocking #52848

Merged
merged 2 commits into from
May 19, 2024
Merged

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 5, 2024

This commit adds experimental module mocking to the test runner.

Fixes: #51164

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner
  • @nodejs/web-infra

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 5, 2024
This function is used to mock the exports of ECMAScript modules, CommonJS
modules, and Node.js builtin modules. Any references to the original module
prior to mocking are not impacted. The following example demonstrates how a mock
is created for a module.
Copy link
Member

Choose a reason for hiding this comment

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

The " Any references to the original module prior to mocking are not impacted." part probably needs emphasizing (that users need to dynamically import their function under test and ensure it was not imported (and resolved its imports) prior to t.mock.module )

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

LGTM for experimental, this was fun to play with. TODOs seem reasonable and the ergonomics bother me a bit but code looks good and that can be discussed after landing.

Copy link

@rozzilla rozzilla left a comment

Choose a reason for hiding this comment

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

Super! 🎉

@benjamingr
Copy link
Member

@nodejs/loaders might wanna take a look too

@cjihrig
Copy link
Contributor Author

cjihrig commented May 5, 2024

the ergonomics bother me a bit

What would you like the ergonomics to look like? ESM is kind of restrictive here in ways that CJS is not. Is there a C++ API I can leverage to uncache or change an existing ES module? If so, I would happily use that instead.

lib/test/mock_loader.js Outdated Show resolved Hide resolved
lib/test/mock_loader.js Show resolved Hide resolved
lib/test/mock_loader.js Outdated Show resolved Hide resolved
lib/test/mock_loader.js Outdated Show resolved Hide resolved
lib/test/mock_loader.js Outdated Show resolved Hide resolved
lib/test/mock_loader.js Outdated Show resolved Hide resolved
lib/test/mock_loader.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
doc/api/test.md Outdated Show resolved Hide resolved
@benjamingr
Copy link
Member

What would you like the ergonomics to look like? ESM is kind of restrictive here in ways that CJS is not. Is there a C++ API I can leverage to uncache or change an existing ES module? If so, I would happily use that instead.

In big'ish new features I'm in the camp of "land and iterate" hence why I read then approved this PR. There is time for API bikeshedding later IMO for this sort of active development work.

That said, it would be nice if:

  • The code warned when you .mock a module that was already loaded
  • The code examples showed a more complete example of mocking through a third file (so imports only happen after .mock happened)

I have no better API ideas (other than changing the original module since it's live bindings which is bad and we shouldn't do).

@cjihrig cjihrig added request-ci Add this label to start a Jenkins CI on a PR. labels May 14, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 14, 2024
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label May 16, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 16, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@GeoffreyBooth GeoffreyBooth left a comment

Choose a reason for hiding this comment

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

Great work! I'm impressed that you managed to achieve this without any changes to the modules internals.

Maybe in a future PR we could expose this in a way that other test frameworks could use it too?

doc/api/test.md Outdated Show resolved Hide resolved
@MoLow
Copy link
Member

MoLow commented May 16, 2024

Maybe in a future PR we could expose this in a way that other test frameworks could use it too?

it is already exposed. you can import { mock } from 'node:test' and use a different framework for the actual testing

@cjihrig cjihrig force-pushed the mock-new branch 2 times, most recently from 03734eb to cb1caed Compare May 16, 2024 14:12
@cjihrig
Copy link
Contributor Author

cjihrig commented May 16, 2024

@MoLow I cancelled your CI run because it's going to fail on Windows: https://ci.nodejs.org/job/node-test-binary-windows-js-suites/27754/RUN_SUBSET=3,nodes=win11-arm64-COMPILED_BY-vs2022-arm64/console

This commit adds experimental module mocking to the test runner.
@cjihrig cjihrig added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label May 16, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 16, 2024

@nodejs nodejs deleted a comment from nodejs-github-bot May 16, 2024
@cjihrig cjihrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels May 16, 2024
@cjihrig cjihrig requested a review from MoLow May 16, 2024 23:06
@cjihrig
Copy link
Contributor Author

cjihrig commented May 18, 2024

This needs a re-approval in order to land via the commit queue.

@cjihrig cjihrig added the semver-minor PRs that contain new features and should be released in the next minor version. label May 18, 2024
@GeoffreyBooth GeoffreyBooth added the commit-queue Add this label to land a pull request using GitHub Actions. label May 19, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 19, 2024
@nodejs-github-bot nodejs-github-bot merged commit a619789 into nodejs:main May 19, 2024
62 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in a619789

@cjihrig cjihrig deleted the mock-new branch May 19, 2024 13:32
@hossain666
Copy link

4983546@@

targos pushed a commit that referenced this pull request Jun 1, 2024
This commit adds experimental module mocking to the test runner.

PR-URL: #52848
Fixes: #51164
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
lukins-cz pushed a commit to lukins-cz/OS-Aplet-node that referenced this pull request Jun 1, 2024
This commit adds experimental module mocking to the test runner.

PR-URL: nodejs#52848
Fixes: nodejs#51164
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
RafaelGSS added a commit that referenced this pull request Jun 7, 2024
Notable changes:

buffer:
  * (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
cli:
  * (SEMVER-MINOR) add `NODE_RUN_PACKAGE_JSON_PATH` env (Yagiz Nizipli) #53058
  * (SEMVER-MINOR) add `NODE_RUN_SCRIPT_NAME` env to `node --run` (Yagiz Nizipli) #53032
doc:
  * (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
  * (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
  * add StefanStojanovic to collaborators (StefanStojanovic) #53118
fs:
  * mark recursive cp methods as stable (Théo LUDWIG) #53127
lib:
  * (SEMVER-MINOR) add EventSource Client (Aras Abbasi) #51575
  * (SEMVER-MINOR) replace MessageEvent with undici's (Matthew Aitken) #52370
module:
  * (SEMVER-MINOR) print amount of load time of a cjs module (Vinicius Lourenço) #52213
net:
  * (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
  * (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src,permission:
  * (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
  * (SEMVER-MINOR) add snapshot testing (Colin Ihrig) #53169
  * (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
  * (SEMVER-MINOR) support module mocking (Colin Ihrig) #52848

PR-URL: TODO
@RafaelGSS RafaelGSS mentioned this pull request Jun 7, 2024
RafaelGSS added a commit that referenced this pull request Jun 7, 2024
Notable changes:

buffer:
  * (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
cli:
  * (SEMVER-MINOR) add `NODE_RUN_PACKAGE_JSON_PATH` env (Yagiz Nizipli) #53058
  * (SEMVER-MINOR) add `NODE_RUN_SCRIPT_NAME` env to `node --run` (Yagiz Nizipli) #53032
doc:
  * (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
  * (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
  * add StefanStojanovic to collaborators (StefanStojanovic) #53118
  * add Marco Ippolito to TSC (Rafael Gonzaga) #53008
fs:
  * mark recursive cp methods as stable (Théo LUDWIG) #53127
lib:
  * (SEMVER-MINOR) add EventSource Client (Aras Abbasi) #51575
  * (SEMVER-MINOR) replace MessageEvent with undici's (Matthew Aitken) #52370
module:
  * (SEMVER-MINOR) print amount of load time of a cjs module (Vinicius Lourenço) #52213
net:
  * (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
  * (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src,permission:
  * (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
  * (SEMVER-MINOR) add snapshot testing (Colin Ihrig) #53169
  * (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
  * (SEMVER-MINOR) support module mocking (Colin Ihrig) #52848

PR-URL: #53379
RafaelGSS added a commit that referenced this pull request Jun 10, 2024
Notable changes:

buffer:
  * (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
cli:
  * (SEMVER-MINOR) add `NODE_RUN_PACKAGE_JSON_PATH` env (Yagiz Nizipli) #53058
  * (SEMVER-MINOR) add `NODE_RUN_SCRIPT_NAME` env to `node --run` (Yagiz Nizipli) #53032
doc:
  * (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
  * (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
  * add StefanStojanovic to collaborators (StefanStojanovic) #53118
  * add Marco Ippolito to TSC (Rafael Gonzaga) #53008
fs:
  * mark recursive cp methods as stable (Théo LUDWIG) #53127
lib:
  * (SEMVER-MINOR) add EventSource Client (Aras Abbasi) #51575
  * (SEMVER-MINOR) replace MessageEvent with undici's (Matthew Aitken) #52370
module:
  * (SEMVER-MINOR) print amount of load time of a cjs module (Vinicius Lourenço) #52213
net:
  * (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
  * (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src:
  * (SEMVER-MINOR) traverse parent folders while running `--run` (Yagiz Nizipli) #53154
src,permission:
  * (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
  * (SEMVER-MINOR) add snapshot testing (Colin Ihrig) #53169
  * (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
  * (SEMVER-MINOR) support module mocking (Colin Ihrig) #52848

PR-URL: #53379
RafaelGSS added a commit that referenced this pull request Jun 10, 2024
Notable changes:

buffer:
  * (SEMVER-MINOR) add .bytes() method to Blob (Matthew Aitken) #53221
cli:
  * (SEMVER-MINOR) add `NODE_RUN_PACKAGE_JSON_PATH` env (Yagiz Nizipli) #53058
  * (SEMVER-MINOR) add `NODE_RUN_SCRIPT_NAME` env to `node --run` (Yagiz Nizipli) #53032
doc:
  * (SEMVER-MINOR) add context.assert docs (Colin Ihrig) #53169
  * (SEMVER-MINOR) improve explanation about built-in modules (Joyee Cheung) #52762
  * add StefanStojanovic to collaborators (StefanStojanovic) #53118
  * add Marco Ippolito to TSC (Rafael Gonzaga) #53008
fs:
  * mark recursive cp methods as stable (Théo LUDWIG) #53127
lib:
  * (SEMVER-MINOR) add EventSource Client (Aras Abbasi) #51575
  * (SEMVER-MINOR) replace MessageEvent with undici's (Matthew Aitken) #52370
module:
  * (SEMVER-MINOR) print amount of load time of a cjs module (Vinicius Lourenço) #52213
net:
  * (SEMVER-MINOR) add new net.server.listen tracing channel (Paolo Insogna) #53136
process:
  * (SEMVER-MINOR) add process.getBuiltinModule(id) (Joyee Cheung) #52762
src:
  * (SEMVER-MINOR) traverse parent folders while running `--run` (Yagiz Nizipli) #53154
src,permission:
  * (SEMVER-MINOR) --allow-wasi & prevent WASI exec (Rafael Gonzaga) #53124
test_runner:
  * (SEMVER-MINOR) add snapshot testing (Colin Ihrig) #53169
  * (SEMVER-MINOR) add context.fullName (Colin Ihrig) #53169
  * (SEMVER-MINOR) support module mocking (Colin Ihrig) #52848

PR-URL: #53379
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Module level mocking
9 participants