Skip to content

Commit 91ce957

Browse files
TrottBethGriggs
authored andcommitted
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: #20628 PR-URL: #24877 Fixes: #20628 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
1 parent 3d87688 commit 91ce957

File tree

2 files changed

+13
-7
lines changed

2 files changed

+13
-7
lines changed

test/sequential/sequential.status

-2
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,3 @@ test-http2-large-file: PASS, FLAKY
2323
[$system==aix]
2424

2525
[$arch==arm]
26-
# https://github.com/nodejs/node/issues/20628
27-
test-http2-session-timeout: PASS,FLAKY

test/sequential/test-http2-session-timeout.js

+13-5
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ if (!common.hasCrypto)
66
const assert = require('assert');
77
const http2 = require('http2');
88

9-
const serverTimeout = common.platformTimeout(200);
10-
119
let requests = 0;
1210
const mustNotCall = () => {
1311
assert.fail(`Timeout after ${requests} request(s)`);
1412
};
1513

1614
const server = http2.createServer();
17-
server.timeout = serverTimeout;
15+
// Disable server timeout until first request. We will set the timeout based on
16+
// how long the first request takes.
17+
server.timeout = 0;
1818

1919
server.on('request', (req, res) => res.end());
2020
server.on('timeout', mustNotCall);
@@ -24,7 +24,7 @@ server.listen(0, common.mustCall(() => {
2424

2525
const url = `http://localhost:${port}`;
2626
const client = http2.connect(url);
27-
const startTime = process.hrtime();
27+
let startTime = process.hrtime();
2828
makeReq();
2929

3030
function makeReq() {
@@ -42,7 +42,15 @@ server.listen(0, common.mustCall(() => {
4242
request.on('end', () => {
4343
const diff = process.hrtime(startTime);
4444
const milliseconds = (diff[0] * 1e3 + diff[1] / 1e6);
45-
if (milliseconds < serverTimeout * 2) {
45+
if (server.timeout === 0) {
46+
// Set the timeout now. First connection will take significantly longer
47+
// than subsequent connections, so using the duration of the first
48+
// connection as the timeout should be robust. Double it anyway for good
49+
// measure.
50+
server.timeout = milliseconds * 2;
51+
startTime = process.hrtime();
52+
makeReq();
53+
} else if (milliseconds < server.timeout * 2) {
4654
makeReq();
4755
} else {
4856
server.removeListener('timeout', mustNotCall);

0 commit comments

Comments
 (0)