-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
useFakeTimers breaks with native promise implementation #7151
Comments
The workaround I've found is to add: global.Promise = require('promise'); to my setup.js |
Hey @ForbesLindesay, thanks for filing and for the workaround 👌 Is this because native promises don't use timers under the hood like a library has to? |
Fake timers in Jest does not fake promises (yet: #6876), however - as you discovered - if you use a polyfill for |
@SimenB that's my thought as well |
I think you're right about why this doesn't work, but I don't think you're really right about this working as intended. Promises are a form of timer, and because in the past everyone was using polyfills, this used to work and only recently stopped working. |
The fact people use promises differently isn't Jest's responsibility - it was never a feature of Jest that you could run Promises using its fake timers. As you've noted, polyfilling it with something that uses timers as its implementation makes it work again |
i'm not clear on what you mean by this?
when fake timers were created, native Promises didn't exist. The intention was always that as new forms of "timer" were added to the language, fake timers would support them. For example, The goal of jest has, for a long time, included being easy to use and un-surprising. This is very surprising behaviour for fake timers to have. |
I would like to expand on this issue since it gets amplified by uses of setTimeouts within the async code: jest.useFakeTimers();
test('timing', async () => {
const shouldResolve = Promise.resolve()
.then(() => console.log('before-promise'))
.then(() => new Promise(r => setTimeout(r, 20)))
.then(() => console.log('after-promise'));
setTimeout(() => console.log('timer'), 100);
jest.runAllTimers();
await shouldResolve;
console.log('end');
});
Expected: before-promise -> after-promise -> timer -> end This issue here is there is nothing to continuously advance the timers once you're within the promise world. shouldResolve will never resolve. Switching to The best solution without replacing promises that i have come up for this is a utility function to continuouslyAdvanceTimers. However your results will still be out of order. const _setTimeout = global.setTimeout;
function continuouslyAdvanceTimers() {
let isCancelled = false;
async function advance() {
while (!isCancelled) {
jest.runOnlyPendingTimers();
await new Promise(r => _setTimeout(r, 1));
}
}
advance();
return () => {
isCancelled = true;
};
}
jest.useFakeTimers();
test('timing', async () => {
const shouldResolve = Promise.resolve()
.then(() => console.log('before-promise'))
.then(() => new Promise(r => setTimeout(r, 20)))
.then(() => console.log('after-promise'));
setTimeout(() => console.log('timer'), 100);
const cancelAdvance = continuouslyAdvanceTimers();
await shouldResolve;
cancelAdvance();
console.log('end');
}); Expected: before-promise -> after-promise -> timer -> end |
test('timing', async () => {
const shouldResolve = Promise.resolve()
.then(() => console.log('before-promise'))
.then(() => new Promise(r => setTimeout(r, 20)))
.then(() => console.log('after-promise'));
setTimeout(() => console.log('timer'), 100);
await Promise.resolve()
jest.runAllTimers()
await shouldResolve
console.log('end');
}); |
I don't think there's any point adding to this issue. The problem is clearly stated and defined. All this needs is for one of the jest maintainers to acknowledge that this is not working as intended, then someone can submit a patch to fix it. It would be good if the "Needs more info" tag could be removed, since this quite clearly doesn't need more info. Please refrain from "me-too" style comments. |
I love this issue, really. After one day sucking I found this and it works now. Miracle. Incredible that I have to do hacks like this to test an async functionality with a test framework that supports async. UPDATE. Example fix: while (!fixture.storageMock.update.mock.calls.length) {
await Promise.resolve();
} |
Note that it is impossible, by JavaScript spec, for an See e.g. petkaantonov/bluebird#1434 and sinonjs/fake-timers#114. Your best bet is probably to follow the Lolex issue - both because Jest is going to move its fake timers implementation to be backed by Lolex, but also because Ben actually maintains Node, so any news on what would allow If at some point there is a way to return custom Promise from async functions in Node, then we can look into adding APIs for it in Jest. Until then, we're unlikely to do anything |
For what it's worth, we have resorted to overwriting // Place this in the test file/test block when you want to immediately invoke
// the callback to setTimeout
window.setTimeout = (fn: () => void, _timeout: number): void => fn() |
Posting this work around in case it helps someone else:
More context here: Broader example: function sleep(ms: number): Promise<void> {
return new Promise((resolve) => {
setTimeout(resolve, ms);
});
}
export async function foo(fn: () => T, waitMs: number): Promise<T> {
await sleep(waitMs);
return fn();
} it('calls fn after x milliseconds', async () => {
jest.useFakeTimers();
const fn = jest.fn(() => 3);
const retVal = foo(fn, 1000);
expect(fn).not.toBeCalled();
await Promise.resolve().then(() => jest.advanceTimersByTime(1000));
expect(fn).toHaveBeenCalledTimes(1);
await expect(retVal).resolves.toBe(3);
}); |
For those looking for the solution to this problem when using |
Marked |
Jest fake timers don't play well with promises and setTimeout so we replace the setTimeout function and don't have to wait for the real timeout. See jestjs/jest#7151
* Set retry limit for LastFMImporter * Export LASTFM_RETRIES and use in tests * Document getPage and remove magic number 4 * Await on retry * Rewrite getPage retry logic and fix associated tests * Replace setTimeout function in LastFM getPage tests Jest fake timers don't play well with promises and setTimeout so we replace the setTimeout function and don't have to wait for the real timeout. See jestjs/jest#7151 * LastFMImporter: Retry only on 4xx errors * LFM importer: throw error instead of returning null Co-authored-by: Monkey Do <contact@monkeydo.digital>
Since |
@tatethurston I hate that this solution works, but it does. 😆 Thank you. |
I read that mocking |
Thanks, Simple and efficient |
Thanks for this thread and particular comment ❤️ async function withAllTimersRun(callback) {
const cancelAdvance = continuouslyAdvanceTimers();
const result = await callback();
await cancelAdvance();
return result;
} So in tests it could be: //...
await withAllTimersRun(() => someFunctionWithALotOfPromisesAndTimeouts());
//... |
Thanks for this information. When I used it with @KamalAman 's solution it worked perfectly. |
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. |
This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one. |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
🐛 Bug Report
Use the native Promise implementation breaks useFakeTimers
To Reproduce
Steps to reproduce the behavior:
Expected behavior
It should log:
This is because
runAllTimers
should trigger the async promise handler first, then the timeout delayed by 100ms, then return control.Actual Behaviour
Link to repl or repo (highly encouraged)
https://repl.it/repls/PhysicalBriefCores
The text was updated successfully, but these errors were encountered: