From de842498faec6bf674acd86adc78258eddc93ada Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 8 Jul 2017 09:05:34 -0700 Subject: [PATCH] test: fix flaky test-https-set-timeout-server Because of a race condition, connection listener may not be invoked if test is run under load. Remove `common.mustCall()` wrapper from the listener. Move the test to `parallel` because it now works under load. Make similar change to http test to keep them in synch even though it is much harder to trigger the race in http. PR-URL: https://github.com/nodejs/node/pull/14134 Fixes: https://github.com/nodejs/node/issues/14133 Reviewed-By: Refael Ackermann Reviewed-By: Benjamin Gruenbaum Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Santiago Gimeno --- test/parallel/test-http-set-timeout-server.js | 7 ++++--- .../test-https-set-timeout-server.js | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) rename test/{sequential => parallel}/test-https-set-timeout-server.js (97%) diff --git a/test/parallel/test-http-set-timeout-server.js b/test/parallel/test-http-set-timeout-server.js index 83fc7dc0462903..4eb41d01f96b26 100644 --- a/test/parallel/test-http-set-timeout-server.js +++ b/test/parallel/test-http-set-timeout-server.js @@ -42,9 +42,10 @@ function run() { } test(function serverTimeout(cb) { - const server = http.createServer(common.mustCall((req, res) => { - // just do nothing, we should get a timeout event. - })); + const server = http.createServer((req, res) => { + // Do nothing. We should get a timeout event. + // Might not be invoked. Do not wrap in common.mustCall(). + }); server.listen(common.mustCall(() => { const s = server.setTimeout(50, common.mustCall((socket) => { socket.destroy(); diff --git a/test/sequential/test-https-set-timeout-server.js b/test/parallel/test-https-set-timeout-server.js similarity index 97% rename from test/sequential/test-https-set-timeout-server.js rename to test/parallel/test-https-set-timeout-server.js index e1b9430583ff39..76425385233dee 100644 --- a/test/sequential/test-https-set-timeout-server.js +++ b/test/parallel/test-https-set-timeout-server.js @@ -54,9 +54,10 @@ function run() { test(function serverTimeout(cb) { const server = https.createServer( serverOptions, - common.mustCall((req, res) => { - // just do nothing, we should get a timeout event. - })); + (req, res) => { + // Do nothing. We should get a timeout event. + // Might not be invoked. Do not wrap in common.mustCall(). + }); server.listen(common.mustCall(() => { const s = server.setTimeout(50, common.mustCall((socket) => { socket.destroy();