From 42a93e4b3986611b25b760d99d345b4f95e7361d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20De=20Boey?= Date: Wed, 26 Apr 2023 12:56:40 +0200 Subject: [PATCH 01/12] chore: remove `styfle/cancel-workflow-action` usage (#4) --- .github/workflows/validate.yml | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 041f8f4..6b5280a 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -11,14 +11,17 @@ on: - 'beta' - 'alpha' - '!all-contributors/**' - pull_request: {} + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true permissions: {} jobs: main: permissions: - actions: write # to cancel/stop running workflows (styfle/cancel-workflow-action) contents: read # to fetch code (actions/checkout) # ignore all-contributors PRs if: ${{ !contains(github.head_ref, 'all-contributors') }} @@ -29,9 +32,6 @@ jobs: node: [14, 16, 18] runs-on: ubuntu-latest steps: - - name: 🛑 Cancel Previous Runs - uses: styfle/cancel-workflow-action@0.9.0 - - name: ⬇️ Checkout repo uses: actions/checkout@v3 with: @@ -63,7 +63,6 @@ jobs: release: permissions: - actions: write # to cancel/stop running workflows (styfle/cancel-workflow-action) contents: write # to create release tags (cycjimmy/semantic-release-action) needs: main @@ -72,9 +71,6 @@ jobs: ${{ github.repository == 'testing-library/web-testing-library' && github.event_name == 'push' }} steps: - - name: 🛑 Cancel Previous Runs - uses: styfle/cancel-workflow-action@0.9.0 - - name: ⬇️ Checkout repo uses: actions/checkout@v3 From 6f6f5a8a760959467f993eea96316b5ee5a5c450 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 3 Oct 2023 10:43:00 +0200 Subject: [PATCH 02/12] Setup Codesandbox CI (#5) --- .codesandbox/ci.json | 5 +++++ .github/workflows/validate.yml | 11 ++++++++++- package.json | 4 ++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 .codesandbox/ci.json diff --git a/.codesandbox/ci.json b/.codesandbox/ci.json new file mode 100644 index 0000000..002bafb --- /dev/null +++ b/.codesandbox/ci.json @@ -0,0 +1,5 @@ +{ + "installCommand": "install:csb", + "sandboxes": ["new", "github/kentcdodds/react-testing-library-examples"], + "node": "18" +} diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 6b5280a..cd107a5 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -29,7 +29,7 @@ jobs: # Otherwise we would not know if the problem is tied to the Node.js version fail-fast: false matrix: - node: [14, 16, 18] + node: [14, 16, 18, 20] runs-on: ubuntu-latest steps: - name: ⬇️ Checkout repo @@ -43,6 +43,10 @@ jobs: with: node-version: ${{ matrix.node }} + # Ideally done by actions/setup-node: https://github.com/actions/setup-node/issues/213 + - name: Setup package manager + run: npm install -g npm@9.2.0 + - name: 📥 Download deps uses: bahmutov/npm-install@v1 with: @@ -64,6 +68,7 @@ jobs: release: permissions: contents: write # to create release tags (cycjimmy/semantic-release-action) + issues: write # to post release that resolves an issue needs: main runs-on: ubuntu-latest @@ -79,6 +84,10 @@ jobs: with: node-version: 14 + # Ideally done by actions/setup-node: https://github.com/actions/setup-node/issues/213 + - name: Setup package manager + run: npm install -g npm@9.2.0 + - name: 📥 Download deps uses: bahmutov/npm-install@v1 with: diff --git a/package.json b/package.json index 903ed09..b28b4a9 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,10 @@ "kcd-scripts": "^11.0.0", "typescript": "^4.1.2" }, + "overrides": { + "browserslist": "4.21.8", + "caniuse-lite": "1.0.30001502" + }, "eslintConfig": { "extends": [ "./node_modules/kcd-scripts/eslint.js", From d86732eefd40c4922687061205023b36dd02d56b Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sat, 14 Jan 2023 10:56:15 +0100 Subject: [PATCH 03/12] feat: Rough sketch for API (includes untested implementation) --- jest.config.js | 10 +- package.json | 1 + src/__tests__/waitFor.test.js | 5 - src/__tests__/waitForDOM.test.js | 164 +++++++++++++++++++++++++++++ src/__tests__/waitForNode.js | 49 +++++++++ src/index.ts | 6 +- src/waitFor.tsx | 172 +++++++++++++++++++++++++++++++ tests/jest.config.dom.js | 13 --- tests/jest.config.js | 7 ++ tests/jest.config.node.js | 15 --- 10 files changed, 398 insertions(+), 44 deletions(-) delete mode 100644 src/__tests__/waitFor.test.js create mode 100644 src/__tests__/waitForDOM.test.js create mode 100644 src/__tests__/waitForNode.js create mode 100644 src/waitFor.tsx delete mode 100644 tests/jest.config.dom.js create mode 100644 tests/jest.config.js delete mode 100644 tests/jest.config.node.js diff --git a/jest.config.js b/jest.config.js index 7a7a2b5..45a27e7 100644 --- a/jest.config.js +++ b/jest.config.js @@ -7,18 +7,14 @@ const { module.exports = { collectCoverageFrom, - coveragePathIgnorePatterns: [ - ...coveragePathIgnorePatterns, - '/__tests__/', - '/__node_tests__/', - ], + coveragePathIgnorePatterns: [...coveragePathIgnorePatterns, '/__tests__/'], coverageThreshold, watchPlugins: [ ...watchPlugins, require.resolve('jest-watch-select-projects'), ], projects: [ - require.resolve('./tests/jest.config.dom.js'), - require.resolve('./tests/jest.config.node.js'), + // No idea why I need to specify a project instead of having a single config + require.resolve('./tests/jest.config.js'), ], } diff --git a/package.json b/package.json index b28b4a9..d96e241 100644 --- a/package.json +++ b/package.json @@ -68,6 +68,7 @@ "jest-watch-select-projects": "^2.0.0", "jsdom": "^16.4.0", "kcd-scripts": "^11.0.0", + "pretty-format": "^29.3.1", "typescript": "^4.1.2" }, "overrides": { diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js deleted file mode 100644 index eff5936..0000000 --- a/src/__tests__/waitFor.test.js +++ /dev/null @@ -1,5 +0,0 @@ -import {waitFor} from '../' - -test('runs', async () => { - await expect(waitFor(() => {})).resolves.toBeUndefined() -}) diff --git a/src/__tests__/waitForDOM.test.js b/src/__tests__/waitForDOM.test.js new file mode 100644 index 0000000..6c1484b --- /dev/null +++ b/src/__tests__/waitForDOM.test.js @@ -0,0 +1,164 @@ +/** + * @jest-environment jsdom + */ + +import * as prettyFormat from 'pretty-format' +import {waitFor as waitForWeb} from '../' + +function jestFakeTimersAreEnabled() { + /* istanbul ignore else */ + // eslint-disable-next-line + 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 +} + +function getWindowFromNode(node) { + if (node.defaultView) { + // node is document + return node.defaultView + } else if (node.ownerDocument && node.ownerDocument.defaultView) { + // node is a DOM node + return node.ownerDocument.defaultView + } else { + // node is window + return node.window + } +} + +/** + * Reference implementation of `waitFor` when a DOM is available. + * Supports fake timers and configureable instrumentation. + */ +function waitFor( + callback, + { + container = document, + interval = 50, + mutationObserverOptions = { + subtree: true, + childList: true, + attributes: true, + characterData: true, + }, + timeout = 1000, + } = {}, +) { + function getElementError(message) { + const prettifiedDOM = prettyFormat(container) + const error = new Error( + [message, prettifiedDOM].filter(Boolean).join('\n\n'), + ) + error.name = 'TestingLibraryElementError' + return error + } + + function handleTimeout(error) { + error.message = getElementError(error.message).message + return error + } + + function advanceTimersWrapper(cb) { + // /dom config. /react uses act() here + return cb() + } + + function runWithExpensiveErrorDiagnosticsDisabled() { + // /dom would disable certain config options when running callback + return callback() + } + + /** @type {import('../').FakeClock} */ + const jestFakeClock = { + advanceTimersByTime: timeoutMS => { + advanceTimersWrapper(() => { + jest.advanceTimersByTime(timeoutMS) + }) + }, + flushPromises: () => { + return advanceTimersWrapper(async () => { + await new Promise(r => { + setTimeout(r, 0) + jest.advanceTimersByTime(0) + }) + }) + }, + } + const clock = jestFakeTimersAreEnabled() ? jestFakeClock : undefined + const controller = new AbortController() + + return new Promise((resolve, reject) => { + let promiseStatus = 'idle' + + function onDone(error, result) { + controller.abort() + if (error === null) { + resolve(result) + } else { + reject(error) + } + } + + function checkCallbackWithExpensiveErrorDiagnosticsDisabled() { + if (promiseStatus === 'pending') return undefined + + const result = runWithExpensiveErrorDiagnosticsDisabled() + if (typeof result?.then === 'function') { + promiseStatus = 'pending' + return result.then( + resolvedValue => { + promiseStatus = 'resolved' + return resolvedValue + }, + rejectedValue => { + promiseStatus = 'rejected' + throw rejectedValue + }, + ) + } + return result + } + + waitForWeb(checkCallbackWithExpensiveErrorDiagnosticsDisabled, { + clock, + interval, + onTimeout: handleTimeout, + signal: controller.signal, + timeout, + }).then( + result => { + onDone(null, result) + }, + error => { + // https://webidl.spec.whatwg.org/#idl-DOMException + // https://dom.spec.whatwg.org/#ref-for-dom-abortcontroller-abortcontroller%E2%91%A0 + const isAbortError = + error.name === 'AbortError' && error.code === DOMException.ABORT_ERR + // Ignore abort errors + if (!isAbortError) { + onDone(error, null) + } + }, + ) + + const {MutationObserver} = getWindowFromNode(container) + const observer = new MutationObserver( + checkCallbackWithExpensiveErrorDiagnosticsDisabled, + ) + observer.observe(container, mutationObserverOptions) + controller.signal.addEventListener('abort', () => { + observer.disconnect() + }) + }) +} + +test('runs', async () => { + await expect(waitFor(() => {})).resolves.toBeUndefined() +}) diff --git a/src/__tests__/waitForNode.js b/src/__tests__/waitForNode.js new file mode 100644 index 0000000..9c83b50 --- /dev/null +++ b/src/__tests__/waitForNode.js @@ -0,0 +1,49 @@ +/** + * @jest-environment node + */ + +import {waitFor as waitForWeb} from '../' + +function jestFakeTimersAreEnabled() { + /* istanbul ignore else */ + // eslint-disable-next-line + 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 +} + +/** + * Reference implementation of `waitFor` that supports Jest fake timers + */ +function waitFor(callback, {interval = 50, timeout = 1000} = {}) { + /** @type {import('../').FakeClock} */ + const jestFakeClock = { + advanceTimersByTime: timeoutMS => { + jest.advanceTimersByTime(timeoutMS) + }, + flushPromises: () => { + return new Promise(r => { + setTimeout(r, 0) + jest.advanceTimersByTime(0) + }) + }, + } + const clock = jestFakeTimersAreEnabled() ? jestFakeClock : undefined + + return waitForWeb(callback, { + clock, + interval, + timeout, + }) +} + +test('runs', async () => { + await expect(waitFor(() => {})).resolves.toBeUndefined() +}) diff --git a/src/index.ts b/src/index.ts index 67efc62..db8295f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,2 @@ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- TODO -export function waitFor(fn: () => Promise): Promise { - return Promise.resolve() -} +export {default as waitFor} from './waitFor' +export type {FakeClock, WaitForOptions} from './waitFor' diff --git a/src/waitFor.tsx b/src/waitFor.tsx new file mode 100644 index 0000000..c79a0fa --- /dev/null +++ b/src/waitFor.tsx @@ -0,0 +1,172 @@ +// This is so the stack trace the developer sees is one that's +// closer to their code (because async stack traces are hard to follow). +function copyStackTrace(target: Error, source: Error) { + if (source.stack !== undefined) { + target.stack = source.stack.replace(source.message, target.message) + } +} + +export interface FakeClock { + advanceTimersByTime: (timeoutMS: number) => void + flushPromises: () => Promise +} + +export interface WaitForOptions { + clock?: FakeClock + /** + * @default 50 + */ + interval?: number + onTimeout?: (error: unknown) => unknown + signal?: AbortSignal + /** + * @default false + */ + showOriginalStackTrace?: boolean + /** + * @default 1000 + */ + timeout?: number +} + +interface WaitForImplOptions extends WaitForOptions { + stackTraceError: Error +} + +function waitForImpl( + callback: () => T | Promise, + { + clock, + timeout = 1000, + showOriginalStackTrace = false, + stackTraceError, + interval = 50, + onTimeout = error => { + return error + }, + signal, + }: WaitForImplOptions, +) { + if (typeof callback !== 'function') { + throw new TypeError('Received `callback` arg must be a function') + } + + return new Promise(async (resolve, reject) => { + let lastError: unknown + let finished = false + let promiseStatus = 'idle' + + const overallTimeoutTimer = setTimeout(handleTimeout, timeout) + const intervalId = setInterval(checkCallback, interval) + + signal?.addEventListener('abort', () => { + onDone(new Error(`Aborted: ${signal.reason}`), null) + }) + + checkCallback() + + if (clock !== undefined) { + // 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, @typescript-eslint/no-unnecessary-condition + while (!finished && !signal?.aborted) { + clock.advanceTimersByTime(interval) + + // It's really important that checkCallback 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. + checkCallback() + + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- No it isn't + if (finished) { + break + } + + // 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 clock.flushPromises() + } + } + + function onDone(error: null, result: unknown): void + function onDone(error: unknown, result: null): void + function onDone(error: null | unknown, result: null | unknown) { + finished = true + clearTimeout(overallTimeoutTimer) + clearInterval(intervalId) + + if (error) { + reject(error) + } else { + resolve(result) + } + } + + function checkCallback() { + if (promiseStatus === 'pending') return + try { + const result = callback() + if ( + result !== null && + typeof result === 'object' && + typeof (result as any).then === 'function' + ) { + const thenable = result as PromiseLike + promiseStatus = 'pending' + thenable.then( + resolvedValue => { + promiseStatus = 'resolved' + onDone(null, resolvedValue) + }, + rejectedValue => { + promiseStatus = 'rejected' + lastError = rejectedValue + }, + ) + } else { + onDone(null, result) + } + // If `callback` throws, wait for the next mutation, interval, or timeout. + } catch (error: unknown) { + // Save the most recent callback error to reject the promise with it in the event of a timeout + lastError = error + } + } + + function handleTimeout() { + let error: Error + if (lastError) { + error = lastError as Error + // TODO: Why special casing this name `TestingLibraryElementError`? + if ( + !showOriginalStackTrace && + error.name === 'TestingLibraryElementError' + ) { + copyStackTrace(error, stackTraceError) + } + } else { + error = new Error('Timed out in waitFor.') + if (!showOriginalStackTrace) { + copyStackTrace(error, stackTraceError) + } + } + onDone(onTimeout(error), null) + } + }) +} + +export default function waitFor( + callback: () => void | Promise, + options: WaitForOptions, +) { + // create the error here so its stack trace is as close to the + // calling code as possible + const stackTraceError = new Error('STACK_TRACE_MESSAGE') + return waitForImpl(callback, {stackTraceError, ...options}) +} diff --git a/tests/jest.config.dom.js b/tests/jest.config.dom.js deleted file mode 100644 index ad0bea1..0000000 --- a/tests/jest.config.dom.js +++ /dev/null @@ -1,13 +0,0 @@ -const path = require('path') -const baseConfig = require('kcd-scripts/jest') - -module.exports = { - ...baseConfig, - rootDir: path.join(__dirname, '..'), - displayName: 'jsdom', - coveragePathIgnorePatterns: [ - ...baseConfig.coveragePathIgnorePatterns, - '/__tests__/', - ], - testEnvironment: 'jest-environment-jsdom', -} diff --git a/tests/jest.config.js b/tests/jest.config.js new file mode 100644 index 0000000..9afa536 --- /dev/null +++ b/tests/jest.config.js @@ -0,0 +1,7 @@ +const path = require('path') +const baseConfig = require('kcd-scripts/jest') + +module.exports = { + ...baseConfig, + rootDir: path.join(__dirname, '..'), +} diff --git a/tests/jest.config.node.js b/tests/jest.config.node.js deleted file mode 100644 index bf37b60..0000000 --- a/tests/jest.config.node.js +++ /dev/null @@ -1,15 +0,0 @@ -const path = require('path') -const baseConfig = require('kcd-scripts/jest') - -module.exports = { - ...baseConfig, - rootDir: path.join(__dirname, '..'), - displayName: 'node', - testEnvironment: 'jest-environment-node', - coveragePathIgnorePatterns: [ - ...baseConfig.coveragePathIgnorePatterns, - '/__tests__/', - '/__node_tests__/', - ], - testMatch: ['**/__node_tests__/**.js'], -} From 92ef85c3237faa1484ef000d35894ed2261eed1f Mon Sep 17 00:00:00 2001 From: eps1lon Date: Tue, 7 Feb 2023 17:57:07 +0100 Subject: [PATCH 04/12] Can be in .ts --- src/{waitFor.tsx => waitFor.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/{waitFor.tsx => waitFor.ts} (100%) diff --git a/src/waitFor.tsx b/src/waitFor.ts similarity index 100% rename from src/waitFor.tsx rename to src/waitFor.ts From 045294b468f0da0c6b75e7fd69a95f0105c3c899 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 8 Feb 2023 17:35:17 +0100 Subject: [PATCH 05/12] Test reference impl --- .../__snapshots__/waitForDOM.test.js.snap | 67 +++++++++++++ src/__tests__/waitForDOM.test.js | 99 ++++++++++++++++--- 2 files changed, 154 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/__snapshots__/waitForDOM.test.js.snap diff --git a/src/__tests__/__snapshots__/waitForDOM.test.js.snap b/src/__tests__/__snapshots__/waitForDOM.test.js.snap new file mode 100644 index 0000000..e31f124 --- /dev/null +++ b/src/__tests__/__snapshots__/waitForDOM.test.js.snap @@ -0,0 +1,67 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`waitFor DOM reference implementation using fake legacy timers timeout 1`] = ` +Not done + +Document { + "location": Location { + "assign": [Function assign], + "hash": "", + "host": "localhost", + "hostname": "localhost", + "href": "http://localhost/", + "origin": "http://localhost", + "pathname": "/", + "port": "", + "protocol": "http:", + "reload": [Function reload], + "replace": [Function replace], + "search": "", + "toString": [Function toString], + }, +} +`; + +exports[`waitFor DOM reference implementation using fake modern timers timeout 1`] = ` +Not done + +Document { + "location": Location { + "assign": [Function assign], + "hash": "", + "host": "localhost", + "hostname": "localhost", + "href": "http://localhost/", + "origin": "http://localhost", + "pathname": "/", + "port": "", + "protocol": "http:", + "reload": [Function reload], + "replace": [Function replace], + "search": "", + "toString": [Function toString], + }, +} +`; + +exports[`waitFor DOM reference implementation using real timers timeout 1`] = ` +Not done + +Document { + "location": Location { + "assign": [Function assign], + "hash": "", + "host": "localhost", + "hostname": "localhost", + "href": "http://localhost/", + "origin": "http://localhost", + "pathname": "/", + "port": "", + "protocol": "http:", + "reload": [Function reload], + "replace": [Function replace], + "search": "", + "toString": [Function toString], + }, +} +`; diff --git a/src/__tests__/waitForDOM.test.js b/src/__tests__/waitForDOM.test.js index 6c1484b..b2b22c8 100644 --- a/src/__tests__/waitForDOM.test.js +++ b/src/__tests__/waitForDOM.test.js @@ -52,7 +52,7 @@ function waitFor( } = {}, ) { function getElementError(message) { - const prettifiedDOM = prettyFormat(container) + const prettifiedDOM = prettyFormat.format(container) const error = new Error( [message, prettifiedDOM].filter(Boolean).join('\n\n'), ) @@ -126,6 +126,22 @@ function waitFor( return result } + const {MutationObserver} = getWindowFromNode(container) + const observer = new MutationObserver(() => { + const result = checkCallbackWithExpensiveErrorDiagnosticsDisabled() + if (typeof result?.then === 'function') { + result.then(resolvedValue => { + onDone(null, resolvedValue) + }) + } else { + onDone(null, result) + } + }) + observer.observe(container, mutationObserverOptions) + controller.signal.addEventListener('abort', () => { + observer.disconnect() + }) + waitForWeb(checkCallbackWithExpensiveErrorDiagnosticsDisabled, { clock, interval, @@ -147,18 +163,77 @@ function waitFor( } }, ) - - const {MutationObserver} = getWindowFromNode(container) - const observer = new MutationObserver( - checkCallbackWithExpensiveErrorDiagnosticsDisabled, - ) - observer.observe(container, mutationObserverOptions) - controller.signal.addEventListener('abort', () => { - observer.disconnect() - }) }) } -test('runs', async () => { - await expect(waitFor(() => {})).resolves.toBeUndefined() +describe.each([ + ['real timers', () => jest.useRealTimers()], + ['fake legacy timers', () => jest.useFakeTimers('legacy')], + ['fake modern timers', () => jest.useFakeTimers('modern')], +])('waitFor DOM reference implementation using %s', (label, useTimers) => { + beforeEach(() => { + useTimers() + }) + + afterEach(() => { + jest.useRealTimers() + }) + + test('void callback', async () => { + await expect(waitFor(() => {})).resolves.toBeUndefined() + }) + + test('callback passes after timeout', async () => { + let state = 'pending' + setTimeout(() => { + state = 'done' + }, 10) + + await expect( + waitFor( + () => { + if (state !== 'done') { + throw new Error('Not done') + } + }, + {interval: 5}, + ), + ).resolves.toBeUndefined() + }) + + test('timeout', async () => { + const state = 'pending' + + await expect( + waitFor( + () => { + if (state !== 'done') { + throw new Error('Not done') + } + }, + {timeout: 10}, + ), + ).rejects.toThrowErrorMatchingSnapshot() + }) + + test('can resolve early due to mutations', async () => { + const container = document.createElement('div') + + setTimeout(() => { + container.appendChild(document.createTextNode('Done')) + }, 50) + + const p = waitFor( + () => { + if (container.textContent !== 'Done') { + throw new Error('Not done') + } + return container.textContent + }, + // this would never resolve with real timers without using a MutationObserver + {container, interval: 200, timeout: 200}, + ) + + await expect(p).resolves.toBe('Done') + }) }) From 39a57d0f8fe49602f19a62b9996f3b5808e943f6 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Thu, 9 Feb 2023 11:55:45 +0100 Subject: [PATCH 06/12] Full test suite --- src/__tests__/waitForNode.js | 193 ++++++++++++++++++++++++++++++++++- src/waitFor.ts | 7 -- 2 files changed, 190 insertions(+), 10 deletions(-) diff --git a/src/__tests__/waitForNode.js b/src/__tests__/waitForNode.js index 9c83b50..2e97cfb 100644 --- a/src/__tests__/waitForNode.js +++ b/src/__tests__/waitForNode.js @@ -4,6 +4,18 @@ import {waitFor as waitForWeb} from '../' +function sleep(timeoutMS, signal) { + return new Promise((resolve, reject) => { + const timeoutID = setTimeout(() => { + resolve() + }, timeoutMS) + signal?.addEventListener('abort', reason => { + clearTimeout(timeoutID) + reject(reason) + }) + }) +} + function jestFakeTimersAreEnabled() { /* istanbul ignore else */ // eslint-disable-next-line @@ -22,7 +34,7 @@ function jestFakeTimersAreEnabled() { /** * Reference implementation of `waitFor` that supports Jest fake timers */ -function waitFor(callback, {interval = 50, timeout = 1000} = {}) { +function waitFor(callback, options) { /** @type {import('../').FakeClock} */ const jestFakeClock = { advanceTimersByTime: timeoutMS => { @@ -39,11 +51,186 @@ function waitFor(callback, {interval = 50, timeout = 1000} = {}) { return waitForWeb(callback, { clock, - interval, - timeout, + ...options, }) } +// TODO: Use jest.replaceProperty(global, 'Error', ErrorWithoutStack) and `jest.restoreAllMocks` +let originalError +beforeEach(() => { + originalError = global.Error +}) +afterEach(() => { + global.Error = originalError +}) + test('runs', async () => { await expect(waitFor(() => {})).resolves.toBeUndefined() }) + +test('ensures the given callback is a function', () => { + expect(() => waitFor(null)).toThrowErrorMatchingInlineSnapshot( + `Received \`callback\` arg must be a function`, + ) +}) + +describe('using fake modern timers', () => { + beforeEach(() => { + jest.useFakeTimers('modern') + }) + afterEach(() => { + jest.useRealTimers() + }) + + test('times out after 1s by default', async () => { + let resolved = false + setTimeout(() => { + resolved = true + }, 1000) + + await expect( + waitFor(() => { + if (!resolved) { + throw new Error('Not resolved') + } + }), + ).rejects.toThrowErrorMatchingInlineSnapshot(`Not resolved`) + }) + + test('times out even if the callback never settled', async () => { + await expect( + waitFor(() => { + return new Promise(() => {}) + }), + ).rejects.toThrowErrorMatchingInlineSnapshot(`Timed out in waitFor.`) + }) + + test('callback can return a promise and is not called again until the promise resolved', async () => { + const callback = jest.fn(() => { + return sleep(20) + }) + + await expect(waitFor(callback, {interval: 1})).resolves.toBeUndefined() + // We configured the waitFor call to ping every 1ms. + // But the callback only resolved after 20ms. + // If we would ping as instructed, we'd have 20+1 calls (1 initial, 20 for pings). + // But the implementation waits for callback to resolve first before checking again. + expect(callback).toHaveBeenCalledTimes(1) + }) + + test('callback is not called again until the promise rejects', async () => { + const callback = jest.fn(async () => { + await sleep(20) + throw new Error('Not done') + }) + + await expect( + waitFor(callback, {interval: 1, timeout: 30}), + ).rejects.toThrowErrorMatchingInlineSnapshot(`Not done`) + // We configured the waitFor call to ping every 1ms. + // But the callback only rejected after 20ms. + // If we would ping as instructed, we'd have 30+1 calls (1 initial, 30 for pings until timeout was reached). + // But the implementation waits for callback to resolve first before checking again. + // So we have 1 for the initial check (that takes 20ms) and one for an interval check after the initial check resolved. + // Next ping would happen at 40ms but we already timed out at this point + expect(callback).toHaveBeenCalledTimes(2) + }) + + test('massages the stack trace to point to the waitFor call not the callback call', async () => { + let waitForError + try { + await waitFor( + () => { + return sleep(100) + }, + {showOriginalStackTrace: false, interval: 100, timeout: 1}, + ) + } catch (caughtError) { + waitForError = caughtError + } + + const stackTrace = waitForError.stack.split('\n').slice(1) + // The earlier a stackframe points to the actual callsite the better + const testStackFrame = stackTrace[1] + const fileLocationRegexp = /\((.*):\d+:\d+\)$/ + expect(testStackFrame).toMatch(fileLocationRegexp) + const [, fileLocation] = testStackFrame.match(fileLocationRegexp) + expect(fileLocation).toBe(__filename) + + expect(waitForError.stack).toMatchInlineSnapshot(` + Error: Timed out in waitFor. + at waitFor (/src/waitFor.ts:163:27) + at waitFor (/src/__tests__/waitForNode.js:52:20) + at Object. (/src/__tests__/waitForNode.js:142:13) + at Promise.then.completed (/node_modules/jest-circus/build/utils.js:391:28) + at new Promise () + at callAsyncCircusFn (/node_modules/jest-circus/build/utils.js:316:10) + at _callCircusTest (/node_modules/jest-circus/build/run.js:218:40) + at processTicksAndRejections (node:internal/process/task_queues:96:5) + at _runTest (/node_modules/jest-circus/build/run.js:155:3) + at _runTestsForDescribeBlock (/node_modules/jest-circus/build/run.js:66:9) + `) + }) + + test('does not crash in runtimes without Error.prototype.stack', async () => { + class ErrorWithoutStack extends Error { + // Not the same as "not having" but close enough + // stack a non-standard property so we have to guard against stack not existing + stack = undefined + } + const originalGlobalError = global.Error + global.Error = ErrorWithoutStack + let waitForError + try { + await waitFor( + () => { + return sleep(100) + }, + {interval: 100, timeout: 1}, + ) + } catch (caughtError) { + waitForError = caughtError + } + // Restore early so that Jest can use Error.prototype.stack again + // Still need global restore in case something goes wrong. + global.Error = originalGlobalError + + // Feel free to update this snapshot. + // It's only used to highlight how bad the default stack trace is if we timeout + // The only frame pointing to this test is the one from the wrapper. + // An actual test would not have any frames pointing to this test. + expect(waitForError.stack).toBeUndefined() + }) + + test('can be configured to throw an error with the original stack trace', async () => { + let waitForError + try { + await waitFor( + () => { + return sleep(100) + }, + {showOriginalStackTrace: true, interval: 100, timeout: 1}, + ) + } catch (caughtError) { + waitForError = caughtError + } + + // Feel free to update this snapshot. + // It's only used to highlight how bad the default stack trace is if we timeout + // The only frame pointing to this test is the one from the wrapper. + // An actual test would not have any frames pointing to this test. + expect(waitForError.stack).toMatchInlineSnapshot(` + Error: Timed out in waitFor. + at handleTimeout (/src/waitFor.ts:147:17) + at callTimer (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:729:24) + at doTickInner (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1289:29) + at doTick (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1370:20) + at Object.tick (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1378:20) + at FakeTimers.advanceTimersByTime (/node_modules/@jest/fake-timers/build/modernFakeTimers.js:101:19) + at Object.advanceTimersByTime (/node_modules/jest-runtime/build/index.js:2228:26) + at Object.advanceTimersByTime (/src/__tests__/waitForNode.js:41:12) + at /src/waitFor.ts:75:15 + at new Promise () + `) + }) +}) diff --git a/src/waitFor.ts b/src/waitFor.ts index c79a0fa..4b3bce3 100644 --- a/src/waitFor.ts +++ b/src/waitFor.ts @@ -143,13 +143,6 @@ function waitForImpl( let error: Error if (lastError) { error = lastError as Error - // TODO: Why special casing this name `TestingLibraryElementError`? - if ( - !showOriginalStackTrace && - error.name === 'TestingLibraryElementError' - ) { - copyStackTrace(error, stackTraceError) - } } else { error = new Error('Timed out in waitFor.') if (!showOriginalStackTrace) { From 066b255724dcc35b3123974c6ff741eba97a9d93 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Thu, 9 Feb 2023 12:12:01 +0100 Subject: [PATCH 07/12] Ensure aborted signal works --- src/__tests__/waitForNode.js | 54 +++++++++++++++++++++++++----------- src/waitFor.ts | 17 ++++++------ 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/__tests__/waitForNode.js b/src/__tests__/waitForNode.js index 2e97cfb..f81c050 100644 --- a/src/__tests__/waitForNode.js +++ b/src/__tests__/waitForNode.js @@ -156,20 +156,6 @@ describe('using fake modern timers', () => { expect(testStackFrame).toMatch(fileLocationRegexp) const [, fileLocation] = testStackFrame.match(fileLocationRegexp) expect(fileLocation).toBe(__filename) - - expect(waitForError.stack).toMatchInlineSnapshot(` - Error: Timed out in waitFor. - at waitFor (/src/waitFor.ts:163:27) - at waitFor (/src/__tests__/waitForNode.js:52:20) - at Object. (/src/__tests__/waitForNode.js:142:13) - at Promise.then.completed (/node_modules/jest-circus/build/utils.js:391:28) - at new Promise () - at callAsyncCircusFn (/node_modules/jest-circus/build/utils.js:316:10) - at _callCircusTest (/node_modules/jest-circus/build/run.js:218:40) - at processTicksAndRejections (node:internal/process/task_queues:96:5) - at _runTest (/node_modules/jest-circus/build/run.js:155:3) - at _runTestsForDescribeBlock (/node_modules/jest-circus/build/run.js:66:9) - `) }) test('does not crash in runtimes without Error.prototype.stack', async () => { @@ -221,7 +207,7 @@ describe('using fake modern timers', () => { // An actual test would not have any frames pointing to this test. expect(waitForError.stack).toMatchInlineSnapshot(` Error: Timed out in waitFor. - at handleTimeout (/src/waitFor.ts:147:17) + at handleTimeout (/src/waitFor.ts:146:17) at callTimer (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:729:24) at doTickInner (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1289:29) at doTick (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1370:20) @@ -229,8 +215,44 @@ describe('using fake modern timers', () => { at FakeTimers.advanceTimersByTime (/node_modules/@jest/fake-timers/build/modernFakeTimers.js:101:19) at Object.advanceTimersByTime (/node_modules/jest-runtime/build/index.js:2228:26) at Object.advanceTimersByTime (/src/__tests__/waitForNode.js:41:12) - at /src/waitFor.ts:75:15 + at /src/waitFor.ts:80:15 at new Promise () `) }) + + test('can be aborted with an AbortSignal', async () => { + const callback = jest.fn(() => { + throw new Error('not done') + }) + const controller = new AbortController() + const waitForError = waitFor(callback, { + signal: controller.signal, + }) + + controller.abort('Bailing out') + + await expect(waitForError).rejects.toThrowErrorMatchingInlineSnapshot( + `Aborted: Bailing out`, + ) + // Initial check + one ping (after which we yield which gives us a chance to advance to the controller.abort call) + expect(callback).toHaveBeenCalledTimes(2) + }) + + test('does not even ping if the signal is already aborted', async () => { + const callback = jest.fn(() => { + throw new Error('not done') + }) + const controller = new AbortController() + controller.abort('Bailing out') + + const waitForError = waitFor(callback, { + signal: controller.signal, + }) + + await expect(waitForError).rejects.toThrowErrorMatchingInlineSnapshot( + `Aborted: Bailing out`, + ) + // Just the initial check + expect(callback).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/waitFor.ts b/src/waitFor.ts index 4b3bce3..5b662bf 100644 --- a/src/waitFor.ts +++ b/src/waitFor.ts @@ -59,9 +59,14 @@ function waitForImpl( const overallTimeoutTimer = setTimeout(handleTimeout, timeout) const intervalId = setInterval(checkCallback, interval) - signal?.addEventListener('abort', () => { - onDone(new Error(`Aborted: ${signal.reason}`), null) - }) + if (signal !== undefined) { + if (signal.aborted) { + onDone(new Error(`Aborted: ${signal.reason}`), null) + } + signal.addEventListener('abort', () => { + onDone(new Error(`Aborted: ${signal.reason}`), null) + }) + } checkCallback() @@ -74,12 +79,6 @@ function waitForImpl( while (!finished && !signal?.aborted) { clock.advanceTimersByTime(interval) - // It's really important that checkCallback 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. - checkCallback() - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- No it isn't if (finished) { break From 28ee0ba6f648adf6b571d99793d41c14db08066d Mon Sep 17 00:00:00 2001 From: eps1lon Date: Thu, 9 Feb 2023 12:20:55 +0100 Subject: [PATCH 08/12] Gate AbortController tests We also test Node.js 14 which doesn't support AbortController --- src/__tests__/waitForNode.js | 38 +++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/__tests__/waitForNode.js b/src/__tests__/waitForNode.js index f81c050..c5c8d83 100644 --- a/src/__tests__/waitForNode.js +++ b/src/__tests__/waitForNode.js @@ -74,6 +74,9 @@ test('ensures the given callback is a function', () => { ) }) +const testAbortController = + typeof AbortController === 'undefined' ? test.skip : test + describe('using fake modern timers', () => { beforeEach(() => { jest.useFakeTimers('modern') @@ -220,7 +223,7 @@ describe('using fake modern timers', () => { `) }) - test('can be aborted with an AbortSignal', async () => { + testAbortController('can be aborted with an AbortSignal', async () => { const callback = jest.fn(() => { throw new Error('not done') }) @@ -238,21 +241,24 @@ describe('using fake modern timers', () => { expect(callback).toHaveBeenCalledTimes(2) }) - test('does not even ping if the signal is already aborted', async () => { - const callback = jest.fn(() => { - throw new Error('not done') - }) - const controller = new AbortController() - controller.abort('Bailing out') + testAbortController( + 'does not even ping if the signal is already aborted', + async () => { + const callback = jest.fn(() => { + throw new Error('not done') + }) + const controller = new AbortController() + controller.abort('Bailing out') - const waitForError = waitFor(callback, { - signal: controller.signal, - }) + const waitForError = waitFor(callback, { + signal: controller.signal, + }) - await expect(waitForError).rejects.toThrowErrorMatchingInlineSnapshot( - `Aborted: Bailing out`, - ) - // Just the initial check - expect(callback).toHaveBeenCalledTimes(1) - }) + await expect(waitForError).rejects.toThrowErrorMatchingInlineSnapshot( + `Aborted: Bailing out`, + ) + // Just the initial check + expect(callback).toHaveBeenCalledTimes(1) + }, + ) }) From 221ad91811aa5e1c8c7ef31680500d16fc42865c Mon Sep 17 00:00:00 2001 From: eps1lon Date: Thu, 9 Feb 2023 12:56:59 +0100 Subject: [PATCH 09/12] Lower coverage threshhold Jest is not suited for this check since has no knowledge about the build matrix. Codecov should check this instead --- jest.config.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/jest.config.js b/jest.config.js index 45a27e7..2571e6a 100644 --- a/jest.config.js +++ b/jest.config.js @@ -8,7 +8,17 @@ const { module.exports = { collectCoverageFrom, coveragePathIgnorePatterns: [...coveragePathIgnorePatterns, '/__tests__/'], - coverageThreshold, + coverageThreshold: { + ...coverageThreshold, + // full coverage across the build matrix (Node.js versions) but not in a single job + // minimum coverage of jobs using different Node.js version + './src/waitFor.ts': { + branches: 96.77, + functions: 100, + lines: 97.95, + statements: 98, + }, + }, watchPlugins: [ ...watchPlugins, require.resolve('jest-watch-select-projects'), From 69b474baca65fd528cb667a566fabe759c10335b Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 3 Oct 2023 09:56:48 +0200 Subject: [PATCH 10/12] Port tests from waitFor DOM --- .../__snapshots__/waitFor.test.js.snap | 7 + src/__tests__/waitFor.test.js | 247 ++++++++++++++++++ 2 files changed, 254 insertions(+) create mode 100644 src/__tests__/__snapshots__/waitFor.test.js.snap create mode 100644 src/__tests__/waitFor.test.js diff --git a/src/__tests__/__snapshots__/waitFor.test.js.snap b/src/__tests__/__snapshots__/waitFor.test.js.snap new file mode 100644 index 0000000..307a7f6 --- /dev/null +++ b/src/__tests__/__snapshots__/waitFor.test.js.snap @@ -0,0 +1,7 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`waitFor DOM reference implementation using fake legacy timers timeout 1`] = `Not done`; + +exports[`waitFor DOM reference implementation using fake modern timers timeout 1`] = `Not done`; + +exports[`waitFor DOM reference implementation using real timers timeout 1`] = `Not done`; diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js new file mode 100644 index 0000000..7fa247b --- /dev/null +++ b/src/__tests__/waitFor.test.js @@ -0,0 +1,247 @@ +/** + * @jest-environment node + */ + +import * as prettyFormat from 'pretty-format' +import {waitFor} from '../' + +function deferred() { + let resolve, reject + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return {promise, resolve, reject} +} + +beforeEach(() => { + jest.useRealTimers() +}) + +test('waits callback to not throw an error', async () => { + const spy = jest.fn() + // we are using random timeout here to simulate a real-time example + // of an async operation calling a callback at a non-deterministic time + const randomTimeout = Math.floor(Math.random() * 60) + setTimeout(spy, randomTimeout) + + await waitFor(() => expect(spy).toHaveBeenCalledTimes(1)) + 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( + () => { + throw error + }, + {timeout: 8, interval: 5}, + ).catch(e => e) + expect(result).toBe(error) +}) + +test('if no error is thrown then throws a timeout error', async () => { + const result = await waitFor( + () => { + // eslint-disable-next-line no-throw-literal + throw undefined + }, + {timeout: 8, interval: 5, onTimeout: e => e}, + ).catch(e => e) + expect(result).toMatchInlineSnapshot(`[Error: Timed out in waitFor.]`) +}) + +test('if showOriginalStackTrace on a timeout error then the stack trace does not include this file', async () => { + const result = await waitFor( + () => { + // eslint-disable-next-line no-throw-literal + throw undefined + }, + {timeout: 8, interval: 5, showOriginalStackTrace: true}, + ).catch(e => e) + expect(result.stack).not.toMatch(__dirname) +}) + +test('uses full stack error trace when showOriginalStackTrace present', async () => { + const error = new Error('Throws the full stack trace') + // even if the error is a TestingLibraryElementError + error.name = 'TestingLibraryElementError' + const originalStackTrace = error.stack + const result = await waitFor( + () => { + throw error + }, + {timeout: 8, interval: 5, showOriginalStackTrace: true}, + ).catch(e => e) + expect(result.stack).toBe(originalStackTrace) +}) + +test('throws nice error if provided callback is not a function', () => { + const someElement = {} + expect(() => waitFor(someElement)).toThrow( + 'Received `callback` arg must be a function', + ) +}) + +test('when a promise is returned, it does not call the callback again until that promise rejects', async () => { + const sleep = t => new Promise(r => setTimeout(r, t)) + const p1 = deferred() + const waitForCb = jest.fn(() => p1.promise) + const waitForPromise = waitFor(waitForCb, {interval: 1}) + expect(waitForCb).toHaveBeenCalledTimes(1) + waitForCb.mockClear() + await sleep(50) + expect(waitForCb).toHaveBeenCalledTimes(0) + + const p2 = deferred() + waitForCb.mockImplementation(() => p2.promise) + + p1.reject('p1 rejection (should not fail this test)') + await sleep(50) + + expect(waitForCb).toHaveBeenCalledTimes(1) + p2.resolve() + + await waitForPromise +}) + +test('when a promise is returned, if that is not resolved within the timeout, then waitFor is rejected', async () => { + const sleep = t => new Promise(r => setTimeout(r, t)) + const {promise} = deferred() + const waitForError = waitFor(() => promise, {timeout: 1}).catch(e => e) + await sleep(5) + + expect((await waitForError).message).toMatchInlineSnapshot( + `Timed out in waitFor.`, + ) +}) + +test('does not work after it resolves', async () => { + jest.useFakeTimers('modern') + let context = 'initial' + + /** @type {import('../').FakeClock} */ + const clock = { + // @testing-library/react usage to ensure `IS_REACT_ACT_ENVIRONMENT` is set when acting. + advanceTimersByTime: async timeoutMS => { + const originalContext = context + context = 'act' + try { + jest.advanceTimersByTime(timeoutMS) + } finally { + context = originalContext + } + }, + flushPromises: async () => { + const originalContext = context + context = 'no-act' + try { + await await new Promise(r => { + setTimeout(r, 0) + jest.advanceTimersByTime(0) + }) + } finally { + context = originalContext + } + }, + } + + let data = null + setTimeout(() => { + data = 'resolved' + }, 100) + + await waitFor( + () => { + // eslint-disable-next-line jest/no-conditional-in-test -- false-positive + if (data === null) { + throw new Error('not found') + } + }, + {clock, interval: 50}, + ) + + expect(context).toEqual('initial') + + await Promise.resolve() + + expect(context).toEqual('initial') +}) + +/** @type {import('../').FakeClock} */ +const jestFakeClock = { + advanceTimersByTime: timeoutMS => { + jest.advanceTimersByTime(timeoutMS) + }, + flushPromises: () => { + return new Promise(r => { + setTimeout(r, 0) + jest.advanceTimersByTime(0) + }) + }, +} +describe.each([ + ['real timers', {useTimers: () => jest.useRealTimers(), clock: undefined}], + [ + 'fake legacy timers', + {useTimers: () => jest.useFakeTimers('legacy'), clock: jestFakeClock}, + ], + [ + 'fake modern timers', + {useTimers: () => jest.useFakeTimers('modern'), clock: jestFakeClock}, + ], +])( + 'waitFor DOM reference implementation using %s', + (label, {useTimers, clock}) => { + beforeEach(() => { + useTimers() + }) + + afterEach(() => { + jest.useRealTimers() + }) + + test('void callback', async () => { + await expect(waitFor(() => {}, {clock})).resolves.toBeUndefined() + }) + + test('callback passes after timeout', async () => { + let state = 'pending' + setTimeout(() => { + state = 'done' + }, 10) + + await expect( + waitFor( + () => { + if (state !== 'done') { + throw new Error('Not done') + } + }, + {clock, interval: 5}, + ), + ).resolves.toBeUndefined() + }) + + test('timeout', async () => { + const state = 'pending' + + await expect( + waitFor( + () => { + if (state !== 'done') { + throw new Error('Not done') + } + }, + {clock, timeout: 10}, + ), + ).rejects.toThrowErrorMatchingSnapshot() + }) + }, +) From 75b76f68b66eb452dd37b2922f1e6b75fe29364e Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 3 Oct 2023 10:28:02 +0200 Subject: [PATCH 11/12] Unify advanceTimersByTime and flushPromises Ports https://github.com/testing-library/dom-testing-library/pull/1229 --- src/__tests__/waitFor.test.js | 8 +------- src/__tests__/waitForDOM.test.js | 15 ++++++--------- .../{waitForNode.js => waitForNode.test.js} | 14 ++++---------- src/waitFor.ts | 11 ++--------- 4 files changed, 13 insertions(+), 35 deletions(-) rename src/__tests__/{waitForNode.js => waitForNode.test.js} (96%) diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js index 7fa247b..1533de0 100644 --- a/src/__tests__/waitFor.test.js +++ b/src/__tests__/waitFor.test.js @@ -176,15 +176,9 @@ test('does not work after it resolves', async () => { /** @type {import('../').FakeClock} */ const jestFakeClock = { - advanceTimersByTime: timeoutMS => { + advanceTimersByTime: async timeoutMS => { jest.advanceTimersByTime(timeoutMS) }, - flushPromises: () => { - return new Promise(r => { - setTimeout(r, 0) - jest.advanceTimersByTime(0) - }) - }, } describe.each([ ['real timers', {useTimers: () => jest.useRealTimers(), clock: undefined}], diff --git a/src/__tests__/waitForDOM.test.js b/src/__tests__/waitForDOM.test.js index b2b22c8..0ab9d17 100644 --- a/src/__tests__/waitForDOM.test.js +++ b/src/__tests__/waitForDOM.test.js @@ -65,6 +65,11 @@ function waitFor( return error } + /** + * @template T + * @param {() => T} cb + * @returns T + */ function advanceTimersWrapper(cb) { // /dom config. /react uses act() here return cb() @@ -78,16 +83,8 @@ function waitFor( /** @type {import('../').FakeClock} */ const jestFakeClock = { advanceTimersByTime: timeoutMS => { - advanceTimersWrapper(() => { - jest.advanceTimersByTime(timeoutMS) - }) - }, - flushPromises: () => { return advanceTimersWrapper(async () => { - await new Promise(r => { - setTimeout(r, 0) - jest.advanceTimersByTime(0) - }) + jest.advanceTimersByTime(timeoutMS) }) }, } diff --git a/src/__tests__/waitForNode.js b/src/__tests__/waitForNode.test.js similarity index 96% rename from src/__tests__/waitForNode.js rename to src/__tests__/waitForNode.test.js index c5c8d83..2bbc744 100644 --- a/src/__tests__/waitForNode.js +++ b/src/__tests__/waitForNode.test.js @@ -37,15 +37,9 @@ function jestFakeTimersAreEnabled() { function waitFor(callback, options) { /** @type {import('../').FakeClock} */ const jestFakeClock = { - advanceTimersByTime: timeoutMS => { + advanceTimersByTime: async timeoutMS => { jest.advanceTimersByTime(timeoutMS) }, - flushPromises: () => { - return new Promise(r => { - setTimeout(r, 0) - jest.advanceTimersByTime(0) - }) - }, } const clock = jestFakeTimersAreEnabled() ? jestFakeClock : undefined @@ -210,15 +204,15 @@ describe('using fake modern timers', () => { // An actual test would not have any frames pointing to this test. expect(waitForError.stack).toMatchInlineSnapshot(` Error: Timed out in waitFor. - at handleTimeout (/src/waitFor.ts:146:17) + at handleTimeout (/src/waitFor.ts:139:17) at callTimer (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:729:24) at doTickInner (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1289:29) at doTick (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1370:20) at Object.tick (/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1378:20) at FakeTimers.advanceTimersByTime (/node_modules/@jest/fake-timers/build/modernFakeTimers.js:101:19) at Object.advanceTimersByTime (/node_modules/jest-runtime/build/index.js:2228:26) - at Object.advanceTimersByTime (/src/__tests__/waitForNode.js:41:12) - at /src/waitFor.ts:80:15 + at Object.advanceTimersByTime (/src/__tests__/waitForNode.test.js:41:12) + at /src/waitFor.ts:85:21 at new Promise () `) }) diff --git a/src/waitFor.ts b/src/waitFor.ts index 5b662bf..0d7a633 100644 --- a/src/waitFor.ts +++ b/src/waitFor.ts @@ -7,7 +7,7 @@ function copyStackTrace(target: Error, source: Error) { } export interface FakeClock { - advanceTimersByTime: (timeoutMS: number) => void + advanceTimersByTime: (timeoutMS: number) => Promise flushPromises: () => Promise } @@ -77,19 +77,12 @@ function waitForImpl( // waiting or when we've timed out. // eslint-disable-next-line no-unmodified-loop-condition, @typescript-eslint/no-unnecessary-condition while (!finished && !signal?.aborted) { - clock.advanceTimersByTime(interval) - - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- No it isn't - if (finished) { - break - } - // 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 clock.flushPromises() + await clock.advanceTimersByTime(interval) } } From 253fe6cd424f61e423779e15c92aa110c09d2ec6 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 3 Oct 2023 10:44:58 +0200 Subject: [PATCH 12/12] format --- src/__tests__/waitForDOM.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/waitForDOM.test.js b/src/__tests__/waitForDOM.test.js index 0ab9d17..4d1e7ff 100644 --- a/src/__tests__/waitForDOM.test.js +++ b/src/__tests__/waitForDOM.test.js @@ -67,7 +67,7 @@ function waitFor( /** * @template T - * @param {() => T} cb + * @param {() => T} cb * @returns T */ function advanceTimersWrapper(cb) {