From 91ce957037cee71c6bb339a99dd6653c0ae9b4c1 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 6 Dec 2018 10:21:05 -0800 Subject: [PATCH] test: make http2 timeout test robust Instead of using magic values for the server timeout in test-http2-session-timeout, measure the duration of the first request (which should be longer than subsequent requests) and use that value. Fixes: https://github.com/nodejs/node/issues/20628 PR-URL: https://github.com/nodejs/node/pull/24877 Fixes: https://github.com/https://github.com/nodejs/node/issues/20628 Reviewed-By: Luigi Pinca Reviewed-By: Matteo Collina Reviewed-By: Anatoli Papirovski Reviewed-By: Colin Ihrig Reviewed-By: Trivikram Kamat --- test/sequential/sequential.status | 2 -- test/sequential/test-http2-session-timeout.js | 18 +++++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/test/sequential/sequential.status b/test/sequential/sequential.status index 89f639777b5ca9..d9963bc6f77328 100644 --- a/test/sequential/sequential.status +++ b/test/sequential/sequential.status @@ -23,5 +23,3 @@ test-http2-large-file: PASS, FLAKY [$system==aix] [$arch==arm] -# https://github.com/nodejs/node/issues/20628 -test-http2-session-timeout: PASS,FLAKY diff --git a/test/sequential/test-http2-session-timeout.js b/test/sequential/test-http2-session-timeout.js index 14a31bad9bd0fc..0fa6a2c13c6b6c 100644 --- a/test/sequential/test-http2-session-timeout.js +++ b/test/sequential/test-http2-session-timeout.js @@ -6,15 +6,15 @@ if (!common.hasCrypto) const assert = require('assert'); const http2 = require('http2'); -const serverTimeout = common.platformTimeout(200); - let requests = 0; const mustNotCall = () => { assert.fail(`Timeout after ${requests} request(s)`); }; const server = http2.createServer(); -server.timeout = serverTimeout; +// Disable server timeout until first request. We will set the timeout based on +// how long the first request takes. +server.timeout = 0; server.on('request', (req, res) => res.end()); server.on('timeout', mustNotCall); @@ -24,7 +24,7 @@ server.listen(0, common.mustCall(() => { const url = `http://localhost:${port}`; const client = http2.connect(url); - const startTime = process.hrtime(); + let startTime = process.hrtime(); makeReq(); function makeReq() { @@ -42,7 +42,15 @@ server.listen(0, common.mustCall(() => { request.on('end', () => { const diff = process.hrtime(startTime); const milliseconds = (diff[0] * 1e3 + diff[1] / 1e6); - if (milliseconds < serverTimeout * 2) { + if (server.timeout === 0) { + // Set the timeout now. First connection will take significantly longer + // than subsequent connections, so using the duration of the first + // connection as the timeout should be robust. Double it anyway for good + // measure. + server.timeout = milliseconds * 2; + startTime = process.hrtime(); + makeReq(); + } else if (milliseconds < server.timeout * 2) { makeReq(); } else { server.removeListener('timeout', mustNotCall);