Skip to content

Commit

Permalink
test: fix flaky test-regress-GH-897
Browse files Browse the repository at this point in the history
Even after being moved to `sequential` in
1ce05ad, `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: #10903
Fixes: #10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
Trott authored and MylesBorins committed Mar 9, 2017
1 parent a08c7f6 commit e59697c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 17 deletions.
20 changes: 20 additions & 0 deletions test/parallel/test-regress-GH-897.js
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 0 additions & 17 deletions test/sequential/test-regress-GH-897.js

This file was deleted.

0 comments on commit e59697c

Please sign in to comment.