Browse Source

test: fix flaky test-timers-blocking-callback

This test was failing on FreeBSD from time to time in the project CI.
The bug the test was written for would guarantee that the timer would
fire at least 100ms late, but the assertion was firing if it was more
than 50ms late.

This changes the assertion to fire when the timer is more than 100ms
late.

I ran a modified version of this test using 0.10.38 (which has the bug)
and 0.10.39 (which has the fix) to confirm that it still fails in the
buggy one and passes in the fixed one.

PR-URL: https://github.com/nodejs/node/pull/9198
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
v6
Rich Trott 8 years ago
parent
commit
5d818ccc22
  1. 11
      test/parallel/test-timers-blocking-callback.js

11
test/parallel/test-timers-blocking-callback.js

@ -9,13 +9,13 @@
* *
* The reason was that the value that represents the current time was not * The reason was that the value that represents the current time was not
* updated between the time the original callback was called and the time * updated between the time the original callback was called and the time
* the added timer was processed by timers.listOnTimeout. That lead the * the added timer was processed by timers.listOnTimeout. That led the
* logic in timers.listOnTimeout to do an incorrect computation that made * logic in timers.listOnTimeout to do an incorrect computation that made
* the added timer fire with a timeout of scheduledTimeout + * the added timer fire with a timeout of scheduledTimeout +
* timeSpentInCallback. * timeSpentInCallback.
* *
* This test makes sure that a timer added by another timer's callback * This test makes sure that a timer added by another timer's callback
* fire with the expected timeout. * fires with the expected timeout.
* *
* It makes sure that it works when the timers list for a given timeout is * It makes sure that it works when the timers list for a given timeout is
* empty (see testAddingTimerToEmptyTimersList) and when the timers list * empty (see testAddingTimerToEmptyTimersList) and when the timers list
@ -44,12 +44,11 @@ function blockingCallback(callback) {
if (nbBlockingCallbackCalls > 1) { if (nbBlockingCallbackCalls > 1) {
latestDelay = Timer.now() - timeCallbackScheduled; latestDelay = Timer.now() - timeCallbackScheduled;
// Even if timers can fire later than when they've been scheduled // Even if timers can fire later than when they've been scheduled
// to fire, they should more than 50% later with a timeout of // to fire, they shouldn't generally be more than 100% late in this case.
// 100ms. Firing later than that would mean that we hit the regression // But they are guaranteed to be at least 100ms late given the bug in
// highlighted in
// https://github.com/nodejs/node-v0.x-archive/issues/15447 and // https://github.com/nodejs/node-v0.x-archive/issues/15447 and
// https://github.com/nodejs/node-v0.x-archive/issues/9333.. // https://github.com/nodejs/node-v0.x-archive/issues/9333..
assert(latestDelay < TIMEOUT * 1.5); assert(latestDelay < TIMEOUT * 2);
if (callback) if (callback)
return callback(); return callback();
} else { } else {

Loading…
Cancel
Save