-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: add test for socket.end callback #24087
Conversation
LGTM but maybe it's redundant because it's probably already covered by |
@lpinca Thank you for your review. You're right. This test is the same as below However, I think this test will ensure that |
@lpinca By the way I looked at #19241 and just realized the need to consume socket data. https://nodejs.org/api/http.html#http_class_http_clientrequest
|
@Ajido that's how |
@lpinca Gotcha. Certainly it was explained already in |
Anyone else's approval? |
@jasnell Thanks! CI jobs has failed in step not related to this PR. Can you please run CI again?
|
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/18969/ |
Landed in 628f955 |
PR-URL: nodejs#24087 Refs: nodejs#23937 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott Many thanks! 😄 |
PR-URL: nodejs#24087 Refs: nodejs#23937 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This test checks the callback of socket.end is called.
Refs: #23937
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes