Sinon: Resolving native ES6 promises doesn't trigger callbacks when using fake timers

[ropez]

In the following test, I expected the callback for the resolved promise to be invoked while inside the test. Apparently, native promises doesn't invoke callbacks synchronously, but schedules them to be called in a manner similar to setTimeout(callback, 0). However, it doesn't actually use setTimeout, so sinon's implementation of fake timers doesn't trigger the callback when calling tick().

describe 'Promise', ->
  beforeEach ->
    @clock = sinon.useFakeTimers()
  afterEach ->
    @clock.restore()
    console.log 'teardown'

  it "should invoke callback", ->
    p = new Promise (resolve, reject) ->
      console.log 'resolving'
      resolve(42)
      console.log 'resolved'
    p.then ->
      console.log 'callback'
    @clock.tick()
    console.log "test finished"

I expect this output:

resolved
callback
test finished
teardown

Instead I get this:

resolved
test finished
teardown
callback

The callback is invoked after the test is finished, so assertions based on what happens inside the callback fail.

It makes no difference whether the promise is resolved before or after calling then().

[JustinLivi]

I've run into this issue as well

[mariusGundersen]

I don't think this can be solved, as the Promise spec doesn't use setTimeout, but schedules a new job to be done right after the current one.

Have you tried returning the promise from the test? Most test libraries support promises now, and if the it() function returns a promise the test runner will wait for the promise to resolve or reject before noting the test as done.

[ropez]

Yes, returning a promise from a test probably works. But when using fake timers, I would expect to be able to compete the test before returning from the test function. That is kind-of the whole point.

I'm pretty sure this can be done by replacing the Promise object, just like the Date object is replaced. To be able to do that, we probably need a compete implementation of the promise spec, since we won't be able to rely on the native implementation.

[JustinLivi]

I see the issue you guys are pointing out here. I wanted to call out the specific situation I ran into however, as based on both the promise specification and sinon documentation it appeared to me that this "should" work. In this case I'm returning the promise to the test but using fake timers appears to prevent the promise from ever resolving.

It seemed to me that both of these tests should work, but the first one passes while the second one fails. I'm using chai with the chai-as-promised plugin with mocha and I get the error "timeout of 2000ms exceeded. Ensure the done() callback is being called in this test."

describe('chai as promised', function() {

    it('should resolve a promise', function() {
        var promise = new Promise(function( resolve ) {
            setTimeout( resolve, 1000 );
        });
        return expect( promise ).to.have.been.fulfilled;
    });

});

describe('sinon.useFakeTimers()', function() {

    before(function() {
        this.clock = sinon.useFakeTimers();
    });

    after(function() {
        this.clock.restore();
    });

    it('should resolve a promise after ticking', function() {

        var promise = new Promise(function( resolve ) {
            setTimeout( resolve, 1000 );
        });

        this.clock.tick( 1001 );

        return expect( promise ).to.have.been.fulfilled;
    });

});

I put together a quick test project: https://github.com/JustinLivi/sinon-promises-test

[JustinLivi]

Restoring before the completion of the test seems to be a viable workaround:

describe('sinon.useFakeTimers()', function() {

    before(function() {
        this.clock = sinon.useFakeTimers();
    });

    it('should resolve a promise after ticking', function() {

        var promise = new Promise(function( resolve ) {
            setTimeout( resolve, 1000 );
        });

        this.clock.tick( 1001 );
        this.clock.restore();

        return expect( promise ).to.have.been.fulfilled;
    });

});

[ropez]

The workaround we're using is to replace the native promise implementation with something simple that depends on setTimeout, when running tests:

window.Promise = require('promise-polyfill')

Sinon.JS could simply do the same when calling useFakeTimers.

[screendriver]

@JustinLivi restoring before completion did it for me :+1:

[otroboe]

So, any solution or workaround ?
I can't modify my app and stop using ES6 Promises, because my tests say so :)

[fpirsch]

I have the problem, not with native promises but with the es6-promise polyfill.
The clock.restore() solution from @JustinLivi fixes the issue.

[fatso83]

No idea why this issue has been lingering here for so long, but this is not an issue with Sinon. Using Promises is essentially performing async logic. Still, all the tests here are using synchronous logic (as the OP indeed mentions). So even though you are faking time, you are still relying on the Promise to execute after your function finishes, which means you need to change your test a bit. This is usually covered in the docs of most test frameworks (here's one from Mocha), but we will address this with some articles featuring test recipes in the upcoming new site.

So changing @JustinLivi's example a bit we end up with the following test:

var sinon = require('sinon');

describe('sinon.useFakeTimers()', function() {

    before(function() {
        this.clock = sinon.useFakeTimers();
    });

    it('should resolve a promise after ticking', function(done) {

        var promise = new Promise(function( resolve ) {
            setTimeout( resolve, 10000 );
        });

        this.clock.tick( 10001 );

        promise
            .then(done) // call done when the promise completes
            .catch(done); // catch any accidental errors
    });

});

Actually, if you are using Mocha, it has a "shortcut" version of the same code above when using Promises, if you just return the promise directly to the test framework:

it('should resolve a promise after ticking', function() {
    var promise = new Promise(function( resolve ) {
        setTimeout( resolve, 10000 );
    });

    this.clock.tick( 10001 );

    return promise;
});

This will

1. Execute the executor function sent as a parameter to the Promise constructor synchronously (see the ES2015 specification), effectively setting up the new timeout.
2. Return the constructed promise.
3. Tick the time.
4. Trigger the timed out function, marking the promise as resolved
5. process (or the browser) will stack a new "tick", resolving the promise and calling any remaining Thenables

Closing this as a non-bug.

[JustinLivi]

@fatso83 I don't see how you're getting the tests to pass the way you've described. I added your example tests to my test repository and I still get timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.

See here: https://github.com/JustinLivi/sinon-promises-test/blob/master/test.js#L60
Also, as far as I can tell, this test still fails: https://github.com/JustinLivi/Sinon.JS/commit/de106e6db2f5cc076b7e3a78635bd9ae2b6be1c2

Edit: based on the comment by @fpirsch it seems it's just when using the es6 promise polyfill? Has anyone tried with any alternative libraries such as bluebird?

[fpirsch]

@fatso83 This is NOT a return promise issue.
In my case this is a Phantom (old, without promises) + es6-promises polyfill + sinon.js issue. In modern browsers the promise is resolved and the tests run fine, but with the promise polyfill the promise never resolves when the timers are faked.

[fatso83]

@JustinLivi and @fpirsch: this bug report was about using _native_ promises. I see @JustinLivi's test project using es6-promise, so I cannot vouch for that. Same goes for other polyfills: that would be another issue. I have tested this with Node 5 and 6, which have native promise support.

Copy-pasting my example code from the previous post and running it (having Mocha and Sinon pre-installed):

$ pbpaste > test.js

$ mocha test.js

  sinon.useFakeTimers()
    ✓ should resolve a promise after ticking

  1 passing (12ms)

@fpirsch: I never said it was a return promise issue. I just mentioned that he could rewrite the test in a shorter form. A tip, not a fix. But you did provide some valuable info: it works in native browsers, but fails in the promise polyfill. That is a fault of your promise lib, not Sinon. Your promise lib most probably does not cache setTimeout and friend, and relies on it for function, thus breaking. I implemented a fix for exactly this in the pinkyswear lib once, so no wonder.

[fatso83]

Checked the source of es6-promise and it does not cache references to setTimeout, so it will be affected by anything sinon does. But I fail to see how that scheduler is relevant here ... There is also a huge list of other options the polyfill will try before falling back on the timeout scheduler. Any browser landing on that fallback must be ancient :)

But anyway, what @fpirsch and @JustinLivi mentions are issues regarding the interoperability between Sinon and the polyfill. And that is another issue. Not seeing how Sinon can do much about it ATM (any PR is more likely to end up in es6-polyfill), but if this is a Sinon issue feel free to open a new issue.

[fatso83]

P.S. I see the tip from @ropez was to use promise-polyfill, and that is indeed our recommendation for a minimal polyfill too, but for the opposite reason of the one given. @mroderick patched that project in January (a few months after @ropez comment) to cache the reference to setTimeout so that the promise would resolve _no matter what sinon was doing_ to the global setTimeout reference. See taylorhakes/promise-polyfill#15 for details.

[fpirsch]

@fatso83 thanks for the info. Unfortunately saving a reference to setTimeout as @mroderick did for promise-polyfill doesn't work in my case. Neither does replacing es6-promise with promise-polyfill :-(
EDIT: it does in a simple phantom+mocha stack, but not in our complete stack with karma. So tired of this.

[fpirsch]

@fatso83 I put up an example here: faketimers
The issue may be with Karma after all... I'll try to delve deeper into this.

[fatso83]

@fpirsch: I think you are on to something. Both you and @JustinLivi are relying on karma as a test runner. They seem very similar. P.S. I deleted my comment as I thought it was totally superfluous, as Justin already supplied a test case that showed the issue, but your example project strengthens the hypothesis of this being a Karma thing, so thanks!

[jasonkuhrt]

This might be related but I'm not sure. Why does the third test below fail? The specific failure is the typical test timeout:

     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.

describe.only('working', () => {
  let clock;
  beforeEach(() => { clock = Sinon.useFakeTimers(); })
  afterEach(() => { clock.restore() })

  const delay = (ms) => (
    new Promise((resolve/* , reject */) => {
      setTimeout(resolve, ms)
    })
  );

  it('1 OK', () => {
    const promise = new Promise((resolve) => {
      setTimeout(resolve, 100)
    })
    clock.tick(200)
    return promise
  })

  it('2 OK', () => {
    const promise = delay(100)
    clock.tick(200)
    return promise
  })

  it('3 FAIL', () => {
    const promise = delay(50).then(() => delay(50))
    clock.tick(200)
    return promise
  })
})

[gautaz]

@jasonkuhrt The third test fails because clock.tick processes executors synchronously and thus the .then(...) is not executed within due to its asynchronous nature:

onFulfilled or onRejected must not be called until the execution context stack contains only platform code.

@fatso83 I do not know the position of lolex developers regarding chained promises that add timers. The current limitation should at least be detailed in the documentation.

Depending on whether this should be supported or not, this issue can be reopened (or a new one can be opened) and depend on a new issue in lolex.
I also guess that if this use case (chaining promises that add timers) has to be taken into account, this will lead to a disrupting lolex API modification (AFAIK clock.tick will have to be asynchronous).

[jasonkuhrt]

@gautaz Ah makes sense now. Thanks for explaining!

[fatso83]

lolex is a sync library, and I am not totally sure how to solve @jasonkuhrt's issue anyhow. Feel free to supply a PR, but modifying clock.tick is not very attractive. I'd rather see an additional async method.

I never had much issues with promises and clock ticking, but that was probably because I had seen that they were synchronous, and added extra clock ticks inbetween.

[gautaz]

@fatso83 You're right, adding an asynchronous tick would be less disrupting (in fact not disrupting at all).
So something like asyncTick might be the right fit. I'll look into it if I can make time.

[fatso83]

@gautaz just curious if you ever got round to supplying a patch to lolex?

[gautaz]

@fatso83 Hi, I've started a branch last year on my side but never got for now the time to finish the job.
I have colleagues that also have been tripped by the same issue, so I can only hope the issue gets big enough to provide me additional time.

Did something change in the meantime concerning lolex API on this particular point?

[fatso83]

It shouldn't have. Very little changes in the codebase the last half year. Pretty stable.

[Diokuz]

Any chances to have simple workaround here? Have same problem. I have two promises and they are resolves by setTimeout. I need to check stuff before any resolve, before second but after first resolve, and after all resolves.

[jakwuh]

The only thing you need is to wait for promise microtask to execute.
So the following approach works perfect:

const tick = async (ms) => {
  clock.tick(ms);
};

it("imitates promise fulfill when called once", async () => {
    new Promise(resolve => setTimeout(resolve, 400)).then(callback);
    expect(callback.callCount).to.eql(0);

    await tick(200);
    expect(callback.callCount).to.eql(0);

    await tick(200);
    expect(callback.callCount).to.eql(1);
  });

[cakoose]

@jakwuh's trick worked for me (Node 8, no transpilation, native promises), except the then callbacks were postponed by a tick.

Updated Comment: The solution in my original comment (below) has issues. Sometimes I needed to have multiple consecutive await Promise.resolve() calls to actually flush everything. Here's something that seems to work a little better:

beforeEach(function() {
    const originalSetImmediate = setImmediate;
    this.clock = sinon.useFakeTimers();
    this.tickAsync = async ms => {
        this.clock.tick(ms);
        await new Promise(resolve => originalSetImmediate(resolve));
    }
});

afterEach(function() {
    this.clock.restore();
});

Original Comment [WARNING: Doesn't work as well as the code above.]

Adding an extra async gap to the tick helper fixed the problem for me:

```js
const tick = async (ms) => {
clock.tick(ms);
await Promise.resolve();
};

[fatso83]

For people interested in the work to improve Sinon in this area (actually, its sibling project lolex), check out the discussions here:

• https://github.com/sinonjs/lolex/issues/114
• https://github.com/sinonjs/lolex/pull/105