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

Provide an API to flush the Promise resolution queue #2157

Open
hon2a opened this issue Nov 23, 2016 · 61 comments
Open

Provide an API to flush the Promise resolution queue #2157

hon2a opened this issue Nov 23, 2016 · 61 comments

Comments

@hon2a
Copy link

hon2a commented Nov 23, 2016

Do you want to request a feature or report a bug?

Feature, I guess, but a pretty important one when testing code that uses Promises.

What is the current behavior?

I have a component that uses Promise wrapping and chaining internally in the follow-up to an external asynchronous action. I'm providing the mock of the async action and resolving the promise it returns in my test.

The component something like this:

class Component extends React.Component {
  // ...
  load() {
    Promise.resolve(this.props.load())
      .then(
        result => result
          ? result
          : Promise.reject(/* ... */)
        () => Promise.reject(/* ... */)
      )
      .then(result => this.props.afterLoad(result));
  }
}

And the test code looks something like this:

const load = jest.fn(() => new Promise(succeed => load.succeed = succeed));
const afterLoad = jest.fn();
const result = 'mock result';
mount(<Component load={load} afterLoad={afterLoad} />);
// ... some interaction that requires the `load`
load.succeed(result);
expect(afterLoad).toHaveBeenCalledWith(result);

The test fails because the expect() is evaluated before the chained promise handlers. I have to replicate the length of the inner promise chain in the test to get what I need, like this:

return Promise.resolve(load.succeed(result))
  // length of the `.then()` chain needs to be at least as long as in the tested code
  .then(() => {})
  .then(() => expect(result).toHaveBeenCalledWith(result));

What is the expected behavior?

I'd expect Jest to provide some sort of an API to flush all pending promise handlers, e.g.:

load.succeed(result);
jest.flushAllPromises();
expect(result).toHaveBeenCalledWith(result);

I've tried runAllTicks and runAllTimers to no effect.


Alternatively, if I'm just missing some already existing feature or pattern, I'm hoping for someone here to point me in the right direction :)

@thymikee
Copy link
Collaborator

While async testing promises it's good to remember that you can return a test function as a Promise, so something like this will work:

test('my promise test', () => { //a test function returning a Promise
  return Promise.resolve(load.succeed(result))
    .then(() => {})
    .then(() => expect(result).toHaveBeenCalledWith(result));
})

Returning a Promise from test function makes Jest aware that this is a async test and to wait until it's resolved or times out.

@hon2a
Copy link
Author

hon2a commented Jan 31, 2017

@thymikee Of course I'm returning the value to make Jest wait - that's completely off the point. Note how you even left the line .then(() => {}) in your code. I don't see how I can describe the problem more concisely than I already did in the opening post. Please read it thorougly and either reopen the issue or describe how to work around it.

I've added the return to the code in the OP to avoid confusion.

@thymikee thymikee reopened this Jan 31, 2017
@pekala
Copy link

pekala commented Feb 11, 2017

Ran into a similar problem, and described it here: https://github.com/pekala/test-problem-example

In short: I'm trying to assert on sequence of actions dispatched to the Redux store as a result of user interaction (simulated using enzyme). The actions as dispatched sync and async using Promises (mocked to resolve immidiately). There seems to be no way to assert after the Promise chain is exhaused, if you don't have direct access to the promise chain. setTimeout(..., 0) works, but it feels hacky and if the assertion in the callback of setTimeout fails, Jest fails with timeout error (instead of assertion error).

The idea of flushAllPromises seems like a solution, although I would think that that's what runAllTicks should do?

@pekala
Copy link

pekala commented Feb 11, 2017

As a follow-up: I tried replacing setTimeout(..., 0) with setImmediate and this seems to both run the assertions after Promise callback microtask queue is exhausted and prevents Jest from timing out on assertion errors. So, this works ok, and is an acceptable solution for my usecase:

test('changing the reddit downloads posts', done => {
    setImmediate(() => {
        // assertions...
        done()
    })
})

@jwbay
Copy link
Contributor

jwbay commented Feb 11, 2017

A helper function can turn that into a promise itself so you don't need to deal with the done callback. It's small enough it's pretty harmless to keep in userland, but I wouldn't complain if it was put on the jest object. Something like this gets used a lot in my projects.

function flushPromises() {
  return new Promise(resolve => setImmediate(resolve));
}

test('', () => {
  somethingThatKicksOffPromiseChain();
  return flushPromises().then(() => {
    expect(...
  });
})

With async await it's almost pretty:

test('', async () => {
  somethingThatKicksOffPromiseChain();
  await flushPromises();
  expect(...
})

@pekala
Copy link

pekala commented Feb 11, 2017

@jwbay that's some nice sugar right there 🍠!

It's true that this flushPromises turns out to be a one-liner, but it's not at all obvious how to get to that one line. So I think it would be a benefit for Jest users to have it available as a util function.

@talkol
Copy link

talkol commented Mar 29, 2017

@pekala the one liner IMO doesn't provide the required behavior because it will not wait until the following pending promise is resolved:

function foo() {  
  return new Promise((res) => {
    setTimeout(() => {
      res()
    }, 2000);
  });
}

What about swizzling Promise and when a new Promise is created add it to some array then flush all promises will await on Promise.all over this array?

@pekala
Copy link

pekala commented Mar 29, 2017

@talkol I think it will, as long as you us the fake timers as well. I haven't tested that though.

@talkol
Copy link

talkol commented Mar 29, 2017

@pekala no need to fake timers with this example since the promise will resolve only after the time is reached
I'm just worried that swizzling Promise will mess with jest inner workings, it's a bit hard core

@pekala
Copy link

pekala commented Mar 29, 2017

If you don't fake timers, your tests will take real 2s+ to complete. I think the best practice would be to remove these types of delays, in which case the flushPromises as proposed by @jwbay does the job.

@talkol
Copy link

talkol commented Mar 29, 2017

All depends on what you're trying to test :) All I'm saying is the timers are an unrelated concern to waiting for the promises

@philwhln
Copy link

We're hitting issues related to Promises not resolving, which are intermixed with setTimeout calls. In jest v19.0.2 we have no problems, but in jest v20.0.0 Promises never enter the resolve/reject functions and so tests fail. Our issue seems to be related this issue of not having an API to flush the Promise resolution queue, but this issue seems to pre-date jest v20.0.0 where we started to see the issue, so I'm not completely sure.

@philwhln
Copy link

This is only solution we've been able to come-up with for some of our tests, since we have a series of alternating setTimeouts and Promises used in the code that eventually calls the onUpdateFailed callback.

  ReactTestUtils.Simulate.submit(form);
  return Promise.resolve()
    .then(() => { jest.runOnlyPendingTimers(); })
    .then(() => { jest.runOnlyPendingTimers(); })
    .then(() => { jest.runOnlyPendingTimers(); })
    .then(() => {
      expect(onUpdateFailed).toHaveBeenCalledTimes(1);
      expect(getErrorMessage(page)).toEqual('Input is invalid.');
    });

Not so pretty, so any advice here greatly appreciated.

@kolman
Copy link

kolman commented Jun 4, 2017

Another example where you cannot return promise from test:

describe('stream from promise', () => {
  it('should wait till promise resolves', () => {
    const stream = Observable.fromPromise(Promise.resolve('foo'));
    const results = [];
    stream.subscribe(data => { results.push(data); });
    jest.runAllTimers();
    expect(results).toEqual(['foo']);
  });
});

This test fails with jest 20.0.4.

@ifours
Copy link

ifours commented Jun 13, 2017

@philwhln 's solution also can be written with async/await

ReactTestUtils.Simulate.submit(form);

await jest.runOnlyPendingTimers();
await jest.runOnlyPendingTimers();
await jest.runOnlyPendingTimers();

expect(onUpdateFailed).toHaveBeenCalledTimes(1);
expect(getErrorMessage(page)).toEqual('Input is invalid.');

@GeeWee
Copy link

GeeWee commented Sep 12, 2017

I would love a utility function that flushed the promise queue

@constantijn
Copy link

I would love a function that flushes the promise queues between tests also.

I'm testing code that uses Promise.all to wrap multiple promises. When one of those wrapped promises fails (because that is what I want to test) the promise immediately returns meaning the other promises sometimes (race condition, non deterministic) return while the next test is running.

This causes all sorts of havoc with my tests having non predictable/repeatable outcomes.

@philipp-spiess
Copy link
Contributor

To properly implement this, we'd need to mock Promise so we can eventually see all enqueued micro tasks to resolve them synchronously. Something in the way of what promise-mock is doing.

There's already an API to flush the micro tasks enqueued with process.nextTick and that API should probably also work with Promises (jest.runAllTicks).

@lukeapage
Copy link
Contributor

I had a solution with jasmine that hooked into the nextTick of Yaku, a promise library and caught nextTick calls and allowed playing them early.
However jest uses promises itself, which made this problematic.
In the end I took Yaku and hacked it to have a flush method which flushes out its queue. By default it runs normally using nextTick, but if you call flush all pending promise handlers execute.
The source is here:
https://github.com/lukeapage/yaku-mock
It could do with tidying up, contacting ysmood to see what they think of it and adding documentation, but it pretty much does what you want and worked for me as a simple solution to make promises sync in tests.

@thymikee
Copy link
Collaborator

As a simple workaround to that I like the @jwbay's solution.

How about we add something similar to jest object?

await jest.nextTick();

Implemented as

const nextTick = () => new Promise(res => process.nextTick(res));

cc @cpojer @SimenB @rogeliog

@eclewlow
Copy link

eclewlow commented May 22, 2018

I am using enzyme to mount React components.

I, too, have functions that expect Promises to execute, but none of the aforementioned fixes worked. I would be able to handle them synchronously in my test - if - the functions returned the Promise objects, using await, but unfortunately the functions do not return the Promise objects.

This is the workaround that I ended up doing using a spy on the global Promise function.

global.Promise = require.requireActual('promise');

it('my test', async () => {
    const spy = sinon.spy(global, 'Promise');

    wrapper.props().dispatch(functionWithPromiseCalls());

    for (let i = 0; i < spy.callCount; i += 1) {
      const promise = spy.getCall(i);
      await promise.returnValue;
    }

    expect(...)
});

@cyberhck
Copy link

cyberhck commented Oct 9, 2020

my usecase:

public static resolvingPromise<T>(result: T, delay: number = 5): Promise<T> {
    return new Promise((resolve) => {
        setTimeout(
            () => {
                resolve(result);
            },
            delay
        );
    });
}

test file:

it("accepts delay as second parameter", async () => {
    const spy = jest.fn();
    MockMiddleware.resolvingPromise({ mock: true }, 50).then(spy);
    jest.advanceTimersByTime(49);
    expect(spy).not.toHaveBeenCalled();
    jest.advanceTimersByTime(1);
    await Promise.resolve(); // without this line, this test won't pass
    expect(spy).toHaveBeenCalled();
});

@sibelius
Copy link

anybody come up with something with async_hooks ?

@thernstig
Copy link
Contributor

thernstig commented Apr 7, 2021

Is there any workaround that works with the modern fake timers, promises (async/await) that could be added to this page: https://jestjs.io/docs/next/timer-mocks, so we all know what do do? This thread is starting to get hard to follow.

I think many of us has a lot of code where promises should resolve after advancing timers (using modern), and that spies/mock functions are expected toHaveBeenCalled when those promises have resolved. If it requires calling some magic function to "flush" these promises it would be nice to have this solution documented (at least until a version where this "just works" without having to do any custom magic).

@thesmart
Copy link

thesmart commented Apr 7, 2021

IMO, a clean solution to this issue is making timer mocks async and advising users to include lint rules about unhandled async. So for example:

test file:

it("accepts delay as second parameter", async () => {
    const spy = jest.fn();
    MockMiddleware.resolvingPromise({ mock: true }, 50).then(spy);
    jest.advanceTimersByTime(49);
    expect(spy).not.toHaveBeenCalled();
    jest.advanceTimersByTime(1);
    await Promise.resolve(); // without this line, this test won't pass
    expect(spy).toHaveBeenCalled();
});

changes to:

it("accepts delay as second parameter", async () => {
    const spy = jest.fn();
    MockMiddleware.resolvingPromise({ mock: true }, 50).then(spy);
    await jest.advanceTimersByTime(49);
    expect(spy).not.toHaveBeenCalled();
    await jest.advanceTimersByTime(1);
    expect(spy).toHaveBeenCalled();
});

In the meantime, perhaps simply documenting that one must call await Promise.resolve(); after advancing timers or else there be dragons. what we should do as a work-around to this pernicious issue.

@thernstig
Copy link
Contributor

thernstig commented Apr 7, 2021

@thesmart and that solution (await Promise.resolve();) works when using modern, no matter of the amount of promises that need to resolve in a chain?

@thesmart
Copy link

thesmart commented Apr 7, 2021

@thesmart and that solution (await Promise.resolve();) works when using modern, no matter of the amount of promises that need to resolve in a chain?

Not sure, honestly. Probably would need to create some test cases for either modern or legacy as part of a patch. The default is now modern (see: sinonjs) but I'm not sure what version of Jest aligned w/ the test case above. Whatever the documented workaround is, it should note what to do for modern and/or legacy.

@dobradovic
Copy link

dobradovic commented Apr 20, 2021

Maybe this is off topic but I really need some help with mocking async / await functions.

I want to test this async / await function using jest/enzyme

 deleteAndReset = async () => {

    await this.submitDeleteMeeting();
    this.props.resetMeetingState();
  }

which occurs on

<BackButton
            data-test="backButton"
             onClick: this.deleteAndReset 
          />

What I tried

describe('Delete and reset on click', () => {

  const props = {
    intl,
    deleteAndReset: jest.fn(),
    submitDeleteMeeting: jest.fn()
  };

  it('on click call deleteAndReset', async () => {
    component = shallow(<StartMeeting {...props} />);

    const backButton = findByDataTestAttr(component, 'backButton');

    expect(backButton.exists()).toEqual(true);

    const instance = component.instance();

    backButton.props().onClick();

    await Promise.resolve(); 

    expect(instance.props.submitDeleteMeeting).toHaveBeenCalled();
  });
});

This is what I got - ```
"expect(jest.fn()).toHaveBeenCalled()

Expected number of calls: >= 1
Received number of calls:    0"
What I am doing wrong with async / await testing?

Sorry for a little bit longer post but this "simple" thing drives me crazy for long time. 

Any advice and suggest would be helpful, thanks for your time! :)

@hon2a
Copy link
Author

hon2a commented Apr 21, 2021

@dobradovic It is off-topic indeed. I'd recommend StackOverflow, unless you'd like to report a bug, in which case a new, separate issue would do the trick.

undergroundwires added a commit to undergroundwires/privacy.sexy that referenced this issue May 4, 2021
The tests mock JS setTimeout API. However promise.resolve() is not working without flushing the promise queue (which could be done just by awaiting Promise.resolve()), similar issue has been discussed in jestjs/jest#2157.
@jwbay
Copy link
Contributor

jwbay commented Aug 12, 2021

This solution was broken for the JSDOM environment in Jest 27, via the removal of setImmediate: #2157 (comment)

Removal: #11222

An equal implementation of waitForPromises now gets a lot trickier, complicated further by the fact that setImmediate in JSDOM apparently bypassed fake timers. So moving to another timer means we need to handle faking.

One solution is switching to setTimeout, and temporarily forcing real timers. For illustration:

return new Promise(resolve => {
  jest.useRealTimers();
  setTimeout(() => resolve(), 0);
  jest.useFakeTimers();
});

Now we obviously don't want to force fake timers if they weren't fake already. I seem to be able to tell whether timers are faked via jest.isMockFunction(setTimeout) ✔️. However, I can't tell which timer style is currently being used; legacy vs. modern ❌. For a widely used function to suddenly need this context to work is quite painful.

@SimenB short of a fully supported jest.waitForPromises, would you instead consider a new Jest API like:

jest.withRealTimers(() => {
  // code here is always run with real timers
});
// original timer state restored -- whether real, legacy, or modern

This could be nice even once legacy is removed, since the test code doesn't need to try and detect the current timer state.

Alternately, has anyone else worked around this differently? I tried process.nextTick and that works some of the time for legacy, but modern timers mock nextTick.

@stephenh
Copy link

@jwbay we've been using requireActual:

export function flushPromises(): Promise<void> {
  return new Promise(jest.requireActual("timers").setImmediate);
}

@acontreras89
Copy link

The following implementation was working just fine (and still does when used with legacy timers):

global.flushPromises = () => new Promise(process.nextTick)

However, it is not working with modern timers because process.nextTick is also mocked. I am curious to know why, since @sinonjs/fake-timers does not do this by default (see the default value of config.toFake)

@zyf0330
Copy link

zyf0330 commented Aug 25, 2022

With idea from @acontreras89 , I don't need to flushPromises, just let Jest doNotFake nextTick to make Promise works.

@peter-bloomfield
Copy link

I've hit a similar problem where I'm trying to test code which automatically retries a series of asynchronous operations. It uses a combination of promises and timers internally, and can't always expose a promise or callback to signal completion.

Based on the suggestion by @stephenh, here's a utility function I wrote to help my tests wait for all pending timers and promises:

const waitForPromisesAndFakeTimers = async () => {
  do {
    jest.runAllTimers();
    await new Promise(jest.requireActual('timers').setImmediate);
  } while (jest.getTimerCount() > 0);
};

The loop is there to catch timers which were scheduled following resolution of other promises. It definitely isn't suitable for all situations (e.g. recursive timers), but it's useful in some cases.

@EduardoSimon
Copy link

@peter-bloomfield Thanks a lot! Your snippet worked like a charm. I have the exact same situation, I have a fetch implementation that retries the requests using setTimeout and wrapping them in a promise each time. I wanted to extract the value out of the promise so I had to await it again:

const requestPromise = request(requestUrl)
await waitForPromisesAndFakeTimers();

const { status } = await requestPromise

expect(status).toEqual('success');

LarrMarburger pushed a commit to LarrMarburger/privacy.sexy that referenced this issue Nov 16, 2023
The tests mock JS setTimeout API. However promise.resolve() is not working without flushing the promise queue (which could be done just by awaiting Promise.resolve()), similar issue has been discussed in jestjs/jest#2157.
ilbertt added a commit to omnia-network/ic-websocket-sdk-js that referenced this issue Dec 8, 2023
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 15, 2024
@odinho
Copy link

odinho commented Feb 15, 2024

Still think this makes sense as a util helper. I've used flushPromises() in all projects since I started following this.

@github-actions github-actions bot removed the Stale label Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests