test: make timers-blocking-callback more reliable

test-timers-blocking-callback may fail erroneously on
resource-constrained machines due to the timing nature of the test.

There is likely no way around the timing issue. This change tries to
decrease the probability of the test failing erroneously by having it
retry a small number of times on failure.

Tested on 0.10.38 (which has a bug that this test was written for) and
(modifying the test slightly to remove ES6 stuff) the test still seems
to fail 100% of the time there, which is what we want/expect.

Fixes: nodejs#14792

test/sequential/test-timers-blocking-callback.js

@@ -1,8 +1,9 @@
 'use strict';
 
 /*
- * This is a regression test for https://github.com/joyent/node/issues/15447
- * and https://github.com/joyent/node/issues/9333.
+ * This is a regression test for
+ * https://github.com/nodejs/node-v0.x-archive/issues/15447 and
+ * and https://github.com/nodejs/node-v0.x-archive/issues/9333.
  *
  * When a timer is added in another timer's callback, its underlying timer
  * handle was started with a timeout that was actually incorrect.
@@ -28,17 +29,26 @@ const Timer = process.binding('timer_wrap').Timer;
 
 const TIMEOUT = 100;
 
-let nbBlockingCallbackCalls = 0;
-let latestDelay = 0;
-let timeCallbackScheduled = 0;
+let nbBlockingCallbackCalls;
+let latestDelay;
+let timeCallbackScheduled;
+
+// These tests are somewhat probablistic so they may fail even when the bug is
+// not present. However, they fail 100% of the time when the bug *is* present,
+// so to increase reliability, allow for a small number of retries. (Keep it
+// small because as currently written, one failure could result in multiple
+// simultaneous retries of the test. Don't want to timer-bomb ourselves.
+// Observed failures are infrequent anyway, so only a small number of retries
+// is hopefully more than sufficient.)
+let retries = 2;
 
 function initTest() {
   nbBlockingCallbackCalls = 0;
   latestDelay = 0;
   timeCallbackScheduled = 0;
 }
 
-function blockingCallback(callback) {
+function blockingCallback(retry, callback) {
   ++nbBlockingCallbackCalls;
 
   if (nbBlockingCallbackCalls > 1) {
@@ -47,34 +57,51 @@ function blockingCallback(callback) {
     // 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 * 2);
+    // https://github.com/nodejs/node-v0.x-archive/issues/9333.
+    if (latestDelay > TIMEOUT * 2) {
+      if (retries > 0) {
+        retries--;
+        return retry(callback);
+      }
+      assert.fail(`timeout delayed by more than 100ms (${latestDelay}ms)`);
+    }
     if (callback)
       return callback();
   } else {
     // block by busy-looping to trigger the issue
     common.busyLoop(TIMEOUT);
 
     timeCallbackScheduled = Timer.now();
-    setTimeout(blockingCallback.bind(null, callback), TIMEOUT);
+    setTimeout(blockingCallback.bind(null, retry, callback), TIMEOUT);
   }
 }
 
-const testAddingTimerToEmptyTimersList = common.mustCall(function(callback) {
+function testAddingTimerToEmptyTimersList(callback) {
   initTest();
   // Call setTimeout just once to make sure the timers list is
   // empty when blockingCallback is called.
-  setTimeout(blockingCallback.bind(null, callback), TIMEOUT);
-});
+  setTimeout(
+    blockingCallback.bind(null, testAddingTimerToEmptyTimersList, callback),
+    TIMEOUT
+  );
+}
 
-const testAddingTimerToNonEmptyTimersList = common.mustCall(function() {
+function testAddingTimerToNonEmptyTimersList() {
   initTest();
   // Call setTimeout twice with the same timeout to make
   // sure the timers list is not empty when blockingCallback is called.
-  setTimeout(blockingCallback, TIMEOUT);
-  setTimeout(blockingCallback, TIMEOUT);
-});
+  setTimeout(
+    blockingCallback.bind(null, testAddingTimerToNonEmptyTimersList),
+    TIMEOUT
+  );
+  setTimeout(
+    blockingCallback.bind(null, testAddingTimerToNonEmptyTimersList),
+    TIMEOUT
+  );
+}
 
 // Run the test for the empty timers list case, and then for the non-empty
-// timers list one
-testAddingTimerToEmptyTimersList(testAddingTimerToNonEmptyTimersList);
+// timers list one.
+testAddingTimerToEmptyTimersList(
+  common.mustCall(testAddingTimerToNonEmptyTimersList)
+);