Skip to content

Commit

Permalink
test: fix flaky test-http2-settings-flood
Browse files Browse the repository at this point in the history
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.

Fixes: nodejs#18251

PR-URL: nodejs#19349
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
  • Loading branch information
Trott committed Mar 16, 2018
1 parent d20f1f0 commit 879f521
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 11 deletions.
1 change: 0 additions & 1 deletion test/sequential/sequential.status
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ test-inspector-bindings : PASS, FLAKY
test-inspector-debug-end : PASS, FLAKY
test-inspector-async-hook-setup-at-signal: PASS, FLAKY
test-http2-ping-flood : PASS, FLAKY
test-http2-settings-flood : PASS, FLAKY

[$system==linux]

Expand Down
25 changes: 15 additions & 10 deletions test/sequential/test-http2-settings-flood.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const http2 = require('http2');
const net = require('net');

const http2util = require('../common/http2');

// Test that settings flooding causes the session to be torn down
Expand All @@ -14,13 +16,16 @@ const kSettings = new http2util.SettingsFrame();

const server = http2.createServer();

let interval;

server.on('stream', common.mustNotCall());
server.on('session', common.mustCall((session) => {
session.on('error', common.expectsError({
code: 'ERR_HTTP2_ERROR',
message:
'Flooding was detected in this HTTP/2 session, and it must be closed'
}));
session.on('error', (e) => {
assert.strictEqual(e.code, 'ERR_HTTP2_ERROR');
assert(e.message.includes('Flooding was detected'));
clearInterval(interval);
});

session.on('close', common.mustCall(() => {
server.close();
}));
Expand All @@ -30,18 +35,18 @@ server.listen(0, common.mustCall(() => {
const client = net.connect(server.address().port);

// nghttp2 uses a limit of 10000 items in it's outbound queue.
// If this number is exceeded, a flooding error is raised. Set
// this lim higher to account for the ones that nghttp2 is
// successfully able to respond to.
// If this number is exceeded, a flooding error is raised.
// TODO(jasnell): Unfortunately, this test is inherently flaky because
// it is entirely dependent on how quickly the server is able to handle
// the inbound frames and whether those just happen to overflow nghttp2's
// outbound queue. The threshold at which the flood error occurs can vary
// from one system to another, and from one test run to another.
client.on('connect', common.mustCall(() => {
client.write(http2util.kClientMagic, () => {
for (let n = 0; n < 35000; n++)
client.write(kSettings.data);
interval = setInterval(() => {
for (let n = 0; n < 10000; n++)
client.write(kSettings.data);
}, 1);
});
}));

Expand Down

0 comments on commit 879f521

Please sign in to comment.