sinon.clock not found

[scinos]

Environment

• Sinon version : 5.0.3
• Environment: OSX with Node 8.9.0

What did you expect to happen?

After Sinon 5, I expect Sinon root object to behave like a Sandbox, including fake timers. Sinon should be a drop-in replacement for most use cases of sandboxes.

What actually happens

Extra logic is required if we were using fakeTimers with the sandbox.

What actually happens

How to reproduce

With Sinon 5, I replaced all my usages of sandboxes with just sinon (like suggested in the guide), but it doesn't work for clocks.

Before:

const sandbox = sinon.sandbox.create();
sandbox.useFakeTimers();
//...
sandbox.clock.restore(); //this works

After:

sinon.useFakeTimers();
sinon.clock.restore(); //sinon.clock is undefined

Workaround:

const clock = sinon.useFakeTimers();
clock.restore();

The work around is trivial (and actually documented in the useFakeTimers documentation), but sinon.clock should exist so sinon is a drop-in replacement for a sandbox.

[mantoni]

I agree. Can you make a pull request to fix this?

[scinos]

I can give it a go this weekend.

[brian-mann]

Just came across this. Totally agree.

[scinos]

Apologies for the confusion, but looks like I got the error wrong. There is a sinon.clock, but it is not an instance of useFakeTimers (sinon.clock !== sinon.useFakeTimers()).

This sinon.clock was added ages ago (it was there in v 0.5.0), but I can't find any documentation or tests for it. It is defined in https://github.com/sinonjs/sinon/blob/master/lib/sinon/util/fake_timers.js#L31 and re-exported in https://github.com/sinonjs/sinon/blob/master/lib/sinon.js#L28. The last time it was used internally was in v1.9.1 (https://github.com/sinonjs/sinon/blob/v1.9.1/lib/sinon/util/fake_timers.js#L372), but I don't think it has been used ever since.

So the question is... is this public API? Do we want to proceed and overwrite sinon.clock with the result of sinon.useFakeTimers()?

Edit: to clarify, there are tests for fake timers that use that exported function from fake_timers.js (https://github.com/sinonjs/sinon/blob/master/test/util/fake-timers-test.js#L19), but there are no tests for it being exposed as sinon.clock.

[fatso83]

I wouldn't know. I got involved from 1.15.

[mantoni]

I checked the docs and couldn't find the sinon.clock object being documented anywhere. All objects used to be created in this way (sinon.clock.create(), sinon.sandbox.create(), ...) and I think this bit is a leftover from those days.

Also related to this suggestion: Currently the fake clock is not restored along with the default sandbox. I think this happens because the clock object isn't assigned to this.clock here: https://github.com/sinonjs/sinon/blob/e2c7132ff905c20d608b32365efab41836823fbd/lib/sinon/sandbox.js#L309

So from my point of view, this is a bug. Sandboxes expose clock and restore them on sandbox.restore() while the default sandbox doesn't.

[mroderick]

So from my point of view, this is a bug. Sandboxes expose clock and restore them on sandbox.restore() while the default sandbox doesn't.

I agree, it's a bug. The default sandbox should behave just like any other sandbox.

[fatso83]

Btw, where it says "sinon.clock is undefined", I think it really should be "sinon.clock.restore is undefined", as sinon.clock() exists all the time. You get this error: TypeError: sinon.clock.restore is not a function