From d4a05b2d9c49d93a9dc3e33d7e4b0ddeaeac6089 Mon Sep 17 00:00:00 2001 From: jklepatch Date: Mon, 12 Jun 2017 11:12:49 +0800 Subject: [PATCH] test: make test-http(s)-set-timeout-server alike Make test-http(s)-set-timeout-server tests more similar and resolve the following issues: * `test-http-set-timeout-server.js` had a `secReceived` check in `serverResponseTimeoutWithPipeline()` that was added to prevent flakiness, but this did not exist in the https counterpart. * `test-https-set-timeout-server.js` utilized `common.mustCall()`, `common.mustNotCall()`, etc., while the http counterpart utilized the old method of checking flags on exit of the process. Refs: https://github.com/nodejs/node/issues/13588 PR-URL: https://github.com/nodejs/node/pull/13625 Reviewed-By: Rich Trott Reviewed-By: Colin Ihrig Reviewed-By: Alexey Orlenko --- test/parallel/test-http-set-timeout-server.js | 66 ++++--------------- .../test-https-set-timeout-server.js | 11 +++- 2 files changed, 22 insertions(+), 55 deletions(-) diff --git a/test/parallel/test-http-set-timeout-server.js b/test/parallel/test-http-set-timeout-server.js index b9cba1d192d0f7..1354dc657261eb 100644 --- a/test/parallel/test-http-set-timeout-server.js +++ b/test/parallel/test-http-set-timeout-server.js @@ -44,38 +44,28 @@ function run() { } test(function serverTimeout(cb) { - let caughtTimeout = false; - process.on('exit', function() { - assert(caughtTimeout); - }); const server = http.createServer(function(req, res) { // just do nothing, we should get a timeout event. }); server.listen(common.mustCall(function() { http.get({ port: server.address().port }).on('error', common.noop); })); - const s = server.setTimeout(50, function(socket) { - caughtTimeout = true; + const s = server.setTimeout(50, common.mustCall(function(socket) { socket.destroy(); server.close(); cb(); - }); + })); assert.ok(s instanceof http.Server); }); test(function serverRequestTimeout(cb) { - let caughtTimeout = false; - process.on('exit', function() { - assert(caughtTimeout); - }); const server = http.createServer(function(req, res) { // just do nothing, we should get a timeout event. - const s = req.setTimeout(50, function() { - caughtTimeout = true; - req.socket.destroy(); + const s = req.setTimeout(50, common.mustCall(function(socket) { + socket.destroy(); server.close(); cb(); - }); + })); assert.ok(s instanceof http.IncomingMessage); }); server.listen(common.mustCall(function() { @@ -88,18 +78,13 @@ test(function serverRequestTimeout(cb) { }); test(function serverResponseTimeout(cb) { - let caughtTimeout = false; - process.on('exit', function() { - assert(caughtTimeout); - }); const server = http.createServer(function(req, res) { // just do nothing, we should get a timeout event. - const s = res.setTimeout(50, function() { - caughtTimeout = true; - res.socket.destroy(); + const s = res.setTimeout(50, common.mustCall(function(socket) { + socket.destroy(); server.close(); cb(); - }); + })); assert.ok(s instanceof http.OutgoingMessage); }); server.listen(common.mustCall(function() { @@ -109,21 +94,11 @@ test(function serverResponseTimeout(cb) { }); test(function serverRequestNotTimeoutAfterEnd(cb) { - let caughtTimeoutOnRequest = false; - let caughtTimeoutOnResponse = false; - process.on('exit', function() { - assert(!caughtTimeoutOnRequest); - assert(caughtTimeoutOnResponse); - }); const server = http.createServer(function(req, res) { // just do nothing, we should get a timeout event. - const s = req.setTimeout(50, function(socket) { - caughtTimeoutOnRequest = true; - }); + const s = req.setTimeout(50, common.mustNotCall()); assert.ok(s instanceof http.IncomingMessage); - res.on('timeout', function(socket) { - caughtTimeoutOnResponse = true; - }); + res.on('timeout', common.mustCall()); }); server.on('timeout', function(socket) { socket.destroy(); @@ -169,29 +144,16 @@ test(function serverResponseTimeoutWithPipeline(cb) { }); test(function idleTimeout(cb) { - let caughtTimeoutOnRequest = false; - let caughtTimeoutOnResponse = false; - let caughtTimeoutOnServer = false; - process.on('exit', function() { - assert(!caughtTimeoutOnRequest); - assert(!caughtTimeoutOnResponse); - assert(caughtTimeoutOnServer); - }); const server = http.createServer(function(req, res) { - req.on('timeout', function(socket) { - caughtTimeoutOnRequest = true; - }); - res.on('timeout', function(socket) { - caughtTimeoutOnResponse = true; - }); + req.on('timeout', common.mustNotCall()); + res.on('timeout', common.mustNotCall()); res.end(); }); - const s = server.setTimeout(50, function(socket) { - caughtTimeoutOnServer = true; + const s = server.setTimeout(50, common.mustCall(function(socket) { socket.destroy(); server.close(); cb(); - }); + })); assert.ok(s instanceof http.Server); server.listen(common.mustCall(function() { const port = server.address().port; diff --git a/test/sequential/test-https-set-timeout-server.js b/test/sequential/test-https-set-timeout-server.js index 100354b54bf074..f6e97ea5981306 100644 --- a/test/sequential/test-https-set-timeout-server.js +++ b/test/sequential/test-https-set-timeout-server.js @@ -137,19 +137,24 @@ test(function serverRequestNotTimeoutAfterEnd(cb) { test(function serverResponseTimeoutWithPipeline(cb) { let caughtTimeout = ''; + let secReceived = false; process.on('exit', function() { assert.strictEqual(caughtTimeout, '/2'); }); const server = https.createServer(serverOptions, function(req, res) { + if (req.url === '/2') + secReceived = true; res.setTimeout(50, function() { caughtTimeout += req.url; }); if (req.url === '/1') res.end(); }); server.on('timeout', function(socket) { - socket.destroy(); - server.close(); - cb(); + if (secReceived) { + socket.destroy(); + server.close(); + cb(); + } }); server.listen(0, function() { const options = {