Skip to content

Commit

Permalink
Codemod tests to waitFor pattern (3/?) (#26299)
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 authored Mar 3, 2023
1 parent ce8a72f commit e64a8f4
Show file tree
Hide file tree
Showing 15 changed files with 406 additions and 400 deletions.
115 changes: 41 additions & 74 deletions packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ let images = [];
let onLoadSpy = null;
let actualLoadSpy = null;

let waitForAll;
let waitFor;
let assertLog;

function PhaseMarkers({children}) {
Scheduler.unstable_yieldValue('render start');
React.useLayoutEffect(() => {
Expand Down Expand Up @@ -94,6 +98,11 @@ describe('ReactDOMImageLoad', () => {
ReactDOMClient = require('react-dom/client');
// Suspense = React.Suspense;

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

onLoadSpy = jest.fn(reactEvent => {
const src = reactEvent.target.getAttribute('src');
Scheduler.unstable_yieldValue('onLoadSpy [' + src + ']');
Expand Down Expand Up @@ -206,26 +215,17 @@ describe('ReactDOMImageLoad', () => {
),
);

expect(Scheduler).toFlushAndYieldThrough([
'render start',
'Img default',
'Yield',
]);
await waitFor(['render start', 'Img default', 'Yield']);
const img = last(images);
loadImage(img);
expect(Scheduler).toHaveYielded([
assertLog([
'actualLoadSpy [default]',
// no onLoadSpy since we have not completed render
]);
expect(Scheduler).toFlushAndYield([
'a',
'load triggered',
'last layout',
'last passive',
]);
await waitForAll(['a', 'load triggered', 'last layout', 'last passive']);
expect(img.__needsDispatch).toBe(true);
loadImage(img);
expect(Scheduler).toHaveYielded([
assertLog([
'actualLoadSpy [default]', // the browser reloading of the image causes this to yield again
'onLoadSpy [default]',
]);
Expand All @@ -244,7 +244,7 @@ describe('ReactDOMImageLoad', () => {
),
);

expect(Scheduler).toFlushAndYieldThrough([
await waitFor([
'render start',
'Img default',
'load triggered',
Expand All @@ -253,11 +253,8 @@ describe('ReactDOMImageLoad', () => {
Scheduler.unstable_requestPaint();
const img = last(images);
loadImage(img);
expect(Scheduler).toHaveYielded([
'actualLoadSpy [default]',
'onLoadSpy [default]',
]);
expect(Scheduler).toFlushAndYield(['last passive']);
assertLog(['actualLoadSpy [default]', 'onLoadSpy [default]']);
await waitForAll(['last passive']);
expect(img.__needsDispatch).toBe(false);
expect(onLoadSpy).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -286,16 +283,12 @@ describe('ReactDOMImageLoad', () => {

React.startTransition(() => root.render(<Base />));

expect(Scheduler).toFlushAndYieldThrough([
'render start',
'Img a',
'Yield',
]);
await waitFor(['render start', 'Img a', 'Yield']);
const img = last(images);
loadImage(img);
expect(Scheduler).toHaveYielded(['actualLoadSpy [a]']);
assertLog(['actualLoadSpy [a]']);

expect(Scheduler).toFlushAndYieldThrough([
await waitFor([
'load triggered',
'last layout',
// the update in layout causes a passive effects flush before a sync render
Expand All @@ -309,7 +302,7 @@ describe('ReactDOMImageLoad', () => {
]);
expect(images.length).toBe(1);
loadImage(img);
expect(Scheduler).toHaveYielded(['actualLoadSpy [b]', 'onLoadSpy [b]']);
assertLog(['actualLoadSpy [b]', 'onLoadSpy [b]']);
expect(onLoadSpy).toHaveBeenCalledTimes(1);
});

Expand All @@ -323,7 +316,7 @@ describe('ReactDOMImageLoad', () => {
</PhaseMarkers>,
);

expect(Scheduler).toFlushAndYield([
await waitForAll([
'render start',
'Img default',
'load triggered',
Expand All @@ -332,10 +325,7 @@ describe('ReactDOMImageLoad', () => {
]);
const img = last(images);
loadImage(img);
expect(Scheduler).toHaveYielded([
'actualLoadSpy [default]',
'onLoadSpy [default]',
]);
assertLog(['actualLoadSpy [default]', 'onLoadSpy [default]']);
expect(onLoadSpy).toHaveBeenCalledTimes(1);
});

Expand Down Expand Up @@ -365,26 +355,17 @@ describe('ReactDOMImageLoad', () => {
),
);

expect(Scheduler).toFlushAndYieldThrough([
'render start',
'Img default',
'Yield',
]);
await waitFor(['render start', 'Img default', 'Yield']);
const img = last(images);
loadImage(img);
expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']);
expect(Scheduler).toFlushAndYield([
'a',
'load triggered',
'last layout',
'last passive',
]);
assertLog(['actualLoadSpy [default]']);
await waitForAll(['a', 'load triggered', 'last layout', 'last passive']);
expect(img.__needsDispatch).toBe(true);
loadImage(img);
// we expect the browser to load the image again but since we are no longer rendering
// the img there will be no onLoad called
expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']);
expect(Scheduler).toFlushWithoutYielding();
assertLog(['actualLoadSpy [default]']);
await waitForAll([]);
expect(onLoadSpy).not.toHaveBeenCalled();
});

Expand Down Expand Up @@ -426,16 +407,16 @@ describe('ReactDOMImageLoad', () => {
),
);

expect(Scheduler).toFlushAndYieldThrough([
await waitFor([
// initial render
'render start',
'Img default',
'Yield',
]);
const img = last(images);
loadImage(img);
expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']);
expect(Scheduler).toFlushAndYield([
assertLog(['actualLoadSpy [default]']);
await waitForAll([
'a',
'load triggered',
// img is present at first
Expand All @@ -450,8 +431,8 @@ describe('ReactDOMImageLoad', () => {
loadImage(img);
// we expect the browser to load the image again but since we are no longer rendering
// the img there will be no onLoad called
expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']);
expect(Scheduler).toFlushWithoutYielding();
assertLog(['actualLoadSpy [default]']);
await waitForAll([]);
expect(onLoadSpy).not.toHaveBeenCalled();
});

Expand Down Expand Up @@ -548,22 +529,18 @@ describe('ReactDOMImageLoad', () => {

root.render(<Base />);

expect(Scheduler).toFlushWithoutYielding();
await waitForAll([]);

React.startTransition(() => externalSetSrc('a'));

expect(Scheduler).toFlushAndYieldThrough([
'YieldingWithImage',
'Img a',
'Yield',
]);
await waitFor(['YieldingWithImage', 'Img a', 'Yield']);
let img = last(images);
loadImage(img);
expect(Scheduler).toHaveYielded(['actualLoadSpy [a]']);
assertLog(['actualLoadSpy [a]']);

ReactDOM.flushSync(() => externalSetSrcAlt('b'));

expect(Scheduler).toHaveYielded([
assertLog([
'YieldingWithImage',
'Img b',
'Yield',
Expand All @@ -576,18 +553,12 @@ describe('ReactDOMImageLoad', () => {
expect(img.__needsDispatch).toBe(true);
loadImage(img);

expect(Scheduler).toHaveYielded(['actualLoadSpy [b]', 'onLoadSpy [b]']);
assertLog(['actualLoadSpy [b]', 'onLoadSpy [b]']);
// why is there another update here?
expect(Scheduler).toFlushAndYield([
'YieldingWithImage',
'Img b',
'Yield',
'b',
'Committed',
]);
await waitForAll(['YieldingWithImage', 'Img b', 'Yield', 'b', 'Committed']);
});

it('preserves the src property / attribute when triggering a potential new load event', () => {
it('preserves the src property / attribute when triggering a potential new load event', async () => {
// this test covers a regression identified in https://github.com/mui/material-ui/pull/31263
// where the resetting of the src property caused the property to change from relative to fully qualified

Expand All @@ -612,17 +583,13 @@ describe('ReactDOMImageLoad', () => {
);

// render to yield to capture state of img src attribute and property before commit
expect(Scheduler).toFlushAndYieldThrough([
'render start',
'Img default',
'Yield',
]);
await waitFor(['render start', 'Img default', 'Yield']);
const img = last(images);
const renderSrcProperty = img.src;
const renderSrcAttr = img.getAttribute('src');

// finish render and commit causing the src property to be rewritten
expect(Scheduler).toFlushAndYield(['a', 'last layout', 'last passive']);
await waitForAll(['a', 'last layout', 'last passive']);
const commitSrcProperty = img.src;
const commitSrcAttr = img.getAttribute('src');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ let ReactDOM;
let ReactDOMClient;
let Scheduler;
let act;
let assertLog;
let waitFor;

describe('ReactDOMNativeEventHeuristic-test', () => {
let container;
Expand All @@ -28,6 +30,10 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
Scheduler = require('scheduler');
act = require('jest-react').act;

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

document.body.appendChild(container);
});

Expand Down Expand Up @@ -301,10 +307,10 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
dispatchAndSetCurrentEvent(target.current, mouseEnterEvent);

// Since mouse end is not discrete, should not have updated yet
expect(Scheduler).toHaveYielded(['not hovered']);
assertLog(['not hovered']);
expect(container.textContent).toEqual('not hovered');

expect(Scheduler).toFlushAndYieldThrough(['hovered']);
await waitFor(['hovered']);
expect(container.textContent).toEqual('hovered');
});
expect(container.textContent).toEqual('hovered');
Expand Down Expand Up @@ -381,7 +387,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
pressEvent.initEvent('click', true, true);
dispatchAndSetCurrentEvent(target, pressEvent);

expect(Scheduler).toHaveYielded(['Count: 0 [after batchedUpdates]']);
assertLog(['Count: 0 [after batchedUpdates]']);
expect(container.textContent).toEqual('Count: 0');

// Intentionally not using `act` so we can observe in between the click
Expand Down
8 changes: 5 additions & 3 deletions packages/react-dom/src/__tests__/ReactDOMNestedEvents-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ describe('ReactDOMNestedEvents', () => {
let Scheduler;
let act;
let useState;
let assertLog;

beforeEach(() => {
jest.resetModules();
Expand All @@ -23,6 +24,9 @@ describe('ReactDOMNestedEvents', () => {
Scheduler = require('scheduler');
act = require('jest-react').act;
useState = React.useState;

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

test('nested event dispatches should not cause updates to flush', async () => {
Expand Down Expand Up @@ -67,9 +71,7 @@ describe('ReactDOMNestedEvents', () => {
await act(async () => {
buttonRef.current.click();
});
expect(Scheduler).toHaveYielded([
'Value right after focus call: Clicked: false, Focused: false',
]);
assertLog(['Value right after focus call: Clicked: false, Focused: false']);
expect(buttonRef.current.innerHTML).toEqual('Clicked: true, Focused: true');
});
});
12 changes: 9 additions & 3 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ let ReactDOMServer = require('react-dom/server');
let Scheduler = require('scheduler');
let act;
let useEffect;
let assertLog;
let waitFor;

describe('ReactDOMRoot', () => {
let container;
Expand All @@ -30,6 +32,10 @@ describe('ReactDOMRoot', () => {
Scheduler = require('scheduler');
act = require('jest-react').act;
useEffect = React.useEffect;

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

it('renders children', () => {
Expand Down Expand Up @@ -255,7 +261,7 @@ describe('ReactDOMRoot', () => {
Scheduler.unstable_yieldValue('callback');
});
expect(container.textContent).toEqual('Hi');
expect(Scheduler).toHaveYielded(['callback']);
assertLog(['callback']);
});

it('warns when unmounting with legacy API (no previous content)', () => {
Expand Down Expand Up @@ -401,10 +407,10 @@ describe('ReactDOMRoot', () => {
await act(async () => {
root.render(<Foo value="b" />);

expect(Scheduler).toHaveYielded(['a']);
assertLog(['a']);
expect(container.textContent).toEqual('a');

expect(Scheduler).toFlushAndYieldThrough(['b']);
await waitFor(['b']);
if (gate(flags => flags.allowConcurrentByDefault)) {
expect(container.textContent).toEqual('a');
} else {
Expand Down
Loading

0 comments on commit e64a8f4

Please sign in to comment.