From 2e5ce2bc2c4b1abc48c514a668bb272484a59830 Mon Sep 17 00:00:00 2001 From: Julien Klepatch Date: Tue, 20 Jun 2017 23:43:57 +0800 Subject: [PATCH] test: make http(s)-set-timeout-server more similar Make test-http(s)-set-timeout-server tests more similar and resolve the following issues: * `test-https-set-timeout-server.js` was missing some `assert` statements, including with `http` module * Both files were missing some calls to `common.mustCall()` * Both files were calling `createServer()` in different ways PR-URL: https://github.com/nodejs/node/pull/13822 Refs: https://github.com/nodejs/node/issues/13588 Refs: https://github.com/nodejs/node/pull/13625 Reviewed-By: Luigi Pinca Reviewed-By: Alexey Orlenko Reviewed-By: James M Snell --- test/parallel/test-http-set-timeout-server.js | 74 ++++++----- .../test-https-set-timeout-server.js | 122 ++++++++++-------- 2 files changed, 111 insertions(+), 85 deletions(-) diff --git a/test/parallel/test-http-set-timeout-server.js b/test/parallel/test-http-set-timeout-server.js index b1b9e98ac0baf7..cd3cfa3bf11c3a 100644 --- a/test/parallel/test-http-set-timeout-server.js +++ b/test/parallel/test-http-set-timeout-server.js @@ -42,22 +42,24 @@ function run() { } test(function serverTimeout(cb) { - const server = http.createServer(function(req, res) { + const server = http.createServer(common.mustCall(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.mustCall()); })); - const s = server.setTimeout(50, common.mustCall(function(socket) { - socket.destroy(); - server.close(); - cb(); + server.listen(common.mustCall(function() { + const s = server.setTimeout(50, common.mustCall(function(socket) { + socket.destroy(); + server.close(); + cb(); + })); + assert.ok(s instanceof http.Server); + http.get({ + port: server.address().port + }).on('error', common.mustCall()); })); - assert.ok(s instanceof http.Server); }); test(function serverRequestTimeout(cb) { - const server = http.createServer(function(req, res) { + const server = http.createServer(common.mustCall(function(req, res) { // just do nothing, we should get a timeout event. const s = req.setTimeout(50, common.mustCall(function(socket) { socket.destroy(); @@ -65,10 +67,12 @@ test(function serverRequestTimeout(cb) { cb(); })); assert.ok(s instanceof http.IncomingMessage); - }); + })); server.listen(common.mustCall(function() { - const port = server.address().port; - const req = http.request({ port: port, method: 'POST' }); + const req = http.request({ + port: server.address().port, + method: 'POST' + }); req.on('error', common.mustCall()); req.write('Hello'); // req is in progress @@ -76,7 +80,7 @@ test(function serverRequestTimeout(cb) { }); test(function serverResponseTimeout(cb) { - const server = http.createServer(function(req, res) { + const server = http.createServer(common.mustCall(function(req, res) { // just do nothing, we should get a timeout event. const s = res.setTimeout(50, common.mustCall(function(socket) { socket.destroy(); @@ -84,28 +88,30 @@ test(function serverResponseTimeout(cb) { cb(); })); assert.ok(s instanceof http.OutgoingMessage); - }); + })); server.listen(common.mustCall(function() { - const port = server.address().port; - http.get({ port: port }).on('error', common.mustCall()); + http.get({ + port: server.address().port + }).on('error', common.mustCall()); })); }); test(function serverRequestNotTimeoutAfterEnd(cb) { - const server = http.createServer(function(req, res) { + const server = http.createServer(common.mustCall(function(req, res) { // just do nothing, we should get a timeout event. const s = req.setTimeout(50, common.mustNotCall()); assert.ok(s instanceof http.IncomingMessage); res.on('timeout', common.mustCall()); - }); - server.on('timeout', function(socket) { + })); + server.on('timeout', common.mustCall(function(socket) { socket.destroy(); server.close(); cb(); - }); + })); server.listen(common.mustCall(function() { - const port = server.address().port; - http.get({ port: port }).on('error', common.mustCall()); + http.get({ + port: server.address().port + }).on('error', common.mustCall()); })); }); @@ -124,16 +130,19 @@ test(function serverResponseTimeoutWithPipeline(cb) { assert.ok(s instanceof http.OutgoingMessage); if (req.url === '/1') res.end(); }); - server.on('timeout', function(socket) { + server.on('timeout', common.mustCall(function(socket) { if (secReceived) { socket.destroy(); server.close(); cb(); } - }); + })); server.listen(common.mustCall(function() { - const port = server.address().port; - const c = net.connect({ port: port, allowHalfOpen: true }, function() { + const options = { + port: server.address().port, + allowHalfOpen: true, + }; + const c = net.connect(options, function() { 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'); @@ -142,11 +151,11 @@ test(function serverResponseTimeoutWithPipeline(cb) { }); test(function idleTimeout(cb) { - const server = http.createServer(function(req, res) { + const server = http.createServer(common.mustCall(function(req, res) { req.on('timeout', common.mustNotCall()); res.on('timeout', common.mustNotCall()); res.end(); - }); + })); const s = server.setTimeout(50, common.mustCall(function(socket) { socket.destroy(); server.close(); @@ -154,8 +163,11 @@ test(function idleTimeout(cb) { })); assert.ok(s instanceof http.Server); server.listen(common.mustCall(function() { - const port = server.address().port; - const c = net.connect({ port: port, allowHalfOpen: true }, function() { + const options = { + port: server.address().port, + allowHalfOpen: true, + }; + const c = net.connect(options, function() { c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n'); // Keep-Alive }); diff --git a/test/sequential/test-https-set-timeout-server.js b/test/sequential/test-https-set-timeout-server.js index f6e97ea5981306..beec5109da054f 100644 --- a/test/sequential/test-https-set-timeout-server.js +++ b/test/sequential/test-https-set-timeout-server.js @@ -30,6 +30,7 @@ if (!common.hasCrypto) { const assert = require('assert'); const fs = require('fs'); const https = require('https'); +const http = require('http'); const tls = require('tls'); const tests = []; @@ -56,10 +57,13 @@ function run() { } test(function serverTimeout(cb) { - const server = https.createServer(serverOptions, function(req, res) { - // just do nothing, we should get a timeout event. - }); - server.listen(0, common.mustCall(function() { + const server = https.createServer( + serverOptions, + common.mustCall(function(req, res) { + // just do nothing, we should get a + // timeout event. + })); + server.listen(common.mustCall(function() { const s = server.setTimeout(50, common.mustCall(function(socket) { socket.destroy(); server.close(); @@ -67,72 +71,79 @@ test(function serverTimeout(cb) { })); assert.ok(s instanceof https.Server); https.get({ - port: this.address().port, + port: server.address().port, rejectUnauthorized: false }).on('error', common.mustCall()); })); }); test(function serverRequestTimeout(cb) { - function handler(req, res) { - // just do nothing, we should get a timeout event. - req.setTimeout(50, common.mustCall(function() { - req.socket.destroy(); - server.close(); - cb(); + const server = https.createServer( + serverOptions, + common.mustCall(function(req, res) { + // just do nothing, we should get a + // timeout event. + const s = req.setTimeout( + 50, + common.mustCall(function(socket) { + socket.destroy(); + server.close(); + cb(); + })); + assert.ok(s instanceof http.IncomingMessage); })); - } - - const server = https.createServer(serverOptions, common.mustCall(handler)); - server.listen(0, function() { + server.listen(common.mustCall(function() { const req = https.request({ - port: this.address().port, + port: server.address().port, method: 'POST', rejectUnauthorized: false }); req.on('error', common.mustCall()); req.write('Hello'); // req is in progress - }); + })); }); test(function serverResponseTimeout(cb) { - function handler(req, res) { - // just do nothing, we should get a timeout event. - res.setTimeout(50, common.mustCall(function() { - res.socket.destroy(); - server.close(); - cb(); + const server = https.createServer( + serverOptions, + common.mustCall(function(req, res) { + // just do nothing, we should get a timeout event. + const s = res.setTimeout(50, common.mustCall(function(socket) { + socket.destroy(); + server.close(); + cb(); + })); + assert.ok(s instanceof http.OutgoingMessage); })); - } - - const server = https.createServer(serverOptions, common.mustCall(handler)); - server.listen(0, function() { + server.listen(common.mustCall(function() { https.get({ - port: this.address().port, + port: server.address().port, rejectUnauthorized: false }).on('error', common.mustCall()); - }); + })); }); test(function serverRequestNotTimeoutAfterEnd(cb) { - function handler(req, res) { - // just do nothing, we should get a timeout event. - req.setTimeout(50, common.mustNotCall()); - res.on('timeout', common.mustCall()); - } - const server = https.createServer(serverOptions, common.mustCall(handler)); - server.on('timeout', function(socket) { + const server = https.createServer( + serverOptions, + common.mustCall(function(req, res) { + // just do nothing, we should get a timeout event. + const s = req.setTimeout(50, common.mustNotCall()); + assert.ok(s instanceof http.IncomingMessage); + res.on('timeout', common.mustCall()); + })); + server.on('timeout', common.mustCall(function(socket) { socket.destroy(); server.close(); cb(); - }); - server.listen(0, function() { + })); + server.listen(common.mustCall(function() { https.get({ - port: this.address().port, + port: server.address().port, rejectUnauthorized: false }).on('error', common.mustCall()); - }); + })); }); test(function serverResponseTimeoutWithPipeline(cb) { @@ -144,9 +155,10 @@ test(function serverResponseTimeoutWithPipeline(cb) { const server = https.createServer(serverOptions, function(req, res) { if (req.url === '/2') secReceived = true; - res.setTimeout(50, function() { + const s = res.setTimeout(50, function() { caughtTimeout += req.url; }); + assert.ok(s instanceof http.OutgoingMessage); if (req.url === '/1') res.end(); }); server.on('timeout', function(socket) { @@ -156,9 +168,9 @@ test(function serverResponseTimeoutWithPipeline(cb) { cb(); } }); - server.listen(0, function() { + server.listen(common.mustCall(function() { const options = { - port: this.address().port, + port: server.address().port, allowHalfOpen: true, rejectUnauthorized: false }; @@ -167,24 +179,26 @@ test(function serverResponseTimeoutWithPipeline(cb) { 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'); }); - }); + })); }); test(function idleTimeout(cb) { - const server = https.createServer(serverOptions, - common.mustCall(function(req, res) { - req.on('timeout', common.mustNotCall()); - res.on('timeout', common.mustNotCall()); - res.end(); - })); - server.setTimeout(50, common.mustCall(function(socket) { + const server = https.createServer( + serverOptions, + common.mustCall(function(req, res) { + req.on('timeout', common.mustNotCall()); + res.on('timeout', common.mustNotCall()); + res.end(); + })); + const s = server.setTimeout(50, common.mustCall(function(socket) { socket.destroy(); server.close(); cb(); })); - server.listen(0, function() { + assert.ok(s instanceof https.Server); + server.listen(common.mustCall(function() { const options = { - port: this.address().port, + port: server.address().port, allowHalfOpen: true, rejectUnauthorized: false }; @@ -192,5 +206,5 @@ test(function idleTimeout(cb) { this.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n'); // Keep-Alive }); - }); + })); });