Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impossible to restore fake timers in certain situations. #2449

Closed
cjbarth opened this issue Apr 6, 2022 · 2 comments
Closed

Impossible to restore fake timers in certain situations. #2449

cjbarth opened this issue Apr 6, 2022 · 2 comments

Comments

@cjbarth
Copy link

cjbarth commented Apr 6, 2022

Describe the bug
When sinon.useFakeTimers is used in a nested fashion, or code crashes or early exist before a timer is restored, it becomes impossible to reset the timers to their default configuration.

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://npm.runkit.com/sinon
  2. Run this code:
var sinon = require("sinon")
let fakeClock;

console.log("Original time: " + new Date().getTime());
fakeClock = sinon.useFakeTimers(Date.parse("2014-06-05T12:07:07.662Z"));
console.log("First fake: " + new Date().getTime());
fakeClock = sinon.useFakeTimers(Date.parse("2018-04-11T14:08:00Z"));
console.log("Second fake: " + new Date().getTime());
fakeClock.restore();
console.log("Expect either 'first fake' or 'original time'");
console.log("First restore: " + new Date().getTime());
fakeClock.restore();
console.log("Expect 'original time'");
console.log("Second restore: " + new Date().getTime());
fakeClock.restore();
console.log("Expect 'original time'");
console.log("Third restore: " + new Date().getTime());

Expected behavior
I expect there to be some way to get the original timers back.

Context:

When writing Mocha tests, often there are multiple nested describe blocks. If each block doesn't handle the timers correctly, particularly child blocks, then this can easily happen with no way to recover or reset. This gets particularly tricky if before* or after* blocks are used to fake timers and then an individual it block also messes with timers.

Additional context
Granted, all these cases above could be consider a bug or poor usage, but there really should be some way to get things back to normal. The problem was realized when trying to debug intermittent failures in tests (we randomize the order of our tests to catch stuff like this). It took quite a while to hunt this problem down. Then, the fix was rather painfully going through every place were a timer was used and making sure that, if a decedent test was using a timer, it was declared locally and cleaned up in a finally block. So, now our code has lots of try...finally in it where there was none before. Otherwise, an abnormal function termination would cause the fake timers to leak and break other tests.

@fatso83
Copy link
Contributor

fatso83 commented Apr 7, 2022

For the simplest case, it should not be too hard to make it work. https://github.com/sinonjs/fake-timers/blob/main/src/fake-timers-src.js#L1752 could just store references to the global timers and we could expose a reset() on the module. This should probably handle most issues, but the fake-timers module supports alternative contexts to be passed in, as would be the case for JSDOM tests, for instance, where you typically pass in some window to be used as the the global. Alterations to window would not be handled with the approach, as you would need to keep the context around somehow.

The source code is relatively easy to work with, so feel free to poke around to get some ideas on how this could be done.

@cjbarth
Copy link
Author

cjbarth commented Apr 9, 2022

I've created a PR to address this in the most uniform way I could think of, taking into account the problem of the JSDOM test that you mention. Please see sinonjs/fake-timers#426 and let me know what you think @fatso83 .

@cjbarth cjbarth closed this as completed Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants