From 54285aace7b9ac8e5f014602eb3469bd5e6f0008 Mon Sep 17 00:00:00 2001 From: Michael Cornacchia Date: Tue, 27 Oct 2015 12:55:09 -0400 Subject: [PATCH 1/2] test: fix race condition in unrefd interval test Before this commit, test-timers-unrefd-interval-still-fire required a 1ms interval to fire 5 times before a 100ms timeout to pass which could cause intermittent failures in CI and in issue #1781. This commit gives the test up to 5x as long to complete while allowing the test to complete as quickly as before when possible. --- .../test-timers-unrefd-interval-still-fires.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-timers-unrefd-interval-still-fires.js b/test/parallel/test-timers-unrefd-interval-still-fires.js index 98bd278b451613..5fe8a98affe490 100644 --- a/test/parallel/test-timers-unrefd-interval-still-fires.js +++ b/test/parallel/test-timers-unrefd-interval-still-fires.js @@ -15,6 +15,14 @@ var timer = setInterval(function() { timer.unref(); -setTimeout(function onTimeout() { - assert.strictEqual(nbIntervalFired, N); -}, 100); +var checkInterval = 100; +var maxNbChecks = 5; +var nbChecks = 1; +setTimeout(function check() { + if (nbChecks >= maxNbChecks || nbIntervalFired == N) { + assert.strictEqual(nbIntervalFired, N); + return; + } + ++nbChecks; + setTimeout(check, checkInterval); +}, checkInterval); From 06d1980353ce26ca1cca5b79cde6b34d40af71d5 Mon Sep 17 00:00:00 2001 From: Michael Cornacchia Date: Wed, 28 Oct 2015 13:58:29 -0400 Subject: [PATCH 2/2] test: clear timeout on last unrefd interval fire --- ...test-timers-unrefd-interval-still-fires.js | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/test/parallel/test-timers-unrefd-interval-still-fires.js b/test/parallel/test-timers-unrefd-interval-still-fires.js index 5fe8a98affe490..a716c6ed645cbd 100644 --- a/test/parallel/test-timers-unrefd-interval-still-fires.js +++ b/test/parallel/test-timers-unrefd-interval-still-fires.js @@ -2,27 +2,27 @@ /* * This test is a regression test for joyent/node#8900. */ -require('../common'); -var assert = require('assert'); +const common = require('../common'); +const assert = require('assert'); -var N = 5; +const TEST_DURATION = common.platformTimeout(100); +const N = 5; var nbIntervalFired = 0; -var timer = setInterval(function() { + +const keepOpen = setTimeout(() => { + console.error('[FAIL] Interval fired %d/%d times.', nbIntervalFired, N); + throw new Error('Test timed out. keepOpen was not canceled.'); +}, TEST_DURATION); + +const timer = setInterval(() => { ++nbIntervalFired; - if (nbIntervalFired === N) + if (nbIntervalFired === N) { clearInterval(timer); + timer._onTimeout = () => { + throw new Error('Unrefd interval fired after being cleared.'); + }; + setImmediate(() => clearTimeout(keepOpen)); + } }, 1); timer.unref(); - -var checkInterval = 100; -var maxNbChecks = 5; -var nbChecks = 1; -setTimeout(function check() { - if (nbChecks >= maxNbChecks || nbIntervalFired == N) { - assert.strictEqual(nbIntervalFired, N); - return; - } - ++nbChecks; - setTimeout(check, checkInterval); -}, checkInterval);