sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
810 stars 108 forks source link

Faked `nextTick` causes Node.js 8.2.0 to hang #126

Closed novemberborn closed 7 years ago

novemberborn commented 7 years ago

https://github.com/novemberborn/ksuid/commit/b4564c5d52835e1ce479dafe9cea938b1a738dfd hangs when tests are run in Node.js 8.2.0, but is fine in 8.1.4.

The fix is to not fake nextTick: https://github.com/novemberborn/ksuid/commit/7239faa5d166b7a241eb78c688d0c0191f72ed06

Tests are run using AVA, which does not directly uses process.nextTick(). I'm not sure why it the hangs occur with Node.js 8.2.0 either. It does seem that faking nextTick by default may result in unexpected behavior.

acud commented 7 years ago

@benjamingr - any idea?

fatso83 commented 7 years ago

Testcase for auto-repro.

Assuming n is installed (npm -g install n)

git clone https://github.com/novemberborn/ksuid.git
cd ksuid
git checkout c18b85bed67cd74e5f411fa27fdce96fa12ea2e7
n 8.2.0
npm i
npm test   # will hang
benjamingr commented 7 years ago

We can start not hooking on nextTick by default or to flush them automatically by default.

Node uses nextTick internally (and also timers)

benjamingr commented 7 years ago

I can work on a PR for either behavior

acud commented 7 years ago

I think that a configuration flag or just adding nextTick to the possible methods to hijack was the right thing to do in the first place; Why not allow developers to have the possibility to choose if to enable this or not?