From 7b3a53ae7d2f84cb604ca8960049ec34591d1f03 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 16 Mar 2018 10:50:03 -0700 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/19395 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- test/sequential/sequential.status | 1 - test/sequential/test-http2-ping-flood.js | 24 ++++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/test/sequential/sequential.status b/test/sequential/sequential.status index ab1fb5e12bf56c..cf4763b00129cc 100644 --- a/test/sequential/sequential.status +++ b/test/sequential/sequential.status @@ -11,7 +11,6 @@ test-inspector-async-call-stack : PASS, FLAKY 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 [$system==linux] diff --git a/test/sequential/test-http2-ping-flood.js b/test/sequential/test-http2-ping-flood.js index 5b47d51be9c5a8..b414aca8a4703a 100644 --- a/test/sequential/test-http2-ping-flood.js +++ b/test/sequential/test-http2-ping-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 ping flooding causes the session to be torn down @@ -15,13 +17,15 @@ const kPing = new http2util.PingFrame(); 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(); })); @@ -31,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 @@ -42,8 +44,10 @@ server.listen(0, common.mustCall(() => { client.on('connect', common.mustCall(() => { client.write(http2util.kClientMagic, () => { client.write(kSettings.data, () => { - for (let n = 0; n < 35000; n++) - client.write(kPing.data); + interval = setInterval(() => { + for (let n = 0; n < 10000; n++) + client.write(kPing.data); + }, 1); }); }); }));