feat: allow passing a function to enableNetConnect()
#1889
Conversation
| */ | ||
| function enableNetConnect(matcher) { | ||
| // TODO-12.x: Replace with `typeof matcher === 'string'`. | ||
| if (_.isString(matcher)) { | ||
| allowNetConnect = new RegExp(matcher) | ||
| } else if (matcher instanceof RegExp) { | ||
| allowNetConnect = matcher | ||
| } else if (matcher instanceof Function) { | ||
| allowNetConnect = { test: matcher } |
There was a problem hiding this comment.
I could also make the internal variable a function, but this seemed quite elegant - let me know if you want me to change this.
tests/test_net_connect.js
Outdated
| @@ -19,8 +19,7 @@ describe('`disableNetConnect()`', () => { | |||
|
|
|||
| await assertRejects( | |||
| got('https://other.example.test/'), | |||
| Error, | |||
| 'Nock: Disallowed net connect for "other.example.test:443/"' | |||
There was a problem hiding this comment.
This is the bug I mentioned in the PR description - this last parameter is used for displaying a message when the assertion fails, not to check the error itself.
There was a problem hiding this comment.
This looks great! Nock already has a big API so it’s worth considering carefully before adding even more, though I think this is one is worth it! If we do change the way we handle configuring the unmocked behavior, this might change, though using a function for this seems like the nicest way to express more complicated scenarios than anything else we’ve seen.
|
|
||
| await got('https://example.test/').catch(() => undefined) // ignore rejection, expected | ||
|
|
||
| expect(matcher).to.have.been.calledOnceWithExactly('example.test:443') |
As mentioned in #1889, it appears that `assertRejects` is not currently used correctly - the third parameter is the message that the assertion emits if it fails, not a matcher for the exception text. This PR fixes all usages of `assertRejects` to correct expect on the error message.
paulmelnikow
changed the title
enableNetConnect()
|
Should we send this out as a minor version, or merge it into |
|
nvm, I just saw this comment #1896 (comment) I'll update the base branch and bring it up to date. |
|
🎉 This PR is included in version 11.9.0-beta.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* feat(1861): allow passing a function to enableNetConnect * Add test to cover passed parameters
* feat(1861): allow passing a function to enableNetConnect * Add test to cover passed parameters
* feat(1861): allow passing a function to enableNetConnect * Add test to cover passed parameters
* feat(1861): allow passing a function to enableNetConnect * Add test to cover passed parameters
* feat(1861): allow passing a function to enableNetConnect * Add test to cover passed parameters
* feat(1861): allow passing a function to enableNetConnect * Add test to cover passed parameters
* feat(1861): allow passing a function to enableNetConnect * Add test to cover passed parameters
Fixes #1861
I've also uncovered a bug in the usage of
assertRejects- the third parameter is not used by the library to match on the error message, but rather the message to print when the expected rejection does not happen. I will create a separate PR to fix all usages ofassertRejectsin the project.