Skip to content

Commit 9f2d594

Browse files
santigimenosantigimeno
committed
test: fix flaky test-https-server-close- tests
Don't initiate the second connection until the first has been established. Refs: nodejs/reliability#289
1 parent 331088f commit 9f2d594

File tree

4 files changed

+101
-92
lines changed

4 files changed

+101
-92
lines changed

‎test/parallel/test-http-server-close-all.js

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,32 +26,34 @@ server.listen(0, function() {
2626
// Create a first request but never finish it
2727
const client1 = connect(port);
2828

29-
client1.on('close', common.mustCall());
30-
31-
client1.on('error', () => {});
29+
client1.on('connect', common.mustCall(() => {
30+
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
31+
const client2 = connect(port);
32+
let response = '';
3233

33-
client1.write('GET / HTTP/1.1');
34+
client2.on('data', common.mustCall((chunk) => {
35+
response += chunk.toString('utf-8');
3436

35-
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
36-
const client2 = connect(port);
37-
let response = '';
37+
if (response.endsWith('0\r\n\r\n')) {
38+
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
39+
assert.strictEqual(connections, 2);
3840

39-
client2.on('data', common.mustCall((chunk) => {
40-
response += chunk.toString('utf-8');
41+
server.closeAllConnections();
42+
server.close(common.mustCall());
4143

42-
if (response.endsWith('0\r\n\r\n')) {
43-
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
44-
assert.strictEqual(connections, 2);
44+
// This timer should never go off as the server.close should shut everything down
45+
setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
46+
}
47+
}));
4548

46-
server.closeAllConnections();
47-
server.close(common.mustCall());
49+
client2.on('close', common.mustCall());
4850

49-
// This timer should never go off as the server.close should shut everything down
50-
setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
51-
}
51+
client2.write('GET / HTTP/1.1\r\n\r\n');
5252
}));
5353

54-
client2.on('close', common.mustCall());
54+
client1.on('close', common.mustCall());
55+
56+
client1.on('error', () => {});
5557

56-
client2.write('GET / HTTP/1.1\r\n\r\n');
58+
client1.write('GET / HTTP/1.1');
5759
});

‎test/parallel/test-http-server-close-idle.js

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,42 +28,44 @@ server.listen(0, function() {
2828
// Create a first request but never finish it
2929
const client1 = connect(port);
3030

31-
client1.on('close', common.mustCall(() => {
32-
client1Closed = true;
33-
}));
31+
client1.on('connect', common.mustCall(() => {
32+
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
33+
const client2 = connect(port);
34+
let response = '';
3435

35-
client1.on('error', () => {});
36+
client2.on('data', common.mustCall((chunk) => {
37+
response += chunk.toString('utf-8');
3638

37-
client1.write('GET / HTTP/1.1');
38-
39-
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
40-
const client2 = connect(port);
41-
let response = '';
39+
if (response.endsWith('0\r\n\r\n')) {
40+
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
41+
assert.strictEqual(connections, 2);
4242

43-
client2.on('data', common.mustCall((chunk) => {
44-
response += chunk.toString('utf-8');
43+
server.closeIdleConnections();
44+
server.close(common.mustCall());
4545

46-
if (response.endsWith('0\r\n\r\n')) {
47-
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
48-
assert.strictEqual(connections, 2);
46+
// Check that only the idle connection got closed
47+
setTimeout(common.mustCall(() => {
48+
assert(!client1Closed);
49+
assert(client2Closed);
4950

50-
server.closeIdleConnections();
51-
server.close(common.mustCall());
51+
server.closeAllConnections();
52+
server.close(common.mustCall());
53+
}), common.platformTimeout(500)).unref();
54+
}
55+
}));
5256

53-
// Check that only the idle connection got closed
54-
setTimeout(common.mustCall(() => {
55-
assert(!client1Closed);
56-
assert(client2Closed);
57+
client2.on('close', common.mustCall(() => {
58+
client2Closed = true;
59+
}));
5760

58-
server.closeAllConnections();
59-
server.close(common.mustCall());
60-
}), common.platformTimeout(500)).unref();
61-
}
61+
client2.write('GET / HTTP/1.1\r\n\r\n');
6262
}));
6363

64-
client2.on('close', common.mustCall(() => {
65-
client2Closed = true;
64+
client1.on('close', common.mustCall(() => {
65+
client1Closed = true;
6666
}));
6767

68-
client2.write('GET / HTTP/1.1\r\n\r\n');
68+
client1.on('error', () => {});
69+
70+
client1.write('GET / HTTP/1.1');
6971
});

‎test/parallel/test-https-server-close-all.js

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,32 +37,34 @@ server.listen(0, function() {
3737
// Create a first request but never finish it
3838
const client1 = connect({ port, rejectUnauthorized: false });
3939

40-
client1.on('close', common.mustCall());
41-
42-
client1.on('error', () => {});
40+
client1.on('connect', common.mustCall(() => {
41+
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
42+
const client2 = connect({ port, rejectUnauthorized: false });
43+
let response = '';
4344

44-
client1.write('GET / HTTP/1.1');
45+
client2.on('data', common.mustCall((chunk) => {
46+
response += chunk.toString('utf-8');
4547

46-
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
47-
const client2 = connect({ port, rejectUnauthorized: false });
48-
let response = '';
48+
if (response.endsWith('0\r\n\r\n')) {
49+
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
50+
assert.strictEqual(connections, 2);
4951

50-
client2.on('data', common.mustCall((chunk) => {
51-
response += chunk.toString('utf-8');
52+
server.closeAllConnections();
53+
server.close(common.mustCall());
5254

53-
if (response.endsWith('0\r\n\r\n')) {
54-
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
55-
assert.strictEqual(connections, 2);
55+
// This timer should never go off as the server.close should shut everything down
56+
setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
57+
}
58+
}));
5659

57-
server.closeAllConnections();
58-
server.close(common.mustCall());
60+
client2.on('close', common.mustCall());
5961

60-
// This timer should never go off as the server.close should shut everything down
61-
setTimeout(common.mustNotCall(), common.platformTimeout(1500)).unref();
62-
}
62+
client2.write('GET / HTTP/1.1\r\n\r\n');
6363
}));
6464

65-
client2.on('close', common.mustCall());
65+
client1.on('close', common.mustCall());
66+
67+
client1.on('error', () => {});
6668

67-
client2.write('GET / HTTP/1.1\r\n\r\n');
69+
client1.write('GET / HTTP/1.1');
6870
});

‎test/parallel/test-https-server-close-idle.js

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -39,42 +39,45 @@ server.listen(0, function() {
3939
// Create a first request but never finish it
4040
const client1 = connect({ port, rejectUnauthorized: false });
4141

42-
client1.on('close', common.mustCall(() => {
43-
client1Closed = true;
44-
}));
42+
client1.on('connect', common.mustCall(() => {
43+
assert.strictEqual(connections, 1);
44+
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
45+
const client2 = connect({ port, rejectUnauthorized: false });
46+
let response = '';
4547

46-
client1.on('error', () => {});
48+
client2.on('data', common.mustCall((chunk) => {
49+
response += chunk.toString('utf-8');
4750

48-
client1.write('GET / HTTP/1.1');
49-
50-
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
51-
const client2 = connect({ port, rejectUnauthorized: false });
52-
let response = '';
51+
if (response.endsWith('0\r\n\r\n')) {
52+
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
53+
assert.strictEqual(connections, 2);
5354

54-
client2.on('data', common.mustCall((chunk) => {
55-
response += chunk.toString('utf-8');
55+
server.closeIdleConnections();
56+
server.close(common.mustCall());
5657

57-
if (response.endsWith('0\r\n\r\n')) {
58-
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
59-
assert.strictEqual(connections, 2);
58+
// Check that only the idle connection got closed
59+
setTimeout(common.mustCall(() => {
60+
assert(!client1Closed);
61+
assert(client2Closed);
6062

61-
server.closeIdleConnections();
62-
server.close(common.mustCall());
63+
server.closeAllConnections();
64+
server.close(common.mustCall());
65+
}), common.platformTimeout(500)).unref();
66+
}
67+
}));
6368

64-
// Check that only the idle connection got closed
65-
setTimeout(common.mustCall(() => {
66-
assert(!client1Closed);
67-
assert(client2Closed);
69+
client2.on('close', common.mustCall(() => {
70+
client2Closed = true;
71+
}));
6872

69-
server.closeAllConnections();
70-
server.close(common.mustCall());
71-
}), common.platformTimeout(500)).unref();
72-
}
73+
client2.write('GET / HTTP/1.1\r\n\r\n');
7374
}));
7475

75-
client2.on('close', common.mustCall(() => {
76-
client2Closed = true;
76+
client1.on('close', common.mustCall(() => {
77+
client1Closed = true;
7778
}));
7879

79-
client2.write('GET / HTTP/1.1\r\n\r\n');
80+
client1.on('error', () => {});
81+
82+
client1.write('GET / HTTP/1.1');
8083
});

0 commit comments

Comments
 (0)