From 425c589f75f23803a59ca27aa84ce40a8be81b4b Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 14 Mar 2018 07:39:07 -0700 Subject: [PATCH] test: fix flaky test-http2-settings-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. Fixes: https://github.com/nodejs/node/issues/18251 PR-URL: https://github.com/nodejs/node/pull/19349 Reviewed-By: James M Snell Reviewed-By: Anatoli Papirovski --- test/sequential/sequential.status | 1 - test/sequential/test-http2-settings-flood.js | 25 ++++++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/test/sequential/sequential.status b/test/sequential/sequential.status index b95db2a111ea67..ab1fb5e12bf56c 100644 --- a/test/sequential/sequential.status +++ b/test/sequential/sequential.status @@ -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] diff --git a/test/sequential/test-http2-settings-flood.js b/test/sequential/test-http2-settings-flood.js index bad4cec9a8d509..15b3696b9c298e 100644 --- a/test/sequential/test-http2-settings-flood.js +++ b/test/sequential/test-http2-settings-flood.js @@ -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 @@ -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(); })); @@ -30,9 +35,7 @@ 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 @@ -40,8 +43,10 @@ server.listen(0, common.mustCall(() => { // 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); }); }));