test: fix flaky test-https-agent-create-connection#11649
test: fix flaky test-https-agent-create-connection#11649santigimeno merged 1 commit intonodejs:masterfrom
Conversation
cc90ab4 to
a60859e
Compare
|
This seems reasonable, but what is the problem with reusing the same port (i.e. do you know why the current version of the test is failing)? |
|
@gibfahn I guess if the first test finishes before the 2nd has connected, the server will be closed, thus the |
|
Assume we'd like this to land sooner than later due to the failures its causing in the builds. CI run: https://ci.nodejs.org/job/node-test-pull-request/6659/ |
|
Only CI failure is unrelated build failure on a Raspberry Pi. |
Trott
left a comment
There was a problem hiding this comment.
LGTM. I'm definitely +1 on making this an exception to the 48-hour rule and landing it much sooner.
|
Stress tests are probably in order, so here we go: Current master: https://ci.nodejs.org/job/node-stress-single-test/1111/nodes=centos5-32/console This PR: https://ci.nodejs.org/job/node-stress-single-test/nodes=centos5-32/1112/console |
|
+1 to landing sooner than 48 hours (as long as the stress test looks okay). |
|
Straightforward stress test didn't fail on master, so I did some shenanigans to increase load (run 4 tests at once) and that seems to have worked to reproduce the problem, although only 1 in 100 times. https://ci.nodejs.org/job/node-stress-single-test/1113/nodes=centos5-32/console So, now I'm going to bump it up to 8 parallel tests and do the same for this PR and compare the results. |
|
Master branch with this test running 8 instances in parallel: https://ci.nodejs.org/job/node-stress-single-test/1114/nodes=centos5-32/console This PR with this test running 8 instances in parallel: https://ci.nodejs.org/job/node-stress-single-test/1115/nodes=centos5-32/console |
|
Last stress test failed 100 out of 100 times on master, passed 100 out of 100 times on this PR. |
Use a different server instance for every test to avoid that they reuse the same port. PR-URL: nodejs#11649 Fixes: nodejs#11644 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
a60859e to
5269e95
Compare
|
Landed in 5269e95. Thanks! |
Use a different server instance for every test to avoid that they reuse the same port. PR-URL: #11649 Fixes: #11644 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Use a different server instance for every test to avoid that they reuse the same port. PR-URL: #11649 Fixes: #11644 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Use a different server instance for every test to avoid that they reuse the same port. PR-URL: #11649 Fixes: #11644 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Use a different server instance for every test to avoid that they reuse the same port. PR-URL: nodejs/node#11649 Fixes: nodejs/node#11644 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Use a different server instance for every test to avoid that they reuse
the same port.
Fixes: #11644
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test