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

Version 2.x not compatible with jest.useFakeTimers('modern'); #391

Closed
ccfz opened this issue Jun 22, 2020 · 10 comments · Fixed by #397
Closed

Version 2.x not compatible with jest.useFakeTimers('modern'); #391

ccfz opened this issue Jun 22, 2020 · 10 comments · Fixed by #397
Labels
bug Something isn't working

Comments

@ccfz
Copy link

ccfz commented Jun 22, 2020

Describe the bug

Using jest.useFakeTimers('modern'); in any spec file, causes the tests to fail due to timeout error:

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

Downgrading to 1.14 fixes this, therefore I believe this is related to 2.x.

I am using jest.useFakeTimers('modern'); in order to call jest.setSystemTime in my test just FYI.

Expected behavior

Tests should pass

Steps to Reproduce

Simple component:

import React from 'react';
import { Text } from 'react-native';

const TestComponent = () => (
  <Text>hello world!</Text>
);

export default TestComponent;

spec file:

import React from 'react';
import { render } from 'react-native-testing-library';

import TestComponent from 'src/components/TestComponent';

jest.useFakeTimers('modern');

describe('TestComponent', () => {
  it('should render hello world!', () => {
    const utils = render(<TestComponent />);

    expect(utils.getByText('hello world!')).not.toBeNull();
  });
});

Versions

    react: 16.13.1 => 16.13.1 
    react-native: 0.62.2 => 0.62.2 
    react-native-testing-library: ^2.1.0 => 2.1.0 
    react-test-renderer: ^16.13.0 => 16.13.1 
@thymikee
Copy link
Member

Thanks, this is interesting. After a brief investigation, I've identified that this code is to "blame", specifically the await flushMicroTasks(); call:

afterEach(async () => {
await flushMicroTasks();
cleanup();
});

flushMicroTasks looks like this:

export function flushMicroTasks(): Promise<any> {
return new Promise((resolve) => setImmediate(resolve));
}

Interesting thing is that it works fine with legacy timers.

The @testing-library/react resolves similar problem by requiring setImmediate from timers node module:
https://github.com/testing-library/react-testing-library/pull/514/files#diff-3ee797e0d5d82fbd2501ceaa19ce170bR16-R18
and returning a thenable:
https://github.com/testing-library/react-testing-library/pull/514/files#diff-3ee797e0d5d82fbd2501ceaa19ce170bR42-R46

Verified that this works with both legacy and modern timers.

@ccfz As a workaround until this is resolved you'll need to skip auto cleanup by setting RNTL_SKIP_AUTO_CLEANUP=1 env when running tests. This didn't surface in v1 because we didn't call flush + cleanup after each test.

cc @SimenB ideas on why this scenario regressed? You have more context on the topic.

Here's a diff with repro and fix that makes it work (likely will send a PR later):

diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js
index e38f05c..874c5be 100644
--- a/src/__tests__/waitFor.test.js
+++ b/src/__tests__/waitFor.test.js
@@ -93,3 +93,10 @@ test('works with fake timers', async () => {

   jest.useRealTimers();
 });
+
+it('should render hello world!', () => {
+  jest.useFakeTimers('modern');
+  const utils = render(<BananaContainer />);
+
+  expect(utils.getByText('Change freshness!')).not.toBeNull();
+});
diff --git a/src/flushMicroTasks.js b/src/flushMicroTasks.js
index c4bb679..37816b7 100644
--- a/src/flushMicroTasks.js
+++ b/src/flushMicroTasks.js
@@ -10,6 +10,22 @@ export default function flushMicrotasksQueue(): Promise<any> {
   return flushMicroTasks();
 }

+let enqueueTask;
+try {
+  // assuming we're in node, let's try to get node's
+  // version of setImmediate, bypassing fake timers if any.
+  enqueueTask = require('timers').setImmediate;
+} catch (_err) {
+  // we're in a browser, do nothing
+  enqueueTask = (resolve) => {
+    resolve();
+  };
+}
+
 export function flushMicroTasks(): Promise<any> {
-  return new Promise((resolve) => setImmediate(resolve));
+  return {
+    then(resolve) {
+      enqueueTask(resolve);
+    },
+  };
 }

@thymikee thymikee added the bug Something isn't working label Jun 22, 2020
@ccfz
Copy link
Author

ccfz commented Jun 22, 2020

@thymikee thanks for the very quick response! Workaround works as expected.

@SimenB
Copy link
Contributor

SimenB commented Jun 23, 2020

Interesting thing is that it works fine with legacy timers.

That's because they actually run immediates (and ticks) on real timers even when time is faked: https://github.com/facebook/jest/blob/4471bbbb2d009a5d71c7beb702cb3cadb46d0a94/packages/jest-fake-timers/src/legacyFakeTimers.ts#L443-L451. So even when you say useFakeTimers, they run on real time (although you can trigger them early by running jest.runAllTimers() etc).

Might make sense to add some sort of jest.isTimeFaked() or something, so test envs/libs can check and try to run them?

@thymikee
Copy link
Member

@SimenB the issue turned out to be using Promise and fixed by converting it to a "thenable", which works for both legacy and modern timers. If you have any ideas why Sinon timers have issues with it, please leave a comment. I'd like to understand that better.

@SimenB
Copy link
Contributor

SimenB commented Jul 10, 2020

Only idea is jestjs/jest#10221 (comment)

@thymikee
Copy link
Member

This suggestion seem to work with Promise.resolve and setImmediate or new Promise and nextTick(), spawning unhandled promise rejections. But it breaks with thenable (which is something that react-testing-library does as well) and new Promise and setImmediate(). Ugh 😛

@ccfz
Copy link
Author

ccfz commented Aug 6, 2020

Hi @thymikee I updated to the latest version and removed the workaround, and I am still getting the same Timeout error.

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

Did my example from above pass for you with the fix implemented?
Thanks for the help!

  npmPackages:
    @testing-library/react-native: ^7.0.1 => 7.0.1 
    react: 16.13.1 => 16.13.1 
    react-native: 0.63.2 => 0.63.2 
    react-test-renderer: ^16.13.1 => 16.13.1 

@dcalhoun
Copy link
Contributor

dcalhoun commented Aug 7, 2020

@ccfz @thymikee I'll chime in to say that I am also still struggling with this issue with react-native-testing-library@2.1.0. In order to use waitFor, I need to call jest.useRealTimers() for the specific test. While verbose, given I have to place it throughout my tests, it works for the most part. However, it doesn't work if I need to use both waitFor and fake timers within the same test.

If it's helpful, then I can try to find time to create a reproduction case for what I am seeing.

@thymikee
Copy link
Member

Creating a new issue and a repro would be appreciated!

@dcalhoun
Copy link
Contributor

dcalhoun commented Aug 14, 2020

@thymikee I have created #506 that includes a test case. Please let me know if I can provide any additional information that would be helpful. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants