Skip to content

Commit

Permalink
test: fix flaky test-https-set-timeout-server
Browse files Browse the repository at this point in the history
Because of a race condition, connection listener may not be invoked if
test is run under load. Remove `common.mustCall()` wrapper from the
listener. Move the test to `parallel` because it now works under load.
Make similar change to http test to keep them in synch even though it is
much harder to trigger the race in http.

PR-URL: #14134
Fixes: #14133
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
  • Loading branch information
Trott authored and MylesBorins committed Sep 5, 2017
1 parent 01d82d8 commit ae7eeff
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
7 changes: 4 additions & 3 deletions test/parallel/test-http-set-timeout-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ function run() {
}

test(function serverTimeout(cb) {
const server = http.createServer(common.mustCall((req, res) => {
// just do nothing, we should get a timeout event.
}));
const server = http.createServer((req, res) => {
// Do nothing. We should get a timeout event.
// Might not be invoked. Do not wrap in common.mustCall().
});
server.listen(common.mustCall(() => {
const s = server.setTimeout(50, common.mustCall((socket) => {
socket.destroy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ function run() {
test(function serverTimeout(cb) {
const server = https.createServer(
serverOptions,
common.mustCall((req, res) => {
// just do nothing, we should get a timeout event.
}));
(req, res) => {
// Do nothing. We should get a timeout event.
// Might not be invoked. Do not wrap in common.mustCall().
});
server.listen(common.mustCall(() => {
const s = server.setTimeout(50, common.mustCall((socket) => {
socket.destroy();
Expand Down

0 comments on commit ae7eeff

Please sign in to comment.