New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#1836 fixes expected Node behaviour for http.get #1837
Conversation
Hi! Thanks for opening this! Could you add a test for the correct behavior? |
Please check now |
t.equal(reqSpy.called, false) | ||
scope.done() | ||
http.request = origHttpReq | ||
t.end() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I run this test without the change, I get a TypeError. Is that the expected behavior?
Could you clarify how this test exercises the bug from #1836?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added http.request
to the spy function and now the test is failing on assertion instead of TypeError.
The test makes sure http.request
is not called when using http.get
. That fixes #1836
tests/test_common.js
Outdated
res.on('end', () => { | ||
t.equal(reqSpy.called, false) | ||
scope.done() | ||
http.request = origHttpReq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For safety, could you move this cleanup code into a t.on('end', () => {})
callback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, done
Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.1.2 to 4.5.3. - [Release notes](https://github.com/wycats/handlebars.js/releases) - [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md) - [Commits](handlebars-lang/handlebars.js@v4.1.2...v4.5.3) Signed-off-by: dependabot[bot] <support@github.com>
* chore(package): update eslint-plugin-node to version 11.0.0 * chore(package): update lockfile package-lock.json Co-authored-by: Paul Melnikow <github@paulmelnikow.com>
* chore(package): update nyc to version 15.0.0 * chore(package): update lockfile package-lock.json Co-authored-by: Paul Melnikow <github@paulmelnikow.com>
* chore(package): update sinon to version 8.0.0 * chore(package): update lockfile package-lock.json Co-authored-by: Paul Melnikow <github@paulmelnikow.com>
* chore(package): update dtslint to version 2.0.0 * chore(package): update lockfile package-lock.json * Update to 2.0.2 * Update lockfile Co-authored-by: Paul Melnikow <github@paulmelnikow.com>
Fix a regression in 11.7.1 due to Jest having different globals: jestjs/jest#2549. Check functions using typeof instead of instanceof.
60ea3d6
to
628b9d7
Compare
Sorry, I fucked up git when trying to rebase. I made a new clean PR at #1853 Closing this |
Fixes #1836