test: fix flaky test-http-client-get-url #13516
Conversation
| http.get(url.parse(u)); | ||
| http.get(new URL(u)); | ||
| }); | ||
| server.listen(0, common.mustCall(() => { |
There was a problem hiding this comment.
nit: could you add common.localhostIPv4 as arg (just for explicitness)
|
🥇 I took at this file, and totally missed the |
| http.get(new URL(u)); | ||
| }); | ||
| server.listen(0, common.mustCall(() => { | ||
| const u = `http://${common.localhostIPv4}:${server.address().port}/foo?bar`; |
There was a problem hiding this comment.
nit 2: could you parameterize /foo?bar here and line 31
|
Added |
|
/cc @nodejs/testing |
Fixed test-http-client-get-url by waiting on HTTP GET requests to finish before closing the server. Fixes: #13507
Fixed test-http-client-get-url by waiting on HTTP GET requests to finish before closing the server. PR-URL: nodejs#13516 Fixes: nodejs#13507 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
|
Landed in 61adb26 |
Fixed test-http-client-get-url by waiting on HTTP GET requests to finish before closing the server. PR-URL: #13516 Fixes: #13507 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
|
This does not land cleanly in LTS. Please feel free to manually backport. Please also feel free to replace do-not-land if it is being backported |
|
Same as for #12876. @MylesBorins This can't be backported. The test flaked because of multiple Docs also seem to confirm this, URL in Node 6 LTS vs URL in Node 8. Please correct me if I'm wrong and also please set the proper labels because I can't :) |
@sebastianplesciuc AFAICT you're right. So the labels are set right. Thanks for following up. |
Fixed
test-http-client-get-urlby waiting on HTTP GET requests to finish before closing the server.Fixes: #13507
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test