test: improvements to various http tests

* Add common/countdown utility
* Numerous improvements to http tests

Backport-PR-URL: #14583
Backport-Reviewed-By: Anna Henningsen <[email protected]>

PR-URL: #14315
Reviewed-By: Benjamin Gruenbaum <[email protected]>

Author: jasnell

test/common/README.md

@@ -338,6 +338,42 @@ The realpath of the 'tmp' directory.
 
 Name of the temp directory used by tests.
 
+## Countdown Module
+
+The `Countdown` module provides a simple countdown mechanism for tests that
+require a particular action to be taken after a given number of completed
+tasks (for instance, shutting down an HTTP server after a specific number of
+requests).
+
+<!-- eslint-disable strict, required-modules -->
+```js
+const Countdown = require('../common/countdown');
+
+function doSomething() {
+  console.log('.');
+}
+
+const countdown = new Countdown(2, doSomething);
+countdown.dec();
+countdown.dec();
+```
+
+### new Countdown(limit, callback)
+
+* `limit` {number}
+* `callback` {function}
+
+Creates a new `Countdown` instance.
+
+### Countdown.prototype.dec()
+
+Decrements the `Countdown` counter.
+
+### Coutndown.prototype.remaining
+
+Specifies the remaining number of times `Countdown.prototype.dec()` must be
+called before the callback is invoked.
+
 ## WPT Module
 
 The wpt.js module is a port of parts of

test/common/countdown.js

@@ -0,0 +1,27 @@
+/* eslint-disable required-modules */
+'use strict';
+
+const assert = require('assert');
+const kLimit = Symbol('limit');
+const kCallback = Symbol('callback');
+
+class Countdown {
+  constructor(limit, cb) {
+    assert.strictEqual(typeof limit, 'number');
+    assert.strictEqual(typeof cb, 'function');
+    this[kLimit] = limit;
+    this[kCallback] = cb;
+  }
+
+  dec() {
+    assert(this[kLimit] > 0, 'Countdown expired');
+    if (--this[kLimit] === 0)
+      this[kCallback]();
+  }
+
+  get remaining() {
+    return this[kLimit];
+  }
+}
+
+module.exports = Countdown;

test/parallel/test-common-countdown.js

@@ -0,0 +1,15 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const Countdown = require('../common/countdown');
+
+let done = '';
+
+const countdown = new Countdown(2, common.mustCall(() => done = true));
+assert.strictEqual(countdown.remaining, 2);
+countdown.dec();
+assert.strictEqual(countdown.remaining, 1);
+countdown.dec();
+assert.strictEqual(countdown.remaining, 0);
+assert.strictEqual(done, true);

test/parallel/test-http-abort-client.js

@@ -24,42 +24,24 @@ const common = require('../common');
 const http = require('http');
 
 let serverRes;
-const server = http.Server(function(req, res) {
-  console.log('Server accepted request.');
+const server = http.Server((req, res) => {
   serverRes = res;
   res.writeHead(200);
   res.write('Part of my res.');
 });
 
-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();
     serverRes.destroy();
 
-    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.resume();
+    res.on('end', common.mustCall());
+    res.on('aborted', common.mustCall());
     res.on('close', common.mustCall());
+    res.socket.on('close', common.mustCall());
   }));
 }));

test/parallel/test-http-abort-queued.js

@@ -20,13 +20,13 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const http = require('http');
 
 let complete;
 
-const server = http.createServer(function(req, res) {
+const server = http.createServer((req, res) => {
   // We should not see the queued /thatotherone request within the server
   // as it should be aborted before it is sent.
   assert.strictEqual(req.url, '/');
@@ -40,10 +40,8 @@ const server = http.createServer(function(req, res) {
 });
 
 
-server.listen(0, function() {
-  console.log('listen', server.address().port);
-
-  const agent = new http.Agent({maxSockets: 1});
+server.listen(0, () => {
+  const agent = new http.Agent({ maxSockets: 1 });
   assert.strictEqual(Object.keys(agent.sockets).length, 0);
 
   const options = {
@@ -55,7 +53,7 @@ server.listen(0, function() {
   };
 
   const req1 = http.request(options);
-  req1.on('response', function(res1) {
+  req1.on('response', (res1) => {
     assert.strictEqual(Object.keys(agent.sockets).length, 1);
     assert.strictEqual(Object.keys(agent.requests).length, 0);
 
@@ -69,7 +67,9 @@ server.listen(0, function() {
     assert.strictEqual(Object.keys(agent.sockets).length, 1);
     assert.strictEqual(Object.keys(agent.requests).length, 1);
 
-    req2.on('error', function(err) {
+    // TODO(jasnell): This event does not appear to currently be triggered.
+    // is this handler actually required?
+    req2.on('error', (err) => {
       // This is expected in response to our explicit abort call
       assert.strictEqual(err.code, 'ECONNRESET');
     });
@@ -80,25 +80,16 @@ server.listen(0, function() {
     assert.strictEqual(Object.keys(agent.sockets).length, 1);
     assert.strictEqual(Object.keys(agent.requests).length, 1);
 
-    console.log(`Got res: ${res1.statusCode}`);
-    console.dir(res1.headers);
-
-    res1.on('data', function(chunk) {
-      console.log(`Read ${chunk.length} bytes`);
-      console.log(' chunk=%j', chunk.toString());
-      complete();
-    });
+    res1.on('data', (chunk) => complete());
 
-    res1.on('end', function() {
-      console.log('Response ended.');
-
-      setTimeout(function() {
+    res1.on('end', common.mustCall(() => {
+      setTimeout(common.mustCall(() => {
         assert.strictEqual(Object.keys(agent.sockets).length, 0);
         assert.strictEqual(Object.keys(agent.requests).length, 0);
 
         server.close();
-      }, 100);
-    });
+      }), 100);
+    }));
   });
 
   req1.end();

test/parallel/test-http-abort-stream-end.js

@@ -20,28 +20,29 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 
 const http = require('http');
 
 const maxSize = 1024;
 let size = 0;
 
-const s = http.createServer(function(req, res) {
-  this.close();
+const server = http.createServer(common.mustCall((req, res) => {
+  server.close();
 
   res.writeHead(200, {'Content-Type': 'text/plain'});
   for (let i = 0; i < maxSize; i++) {
-    res.write('x' + i);
+    res.write(`x${i}`);
   }
   res.end();
-});
+}));
 
 let aborted = false;
-s.listen(0, function() {
-  const req = http.get('http://localhost:' + s.address().port, function(res) {
-    res.on('data', function(chunk) {
+server.listen(0, () => {
+
+  const res = common.mustCall((res) => {
+    res.on('data', (chunk) => {
       size += chunk.length;
       assert(!aborted, 'got data after abort');
       if (size > maxSize) {
@@ -50,11 +51,9 @@ s.listen(0, function() {
         size = maxSize;
       }
     });
+
+    req.on('abort', common.mustCall(() => assert.strictEqual(size, maxSize)));
   });
-});
 
-process.on('exit', function() {
-  assert(aborted);
-  assert.strictEqual(size, maxSize);
-  console.log('ok');
+  const req = http.get(`http://localhost:${server.address().port}`, res);
 });

test/parallel/test-http-after-connect.js

@@ -23,61 +23,51 @@
 const common = require('../common');
 const assert = require('assert');
 const http = require('http');
+const Countdown = require('../common/countdown');
 
-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');
+
+const countdown = new Countdown(2, common.mustCall(() => server.close()));
+
+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, common.mustCall(() => {
   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();
-});
+}));
 
 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', common.mustCall(() => {
       assert.strictEqual(data, `/request${i}`);
-      ++clientResponses;
-      if (clientResponses === 2) {
-        server.close();
-      }
-    });
+      countdown.dec();
+    }));
   }));
 }