From c0c46610fa6cdc80bdee78e765b68583ab8bebb6 Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Thu, 26 May 2022 19:45:29 +0200 Subject: [PATCH] test: fix flaky test-https-server-close- tests Don't initiate the second connection until the first has been established. Refs: https://github.com/nodejs/reliability/issues/289 --- test/parallel/test-http-server-close-all.js | 40 ++++++------- test/parallel/test-http-server-close-idle.js | 56 ++++++++++--------- test/parallel/test-https-server-close-all.js | 40 ++++++------- test/parallel/test-https-server-close-idle.js | 56 ++++++++++--------- 4 files changed, 100 insertions(+), 92 deletions(-) diff --git a/test/parallel/test-http-server-close-all.js b/test/parallel/test-http-server-close-all.js index 79fc5c75b70daf..cf7d90b868162d 100644 --- 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 index b9389f1e599c72..5218212996b159 100644 --- 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 index 408af625d1f773..c388260db3b484 100644 --- 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 index ea43c4367cb738..eb5a8c3585bb96 100644 --- 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'); });