From 61e6ba2d6c0127c350f18fd67e2682cab00f9cb4 Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Tue, 17 Apr 2018 10:48:29 +0200 Subject: [PATCH 1/2] test: fix flaky http-server-keep-alive-timeout Make sure the connection is not closed until the 3 requests and the `timeout` event are received. Some code refactoring to avoid duplicated code. Fixes: https://github.com/nodejs/node/issues/20013 --- .../test-http-server-keep-alive-timeout.js | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/test/parallel/test-http-server-keep-alive-timeout.js b/test/parallel/test-http-server-keep-alive-timeout.js index 767963fbdc2f38..09b2297ba44b2e 100644 --- a/test/parallel/test-http-server-keep-alive-timeout.js +++ b/test/parallel/test-http-server-keep-alive-timeout.js @@ -18,15 +18,29 @@ function run() { if (fn) fn(run); } -test(function serverEndKeepAliveTimeoutWithPipeline(cb) { +function done(server, socket, cb) { + socket.destroy(); + server.close(); + cb(); +} + +function server_test(with_pipeline, cb) { + let got_all = false; + let timedout = false; const server = http.createServer(common.mustCall((req, res) => { - res.end(); + if (with_pipeline) + res.end(); + if (req.url === '/3') { + got_all = true; + if (timedout) + done(server, req.socket, cb); + } }, 3)); server.setTimeout(500, common.mustCall((socket) => { // End this test and call `run()` for the next test (if any). - socket.destroy(); - server.close(); - cb(); + timedout = true; + if (got_all) + done(server, socket, cb); })); server.keepAliveTimeout = 50; server.listen(0, common.mustCall(() => { @@ -40,26 +54,12 @@ test(function serverEndKeepAliveTimeoutWithPipeline(cb) { c.write('GET /3 HTTP/1.1\r\nHost: localhost\r\n\r\n'); }); })); +} + +test(function serverEndKeepAliveTimeoutWithPipeline(cb) { + server_test(true, cb); }); test(function serverNoEndKeepAliveTimeoutWithPipeline(cb) { - const server = http.createServer(common.mustCall(3)); - server.setTimeout(500, common.mustCall((socket) => { - // End this test and call `run()` for the next test (if any). - socket.destroy(); - server.close(); - cb(); - })); - server.keepAliveTimeout = 50; - server.listen(0, common.mustCall(() => { - const options = { - port: server.address().port, - allowHalfOpen: true - }; - const c = net.connect(options, () => { - c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n'); - c.write('GET /2 HTTP/1.1\r\nHost: localhost\r\n\r\n'); - c.write('GET /3 HTTP/1.1\r\nHost: localhost\r\n\r\n'); - }); - })); + server_test(false, cb); }); From b68bd99a380291646d32ad90a24725c1eee7aa8b Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Tue, 17 Apr 2018 19:02:07 +0200 Subject: [PATCH 2/2] fixup --- test/parallel/test-http-server-keep-alive-timeout.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-http-server-keep-alive-timeout.js b/test/parallel/test-http-server-keep-alive-timeout.js index 09b2297ba44b2e..048291613fde94 100644 --- a/test/parallel/test-http-server-keep-alive-timeout.js +++ b/test/parallel/test-http-server-keep-alive-timeout.js @@ -20,11 +20,10 @@ function run() { function done(server, socket, cb) { socket.destroy(); - server.close(); - cb(); + server.close(cb); } -function server_test(with_pipeline, cb) { +function serverTest(with_pipeline, cb) { let got_all = false; let timedout = false; const server = http.createServer(common.mustCall((req, res) => { @@ -36,7 +35,7 @@ function server_test(with_pipeline, cb) { done(server, req.socket, cb); } }, 3)); - server.setTimeout(500, common.mustCall((socket) => { + server.setTimeout(500, common.mustCallAtLeast((socket) => { // End this test and call `run()` for the next test (if any). timedout = true; if (got_all) @@ -57,9 +56,9 @@ function server_test(with_pipeline, cb) { } test(function serverEndKeepAliveTimeoutWithPipeline(cb) { - server_test(true, cb); + serverTest(true, cb); }); test(function serverNoEndKeepAliveTimeoutWithPipeline(cb) { - server_test(false, cb); + serverTest(false, cb); });