Jest: "Modern" fake-timer implementation doesn't work with PromiseJS.

[chrisbobbe]

Awesome work on https://github.com/facebook/jest/pull/7776, thanks for that!!

🐛 Bug Report

I'm using Jest 26.1.0.

Testing async code with Promises, as described in this Jest doc, doesn't seem to work with jest.useFakeTimers('modern'); (from https://github.com/facebook/jest/pull/7776; documented here) if you're using the 'promise' library from NPM.

For a while (starting in https://github.com/facebook/react-native/commit/3ff39870ce776c653823e1733363be0401896294), React Native has used that library as part of their Jest setup (which you get if you use preset: 'react-native'), so I expect this will affect most people using React Native (this is how I encountered it).

Otherwise, I'm not sure how often people use that 'promise' library, but it's an official solution given in a Jest troubleshooting doc, and pops up as a workaround for some issues in this repo (e.g. here).

This is arguably a regression (or would be when 'modern' becomes the default), as this behavior isn't observed with jest.useFakeTimers('legacy'); or jest.useRealTimers().

To Reproduce

E.g.,

global.Promise = require('promise');

jest.useFakeTimers('modern');

test('1 equals 1', async () => {
  await Promise.resolve();
  expect(1).toEqual(1);
});

Expected behavior

I expect the test to pass. Instead, five seconds go by and I get this failure:

  ✕ 1 equals 1 (5006 ms)

  ● 1 equals 1

    : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      3 | jest.useFakeTimers('modern');
      4 | 
    > 5 | test('1 equals 1', async () => {
        | ^
      6 |   await Promise.resolve();
      7 |   expect(1).toEqual(1);
      8 | });

      at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:116:22)
      at Object.<anonymous> (fetchData.test.js:5:1)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        5.762 s, estimated 6 s
Ran all test suites.
error Command failed with exit code 1.

Link to repl or repo (highly encouraged)

In the repro linked below, I've closely imitated all the tests from the sections in the "Testing Asynchronous Code" doc that use Promises, to suggest that the failure happens in usual, well-documented ways of testing (provided you're using PromiseJS).

In its current state, you should observe these timeout errors on tests that use Promises (just run yarn and yarn test).

Try uncommenting global.Promise = require('promise'); at the top of fetchData.test.js; the tests should all pass.

Try doing something other than jest.useFakeTimers('modern');; the tests should all pass.

(Incidentally, note that accidentally calling jest.useFakeTimers multiple times with different arguments in the same file seems disruptive and might hamper your investigation. Also, be sure to comment out jest.runAllTimers() if you're using real timers.)

https://github.com/chrisbobbe/jest-async-modern-timers-repro

envinfo

  System:
    OS: macOS 10.15.5
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 10.20.1 - ~/.nvm/versions/node/v10.20.1/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.4 - ~/.nvm/versions/node/v10.20.1/bin/npm
  npmPackages:
    jest: ^26.1.0 => 26.1.0 

[Hedi-s]

I have the same error with nock 12.0.3 and jest 26.1.0

The test case

test('nock', async () => {
    jest.useFakeTimers('modern');
    nock('http://modern').get("/").reply(200);
    await fetch(`http://modern`, { method: "GET" });
});

Failing :

 : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:
  > 18 |         test('nock', async () => {
    |         ^
    19 |             jest.useFakeTimers('modern');
    20 |             nock('http://modern').get("/").reply(200);
    21 |             await fetch(`http://modern`, { method: "GET" });

It's working when I replace

• 'modern' by 'legacy'
  or
• fetch(...) by Promise.resolve() (nock is not actually used)

[SimenB]

Hmm, I wonder if this is because we fake process.nextTick as well. As a test, can you modify your node_modules/@jest/fake-timers/build/modernFakeTimers.js (or wherever it's located) from the line const toFake = Object.keys(this._fakeTimers.timers); to const toFake = Object.keys(this._fakeTimers.timers).filter(name => name !== 'nextTick');. That should exclude it. This is the default behavior of @sinonjs/fake-timers, which I've changed in Jest's implementation.

---

As an aside, one should never replace global.Promise - that suggestion is outdated. E.g. when using async-await you will always get the native Promise regardless of the value of global.Promise.

[Hedi-s]

modify your node_modules/@jest/fake-timers/build/modernFakeTimers.js (or wherever it's located) from the line const toFake = Object.keys(this._fakeTimers.timers); to const toFake = Object.keys(this._fakeTimers.timers).filter(name => name !== 'nextTick');.

The test with nock is working fine after doing it.
Since I tried 'modern' for _.debounce it's worth mentionning that a test I have with debounce is still working (It was not obvious to me).

[chrisbobbe]

As a test, can you modify your node_modules/@jest/fake-timers/build/modernFakeTimers.js (or wherever it's located) from the line const toFake = Object.keys(this._fakeTimers.timers); to const toFake = Object.keys(this._fakeTimers.timers).filter(name => name !== 'nextTick');.

Yep—when I do this, it works! Looks like https://github.com/sinonjs/fake-timers/pull/323 is related.

As an aside, one should never replace global.Promise - that suggestion is outdated. E.g. when using async-await you will always get the native Promise regardless of the value of global.Promise.

Thanks for the heads-up! Yeah, a priori, I'm not really interested in replacing global.Promise. But I'm using React Native, with preset: 'react-native', so it's being done whether I like it or not (RN has been doing this since https://github.com/facebook/react-native/commit/3ff39870ce776c653823e1733363be0401896294). I suppose I could make a PR to react-native and fork it in the meantime, if it came to that.

[chrisbobbe]

But I'm using React Native, with preset: 'react-native', so it's being done whether I like it or not

Opened https://github.com/facebook/react-native/issues/29303 to find out more about React Native's motives here. 🔍

[tomdaniel0x01]

Hmm, I wonder if this is because we fake process.nextTick as well. As a test, can you modify your node_modules/@jest/fake-timers/build/modernFakeTimers.js (or wherever it's located) from the line const toFake = Object.keys(this._fakeTimers.timers); to const toFake = Object.keys(this._fakeTimers.timers).filter(name => name !== 'nextTick');. That should exclude it. This is the default behavior of @sinonjs/fake-timers, which I've changed in Jest's implementation.

As an aside, one should never replace global.Promise - that suggestion is outdated. E.g. when using async-await you will always get the native Promise regardless of the value of global.Promise.

After waisting a day troubleshooting why promises weren't working, this worked! Thank you.

I think Jest should resort back to the way Lolex was before. See https://github.com/sinonjs/fake-timers/issues/114 for more context as to why Lolex didn't fake nextTick. Most people will not want the current behavior. Maybe Jest can provide an option to fake nextTick, but not by default.

[squareloop1]

Filtering out 'nextTick' works for native promises and should really be the default behaviour. Is there any workaround in the meantime for this without tinkering in node_modules to make this usable in CI/CD until a fix is out?

[tomdaniel0x01]

Filtering out 'nextTick' works for native promises and should really be the default behaviour. Is there any workaround in the meantime for this without tinkering in node_modules to make this usable in CI/CD until a fix is out?

@squareloop1 I'd recommend using this nifty little tool. I just used it to patch Jest to the working fake-timers code. The patch even works seamlessly when updating a package. It's unfortunate, but I've also had to use it on several other packages that I'm using in my React Native app. A complete life saver.

[squareloop1]

@tomdaniel0x01 thanks, I think I will use fake timers from sinon for now, as they seemed to work. But I'd would rather get rid of another dependency and use it in jest. Its the only I am really missing in jest.

I have to add to this issue for clarity: It does not only affect non-native global.promise replacements, it also makes native promises in Node.JS with TypeScript stop working.

[sbalay]

Hey @chrisbobbe, great work with the triaging of this issue! Did you get any updates about it recently? I did not see any follow up or response in the issue that you created in the RN repo.

FYI, I've found a more friendly solution than forking the entire RN dependency: hijack the react-native jest preset instead. Here you can see an example of how it's done

[chrisbobbe]

Hey @chrisbobbe, great work with the triaging of this issue!

Thanks! 🙂

Did you get any updates about it recently? I did not see any follow up or response in the issue that you created in the RN repo.

No, unfortunately I haven't had any updates.

FYI, I've found a more friendly solution than forking the entire RN dependency: hijack the react-native jest preset instead. Here you can see an example of how it's done

Great, thanks for the tip!!

[pke]

FYI, I've found a more friendly solution than forking the entire RN dependency: hijack the react-native jest preset instead. Here you can see an example of how it's done

@sbalay Thank you very much for the hint to this custom preset! It saved me more time to investigate why my tests did not work:

beforeAll(() => {
  jest.useFakeTimers("modern")
})

afterEach(cleanup)

afterAll(() => {
  jest.useRealTimers()
})

it("should report time updates every 30 minutes", () => {
    jest.setSystemTime(new Date("2020-11-19T10:00:00.000Z"))
    const { result } = renderHook(() => useDateTime("minute"))
    expect(result.current.toISOString()).toEqual("2020-11-19T10:00:00.000Z")
    act(() => {
      jest.advanceTimersByTime(1000 * 1 * 60)
    })
    expect(result.current.toISOString()).toEqual("2020-11-19T10:01:00.000Z")
  })

I did not use any async functions in my test, just setTimeout but even with an empty test, merely calling useFakeTimers made the empty test just time out.

Do you think we should report this bug to the jest react-native package?