Skip to content

Commit

Permalink
Codemod tests to waitFor pattern (1/?)
Browse files Browse the repository at this point in the history 
This converts some of our test suite to use the `waitFor` test pattern,
instead of the `expect(Scheduler).toFlushAndYield` pattern. Most of
these changes are automated with jscodeshift, with some slight manual
cleanup in certain cases.

See #26285 for full context.
  • Loading branch information
@acdlite
acdlite committed Mar 3, 2023
1 parent e524467 commit 7774b3e
Show file tree
Hide file tree
Showing 15 changed files with 694 additions and 903 deletions.
9 changes: 5 additions & 4 deletions packages/react-reconciler/src/__tests__/DebugTracing-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
describe('DebugTracing', () => {
let React;
let ReactTestRenderer;
let Scheduler;
let waitForPaint;

let logs;

Expand All @@ -27,7 +27,8 @@ describe('DebugTracing', () => {

React = require('react');
ReactTestRenderer = require('react-test-renderer');
Scheduler = require('scheduler');
const InternalTestUtils = require('internal-test-utils');
waitForPaint = InternalTestUtils.waitForPaint;

logs = [];

Expand Down Expand Up @@ -100,7 +101,7 @@ describe('DebugTracing', () => {
});

// @gate experimental && build === 'development' && enableDebugTracing && enableCPUSuspense
it('should log sync render with CPU suspense', () => {
it('should log sync render with CPU suspense', async () => {
function Example() {
console.log('<Example/>');
return null;
Expand Down Expand Up @@ -129,7 +130,7 @@ describe('DebugTracing', () => {

logs.splice(0);

expect(Scheduler).toFlushUntilNextPaint([]);
await waitForPaint([]);

expect(logs).toEqual([
`group: ⚛️ render (${RETRY_LANE_STRING})`,
Expand Down
64 changes: 35 additions & 29 deletions packages/react-reconciler/src/__tests__/ReactActWarnings-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

let React;
let Scheduler;
let waitForAll;
let assertLog;
let ReactNoop;
let useState;
let act;
Expand All @@ -32,6 +34,10 @@ describe('act warnings', () => {
startTransition = React.startTransition;
getCacheForType = React.unstable_getCacheForType;
caches = [];

const InternalTestUtils = require('internal-test-utils');
waitForAll = InternalTestUtils.waitForAll;
assertLog = InternalTestUtils.assertLog;
});

function createTextCache() {
Expand Down Expand Up @@ -134,17 +140,17 @@ describe('act warnings', () => {
}
}

function withActEnvironment(value, scope) {
async function withActEnvironment(value, scope) {
const prevValue = global.IS_REACT_ACT_ENVIRONMENT;
global.IS_REACT_ACT_ENVIRONMENT = value;
try {
return scope();
return await scope();
} finally {
global.IS_REACT_ACT_ENVIRONMENT = prevValue;
}
}

test('warns about unwrapped updates only if environment flag is enabled', () => {
test('warns about unwrapped updates only if environment flag is enabled', async () => {
let setState;
function App() {
const [state, _setState] = useState(0);
Expand All @@ -154,34 +160,34 @@ describe('act warnings', () => {

const root = ReactNoop.createRoot();
root.render(<App />);
expect(Scheduler).toFlushAndYield([0]);
await waitForAll([0]);
expect(root).toMatchRenderedOutput('0');

// Default behavior. Flag is undefined. No warning.
expect(global.IS_REACT_ACT_ENVIRONMENT).toBe(undefined);
setState(1);
expect(Scheduler).toFlushAndYield([1]);
await waitForAll([1]);
expect(root).toMatchRenderedOutput('1');

// Flag is true. Warn.
withActEnvironment(true, () => {
await withActEnvironment(true, async () => {
expect(() => setState(2)).toErrorDev(
'An update to App inside a test was not wrapped in act',
);
expect(Scheduler).toFlushAndYield([2]);
await waitForAll([2]);
expect(root).toMatchRenderedOutput('2');
});

// Flag is false. No warning.
withActEnvironment(false, () => {
await withActEnvironment(false, async () => {
setState(3);
expect(Scheduler).toFlushAndYield([3]);
await waitForAll([3]);
expect(root).toMatchRenderedOutput('3');
});
});

// @gate __DEV__
test('act warns if the environment flag is not enabled', () => {
test('act warns if the environment flag is not enabled', async () => {
let setState;
function App() {
const [state, _setState] = useState(0);
Expand All @@ -191,7 +197,7 @@ describe('act warnings', () => {

const root = ReactNoop.createRoot();
root.render(<App />);
expect(Scheduler).toFlushAndYield([0]);
await waitForAll([0]);
expect(root).toMatchRenderedOutput('0');

// Default behavior. Flag is undefined. Warn.
Expand All @@ -204,20 +210,20 @@ describe('act warnings', () => {
'The current testing environment is not configured to support act(...)',
{withoutStack: true},
);
expect(Scheduler).toHaveYielded([1]);
assertLog([1]);
expect(root).toMatchRenderedOutput('1');

// Flag is true. Don't warn.
withActEnvironment(true, () => {
await withActEnvironment(true, () => {
act(() => {
setState(2);
});
expect(Scheduler).toHaveYielded([2]);
assertLog([2]);
expect(root).toMatchRenderedOutput('2');
});

// Flag is false. Warn.
withActEnvironment(false, () => {
await withActEnvironment(false, () => {
expect(() => {
act(() => {
setState(1);
Expand All @@ -226,13 +232,13 @@ describe('act warnings', () => {
'The current testing environment is not configured to support act(...)',
{withoutStack: true},
);
expect(Scheduler).toHaveYielded([1]);
assertLog([1]);
expect(root).toMatchRenderedOutput('1');
});
});

test('warns if root update is not wrapped', () => {
withActEnvironment(true, () => {
test('warns if root update is not wrapped', async () => {
await withActEnvironment(true, () => {
const root = ReactNoop.createRoot();
expect(() => root.render('Hi')).toErrorDev(
// TODO: Better error message that doesn't make it look like "Root" is
Expand Down Expand Up @@ -266,18 +272,18 @@ describe('act warnings', () => {
});

// @gate __DEV__
test('warns even if update is synchronous', () => {
test('warns even if update is synchronous', async () => {
let setState;
function App() {
const [state, _setState] = useState(0);
setState = _setState;
return <Text text={state} />;
}

withActEnvironment(true, () => {
await withActEnvironment(true, () => {
const root = ReactNoop.createRoot();
act(() => root.render(<App />));
expect(Scheduler).toHaveYielded([0]);
assertLog([0]);
expect(root).toMatchRenderedOutput('0');

// Even though this update is synchronous, we should still fire a warning,
Expand All @@ -286,14 +292,14 @@ describe('act warnings', () => {
'An update to App inside a test was not wrapped in act(...)',
);

expect(Scheduler).toHaveYielded([1]);
assertLog([1]);
expect(root).toMatchRenderedOutput('1');
});
});

// @gate __DEV__
// @gate enableLegacyCache
test('warns if Suspense retry is not wrapped', () => {
test('warns if Suspense retry is not wrapped', async () => {
function App() {
return (
<Suspense fallback={<Text text="Loading..." />}>
Expand All @@ -302,12 +308,12 @@ describe('act warnings', () => {
);
}

withActEnvironment(true, () => {
await withActEnvironment(true, () => {
const root = ReactNoop.createRoot();
act(() => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded(['Suspend! [Async]', 'Loading...']);
assertLog(['Suspend! [Async]', 'Loading...']);
expect(root).toMatchRenderedOutput('Loading...');

// This is a retry, not a ping, because we already showed a fallback.
Expand All @@ -321,7 +327,7 @@ describe('act warnings', () => {

// @gate __DEV__
// @gate enableLegacyCache
test('warns if Suspense ping is not wrapped', () => {
test('warns if Suspense ping is not wrapped', async () => {
function App({showMore}) {
return (
<Suspense fallback={<Text text="Loading..." />}>
Expand All @@ -330,20 +336,20 @@ describe('act warnings', () => {
);
}

withActEnvironment(true, () => {
await withActEnvironment(true, () => {
const root = ReactNoop.createRoot();
act(() => {
root.render(<App showMore={false} />);
});
expect(Scheduler).toHaveYielded(['(empty)']);
assertLog(['(empty)']);
expect(root).toMatchRenderedOutput('(empty)');

act(() => {
startTransition(() => {
root.render(<App showMore={true} />);
});
});
expect(Scheduler).toHaveYielded(['Suspend! [Async]', 'Loading...']);
assertLog(['Suspend! [Async]', 'Loading...']);
expect(root).toMatchRenderedOutput('(empty)');

// This is a ping, not a retry, because no fallback is showing.
Expand Down
36 changes: 19 additions & 17 deletions packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ let React;
let ReactFeatureFlags;
let ReactNoop;
let Scheduler;
let waitForAll;
let assertLog;
let ReactCache;
let Suspense;
let TextResource;
Expand All @@ -18,6 +20,10 @@ describe('ReactBlockingMode', () => {
ReactCache = require('react-cache');
Suspense = React.Suspense;

const InternalTestUtils = require('internal-test-utils');
waitForAll = InternalTestUtils.waitForAll;
assertLog = InternalTestUtils.assertLog;

TextResource = ReactCache.unstable_createResource(
([text, ms = 0]) => {
return new Promise((resolve, reject) =>
Expand Down Expand Up @@ -52,7 +58,7 @@ describe('ReactBlockingMode', () => {
}
}

it('updates flush without yielding in the next event', () => {
it('updates flush without yielding in the next event', async () => {
const root = ReactNoop.createRoot();

root.render(
Expand All @@ -66,12 +72,11 @@ describe('ReactBlockingMode', () => {
// Nothing should have rendered yet
expect(root).toMatchRenderedOutput(null);

// Everything should render immediately in the next event
expect(Scheduler).toFlushAndYield(['A', 'B', 'C']);
await waitForAll(['A', 'B', 'C']);
expect(root).toMatchRenderedOutput('ABC');
});

it('layout updates flush synchronously in same event', () => {
it('layout updates flush synchronously in same event', async () => {
const {useLayoutEffect} = React;

function App() {
Expand All @@ -84,9 +89,9 @@ describe('ReactBlockingMode', () => {
const root = ReactNoop.createRoot();
root.render(<App />);
expect(root).toMatchRenderedOutput(null);
expect(Scheduler).toHaveYielded([]);
assertLog([]);

expect(Scheduler).toFlushAndYield(['Hi', 'Layout effect']);
await waitForAll(['Hi', 'Layout effect']);
expect(root).toMatchRenderedOutput('Hi');
});

Expand All @@ -106,15 +111,15 @@ describe('ReactBlockingMode', () => {
</Suspense>,
);

expect(Scheduler).toFlushAndYield(['A', 'Suspend! [B]', 'C', 'Loading...']);
await waitForAll(['A', 'Suspend! [B]', 'C', 'Loading...']);
// In Legacy Mode, A and B would mount in a hidden primary tree. In
// Concurrent Mode, nothing in the primary tree should mount. But the
// fallback should mount immediately.
expect(root).toMatchRenderedOutput('Loading...');

await jest.advanceTimersByTime(1000);
expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
expect(Scheduler).toFlushAndYield(['A', 'B', 'C']);
assertLog(['Promise resolved [B]']);
await waitForAll(['A', 'B', 'C']);
expect(root).toMatchRenderedOutput(
<>
<span>A</span>
Expand All @@ -124,7 +129,7 @@ describe('ReactBlockingMode', () => {
);
});

it('flushSync does not flush batched work', () => {
it('flushSync does not flush batched work', async () => {
const {useState, forwardRef, useImperativeHandle} = React;
const root = ReactNoop.createRoot();

Expand All @@ -143,8 +148,7 @@ describe('ReactBlockingMode', () => {
</>,
);

// Mount
expect(Scheduler).toFlushAndYield(['A0', 'B0']);
await waitForAll(['A0', 'B0']);
expect(root).toMatchRenderedOutput('A0B0');

// Schedule a batched update to the first sibling
Expand All @@ -159,15 +163,13 @@ describe('ReactBlockingMode', () => {

// Now flush the first update
if (gate(flags => flags.enableUnifiedSyncLane)) {
expect(Scheduler).toHaveYielded(['A1', 'B1']);
assertLog(['A1', 'B1']);
expect(root).toMatchRenderedOutput('A1B1');
} else {
// Only the second update should have flushed synchronously
expect(Scheduler).toHaveYielded(['B1']);
assertLog(['B1']);
expect(root).toMatchRenderedOutput('A0B1');

// Now flush the first update
expect(Scheduler).toFlushAndYield(['A1']);
await waitForAll(['A1']);
expect(root).toMatchRenderedOutput('A1B1');
}
});
Expand Down
Loading

0 comments on commit 7774b3e

Please sign in to comment.