Skip to content

Commit e3ce084

Browse files
TrottMylesBorins
authored andcommitted
test: fix flaky test-http2-ping-flood
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. PR-URL: #19395 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 0e6f720 commit e3ce084

File tree

2 files changed

+14
-11
lines changed

2 files changed

+14
-11
lines changed

test/sequential/sequential.status

-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ test-inspector-async-call-stack : PASS, FLAKY
1111
test-inspector-bindings : PASS, FLAKY
1212
test-inspector-debug-end : PASS, FLAKY
1313
test-inspector-async-hook-setup-at-signal: PASS, FLAKY
14-
test-http2-ping-flood : PASS, FLAKY
1514

1615
[$system==linux]
1716

test/sequential/test-http2-ping-flood.js

+14-10
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ const common = require('../common');
44
if (!common.hasCrypto)
55
common.skip('missing crypto');
66

7+
const assert = require('assert');
78
const http2 = require('http2');
89
const net = require('net');
10+
911
const http2util = require('../common/http2');
1012

1113
// Test that ping flooding causes the session to be torn down
@@ -15,13 +17,15 @@ const kPing = new http2util.PingFrame();
1517

1618
const server = http2.createServer();
1719

20+
let interval;
21+
1822
server.on('stream', common.mustNotCall());
1923
server.on('session', common.mustCall((session) => {
20-
session.on('error', common.expectsError({
21-
code: 'ERR_HTTP2_ERROR',
22-
message:
23-
'Flooding was detected in this HTTP/2 session, and it must be closed'
24-
}));
24+
session.on('error', (e) => {
25+
assert.strictEqual(e.code, 'ERR_HTTP2_ERROR');
26+
assert(e.message.includes('Flooding was detected'));
27+
clearInterval(interval);
28+
});
2529
session.on('close', common.mustCall(() => {
2630
server.close();
2731
}));
@@ -31,9 +35,7 @@ server.listen(0, common.mustCall(() => {
3135
const client = net.connect(server.address().port);
3236

3337
// nghttp2 uses a limit of 10000 items in it's outbound queue.
34-
// If this number is exceeded, a flooding error is raised. Set
35-
// this lim higher to account for the ones that nghttp2 is
36-
// successfully able to respond to.
38+
// If this number is exceeded, a flooding error is raised.
3739
// TODO(jasnell): Unfortunately, this test is inherently flaky because
3840
// it is entirely dependent on how quickly the server is able to handle
3941
// the inbound frames and whether those just happen to overflow nghttp2's
@@ -42,8 +44,10 @@ server.listen(0, common.mustCall(() => {
4244
client.on('connect', common.mustCall(() => {
4345
client.write(http2util.kClientMagic, () => {
4446
client.write(kSettings.data, () => {
45-
for (let n = 0; n < 35000; n++)
46-
client.write(kPing.data);
47+
interval = setInterval(() => {
48+
for (let n = 0; n < 10000; n++)
49+
client.write(kPing.data);
50+
}, 1);
4751
});
4852
});
4953
}));

0 commit comments

Comments
 (0)