From 70edb16f035aa5ec5196aa54901bd794d42d4bb3 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Wed, 15 Mar 2023 15:20:38 +0100 Subject: [PATCH 1/5] Flush all pending microtasks and updates before returning from waitFor --- src/__tests__/waitFor.test.tsx | 49 ++++++++++++++++++++++++++++++++++ src/waitFor.ts | 5 +++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/__tests__/waitFor.test.tsx b/src/__tests__/waitFor.test.tsx index 15daf8ac7..fc82979b1 100644 --- a/src/__tests__/waitFor.test.tsx +++ b/src/__tests__/waitFor.test.tsx @@ -263,3 +263,52 @@ test.each([false, true])( expect(mockFn).toHaveBeenCalledTimes(3); } ); + +test.each([false, true])( + 'flushes scheduled updates before returning (fakeTimers = %s)', + async (fakeTimers) => { + if (fakeTimers) { + jest.useFakeTimers(); + } + + function Apple({ onPress }: { onPress: (color: string) => void }) { + const [color, setColor] = React.useState('green'); + const [syncedColor, setSyncedColor] = React.useState(color); + + // On mount, set the color to "red" in a promise microtask + React.useEffect(() => { + Promise.resolve('red').then((c) => setColor(c)); + }, []); + + // Sync the `color` state to `syncedColor` state, but with a delay caused by the effect + React.useEffect(() => { + setSyncedColor(color); + }, [color]); + + return ( + + Apple + {color} + onPress(syncedColor)}> + Trigger + + + ); + } + + const spy = jest.fn(); + const { getByText } = render(); + + // This `waitFor` will succeed on first check, because the "Apple" text is there + // since the initial mount. + await waitFor(() => getByText('Apple')); + + // This `waitFor` will also succeed on first check, because the promise that sets the + // `color` state to "red" resolves right after the previous `await waitFor` statement. + await waitFor(() => getByText('red')); + + // Check that the `onPress` callback is called with the already-updated value of `syncedColor`. + fireEvent.press(getByText('Trigger')); + expect(spy).toHaveBeenCalledWith('red'); + } +); diff --git a/src/waitFor.ts b/src/waitFor.ts index 0077ee45a..d8c0c0d66 100644 --- a/src/waitFor.ts +++ b/src/waitFor.ts @@ -196,7 +196,10 @@ export default async function waitFor( setReactActEnvironment(false); try { - return await waitForInternal(expectation, optionsWithStackTrace); + const result = await waitForInternal(expectation, optionsWithStackTrace); + // drain the microtask queue before restoring the `act` environment + await new Promise((resolve) => setImmediate(resolve)); + return result; } finally { setReactActEnvironment(previousActEnvironment); } From 1c1107473645ce5683fbb124b03ae42177179830 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Sat, 18 Mar 2023 09:54:05 +0100 Subject: [PATCH 2/5] Disable a linter error, its advice is not good --- src/__tests__/waitFor.test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/__tests__/waitFor.test.tsx b/src/__tests__/waitFor.test.tsx index fc82979b1..7cb53eff6 100644 --- a/src/__tests__/waitFor.test.tsx +++ b/src/__tests__/waitFor.test.tsx @@ -277,6 +277,9 @@ test.each([false, true])( // On mount, set the color to "red" in a promise microtask React.useEffect(() => { + // we want to use a Promise, as async functions are not very ergonomic as effects, + // and we're sure we don't need a catch handler. + // eslint-disable-next-line , promise/prefer-await-to-then, promise/catch-or-return Promise.resolve('red').then((c) => setColor(c)); }, []); From 70fc3cde5edd0feb4e418391b04a8ebefbf04b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Jastrze=CC=A8bski?= Date: Thu, 23 Mar 2023 09:39:42 +0100 Subject: [PATCH 3/5] refactor: use `flushMicroTasks` util --- src/flushMicroTasks.ts | 2 +- src/waitFor.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/flushMicroTasks.ts b/src/flushMicroTasks.ts index 011f01d6e..7b964a9d6 100644 --- a/src/flushMicroTasks.ts +++ b/src/flushMicroTasks.ts @@ -2,7 +2,7 @@ import { setImmediate } from './helpers/timers'; type Thenable = { then: (callback: () => T) => unknown }; -export function flushMicroTasks(): Thenable { +export function flushMicroTasks(): Thenable { return { // using "thenable" instead of a Promise, because otherwise it breaks when // using "modern" fake timers diff --git a/src/waitFor.ts b/src/waitFor.ts index d8c0c0d66..58feb224a 100644 --- a/src/waitFor.ts +++ b/src/waitFor.ts @@ -1,6 +1,7 @@ /* globals jest */ import act, { setReactActEnvironment, getIsReactActEnvironment } from './act'; import { getConfig } from './config'; +import { flushMicroTasks } from './flushMicroTasks'; import { ErrorWithStack, copyStackTrace } from './helpers/errors'; import { setTimeout, @@ -197,8 +198,8 @@ export default async function waitFor( try { const result = await waitForInternal(expectation, optionsWithStackTrace); - // drain the microtask queue before restoring the `act` environment - await new Promise((resolve) => setImmediate(resolve)); + // Flush the microtask queue before restoring the `act` environment + await flushMicroTasks(); return result; } finally { setReactActEnvironment(previousActEnvironment); From c229766b171dc72ee3df906905c453e23c706daf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Jastrze=CC=A8bski?= Date: Thu, 23 Mar 2023 09:39:52 +0100 Subject: [PATCH 4/5] refactor: add legacy fake timer test --- src/__tests__/waitFor.test.tsx | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/__tests__/waitFor.test.tsx b/src/__tests__/waitFor.test.tsx index 7cb53eff6..0aa3a2acb 100644 --- a/src/__tests__/waitFor.test.tsx +++ b/src/__tests__/waitFor.test.tsx @@ -264,11 +264,15 @@ test.each([false, true])( } ); -test.each([false, true])( - 'flushes scheduled updates before returning (fakeTimers = %s)', - async (fakeTimers) => { +test.each([ + [false, false], + [true, false], + [true, true], +])( + 'flushes scheduled updates before returning (fakeTimers = %s, legacyFakeTimers = %s)', + async (fakeTimers, legacyFakeTimers) => { if (fakeTimers) { - jest.useFakeTimers(); + jest.useFakeTimers({ legacyFakeTimers }); } function Apple({ onPress }: { onPress: (color: string) => void }) { @@ -289,8 +293,7 @@ test.each([false, true])( }, [color]); return ( - - Apple + {color} onPress(syncedColor)}> Trigger @@ -299,19 +302,19 @@ test.each([false, true])( ); } - const spy = jest.fn(); - const { getByText } = render(); + const onPress = jest.fn(); + const view = render(); - // This `waitFor` will succeed on first check, because the "Apple" text is there + // Required: this `waitFor` will succeed on first check, because the "root" view is there // since the initial mount. - await waitFor(() => getByText('Apple')); + await waitFor(() => view.getByTestId('root')); // This `waitFor` will also succeed on first check, because the promise that sets the // `color` state to "red" resolves right after the previous `await waitFor` statement. - await waitFor(() => getByText('red')); + await waitFor(() => view.getByText('red')); // Check that the `onPress` callback is called with the already-updated value of `syncedColor`. - fireEvent.press(getByText('Trigger')); - expect(spy).toHaveBeenCalledWith('red'); + fireEvent.press(view.getByText('Trigger')); + expect(onPress).toHaveBeenCalledWith('red'); } ); From ac9597b612536ce23ee1014f8dc59dbd7ac9f602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Jastrze=CC=A8bski?= Date: Thu, 23 Mar 2023 09:53:23 +0100 Subject: [PATCH 5/5] refactor: tweaks --- src/__tests__/waitFor.test.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/__tests__/waitFor.test.tsx b/src/__tests__/waitFor.test.tsx index 0aa3a2acb..6e73fb3a5 100644 --- a/src/__tests__/waitFor.test.tsx +++ b/src/__tests__/waitFor.test.tsx @@ -281,9 +281,7 @@ test.each([ // On mount, set the color to "red" in a promise microtask React.useEffect(() => { - // we want to use a Promise, as async functions are not very ergonomic as effects, - // and we're sure we don't need a catch handler. - // eslint-disable-next-line , promise/prefer-await-to-then, promise/catch-or-return + // eslint-disable-next-line promise/prefer-await-to-then, promise/catch-or-return Promise.resolve('red').then((c) => setColor(c)); }, []); @@ -302,7 +300,7 @@ test.each([ ); } - const onPress = jest.fn(); + const onPress = jest.fn(); const view = render(); // Required: this `waitFor` will succeed on first check, because the "root" view is there