From e20f3577d025e8ef60c21636587ce51135c44b98 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Tue, 30 May 2017 13:06:52 -0700
Subject: [PATCH] test: improve test-https-server-keep-alive-timeout

The test is flaky under load. These changes greatly improve reliability.

* Use a recurring interval to determine when the test should end rather
  than a timer.
* Increase server timeout to 500ms to allow for events being delayed by
  system load

Changing to an interval has the added benefit of reducing the test run
time from over 2 seconds to under 1 second.

Fixes: https://github.com/nodejs/node/issues/13307

PR-URL: https://github.com/nodejs/node/pull/13312
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
---
 .../test-https-server-keep-alive-timeout.js   | 38 +++++--------------
 1 file changed, 10 insertions(+), 28 deletions(-)

diff --git a/test/parallel/test-https-server-keep-alive-timeout.js b/test/parallel/test-https-server-keep-alive-timeout.js
index c89041489e0aef..d1e9ed67889ac6 100644
--- a/test/parallel/test-https-server-keep-alive-timeout.js
+++ b/test/parallel/test-https-server-keep-alive-timeout.js
@@ -30,24 +30,20 @@ function run() {
 }
 
 test(function serverKeepAliveTimeoutWithPipeline(cb) {
-  let socket;
-  let destroyedSockets = 0;
-  let timeoutCount = 0;
   let requestCount = 0;
   process.on('exit', function() {
-    assert.strictEqual(timeoutCount, 1);
     assert.strictEqual(requestCount, 3);
-    assert.strictEqual(destroyedSockets, 1);
   });
   const server = https.createServer(serverOptions, (req, res) => {
-    socket = req.socket;
     requestCount++;
     res.end();
   });
-  server.setTimeout(200, (socket) => {
-    timeoutCount++;
+  server.setTimeout(500, common.mustCall((socket) => {
+    // End this test and call `run()` for the next test (if any).
     socket.destroy();
-  });
+    server.close();
+    cb();
+  }));
   server.keepAliveTimeout = 50;
   server.listen(0, common.mustCall(() => {
     const options = {
@@ -60,32 +56,23 @@ test(function serverKeepAliveTimeoutWithPipeline(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');
     });
-    setTimeout(() => {
-      server.close();
-      if (socket.destroyed) destroyedSockets++;
-      cb();
-    }, 1000);
   }));
 });
 
 test(function serverNoEndKeepAliveTimeoutWithPipeline(cb) {
-  let socket;
-  let destroyedSockets = 0;
-  let timeoutCount = 0;
   let requestCount = 0;
   process.on('exit', () => {
-    assert.strictEqual(timeoutCount, 1);
     assert.strictEqual(requestCount, 3);
-    assert.strictEqual(destroyedSockets, 1);
   });
   const server = https.createServer(serverOptions, (req, res) => {
-    socket = req.socket;
     requestCount++;
   });
-  server.setTimeout(200, (socket) => {
-    timeoutCount++;
+  server.setTimeout(500, common.mustCall((socket) => {
+    // End this test and call `run()` for the next test (if any).
     socket.destroy();
-  });
+    server.close();
+    cb();
+  }));
   server.keepAliveTimeout = 50;
   server.listen(0, common.mustCall(() => {
     const options = {
@@ -98,10 +85,5 @@ test(function serverNoEndKeepAliveTimeoutWithPipeline(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');
     });
-    setTimeout(() => {
-      server.close();
-      if (socket && socket.destroyed) destroyedSockets++;
-      cb();
-    }, 1000);
   }));
 });