test: reach res._dump after abort ClientRequest #24191
Conversation
addaleax
added
http
tadhgcreedon
force-pushed
the
http-client-request-abort-response-dump
branch
from
73a1820
to
4359904
Compare
|
@tadhgcreedon I’m not sure, but would it maybe make sense to test both cases here, e.g. by creating a new test file rather than modifying an existing one? I think this might be affecting the previous functionality of the test… (/cc @nodejs/http) |
|
Yeah, a separate new test file may be better |
|
Sure, makes sense. Will create a new test file then update. |
tadhgcreedon
force-pushed
the
http-client-request-abort-response-dump
branch
from
3179181
to
1a544be
Compare
|
Hi @tadhgcreedon, the author of the commits in this PR is not registered on GitHub (steps defined here) Either:
|
tadhgcreedon
force-pushed
the
http-client-request-abort-response-dump
branch
from
1a544be
to
c37a91d
Compare
|
@addaleax LGTY? |
|
|
||
| req.end(); | ||
| req.abort(); | ||
| req.abort(); |
There was a problem hiding this comment.
Havn't checked this completely, but two req.aborts are intentional?
There was a problem hiding this comment.
/ping @tadhgcreedon Two abort() calls like that are intentional? If so, great. If not, can you remove one? Or I can totally remove one. Either way. I just want to see this get merged....
|
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19013/ |
PR-URL: nodejs#24191 Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Rich Trott <[email protected]>
|
Landed in f5b8853. Thanks for the contribution! 🎉 (If you're interested in other possible contributions to Node.js but don't have a good idea of where to start looking, some ideas are posted at https://www.nodetodo.org/next-steps/.) |
PR-URL: #24191 Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#24191 Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #24191 Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #24191 Reviewed-By: Ouyang Yadong <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Adds test coverage to
lib/_http_client.jsby reaching a line of code inside a callback that gets registered on the'response'event after aborting a client request, by emitting that event.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes