Skip to content

Commit

Permalink
test: fix flaky http(s)-set-server-timeout tests
Browse files Browse the repository at this point in the history
The tests include a callback that might not be invoked but is wrapped in
common.mustCall(). Remove the common.mustCall() wrapper and add a
comment explaining that it should not be added.

Add a new test case that sets the timeout to 1ms and waits for both the
connection handler and the timeout handler to be invoked. This version
keeps the common.mustCall() wrapper intact around the connection handler
(although it's mostly semantic and not necessary for the test as the
test will certainly fail or time out if that handler isn't invoked).

Fixes: nodejs#11768
  • Loading branch information
Trott committed Jul 24, 2017
1 parent 5c2d1af commit 845e15d
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 9 deletions.
43 changes: 41 additions & 2 deletions test/parallel/test-http-set-timeout-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,12 @@ test(function serverResponseTimeoutWithPipeline(cb) {
});

test(function idleTimeout(cb) {
const server = http.createServer(common.mustCall((req, res) => {
// Do not wrap the callback in common.mustCall(). It might not be invoked.
const server = http.createServer((req, res) => {
req.on('timeout', common.mustNotCall());
res.on('timeout', common.mustNotCall());
res.end();
}));
});
const s = server.setTimeout(50, common.mustCall((socket) => {
socket.destroy();
server.close();
Expand All @@ -174,3 +175,41 @@ test(function idleTimeout(cb) {
});
}));
});

test(function fastTimeout(cb) {
let connectionHandlerInvoked = false;
let timeoutHandlerInvoked = false;
let connectionSocket;

function invokeCallbackIfDone() {
if (connectionHandlerInvoked && timeoutHandlerInvoked) {
connectionSocket.destroy();
server.close();
cb();
}
}

const server = http.createServer(common.mustCall((req, res) => {
req.on('timeout', common.mustNotCall());
res.on('timeout', common.mustNotCall());
res.end();
connectionHandlerInvoked = true;
invokeCallbackIfDone();
}));
const s = server.setTimeout(1, common.mustCall((socket) => {
connectionSocket = socket;
timeoutHandlerInvoked = true;
invokeCallbackIfDone();
}));
assert.ok(s instanceof http.Server);
server.listen(common.mustCall(() => {
const options = {
port: server.address().port,
allowHalfOpen: true,
};
const c = net.connect(options, () => {
c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
// Keep-Alive
});
}));
});
54 changes: 47 additions & 7 deletions test/parallel/test-https-set-timeout-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,12 @@ test(function serverResponseTimeoutWithPipeline(cb) {
});

test(function idleTimeout(cb) {
const server = https.createServer(
serverOptions,
common.mustCall((req, res) => {
req.on('timeout', common.mustNotCall());
res.on('timeout', common.mustNotCall());
res.end();
}));
// Do not wrap the callback in common.mustCall(). It might not be invoked.
const server = https.createServer(serverOptions, (req, res) => {
req.on('timeout', common.mustNotCall());
res.on('timeout', common.mustNotCall());
res.end();
});
const s = server.setTimeout(50, common.mustCall((socket) => {
socket.destroy();
server.close();
Expand All @@ -200,3 +199,44 @@ test(function idleTimeout(cb) {
});
}));
});

test(function fastTimeout(cb) {
let connectionHandlerInvoked = false;
let timeoutHandlerInvoked = false;
let connectionSocket;

function invokeCallbackIfDone() {
if (connectionHandlerInvoked && timeoutHandlerInvoked) {
connectionSocket.destroy();
server.close();
cb();
}
}

const server = https.createServer(serverOptions, common.mustCall(
(req, res) => {
req.on('timeout', common.mustNotCall());
res.on('timeout', common.mustNotCall());
res.end();
connectionHandlerInvoked = true;
invokeCallbackIfDone();
}
));
const s = server.setTimeout(1, common.mustCall((socket) => {
connectionSocket = socket;
timeoutHandlerInvoked = true;
invokeCallbackIfDone();
}));
assert.ok(s instanceof https.Server);
server.listen(common.mustCall(() => {
const options = {
port: server.address().port,
allowHalfOpen: true,
rejectUnauthorized: false
};
const c = tls.connect(options, () => {
c.write('GET /1 HTTP/1.1\r\nHost: localhost\r\n\r\n');
// Keep-Alive
});
}));
});

0 comments on commit 845e15d

Please sign in to comment.