Skip to content

Commit

Permalink
Improve tests that use discrete events (#20667)
Browse files Browse the repository at this point in the history
  • Loading branch information
rickhanlonii authored Jan 27, 2021
1 parent d13f5b9 commit 9c32622
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 99 deletions.
17 changes: 11 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMHooks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
let React;
let ReactDOM;
let Scheduler;
let act;

describe('ReactDOMHooks', () => {
let container;
Expand All @@ -22,6 +23,7 @@ describe('ReactDOMHooks', () => {
React = require('react');
ReactDOM = require('react-dom');
Scheduler = require('scheduler');
act = require('react-dom/test-utils').unstable_concurrentAct;

container = document.createElement('div');
document.body.appendChild(container);
Expand Down Expand Up @@ -106,7 +108,7 @@ describe('ReactDOMHooks', () => {
});

// @gate experimental
it('should not bail out when an update is scheduled from within an event handler in Concurrent Mode', () => {
it('should not bail out when an update is scheduled from within an event handler in Concurrent Mode', async () => {
const {createRef, useCallback, useState} = React;

const Example = ({inputRef, labelRef}) => {
Expand All @@ -132,11 +134,14 @@ describe('ReactDOMHooks', () => {
Scheduler.unstable_flushAll();

inputRef.current.value = 'abc';
inputRef.current.dispatchEvent(
new Event('input', {bubbles: true, cancelable: true}),
);

Scheduler.unstable_flushAll();
await act(async () => {
inputRef.current.dispatchEvent(
new Event('input', {
bubbles: true,
cancelable: true,
}),
);
});

expect(labelRef.current.innerHTML).toBe('abc');
});
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ function runActTests(label, render, unmount, rerender) {
expect(Scheduler).toHaveYielded([100]);
});

it('flushes effects on every call', () => {
it('flushes effects on every call', async () => {
function App() {
const [ctr, setCtr] = React.useState(0);
React.useEffect(() => {
Expand All @@ -209,16 +209,16 @@ function runActTests(label, render, unmount, rerender) {
button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
}

act(() => {
await act(async () => {
click();
click();
click();
});
// it consolidates the 3 updates, then fires the effect
expect(Scheduler).toHaveYielded([3]);
act(click);
await act(async () => click());
expect(Scheduler).toHaveYielded([4]);
act(click);
await act(async () => click());
expect(Scheduler).toHaveYielded([5]);
expect(button.innerHTML).toBe('5');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ describe('ChangeEventPlugin', () => {
});

// @gate experimental
it('is async for non-input events', () => {
it('is async for non-input events', async () => {
const root = ReactDOM.unstable_createRoot(container);
let input;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('SimpleEventPlugin', function() {
let React;
let ReactDOM;
let Scheduler;
let TestUtils;

let onClick;
let container;
Expand All @@ -39,6 +40,7 @@ describe('SimpleEventPlugin', function() {
React = require('react');
ReactDOM = require('react-dom');
Scheduler = require('scheduler');
TestUtils = require('react-dom/test-utils');

onClick = jest.fn();
});
Expand Down Expand Up @@ -314,7 +316,7 @@ describe('SimpleEventPlugin', function() {
});

// @gate experimental
it('end result of many interactive updates is deterministic', () => {
it('end result of many interactive updates is deterministic', async () => {
container = document.createElement('div');
const root = ReactDOM.unstable_createRoot(container);
document.body.appendChild(container);
Expand Down Expand Up @@ -361,12 +363,14 @@ describe('SimpleEventPlugin', function() {
expect(button.textContent).toEqual('Count: 0');

// Click the button many more times
click();
click();
click();
click();
click();
click();
await TestUtils.act(async () => {
click();
click();
click();
click();
click();
click();
});

// Flush the remaining work
Scheduler.unstable_flushAll();
Expand All @@ -375,7 +379,7 @@ describe('SimpleEventPlugin', function() {
});

// @gate experimental
it('flushes discrete updates in order', () => {
it('flushes discrete updates in order', async () => {
container = document.createElement('div');
document.body.appendChild(container);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ describe('ReactIncrementalErrorHandling', () => {
expect(ReactNoop.getChildren()).toEqual([span('Caught an error: oops!')]);
});

it("retries at a lower priority if there's additional pending work", () => {
// @gate experimental
it("retries at a lower priority if there's additional pending work", async () => {
function App(props) {
if (props.isBroken) {
Scheduler.unstable_yieldValue('error');
Expand All @@ -252,14 +253,14 @@ describe('ReactIncrementalErrorHandling', () => {
});
}

ReactNoop.discreteUpdates(() => {
ReactNoop.render(<App isBroken={true} />, onCommit);
});
ReactNoop.render(<App isBroken={true} />, onCommit);
expect(Scheduler).toFlushAndYieldThrough(['error']);
interrupt();

// This update is in a separate batch
ReactNoop.render(<App isBroken={false} />, onCommit);
React.unstable_startTransition(() => {
// This update is in a separate batch
ReactNoop.render(<App isBroken={false} />, onCommit);
});

expect(Scheduler).toFlushAndYieldThrough([
// The first render fails. But because there's a lower priority pending
Expand Down Expand Up @@ -311,16 +312,16 @@ describe('ReactIncrementalErrorHandling', () => {
});
}

ReactNoop.discreteUpdates(() => {
ReactNoop.render(<App isBroken={true} />, onCommit);
});
ReactNoop.render(<App isBroken={true} />, onCommit);
expect(Scheduler).toFlushAndYieldThrough(['error']);
interrupt();

expect(ReactNoop).toMatchRenderedOutput(null);

// This update is in a separate batch
ReactNoop.render(<App isBroken={false} />, onCommit);
React.unstable_startTransition(() => {
// This update is in a separate batch
ReactNoop.render(<App isBroken={false} />, onCommit);
});

expect(Scheduler).toFlushAndYieldThrough([
// The first render fails. But because there's a lower priority pending
Expand Down Expand Up @@ -1786,6 +1787,7 @@ describe('ReactIncrementalErrorHandling', () => {
});
}

// @gate experimental
it('uncaught errors should be discarded if the render is aborted', async () => {
const root = ReactNoop.createRoot();

Expand All @@ -1795,22 +1797,24 @@ describe('ReactIncrementalErrorHandling', () => {
}

await ReactNoop.act(async () => {
ReactNoop.discreteUpdates(() => {
root.render(<Oops />);
});
root.render(<Oops />);

// Render past the component that throws, then yield.
expect(Scheduler).toFlushAndYieldThrough(['Oops']);
expect(root).toMatchRenderedOutput(null);
// Interleaved update. When the root completes, instead of throwing the
// error, it should try rendering again. This update will cause it to
// recover gracefully.
root.render('Everything is fine.');
React.unstable_startTransition(() => {
root.render('Everything is fine.');
});
});

// Should finish without throwing.
expect(root).toMatchRenderedOutput('Everything is fine.');
});

// @gate experimental
it('uncaught errors are discarded if the render is aborted, case 2', async () => {
const {useState} = React;
const root = ReactNoop.createRoot();
Expand All @@ -1835,21 +1839,20 @@ describe('ReactIncrementalErrorHandling', () => {
});

await ReactNoop.act(async () => {
// Schedule a high pri and a low pri update on the root.
ReactNoop.discreteUpdates(() => {
root.render(<Oops />);
// Schedule a default pri and a low pri update on the root.
root.render(<Oops />);
React.unstable_startTransition(() => {
root.render(<AllGood />);
});
root.render(<AllGood />);
// Render through just the high pri update. The low pri update remains on

// Render through just the default pri update. The low pri update remains on
// the queue.
expect(Scheduler).toFlushAndYieldThrough(['Everything is fine.']);

// Schedule a high pri update on a child that triggers an error.
// Schedule a default pri update on a child that triggers an error.
// The root should capture this error. But since there's still a pending
// update on the root, the error should be suppressed.
ReactNoop.discreteUpdates(() => {
setShouldThrow(true);
});
setShouldThrow(true);
});
// Should render the final state without throwing the error.
expect(Scheduler).toHaveYielded(['Everything is fine.']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,14 +570,13 @@ describe('ReactSuspenseWithNoopRenderer', () => {
);
}

// Schedule a high pri update and a low pri update, without rendering in
// between.
ReactNoop.discreteUpdates(() => {
// High pri
ReactNoop.render(<App />);
});
// Schedule a default pri update and a low pri update, without rendering in between.
// Default pri
ReactNoop.render(<App />);
// Low pri
ReactNoop.render(<App hide={true} />);
React.unstable_startTransition(() => {
ReactNoop.render(<App hide={true} />);
});

expect(Scheduler).toFlushAndYield([
// The first update suspends
Expand Down Expand Up @@ -1879,9 +1878,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
ReactNoop.render(<Foo />);
expect(Scheduler).toFlushAndYield(['Foo']);

ReactNoop.discreteUpdates(() =>
ReactNoop.render(<Foo renderContent={true} />),
);
ReactNoop.render(<Foo renderContent={true} />);
expect(Scheduler).toFlushAndYieldThrough(['Foo']);

// Advance some time.
Expand Down Expand Up @@ -3080,48 +3077,48 @@ describe('ReactSuspenseWithNoopRenderer', () => {
// Schedule an update inside the Suspense boundary that suspends.
setAppText('B');
expect(Scheduler).toFlushAndYield(['Suspend! [B]', 'Loading...']);
});

// Commit the placeholder
await advanceTimers(250);
expect(root).toMatchRenderedOutput(
<>
<span hidden={true} prop="A" />
<span prop="Loading..." />
</>,
);
expect(root).toMatchRenderedOutput(
<>
<span hidden={true} prop="A" />
<span prop="Loading..." />
</>,
);

// Schedule a high pri update on the boundary, and a lower pri update
// on the fallback. We're testing to make sure the fallback can still
// update even though the primary tree is suspended.{
ReactNoop.discreteUpdates(() => {
setAppText('C');
// Schedule a default pri update on the boundary, and a lower pri update
// on the fallback. We're testing to make sure the fallback can still
// update even though the primary tree is suspended.
await ReactNoop.act(async () => {
setAppText('C');
React.unstable_startTransition(() => {
setFallbackText('Still loading...');
});
setFallbackText('Still loading...');
});

expect(Scheduler).toFlushAndYield([
// First try to render the high pri update. Still suspended.
'Suspend! [C]',
'Loading...',
expect(Scheduler).toHaveYielded([
// First try to render the high pri update. Still suspended.
'Suspend! [C]',
'Loading...',

// In the expiration times model, once the high pri update suspends,
// we can't be sure if there's additional work at a lower priority
// that might unblock the tree. We do know that there's a lower
// priority update *somehwere* in the entire root, though (the update
// to the fallback). So we try rendering one more time, just in case.
// TODO: We shouldn't need to do this with lanes, because we always
// know exactly which lanes have pending work in each tree.
'Suspend! [C]',

// Then complete the update to the fallback.
'Still loading...',
]);
expect(root).toMatchRenderedOutput(
<>
<span hidden={true} prop="A" />
<span prop="Still loading..." />
</>,
);
});
// In the expiration times model, once the high pri update suspends,
// we can't be sure if there's additional work at a lower priority
// that might unblock the tree. We do know that there's a lower
// priority update *somehwere* in the entire root, though (the update
// to the fallback). So we try rendering one more time, just in case.
// TODO: We shouldn't need to do this with lanes, because we always
// know exactly which lanes have pending work in each tree.
'Suspend! [C]',

// Then complete the update to the fallback.
'Still loading...',
]);
expect(root).toMatchRenderedOutput(
<>
<span hidden={true} prop="A" />
<span prop="Still loading..." />
</>,
);
},
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1397,12 +1397,13 @@ describe('useMutableSource', () => {
// Now mutate A. Both hooks should update.
// This is at high priority so that it doesn't get batched with default
// priority updates that might fire during the passive effect
ReactNoop.discreteUpdates(() => {
mutateA('a1');
await ReactNoop.act(async () => {
ReactNoop.discreteUpdates(() => {
mutateA('a1');
});
});
expect(Scheduler).toFlushUntilNextPaint([]);

expect(root.getChildrenAsJSX()).toEqual('first: a1, second: a1');
expect(root).toMatchRenderedOutput('first: a1, second: a1');
});

expect(root.getChildrenAsJSX()).toEqual('first: a1, second: a1');
Expand Down
Loading

0 comments on commit 9c32622

Please sign in to comment.