test: refactor two http client timeout tests

lpinca · addaleax · commit 6323a9fc57f1 · 2019-02-13T21:57:05.000+01:00
Refactor test-http-client-set-timeout and test-http-client-timeout-option-with-agent. PR-URL: #25473 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/test/parallel/test-http-client-set-timeout.js b/test/parallel/test-http-client-set-timeout.js
@@ -1,46 +1,48 @@
 'use strict';
-const common = require('../common');
 
-// Test that `req.setTimeout` will fired exactly once.
+// Test that the `'timeout'` event is emitted exactly once if the `timeout`
+// option and `request.setTimeout()` are used together.
 
-const assert = require('assert');
-const http = require('http');
+const { expectsError, mustCall } = require('../common');
+const { strictEqual } = require('assert');
+const { createServer, get } = require('http');
 
-const HTTP_CLIENT_TIMEOUT = 2000;
-
-const options = {
-  method: 'GET',
-  port: undefined,
-  host: '127.0.0.1',
-  path: '/',
-  timeout: HTTP_CLIENT_TIMEOUT,
-};
-
-const server = http.createServer(() => {
+const server = createServer(() => {
   // Never respond.
 });
 
-server.listen(0, options.host, function() {
-  doRequest();
-});
-
-function doRequest() {
-  options.port = server.address().port;
-  const req = http.request(options);
-  req.setTimeout(HTTP_CLIENT_TIMEOUT / 2);
-  req.on('error', () => {
-    // This space is intentionally left blank.
+server.listen(0, mustCall(() => {
+  const req = get({
+    port: server.address().port,
+    timeout: 2000,
   });
-  req.on('close', common.mustCall(() => server.close()));
 
-  let timeout_events = 0;
-  req.on('timeout', common.mustCall(() => {
-    timeout_events += 1;
+  req.setTimeout(1000);
+
+  req.on('socket', mustCall((socket) => {
+    strictEqual(socket.timeout, 2000);
+
+    socket.on('connect', mustCall(() => {
+      strictEqual(socket.timeout, 1000);
+
+      // Reschedule the timer to not wait 1 sec and make the test finish faster.
+      socket.setTimeout(10);
+      strictEqual(socket.timeout, 10);
+    }));
+  }));
+
+  req.on('error', expectsError({
+    type: Error,
+    code: 'ECONNRESET',
+    message: 'socket hang up'
   }));
-  req.end();
 
-  setTimeout(function() {
+  req.on('close', mustCall(() => {
+    server.close();
+  }));
+
+  req.on('timeout', mustCall(() => {
+    strictEqual(req.socket.listenerCount('timeout'), 0);
     req.destroy();
-    assert.strictEqual(timeout_events, 1);
-  }, common.platformTimeout(HTTP_CLIENT_TIMEOUT));
-}
+  }));
+}));
diff --git a/test/parallel/test-http-client-timeout-option-with-agent.js b/test/parallel/test-http-client-timeout-option-with-agent.js
@@ -1,58 +1,23 @@
 'use strict';
-const common = require('../common');
 
-// Test that when http request uses both timeout and agent,
-// timeout will work as expected.
+// Test that the request `timeout` option has precedence over the agent
+// `timeout` option.
 
-const assert = require('assert');
-const http = require('http');
+const { mustCall } = require('../common');
+const { Agent, get } = require('http');
+const { strictEqual } = require('assert');
 
-const HTTP_AGENT_TIMEOUT = 1000;
-const HTTP_CLIENT_TIMEOUT = 3000;
-
-const agent = new http.Agent({ timeout: HTTP_AGENT_TIMEOUT });
-const options = {
-  method: 'GET',
-  port: undefined,
-  host: '127.0.0.1',
-  path: '/',
-  timeout: HTTP_CLIENT_TIMEOUT,
-  agent,
-};
-
-const server = http.createServer(() => {
-  // Never respond.
-});
-
-server.listen(0, options.host, () => {
-  doRequest();
+const request = get({
+  agent: new Agent({ timeout: 50 }),
+  lookup: () => {},
+  timeout: 100
 });
 
-function doRequest() {
-  options.port = server.address().port;
-  const start = process.hrtime.bigint();
-  const req = http.request(options);
-  req.on('error', () => {
-    // This space is intentionally left blank.
-  });
-  req.on('close', common.mustCall(() => server.close()));
+request.on('socket', mustCall((socket) => {
+  strictEqual(socket.timeout, 100);
 
-  let timeout_events = 0;
-  req.on('timeout', common.mustCall(() => {
-    timeout_events += 1;
-    const duration = process.hrtime.bigint() - start;
-    // The timeout event cannot be precisely timed. It will delay
-    // some number of milliseconds.
-    assert.ok(
-      duration >= BigInt(HTTP_CLIENT_TIMEOUT * 1e6),
-      `duration ${duration}ms less than timeout ${HTTP_CLIENT_TIMEOUT}ms`
-    );
-  }));
-  req.end();
+  const listeners = socket.listeners('timeout');
 
-  setTimeout(() => {
-    req.destroy();
-    assert.strictEqual(timeout_events, 1);
-    // Ensure the `timeout` event fired only once.
-  }, common.platformTimeout(HTTP_CLIENT_TIMEOUT * 2));
-}
+  strictEqual(listeners.length, 1);
+  strictEqual(listeners[0], request.timeoutCb);
+}));
