Fake promises for fake timers

[Berzeg]

Summary

This PR introduces a way to mock promises for use with fake timers. Up to this point, using the fake timers API would change the order in which promises are being run. When calling jest.runAllTimers() or jest.runAllImmediates() the fake timers class would give priority to process.nextTick callbacks, followed by setImmediate callbacks, followed by the next elapsed timer/interval. This run-order overlooks promise callbacks, which are considered micro-tasks in the node run-time environment, and are supposed to be run after nextTick callbacks, but before any setImmediate, setTimeout, or setInterval callbacks.

A related issue has already been raised: https://github.com/facebook/jest/issues/2157

Test plan

Since this PR introduces a class that mocks the Promise API introduced in es6 there are tests that make sure each promise function call would result in the expected behaviour. To run these tests, cd to the jest project root and call:

yarn test ./packages/jest-util/src/__tests__/fake_promises.test.js

Moreover, this PR required integration between fake promises and fake timers. To test these changes run:

yarn test ./packages/jest-util/src/__tests__/fake_timers.test.js

Usage

The promise mock API interface is largely inspired by that of timer mocks. Before using the promise mock API you should:

1. Configure the jest object in your package.json file by setting the option "compileAsyncToGenerator": true, or call jest from the command line with the flag --compileAsyncToGenerator.

2. Call jest.useFakePromises() in your tests. Or, to fake all promises in your test, you can configure your jest object in your package.json file by setting the option "promises": fake or call jest from the command line with the option --promises=fake.

You can call jest.useRealPromises() to use real promises (real promises are used by default). When using fake promises you can make calls to the promise class as you normally would (i.e. Promise.resolve(value), Promise.reject(reason), [Promise].then(onResolved, onRejected), [Promise].catch(onRejected), new Promise((resolve, reject) => {...})). To empty the promise queue you should call jest.runAllPromises().

To use fake promises with fake timers simply call jest.useFakeTimers() and jest.useFakePromises() either at the top of your test file, in a beforeEach()/beforeAll() method's callback, or in your test callback before using promises/timers. Calling jest.runAllTimers() with fake promises and timers enabled would run all callbacks from calls to nextTick, promises, immediates, timeouts, and intervals in-order as they would in the node environment, but without waiting.

Example

'use strict'

jest.useFakeTimers();
jest.useFakePromises();

test('all nextTicks, promises, and timers should run in order', () => {
  const runOrder = [];
  const immediateCallback = jest.fn(() => runOrder.push('immediate'));
  const promiseCallback = jest.fn(() => runOrder.push('promise'));
  const nextTickCallback = jest.fn(() => runOrder.push('nextTick'));
  
  setImmediate(immediateCallback);
  Promise.resolve(0).then(promiseCallback);
  process.nextTick(nextTickCallback);

  expect(immediateCallback.mock.calls.length).toBe(0);
  expect(promiseCallback.mock.calls.length).toBe(0);
  expect(nextTickCallback.mock.calls.length).toBe(0);

  jest.runAllTimers();

  expect(immediateCallback.mock.calls.length).toBe(1);
  expect(promiseCallback.mock.calls.length).toBe(1);
  expect(nextTickCallback.mock.calls.length).toBe(1);

  expect(runOrder).toEqual(['nextTick', 'promise', 'immediate']);
});

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[SimenB]

@mjesun @cpojer @aaronabramov how does this fit in with the way you mock promises at FB?

[SimenB]

Thanks for the PR! Interesting feature, not sure where I stand on it. Wouldn't it be enough to do global.Promise = require('promise/setimmediate'); in your own code? Then runAllTicks should work, IIRC.

Question, does this work with async-await?

---

Also, what's this?

[Berzeg]

@simenb thanks for the review!

I took a quick look at “promise/setImmediate”, and if I understand correctly then the promises would still be running in a different order than in the node environment. For example, if you call

global.Promise = require(‘promise/setImmediate’);

then the example test I included in my PR description would fail, because the setImmediate callback would run before the promise callback.

---

Re: async-await

Not sure if it works. I think async-await is just syntactic sugar that reuses the Promise class. I need to test it before I can say if it works or not.

---

Re: No changes

I don’t know what this is. Will investigate further tonight.

---

Also, I was thinking of using fake promises by default when using fake timers, but I was thinking maybe there should be a grace period, where users have to explicitly use fake promises. Otherwise, I can change this PR to use fake/real promises whenever fake/real timers are used.

[Berzeg]

Oh, and the idea behind fake promises is for them to be used implicitly with fake timers so that jest users don’t get caught off-guard by the run-order discrepancy (i.e. micro/macro-task run-order should be be the same in node environment vs. when using fake timers in jest).

[codecov-io]

Codecov Report

Merging #6876 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6876   +/-   ##
=======================================
  Coverage   66.68%   66.68%           
=======================================
  Files         254      254           
  Lines       10915    10915           
  Branches        4        4           
=======================================
  Hits         7279     7279           
  Misses       3635     3635           
  Partials        1        1

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e4b91e3...954a164. Read the comment docs.

[mjesun]

This is pretty much what FB does internally. To give you a broad vision on that:

• Our tests use fake timers, so that we can execute synchronously code that would otherwise be asynchronous.
• We override Promise with an 1:1 compatible version that uses process.nextTick.
• async and await get transpiled to function* and yield, and wrapped with Promises. This is a must. If you override Promise and run async, you will get a native Promise still.

(Think about overriding Array and using [3], you're not going to get a custom instance of your overridden Array there). 🙂

[SimenB]

@mjesun thanks! I take that comment to mean you're not against this landing in core then? Would be awesome if you could test it out in the FB test suite once it's ready to land as well 🙂 Might make it a bit easier to land the Lolex change as well at that point

[mjesun]

My only concern in regards to the change is the need to transpile async / await, and how that plays with the fact that right now the user decides how to transpile their files. Aside from that, the implementation looks good to me.

[Berzeg]

Re: runAllPromises with runAllTicks paremeter

fake promises no longer have dependencies with nextTick mocks. It is the responsibility of fakePromise's owner (i.e. fakeTimer) to run nextTicks/promises/timers in order.

---

Re: async-await

If we don't mock promises scheduled using async-await then this PR would take jest one step forward, and one step back 😖

I can add a feature to babel-jest that transpiles async-await syntax to explicit promises. Additionally, fakePromises can warn the user if fake promises are being used AND babel-jest is not being used.

[mjesun]

Sounds good to me.

[SimenB]

I don't think we should automatically compile away async-await. Rather, it should be implemented in babel-jest directly together with #6949 when we can inspect what plugins will be loaded for Babel

[SimenB]

Also, I wonder what we should do when async-await becomes more normal on node_modules. It might be that we can't be clever and we just have to be really clear in the docs that async-await needs to be compiled away for this to work

[mjesun]

For the short term, I think it's necessary to compile async and await outside of babel-jest. If we don't, and someone registers their own Babel preset which does not transpile them anymore, tests requiring fake timers to work will start failing. From the point of view of the average user, this is unexpected and probably hardly understandable.

For the long term, these operations should maybe be done using asynchronous hooks. I'm not sure up to which point we have control at performing operations on the same tick via those hooks, but to me it sounds like a good place to start investigating.

[Berzeg]

Solution A: always compile async-await syntax to generators (with promises), but instead of doing that compilation step in babel-jest it should happen somewhere else, like jest-runtime/src/script_transformer.js.

Solution B: compile async-await to generators selectively, only for tests that use fake promises. I think there are a few caveats for this solution. For instance, you have to know if a test is using fake promises before running the test callback (not trivial, since test or it callbacks can call jest.useFakePromises()).

Just to be clear, are either of these the short-term solution you had in mind?

[mjesun]

Yes, solution A describes my short-term idea exactly. 🙂

cc’ing @rubennorte since we had a related conversation offline few days ago.

[SimenB]

BTW, I think it definitely should be opt-in to transpile async-await, especially as we might be double-transpiling and potentially adding quite some overhead since we'll have to transpile node_modules. Debugging transformed async-await is also quite painful compared to real async await. If we have the logic outside of babel-jest that means we'll have to run Babel twice (making sure to not mess up sourcemaps, although Babel should handle that for us if we pass the correct options). Another con of not keeping it in the transformer is that we'll have to implement caching (might not be too bad, though)

I'm not sure how to best alert users to the fact that they need to turn on transpilation of those things, but I can already imagine the huge amount of issues stemming from it

[Berzeg]

I modified fake promises with the recent comments in mind. To use fake promises you would have to set the new jest option compileAsyncToGenerator to true. This will add a new compile step to each file in the project being tested. It removes the dependency on babel-jest or on the user's personal babel preset. If fake promises are used with compileAsyncToGenerator set to false then the user would be warned.

[SimenB]

Sorta related: https://github.com/sinonjs/lolex/issues/114 (and #5171)

[deanshub]

So I'm guessing this isn't going to get merged?

[jsphstls]

The promises vs useFakeTimers rabbit hole leads here. What's next for this PR ?

[Berzeg]

There is no nice way to make this work with async-await syntax. IMO the best thing we could do would be to make the limitations of fakePromises clear in the documentation. We could also offer a solution in the docs: perform async-to-generator conversion yourself.

I can remove the jest option compileAsyncToGenerator, and remove the warning that would currently show up if you use fake promises with compileAsyncToGenerator set to false. This would make fake promises pretty straight forward to use.

@SimenB do you have any ideas on how to make this feature a good fit for jest?

cc: @jsphstls

[github-actions[bot]]

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

[github-actions[bot]]

This PR was closed because it has been stalled for 30 days with no activity. Please open a new PR if the issue is still relevant, linking to this one.

[github-actions[bot]]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.