From 9d5170f850921953613c34a405c643d8894555e3 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Thu, 19 Jan 2017 11:19:07 -0800
Subject: [PATCH] test: fix flaky test-regress-GH-897

Even after being moved to `sequential` in
1ce05ad5401447cff6df6e62e22ff5cf052e5c92, `test-regress-GH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: https://github.com/nodejs/node/pull/10903
Fixes: https://github.com/nodejs/node/issues/10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
---
 test/parallel/test-regress-GH-897.js   | 20 ++++++++++++++++++++
 test/sequential/test-regress-GH-897.js | 17 -----------------
 2 files changed, 20 insertions(+), 17 deletions(-)
 create mode 100644 test/parallel/test-regress-GH-897.js
 delete mode 100644 test/sequential/test-regress-GH-897.js

diff --git a/test/parallel/test-regress-GH-897.js b/test/parallel/test-regress-GH-897.js
new file mode 100644
index 00000000000000..911ad6b6b2e112
--- /dev/null
+++ b/test/parallel/test-regress-GH-897.js
@@ -0,0 +1,20 @@
+'use strict';
+
+// Test for bug where a timer duration greater than 0 ms but less than 1 ms
+// resulted in the duration being set for 1000 ms. The expected behavior is
+// that the timeout would be set for 1 ms, and thus fire before timers set
+// with values greater than 1ms.
+//
+// Ref: https://github.com/nodejs/node-v0.x-archive/pull/897
+
+const common = require('../common');
+
+let timer;
+
+setTimeout(function() {
+  clearTimeout(timer);
+}, 0.1); // 0.1 should be treated the same as 1, not 1000...
+
+timer = setTimeout(function() {
+  common.fail('timers fired out of order');
+}, 2); // ...so this timer should fire second.
diff --git a/test/sequential/test-regress-GH-897.js b/test/sequential/test-regress-GH-897.js
deleted file mode 100644
index 7b1297efd5a1b7..00000000000000
--- a/test/sequential/test-regress-GH-897.js
+++ /dev/null
@@ -1,17 +0,0 @@
-'use strict';
-
-// Test for bug where a timer duration greater than 0 ms but less than 1 ms
-// resulted in the duration being set for 1000 ms. The expected behavior is
-// that the timeout would be set for 1 ms, and thus fire more-or-less
-// immediately.
-//
-// Ref: https://github.com/nodejs/node-v0.x-archive/pull/897
-
-const common = require('../common');
-const assert = require('assert');
-
-const t = Date.now();
-setTimeout(common.mustCall(function() {
-  const diff = Date.now() - t;
-  assert.ok(diff < 100, `timer fired after ${diff} ms`);
-}), 0.1);