From 3e9e3737d400b9e84562cac05a3f159309d989d9 Mon Sep 17 00:00:00 2001 From: Michael James Duminy Date: Wed, 7 Oct 2020 23:54:47 +0200 Subject: [PATCH 01/12] capture global timers, use them in waitFor, tests+ --- package.json | 3 +- src/__tests__/timerUtils.js | 21 +++++ src/__tests__/timers.test.js | 23 ++++++ src/__tests__/waitFor.test.js | 75 ++++++++++++------ .../waitForElementToBeRemoved.test.js | 42 ++++------ src/helpers/getTimerFuncs.js | 77 +++++++++++++++++++ src/waitFor.js | 13 +++- 7 files changed, 199 insertions(+), 55 deletions(-) create mode 100644 src/__tests__/timerUtils.js create mode 100644 src/__tests__/timers.test.js create mode 100644 src/helpers/getTimerFuncs.js diff --git a/package.json b/package.json index 7c67376c9..49850a848 100644 --- a/package.json +++ b/package.json @@ -75,6 +75,7 @@ "js", "json" ], - "rootDir": "./src" + "rootDir": "./src", + "testPathIgnorePatterns": ["timerUtils"] } } diff --git a/src/__tests__/timerUtils.js b/src/__tests__/timerUtils.js new file mode 100644 index 000000000..649a8cf42 --- /dev/null +++ b/src/__tests__/timerUtils.js @@ -0,0 +1,21 @@ +const FakeTimerTypes = [ + 'default', + 'legacy', + // 'modern', // broken for now +]; + +function setupFakeTimers(fakeTimerType) { + switch (fakeTimerType) { + case 'legacy': + case 'modern': { + jest.useFakeTimers(fakeTimerType); + break; + } + case 'default': + default: { + jest.useFakeTimers(); + } + } +} + +export { FakeTimerTypes, setupFakeTimers }; diff --git a/src/__tests__/timers.test.js b/src/__tests__/timers.test.js new file mode 100644 index 000000000..ea0844703 --- /dev/null +++ b/src/__tests__/timers.test.js @@ -0,0 +1,23 @@ +import waitFor from '../waitFor'; +import { FakeTimerTypes, setupFakeTimers } from './timerUtils'; + +describe.each(FakeTimerTypes)('%s fake timers tests', (fakeTimerType) => { + beforeEach(() => setupFakeTimers(fakeTimerType)); + + test('it successfully runs tests', () => { + expect(true).toBeTruthy(); + }); + + test('it successfully uses waitFor', async () => { + await waitFor(() => { + expect(true).toBeTruthy(); + }); + }); + + test('it successfully uses waitFor with real timers', async () => { + jest.useRealTimers(); + await waitFor(() => { + expect(true).toBeTruthy(); + }); + }); +}); diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js index 78a05c310..c9c463026 100644 --- a/src/__tests__/waitFor.test.js +++ b/src/__tests__/waitFor.test.js @@ -2,6 +2,7 @@ import * as React from 'react'; import { View, Text, TouchableOpacity } from 'react-native'; import { render, fireEvent, waitFor } from '..'; +import { FakeTimerTypes, setupFakeTimers } from './timerUtils'; class Banana extends React.Component { changeFresh = () => { @@ -79,36 +80,60 @@ test('waits for element with custom interval', async () => { expect(mockFn).toHaveBeenCalledTimes(3); }); -test('works with legacy fake timers', async () => { - jest.useFakeTimers('legacy'); +test.each(FakeTimerTypes)( + 'waits for element until it stops throwing using %s fake timers', + async (fakeTimerType) => { + setupFakeTimers(fakeTimerType); + const { getByText, queryByText } = render(); - const mockFn = jest.fn(() => { - throw Error('test'); - }); + fireEvent.press(getByText('Change freshness!')); + expect(queryByText('Fresh')).toBeNull(); - try { - waitFor(() => mockFn(), { timeout: 400, interval: 200 }); - } catch (e) { - // suppress + jest.advanceTimersByTime(300); + const freshBananaText = await waitFor(() => getByText('Fresh')); + + expect(freshBananaText.props.children).toBe('Fresh'); } - jest.advanceTimersByTime(400); +); - expect(mockFn).toHaveBeenCalledTimes(3); -}); +test.each(FakeTimerTypes)( + 'waits for assertion until timeout is met with %s fake timers', + async (fakeTimerType) => { + setupFakeTimers(fakeTimerType); -test('works with fake timers', async () => { - jest.useFakeTimers('modern'); + const mockFn = jest.fn(() => { + throw Error('test'); + }); - const mockFn = jest.fn(() => { - throw Error('test'); - }); + try { + await waitFor(() => mockFn(), { timeout: 400, interval: 200 }); + } catch (error) { + // suppress + } - try { - waitFor(() => mockFn(), { timeout: 400, interval: 200 }); - } catch (e) { - // suppress + expect(mockFn).toHaveBeenCalledTimes(3); } - jest.advanceTimersByTime(400); - - expect(mockFn).toHaveBeenCalledTimes(3); -}); +); + +test.each(FakeTimerTypes)( + 'awaiting something that succeeds before timeout works with %s fake timers', + async (fakeTimerType) => { + setupFakeTimers(fakeTimerType); + + let calls = 0; + const mockFn = jest.fn(() => { + calls += 1; + if (calls < 3) { + throw Error('test'); + } + }); + + try { + await waitFor(() => mockFn(), { timeout: 400, interval: 200 }); + } catch (error) { + // suppress + } + + expect(mockFn).toHaveBeenCalledTimes(3); + } +); diff --git a/src/__tests__/waitForElementToBeRemoved.test.js b/src/__tests__/waitForElementToBeRemoved.test.js index ad5add433..22c54d6fa 100644 --- a/src/__tests__/waitForElementToBeRemoved.test.js +++ b/src/__tests__/waitForElementToBeRemoved.test.js @@ -2,6 +2,7 @@ import React, { useState } from 'react'; import { View, Text, TouchableOpacity } from 'react-native'; import { render, fireEvent, waitForElementToBeRemoved } from '..'; +import { FakeTimerTypes, setupFakeTimers } from './timerUtils'; const TestSetup = ({ shouldUseDelay = true }) => { const [isAdded, setIsAdded] = useState(true); @@ -130,30 +131,21 @@ test('waits with custom interval', async () => { expect(mockFn).toHaveBeenCalledTimes(4); }); -test('works with legacy fake timers', async () => { - jest.useFakeTimers('legacy'); +test.each(FakeTimerTypes)( + 'works with %s fake timers', + async (fakeTimerType) => { + setupFakeTimers(fakeTimerType); - const mockFn = jest.fn(() => ); - - waitForElementToBeRemoved(() => mockFn(), { - timeout: 400, - interval: 200, - }); + const mockFn = jest.fn(() => ); - jest.advanceTimersByTime(400); - expect(mockFn).toHaveBeenCalledTimes(4); -}); - -test('works with fake timers', async () => { - jest.useFakeTimers('modern'); - - const mockFn = jest.fn(() => ); - - waitForElementToBeRemoved(() => mockFn(), { - timeout: 400, - interval: 200, - }); - - jest.advanceTimersByTime(400); - expect(mockFn).toHaveBeenCalledTimes(4); -}); + try { + await waitForElementToBeRemoved(() => mockFn(), { + timeout: 400, + interval: 200, + }); + } catch (e) { + // Suppress expected error + } + expect(mockFn).toHaveBeenCalledTimes(4); + } +); diff --git a/src/helpers/getTimerFuncs.js b/src/helpers/getTimerFuncs.js new file mode 100644 index 000000000..d881d049b --- /dev/null +++ b/src/helpers/getTimerFuncs.js @@ -0,0 +1,77 @@ +/* eslint-disable no-undef */ + +// Contents of this file sourced directly from https://github.com/testing-library/dom-testing-library/blob/master/src/helpers.js + +const globalObj = typeof window === 'undefined' ? global : window; + +// Currently this fn only supports jest timers, but it could support other test runners in the future. +function runWithRealTimers(callback) { + const fakeTimersType = getJestFakeTimersType(); + if (fakeTimersType) { + jest.useRealTimers(); + } + + const callbackReturnValue = callback(); + + if (fakeTimersType) { + jest.useFakeTimers(fakeTimersType); + } + + return callbackReturnValue; +} + +function getJestFakeTimersType() { + // istanbul ignore if + if ( + typeof jest === 'undefined' || + typeof globalObj.setTimeout === 'undefined' + ) { + return null; + } + + if ( + typeof globalObj.setTimeout._isMockFunction !== 'undefined' && + globalObj.setTimeout._isMockFunction + ) { + return 'legacy'; + } + + if ( + typeof globalObj.setTimeout.clock !== 'undefined' && + typeof jest.getRealSystemTime !== 'undefined' + ) { + try { + // jest.getRealSystemTime is only supported for Jest's `modern` fake timers and otherwise throws + jest.getRealSystemTime(); + return 'modern'; + } catch { + // not using Jest's modern fake timers + } + } + return null; +} + +// we only run our tests in node, and setImmediate is supported in node. +// istanbul ignore next +function setImmediatePolyfill(fn) { + return globalObj.setTimeout(fn, 0); +} + +function getTimeFunctions() { + // istanbul ignore next + return { + clearTimeoutFn: globalObj.clearTimeout, + setImmediateFn: globalObj.setImmediate || setImmediatePolyfill, + setTimeoutFn: globalObj.setTimeout, + }; +} + +const { clearTimeoutFn, setImmediateFn, setTimeoutFn } = runWithRealTimers( + getTimeFunctions +); + +export { + clearTimeoutFn as clearTimeout, + setImmediateFn as setImmediate, + setTimeoutFn as setTimeout, +}; diff --git a/src/waitFor.js b/src/waitFor.js index 45c929771..ab65c4b1d 100644 --- a/src/waitFor.js +++ b/src/waitFor.js @@ -7,6 +7,7 @@ import { throwRemovedFunctionError, copyStackTrace, } from './helpers/errors'; +import { setTimeout } from './helpers/getTimerFuncs'; const DEFAULT_TIMEOUT = 4500; const DEFAULT_INTERVAL = 50; @@ -28,14 +29,17 @@ function waitForInternal( options?: WaitForOptions ): Promise { const timeout = options?.timeout ?? DEFAULT_TIMEOUT; - const interval = options?.interval ?? DEFAULT_INTERVAL; - const startTime = Date.now(); + let interval = options?.interval ?? DEFAULT_INTERVAL; // Being able to display a useful stack trace requires generating it before doing anything async const stackTraceError = new ErrorWithStack('STACK_TRACE_ERROR', waitFor); + if (interval < 1) interval = 1; + const maxTries = Math.ceil(timeout / interval); + let tries = 0; + return new Promise((resolve, reject) => { const rejectOrRerun = (error) => { - if (Date.now() - startTime >= timeout) { + if (tries > maxTries) { copyStackTrace(error, stackTraceError); reject(error); return; @@ -43,8 +47,9 @@ function waitForInternal( setTimeout(runExpectation, interval); }; function runExpectation() { + tries += 1; try { - const result = expectation(); + const result = Promise.resolve(expectation()); resolve(result); } catch (error) { rejectOrRerun(error); From b88aa2bc78e8ce91fe8ba74ad2c03daa159955e1 Mon Sep 17 00:00:00 2001 From: Michael James Duminy Date: Thu, 8 Oct 2020 09:19:20 +0200 Subject: [PATCH 02/12] use real timers in flushMicroTasks --- src/flushMicroTasks.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/flushMicroTasks.js b/src/flushMicroTasks.js index 5a5a0153d..ff36a44cd 100644 --- a/src/flushMicroTasks.js +++ b/src/flushMicroTasks.js @@ -1,5 +1,6 @@ // @flow import { printDeprecationWarning } from './helpers/errors'; +import { setImmediate } from './helpers/getTimerFuncs'; type Thenable = { then: (() => T) => mixed }; From 08351f7f6bfe8734d35ce8cd693e3da5eab98f1f Mon Sep 17 00:00:00 2001 From: Michael James Duminy Date: Thu, 8 Oct 2020 10:45:20 +0200 Subject: [PATCH 03/12] test to show changes with modern timer mocks --- src/__tests__/timerUtils.js | 8 +++++++- src/__tests__/waitFor.test.js | 33 ++++++++++++++++++++++++++++++--- src/helpers/getTimerFuncs.js | 3 --- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/__tests__/timerUtils.js b/src/__tests__/timerUtils.js index 649a8cf42..534e298f8 100644 --- a/src/__tests__/timerUtils.js +++ b/src/__tests__/timerUtils.js @@ -1,3 +1,5 @@ +import { setTimeout } from '../helpers/getTimerFuncs'; + const FakeTimerTypes = [ 'default', 'legacy', @@ -18,4 +20,8 @@ function setupFakeTimers(fakeTimerType) { } } -export { FakeTimerTypes, setupFakeTimers }; +async function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +export { FakeTimerTypes, setupFakeTimers, sleep }; diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js index c9c463026..92f1fa1a6 100644 --- a/src/__tests__/waitFor.test.js +++ b/src/__tests__/waitFor.test.js @@ -1,8 +1,8 @@ // @flow import * as React from 'react'; -import { View, Text, TouchableOpacity } from 'react-native'; -import { render, fireEvent, waitFor } from '..'; -import { FakeTimerTypes, setupFakeTimers } from './timerUtils'; +import { Text, TouchableOpacity, View } from 'react-native'; +import { fireEvent, render, waitFor } from '..'; +import { FakeTimerTypes, setupFakeTimers, sleep } from './timerUtils'; class Banana extends React.Component { changeFresh = () => { @@ -137,3 +137,30 @@ test.each(FakeTimerTypes)( expect(mockFn).toHaveBeenCalledTimes(3); } ); + +// this test leads to a console error: Warning: You called act(async () => ...) without await, but does not fail the test +// it is included to show that the previous approach of faking modern timers still works +// the gotcha is that the try catch will fail to catch the final error, which is why we need to stop throwing +test('non-awaited approach is not affected by fake modern timers', async () => { + jest.useFakeTimers('modern'); + + let calls = 0; + const mockFn = jest.fn(() => { + calls += 1; + if (calls < 3) { + throw Error('test'); + } + }); + + try { + waitFor(() => mockFn(), { timeout: 400, interval: 200 }); + } catch (e) { + // suppress + } + jest.advanceTimersByTime(400); + expect(mockFn).toHaveBeenCalledTimes(0); + + jest.useRealTimers(); + await sleep(500); + expect(mockFn).toHaveBeenCalledTimes(3); +}); diff --git a/src/helpers/getTimerFuncs.js b/src/helpers/getTimerFuncs.js index d881d049b..6f5f1317a 100644 --- a/src/helpers/getTimerFuncs.js +++ b/src/helpers/getTimerFuncs.js @@ -21,7 +21,6 @@ function runWithRealTimers(callback) { } function getJestFakeTimersType() { - // istanbul ignore if if ( typeof jest === 'undefined' || typeof globalObj.setTimeout === 'undefined' @@ -52,13 +51,11 @@ function getJestFakeTimersType() { } // we only run our tests in node, and setImmediate is supported in node. -// istanbul ignore next function setImmediatePolyfill(fn) { return globalObj.setTimeout(fn, 0); } function getTimeFunctions() { - // istanbul ignore next return { clearTimeoutFn: globalObj.clearTimeout, setImmediateFn: globalObj.setImmediate || setImmediatePolyfill, From f45177edc1324f844c49d9097a0473568094f932 Mon Sep 17 00:00:00 2001 From: Michael James Duminy Date: Sun, 25 Oct 2020 19:13:49 +0100 Subject: [PATCH 04/12] use enum-like object for timer modes --- src/__tests__/timerUtils.js | 18 +++++++++--------- src/__tests__/waitFor.test.js | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/__tests__/timerUtils.js b/src/__tests__/timerUtils.js index 534e298f8..cc1a2466d 100644 --- a/src/__tests__/timerUtils.js +++ b/src/__tests__/timerUtils.js @@ -1,19 +1,19 @@ import { setTimeout } from '../helpers/getTimerFuncs'; -const FakeTimerTypes = [ - 'default', - 'legacy', - // 'modern', // broken for now -]; +const TimerMode = { + Default: 'default', + Legacy: 'legacy', + Modern: 'modern', // broken for now +}; function setupFakeTimers(fakeTimerType) { switch (fakeTimerType) { - case 'legacy': - case 'modern': { + case TimerMode.Legacy: + case TimerMode.Modern: { jest.useFakeTimers(fakeTimerType); break; } - case 'default': + case TimerMode.Default: default: { jest.useFakeTimers(); } @@ -24,4 +24,4 @@ async function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } -export { FakeTimerTypes, setupFakeTimers, sleep }; +export { TimerMode, setupFakeTimers, sleep }; diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js index 92f1fa1a6..fc3ea7e99 100644 --- a/src/__tests__/waitFor.test.js +++ b/src/__tests__/waitFor.test.js @@ -2,7 +2,7 @@ import * as React from 'react'; import { Text, TouchableOpacity, View } from 'react-native'; import { fireEvent, render, waitFor } from '..'; -import { FakeTimerTypes, setupFakeTimers, sleep } from './timerUtils'; +import { TimerMode, setupFakeTimers, sleep } from './timerUtils'; class Banana extends React.Component { changeFresh = () => { @@ -80,7 +80,7 @@ test('waits for element with custom interval', async () => { expect(mockFn).toHaveBeenCalledTimes(3); }); -test.each(FakeTimerTypes)( +test.each([TimerMode.Default, TimerMode.Legacy])( 'waits for element until it stops throwing using %s fake timers', async (fakeTimerType) => { setupFakeTimers(fakeTimerType); @@ -96,7 +96,7 @@ test.each(FakeTimerTypes)( } ); -test.each(FakeTimerTypes)( +test.each([TimerMode.Default, TimerMode.Legacy])( 'waits for assertion until timeout is met with %s fake timers', async (fakeTimerType) => { setupFakeTimers(fakeTimerType); @@ -115,7 +115,7 @@ test.each(FakeTimerTypes)( } ); -test.each(FakeTimerTypes)( +test.each([TimerMode.Default, TimerMode.Legacy])( 'awaiting something that succeeds before timeout works with %s fake timers', async (fakeTimerType) => { setupFakeTimers(fakeTimerType); @@ -142,7 +142,7 @@ test.each(FakeTimerTypes)( // it is included to show that the previous approach of faking modern timers still works // the gotcha is that the try catch will fail to catch the final error, which is why we need to stop throwing test('non-awaited approach is not affected by fake modern timers', async () => { - jest.useFakeTimers('modern'); + setupFakeTimers(TimerMode.Modern); let calls = 0; const mockFn = jest.fn(() => { From c4330201a2fe532e22d7a831dfced5bdfe6fcb72 Mon Sep 17 00:00:00 2001 From: Michael James Duminy Date: Sun, 25 Oct 2020 20:18:28 +0100 Subject: [PATCH 05/12] fix tests --- src/__tests__/timers.test.js | 35 ++++++++++--------- .../waitForElementToBeRemoved.test.js | 4 +-- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/__tests__/timers.test.js b/src/__tests__/timers.test.js index ea0844703..d8946adef 100644 --- a/src/__tests__/timers.test.js +++ b/src/__tests__/timers.test.js @@ -1,23 +1,26 @@ import waitFor from '../waitFor'; -import { FakeTimerTypes, setupFakeTimers } from './timerUtils'; +import { TimerMode, setupFakeTimers } from './timerUtils'; -describe.each(FakeTimerTypes)('%s fake timers tests', (fakeTimerType) => { - beforeEach(() => setupFakeTimers(fakeTimerType)); +describe.each([TimerMode.Default, TimerMode.Legacy])( + '%s fake timers tests', + (fakeTimerType) => { + beforeEach(() => setupFakeTimers(fakeTimerType)); - test('it successfully runs tests', () => { - expect(true).toBeTruthy(); - }); - - test('it successfully uses waitFor', async () => { - await waitFor(() => { + test('it successfully runs tests', () => { expect(true).toBeTruthy(); }); - }); - test('it successfully uses waitFor with real timers', async () => { - jest.useRealTimers(); - await waitFor(() => { - expect(true).toBeTruthy(); + test('it successfully uses waitFor', async () => { + await waitFor(() => { + expect(true).toBeTruthy(); + }); + }); + + test('it successfully uses waitFor with real timers', async () => { + jest.useRealTimers(); + await waitFor(() => { + expect(true).toBeTruthy(); + }); }); - }); -}); + } +); diff --git a/src/__tests__/waitForElementToBeRemoved.test.js b/src/__tests__/waitForElementToBeRemoved.test.js index 22c54d6fa..705e7ad9f 100644 --- a/src/__tests__/waitForElementToBeRemoved.test.js +++ b/src/__tests__/waitForElementToBeRemoved.test.js @@ -2,7 +2,7 @@ import React, { useState } from 'react'; import { View, Text, TouchableOpacity } from 'react-native'; import { render, fireEvent, waitForElementToBeRemoved } from '..'; -import { FakeTimerTypes, setupFakeTimers } from './timerUtils'; +import { TimerMode, setupFakeTimers } from './timerUtils'; const TestSetup = ({ shouldUseDelay = true }) => { const [isAdded, setIsAdded] = useState(true); @@ -131,7 +131,7 @@ test('waits with custom interval', async () => { expect(mockFn).toHaveBeenCalledTimes(4); }); -test.each(FakeTimerTypes)( +test.each([TimerMode.Default, TimerMode.Legacy])( 'works with %s fake timers', async (fakeTimerType) => { setupFakeTimers(fakeTimerType); From 1928ec5a3f08c9ea7e9c32c135028fd359883006 Mon Sep 17 00:00:00 2001 From: Michael James Duminy Date: Thu, 21 Jan 2021 01:27:44 +0200 Subject: [PATCH 06/12] remove default timer mode, add flow types export other timer helpers, adjust tests and docs --- src/__tests__/timerUtils.js | 23 +++------- src/__tests__/timers.test.js | 8 ++-- src/__tests__/waitFor.test.js | 43 ++++--------------- .../waitForElementToBeRemoved.test.js | 10 ++--- src/flushMicroTasks.js | 2 +- src/helpers/{getTimerFuncs.js => timers.js} | 30 +++++++++---- website/docs/API.md | 4 -- 7 files changed, 46 insertions(+), 74 deletions(-) rename src/helpers/{getTimerFuncs.js => timers.js} (67%) diff --git a/src/__tests__/timerUtils.js b/src/__tests__/timerUtils.js index cc1a2466d..5132745d1 100644 --- a/src/__tests__/timerUtils.js +++ b/src/__tests__/timerUtils.js @@ -1,27 +1,14 @@ -import { setTimeout } from '../helpers/getTimerFuncs'; +// @flow + +import { setTimeout } from '../helpers/timers'; const TimerMode = { - Default: 'default', Legacy: 'legacy', Modern: 'modern', // broken for now }; -function setupFakeTimers(fakeTimerType) { - switch (fakeTimerType) { - case TimerMode.Legacy: - case TimerMode.Modern: { - jest.useFakeTimers(fakeTimerType); - break; - } - case TimerMode.Default: - default: { - jest.useFakeTimers(); - } - } -} - -async function sleep(ms) { +async function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } -export { TimerMode, setupFakeTimers, sleep }; +export { TimerMode, sleep }; diff --git a/src/__tests__/timers.test.js b/src/__tests__/timers.test.js index d8946adef..f1ef62eb1 100644 --- a/src/__tests__/timers.test.js +++ b/src/__tests__/timers.test.js @@ -1,10 +1,12 @@ import waitFor from '../waitFor'; -import { TimerMode, setupFakeTimers } from './timerUtils'; +import { TimerMode } from './timerUtils'; -describe.each([TimerMode.Default, TimerMode.Legacy])( +describe.each([TimerMode.Legacy, TimerMode.Modern])( '%s fake timers tests', (fakeTimerType) => { - beforeEach(() => setupFakeTimers(fakeTimerType)); + beforeEach(() => { + jest.useFakeTimers(fakeTimerType); + }); test('it successfully runs tests', () => { expect(true).toBeTruthy(); diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js index fc3ea7e99..75e3dd5af 100644 --- a/src/__tests__/waitFor.test.js +++ b/src/__tests__/waitFor.test.js @@ -2,7 +2,7 @@ import * as React from 'react'; import { Text, TouchableOpacity, View } from 'react-native'; import { fireEvent, render, waitFor } from '..'; -import { TimerMode, setupFakeTimers, sleep } from './timerUtils'; +import { TimerMode } from './timerUtils'; class Banana extends React.Component { changeFresh = () => { @@ -77,13 +77,13 @@ test('waits for element with custom interval', async () => { // suppress } - expect(mockFn).toHaveBeenCalledTimes(3); + expect(mockFn).toHaveBeenCalledTimes(2); }); -test.each([TimerMode.Default, TimerMode.Legacy])( +test.each([TimerMode.Legacy, TimerMode.Modern])( 'waits for element until it stops throwing using %s fake timers', async (fakeTimerType) => { - setupFakeTimers(fakeTimerType); + jest.useFakeTimers(fakeTimerType); const { getByText, queryByText } = render(); fireEvent.press(getByText('Change freshness!')); @@ -96,10 +96,10 @@ test.each([TimerMode.Default, TimerMode.Legacy])( } ); -test.each([TimerMode.Default, TimerMode.Legacy])( +test.each([TimerMode.Legacy, TimerMode.Modern])( 'waits for assertion until timeout is met with %s fake timers', async (fakeTimerType) => { - setupFakeTimers(fakeTimerType); + jest.useFakeTimers(fakeTimerType); const mockFn = jest.fn(() => { throw Error('test'); @@ -115,10 +115,10 @@ test.each([TimerMode.Default, TimerMode.Legacy])( } ); -test.each([TimerMode.Default, TimerMode.Legacy])( +test.each([TimerMode.Legacy, TimerMode.Legacy])( 'awaiting something that succeeds before timeout works with %s fake timers', async (fakeTimerType) => { - setupFakeTimers(fakeTimerType); + jest.useFakeTimers(fakeTimerType); let calls = 0; const mockFn = jest.fn(() => { @@ -137,30 +137,3 @@ test.each([TimerMode.Default, TimerMode.Legacy])( expect(mockFn).toHaveBeenCalledTimes(3); } ); - -// this test leads to a console error: Warning: You called act(async () => ...) without await, but does not fail the test -// it is included to show that the previous approach of faking modern timers still works -// the gotcha is that the try catch will fail to catch the final error, which is why we need to stop throwing -test('non-awaited approach is not affected by fake modern timers', async () => { - setupFakeTimers(TimerMode.Modern); - - let calls = 0; - const mockFn = jest.fn(() => { - calls += 1; - if (calls < 3) { - throw Error('test'); - } - }); - - try { - waitFor(() => mockFn(), { timeout: 400, interval: 200 }); - } catch (e) { - // suppress - } - jest.advanceTimersByTime(400); - expect(mockFn).toHaveBeenCalledTimes(0); - - jest.useRealTimers(); - await sleep(500); - expect(mockFn).toHaveBeenCalledTimes(3); -}); diff --git a/src/__tests__/waitForElementToBeRemoved.test.js b/src/__tests__/waitForElementToBeRemoved.test.js index 705e7ad9f..73c2e4e8e 100644 --- a/src/__tests__/waitForElementToBeRemoved.test.js +++ b/src/__tests__/waitForElementToBeRemoved.test.js @@ -2,7 +2,7 @@ import React, { useState } from 'react'; import { View, Text, TouchableOpacity } from 'react-native'; import { render, fireEvent, waitForElementToBeRemoved } from '..'; -import { TimerMode, setupFakeTimers } from './timerUtils'; +import { TimerMode } from './timerUtils'; const TestSetup = ({ shouldUseDelay = true }) => { const [isAdded, setIsAdded] = useState(true); @@ -121,7 +121,7 @@ test('waits with custom interval', async () => { try { await waitForElementToBeRemoved(() => mockFn(), { - timeout: 400, + timeout: 600, interval: 200, }); } catch (e) { @@ -131,10 +131,10 @@ test('waits with custom interval', async () => { expect(mockFn).toHaveBeenCalledTimes(4); }); -test.each([TimerMode.Default, TimerMode.Legacy])( +test.each([TimerMode.Legacy, TimerMode.Modern])( 'works with %s fake timers', async (fakeTimerType) => { - setupFakeTimers(fakeTimerType); + jest.useFakeTimers(fakeTimerType); const mockFn = jest.fn(() => ); @@ -146,6 +146,6 @@ test.each([TimerMode.Default, TimerMode.Legacy])( } catch (e) { // Suppress expected error } - expect(mockFn).toHaveBeenCalledTimes(4); + expect(mockFn).toHaveBeenCalledTimes(2); } ); diff --git a/src/flushMicroTasks.js b/src/flushMicroTasks.js index ff36a44cd..666cbc088 100644 --- a/src/flushMicroTasks.js +++ b/src/flushMicroTasks.js @@ -1,6 +1,6 @@ // @flow import { printDeprecationWarning } from './helpers/errors'; -import { setImmediate } from './helpers/getTimerFuncs'; +import { setImmediate } from './helpers/timers'; type Thenable = { then: (() => T) => mixed }; diff --git a/src/helpers/getTimerFuncs.js b/src/helpers/timers.js similarity index 67% rename from src/helpers/getTimerFuncs.js rename to src/helpers/timers.js index 6f5f1317a..bc9080bde 100644 --- a/src/helpers/getTimerFuncs.js +++ b/src/helpers/timers.js @@ -1,11 +1,11 @@ -/* eslint-disable no-undef */ - -// Contents of this file sourced directly from https://github.com/testing-library/dom-testing-library/blob/master/src/helpers.js +// Most content of this file sourced directly from https://github.com/testing-library/dom-testing-library/blob/master/src/helpers.js +// @flow +/* globals jest */ const globalObj = typeof window === 'undefined' ? global : window; // Currently this fn only supports jest timers, but it could support other test runners in the future. -function runWithRealTimers(callback) { +function runWithRealTimers(callback: () => T): T { const fakeTimersType = getJestFakeTimersType(); if (fakeTimersType) { jest.useRealTimers(); @@ -21,6 +21,7 @@ function runWithRealTimers(callback) { } function getJestFakeTimersType() { + // istanbul ignore if if ( typeof jest === 'undefined' || typeof globalObj.setTimeout === 'undefined' @@ -37,10 +38,12 @@ function getJestFakeTimersType() { if ( typeof globalObj.setTimeout.clock !== 'undefined' && + // $FlowIgnore[prop-missing] typeof jest.getRealSystemTime !== 'undefined' ) { try { // jest.getRealSystemTime is only supported for Jest's `modern` fake timers and otherwise throws + // $FlowExpectedError jest.getRealSystemTime(); return 'modern'; } catch { @@ -50,12 +53,21 @@ function getJestFakeTimersType() { return null; } +const jestFakeTimersAreEnabled = (): boolean => + Boolean(getJestFakeTimersType()); + // we only run our tests in node, and setImmediate is supported in node. function setImmediatePolyfill(fn) { return globalObj.setTimeout(fn, 0); } -function getTimeFunctions() { +type BindTimeFunctions = { + clearTimeoutFn: typeof clearTimeout, + setImmediateFn: typeof setImmediate, + setTimeoutFn: typeof setTimeout, +}; + +function bindTimeFunctions(): BindTimeFunctions { return { clearTimeoutFn: globalObj.clearTimeout, setImmediateFn: globalObj.setImmediate || setImmediatePolyfill, @@ -63,11 +75,13 @@ function getTimeFunctions() { }; } -const { clearTimeoutFn, setImmediateFn, setTimeoutFn } = runWithRealTimers( - getTimeFunctions -); +const { clearTimeoutFn, setImmediateFn, setTimeoutFn } = (runWithRealTimers( + bindTimeFunctions +): BindTimeFunctions); export { + runWithRealTimers, + jestFakeTimersAreEnabled, clearTimeoutFn as clearTimeout, setImmediateFn as setImmediate, setTimeoutFn as setTimeout, diff --git a/website/docs/API.md b/website/docs/API.md index 96cd9ed0d..c2d57f1a7 100644 --- a/website/docs/API.md +++ b/website/docs/API.md @@ -366,8 +366,6 @@ test('waiting for an Banana to be ready', async () => { In order to properly use `waitFor` you need at least React >=16.9.0 (featuring async `act`) or React Native >=0.60 (which comes with React >=16.9.0). ::: -If you're using Jest's [Timer Mocks](https://jestjs.io/docs/en/timer-mocks#docsNav), remember not to await the return of `waitFor` as it will stall your tests. - ## `waitForElementToBeRemoved` - [`Example code`](https://github.com/callstack/react-native-testing-library/blob/master/src/__tests__/waitForElementToBeRemoved.test.js) @@ -404,8 +402,6 @@ You can use any of `getBy`, `getAllBy`, `queryBy` and `queryAllBy` queries for ` In order to properly use `waitForElementToBeRemoved` you need at least React >=16.9.0 (featuring async `act`) or React Native >=0.60 (which comes with React >=16.9.0). ::: -If you're using Jest's [Timer Mocks](https://jestjs.io/docs/en/timer-mocks#docsNav), remember not to await the return of `waitFor` as it will stall your tests. - ## `within`, `getQueriesForElement` - [`Example code`](https://github.com/callstack/react-native-testing-library/blob/master/src/__tests__/within.test.js) From 90b611593bc24298536e02656d206ee2579d61e5 Mon Sep 17 00:00:00 2001 From: Michael James Duminy Date: Thu, 21 Jan 2021 01:28:15 +0200 Subject: [PATCH 07/12] base waitFor on RTL implementation --- src/waitFor.js | 169 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 144 insertions(+), 25 deletions(-) diff --git a/src/waitFor.js b/src/waitFor.js index ab65c4b1d..7313def92 100644 --- a/src/waitFor.js +++ b/src/waitFor.js @@ -1,4 +1,5 @@ // @flow +/* globals jest */ import * as React from 'react'; import act from './act'; @@ -7,9 +8,14 @@ import { throwRemovedFunctionError, copyStackTrace, } from './helpers/errors'; -import { setTimeout } from './helpers/getTimerFuncs'; +import { + setTimeout, + clearTimeout, + setImmediate, + jestFakeTimersAreEnabled, +} from './helpers/timers'; -const DEFAULT_TIMEOUT = 4500; +const DEFAULT_TIMEOUT = 1000; const DEFAULT_INTERVAL = 50; function checkReactVersionAtLeast(major: number, minor: number): boolean { @@ -22,40 +28,149 @@ function checkReactVersionAtLeast(major: number, minor: number): boolean { export type WaitForOptions = { timeout?: number, interval?: number, + stackTraceError?: ErrorWithStack, }; function waitForInternal( expectation: () => T, - options?: WaitForOptions + { + timeout = DEFAULT_TIMEOUT, + interval = DEFAULT_INTERVAL, + stackTraceError, + }: WaitForOptions ): Promise { - const timeout = options?.timeout ?? DEFAULT_TIMEOUT; - let interval = options?.interval ?? DEFAULT_INTERVAL; - // Being able to display a useful stack trace requires generating it before doing anything async - const stackTraceError = new ErrorWithStack('STACK_TRACE_ERROR', waitFor); + if (typeof expectation !== 'function') { + throw new TypeError('Received `expectation` arg must be a function'); + } + + // eslint-disable-next-line no-async-promise-executor + return new Promise(async (resolve, reject) => { + let lastError, intervalId; + let finished = false; + let promiseStatus = 'idle'; + + const overallTimeoutTimer = setTimeout(handleTimeout, timeout); - if (interval < 1) interval = 1; - const maxTries = Math.ceil(timeout / interval); - let tries = 0; + const usingFakeTimers = jestFakeTimersAreEnabled(); + if (usingFakeTimers) { + checkExpectation(); + // this is a dangerous rule to disable because it could lead to an + // infinite loop. However, eslint isn't smart enough to know that we're + // setting finished inside `onDone` which will be called when we're done + // waiting or when we've timed out. + // eslint-disable-next-line no-unmodified-loop-condition + while (!finished) { + if (!jestFakeTimersAreEnabled()) { + const error = new Error( + `Changed from using fake timers to real timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to real timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830` + ); + if (stackTraceError) { + copyStackTrace(error, stackTraceError); + } + reject(error); + return; + } + // we *could* (maybe should?) use `advanceTimersToNextTimer` but it's + // possible that could make this loop go on forever if someone is using + // third party code that's setting up recursive timers so rapidly that + // the user's timer's don't get a chance to resolve. So we'll advance + // by an interval instead. (We have a test for this case). + jest.advanceTimersByTime(interval); + + // It's really important that checkExpectation is run *before* we flush + // in-flight promises. To be honest, I'm not sure why, and I can't quite + // think of a way to reproduce the problem in a test, but I spent + // an entire day banging my head against a wall on this. + checkExpectation(); + + // In this rare case, we *need* to wait for in-flight promises + // to resolve before continuing. We don't need to take advantage + // of parallelization so we're fine. + // https://stackoverflow.com/a/59243586/971592 + // eslint-disable-next-line no-await-in-loop + await new Promise((resolve) => setImmediate(resolve)); + } + } else { + intervalId = setInterval(checkRealTimersCallback, interval); + checkExpectation(); + } + + function onDone(error, result) { + finished = true; + clearTimeout(overallTimeoutTimer); + + if (!usingFakeTimers) { + clearInterval(intervalId); + } - return new Promise((resolve, reject) => { - const rejectOrRerun = (error) => { - if (tries > maxTries) { - copyStackTrace(error, stackTraceError); + if (error) { reject(error); - return; + } else { + // $FlowIgnore[incompatible-return] error and result are mutually exclusive + resolve(result); + } + } + + function checkRealTimersCallback() { + if (jestFakeTimersAreEnabled()) { + const error = new Error( + `Changed from using real timers to fake timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to fake timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830` + ); + if (stackTraceError) { + copyStackTrace(error, stackTraceError); + } + return reject(error); + } else { + return checkExpectation(); } - setTimeout(runExpectation, interval); - }; - function runExpectation() { - tries += 1; + } + + function checkExpectation() { + if (promiseStatus === 'pending') return; try { - const result = Promise.resolve(expectation()); - resolve(result); + const result = expectation(); + + // $FlowIgnore[incompatible-type] + if (typeof result?.then === 'function') { + promiseStatus = 'pending'; + // eslint-disable-next-line promise/catch-or-return + result.then( + (resolvedValue) => { + promiseStatus = 'resolved'; + onDone(null, resolvedValue); + return; + }, + (rejectedValue) => { + promiseStatus = 'rejected'; + lastError = rejectedValue; + return; + } + ); + } else { + onDone(null, result); + } + // If `callback` throws, wait for the next mutation, interval, or timeout. } catch (error) { - rejectOrRerun(error); + // Save the most recent callback error to reject the promise with it in the event of a timeout + lastError = error; + } + } + + function handleTimeout() { + let error; + if (lastError) { + error = lastError; + if (stackTraceError) { + copyStackTrace(error, stackTraceError); + } + } else { + error = new Error('Timed out in waitFor.'); + if (stackTraceError) { + copyStackTrace(error, stackTraceError); + } } + onDone(error, null); } - setTimeout(runExpectation, 0); }); } @@ -63,15 +178,19 @@ export default async function waitFor( expectation: () => T, options?: WaitForOptions ): Promise { + // Being able to display a useful stack trace requires generating it before doing anything async + const stackTraceError = new ErrorWithStack('STACK_TRACE_ERROR', waitFor); + const optionsWithStackTrace = { stackTraceError, ...options }; + if (!checkReactVersionAtLeast(16, 9)) { - return waitForInternal(expectation, options); + return waitForInternal(expectation, optionsWithStackTrace); } let result: T; //$FlowFixMe: `act` has incorrect flow typing await act(async () => { - result = await waitForInternal(expectation, options); + result = await waitForInternal(expectation, optionsWithStackTrace); }); //$FlowFixMe: either we have result or `waitFor` threw error From ba2d439ac6cce982b9400bed54308f7892d0c127 Mon Sep 17 00:00:00 2001 From: Mike Duminy Date: Mon, 8 Feb 2021 12:47:32 +0100 Subject: [PATCH 08/12] [jest] Capture and restore global promise --- jest/preset.js | 9 +++++++++ jest/restore-promise.js | 1 + jest/save-promise.js | 1 + package.json | 2 +- 4 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 jest/preset.js create mode 100644 jest/restore-promise.js create mode 100644 jest/save-promise.js diff --git a/jest/preset.js b/jest/preset.js new file mode 100644 index 000000000..f3755d462 --- /dev/null +++ b/jest/preset.js @@ -0,0 +1,9 @@ +const reactNativePreset = require('react-native/jest-preset'); + +module.exports = Object.assign({}, reactNativePreset, { + // this is needed to make modern fake timers work + // because the react-native preset overrides global.Promise + setupFiles: [require.resolve('./save-promise.js')] + .concat(reactNativePreset.setupFiles) + .concat([require.resolve('./restore-promise.js')]), +}); diff --git a/jest/restore-promise.js b/jest/restore-promise.js new file mode 100644 index 000000000..a064a83e3 --- /dev/null +++ b/jest/restore-promise.js @@ -0,0 +1 @@ +global.Promise = global.originalPromise; \ No newline at end of file diff --git a/jest/save-promise.js b/jest/save-promise.js new file mode 100644 index 000000000..8e0f6d840 --- /dev/null +++ b/jest/save-promise.js @@ -0,0 +1 @@ +global.originalPromise = Promise; \ No newline at end of file diff --git a/package.json b/package.json index 49850a848..66cb8e379 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,7 @@ "build": "rm -rf build; babel src --out-dir build --ignore 'src/__tests__/*'" }, "jest": { - "preset": "react-native", + "preset": "../jest/preset.js", "moduleFileExtensions": [ "js", "json" From 84280fb49336beaa5d238a4536192ce8b7b94012 Mon Sep 17 00:00:00 2001 From: Mike Duminy Date: Mon, 8 Feb 2021 12:48:34 +0100 Subject: [PATCH 09/12] [waitFor] Simulate intervals when using fake timers --- src/waitFor.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/waitFor.js b/src/waitFor.js index 7313def92..79285c357 100644 --- a/src/waitFor.js +++ b/src/waitFor.js @@ -52,6 +52,7 @@ function waitForInternal( const overallTimeoutTimer = setTimeout(handleTimeout, timeout); const usingFakeTimers = jestFakeTimersAreEnabled(); + if (usingFakeTimers) { checkExpectation(); // this is a dangerous rule to disable because it could lead to an @@ -59,6 +60,7 @@ function waitForInternal( // setting finished inside `onDone` which will be called when we're done // waiting or when we've timed out. // eslint-disable-next-line no-unmodified-loop-condition + let fakeTimeRemaining = timeout; while (!finished) { if (!jestFakeTimersAreEnabled()) { const error = new Error( @@ -70,6 +72,14 @@ function waitForInternal( reject(error); return; } + + // when fake timers are used we want to simulate the interval time passing + if (fakeTimeRemaining <= 0) { + return; + } else { + fakeTimeRemaining -= interval; + } + // we *could* (maybe should?) use `advanceTimersToNextTimer` but it's // possible that could make this loop go on forever if someone is using // third party code that's setting up recursive timers so rapidly that From 508603856fe02f068fd9f36c0530fec79005103c Mon Sep 17 00:00:00 2001 From: Mike Duminy Date: Mon, 8 Feb 2021 12:49:13 +0100 Subject: [PATCH 10/12] [tests] Fix waitForElementToBeRemoved --- src/__tests__/waitForElementToBeRemoved.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/__tests__/waitForElementToBeRemoved.test.js b/src/__tests__/waitForElementToBeRemoved.test.js index 73c2e4e8e..7c56936b0 100644 --- a/src/__tests__/waitForElementToBeRemoved.test.js +++ b/src/__tests__/waitForElementToBeRemoved.test.js @@ -146,6 +146,8 @@ test.each([TimerMode.Legacy, TimerMode.Modern])( } catch (e) { // Suppress expected error } - expect(mockFn).toHaveBeenCalledTimes(2); + + // waitForElementToBeRemoved runs an initial call of the expectation + expect(mockFn).toHaveBeenCalledTimes(4); } ); From 156794252590c29c10474bcad20d038ef060e442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Mar 2021 12:47:36 +0100 Subject: [PATCH 11/12] cleanups; use more specific global --- jest/preset.js | 5 +++-- jest/restore-promise.js | 2 +- jest/save-promise.js | 2 +- src/__tests__/timers.test.js | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/jest/preset.js b/jest/preset.js index f3755d462..40fe80597 100644 --- a/jest/preset.js +++ b/jest/preset.js @@ -1,9 +1,10 @@ const reactNativePreset = require('react-native/jest-preset'); -module.exports = Object.assign({}, reactNativePreset, { +module.exports = { + ...reactNativePreset, // this is needed to make modern fake timers work // because the react-native preset overrides global.Promise setupFiles: [require.resolve('./save-promise.js')] .concat(reactNativePreset.setupFiles) .concat([require.resolve('./restore-promise.js')]), -}); +}; diff --git a/jest/restore-promise.js b/jest/restore-promise.js index a064a83e3..75bf16f96 100644 --- a/jest/restore-promise.js +++ b/jest/restore-promise.js @@ -1 +1 @@ -global.Promise = global.originalPromise; \ No newline at end of file +global.Promise = global.$RNTL_ORIGINAL_PROMISE; diff --git a/jest/save-promise.js b/jest/save-promise.js index 8e0f6d840..42ad2257c 100644 --- a/jest/save-promise.js +++ b/jest/save-promise.js @@ -1 +1 @@ -global.originalPromise = Promise; \ No newline at end of file +global.$RNTL_ORIGINAL_PROMISE = Promise; diff --git a/src/__tests__/timers.test.js b/src/__tests__/timers.test.js index f1ef62eb1..c53d9420d 100644 --- a/src/__tests__/timers.test.js +++ b/src/__tests__/timers.test.js @@ -1,3 +1,4 @@ +// @flow import waitFor from '../waitFor'; import { TimerMode } from './timerUtils'; From 8c88d6a88cf03e920ffa1bd98ccee0ff00f068e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Mar 2021 13:26:20 +0100 Subject: [PATCH 12/12] rename globals to match others --- jest/restore-promise.js | 2 +- jest/save-promise.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jest/restore-promise.js b/jest/restore-promise.js index 75bf16f96..196b35417 100644 --- a/jest/restore-promise.js +++ b/jest/restore-promise.js @@ -1 +1 @@ -global.Promise = global.$RNTL_ORIGINAL_PROMISE; +global.Promise = global.RNTL_ORIGINAL_PROMISE; diff --git a/jest/save-promise.js b/jest/save-promise.js index 42ad2257c..30a5be234 100644 --- a/jest/save-promise.js +++ b/jest/save-promise.js @@ -1 +1 @@ -global.$RNTL_ORIGINAL_PROMISE = Promise; +global.RNTL_ORIGINAL_PROMISE = Promise;