From b23ec7c097f51f0b40a4f110921894b82e3dc2cb Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 8 May 2017 12:50:29 -0700 Subject: [PATCH] test: refactor http tests * Use common.mustCall where appropriate * Remove extraneous console output * Misc cleanups --- test/parallel/test-http-1.0-keep-alive.js | 11 ++--- test/parallel/test-http-1.0.js | 17 +++---- test/parallel/test-http-abort-before-end.js | 6 +-- test/parallel/test-http-abort-client.js | 36 ++++---------- test/parallel/test-http-after-connect.js | 34 +++++--------- .../test-http-agent-destroyed-socket.js | 42 +++++++---------- .../parallel/test-http-agent-error-on-idle.js | 14 +++--- test/parallel/test-http-agent-false.js | 20 +++----- test/parallel/test-http-agent-keepalive.js | 47 +++++++++---------- .../test-http-client-read-in-error.js | 14 ++---- .../test-http-client-timeout-event.js | 10 ++-- .../test-http-client-timeout-option.js | 16 +++---- 12 files changed, 98 insertions(+), 169 deletions(-) diff --git a/test/parallel/test-http-1.0-keep-alive.js b/test/parallel/test-http-1.0-keep-alive.js index f3b69a4cd059dd..69134e282fc649 100644 --- a/test/parallel/test-http-1.0-keep-alive.js +++ b/test/parallel/test-http-1.0-keep-alive.js @@ -20,7 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; -require('../common'); +const common = require('../common'); const http = require('http'); const net = require('net'); @@ -107,7 +107,8 @@ function check(tests) { const test = tests[0]; let server; if (test) { - server = http.createServer(serverHandler).listen(0, '127.0.0.1', client); + server = http.createServer(serverHandler) + .listen(0, '127.0.0.1', common.mustCall(client)); } let current = 0; @@ -118,9 +119,8 @@ function check(tests) { function serverHandler(req, res) { if (current + 1 === test.responses.length) this.close(); const ctx = test.responses[current]; - console.error('< SERVER SENDING RESPONSE', ctx); res.writeHead(200, ctx.headers); - ctx.chunks.slice(0, -1).forEach(function(chunk) { res.write(chunk); }); + ctx.chunks.slice(0, -1).forEach((chunk) => res.write(chunk)); res.end(ctx.chunks[ctx.chunks.length - 1]); } @@ -131,19 +131,16 @@ function check(tests) { function connected() { const ctx = test.requests[current]; - console.error(' > CLIENT SENDING REQUEST', ctx); conn.setEncoding('utf8'); conn.write(ctx.data); function onclose() { - console.error(' > CLIENT CLOSE'); if (!ctx.expectClose) throw new Error('unexpected close'); client(); } conn.on('close', onclose); function ondata(s) { - console.error(' > CLIENT ONDATA %j %j', s.length, s.toString()); current++; if (ctx.expectClose) return; conn.removeListener('close', onclose); diff --git a/test/parallel/test-http-1.0.js b/test/parallel/test-http-1.0.js index f67d949bc8f5f6..045ca1c54eb63c 100644 --- a/test/parallel/test-http-1.0.js +++ b/test/parallel/test-http-1.0.js @@ -34,26 +34,21 @@ function test(handler, request_generator, response_validator) { let server_response = ''; server.listen(0); - server.on('listening', function() { - const c = net.createConnection(this.address().port); + server.on('listening', common.mustCall(() => { + const c = net.createConnection(server.address().port); c.setEncoding('utf8'); - c.on('connect', function() { - c.write(request_generator()); - }); + c.on('connect', common.mustCall(() => c.write(request_generator()))); + c.on('data', common.mustCall((chunk) => server_response += chunk)); - c.on('data', function(chunk) { - server_response += chunk; - }); - - c.on('end', common.mustCall(function() { + c.on('end', common.mustCall(() => { client_got_eof = true; c.end(); server.close(); response_validator(server_response, client_got_eof, false); })); - }); + })); } { diff --git a/test/parallel/test-http-abort-before-end.js b/test/parallel/test-http-abort-before-end.js index 37d1291b074127..210115fbabdddd 100644 --- a/test/parallel/test-http-abort-before-end.js +++ b/test/parallel/test-http-abort-before-end.js @@ -33,11 +33,7 @@ server.listen(0, function() { port: this.address().port }); - req.on('error', function(ex) { - // https://github.com/joyent/node/issues/1399#issuecomment-2597359 - // abort() should emit an Error, not the net.Socket object - assert(ex instanceof Error); - }); + req.on('error', common.mustNotCall()); req.abort(); req.end(); diff --git a/test/parallel/test-http-abort-client.js b/test/parallel/test-http-abort-client.js index ada72dbcd04c39..935dd19557db03 100644 --- a/test/parallel/test-http-abort-client.js +++ b/test/parallel/test-http-abort-client.js @@ -23,42 +23,24 @@ const common = require('../common'); const http = require('http'); -const server = http.Server(function(req, res) { - console.log('Server accepted request.'); +const server = http.Server(common.mustCall((req, res) => { res.writeHead(200); res.write('Part of my res.'); res.destroy(); -}); +})); -server.listen(0, common.mustCall(function() { +server.listen(0, common.mustCall(() => { http.get({ - port: this.address().port, + port: server.address().port, headers: { connection: 'keep-alive' } - }, common.mustCall(function(res) { + }, common.mustCall((res) => { server.close(); - console.log(`Got res: ${res.statusCode}`); - console.dir(res.headers); - - res.on('data', function(chunk) { - console.log(`Read ${chunk.length} bytes`); - console.log(' chunk=%j', chunk.toString()); - }); - - res.on('end', function() { - console.log('Response ended.'); - }); - - res.on('aborted', function() { - console.log('Response aborted.'); - }); - - res.socket.on('close', function() { - console.log('socket closed, but not res'); - }); - - // it would be nice if this worked: + res.on('data', common.mustCall()); + res.on('end', common.mustCall()); + res.on('aborted', common.mustCall()); + res.socket.on('close', common.mustCall()); res.on('close', common.mustCall()); })); })); diff --git a/test/parallel/test-http-after-connect.js b/test/parallel/test-http-after-connect.js index d8a711c8d69a26..c5b6a40743afd1 100644 --- a/test/parallel/test-http-after-connect.js +++ b/test/parallel/test-http-after-connect.js @@ -26,36 +26,29 @@ const http = require('http'); let clientResponses = 0; -const server = http.createServer(common.mustCall(function(req, res) { - console.error('Server got GET request'); +const server = http.createServer(common.mustCall((req, res) => { req.resume(); res.writeHead(200); res.write(''); - setTimeout(function() { - res.end(req.url); - }, 50); + setTimeout(() => res.end(req.url), 50); }, 2)); -server.on('connect', common.mustCall(function(req, socket) { - console.error('Server got CONNECT request'); +server.on('connect', common.mustCall((req, socket) => { socket.write('HTTP/1.1 200 Connection established\r\n\r\n'); socket.resume(); - socket.on('end', function() { - socket.end(); - }); + socket.on('end', () => socket.end()); })); -server.listen(0, function() { +server.listen(0, () => { const req = http.request({ - port: this.address().port, + port: server.address().port, method: 'CONNECT', path: 'google.com:80' }); - req.on('connect', common.mustCall(function(res, socket) { - console.error('Client got CONNECT response'); + req.on('connect', common.mustCall((res, socket) => { socket.end(); - socket.on('end', function() { + socket.on('end', common.mustCall(() => { doRequest(0); doRequest(1); - }); + })); socket.resume(); })); req.end(); @@ -65,14 +58,11 @@ function doRequest(i) { http.get({ port: server.address().port, path: `/request${i}` - }, common.mustCall(function(res) { - console.error('Client got GET response'); + }, common.mustCall((res) => { let data = ''; res.setEncoding('utf8'); - res.on('data', function(chunk) { - data += chunk; - }); - res.on('end', function() { + res.on('data', (chunk) => data += chunk); + res.on('end', () => { assert.strictEqual(data, `/request${i}`); ++clientResponses; if (clientResponses === 2) { diff --git a/test/parallel/test-http-agent-destroyed-socket.js b/test/parallel/test-http-agent-destroyed-socket.js index 322373e69cdd4e..5a63ee5793bb63 100644 --- a/test/parallel/test-http-agent-destroyed-socket.js +++ b/test/parallel/test-http-agent-destroyed-socket.js @@ -24,34 +24,28 @@ const common = require('../common'); const assert = require('assert'); const http = require('http'); -const server = http.createServer(function(req, res) { +const server = http.createServer(common.mustCall((req, res) => { res.writeHead(200, {'Content-Type': 'text/plain'}); res.end('Hello World\n'); -}).listen(0, common.mustCall(function() { +}, 2)).listen(0, common.mustCall(() => { const agent = new http.Agent({maxSockets: 1}); - agent.on('free', function(socket) { - console.log('freeing socket. destroyed? ', socket.destroyed); - }); + agent.on('free', common.mustCall(3)); const requestOptions = { agent: agent, host: 'localhost', - port: this.address().port, + port: server.address().port, path: '/' }; - const request1 = http.get(requestOptions, common.mustCall(function(response) { + const request1 = http.get(requestOptions, common.mustCall((response) => { // assert request2 is queued in the agent const key = agent.getName(requestOptions); assert.strictEqual(agent.requests[key].length, 1); - console.log('got response1'); - request1.socket.on('close', function() { - console.log('request1 socket closed'); - }); - response.pipe(process.stdout); - response.on('end', common.mustCall(function() { - console.log('response1 done'); + request1.socket.on('close', common.mustCall()); + response.on('data', common.mustCall()); + response.on('end', common.mustCall(() => { ///////////////////////////////// // // THE IMPORTANT PART @@ -65,11 +59,10 @@ const server = http.createServer(function(req, res) { // is triggered. request1.socket.destroy(); - response.once('close', function() { + response.once('close', () => { // assert request2 was removed from the queue assert(!agent.requests[key]); - console.log("waiting for request2.onSocket's nextTick"); - process.nextTick(common.mustCall(function() { + process.nextTick(common.mustCall(() => { // assert that the same socket was not assigned to request2, // since it was destroyed. assert.notStrictEqual(request1.socket, request2.socket); @@ -79,25 +72,22 @@ const server = http.createServer(function(req, res) { })); })); - const request2 = http.get(requestOptions, common.mustCall(function(response) { + const request2 = http.get(requestOptions, common.mustCall((response) => { assert(!request2.socket.destroyed); assert(request1.socket.destroyed); // assert not reusing the same socket, since it was destroyed. assert.notStrictEqual(request1.socket, request2.socket); - console.log('got response2'); let gotClose = false; let gotResponseEnd = false; - request2.socket.on('close', function() { - console.log('request2 socket closed'); + request2.socket.on('close', common.mustCall(() => { gotClose = true; done(); - }); - response.pipe(process.stdout); - response.on('end', function() { - console.log('response2 done'); + })); + response.on('data', common.mustCall()); + response.on('end', common.mustCall(() => { gotResponseEnd = true; done(); - }); + })); function done() { if (gotResponseEnd && gotClose) diff --git a/test/parallel/test-http-agent-error-on-idle.js b/test/parallel/test-http-agent-error-on-idle.js index 2f270c0d30ca0b..7ecaa9f0194b2d 100644 --- a/test/parallel/test-http-agent-error-on-idle.js +++ b/test/parallel/test-http-agent-error-on-idle.js @@ -1,21 +1,21 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const http = require('http'); const Agent = http.Agent; -const server = http.createServer(function(req, res) { +const server = http.createServer((req, res) => { res.end('hello world'); }); -server.listen(0, function() { +server.listen(0, () => { const agent = new Agent({ keepAlive: true, }); const requestParams = { host: 'localhost', - port: this.address().port, + port: server.address().port, agent: agent, path: '/' }; @@ -25,8 +25,8 @@ server.listen(0, function() { get(function(res) { assert.strictEqual(res.statusCode, 200); res.resume(); - res.on('end', function() { - process.nextTick(function() { + res.on('end', common.mustCall(() => { + process.nextTick(() => { const freeSockets = agent.freeSockets[socketKey]; assert.strictEqual(freeSockets.length, 1, `expect a free socket on ${socketKey}`); @@ -37,7 +37,7 @@ server.listen(0, function() { get(done); }); - }); + })); }); function get(callback) { diff --git a/test/parallel/test-http-agent-false.js b/test/parallel/test-http-agent-false.js index 0e633c7c103b76..8b7f4c3fa14690 100644 --- a/test/parallel/test-http-agent-false.js +++ b/test/parallel/test-http-agent-false.js @@ -20,7 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const http = require('http'); @@ -35,20 +35,14 @@ const opts = { agent: false }; -let good = false; -process.on('exit', function() { - assert(good, 'expected either an "error" or "response" event'); -}); - // we just want an "error" (no local HTTP server on port 80) or "response" // to happen (user happens ot have HTTP server running on port 80). // As long as the process doesn't crash from a C++ assertion then we're good. +const fn = common.mustCall(); const req = http.request(opts); -req.on('response', function(res) { - good = true; -}); -req.on('error', function(err) { - // an "error" event is ok, don't crash the process - good = true; -}); + +// Both the below use the same common.mustCall() instance, so exactly one +// (but not both) would be called +req.on('response', fn); +req.on('error', fn); req.end(); diff --git a/test/parallel/test-http-agent-keepalive.js b/test/parallel/test-http-agent-keepalive.js index c695b55bfa5ee7..d3b01c20e2c17c 100644 --- a/test/parallel/test-http-agent-keepalive.js +++ b/test/parallel/test-http-agent-keepalive.js @@ -34,7 +34,7 @@ const agent = new Agent({ maxFreeSockets: 5 }); -const server = http.createServer(function(req, res) { +const server = http.createServer(common.mustCall((req, res) => { if (req.url === '/error') { res.destroy(); return; @@ -46,7 +46,7 @@ const server = http.createServer(function(req, res) { }); } res.end('hello world'); -}); +}, 4)); function get(path, callback) { return http.get({ @@ -65,82 +65,79 @@ function checkDataAndSockets(body) { function second() { // request second, use the same socket - get('/second', function(res) { + get('/second', (res) => { assert.strictEqual(res.statusCode, 200); res.on('data', checkDataAndSockets); - res.on('end', function() { + res.on('end', common.mustCall(() => { assert.strictEqual(agent.sockets[name].length, 1); assert.strictEqual(agent.freeSockets[name], undefined); - process.nextTick(function() { + process.nextTick(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name].length, 1); remoteClose(); }); - }); + })); }); } function remoteClose() { // mock remote server close the socket - get('/remote_close', function(res) { + get('/remote_close', common.mustCall((res) => { assert.deepStrictEqual(res.statusCode, 200); res.on('data', checkDataAndSockets); - res.on('end', function() { + res.on('end', common.mustCall(() => { assert.strictEqual(agent.sockets[name].length, 1); assert.strictEqual(agent.freeSockets[name], undefined); process.nextTick(function() { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name].length, 1); // waitting remote server close the socket - setTimeout(function() { + setTimeout(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name], undefined, 'freeSockets is not empty'); remoteError(); }, common.platformTimeout(200)); }); - }); - }); + })); + })); } function remoteError() { // remove server will destroy ths socket - const req = get('/error', function(res) { - throw new Error('should not call this function'); - }); - req.on('error', function(err) { + const req = get('/error', common.mustNotCall()); + req.on('error', common.mustCall((err) => { assert.ok(err); assert.strictEqual(err.message, 'socket hang up'); assert.strictEqual(agent.sockets[name].length, 1); assert.strictEqual(agent.freeSockets[name], undefined); // Wait socket 'close' event emit - setTimeout(function() { + setTimeout(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name], undefined); done(); }, common.platformTimeout(1)); - }); + })); } function done() { - console.log('http keepalive agent test success.'); process.exit(0); } -server.listen(0, function() { +server.listen(0, common.mustCall(() => { name = `localhost:${server.address().port}:`; // request first, and keep alive - get('/first', function(res) { + get('/first', common.mustCall((res) => { assert.strictEqual(res.statusCode, 200); res.on('data', checkDataAndSockets); - res.on('end', function() { + res.on('end', common.mustCall(() => { assert.strictEqual(agent.sockets[name].length, 1); assert.strictEqual(agent.freeSockets[name], undefined); - process.nextTick(function() { + process.nextTick(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name].length, 1); second(); }); - }); - }); -}); + })); + })); +})); diff --git a/test/parallel/test-http-client-read-in-error.js b/test/parallel/test-http-client-read-in-error.js index 69b24f73771d41..efd902cd9f768b 100644 --- a/test/parallel/test-http-client-read-in-error.js +++ b/test/parallel/test-http-client-read-in-error.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +const common = require('../common'); const net = require('net'); const http = require('http'); const util = require('util'); @@ -13,9 +13,7 @@ Agent.prototype.createConnection = function() { const self = this; const socket = new net.Socket(); - socket.on('error', function() { - socket.push('HTTP/1.1 200\r\n\r\n'); - }); + socket.on('error', () => socket.push('HTTP/1.1 200\r\n\r\n')); socket.on('newListener', function onNewListener(name) { if (name !== 'error') @@ -23,9 +21,7 @@ Agent.prototype.createConnection = function() { socket.removeListener('newListener', onNewListener); // Let other listeners to be set up too - process.nextTick(function() { - self.breakSocket(socket); - }); + process.nextTick(() => self.breakSocket(socket)); }); return socket; @@ -39,6 +35,4 @@ const agent = new Agent(); http.request({ agent: agent -}).once('error', function() { - console.log('ignore'); -}); +}).once('error', common.noop); diff --git a/test/parallel/test-http-client-timeout-event.js b/test/parallel/test-http-client-timeout-event.js index c76d56ef256407..bc0398fb806786 100644 --- a/test/parallel/test-http-client-timeout-event.js +++ b/test/parallel/test-http-client-timeout-event.js @@ -32,12 +32,10 @@ const options = { const server = http.createServer(); -server.listen(0, options.host, function() { - options.port = this.address().port; +server.listen(0, options.host, common.mustCall(() => { + options.port = server.address().port; const req = http.request(options); - req.on('error', function() { - // this space is intentionally left blank - }); + req.on('error', common.mustCall()); req.on('close', common.mustCall(() => server.close())); req.setTimeout(1); @@ -48,4 +46,4 @@ server.listen(0, options.host, function() { }, 100); }); })); -}); +})); diff --git a/test/parallel/test-http-client-timeout-option.js b/test/parallel/test-http-client-timeout-option.js index 6741fbe0bd1e35..337c3c29e6ffc4 100644 --- a/test/parallel/test-http-client-timeout-option.js +++ b/test/parallel/test-http-client-timeout-option.js @@ -13,21 +13,17 @@ const options = { const server = http.createServer(); -server.listen(0, options.host, function() { - options.port = this.address().port; +server.listen(0, options.host, common.mustCall(() => { + options.port = server.address().port; const req = http.request(options); - req.on('error', function() { - // this space is intentionally left blank - }); + req.on('error', common.mustCall()); req.on('close', common.mustCall(() => server.close())); let timeout_events = 0; req.on('timeout', common.mustCall(() => timeout_events += 1)); - setTimeout(function() { + setTimeout(() => { req.destroy(); assert.strictEqual(timeout_events, 1); }, common.platformTimeout(100)); - setTimeout(function() { - req.end(); - }, common.platformTimeout(10)); -}); + setTimeout(() => req.end(), common.platformTimeout(10)); +}));