Skip to content

Commit

Permalink
Convert more Suspense tests to use act (2/n) (#26610)
Browse files Browse the repository at this point in the history
Many of our Suspense-related tests were written before the `act` API was
introduced, and use the lower level `waitFor` helpers instead. So they
are less resilient to changes in implementation details than they could
be.

This converts some of our test suite to use `act` in more places. I
found these while working on a PR to expand our fallback throttling
mechanism to include all renders that result from a promise resolving,
even if there are no more fallbacks in the tree.

I think this covers all the remaining tests that are affected.
  • Loading branch information
acdlite authored and kassens committed Apr 21, 2023
1 parent 1891d5f commit 3e7a0ae
Show file tree
Hide file tree
Showing 13 changed files with 422 additions and 429 deletions.
60 changes: 36 additions & 24 deletions packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ let textResourceShouldFail;
let waitForAll;
let assertLog;
let waitForThrow;
let act;

describe('ReactCache', () => {
beforeEach(() => {
Expand All @@ -40,6 +41,7 @@ describe('ReactCache', () => {
waitForAll = InternalTestUtils.waitForAll;
assertLog = InternalTestUtils.assertLog;
waitForThrow = InternalTestUtils.waitForThrow;
act = InternalTestUtils.act;

TextResource = createResource(
([text, ms = 0]) => {
Expand Down Expand Up @@ -145,11 +147,14 @@ describe('ReactCache', () => {
await waitForAll(['Suspend! [Hi]', 'Loading...']);

textResourceShouldFail = true;
jest.advanceTimersByTime(100);
assertLog(['Promise rejected [Hi]']);

await waitForThrow('Failed to load: Hi');
assertLog(['Error! [Hi]', 'Error! [Hi]']);
let error;
try {
await act(() => jest.advanceTimersByTime(100));
} catch (e) {
error = e;
}
expect(error.message).toMatch('Failed to load: Hi');
assertLog(['Promise rejected [Hi]', 'Error! [Hi]', 'Error! [Hi]']);

// Should throw again on a subsequent read
root.update(<App />);
Expand Down Expand Up @@ -217,9 +222,8 @@ describe('ReactCache', () => {
assertLog(['Promise resolved [2]']);
await waitForAll([1, 2, 'Suspend! [3]']);

jest.advanceTimersByTime(100);
assertLog(['Promise resolved [3]']);
await waitForAll([1, 2, 3]);
await act(() => jest.advanceTimersByTime(100));
assertLog(['Promise resolved [3]', 1, 2, 3]);

expect(root).toMatchRenderedOutput('123');

Expand All @@ -234,13 +238,17 @@ describe('ReactCache', () => {

await waitForAll([1, 'Suspend! [4]', 'Loading...']);

jest.advanceTimersByTime(100);
assertLog(['Promise resolved [4]']);
await waitForAll([1, 4, 'Suspend! [5]']);

jest.advanceTimersByTime(100);
assertLog(['Promise resolved [5]']);
await waitForAll([1, 4, 5]);
await act(() => jest.advanceTimersByTime(100));
assertLog([
'Promise resolved [4]',
1,
4,
'Suspend! [5]',
'Promise resolved [5]',
1,
4,
5,
]);

expect(root).toMatchRenderedOutput('145');

Expand All @@ -262,13 +270,18 @@ describe('ReactCache', () => {
'Suspend! [2]',
'Loading...',
]);
jest.advanceTimersByTime(100);
assertLog(['Promise resolved [2]']);
await waitForAll([1, 2, 'Suspend! [3]']);

jest.advanceTimersByTime(100);
assertLog(['Promise resolved [3]']);
await waitForAll([1, 2, 3]);
await act(() => jest.advanceTimersByTime(100));
assertLog([
'Promise resolved [2]',
1,
2,
'Suspend! [3]',
'Promise resolved [3]',
1,
2,
3,
]);
expect(root).toMatchRenderedOutput('123');
});

Expand All @@ -291,9 +304,8 @@ describe('ReactCache', () => {

await waitForAll(['Loading...']);

jest.advanceTimersByTime(1000);
assertLog(['Promise resolved [B]', 'Promise resolved [A]']);
await waitForAll(['Result']);
await act(() => jest.advanceTimersByTime(1000));
assertLog(['Promise resolved [B]', 'Promise resolved [A]', 'Result']);
expect(root).toMatchRenderedOutput('Result');
});

Expand Down
14 changes: 8 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ let waitFor;
let waitForAll;
let assertLog;
let waitForPaint;
let clientAct;

function resetJSDOM(markup) {
// Test Environment
Expand Down Expand Up @@ -74,6 +75,7 @@ describe('ReactDOMFizzServer', () => {
waitFor = InternalTestUtils.waitFor;
waitForPaint = InternalTestUtils.waitForPaint;
assertLog = InternalTestUtils.assertLog;
clientAct = InternalTestUtils.act;

if (gate(flags => flags.source)) {
// The `with-selector` module composes the main `use-sync-external-store`
Expand Down Expand Up @@ -1191,8 +1193,8 @@ describe('ReactDOMFizzServer', () => {
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// We now resolve it on the client.
resolveText('Hello');
await waitForAll([]);
await clientAct(() => resolveText('Hello'));
assertLog([]);

// The client rendered HTML is now in place.
expect(getVisibleChildren(container)).toEqual(
Expand Down Expand Up @@ -2884,10 +2886,10 @@ describe('ReactDOMFizzServer', () => {
</div>,
);

await act(() => {
await clientAct(() => {
resolveText('Yay!');
});
await waitForAll(['Yay!']);
assertLog(['Yay!']);
expect(getVisibleChildren(container)).toEqual(
<div>
<span />
Expand Down Expand Up @@ -4310,10 +4312,10 @@ describe('ReactDOMFizzServer', () => {
<h1>Loading...</h1>
</div>,
);
await unsuspend();
await clientAct(() => unsuspend());
// Since our client components only throw on the very first render there are no
// new throws in this pass
await waitForAll([]);
assertLog([]);
expect(mockError.mock.calls).toEqual([]);

expect(getVisibleChildren(container)).toEqual(
Expand Down
4 changes: 3 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ let waitForAll;
let waitForThrow;
let assertLog;
let Scheduler;
let clientAct;

function resetJSDOM(markup) {
// Test Environment
Expand Down Expand Up @@ -71,6 +72,7 @@ describe('ReactDOMFloat', () => {
waitForAll = InternalTestUtils.waitForAll;
waitForThrow = InternalTestUtils.waitForThrow;
assertLog = InternalTestUtils.assertLog;
clientAct = InternalTestUtils.act;

textCache = new Map();
loadCache = new Set();
Expand Down Expand Up @@ -1186,7 +1188,7 @@ body {
// events have already fired. This requires the load to be awaited for the commit to have a chance to flush
// We could change this by tracking the loadingState's fulfilled status directly on the loadingState similar
// to thenables however this slightly increases the fizz runtime code size.
await loadStylesheets();
await clientAct(() => loadStylesheets());
assertLog(['load stylesheet: foo']);
expect(getMeaningfulChildren(document)).toEqual(
<html>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ let assertLog;
let ReactCache;
let Suspense;
let TextResource;
let act;

describe('ReactBlockingMode', () => {
beforeEach(() => {
Expand All @@ -23,6 +24,7 @@ describe('ReactBlockingMode', () => {
const InternalTestUtils = require('internal-test-utils');
waitForAll = InternalTestUtils.waitForAll;
assertLog = InternalTestUtils.assertLog;
act = InternalTestUtils.act;

TextResource = ReactCache.unstable_createResource(
([text, ms = 0]) => {
Expand Down Expand Up @@ -117,9 +119,8 @@ describe('ReactBlockingMode', () => {
// fallback should mount immediately.
expect(root).toMatchRenderedOutput('Loading...');

await jest.advanceTimersByTime(1000);
assertLog(['Promise resolved [B]']);
await waitForAll(['A', 'B', 'C']);
await act(() => jest.advanceTimersByTime(1000));
assertLog(['Promise resolved [B]', 'A', 'B', 'C']);
expect(root).toMatchRenderedOutput(
<>
<span>A</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3623,9 +3623,8 @@ describe('ReactHooksWithNoopRenderer', () => {
</>,
);

await resolveText('A');
assertLog(['Promise resolved [A]']);
await waitForAll(['A']);
await act(() => resolveText('A'));
assertLog(['Promise resolved [A]', 'A']);
expect(ReactNoop).toMatchRenderedOutput(
<>
<span prop="A" />
Expand Down
Loading

0 comments on commit 3e7a0ae

Please sign in to comment.