Add mock API - #698

Closed
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api
Closed

Add mock API#698
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api

Conversation

@ruyadorno

Copy link
Copy Markdown

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

t.test('testing something, t => {
constmyModule=t.mock('../my-module.js',{fs: {readFileSync: ()=>'foo'}})t.equal(myModule.bar(),'foo','should receive expected content')})

ruyadornoand others added 6 commits October 14, 2020 23:04
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

@isaacsisaacs left a comment

Copy link
Copy Markdown
Member

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.)

Comment threadlib/mock.js Outdated
@coreyfarrell

Copy link
Copy Markdown
Member

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 t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

Not intending to be a blocker on this but I wanted to raise these points.

@ruyadorno

Copy link
Copy Markdown
Author

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.

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 t.mock() cjs usable before jumping down that other rabbit hole.

Another question, what is the benefit of t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

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 t and a very good comparison I can think of is the addition of the Fixtures API (t.testdir, t.fixture, etc over require('tacks') and similars) - it has changed SO MUCH for the better my personal usage that it was the turning point for tap to become my default test runner for every new project - all that to say that my contribution to add t.mock() is also in the hope of steering the project a little bit more in favor of this kind of out-of-the-box complete test runner experience, that to a large extent tap already has by providing coverage out of the box, fixtures, snapshots, etc

Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄

@isaacs

isaacs commented Oct 22, 2020

Copy link
Copy Markdown
Member

Here's how to get the filename of the file that's calling t.mock() so that all the paths can be resolved from there:

// in lib/test.jsclassTest{// ....mock(module,mocks){const{file}=stack.at(Test.prototype.mock)constresolved=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.
@ruyadorno

Copy link
Copy Markdown
Author

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 test/test.js 😊

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.
@ruyadorno

ruyadorno commented Nov 17, 2020

Copy link
Copy Markdown
Author

[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this t.mock implementation and during that process I found some very specific edge cases which I'm currently tackling - I'll make sure to add a ref between the PR with the rewrote mocks for npm and this one once that's ready to go. 😊

@ruyadorno
ruyadornoforce-pushed the add-mock-api branch 2 times, most recently from cafcec4 to 6c5cb81CompareDecember 17, 2020 22:57
@ruyadorno

Copy link
Copy Markdown
Author

Here's the PR that updates the npm cli to use t.mock instead: npm/cli#2370

isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
@isaacsisaacs mentioned this pull request Feb 16, 2021
isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
coreyfarrell pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@coreyfarrell@isaacs
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Add mock API - #698

Closed
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api
Closed

Add mock API#698
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api

Conversation

@ruyadorno

Copy link
Copy Markdown

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

t.test('testing something, t => {
constmyModule=t.mock('../my-module.js',{fs: {readFileSync: ()=>'foo'}})t.equal(myModule.bar(),'foo','should receive expected content')})

ruyadornoand others added 6 commits October 14, 2020 23:04
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

@isaacsisaacs left a comment

Copy link
Copy Markdown
Member

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.)

Comment threadlib/mock.js Outdated
@coreyfarrell

Copy link
Copy Markdown
Member

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 t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

Not intending to be a blocker on this but I wanted to raise these points.

@ruyadorno

Copy link
Copy Markdown
Author

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.

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 t.mock() cjs usable before jumping down that other rabbit hole.

Another question, what is the benefit of t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

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 t and a very good comparison I can think of is the addition of the Fixtures API (t.testdir, t.fixture, etc over require('tacks') and similars) - it has changed SO MUCH for the better my personal usage that it was the turning point for tap to become my default test runner for every new project - all that to say that my contribution to add t.mock() is also in the hope of steering the project a little bit more in favor of this kind of out-of-the-box complete test runner experience, that to a large extent tap already has by providing coverage out of the box, fixtures, snapshots, etc

Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄

@isaacs

isaacs commented Oct 22, 2020

Copy link
Copy Markdown
Member

Here's how to get the filename of the file that's calling t.mock() so that all the paths can be resolved from there:

// in lib/test.jsclassTest{// ....mock(module,mocks){const{file}=stack.at(Test.prototype.mock)constresolved=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.
@ruyadorno

Copy link
Copy Markdown
Author

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 test/test.js 😊

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.
@ruyadorno

ruyadorno commented Nov 17, 2020

Copy link
Copy Markdown
Author

[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this t.mock implementation and during that process I found some very specific edge cases which I'm currently tackling - I'll make sure to add a ref between the PR with the rewrote mocks for npm and this one once that's ready to go. 😊

@ruyadorno
ruyadornoforce-pushed the add-mock-api branch 2 times, most recently from cafcec4 to 6c5cb81CompareDecember 17, 2020 22:57
@ruyadorno

Copy link
Copy Markdown
Author

Here's the PR that updates the npm cli to use t.mock instead: npm/cli#2370

isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
@isaacsisaacs mentioned this pull request Feb 16, 2021
isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
coreyfarrell pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@coreyfarrell@isaacs
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add mock API - #698

Closed
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api
Closed

Add mock API#698
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api

Conversation

@ruyadorno

Copy link
Copy Markdown

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

t.test('testing something, t => {
constmyModule=t.mock('../my-module.js',{fs: {readFileSync: ()=>'foo'}})t.equal(myModule.bar(),'foo','should receive expected content')})

ruyadornoand others added 6 commits October 14, 2020 23:04
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

@isaacsisaacs left a comment

Copy link
Copy Markdown
Member

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.)

Comment threadlib/mock.js Outdated
@coreyfarrell

Copy link
Copy Markdown
Member

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 t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

Not intending to be a blocker on this but I wanted to raise these points.

@ruyadorno

Copy link
Copy Markdown
Author

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.

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 t.mock() cjs usable before jumping down that other rabbit hole.

Another question, what is the benefit of t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

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 t and a very good comparison I can think of is the addition of the Fixtures API (t.testdir, t.fixture, etc over require('tacks') and similars) - it has changed SO MUCH for the better my personal usage that it was the turning point for tap to become my default test runner for every new project - all that to say that my contribution to add t.mock() is also in the hope of steering the project a little bit more in favor of this kind of out-of-the-box complete test runner experience, that to a large extent tap already has by providing coverage out of the box, fixtures, snapshots, etc

Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄

@isaacs

isaacs commented Oct 22, 2020

Copy link
Copy Markdown
Member

Here's how to get the filename of the file that's calling t.mock() so that all the paths can be resolved from there:

// in lib/test.jsclassTest{// ....mock(module,mocks){const{file}=stack.at(Test.prototype.mock)constresolved=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.
@ruyadorno

Copy link
Copy Markdown
Author

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 test/test.js 😊

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.
@ruyadorno

ruyadorno commented Nov 17, 2020

Copy link
Copy Markdown
Author

[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this t.mock implementation and during that process I found some very specific edge cases which I'm currently tackling - I'll make sure to add a ref between the PR with the rewrote mocks for npm and this one once that's ready to go. 😊

@ruyadorno
ruyadornoforce-pushed the add-mock-api branch 2 times, most recently from cafcec4 to 6c5cb81CompareDecember 17, 2020 22:57
@ruyadorno

Copy link
Copy Markdown
Author

Here's the PR that updates the npm cli to use t.mock instead: npm/cli#2370

isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
@isaacsisaacs mentioned this pull request Feb 16, 2021
isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
coreyfarrell pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@coreyfarrell@isaacs
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add mock API - #698

Closed
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api
Closed

Add mock API#698
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api

Conversation

@ruyadorno

Copy link
Copy Markdown

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

t.test('testing something, t => {
constmyModule=t.mock('../my-module.js',{fs: {readFileSync: ()=>'foo'}})t.equal(myModule.bar(),'foo','should receive expected content')})

ruyadornoand others added 6 commits October 14, 2020 23:04
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

@isaacsisaacs left a comment

Copy link
Copy Markdown
Member

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.)

Comment threadlib/mock.js Outdated
@coreyfarrell

Copy link
Copy Markdown
Member

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 t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

Not intending to be a blocker on this but I wanted to raise these points.

@ruyadorno

Copy link
Copy Markdown
Author

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.

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 t.mock() cjs usable before jumping down that other rabbit hole.

Another question, what is the benefit of t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

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 t and a very good comparison I can think of is the addition of the Fixtures API (t.testdir, t.fixture, etc over require('tacks') and similars) - it has changed SO MUCH for the better my personal usage that it was the turning point for tap to become my default test runner for every new project - all that to say that my contribution to add t.mock() is also in the hope of steering the project a little bit more in favor of this kind of out-of-the-box complete test runner experience, that to a large extent tap already has by providing coverage out of the box, fixtures, snapshots, etc

Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄

@isaacs

isaacs commented Oct 22, 2020

Copy link
Copy Markdown
Member

Here's how to get the filename of the file that's calling t.mock() so that all the paths can be resolved from there:

// in lib/test.jsclassTest{// ....mock(module,mocks){const{file}=stack.at(Test.prototype.mock)constresolved=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.
@ruyadorno

Copy link
Copy Markdown
Author

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 test/test.js 😊

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.
@ruyadorno

ruyadorno commented Nov 17, 2020

Copy link
Copy Markdown
Author

[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this t.mock implementation and during that process I found some very specific edge cases which I'm currently tackling - I'll make sure to add a ref between the PR with the rewrote mocks for npm and this one once that's ready to go. 😊

@ruyadorno
ruyadornoforce-pushed the add-mock-api branch 2 times, most recently from cafcec4 to 6c5cb81CompareDecember 17, 2020 22:57
@ruyadorno

Copy link
Copy Markdown
Author

Here's the PR that updates the npm cli to use t.mock instead: npm/cli#2370

isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
@isaacsisaacs mentioned this pull request Feb 16, 2021
isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
coreyfarrell pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@coreyfarrell@isaacs
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Add mock API - #698

Closed
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api
Closed

Add mock API#698
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api

Conversation

@ruyadorno

Copy link
Copy Markdown

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

t.test('testing something, t => {
constmyModule=t.mock('../my-module.js',{fs: {readFileSync: ()=>'foo'}})t.equal(myModule.bar(),'foo','should receive expected content')})

ruyadornoand others added 6 commits October 14, 2020 23:04
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

@isaacsisaacs left a comment

Copy link
Copy Markdown
Member

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.)

Comment threadlib/mock.js Outdated
@coreyfarrell

Copy link
Copy Markdown
Member

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 t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

Not intending to be a blocker on this but I wanted to raise these points.

@ruyadorno

Copy link
Copy Markdown
Author

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.

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 t.mock() cjs usable before jumping down that other rabbit hole.

Another question, what is the benefit of t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

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 t and a very good comparison I can think of is the addition of the Fixtures API (t.testdir, t.fixture, etc over require('tacks') and similars) - it has changed SO MUCH for the better my personal usage that it was the turning point for tap to become my default test runner for every new project - all that to say that my contribution to add t.mock() is also in the hope of steering the project a little bit more in favor of this kind of out-of-the-box complete test runner experience, that to a large extent tap already has by providing coverage out of the box, fixtures, snapshots, etc

Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄

@isaacs

isaacs commented Oct 22, 2020

Copy link
Copy Markdown
Member

Here's how to get the filename of the file that's calling t.mock() so that all the paths can be resolved from there:

// in lib/test.jsclassTest{// ....mock(module,mocks){const{file}=stack.at(Test.prototype.mock)constresolved=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.
@ruyadorno

Copy link
Copy Markdown
Author

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 test/test.js 😊

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.
@ruyadorno

ruyadorno commented Nov 17, 2020

Copy link
Copy Markdown
Author

[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this t.mock implementation and during that process I found some very specific edge cases which I'm currently tackling - I'll make sure to add a ref between the PR with the rewrote mocks for npm and this one once that's ready to go. 😊

@ruyadorno
ruyadornoforce-pushed the add-mock-api branch 2 times, most recently from cafcec4 to 6c5cb81CompareDecember 17, 2020 22:57
@ruyadorno

Copy link
Copy Markdown
Author

Here's the PR that updates the npm cli to use t.mock instead: npm/cli#2370

isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
@isaacsisaacs mentioned this pull request Feb 16, 2021
isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
coreyfarrell pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@coreyfarrell@isaacs
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add mock API - #698

Closed
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api
Closed

Add mock API#698
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api

Conversation

@ruyadorno

Copy link
Copy Markdown

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

t.test('testing something, t => {
constmyModule=t.mock('../my-module.js',{fs: {readFileSync: ()=>'foo'}})t.equal(myModule.bar(),'foo','should receive expected content')})

ruyadornoand others added 6 commits October 14, 2020 23:04
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

@isaacsisaacs left a comment

Copy link
Copy Markdown
Member

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.)

Comment threadlib/mock.js Outdated
@coreyfarrell

Copy link
Copy Markdown
Member

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 t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

Not intending to be a blocker on this but I wanted to raise these points.

@ruyadorno

Copy link
Copy Markdown
Author

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.

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 t.mock() cjs usable before jumping down that other rabbit hole.

Another question, what is the benefit of t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

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 t and a very good comparison I can think of is the addition of the Fixtures API (t.testdir, t.fixture, etc over require('tacks') and similars) - it has changed SO MUCH for the better my personal usage that it was the turning point for tap to become my default test runner for every new project - all that to say that my contribution to add t.mock() is also in the hope of steering the project a little bit more in favor of this kind of out-of-the-box complete test runner experience, that to a large extent tap already has by providing coverage out of the box, fixtures, snapshots, etc

Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄

@isaacs

isaacs commented Oct 22, 2020

Copy link
Copy Markdown
Member

Here's how to get the filename of the file that's calling t.mock() so that all the paths can be resolved from there:

// in lib/test.jsclassTest{// ....mock(module,mocks){const{file}=stack.at(Test.prototype.mock)constresolved=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.
@ruyadorno

Copy link
Copy Markdown
Author

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 test/test.js 😊

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.
@ruyadorno

ruyadorno commented Nov 17, 2020

Copy link
Copy Markdown
Author

[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this t.mock implementation and during that process I found some very specific edge cases which I'm currently tackling - I'll make sure to add a ref between the PR with the rewrote mocks for npm and this one once that's ready to go. 😊

@ruyadorno
ruyadornoforce-pushed the add-mock-api branch 2 times, most recently from cafcec4 to 6c5cb81CompareDecember 17, 2020 22:57
@ruyadorno

Copy link
Copy Markdown
Author

Here's the PR that updates the npm cli to use t.mock instead: npm/cli#2370

isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
@isaacsisaacs mentioned this pull request Feb 16, 2021
isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
coreyfarrell pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@coreyfarrell@isaacs
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add mock API - #698

Closed
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api
Closed

Add mock API#698
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api

Conversation

@ruyadorno

Copy link
Copy Markdown

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

t.test('testing something, t => {
constmyModule=t.mock('../my-module.js',{fs: {readFileSync: ()=>'foo'}})t.equal(myModule.bar(),'foo','should receive expected content')})

ruyadornoand others added 6 commits October 14, 2020 23:04
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

@isaacsisaacs left a comment

Copy link
Copy Markdown
Member

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.)

Comment threadlib/mock.js Outdated
@coreyfarrell

Copy link
Copy Markdown
Member

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 t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

Not intending to be a blocker on this but I wanted to raise these points.

@ruyadorno

Copy link
Copy Markdown
Author

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.

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 t.mock() cjs usable before jumping down that other rabbit hole.

Another question, what is the benefit of t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

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 t and a very good comparison I can think of is the addition of the Fixtures API (t.testdir, t.fixture, etc over require('tacks') and similars) - it has changed SO MUCH for the better my personal usage that it was the turning point for tap to become my default test runner for every new project - all that to say that my contribution to add t.mock() is also in the hope of steering the project a little bit more in favor of this kind of out-of-the-box complete test runner experience, that to a large extent tap already has by providing coverage out of the box, fixtures, snapshots, etc

Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄

@isaacs

isaacs commented Oct 22, 2020

Copy link
Copy Markdown
Member

Here's how to get the filename of the file that's calling t.mock() so that all the paths can be resolved from there:

// in lib/test.jsclassTest{// ....mock(module,mocks){const{file}=stack.at(Test.prototype.mock)constresolved=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.
@ruyadorno

Copy link
Copy Markdown
Author

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 test/test.js 😊

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.
@ruyadorno

ruyadorno commented Nov 17, 2020

Copy link
Copy Markdown
Author

[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this t.mock implementation and during that process I found some very specific edge cases which I'm currently tackling - I'll make sure to add a ref between the PR with the rewrote mocks for npm and this one once that's ready to go. 😊

@ruyadorno
ruyadornoforce-pushed the add-mock-api branch 2 times, most recently from cafcec4 to 6c5cb81CompareDecember 17, 2020 22:57
@ruyadorno

Copy link
Copy Markdown
Author

Here's the PR that updates the npm cli to use t.mock instead: npm/cli#2370

isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
@isaacsisaacs mentioned this pull request Feb 16, 2021
isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
coreyfarrell pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@coreyfarrell@isaacs
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Add mock API - #698

Closed
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api
Closed

Add mock API#698
ruyadorno wants to merge 22 commits into
tapjs:masterfrom
ruyadorno:add-mock-api

Conversation

@ruyadorno

Copy link
Copy Markdown

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

t.test('testing something, t => {
constmyModule=t.mock('../my-module.js',{fs: {readFileSync: ()=>'foo'}})t.equal(myModule.bar(),'foo','should receive expected content')})

ruyadornoand others added 6 commits October 14, 2020 23:04
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

@isaacsisaacs left a comment

Copy link
Copy Markdown
Member

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.)

Comment threadlib/mock.js Outdated
@coreyfarrell

Copy link
Copy Markdown
Member

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 t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

Not intending to be a blocker on this but I wanted to raise these points.

@ruyadorno

Copy link
Copy Markdown
Author

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.

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 t.mock() cjs usable before jumping down that other rabbit hole.

Another question, what is the benefit of t.mock over packaging this as an auxiliary module where users would require('tap-mock')? Even if tap has t.mock = require('tap-mock') my bias tends towards isolating stuff into smaller modules as I find they get tested better.

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 t and a very good comparison I can think of is the addition of the Fixtures API (t.testdir, t.fixture, etc over require('tacks') and similars) - it has changed SO MUCH for the better my personal usage that it was the turning point for tap to become my default test runner for every new project - all that to say that my contribution to add t.mock() is also in the hope of steering the project a little bit more in favor of this kind of out-of-the-box complete test runner experience, that to a large extent tap already has by providing coverage out of the box, fixtures, snapshots, etc

Hope that makes sense to you @coreyfarrell and thanks, I appreciate your feedback 😄

@isaacs

isaacs commented Oct 22, 2020

Copy link
Copy Markdown
Member

Here's how to get the filename of the file that's calling t.mock() so that all the paths can be resolved from there:

// in lib/test.jsclassTest{// ....mock(module,mocks){const{file}=stack.at(Test.prototype.mock)constresolved=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.
@ruyadorno

Copy link
Copy Markdown
Author

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 test/test.js 😊

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.
@ruyadorno

ruyadorno commented Nov 17, 2020

Copy link
Copy Markdown
Author

[update] I'm in the process of replacing all mock usage within the https://github.com/npm/cli codebase with this t.mock implementation and during that process I found some very specific edge cases which I'm currently tackling - I'll make sure to add a ref between the PR with the rewrote mocks for npm and this one once that's ready to go. 😊

@ruyadorno
ruyadornoforce-pushed the add-mock-api branch 2 times, most recently from cafcec4 to 6c5cb81CompareDecember 17, 2020 22:57
@ruyadorno

Copy link
Copy Markdown
Author

Here's the PR that updates the npm cli to use t.mock instead: npm/cli#2370

isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
@isaacsisaacs mentioned this pull request Feb 16, 2021
isaacs pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
coreyfarrell pushed a commit to tapjs/libtap that referenced this pull request Feb 16, 2021
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#698Closes: tapjs/tapjs#698
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ruyadorno@coreyfarrell@isaacs