Skip to content

Commit

Permalink
Convert ReactLazy-test to waitFor pattern (#26304)
Browse files Browse the repository at this point in the history
I'm in the process of codemodding our test suite to the waitFor pattern.
See #26285 for full context.

This module required a lot of manual changes so I'm doing it as its own
PR. The reason is that most of the tests involved simulating an async
import by wrapping them in `Promise.resolve()`, which means they would
immediately resolve the next time the microtask queue was flushed. I
rewrote the tests to resolve the simulated import explicitly.

While converting these tests, I also realized that the `waitFor` helpers
weren't properly waiting for the entire microtask queue to recursively
finish — if a microtask schedules another microtask, the subsequent one
wouldn't fire until after `waitFor` had resolved. To fix this, I used
the same strategy as `act` — wait for a real task to finish before
proceeding, such as a message event.
  • Loading branch information
acdlite authored Mar 4, 2023
1 parent 03462cf commit 9a52cc8
Show file tree
Hide file tree
Showing 6 changed files with 305 additions and 262 deletions.
33 changes: 23 additions & 10 deletions packages/internal-test-utils/ReactInternalTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import * as SchedulerMock from 'scheduler/unstable_mock';
import {diff} from 'jest-diff';
import {equals} from '@jest/expect-utils';
import enqueueTask from './enqueueTask';

function assertYieldsWereCleared(Scheduler) {
const actualYields = Scheduler.unstable_clearYields();
Expand All @@ -22,6 +23,12 @@ function assertYieldsWereCleared(Scheduler) {
}
}

async function waitForMicrotasks() {
return new Promise(resolve => {
enqueueTask(() => resolve());
});
}

export async function waitFor(expectedLog) {
assertYieldsWereCleared(SchedulerMock);

Expand All @@ -33,7 +40,7 @@ export async function waitFor(expectedLog) {
const actualLog = [];
do {
// Wait until end of current task/microtask.
await null;
await waitForMicrotasks();
if (SchedulerMock.unstable_hasPendingWork()) {
SchedulerMock.unstable_flushNumberOfYields(
expectedLog.length - actualLog.length,
Expand All @@ -44,7 +51,7 @@ export async function waitFor(expectedLog) {
} else {
// Once we've reached the expected sequence, wait one more microtask to
// flush any remaining synchronous work.
await null;
await waitForMicrotasks();
actualLog.push(...SchedulerMock.unstable_clearYields());
break;
}
Expand Down Expand Up @@ -72,11 +79,11 @@ export async function waitForAll(expectedLog) {
// Create the error object before doing any async work, to get a better
// stack trace.
const error = new Error();
Error.captureStackTrace(error, waitFor);
Error.captureStackTrace(error, waitForAll);

do {
// Wait until end of current task/microtask.
await null;
await waitForMicrotasks();
if (!SchedulerMock.unstable_hasPendingWork()) {
// There's no pending work, even after a microtask. Stop flushing.
break;
Expand All @@ -103,11 +110,11 @@ export async function waitForThrow(expectedError: mixed) {
// Create the error object before doing any async work, to get a better
// stack trace.
const error = new Error();
Error.captureStackTrace(error, waitFor);
Error.captureStackTrace(error, waitForThrow);

do {
// Wait until end of current task/microtask.
await null;
await waitForMicrotasks();
if (!SchedulerMock.unstable_hasPendingWork()) {
// There's no pending work, even after a microtask. Stop flushing.
error.message = 'Expected something to throw, but nothing did.';
Expand All @@ -119,7 +126,13 @@ export async function waitForThrow(expectedError: mixed) {
if (equals(x, expectedError)) {
return;
}
if (typeof x === 'object' && x !== null && x.message === expectedError) {
if (
typeof expectedError === 'string' &&
typeof x === 'object' &&
x !== null &&
typeof x.message === 'string' &&
x.message.includes(expectedError)
) {
return;
}
error.message = `
Expand All @@ -142,15 +155,15 @@ export async function waitForPaint(expectedLog) {
// Create the error object before doing any async work, to get a better
// stack trace.
const error = new Error();
Error.captureStackTrace(error, waitFor);
Error.captureStackTrace(error, waitForPaint);

// Wait until end of current task/microtask.
await null;
await waitForMicrotasks();
if (SchedulerMock.unstable_hasPendingWork()) {
// Flush until React yields.
SchedulerMock.unstable_flushUntilNextPaint();
// Wait one more microtask to flush any remaining synchronous work.
await null;
await waitForMicrotasks();
}

const actualLog = SchedulerMock.unstable_clearYields();
Expand Down
50 changes: 50 additions & 0 deletions packages/internal-test-utils/enqueueTask.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

let didWarnAboutMessageChannel = false;
let enqueueTaskImpl = null;

// Same as shared/enqeuueTask, but while that one used by the public
// implementation of `act`, this is only used by our internal testing helpers.
export default function enqueueTask(task: () => void): void {
if (enqueueTaskImpl === null) {
try {
// read require off the module object to get around the bundlers.
// we don't want them to detect a require and bundle a Node polyfill.
const requireString = ('require' + Math.random()).slice(0, 7);
const nodeRequire = module && module[requireString];
// assuming we're in node, let's try to get node's
// version of setImmediate, bypassing fake timers if any.
enqueueTaskImpl = nodeRequire.call(module, 'timers').setImmediate;
} catch (_err) {
// we're in a browser
// we can't use regular timers because they may still be faked
// so we try MessageChannel+postMessage instead
enqueueTaskImpl = function (callback: () => void) {
if (__DEV__) {
if (didWarnAboutMessageChannel === false) {
didWarnAboutMessageChannel = true;
if (typeof MessageChannel === 'undefined') {
console['error'](
'This browser does not have a MessageChannel implementation, ' +
'so enqueuing tasks via await act(async () => ...) will fail. ' +
'Please file an issue at https://github.com/facebook/react/issues ' +
'if you encounter this warning.',
);
}
}
}
const channel = new MessageChannel();
channel.port1.onmessage = callback;
channel.port2.postMessage(undefined);
};
}
}
return enqueueTaskImpl(task);
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('ReactCache', () => {
);

if (__DEV__) {
expect(async () => {
await expect(async () => {
await waitForAll(['App', 'Loading...']);
}).toErrorDev([
'Invalid key type. Expected a string, number, symbol, or ' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('ReactIncrementalScheduling', () => {
expect(ReactNoop).toMatchRenderedOutput(<span prop={5} />);
});

it('works on deferred roots in the order they were scheduled', () => {
it('works on deferred roots in the order they were scheduled', async () => {
const {useEffect} = React;
function Text({text}) {
useEffect(() => {
Expand All @@ -114,7 +114,7 @@ describe('ReactIncrementalScheduling', () => {
expect(ReactNoop.getChildrenAsJSX('c')).toEqual('c:1');

// Schedule deferred work in the reverse order
act(async () => {
await act(async () => {
React.startTransition(() => {
ReactNoop.renderToRootWithID(<Text text="c:2" />, 'c');
ReactNoop.renderToRootWithID(<Text text="b:2" />, 'b');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ describe('ReactIncrementalUpdates', () => {
expect(ReactNoop).toMatchRenderedOutput(<span prop="derived state" />);
});

it('regression: does not expire soon due to layout effects in the last batch', () => {
it('regression: does not expire soon due to layout effects in the last batch', async () => {
const {useState, useLayoutEffect} = React;

let setCount;
Expand All @@ -533,7 +533,7 @@ describe('ReactIncrementalUpdates', () => {
return null;
}

act(async () => {
await act(async () => {
React.startTransition(() => {
ReactNoop.render(<App />);
});
Expand Down
Loading

0 comments on commit 9a52cc8

Please sign in to comment.