Skip to content

Commit

Permalink
test: fix flaky test-https-server-close- tests
Browse files Browse the repository at this point in the history 
Don't initiate the second connection until the first has been
established.

Refs: nodejs/reliability#289
  • Loading branch information
@santigimeno
santigimeno committed May 26, 2022
1 parent 331088f commit 9f2d594
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 92 deletions.
40 changes: 21 additions & 19 deletions test/parallel/test-http-server-close-all.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
56 changes: 29 additions & 27 deletions test/parallel/test-http-server-close-idle.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
40 changes: 21 additions & 19 deletions test/parallel/test-https-server-close-all.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
57 changes: 30 additions & 27 deletions test/parallel/test-https-server-close-idle.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,42 +39,45 @@ 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(() => {
assert.strictEqual(connections, 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 = '';

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');
});

0 comments on commit 9f2d594

Please sign in to comment.