From 9d5170f850921953613c34a405c643d8894555e3 Mon Sep 17 00:00:00 2001 From: Rich Trott 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 Reviewed-By: Luigi Pinca --- 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);