Skip to content

Commit

Permalink
test: fix flaky test-timers-blocking-callback
Browse files Browse the repository at this point in the history
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: #9198
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Julien Gilli <[email protected]>
  • Loading branch information
Trott authored and jasnell committed Oct 24, 2016
1 parent 5ba02bf commit a717be8
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions test/parallel/test-timers-blocking-callback.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
*
* 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
* 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
* the added timer fire with a timeout of scheduledTimeout +
* timeSpentInCallback.
*
* 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
* empty (see testAddingTimerToEmptyTimersList) and when the timers list
Expand Down Expand Up @@ -44,12 +44,11 @@ function blockingCallback(callback) {
if (nbBlockingCallbackCalls > 1) {
latestDelay = Timer.now() - timeCallbackScheduled;
// Even if timers can fire later than when they've been scheduled
// to fire, they should more than 50% later with a timeout of
// 100ms. Firing later than that would mean that we hit the regression
// highlighted in
// to fire, they shouldn't generally be more than 100% late in this case.
// But they are guaranteed to be at least 100ms late given the bug in
// https://github.com/nodejs/node-v0.x-archive/issues/15447 and
// https://github.com/nodejs/node-v0.x-archive/issues/9333..
assert(latestDelay < TIMEOUT * 1.5);
assert(latestDelay < TIMEOUT * 2);
if (callback)
return callback();
} else {
Expand Down

0 comments on commit a717be8

Please sign in to comment.