Open hon2a opened 7 years ago
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.
@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.
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?
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()
})
})
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(...
})
@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.
@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?
@talkol I think it will, as long as you us the fake timers as well. I haven't tested that though.
@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
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.
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
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.
This is only solution we've been able to come-up with for some of our tests, since we have a series of alternating setTimeout
s and Promise
s 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.
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.
@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.');
I would love a utility function that flushed the promise queue
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.
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
).
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.
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
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(...)
});
I encountered a use case for this (thanks @jwbay for the awesome technique)
For example, you want to check that your function has a timeout, and that the timeout is precisely enforced:
jest.useFakeTimers();
const EXPECTED_DEFAULT_TIMEOUT_MS = 10000;
const catchHandler = jest.fn().mockImplementationOnce(err => {
expect(err).not.toBeNull();
expect(err.message).toContain('timeout');
});
// launch the async func returning a promise
fetchStuffWithTimeout().catch(catchHandler);
expect(catchHandler).not.toHaveBeenCalled(); // not yet
jest.advanceTimersByTime(EXPECTED_DEFAULT_TIMEOUT_MS - 1);
await flushPromises();
expect(catchHandler).not.toHaveBeenCalled(); // not yet
jest.advanceTimersByTime(1);
await flushPromises();
expect(catchHandler).toHaveBeenCalledTimes(1); // ok, rejected precisely
returning a promise doesn't allow checking the precise timing of the resolution/rejection.
A promise flushing is needed there. Without it, the expectation is called too early.
Hope this helps narrowing down the problem.
For people following along, there's an open PR for this here: #6876
Cross posting from https://github.com/airbnb/enzyme/issues/1587
I wonder if the following pattern should be enough to solve this problem, and if I'm doing something that is considered bad practice and I shouldn't be doing.
What do people think about this approach?
export class MyComponent extends React.Component {
constructor (props) {
super(props)
this.hasFinishedAsync = new Promise((resolve, reject) => {
this.finishedAsyncResolve = resolve
})
}
componentDidMount () {
this.doSomethingAsync()
}
async doSomethingAsync () {
try {
actuallyDoAsync()
this.props.callback()
this.finishedAsyncResolve('success')
} catch (error) {
this.props.callback()
this.finishedAsyncResolve('error')
}
}
// the rest of the component
}
And in the tests:
it(`should properly await for async code to finish`, () => {
const mockCallback = jest.fn()
const wrapper = shallow(<MyComponent callback={mockCallback}/>)
expect(mockCallback.mock.calls.length).toBe(0)
await wrapper.instance().hasFinishedAsync
expect(mockCallback.mock.calls.length).toBe(1)
})
I had an issue when the async call was not done straight in componentDidMount, but it was calling an async function, that was calling another async function and so on. If I added an extra async step in all the async chain, I'd need to add an extra .then()
or an extra await
, but this is working just fine.
Is there a reason why I shouldn't be using this approach or does this looks good to people?
I went on an adventure in doing this in userland and found it's actually doable and not so bad (though there are quite a few pitfalls to be run into if you don't have a map). Here's an experience report that's (hopefully) detailed enough to use directly; a TLDR is to transpile async
/await
down to promises, and swap native promises for bluebird and native timers for lolex; transpile everything, including node_modules/
; queueMicrotask
is the primitive you need for promises, but by default lolex won't provide it due to JSDOM not providing it.
I ran into the same issue with jest.mockAllTimers()
and React components that call a Promise
in componentDidMount()
.
The solution from #issuecomment-279171856 solved the problem in an elegant way.
We need something similar in the official Jest API!
I just recently ran into an issue while upgrading a bunch of stuff, it revealed an issue in a bunch of tests where we were not always waiting for promises to finish. And while methods like await new Promise(resolve => setImmediate(resolve));
did work in simple cases, I found in my tests, i'd have to run that a few times to clear the pipe. Which is what @quasicomputational mentioned in their exploration here. Unfortunately, I don't think there is a way to know when that pipe is clear without intercepting the promises as they get created. So I created a little library to do that...promise-spy. Though, I did have one test that was using fake timers and it didn't work with that...so its not yet a fully working solution.
Though I also imagine they may only work with async
/await
alls in your code to be tested IF they are transpiled to promises. If they are not transpiled to promises, then this library would not be able to hook into them and wait for them to complete.
I found myself having this same issue and I realized: we should not flush pending promises but instead we should have the entire test fail if there are pending promises. This way we will be forced to abort pending promises inside of the tested code using the Abort Controller: https://developers.google.com/web/updates/2017/09/abortable-fetch Having jest flushing promises is equal to say "Concurrency is hard so let's not test it". In reality it should be quite the opposite. Since concurrency is hard we should test it even more and not allow at all a test to pass with pending promises.
Given the mess on aborting promises on this Stackoverflow question is clear is not (YET) an easy thing to do: https://stackoverflow.com/a/53933849/373542 I'm going to try to write a KISS implementation of aborting my fetch promises and will post the result here.
@giorgio-zamparelli: "Concurrency is hard so let's not test it" is completely beside the point of the original report. The issue is not concerned with pending promises but rather with the fact that awaiting propagation of promise resolution through async code in tests is needlessly hard.
I think flushing promises would be curing the symptoms instead of the illness.
Promises should resolve normally in tests without the need to be flushed.
If there is a pending promise in your test you should either wait for it to resolve using for example wait
from @testing-library/react
OR if the pending promise is not part of the scope of the test you should either mock the code starting it or you should abort the pending promise somewhere like on the React willUnmount lifecycle event using the AbortController
The AbortController is a new API that almost no-one is using and I have the feeling is the fix for most of the hanging promises in tests.
PROVE ME WRONG: I could be easily proven wrong if someone that reported having problems with pending problems in this Issue already tried using AbortController and jest.mock and it wasn't enough.
@giorgio-zamparelli: Maybe the misunderstanding stems from my use of the phrase "flush all pending promise handlers" (and if it does, I'm sorry). As you'll hopefully see if you read the issue description thoroughly, I meant "pending handlers of promises".
So, to reiterate, we're not talking about pending promises here (in any way), but rather about flushing promise resolution with minimum hassle. Or, in other words, about transparently and deterministically getting from the point where a promise is resolved to the point where all the subsequent effects tied to it are invoked (so that we can test the result of this).
I recently released flush-microtasks
for this purpose. It borrows its implementation from React, which is surprisingly more complex than @jwbay's solution here or @thymikee's solution here. I'm unsure whether the complexity makes any meaningful difference, but I assume it accounts for edge cases not considered by the other solutions in this thread. I only used that implementation because react-testing-library
uses it (see here), but does not expose it.
import { flushMicroTasks } from 'flush-microtasks'
await flushMicroTasks()
@aleclarson Is there any difference between flush-microtasks and flush-promises
@ramusus Looks like flush-promises
uses the same approach as @jwbay's solution.
https://github.com/kentor/flush-promises/blob/46f58770b14fb74ce1ff27da00837c7e722b9d06/index.js
RTL has also copied React's code: https://github.com/testing-library/react-testing-library/blob/8db62fee6303d16e0d5c933ec1fab5841dd2109b/src/flush-microtasks.js
EDIT: hah, already mentioned :grinning:
I'm not sure we need to build it into Jest when people can use that? Maybe we can link to it in the docs? This issue is about flushing them synchronously, which I think is beyond what we want to do (especially as it's impossible with async-await
)
The flushPromises
solution works only on Promises which are resolved immediately but not on those which are still pending.
Hmm, good point. I don't know if it's possible to track pending
promises somehow. Might be able to do something clever with async_hooks
, not sure. Probably gonna be painful trying to differentiate between promises created by userland code and promises created by Jest and its dependencies
I've tried to wrap/mock the Promise
object to include a counter but that doesn't work:
const _promise = window.Promise;
window.Promise = function(promiseFunction){
// counter
return new _promise(promiseFunction);
}
Main issue is async
functions which do not use the global Promise
at all
Ok, I've found a really hacky way like this one.
wrapper.update()
from enzyme. Do here something similar if necessary.I know, it's not a good practice to adjust code to the tests BUT I use this logic already on server side rendering. But in the end it's just waiting. ¯\_(ツ)_/¯
There's an interesting update to this in Jest 26, where fake timers are now based on @sinon/fake-timers (if enabled with jest.useFakeTimers('modern')
).
I tried the modern fake timers with my tests, and unfortunately it causes the await new Promise(resolve => setImmediate(resolve));
hack to hang indefinitely. Fortunately, @sinon/fake-timers
includes a several *Async()
methods which "will also break the event loop, allowing any scheduled promise callbacks to execute before running the timers.". Unfortunately, I don't see any way to get the clock
object via the Jest APIs.
Anyone know how to get Jest to give us that clock
object?
Like others, my motivation to use await new Promise(setImmediate);
is to flush resolvable promises, so that I can unit test their impact on the system.
It would seem the "modern" fake timers indeed underperform others by timing out seemingly nonsensically.
Here's some unit tests to describe this:
describe('flushing of js-queues using different timers', () => {
beforeAll(() => {
// It would take the failing test 5 long seconds to time out.
jest.setTimeout(100);
});
it.each([
[
'given real timers',
() => {
jest.useRealTimers();
},
],
['given no timers', () => {}],
[
'given "legacy" fake timers',
() => {
jest.useFakeTimers('legacy');
},
],
[
// This is the the failing scenario, not working like the other timers.
'given "modern" fake timers',
() => {
jest.useFakeTimers('modern');
},
],
])(
'%s, when using setImmediate to flush, flushes a promise without timing out',
async (_, initializeScenarioSpecificTimers) => {
initializeScenarioSpecificTimers();
let promiseIsFlushed = false;
Promise.resolve().then(() => {
promiseIsFlushed = true;
});
// Flush promises
await new Promise(setImmediate);
expect(promiseIsFlushed).toBe(true);
},
);
});
I feel like the previous test should not fail as it does.
For me the workaround was to flush promises by using the node-native "setImmediate" from package "timers", instead of the global "setImmediate". Having this, the following passes:
import { setImmediate as flushMicroTasks } from 'timers';
it('given "modern" fake timers, when using native timers to flush, flushes a promise without timing out', async () => {
jest.useFakeTimers('modern');
let promiseIsFlushed = false;
Promise.resolve().then(() => {
promiseIsFlushed = true;
});
// Flush micro and macro -tasks
await new Promise(flushMicroTasks);
expect(promiseIsFlushed).toBe(true);
});
Thanks @aleclarson.
Here's our solution for this issue:
https://github.com/team-igniter-from-houston-inc/async-fn https://medium.com/houston-io/how-to-unit-test-asynchronous-code-for-javascript-in-2020-41c124be2552
Test code can be written like:
// Note: asyncFn(), extends jest.fn() with a way to control resolving/rejecting of a promise
const load = asyncFn();
const afterLoad = jest.fn();
const result = 'mock result';
mount(<Component load={load} afterLoad={afterLoad} />);
// ... some interaction that requires the `load`
// Note: New way to controlling when promise resolves
await load.resolve(result);
expect(afterLoad).toHaveBeenCalledWith(result);
Note how you don't need to know anything about flushing promises or running timers.
@jansav nice/+1. Fwiw I've seen that approach called the deferred pattern. I think it does make for nicer tests.
It seems to me that the issue with fake timers is that it breaks the natural run-loop for how timers are supposed to function. I wonder why we can't simply have the jest timer run functions be async? Changing timers to resolve synchronously does make the test code look neat, but its causing this massive side-effect.
my usecase:
public static resolvingPromise<T>(result: T, delay: number = 5): Promise<T> {
return new Promise((resolve) => {
setTimeout(
() => {
resolve(result);
},
delay
);
});
}
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();
});
anybody come up with something with async_hooks ?
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).
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.
@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?
Do you want to request a feature or report a bug?
Feature, I guess, but a pretty important one when testing code that uses
Promise
s.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:
And the test code looks something like this:
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:What is the expected behavior?
I'd expect Jest to provide some sort of an API to flush all pending promise handlers, e.g.:
I've tried
runAllTicks
andrunAllTimers
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 :)