Skip to content

Commit

Permalink
Codemod tests to waitFor pattern (1/?) (#26288)
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 committed Mar 3, 2023
1 parent e524467 commit 25a8b97
Show file tree
Hide file tree
Showing 15 changed files with 698 additions and 905 deletions.
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
68 changes: 37 additions & 31 deletions packages/react-reconciler/src/__tests__/ReactActWarnings-test.js
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 All @@ -244,7 +250,7 @@ describe('act warnings', () => {
});

// @gate __DEV__
test('warns if class update is not wrapped', () => {
test('warns if class update is not wrapped', async () => {
let app;
class App extends React.Component {
state = {count: 0};
Expand All @@ -254,7 +260,7 @@ describe('act warnings', () => {
}
}

withActEnvironment(true, () => {
await withActEnvironment(true, () => {
const root = ReactNoop.createRoot();
act(() => {
root.render(<App />);
Expand All @@ -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
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 25a8b97

Please sign in to comment.