From 5b2640aca66b9ccf611baa88188e0807df3a73c7 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Tue, 23 Jun 2020 10:14:53 -0600 Subject: [PATCH] feat(waitFor): add complete support for fake timers (#662) Closes #661 --- .../__snapshots__/wait-for-dom-change.js.snap | 37 ---- src/__tests__/deprecation-warnings.js | 30 ++++ src/__tests__/example.js | 92 ---------- src/__tests__/fake-timers.js | 167 +++++------------- src/__tests__/wait-for-dom-change.js | 52 +----- .../wait-for-element-to-be-removed.js | 62 +------ src/__tests__/wait-for-element.js | 56 +----- src/__tests__/wait-for.js | 6 + src/helpers.js | 54 +++--- src/wait-for.js | 56 ++++-- 10 files changed, 167 insertions(+), 445 deletions(-) delete mode 100644 src/__tests__/__snapshots__/wait-for-dom-change.js.snap create mode 100644 src/__tests__/deprecation-warnings.js delete mode 100644 src/__tests__/example.js diff --git a/src/__tests__/__snapshots__/wait-for-dom-change.js.snap b/src/__tests__/__snapshots__/wait-for-dom-change.js.snap deleted file mode 100644 index 86faa06b..00000000 --- a/src/__tests__/__snapshots__/wait-for-dom-change.js.snap +++ /dev/null @@ -1,37 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`timers works with fake timers 1`] = ` -Array [ - Object { - "addedNodes": NodeList [], - "attributeName": "id", - "attributeNamespace": null, - "nextSibling": null, - "oldValue": null, - "previousSibling": null, - "removedNodes": NodeList [], - "target":
, - "type": "attributes", - }, -] -`; - -exports[`timers works with real timers 1`] = ` -Array [ - Object { - "addedNodes": NodeList [], - "attributeName": "id", - "attributeNamespace": null, - "nextSibling": null, - "oldValue": null, - "previousSibling": null, - "removedNodes": NodeList [], - "target":
, - "type": "attributes", - }, -] -`; diff --git a/src/__tests__/deprecation-warnings.js b/src/__tests__/deprecation-warnings.js new file mode 100644 index 00000000..14744159 --- /dev/null +++ b/src/__tests__/deprecation-warnings.js @@ -0,0 +1,30 @@ +import {waitForElement, waitForDomChange, wait} from '..' + +afterEach(() => { + console.warn.mockClear() +}) + +test('deprecation warnings only warn once', async () => { + await wait(() => {}, {timeout: 1}) + await waitForElement(() => {}, {timeout: 1}).catch(e => e) + await waitForDomChange({timeout: 1}).catch(e => e) + expect(console.warn.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "\`wait\` has been deprecated and replaced by \`waitFor\` instead. In most cases you should be able to find/replace \`wait\` with \`waitFor\`. Learn more: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.", + ], + Array [ + "\`waitForElement\` has been deprecated. Use a \`find*\` query (preferred: https://testing-library.com/docs/dom-testing-library/api-queries#findby) or use \`waitFor\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor", + ], + Array [ + "\`waitForDomChange\` has been deprecated. Use \`waitFor\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.", + ], + ] + `) + + console.warn.mockClear() + await wait(() => {}, {timeout: 1}) + await waitForElement(() => {}, {timeout: 1}).catch(e => e) + await waitForDomChange({timeout: 1}).catch(e => e) + expect(console.warn).not.toHaveBeenCalled() +}) diff --git a/src/__tests__/example.js b/src/__tests__/example.js deleted file mode 100644 index 792d7d1f..00000000 --- a/src/__tests__/example.js +++ /dev/null @@ -1,92 +0,0 @@ -// query utilities: -import { - getByLabelText, - getByText, - getByTestId, - queryByTestId, - wait, - fireEvent, -} from '../' - -function getExampleDOM() { - // This is just a raw example of setting up some DOM - // that we can interact with. Swap this with your UI - // framework of choice 😉 - const div = document.createElement('div') - div.innerHTML = ` - - - - ` - const button = div.querySelector('button') - const input = div.querySelector('input') - button.addEventListener('click', () => { - // let's pretend this is making a server request, so it's async - // (you'd want to mock this imaginary request in your unit tests)... - setTimeout(() => { - const printedUsernameContainer = document.createElement('div') - printedUsernameContainer.innerHTML = ` -
${input.value}
- ` - div.appendChild(printedUsernameContainer) - }, Math.floor(Math.random() * 200)) - }) - return div -} - -test('examples of some things', async () => { - const famousWomanInHistory = 'Ada Lovelace' - const container = getExampleDOM() - - // Get form elements by their label text. - // An error will be thrown if one cannot be found (accessibility FTW!) - const input = getByLabelText(container, 'Username') - fireEvent.change(input, {target: {value: famousWomanInHistory}}) - - // Get elements by their text, just like a real user does. - getByText(container, 'Print Username').click() - - await wait(() => - expect(queryByTestId(container, 'printed-username')).toBeTruthy(), - ) - - // getByTestId and queryByTestId are an escape hatch to get elements - // by a test id (could also attempt to get this element by it's text) - expect(getByTestId(container, 'printed-username')).toHaveTextContent( - famousWomanInHistory, - ) - expect(container).toMatchInlineSnapshot(` -
- - - - - - - - - - - -
- - -
- Ada Lovelace -
- - -
-
-`) -}) diff --git a/src/__tests__/fake-timers.js b/src/__tests__/fake-timers.js index d3148528..9f451956 100644 --- a/src/__tests__/fake-timers.js +++ b/src/__tests__/fake-timers.js @@ -1,139 +1,66 @@ +import {waitFor, waitForElementToBeRemoved} from '..' import {render} from './helpers/test-utils' -// Because we're using fake timers here and I don't want these tests to run -// for the actual length of the test (because it's waiting for a timeout error) -// we'll mock the setTimeout, clearTimeout, and setImmediate to be the ones -// that jest will mock for us. -jest.mock('../helpers', () => { - const actualHelpers = jest.requireActual('../helpers') - return { - ...actualHelpers, - setTimeout, - clearTimeout, - setImmediate, - } +beforeAll(() => { + jest.useFakeTimers() }) -jest.useFakeTimers() - -// Because of the way jest mocking works here's the order of things (and no, the order of the code above doesn't make a difference): -// 1. Just mocks '../helpers' and setTimeout/clearTimeout/setImmediate are set to their "correct" values -// 2. We tell Jest to use fake timers -// 3. We reset the modules and we mock '../helpers' again so now setTimeout/clearTimeout/setImmediate are set to their mocked values -// We're only doing this because want to mock those values so this test doesn't take 4501ms to run. -jest.resetModules() - -const { - wait, - waitForElement, - waitForDomChange, - waitForElementToBeRemoved, -} = require('../') - -test('waitForElementToBeRemoved: times out after 4500ms by default', () => { - const {container} = render(`
`) - // there's a bug with this rule here... - // eslint-disable-next-line jest/valid-expect - const promise = expect( - waitForElementToBeRemoved(() => container), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"Timed out in waitForElementToBeRemoved."`, - ) - jest.advanceTimersByTime(4501) - return promise +afterAll(() => { + jest.useRealTimers() }) -test('wait: can time out', async () => { - const promise = wait(() => { - // eslint-disable-next-line no-throw-literal - throw undefined - }) - jest.advanceTimersByTime(4600) - await expect(promise).rejects.toThrow(/timed out/i) -}) - -test('waitForElement: can time out', async () => { - const promise = waitForElement(() => {}) - jest.advanceTimersByTime(4600) - await expect(promise).rejects.toThrow(/timed out/i) -}) +async function runWaitFor() { + const response = 'data' + const doAsyncThing = () => + new Promise(r => setTimeout(() => r(response), 300)) + let result + doAsyncThing().then(r => (result = r)) -test('waitForElement: can specify our own timeout time', async () => { - const promise = waitForElement(() => {}, {timeout: 4700}) - const handler = jest.fn() - promise.then(handler, handler) - // advance beyond the default - jest.advanceTimersByTime(4600) - // promise was neither rejected nor resolved - expect(handler).toHaveBeenCalledTimes(0) + await waitFor(() => expect(result).toBe(response)) +} - // advance beyond our specified timeout - jest.advanceTimersByTime(150) - - // timed out - await expect(promise).rejects.toThrow(/timed out/i) +test('real timers', async () => { + // the only difference when not using fake timers is this test will + // have to wait the full length of the timeout + await runWaitFor() }) -test('waitForDomChange: can time out', async () => { - const promise = waitForDomChange() - jest.advanceTimersByTime(4600) - await expect(promise).rejects.toThrow(/timed out/i) +test('legacy', async () => { + jest.useFakeTimers('legacy') + await runWaitFor() }) -test('waitForDomChange: can specify our own timeout time', async () => { - const promise = waitForDomChange({timeout: 4700}) - const handler = jest.fn() - promise.then(handler, handler) - // advance beyond the default - jest.advanceTimersByTime(4600) - // promise was neither rejected nor resolved - expect(handler).toHaveBeenCalledTimes(0) - - // advance beyond our specified timeout - jest.advanceTimersByTime(150) - - // timed out - await expect(promise).rejects.toThrow(/timed out/i) +test('modern', async () => { + jest.useFakeTimers() + await runWaitFor() }) -test('wait: ensures the interval is greater than 0', async () => { - // Arrange - const spy = jest.fn() - spy.mockImplementationOnce(() => { - throw new Error('first time does not work') - }) - const promise = wait(spy, {interval: 0}) - expect(spy).toHaveBeenCalledTimes(1) - spy.mockClear() - - // Act - // this line will throw an error if wait does not make the interval 1 instead of 0 - // which is why it does that! - jest.advanceTimersByTime(0) - - // Assert - expect(spy).toHaveBeenCalledTimes(0) - spy.mockImplementationOnce(() => 'second time does work') - - // Act - jest.advanceTimersByTime(1) - await promise - - // Assert - expect(spy).toHaveBeenCalledTimes(1) +test('fake timer timeout', async () => { + jest.useFakeTimers() + await expect( + waitFor( + () => { + throw new Error('always throws') + }, + {timeout: 10}, + ), + ).rejects.toMatchInlineSnapshot(`[Error: always throws]`) }) -test('wait: times out if it runs out of attempts', () => { - const spy = jest.fn(() => { - throw new Error('example error') - }) +test('times out after 1000ms by default', async () => { + const {container} = render(`
`) + const start = performance.now() // there's a bug with this rule here... // eslint-disable-next-line jest/valid-expect - const promise = expect( - wait(spy, {interval: 1, timeout: 3}), - ).rejects.toThrowErrorMatchingInlineSnapshot(`"example error"`) - jest.advanceTimersByTime(1) - jest.advanceTimersByTime(1) - jest.advanceTimersByTime(1) - return promise + await expect( + waitForElementToBeRemoved(() => container), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"Timed out in waitForElementToBeRemoved."`, + ) + // NOTE: this assertion ensures that even when we have fake timers, the + // timeout still takes the full 1000ms + // unfortunately, timeout clocks aren't super accurate, so we simply verify + // that it's greater than or equal to 900ms. That's enough to be confident + // that we're using real timers. + expect(performance.now() - start).toBeGreaterThanOrEqual(900) }) diff --git a/src/__tests__/wait-for-dom-change.js b/src/__tests__/wait-for-dom-change.js index c843bb63..3db4d775 100644 --- a/src/__tests__/wait-for-dom-change.js +++ b/src/__tests__/wait-for-dom-change.js @@ -1,16 +1,8 @@ +import {waitForDomChange} from '..' import {renderIntoDocument} from './helpers/test-utils' -function importModule() { - return require('../').waitForDomChange -} - -let waitForDomChange - -beforeEach(() => { +afterEach(() => { jest.useRealTimers() - jest.resetModules() - waitForDomChange = importModule() - console.warn.mockClear() }) test('waits for the dom to change in the document', async () => { @@ -35,13 +27,6 @@ test('waits for the dom to change in the document', async () => { }, ] `) - expect(console.warn.mock.calls).toMatchInlineSnapshot(` -Array [ - Array [ - "\`waitForDomChange\` has been deprecated. Use \`waitFor\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.", - ], -] -`) }) test('waits for the dom to change in a specified container', async () => { @@ -67,36 +52,3 @@ test('waits for the dom to change in a specified container', async () => { ] `) }) - -describe('timers', () => { - const expectElementToChange = async () => { - const importedWaitForDomChange = importModule() - const {container} = renderIntoDocument('
') - - setTimeout(() => container.firstChild.setAttribute('id', 'foo'), 100) - - const promise = importedWaitForDomChange({container, timeout: 200}) - - if (setTimeout._isMockFunction) { - jest.advanceTimersByTime(110) - } - - await expect(promise).resolves.toMatchSnapshot() - } - - it('works with real timers', async () => { - jest.useRealTimers() - await expectElementToChange() - }) - it('works with fake timers', async () => { - jest.useFakeTimers() - await expectElementToChange() - }) -}) - -test("doesn't change jest's timers value when importing the module", () => { - jest.useFakeTimers() - importModule() - - expect(window.setTimeout._isMockFunction).toEqual(true) -}) diff --git a/src/__tests__/wait-for-element-to-be-removed.js b/src/__tests__/wait-for-element-to-be-removed.js index f29bf86b..ffb0cf9b 100644 --- a/src/__tests__/wait-for-element-to-be-removed.js +++ b/src/__tests__/wait-for-element-to-be-removed.js @@ -1,17 +1,6 @@ +import {waitForElementToBeRemoved} from '..' import {renderIntoDocument} from './helpers/test-utils' -function importModule() { - return require('../').waitForElementToBeRemoved -} - -let waitForElementToBeRemoved - -beforeEach(() => { - jest.useRealTimers() - jest.resetModules() - waitForElementToBeRemoved = importModule() -}) - test('resolves on mutation only when the element is removed', async () => { const {queryAllByTestId} = renderIntoDocument(`
@@ -89,55 +78,6 @@ test('after successful removal, fullfills promise with empty value (undefined)', return expect(waitResult).resolves.toBeUndefined() }) -describe('timers', () => { - const expectElementToBeRemoved = async () => { - const importedWaitForElementToBeRemoved = importModule() - - const {queryAllByTestId} = renderIntoDocument(` -
-
-`) - const divs = queryAllByTestId('div') - // first mutation - setTimeout(() => { - divs.forEach(d => d.setAttribute('id', 'mutated')) - }) - // removal - setTimeout(() => { - divs.forEach(div => div.parentElement.removeChild(div)) - }, 100) - - const promise = importedWaitForElementToBeRemoved( - () => queryAllByTestId('div'), - { - timeout: 200, - }, - ) - - if (setTimeout._isMockFunction) { - jest.advanceTimersByTime(110) - } - - await promise - } - - it('works with real timers', async () => { - jest.useRealTimers() - await expectElementToBeRemoved() - }) - it('works with fake timers', async () => { - jest.useFakeTimers() - await expectElementToBeRemoved() - }) -}) - -test("doesn't change jest's timers value when importing the module", () => { - jest.useFakeTimers() - importModule() - - expect(window.setTimeout._isMockFunction).toEqual(true) -}) - test('rethrows non-testing-lib errors', () => { let throwIt = false const div = document.createElement('div') diff --git a/src/__tests__/wait-for-element.js b/src/__tests__/wait-for-element.js index c1b7016b..e766b47a 100644 --- a/src/__tests__/wait-for-element.js +++ b/src/__tests__/wait-for-element.js @@ -1,16 +1,8 @@ +import {waitForElement} from '..' import {render, renderIntoDocument} from './helpers/test-utils' -function importModule() { - return require('../').waitForElement -} - -let waitForElement - -beforeEach(() => { +afterEach(() => { jest.useRealTimers() - jest.resetModules() - waitForElement = importModule() - console.warn.mockClear() }) test('waits for element to appear in the document', async () => { @@ -19,13 +11,12 @@ test('waits for element to appear in the document', async () => { setTimeout(() => rerender('
')) const element = await promise expect(element).toBeInTheDocument() - expect(console.warn.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - "\`waitForElement\` has been deprecated. Use a \`find*\` query (preferred: https://testing-library.com/docs/dom-testing-library/api-queries#findby) or use \`waitFor\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor", - ], - ] - `) +}) + +test('can time out', async () => { + await expect(waitForElement(() => {}, {timeout: 1})).rejects.toThrow( + /timed out/i, + ) }) test('waits for element to appear in a specified container', async () => { @@ -67,34 +58,3 @@ test('waits until callback does not return null', async () => { test('throws error if no callback is provided', async () => { await expect(waitForElement()).rejects.toThrow(/callback/i) }) - -describe('timers', () => { - const expectElementToExist = async () => { - const importedWaitForElement = importModule() - - const {rerender, getByTestId} = renderIntoDocument('
') - - setTimeout(() => rerender('
'), 100) - - const promise = importedWaitForElement(() => getByTestId('div'), { - timeout: 200, - }) - - if (setTimeout._isMockFunction) { - jest.advanceTimersByTime(110) - } - - const element = await promise - - await expect(element).toBeInTheDocument() - } - - it('works with real timers', async () => { - jest.useRealTimers() - await expectElementToExist() - }) - it('works with fake timers', async () => { - jest.useFakeTimers() - await expectElementToExist() - }) -}) diff --git a/src/__tests__/wait-for.js b/src/__tests__/wait-for.js index 2c7fa964..082d9dc1 100644 --- a/src/__tests__/wait-for.js +++ b/src/__tests__/wait-for.js @@ -12,6 +12,12 @@ test('waits callback to not throw an error', async () => { expect(spy).toHaveBeenCalledWith() }) +// we used to have a limitation where we had to set an interval of 0 to 1 +// otherwise there would be problems. I don't think this limitation exists +// anymore, but we'll keep this test around to make sure a problem doesn't +// crop up. +test('can accept an interval of 0', () => waitFor(() => {}, {interval: 0})) + test('can timeout after the given timeout time', async () => { const error = new Error('throws every time') const result = await waitFor( diff --git a/src/helpers.js b/src/helpers.js index feab0aa6..3143086a 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -2,44 +2,53 @@ 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 usingJestAndTimers = - typeof jest !== 'undefined' && typeof globalObj.setTimeout !== 'undefined' - const usingLegacyJestFakeTimers = - usingJestAndTimers && + 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' + } - let usingModernJestFakeTimers = false if ( - usingJestAndTimers && 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() - usingModernJestFakeTimers = true + return 'modern' } catch { // not using Jest's modern fake timers } } - - const usingJestFakeTimers = - usingLegacyJestFakeTimers || usingModernJestFakeTimers - - if (usingJestFakeTimers) { - jest.useRealTimers() - } - - const callbackReturnValue = callback() - - if (usingJestFakeTimers) { - jest.useFakeTimers(usingModernJestFakeTimers ? 'modern' : 'legacy') - } - - return callbackReturnValue + return null } +const jestFakeTimersAreEnabled = () => Boolean(getJestFakeTimersType()) + // we only run our tests in node, and setImmediate is supported in node. // istanbul ignore next function setImmediatePolyfill(fn) { @@ -117,4 +126,5 @@ export { setTimeoutFn as setTimeout, runWithRealTimers, checkContainerType, + jestFakeTimersAreEnabled, } diff --git a/src/wait-for.js b/src/wait-for.js index b271a00d..0a633f3c 100644 --- a/src/wait-for.js +++ b/src/wait-for.js @@ -1,10 +1,13 @@ import { getWindowFromNode, getDocument, + jestFakeTimersAreEnabled, + // We import these from the helpers rather than using the global version + // because these will be *real* timers, regardless of whether we're in + // an environment that's faked the timers out. setImmediate, setTimeout, clearTimeout, - runWithRealTimers, } from './helpers' import {getConfig, runWithExpensiveErrorDiagnosticsDisabled} from './config' @@ -34,23 +37,46 @@ function waitFor( throw new TypeError('Received `callback` arg must be a function') } - if (interval < 1) interval = 1 - return new Promise((resolve, reject) => { - let lastError + return new Promise(async (resolve, reject) => { + let lastError, intervalId, observer + let finished = false + const overallTimeoutTimer = setTimeout(onTimeout, timeout) - const intervalId = setInterval(checkCallback, interval) - const {MutationObserver} = getWindowFromNode(container) - const observer = new MutationObserver(checkCallback) - runWithRealTimers(() => - observer.observe(container, mutationObserverOptions), - ) - checkCallback() + const usingFakeTimers = jestFakeTimersAreEnabled() + if (usingFakeTimers) { + checkCallback() + // 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) { + jest.advanceTimersByTime(interval) + // 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(r => setImmediate(r)) + checkCallback() + } + } else { + intervalId = setInterval(checkCallback, interval) + const {MutationObserver} = getWindowFromNode(container) + observer = new MutationObserver(checkCallback) + observer.observe(container, mutationObserverOptions) + checkCallback() + } function onDone(error, result) { + finished = true clearTimeout(overallTimeoutTimer) - clearInterval(intervalId) - setImmediate(() => observer.disconnect()) + + if (!usingFakeTimers) { + clearInterval(intervalId) + setImmediate(() => observer.disconnect()) + } if (error) { reject(error) @@ -62,9 +88,9 @@ function waitFor( function checkCallback() { try { onDone(null, runWithExpensiveErrorDiagnosticsDisabled(callback)) - // If `callback` throws, wait for the next mutation or timeout. + // If `callback` throws, wait for the next mutation, interval, or timeout. } catch (error) { - // Save the callback error to reject the promise with it. + // Save the most recent callback error to reject the promise with it in the event of a timeout lastError = error } }