-
-
Notifications
You must be signed in to change notification settings - Fork 272
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
Add mock API #698
Add mock API #698
Conversation
Building from the work started by @nlf ref: https://gist.github.com/nlf/52ca6adab49e5b3939ba37c7f0fc51c6 Co-authored-by: nlf <quitlahok@gmail.com>
- Added param checks and throws TypeErrors on unexpected usage - Simplified a bit some of the logic aroung builtin modules cache - Fixed some errors introduced while porting the original work from @nlf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this! Only comment is that the module and the mock keys should be modules relative to the file calling t.mock()
, not tap
itself. (Basically, should work the same as require-inject
in that way, since while it does have a bit of a learning curve, it's pretty easy to grok and harder to get wrong, once it is learned.)
I'm a bit concerned with having something in tap that is dependent on tested code being CJS. I think a warning that this will not work on ESM modules needs to be added to the documentation. Another question, what is the benefit of Not intending to be a blocker on this but I wanted to raise these points. |
sure! I do want to have esm support too and would be willing to follow up with the work to get it there but I guess esm has a bunch of different discussions such as supporting only dynamic imports, etc and I would love to get
I guess this is the very opinionated and personal-preference aspect of the developer experience involved around using the test runner, I for one much prefer the DX of having these essential built-in features right there in Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄 |
Here's how to get the filename of the file that's calling // in lib/test.js
class Test {
// ....
mock (module, mocks) {
const {file} = stack.at(Test.prototype.mock)
const resolved = path.resolve(file)
// console.error(resolved)
// then resolve module paths from there, pass to the Mock object, etc.
}
// ....
} EDIT: updated because I noticed that stack-utils has a helper method that does all of this, in a slightly more efficient way, setting the stackTraceLimit to 1 since we only need a single frame. |
The mock keys should now be paths relative to the current script/tests that is defining it.
hey @isaacs I took the time to implement the suggested changes and clean things up over the weekend - also added many variations of the scenarios we chatted about to make sure module resolution works as intended in |
216ccb6
to
0124de8
Compare
Add support to modules that instant executing a required module. require('./something')() This mock-fn-as-callback-style was used in test found in npm/cli: t.test('mock as callback style', t => { t.mock('../my-module.js', { '../sub-module.js': arg => { t.equal(arg, 'expected') t.end() } }) }) Also added more varied situations to stress test the module replacement, such as using t.mock within one of the defined mocks, require calls at execution time along with tests for the mock as callback style.
0124de8
to
51f45f5
Compare
[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this |
886d954
to
3745cb2
Compare
6359223
to
c196db8
Compare
5975cb4
to
7f6245c
Compare
ccc1f25
to
5980b7b
Compare
a76c9a1
to
8af0892
Compare
cafcec4
to
6c5cb81
Compare
Here's the PR that updates the npm cli to use |
This brings into **tap** the standard mocking system we have been using across the ecosystem of packages from the npm cli which consists into hijacking the `require` calls from a given module and defining whatever mock we want via something as simple as a key/value object. It's a very conscious decision to make it a very opinionated API, as stated in https://github.com/tapjs/node-tap#tutti-i-gusti-sono-gusti - focusing only on the pattern that have been the standard way we handle mocks. It builds on the initial draft work from @nlf (ref: https://gist.github.com/nlf/52ca6adab49e5b3939ba37c7f0fc51c6) and initial brainstorming of such an API with @mikemimik - thanks ❤️ Example: ```js t.test('testing something, t => { const myModule = t.mock('../my-module.js', { fs: { readFileSync: () => 'foo' } }) t.equal(myModule.bar(), 'foo', 'should receive expected content') }) ``` Credit: @ruyadorno, @nlf Reviewed-by: @isaacs PR-URL: tapjs/tapjs#698 Closes: tapjs/tapjs#698
This brings into **tap** the standard mocking system we have been using across the ecosystem of packages from the npm cli which consists into hijacking the `require` calls from a given module and defining whatever mock we want via something as simple as a key/value object. It's a very conscious decision to make it a very opinionated API, as stated in https://github.com/tapjs/node-tap#tutti-i-gusti-sono-gusti - focusing only on the pattern that have been the standard way we handle mocks. It builds on the initial draft work from @nlf (ref: https://gist.github.com/nlf/52ca6adab49e5b3939ba37c7f0fc51c6) and initial brainstorming of such an API with @mikemimik - thanks ❤️ Example: ```js t.test('testing something, t => { const myModule = t.mock('../my-module.js', { fs: { readFileSync: () => 'foo' } }) t.equal(myModule.bar(), 'foo', 'should receive expected content') }) ``` Credit: @ruyadorno, @nlf Reviewed-by: @isaacs PR-URL: tapjs/tapjs#698 Closes: tapjs/tapjs#698
This brings into **tap** the standard mocking system we have been using across the ecosystem of packages from the npm cli which consists into hijacking the `require` calls from a given module and defining whatever mock we want via something as simple as a key/value object. It's a very conscious decision to make it a very opinionated API, as stated in https://github.com/tapjs/node-tap#tutti-i-gusti-sono-gusti - focusing only on the pattern that have been the standard way we handle mocks. It builds on the initial draft work from @nlf (ref: https://gist.github.com/nlf/52ca6adab49e5b3939ba37c7f0fc51c6) and initial brainstorming of such an API with @mikemimik - thanks ❤️ Example: ```js t.test('testing something, t => { const myModule = t.mock('../my-module.js', { fs: { readFileSync: () => 'foo' } }) t.equal(myModule.bar(), 'foo', 'should receive expected content') }) ``` Credit: @ruyadorno, @nlf Reviewed-by: @isaacs PR-URL: tapjs/tapjs#698 Closes: tapjs/tapjs#698
This brings into tap the standard mocking system we have been using across the ecosystem of packages from the npm cli which consists into hijacking the
require
calls from a given module and defining whatever mock we want via something as simple as a key/value object.It's a very conscious decision to make it a very opinionated API, as stated in https://github.com/tapjs/node-tap#tutti-i-gusti-sono-gusti - focusing only on the pattern that have been the standard way we handle mocks.
It builds on the initial draft work from @nlf (ref: https://gist.github.com/nlf/52ca6adab49e5b3939ba37c7f0fc51c6) and initial brainstorming of such an API with @mikemimik - thanks ❤️
Example