From b2f3968d13c640aa1934475374fe1cdb6be75e44 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sat, 8 Oct 2022 13:54:28 +0200 Subject: [PATCH 1/5] Add intended behavior --- src/__tests__/end-to-end.js | 197 +++++++++++++++++++++++++----------- 1 file changed, 137 insertions(+), 60 deletions(-) diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index cf222aec..bad1528b 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -1,73 +1,150 @@ import * as React from 'react' import {render, waitForElementToBeRemoved, screen, waitFor} from '../' -const fetchAMessage = () => - new Promise(resolve => { - // 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() * 100) - setTimeout(() => { - resolve({returnedMessage: 'Hello World'}) - }, randomTimeout) - }) - -function ComponentWithLoader() { - const [state, setState] = React.useState({data: undefined, loading: true}) - React.useEffect(() => { - let cancelled = false - fetchAMessage().then(data => { - if (!cancelled) { - setState({data, loading: false}) - } +describe.each([ + ['real timers', () => jest.useRealTimers()], + ['fake legacy timers', () => jest.useFakeTimers('legacy')], + ['fake modern timers', () => jest.useFakeTimers('modern')], +])( + 'it waits for the data to be loaded in a macrotask using %s', + (label, useTimers) => { + beforeEach(() => { + useTimers() + }) + + afterEach(() => { + jest.useRealTimers() }) - return () => { - cancelled = true + const fetchAMessageInAMacrotask = () => + new Promise(resolve => { + // 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() * 100) + setTimeout(() => { + resolve({returnedMessage: 'Hello World'}) + }, randomTimeout) + }) + + function ComponentWithMacrotaskLoader() { + const [state, setState] = React.useState({data: undefined, loading: true}) + React.useEffect(() => { + let cancelled = false + fetchAMessageInAMacrotask().then(data => { + if (!cancelled) { + setState({data, loading: false}) + } + }) + + return () => { + cancelled = true + } + }, []) + + if (state.loading) { + return
Loading...
+ } + + return ( +
+ Loaded this message: {state.data.returnedMessage}! +
+ ) } - }, []) - if (state.loading) { - return
Loading...
- } + test('waitForElementToBeRemoved', async () => { + render() + const loading = () => screen.getByText('Loading...') + await waitForElementToBeRemoved(loading) + expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) + }) + + test('waitFor', async () => { + render() + // eslint-disable-next-line testing-library/prefer-find-by -- Sir, this is a test. + await waitFor(() => screen.getByText(/Loading../)) + // eslint-disable-next-line testing-library/prefer-find-by -- Sir, this is a test. + await waitFor(() => screen.getByText(/Loaded this message:/)) + expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) + }) - return ( -
- Loaded this message: {state.data.returnedMessage}! -
- ) -} + test('findBy', async () => { + render() + await expect(screen.findByTestId('message')).resolves.toHaveTextContent( + /Hello World/, + ) + }) + }, +) describe.each([ ['real timers', () => jest.useRealTimers()], ['fake legacy timers', () => jest.useFakeTimers('legacy')], ['fake modern timers', () => jest.useFakeTimers('modern')], -])('it waits for the data to be loaded using %s', (label, useTimers) => { - beforeEach(() => { - useTimers() - }) - - afterEach(() => { - jest.useRealTimers() - }) - - test('waitForElementToBeRemoved', async () => { - render() - const loading = () => screen.getByText('Loading...') - await waitForElementToBeRemoved(loading) - expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) - }) - - test('waitFor', async () => { - render() - const message = () => screen.getByText(/Loaded this message:/) - await waitFor(message) - expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) - }) - - test('findBy', async () => { - render() - await expect(screen.findByTestId('message')).resolves.toHaveTextContent( - /Hello World/, - ) - }) -}) +])( + 'it waits for the data to be loaded in a microtask using %s', + (label, useTimers) => { + beforeEach(() => { + useTimers() + }) + + afterEach(() => { + jest.useRealTimers() + }) + + const fetchAMessageInAMicrotask = () => + Promise.resolve({ + status: 200, + json: () => Promise.resolve({title: 'Hello World'}), + }) + + function ComponentWithMicrotaskLoader() { + const [fetchState, setFetchState] = React.useState({fetching: true}) + + React.useEffect(() => { + if (fetchState.fetching) { + fetchAMessageInAMicrotask().then(res => { + return res.json().then(data => { + setFetchState({todo: data.title, fetching: false}) + }) + }) + } + }, [fetchState]) + + if (fetchState.fetching) { + return

Loading..

+ } + + return ( +
Loaded this message: {fetchState.todo}
+ ) + } + + test('waitForElementToBeRemoved', async () => { + render() + const loading = () => screen.getByText('Loading..') + await waitForElementToBeRemoved(loading) + expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) + }) + + test('waitFor', async () => { + render() + await waitFor(() => { + // eslint-disable-next-line testing-library/prefer-explicit-assert -- Sir, this is a test. + screen.getByText('Loading..') + }) + await waitFor(() => { + // eslint-disable-next-line testing-library/prefer-explicit-assert -- Sir, this is a test. + screen.getByText(/Loaded this message:/) + }) + expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) + }) + + test('findBy', async () => { + render() + await expect(screen.findByTestId('message')).resolves.toHaveTextContent( + /Hello World/, + ) + }) + }, +) From d0da54cc9636aa6c1d76b42b39199c152314cb19 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sat, 8 Oct 2022 13:54:40 +0200 Subject: [PATCH 2/5] fix: Prevent "missing act" warning for in-flight promises --- src/pure.js | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/pure.js b/src/pure.js index 94b3b2bd..af90a273 100644 --- a/src/pure.js +++ b/src/pure.js @@ -12,6 +12,19 @@ import act, { } from './act-compat' import {fireEvent} from './fire-event' +function jestFakeTimersAreEnabled() { + /* 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 +} + configureDTL({ unstable_advanceTimersWrapper: cb => { return act(cb) @@ -23,7 +36,21 @@ configureDTL({ const previousActEnvironment = getIsReactActEnvironment() setReactActEnvironment(false) try { - return await cb() + const result = await cb() + // Drain microtask queue. + // Otherwise we'll restore the previous act() environment, before we resolve the `waitFor` call. + // The caller would have no chance to wrap the in-flight Promises in `act()` + await new Promise(resolve => { + setTimeout(() => { + resolve() + }, 0) + + if (jestFakeTimersAreEnabled()) { + jest.advanceTimersByTime(0) + } + }) + + return result } finally { setReactActEnvironment(previousActEnvironment) } From d2940a5a93883a9f97551c3fbf73a23f5c84dd6d Mon Sep 17 00:00:00 2001 From: eps1lon Date: Thu, 16 Feb 2023 11:27:10 +0100 Subject: [PATCH 3/5] Disable TL lint rules in tests --- package.json | 2 ++ src/__tests__/end-to-end.js | 4 ---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index b475390a..973f4a48 100644 --- a/package.json +++ b/package.json @@ -82,6 +82,8 @@ "testing-library/no-debugging-utils": "off", "testing-library/no-dom-import": "off", "testing-library/no-unnecessary-act": "off", + "testing-library/prefer-explicit-assert": "off", + "testing-library/prefer-find-by": "off", "testing-library/prefer-user-event": "off" } }, diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index bad1528b..9f0d1e3c 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -61,9 +61,7 @@ describe.each([ test('waitFor', async () => { render() - // eslint-disable-next-line testing-library/prefer-find-by -- Sir, this is a test. await waitFor(() => screen.getByText(/Loading../)) - // eslint-disable-next-line testing-library/prefer-find-by -- Sir, this is a test. await waitFor(() => screen.getByText(/Loaded this message:/)) expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) }) @@ -130,11 +128,9 @@ describe.each([ test('waitFor', async () => { render() await waitFor(() => { - // eslint-disable-next-line testing-library/prefer-explicit-assert -- Sir, this is a test. screen.getByText('Loading..') }) await waitFor(() => { - // eslint-disable-next-line testing-library/prefer-explicit-assert -- Sir, this is a test. screen.getByText(/Loaded this message:/) }) expect(screen.getByTestId('message')).toHaveTextContent(/Hello World/) From d935fb1233c40d5aba366fdf0f1d6df4b17af85d Mon Sep 17 00:00:00 2001 From: eps1lon Date: Thu, 16 Feb 2023 11:29:46 +0100 Subject: [PATCH 4/5] Implementation without macrotask --- src/pure.js | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/src/pure.js b/src/pure.js index af90a273..39c02cc3 100644 --- a/src/pure.js +++ b/src/pure.js @@ -12,19 +12,6 @@ import act, { } from './act-compat' import {fireEvent} from './fire-event' -function jestFakeTimersAreEnabled() { - /* 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 -} - configureDTL({ unstable_advanceTimersWrapper: cb => { return act(cb) @@ -40,15 +27,7 @@ configureDTL({ // Drain microtask queue. // Otherwise we'll restore the previous act() environment, before we resolve the `waitFor` call. // The caller would have no chance to wrap the in-flight Promises in `act()` - await new Promise(resolve => { - setTimeout(() => { - resolve() - }, 0) - - if (jestFakeTimersAreEnabled()) { - jest.advanceTimersByTime(0) - } - }) + await Promise.resolve().then(() => {}) return result } finally { From bfd1db76c7dc95de0f410390d44aa3537b4a1378 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Thu, 16 Feb 2023 11:37:08 +0100 Subject: [PATCH 5/5] Now I member --- src/__tests__/end-to-end.js | 24 +++++++++++++++++++++--- src/pure.js | 24 +++++++++++++++++++++++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/__tests__/end-to-end.js b/src/__tests__/end-to-end.js index 9f0d1e3c..005591d3 100644 --- a/src/__tests__/end-to-end.js +++ b/src/__tests__/end-to-end.js @@ -102,9 +102,27 @@ describe.each([ React.useEffect(() => { if (fetchState.fetching) { fetchAMessageInAMicrotask().then(res => { - return res.json().then(data => { - setFetchState({todo: data.title, fetching: false}) - }) + return ( + res + .json() + // By spec, the runtime can only yield back to the event loop once + // the microtask queue is empty. + // So we ensure that we actually wait for that as well before yielding back from `waitFor`. + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => data) + .then(data => { + setFetchState({todo: data.title, fetching: false}) + }) + ) }) } }, [fetchState]) diff --git a/src/pure.js b/src/pure.js index 39c02cc3..845aede1 100644 --- a/src/pure.js +++ b/src/pure.js @@ -12,6 +12,20 @@ import act, { } from './act-compat' import {fireEvent} from './fire-event' +function jestFakeTimersAreEnabled() { + /* istanbul ignore else */ + if (typeof jest !== 'undefined' && jest !== null) { + return ( + // legacy timers + setTimeout._isMockFunction === true || // modern timers + // eslint-disable-next-line prefer-object-has-own -- No Object.hasOwn in all target environments we support. + Object.prototype.hasOwnProperty.call(setTimeout, 'clock') + ) + } // istanbul ignore next + + return false +} + configureDTL({ unstable_advanceTimersWrapper: cb => { return act(cb) @@ -27,7 +41,15 @@ configureDTL({ // Drain microtask queue. // Otherwise we'll restore the previous act() environment, before we resolve the `waitFor` call. // The caller would have no chance to wrap the in-flight Promises in `act()` - await Promise.resolve().then(() => {}) + await new Promise(resolve => { + setTimeout(() => { + resolve() + }, 0) + + if (jestFakeTimersAreEnabled()) { + jest.advanceTimersByTime(0) + } + }) return result } finally {