Skip to content

Commit

Permalink
test: make http2 timeout test robust
Browse files Browse the repository at this point in the history
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: nodejs#20628

PR-URL: nodejs#24877
Fixes: nodejs#20628
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
  • Loading branch information
Trott committed Dec 8, 2018
1 parent 008b904 commit 3fe00ef
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
2 changes: 0 additions & 2 deletions test/sequential/sequential.status
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 13 additions & 5 deletions test/sequential/test-http2-session-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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() {
Expand All @@ -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);
Expand Down

0 comments on commit 3fe00ef

Please sign in to comment.