Feature: clear all Timers when doing restore

[sasaxing]

We understand you have a problem and are in a hurry, but please provide us with some info to make it much more likely for your issue to be understood, worked on and resolved quickly.

• FakeTimers version : please verify that the bug exists in the latest FakeTimers release 9.0.11
• Environment : node 16.14.2
• Example URL :
• Other libraries you are using: mocha for showing the behavior.

What did you expect to happen?

When I start to useFakeTimers, all global Timer functions in Node will be replaced by Sinon, including the clear methods. It means if the Timers are set up after fakeTimer is activated, and Sinon is restored before all pending Timers are cleared, it will be not possible to clear the timers afterward.

I'd expect there's an API to opt-in clearing all pending Timers that are set after fakeTimer is activated on doing sandbox.restore(), it will bring more predictivity to the test behavior, especially when those Timers are set by the libs where the TimerIDs are not trackable.

To add to it, this issue can happen a lot when testing codes that work with RxJS, which heavily relies on clearInterval to work well to clean up the ongoing tasks, otherwise an explicit error will be thrown in NodeJS process (see RxJS code)

What actually happens

as described above.

How to reproduce

Describe with code how to reproduce the faulty behaviour or link to code on JSBin or similar

Really long code sample or stacktrace

// I'm using mocha here it('fakeTimer', async function () { const clock = sandbox.useFakeTimers(); const spy = sandbox.spy(); const intervalId = setInterval(spy, 100); console.log('about to tick 1s'); await clock.tickAsync(1000); sandbox.assert.callCount(spy, 10); console.log('about to tick 1s'); await clock.tickAsync(1000); sandbox.assert.callCount(spy, 20); sandbox.resetHistory(); sandbox.restore(); clearInterval(intervalId); // this will simply not work, because clearInterval is different from the one Sinon is using, and it's not possible clear the Timers Sinon has set. In this example we can move this line before sandbox.restore(); to solve, but if we can't get the intervalId then we can't clear it. console.log('about to tick 1s (after clearInterval)'); await clock.tickAsync(1000); sandbox.assert.callCount(spy, 0); }); // I had this as a workaround it('fakeTimer', async function () { const clock = sandbox.useFakeTimers(); const spy = sandbox.spy(); const setIntervalSpy = sandbox.spy(global, 'setInterval'); const intervalId = setInterval(spy, 100); console.log('about to tick 1s'); await clock.tickAsync(1000); sandbox.assert.callCount(spy, 10); console.log('about to tick 1s'); await clock.tickAsync(1000); sandbox.assert.callCount(spy, 20); setIntervalSpy.returnValues.map((intervalTimerId) => global.clearInterval(intervalTimerId)); sandbox.resetHistory(); // this has to be before restore... sandbox.restore(); console.log('about to tick 1s (after clearInterval)'); await clock.tickAsync(1000); sandbox.assert.callCount(spy, 0); });

[fatso83]

Thanks for chiming in 🙂 I have tried reading this issue multiple times, but I am still not sure what the problem is. What is it that is not working or that you would like to have changed?

AFAIK, calling restore() on a sandbox will implicitly invoke the restore method of the installed clock, thus removing all pending timers as a side effect. There should be nothing else to clear after the restore, in other words. So I don't get the problem.

Might want to have a look at the docs: https://sinonjs.org/releases/latest/fake-timers/ or possibly just play with the fake-timers lib directly to avoid Sinon specifics confusing the matter.

The actual fake-timers code is very easy to read and is just a single file, so I really recommend checking it out if interested. You'll see the timers are simply added to an array that is looped over at every tick. Nothing fancy.

[sasaxing]

Thanks @fatso83 for responding so fast. just to be sure I get it right

calling restore() on a sandbox will implicitly invoke the restore method of the installed clock, thus removing all pending timers as a side effect.

This means all running Timers should be stopped and not continue the tasks if sandbox.restore is called, right? I wrote some small tests to make it a bit more clear here. I try to implement what I had in mind to hopefully help explaining, would love to see your feedback on that.

And will check out the source code to learn a bit more.

[fatso83]

Yes, that should be right.

I wrote some small tests to make it a bit more clear

Very smart, I took the tip of Sinon's original author Christian Johansen in his TDD book on JS and created a repo for learning tests years ago. Very nice for solidifying understanding.

[fatso83]

Not sure what you intend to achieve by that ClearableSinon wrapper.

When you call sinon.restore() or sandbox.restore() you call the Sandbox#restore method, which in turns calls clock.uninstall().

The clock simply clears stubbed timers and date constructors and returns the list of pending timers.

You can see how they are created here: https://github.com/sinonjs/fake-timers/blob/main/src/fake-timers-src.js#L585

There should be nothing to clean up. If you could point me to some test failing I would maybe undersand a bit better?

Try this: https://runkit.com/fatso83/sinon-issue-reproducible-bug-template

[sasaxing]

https://runkit.com/xiaosha/628d2d19e3e2060008656cd5 I try to make 2 tests, one with original sinon, the second clearable sinon. Both do the same steps, and assert same expectation, while the first will fail but second pass. The only difference is ClearableSinon does some more thing during restore.

[fatso83]

Ah, now I get you. There is a bit of an unfortunate overloading of terms here: "clearing" is not "clearing" :)

If you look at the source links above (which I did to understand how your code worked as it did) you will see that fake-timers just returns clock.timers to the caller on unstall. It does not remove the scheduled timed executions.

That means, when you have a reference to clock, it will have pretty much the same internal state before and after calling uninstall(). That explains the behavior in your test.

Now, why is this NOT a problem? Your test does something artificial: it manipulates a clock that does not exist in "the real world", but only in your test world. You have already uninstalled it from overriding Date, setTimeout, etc, so any issues you have is in your tests, not issues exercised by actual implementation code.

What you are actually testing, is the behavior of the testing code :-)

I guess the one thing we should have done regardless for improved usabilityis to set some internal state that says the clock is uninstalled, so that calling any method on the clock after uninstalling should throw an error.

If you want, you can of course submit a PR to just clear the timers. You would then just store a temporary version of clock.timers before resetting it and return the value before clearing. Ref the below (see the link):

The clock simply clears stubbed timers and date constructors and returns the list of pending timers.

[fatso83]

Btw, with regard to the RXJs code, are you saying this code is throwing: intervalProvider.clearInterval(id); when given an unknown id? Any normal clearInterval function should simply do nothing, but I understand you request if it is a user-land library that is the intervalProvider and that throws on unknown ids.

Just submit a PR to clear the list before returning the pending timers that would just be left there.