diff --git a/package.json b/package.json index 3ef5dcd6d..8f9420fb2 100644 --- a/package.json +++ b/package.json @@ -45,8 +45,7 @@ "@types/testing-library__dom": "^6.12.1", "aria-query": "^4.0.2", "dom-accessibility-api": "^0.3.0", - "pretty-format": "^25.1.0", - "wait-for-expect": "^3.0.2" + "pretty-format": "^25.1.0" }, "devDependencies": { "@testing-library/jest-dom": "^5.1.1", @@ -61,7 +60,8 @@ "rules": { "import/prefer-default-export": "off", "import/no-unassigned-import": "off", - "import/no-useless-path-segments": "off" + "import/no-useless-path-segments": "off", + "no-console": "off" } }, "eslintIgnore": [ diff --git a/src/__tests__/fake-timers.js b/src/__tests__/fake-timers.js index 50f1c6766..d31485280 100644 --- a/src/__tests__/fake-timers.js +++ b/src/__tests__/fake-timers.js @@ -24,6 +24,7 @@ jest.useFakeTimers() jest.resetModules() const { + wait, waitForElement, waitForDomChange, waitForElementToBeRemoved, @@ -42,6 +43,15 @@ test('waitForElementToBeRemoved: times out after 4500ms by default', () => { return promise }) +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) @@ -85,3 +95,45 @@ test('waitForDomChange: can specify our own timeout time', async () => { // timed out await expect(promise).rejects.toThrow(/timed out/i) }) + +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('wait: times out if it runs out of attempts', () => { + const spy = jest.fn(() => { + throw new Error('example error') + }) + // 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 +}) diff --git a/src/__tests__/pretty-dom.js b/src/__tests__/pretty-dom.js index 22cf95aa1..fd2978886 100644 --- a/src/__tests__/pretty-dom.js +++ b/src/__tests__/pretty-dom.js @@ -77,5 +77,3 @@ describe('prettyDOM fails with first parameter without outerHTML field', () => { ) }) }) - -/* eslint no-console:0 */ diff --git a/src/__tests__/role-helpers.js b/src/__tests__/role-helpers.js index 47a58e9bc..66b41d525 100644 --- a/src/__tests__/role-helpers.js +++ b/src/__tests__/role-helpers.js @@ -184,5 +184,3 @@ test.each([ expect(isInaccessible(container.querySelector('button'))).toBe(expected) }) - -/* eslint no-console:0 */ diff --git a/src/__tests__/screen.js b/src/__tests__/screen.js index d9c99d82f..a2655adc0 100644 --- a/src/__tests__/screen.js +++ b/src/__tests__/screen.js @@ -61,5 +61,3 @@ test('exposes debug method', () => { `) console.log.mockClear() }) - -/* eslint no-console:0 */ diff --git a/src/__tests__/wait-for-dom-change.js b/src/__tests__/wait-for-dom-change.js index 5b181b645..2fb7b2975 100644 --- a/src/__tests__/wait-for-dom-change.js +++ b/src/__tests__/wait-for-dom-change.js @@ -10,6 +10,7 @@ beforeEach(() => { jest.useRealTimers() jest.resetModules() waitForDomChange = importModule() + console.warn.mockClear() }) test('waits for the dom to change in the document', async () => { @@ -34,6 +35,13 @@ test('waits for the dom to change in the document', async () => { }, ] `) + expect(console.warn.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + "\`waitForDomChange\` has been deprecated. Use \`wait\` instead: https://testing-library.com/docs/dom-testing-library/api-async#wait.", + ], +] +`) }) test('waits for the dom to change in a specified container', async () => { diff --git a/src/__tests__/wait-for-element-to-be-removed.js b/src/__tests__/wait-for-element-to-be-removed.js index e52dfd21b..ce90fe030 100644 --- a/src/__tests__/wait-for-element-to-be-removed.js +++ b/src/__tests__/wait-for-element-to-be-removed.js @@ -49,7 +49,7 @@ test('requires a function as the first parameter', () => { return expect( waitForElementToBeRemoved(), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"waitForElementToBeRemoved requires a function as the first parameter"`, + `"waitForElementToBeRemoved requires a callback as the first parameter"`, ) }) @@ -57,7 +57,7 @@ test('requires an element to exist first', () => { return expect( waitForElementToBeRemoved(() => null), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal."`, + `"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal."`, ) }) @@ -65,7 +65,7 @@ test('requires an unempty array of elements to exist first', () => { return expect( waitForElementToBeRemoved(() => []), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal."`, + `"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal."`, ) }) @@ -117,3 +117,18 @@ test("doesn't change jest's timers value when importing the module", () => { expect(window.setTimeout._isMockFunction).toEqual(true) }) + +test('rethrows non-testing-lib errors', () => { + let throwIt = false + const div = document.createElement('div') + const error = new Error('my own error') + return expect( + waitForElementToBeRemoved(() => { + if (throwIt) { + throw error + } + throwIt = true + return div + }), + ).rejects.toBe(error) +}) diff --git a/src/__tests__/wait-for-element.js b/src/__tests__/wait-for-element.js index 3dc296875..2ed61b8df 100644 --- a/src/__tests__/wait-for-element.js +++ b/src/__tests__/wait-for-element.js @@ -10,6 +10,7 @@ beforeEach(() => { jest.useRealTimers() jest.resetModules() waitForElement = importModule() + console.warn.mockClear() }) test('waits for element to appear in the document', async () => { @@ -18,6 +19,13 @@ 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 \`wait\` instead (it's the same API, so you can find/replace): https://testing-library.com/docs/dom-testing-library/api-async#wait", + ], + ] + `) }) test('waits for element to appear in a specified container', async () => { diff --git a/src/__tests__/wait.js b/src/__tests__/wait.js index bbe65e0a6..70e7a83a5 100644 --- a/src/__tests__/wait.js +++ b/src/__tests__/wait.js @@ -17,3 +17,14 @@ test('wait defaults to a noop callback', async () => { await wait() expect(handler).toHaveBeenCalledTimes(1) }) + +test('can timeout after the given timeout time', async () => { + const error = new Error('throws every time') + const result = await wait( + () => { + throw error + }, + {timeout: 8, interval: 5}, + ).catch(e => e) + expect(result).toBe(error) +}) diff --git a/src/config.js b/src/config.js index dbb80da07..001539632 100644 --- a/src/config.js +++ b/src/config.js @@ -5,7 +5,7 @@ import {prettyDOM} from './pretty-dom' // './queries' are query functions. let config = { testIdAttribute: 'data-testid', - asyncUtilTimeout: 4500, + asyncUtilTimeout: 1000, // this is to support React's async `act` function. // forcing react-testing-library to wrap all async functions would've been // a total nightmare (consider wrapping every findBy* query and then also @@ -19,9 +19,11 @@ let config = { // called when getBy* queries fail. (message, container) => Error getElementError(message, container) { - return new Error( + const error = new Error( [message, prettyDOM(container)].filter(Boolean).join('\n\n'), ) + error.name = 'TestingLibraryElementError' + return error }, } diff --git a/src/pretty-dom.js b/src/pretty-dom.js index 9bfe0a584..56e8e90d5 100644 --- a/src/pretty-dom.js +++ b/src/pretty-dom.js @@ -18,7 +18,7 @@ const inNode = () => const getMaxLength = dom => inCypress(dom) ? 0 - : typeof process !== 'undefined' && process.env.DEBUG_PRINT_LIMIT || 7000 + : (typeof process !== 'undefined' && process.env.DEBUG_PRINT_LIMIT) || 7000 const {DOMElement, DOMCollection} = prettyFormat.plugins @@ -64,5 +64,3 @@ function prettyDOM(dom, maxLength, options) { const logDOM = (...args) => console.log(prettyDOM(...args)) export {prettyDOM, logDOM} - -/* eslint no-console:0 */ diff --git a/src/role-helpers.js b/src/role-helpers.js index 5027ec318..eee0010ab 100644 --- a/src/role-helpers.js +++ b/src/role-helpers.js @@ -176,5 +176,3 @@ export { prettyRoles, isInaccessible, } - -/* eslint no-console:0 */ diff --git a/src/wait-for-dom-change.js b/src/wait-for-dom-change.js index 09b742bfb..566746e1e 100644 --- a/src/wait-for-dom-change.js +++ b/src/wait-for-dom-change.js @@ -8,6 +8,11 @@ import { } from './helpers' import {getConfig} from './config' +let hasWarned = false + +// deprecated... TODO: remove this method. People should use wait instead +// the reasoning is that waiting for just any DOM change is an implementation +// detail. People should be waiting for a specific thing to change. function waitForDomChange({ container = getDocument(), timeout = getConfig().asyncUtilTimeout, @@ -18,6 +23,12 @@ function waitForDomChange({ characterData: true, }, } = {}) { + if (!hasWarned) { + hasWarned = true + console.warn( + `\`waitForDomChange\` has been deprecated. Use \`wait\` instead: https://testing-library.com/docs/dom-testing-library/api-async#wait.`, + ) + } return new Promise((resolve, reject) => { const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) diff --git a/src/wait-for-element-to-be-removed.js b/src/wait-for-element-to-be-removed.js index 8bcbccb20..e04ae2b42 100644 --- a/src/wait-for-element-to-be-removed.js +++ b/src/wait-for-element-to-be-removed.js @@ -1,85 +1,44 @@ -import { - getDocument, - newMutationObserver, - setImmediate, - setTimeout, - clearTimeout, - runWithRealTimers, -} from './helpers' -import {getConfig} from './config' +import {wait} from './wait' -function waitForElementToBeRemoved( - callback, - { - container = getDocument(), - timeout = getConfig().asyncUtilTimeout, - mutationObserverOptions = { - subtree: true, - childList: true, - attributes: true, - characterData: true, - }, - } = {}, -) { - return new Promise((resolve, reject) => { - if (typeof callback !== 'function') { - reject( - new Error( - 'waitForElementToBeRemoved requires a function as the first parameter', - ), - ) - } - const timer = setTimeout(onTimeout, timeout) - const observer = newMutationObserver(onMutation) +const isRemoved = result => !result || (Array.isArray(result) && !result.length) + +async function waitForElementToBeRemoved(callback, options) { + if (!callback) { + return Promise.reject( + new Error( + 'waitForElementToBeRemoved requires a callback as the first parameter', + ), + ) + } + + // Check if the element is not present synchronously, + // As the name implies, waitForElementToBeRemoved should check `present` --> `removed` + if (isRemoved(callback())) { + throw new Error( + 'The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist(s) before waiting for removal.', + ) + } - // Check if the element is not present synchronously, - // As the name waitForElementToBeRemoved should check `present` --> `removed` + return wait(() => { + let result try { - const result = callback() - if (!result || (Array.isArray(result) && !result.length)) { - onDone( - new Error( - 'The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal.', - ), - ) - } else { - // Only observe for mutations only if there is element while checking synchronously - runWithRealTimers(() => - observer.observe(container, mutationObserverOptions), - ) - } + result = callback() } catch (error) { - onDone(error) - } - - function onDone(error, result) { - clearTimeout(timer) - setImmediate(() => observer.disconnect()) - if (error) { - reject(error) - } else { - resolve(result) - } - } - function onMutation() { - try { - const result = callback() - if (!result || (Array.isArray(result) && !result.length)) { - onDone(null, true) - } - // If `callback` returns truthy value, wait for the next mutation or timeout. - } catch (error) { - onDone(null, true) + if (error.name === 'TestingLibraryElementError') { + return true } + throw error } - function onTimeout() { - onDone(new Error('Timed out in waitForElementToBeRemoved.'), null) + if (!isRemoved(result)) { + throw new Error('Timed out in waitForElementToBeRemoved.') } - }) + return true + }, options) } -function waitForElementToBeRemovedWrapper(...args) { - return getConfig().asyncWrapper(() => waitForElementToBeRemoved(...args)) -} +export {waitForElementToBeRemoved} -export {waitForElementToBeRemovedWrapper as waitForElementToBeRemoved} +/* +eslint + require-await: "off" +*/ diff --git a/src/wait-for-element.js b/src/wait-for-element.js index 30fe0e9a6..e86c18603 100644 --- a/src/wait-for-element.js +++ b/src/wait-for-element.js @@ -1,71 +1,32 @@ -import { - newMutationObserver, - getDocument, - setImmediate, - setTimeout, - clearTimeout, - runWithRealTimers, -} from './helpers' -import {getConfig} from './config' +import {wait} from './wait' -function waitForElement( - callback, - { - container = getDocument(), - timeout = getConfig().asyncUtilTimeout, - mutationObserverOptions = { - subtree: true, - childList: true, - attributes: true, - characterData: true, - }, - } = {}, -) { - return new Promise((resolve, reject) => { - if (typeof callback !== 'function') { - reject( - new Error('waitForElement requires a callback as the first parameter'), - ) - return - } - let lastError - const timer = setTimeout(onTimeout, timeout) +let hasWarned = false - const observer = newMutationObserver(onMutation) - runWithRealTimers(() => - observer.observe(container, mutationObserverOptions), +// deprecated... TODO: remove this method. People should use a find* query or +// wait instead the reasoning is that this doesn't really do anything useful +// that you can't get from using find* or wait. +async function waitForElement(callback, options) { + if (!hasWarned) { + hasWarned = true + console.warn( + `\`waitForElement\` has been deprecated. Use a \`find*\` query (preferred: https://testing-library.com/docs/dom-testing-library/api-queries#findby) or use \`wait\` instead (it's the same API, so you can find/replace): https://testing-library.com/docs/dom-testing-library/api-async#wait`, ) - function onDone(error, result) { - clearTimeout(timer) - setImmediate(() => observer.disconnect()) - if (error) { - reject(error) - } else { - resolve(result) - } - } - function onMutation() { - try { - const result = callback() - if (result) { - onDone(null, result) - } - // If `callback` returns falsy value, wait for the next mutation or timeout. - } catch (error) { - // Save the callback error to reject the promise with it. - lastError = error - // If `callback` throws an error, wait for the next mutation or timeout. - } + } + if (!callback) { + throw new Error('waitForElement requires a callback as the first parameter') + } + return wait(() => { + const result = callback() + if (!result) { + throw new Error('Timed out in waitForElement.') } - function onTimeout() { - onDone(lastError || new Error('Timed out in waitForElement.'), null) - } - onMutation() - }) + return result + }, options) } -function waitForElementWrapper(...args) { - return getConfig().asyncWrapper(() => waitForElement(...args)) -} +export {waitForElement} -export {waitForElementWrapper as waitForElement} +/* +eslint + require-await: "off" +*/ diff --git a/src/wait.js b/src/wait.js index 193aa0573..86c7596d5 100644 --- a/src/wait.js +++ b/src/wait.js @@ -1,8 +1,64 @@ -import waitForExpect from 'wait-for-expect' +import { + newMutationObserver, + getDocument, + setImmediate, + setTimeout, + clearTimeout, + runWithRealTimers, +} from './helpers' import {getConfig} from './config' -function wait(callback = () => {}, {timeout = getConfig().asyncUtilTimeout, interval = 50} = {}) { - return waitForExpect(callback, timeout, interval) +function wait( + callback = () => {}, + { + container = getDocument(), + timeout = getConfig().asyncUtilTimeout, + interval = 50, + mutationObserverOptions = { + subtree: true, + childList: true, + attributes: true, + characterData: true, + }, + } = {}, +) { + if (interval < 1) interval = 1 + return new Promise((resolve, reject) => { + let lastError + const overallTimeoutTimer = setTimeout(onTimeout, timeout) + const intervalId = setInterval(checkCallback, interval) + + const observer = newMutationObserver(checkCallback) + runWithRealTimers(() => + observer.observe(container, mutationObserverOptions), + ) + checkCallback() + + function onDone(error, result) { + clearTimeout(overallTimeoutTimer) + clearInterval(intervalId) + setImmediate(() => observer.disconnect()) + if (error) { + reject(error) + } else { + resolve(result) + } + } + + function checkCallback() { + try { + onDone(null, callback()) + // If `callback` throws, wait for the next mutation or timeout. + } catch (error) { + // Save the callback error to reject the promise with it. + lastError = error + } + } + + function onTimeout() { + onDone(lastError || new Error('Timed out in wait.'), null) + } + }) } function waitWrapper(...args) { diff --git a/tests/setup-env.js b/tests/setup-env.js index 5aefc00b2..99b03bdba 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -25,3 +25,17 @@ expect.addSnapshotSerializer({ ) }, }) + +beforeAll(() => { + const originalWarn = console.warn + jest.spyOn(console, 'warn').mockImplementation((...args) => { + if (args[0] && args[0].includes && args[0].includes('deprecated')) { + return + } + originalWarn(...args) + }) +}) + +afterAll(() => { + console.warn.mockRestore() +})