From 217c891728ca86a0336bd0d610744112ac130228 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 1 Jun 2021 10:20:40 +0200 Subject: [PATCH 1/3] fix: Only use a single clock --- src/__tests__/deprecation-warnings.js | 30 --------- src/__tests__/fake-timers.js | 2 +- src/__tests__/helpers.js | 71 +-------------------- src/__tests__/wait-for-dom-change.js | 54 ---------------- src/helpers.js | 88 +++------------------------ src/index.js | 1 - src/wait-for-dom-change.js | 64 ------------------- src/wait-for.js | 26 ++------ 8 files changed, 17 insertions(+), 319 deletions(-) delete mode 100644 src/__tests__/deprecation-warnings.js delete mode 100644 src/__tests__/wait-for-dom-change.js delete mode 100644 src/wait-for-dom-change.js diff --git a/src/__tests__/deprecation-warnings.js b/src/__tests__/deprecation-warnings.js deleted file mode 100644 index 14744159..00000000 --- a/src/__tests__/deprecation-warnings.js +++ /dev/null @@ -1,30 +0,0 @@ -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__/fake-timers.js b/src/__tests__/fake-timers.js index af2a09ac..4665e8e8 100644 --- a/src/__tests__/fake-timers.js +++ b/src/__tests__/fake-timers.js @@ -68,7 +68,7 @@ test('recursive timers do not cause issues', async () => { } startTimer() - await runWaitFor({time: 800}, {timeout: 100}) + await runWaitFor({time: 800}, {timeout: 900}) recurse = false }) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index 07b53007..2033dbde 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -1,10 +1,5 @@ import {screen} from '../' -import { - getDocument, - getWindowFromNode, - checkContainerType, - runWithRealTimers, -} from '../helpers' +import {getDocument, getWindowFromNode, checkContainerType} from '../helpers' test('returns global document if exists', () => { expect(getDocument()).toBe(document) @@ -61,67 +56,3 @@ describe('query container validation throws when validation fails', () => { ) }) }) - -describe('run with real timers', () => { - const realSetTimeout = global.setTimeout - - afterEach(() => { - // restore timers replaced by jest.useFakeTimers() - jest.useRealTimers() - // restore setTimeout replaced by assignment - global.setTimeout = realSetTimeout - }) - - test('use real timers when timers are faked with jest.useFakeTimers(legacy)', () => { - // legacy timers use mocks and do not rely on a clock instance - jest.useFakeTimers('legacy') - runWithRealTimers(() => { - expect(global.setTimeout).toBe(realSetTimeout) - }) - expect(global.setTimeout._isMockFunction).toBe(true) - expect(global.setTimeout.clock).toBeUndefined() - }) - - test('use real timers when timers are faked with jest.useFakeTimers(modern)', () => { - // modern timers use a clock instance instead of a mock - jest.useFakeTimers('modern') - runWithRealTimers(() => { - expect(global.setTimeout).toBe(realSetTimeout) - }) - expect(global.setTimeout._isMockFunction).toBeUndefined() - expect(global.setTimeout.clock).toBeDefined() - }) - - test('do not use real timers when timers are not faked with jest.useFakeTimers', () => { - // useFakeTimers is not used, timers are faked in some other way - const fakedSetTimeout = callback => { - callback() - } - fakedSetTimeout.clock = jest.fn() - global.setTimeout = fakedSetTimeout - - runWithRealTimers(() => { - expect(global.setTimeout).toBe(fakedSetTimeout) - }) - expect(global.setTimeout).toBe(fakedSetTimeout) - }) - - describe('run with setImmediate and clearImmediate deleted', () => { - const setImmediate = global.setImmediate - const clearImmediate = global.clearImmediate - - beforeEach(() => { - delete global.setImmediate - delete global.clearImmediate - }) - - afterEach(() => { - global.setImmediate = setImmediate - global.clearImmediate = clearImmediate - }) - - test('safe check for setImmediate and clearImmediate', () => { - expect(() => runWithRealTimers(() => {})).not.toThrow() - }) - }) -}) diff --git a/src/__tests__/wait-for-dom-change.js b/src/__tests__/wait-for-dom-change.js deleted file mode 100644 index 3db4d775..00000000 --- a/src/__tests__/wait-for-dom-change.js +++ /dev/null @@ -1,54 +0,0 @@ -import {waitForDomChange} from '..' -import {renderIntoDocument} from './helpers/test-utils' - -afterEach(() => { - jest.useRealTimers() -}) - -test('waits for the dom to change in the document', async () => { - const {container} = renderIntoDocument('
') - const promise = waitForDomChange() - setTimeout(() => container.firstChild.setAttribute('id', 'foo')) - const mutationResult = await promise - expect(mutationResult).toMatchInlineSnapshot(` - Array [ - Object { - "addedNodes": NodeList [], - "attributeName": "id", - "attributeNamespace": null, - "nextSibling": null, - "oldValue": null, - "previousSibling": null, - "removedNodes": NodeList [], - "target":
, - "type": "attributes", - }, - ] - `) -}) - -test('waits for the dom to change in a specified container', async () => { - const {container} = renderIntoDocument('
') - const promise = waitForDomChange({container}) - setTimeout(() => container.firstChild.setAttribute('id', 'foo')) - const mutationResult = await promise - expect(mutationResult).toMatchInlineSnapshot(` - Array [ - Object { - "addedNodes": NodeList [], - "attributeName": "id", - "attributeNamespace": null, - "nextSibling": null, - "oldValue": null, - "previousSibling": null, - "removedNodes": NodeList [], - "target":
, - "type": "attributes", - }, - ] - `) -}) diff --git a/src/helpers.js b/src/helpers.js index 8cdecc78..c70218cc 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -1,85 +1,21 @@ -const globalObj = typeof window === 'undefined' ? global : window // Constant node.nodeType for text nodes, see: // https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeType#Node_type_constants const TEXT_NODE = 3 -// Currently this fn only supports jest timers, but it could support other test runners in the future. -function runWithRealTimers(callback) { - return hasJestTimers() - ? runWithJestRealTimers(callback).callbackReturnValue - : // istanbul ignore next - callback() -} - -function hasJestTimers() { - return ( - typeof jest !== 'undefined' && - jest !== null && - typeof jest.useRealTimers === 'function' - ) -} - -function runWithJestRealTimers(callback) { - const timerAPI = { - clearInterval, - clearTimeout, - setInterval, - setTimeout, - } - - // For more on why we have the check here, - // checkout https://github.com/testing-library/dom-testing-library/issues/914 - if (typeof setImmediate === 'function') { - timerAPI.setImmediate = setImmediate - } - if (typeof clearImmediate === 'function') { - timerAPI.clearImmediate = clearImmediate - } - - jest.useRealTimers() - - const callbackReturnValue = callback() - - const usedFakeTimers = Object.entries(timerAPI).some( - ([name, func]) => func !== globalObj[name], - ) - - if (usedFakeTimers) { - jest.useFakeTimers(timerAPI.setTimeout?.clock ? 'modern' : 'legacy') - } - - return { - callbackReturnValue, - usedFakeTimers, - } -} - function jestFakeTimersAreEnabled() { - return hasJestTimers() - ? runWithJestRealTimers(() => {}).usedFakeTimers - : // istanbul ignore next - false -} - -// 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, + /* istanbul ignore else */ + if (typeof jest !== 'undefined' && jest !== null) { + return ( + // legacy timers + setTimeout._isMockFunction === true || + // modern timers + Object.prototype.hasOwnProperty.call(setTimeout, 'clock') + ) } + // istanbul ignore next + return false } -const {clearTimeoutFn, setImmediateFn, setTimeoutFn} = runWithRealTimers( - getTimeFunctions, -) - function getDocument() { /* istanbul ignore if */ if (typeof window === 'undefined') { @@ -144,10 +80,6 @@ function checkContainerType(container) { export { getWindowFromNode, getDocument, - clearTimeoutFn as clearTimeout, - setImmediateFn as setImmediate, - setTimeoutFn as setTimeout, - runWithRealTimers, checkContainerType, jestFakeTimersAreEnabled, TEXT_NODE, diff --git a/src/index.js b/src/index.js index 2a279383..660d0849 100644 --- a/src/index.js +++ b/src/index.js @@ -6,7 +6,6 @@ export * from './queries' export * from './wait-for' export * from './wait-for-element' export * from './wait-for-element-to-be-removed' -export * from './wait-for-dom-change' export {getDefaultNormalizer} from './matches' export * from './get-node-text' export * from './events' diff --git a/src/wait-for-dom-change.js b/src/wait-for-dom-change.js deleted file mode 100644 index 1344db9d..00000000 --- a/src/wait-for-dom-change.js +++ /dev/null @@ -1,64 +0,0 @@ -import { - getWindowFromNode, - getDocument, - setImmediate, - setTimeout, - clearTimeout, - runWithRealTimers, -} 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, - mutationObserverOptions = { - subtree: true, - childList: true, - attributes: true, - characterData: true, - }, -} = {}) { - if (!hasWarned) { - hasWarned = true - console.warn( - `\`waitForDomChange\` has been deprecated. Use \`waitFor\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.`, - ) - } - return new Promise((resolve, reject) => { - const timer = setTimeout(onTimeout, timeout) - const {MutationObserver} = getWindowFromNode(container) - const observer = new MutationObserver(onMutation) - runWithRealTimers(() => - observer.observe(container, mutationObserverOptions), - ) - - function onDone(error, result) { - clearTimeout(timer) - setImmediate(() => observer.disconnect()) - if (error) { - reject(error) - } else { - resolve(result) - } - } - - function onMutation(mutationsList) { - onDone(null, mutationsList) - } - - function onTimeout() { - onDone(new Error('Timed out in waitForDomChange.'), null) - } - }) -} - -function waitForDomChangeWrapper(...args) { - return getConfig().asyncWrapper(() => waitForDomChange(...args)) -} - -export {waitForDomChangeWrapper as waitForDomChange} diff --git a/src/wait-for.js b/src/wait-for.js index 860b7a15..0a1e75ed 100644 --- a/src/wait-for.js +++ b/src/wait-for.js @@ -5,9 +5,6 @@ import { // 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, checkContainerType, } from './helpers' import {getConfig, runWithExpensiveErrorDiagnosticsDisabled} from './config' @@ -87,7 +84,10 @@ function waitFor( // 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)) + await new Promise(r => { + setTimeout(r, 0) + jest.advanceTimersByTime(0) + }) } } else { try { @@ -187,23 +187,7 @@ function waitForWrapper(callback, options) { ) } -let hasWarned = false - -// deprecated... TODO: remove this method. We renamed this to `waitFor` so the -// code people write reads more clearly. -function wait(...args) { - // istanbul ignore next - const [first = () => {}, ...rest] = args - if (!hasWarned) { - hasWarned = true - console.warn( - `\`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.`, - ) - } - return waitForWrapper(first, ...rest) -} - -export {waitForWrapper as waitFor, wait} +export {waitForWrapper as waitFor} /* eslint From fa3935cf18f9f6909b80d0d8cc48d34caed98499 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 1 Jun 2021 11:01:49 +0200 Subject: [PATCH 2/3] jest 26 fake timers also mock performance --- src/__tests__/fake-timers.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/__tests__/fake-timers.js b/src/__tests__/fake-timers.js index 4665e8e8..69919771 100644 --- a/src/__tests__/fake-timers.js +++ b/src/__tests__/fake-timers.js @@ -50,12 +50,11 @@ test('times out after 1000ms by default', async () => { ).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) + // NOTE: this assertion ensures that the timeout runs in the declared (fake) clock + // while in real time the time was only a fraction since the real clock is only bound by the CPU + // So 10ms is really just an approximation on how long the CPU needs to execute our code. + // If people want to timeout in real time they should rely on their test runners. + expect(performance.now() - start).toBeLessThanOrEqual(10) }) test('recursive timers do not cause issues', async () => { From 24f082b3ad2ac6a18c6a474ed9b4f04469c6adca Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 1 Jun 2021 11:16:36 +0200 Subject: [PATCH 3/3] Update types --- types/index.d.ts | 1 - types/wait-for-dom-change.d.ts | 7 ------- 2 files changed, 8 deletions(-) delete mode 100644 types/wait-for-dom-change.d.ts diff --git a/types/index.d.ts b/types/index.d.ts index 38830ff4..6f77a69c 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -12,7 +12,6 @@ export * from './query-helpers' export * from './screen' export * from './wait' export * from './wait-for' -export * from './wait-for-dom-change' export * from './wait-for-element' export * from './wait-for-element-to-be-removed' export * from './matches' diff --git a/types/wait-for-dom-change.d.ts b/types/wait-for-dom-change.d.ts deleted file mode 100644 index 44a09875..00000000 --- a/types/wait-for-dom-change.d.ts +++ /dev/null @@ -1,7 +0,0 @@ -import {waitForOptions} from './wait-for' - -/** - * @deprecated `waitForDomChange` has been deprecated. - * Use `waitFor` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor. - */ -export function waitForDomChange(options?: waitForOptions): Promise