test: fix flaky test-https-server-close- tests

Don't initiate the second connection until the first has been established. Refs: nodejs/reliability#289 PR-URL: #43216 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Jul 12, 2022 · 1022c0d · 1022c0d
1 parent 3ca9e65
commit 1022c0d
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 92 deletions.
diff --git a/test/parallel/test-http-server-close-all.js b/test/parallel/test-http-server-close-all.js
@@ -26,32 +26,34 @@ server.listen(0, function() {
   // Create a first request but never finish it
   const client1 = connect(port);
 
-  client1.on('close', common.mustCall());
-
-  client1.on('error', () => {});
+  client1.on('connect', common.mustCall(() => {
+    // Create a second request, let it finish but leave the connection opened using HTTP keep-alive
+    const client2 = connect(port);
+    let response = '';
 
-  client1.write('GET / HTTP/1.1');
+    client2.on('data', common.mustCall((chunk) => {
+      response += chunk.toString('utf-8');
 
-  // Create a second request, let it finish but leave the connection opened using HTTP keep-alive
-  const client2 = connect(port);
-  let response = '';
+      if (response.endsWith('0\r\n\r\n')) {
+        assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
+        assert.strictEqual(connections, 2);
 
-  client2.on('data', common.mustCall((chunk) => {
-    response += chunk.toString('utf-8');
+        server.closeAllConnections();
+        server.close(common.mustCall());
 
-    if (response.endsWith('0\r\n\r\n')) {
-      assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
-      assert.strictEqual(connections, 2);
+        // This timer should never go off as the server.close should shut everything down
+        setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
+      }
+    }));
 
-      server.closeAllConnections();
-      server.close(common.mustCall());
+    client2.on('close', common.mustCall());
 
-      // This timer should never go off as the server.close should shut everything down
-      setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
-    }
+    client2.write('GET / HTTP/1.1\r\n\r\n');
   }));
 
-  client2.on('close', common.mustCall());
+  client1.on('close', common.mustCall());
+
+  client1.on('error', () => {});
 
-  client2.write('GET / HTTP/1.1\r\n\r\n');
+  client1.write('GET / HTTP/1.1');
 });
diff --git a/test/parallel/test-http-server-close-idle.js b/test/parallel/test-http-server-close-idle.js
@@ -28,42 +28,44 @@ server.listen(0, function() {
   // Create a first request but never finish it
   const client1 = connect(port);
 
-  client1.on('close', common.mustCall(() => {
-    client1Closed = true;
-  }));
+  client1.on('connect', common.mustCall(() => {
+    // Create a second request, let it finish but leave the connection opened using HTTP keep-alive
+    const client2 = connect(port);
+    let response = '';
 
-  client1.on('error', () => {});
+    client2.on('data', common.mustCall((chunk) => {
+      response += chunk.toString('utf-8');
 
-  client1.write('GET / HTTP/1.1');
-
-  // Create a second request, let it finish but leave the connection opened using HTTP keep-alive
-  const client2 = connect(port);
-  let response = '';
+      if (response.endsWith('0\r\n\r\n')) {
+        assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
+        assert.strictEqual(connections, 2);
 
-  client2.on('data', common.mustCall((chunk) => {
-    response += chunk.toString('utf-8');
+        server.closeIdleConnections();
+        server.close(common.mustCall());
 
-    if (response.endsWith('0\r\n\r\n')) {
-      assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
-      assert.strictEqual(connections, 2);
+        // Check that only the idle connection got closed
+        setTimeout(common.mustCall(() => {
+          assert(!client1Closed);
+          assert(client2Closed);
 
-      server.closeIdleConnections();
-      server.close(common.mustCall());
+          server.closeAllConnections();
+          server.close(common.mustCall());
+        }), common.platformTimeout(500)).unref();
+      }
+    }));
 
-      // Check that only the idle connection got closed
-      setTimeout(common.mustCall(() => {
-        assert(!client1Closed);
-        assert(client2Closed);
+    client2.on('close', common.mustCall(() => {
+      client2Closed = true;
+    }));
 
-        server.closeAllConnections();
-        server.close(common.mustCall());
-      }), common.platformTimeout(500)).unref();
-    }
+    client2.write('GET / HTTP/1.1\r\n\r\n');
   }));
 
-  client2.on('close', common.mustCall(() => {
-    client2Closed = true;
+  client1.on('close', common.mustCall(() => {
+    client1Closed = true;
   }));
 
-  client2.write('GET / HTTP/1.1\r\n\r\n');
+  client1.on('error', () => {});
+
+  client1.write('GET / HTTP/1.1');
 });
diff --git a/test/parallel/test-https-server-close-all.js b/test/parallel/test-https-server-close-all.js
@@ -37,32 +37,34 @@ server.listen(0, function() {
   // Create a first request but never finish it
   const client1 = connect({ port, rejectUnauthorized: false });
 
-  client1.on('close', common.mustCall());
-
-  client1.on('error', () => {});
+  client1.on('connect', common.mustCall(() => {
+    // Create a second request, let it finish but leave the connection opened using HTTP keep-alive
+    const client2 = connect({ port, rejectUnauthorized: false });
+    let response = '';
 
-  client1.write('GET / HTTP/1.1');
+    client2.on('data', common.mustCall((chunk) => {
+      response += chunk.toString('utf-8');
 
-  // Create a second request, let it finish but leave the connection opened using HTTP keep-alive
-  const client2 = connect({ port, rejectUnauthorized: false });
-  let response = '';
+      if (response.endsWith('0\r\n\r\n')) {
+        assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
+        assert.strictEqual(connections, 2);
 
-  client2.on('data', common.mustCall((chunk) => {
-    response += chunk.toString('utf-8');
+        server.closeAllConnections();
+        server.close(common.mustCall());
 
-    if (response.endsWith('0\r\n\r\n')) {
-      assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
-      assert.strictEqual(connections, 2);
+        // This timer should never go off as the server.close should shut everything down
+        setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
+      }
+    }));
 
-      server.closeAllConnections();
-      server.close(common.mustCall());
+    client2.on('close', common.mustCall());
 
-      // This timer should never go off as the server.close should shut everything down
-      setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
-    }
+    client2.write('GET / HTTP/1.1\r\n\r\n');
   }));
 
-  client2.on('close', common.mustCall());
+  client1.on('close', common.mustCall());
+
+  client1.on('error', () => {});
 
-  client2.write('GET / HTTP/1.1\r\n\r\n');
+  client1.write('GET / HTTP/1.1');
 });
diff --git a/test/parallel/test-https-server-close-idle.js b/test/parallel/test-https-server-close-idle.js
@@ -39,42 +39,44 @@ server.listen(0, function() {
   // Create a first request but never finish it
   const client1 = connect({ port, rejectUnauthorized: false });
 
-  client1.on('close', common.mustCall(() => {
-    client1Closed = true;
-  }));
+  client1.on('connect', common.mustCall(() => {
+    // Create a second request, let it finish but leave the connection opened using HTTP keep-alive
+    const client2 = connect({ port, rejectUnauthorized: false });
+    let response = '';
 
-  client1.on('error', () => {});
+    client2.on('data', common.mustCall((chunk) => {
+      response += chunk.toString('utf-8');
 
-  client1.write('GET / HTTP/1.1');
-
-  // Create a second request, let it finish but leave the connection opened using HTTP keep-alive
-  const client2 = connect({ port, rejectUnauthorized: false });
-  let response = '';
+      if (response.endsWith('0\r\n\r\n')) {
+        assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
+        assert.strictEqual(connections, 2);
 
-  client2.on('data', common.mustCall((chunk) => {
-    response += chunk.toString('utf-8');
+        server.closeIdleConnections();
+        server.close(common.mustCall());
 
-    if (response.endsWith('0\r\n\r\n')) {
-      assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
-      assert.strictEqual(connections, 2);
+        // Check that only the idle connection got closed
+        setTimeout(common.mustCall(() => {
+          assert(!client1Closed);
+          assert(client2Closed);
 
-      server.closeIdleConnections();
-      server.close(common.mustCall());
+          server.closeAllConnections();
+          server.close(common.mustCall());
+        }), common.platformTimeout(500)).unref();
+      }
+    }));
 
-      // Check that only the idle connection got closed
-      setTimeout(common.mustCall(() => {
-        assert(!client1Closed);
-        assert(client2Closed);
+    client2.on('close', common.mustCall(() => {
+      client2Closed = true;
+    }));
 
-        server.closeAllConnections();
-        server.close(common.mustCall());
-      }), common.platformTimeout(500)).unref();
-    }
+    client2.write('GET / HTTP/1.1\r\n\r\n');
   }));
 
-  client2.on('close', common.mustCall(() => {
-    client2Closed = true;
+  client1.on('close', common.mustCall(() => {
+    client1Closed = true;
   }));
 
-  client2.write('GET / HTTP/1.1\r\n\r\n');
+  client1.on('error', () => {});
+
+  client1.write('GET / HTTP/1.1');
 });