Skip to content

Commit 1022c0d

Browse files
santigimenotargos
santigimeno
authored and
targos
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 PR-URL: #43216 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 3ca9e65 commit 1022c0d

4 files changed

+100
-92
lines changed

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

+21-19
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

+29-27
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

+21-19
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

+29-27
Original file line numberDiff line numberDiff line change
@@ -39,42 +39,44 @@ 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+
// Create a second request, let it finish but leave the connection opened using HTTP keep-alive
44+
const client2 = connect({ port, rejectUnauthorized: false });
45+
let response = '';
4546

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

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 = '';
50+
if (response.endsWith('0\r\n\r\n')) {
51+
assert(response.startsWith('HTTP/1.1 200 OK\r\nConnection: keep-alive'));
52+
assert.strictEqual(connections, 2);
5353

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

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);
57+
// Check that only the idle connection got closed
58+
setTimeout(common.mustCall(() => {
59+
assert(!client1Closed);
60+
assert(client2Closed);
6061

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

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

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

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

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

0 commit comments

Comments
 (0)