Skip to content

Commit

Permalink
Codemod tests to waitFor pattern (2/?) (#26296)
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 authored Mar 3, 2023
1 parent 1f1f8eb commit ce8a72f
Show file tree
Hide file tree
Showing 14 changed files with 449 additions and 388 deletions.
76 changes: 34 additions & 42 deletions packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ let Scheduler;
let Suspense;
let TextResource;
let textResourceShouldFail;
let waitForAll;
let assertLog;

describe('ReactCache', () => {
beforeEach(() => {
Expand All @@ -33,6 +35,10 @@ describe('ReactCache', () => {
ReactTestRenderer = require('react-test-renderer');
Scheduler = require('scheduler');

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

TextResource = createResource(
([text, ms = 0]) => {
let listeners = null;
Expand Down Expand Up @@ -105,7 +111,7 @@ describe('ReactCache', () => {
}
}

it('throws a promise if the requested value is not in the cache', () => {
it('throws a promise if the requested value is not in the cache', async () => {
function App() {
return (
<Suspense fallback={<Text text="Loading..." />}>
Expand All @@ -118,11 +124,11 @@ describe('ReactCache', () => {
unstable_isConcurrent: true,
});

expect(Scheduler).toFlushAndYield(['Suspend! [Hi]', 'Loading...']);
await waitForAll(['Suspend! [Hi]', 'Loading...']);

jest.advanceTimersByTime(100);
expect(Scheduler).toHaveYielded(['Promise resolved [Hi]']);
expect(Scheduler).toFlushAndYield(['Hi']);
assertLog(['Promise resolved [Hi]']);
await waitForAll(['Hi']);
});

it('throws an error on the subsequent read if the promise is rejected', async () => {
Expand All @@ -138,22 +144,22 @@ describe('ReactCache', () => {
unstable_isConcurrent: true,
});

expect(Scheduler).toFlushAndYield(['Suspend! [Hi]', 'Loading...']);
await waitForAll(['Suspend! [Hi]', 'Loading...']);

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

expect(Scheduler).toFlushAndThrow('Failed to load: Hi');
expect(Scheduler).toHaveYielded(['Error! [Hi]', 'Error! [Hi]']);
assertLog(['Error! [Hi]', 'Error! [Hi]']);

// Should throw again on a subsequent read
root.update(<App />);
expect(Scheduler).toFlushAndThrow('Failed to load: Hi');
expect(Scheduler).toHaveYielded(['Error! [Hi]', 'Error! [Hi]']);
assertLog(['Error! [Hi]', 'Error! [Hi]']);
});

it('warns if non-primitive key is passed to a resource without a hash function', () => {
it('warns if non-primitive key is passed to a resource without a hash function', async () => {
const BadTextResource = createResource(([text, ms = 0]) => {
return new Promise((resolve, reject) =>
setTimeout(() => {
Expand All @@ -177,16 +183,16 @@ describe('ReactCache', () => {
);

if (__DEV__) {
expect(() => {
expect(Scheduler).toFlushAndYield(['App', 'Loading...']);
expect(async () => {
await waitForAll(['App', 'Loading...']);
}).toErrorDev([
'Invalid key type. Expected a string, number, symbol, or ' +
'boolean, but instead received: Hi,100\n\n' +
'To use non-primitive values as keys, you must pass a hash ' +
'function as the second argument to createResource().',
]);
} else {
expect(Scheduler).toFlushAndYield(['App', 'Loading...']);
await waitForAll(['App', 'Loading...']);
}
});

Expand All @@ -204,19 +210,19 @@ describe('ReactCache', () => {
unstable_isConcurrent: true,
},
);
expect(Scheduler).toFlushAndYield([
await waitForAll([
'Suspend! [1]',
'Suspend! [2]',
'Suspend! [3]',
'Loading...',
]);
jest.advanceTimersByTime(100);
expect(Scheduler).toHaveYielded([
assertLog([
'Promise resolved [1]',
'Promise resolved [2]',
'Promise resolved [3]',
]);
expect(Scheduler).toFlushAndYield([1, 2, 3]);
await waitForAll([1, 2, 3]);
expect(root).toMatchRenderedOutput('123');

// Render 1, 4, 5
Expand All @@ -228,18 +234,10 @@ describe('ReactCache', () => {
</Suspense>,
);

expect(Scheduler).toFlushAndYield([
1,
'Suspend! [4]',
'Suspend! [5]',
'Loading...',
]);
await waitForAll([1, 'Suspend! [4]', 'Suspend! [5]', 'Loading...']);
jest.advanceTimersByTime(100);
expect(Scheduler).toHaveYielded([
'Promise resolved [4]',
'Promise resolved [5]',
]);
expect(Scheduler).toFlushAndYield([1, 4, 5]);
assertLog(['Promise resolved [4]', 'Promise resolved [5]']);
await waitForAll([1, 4, 5]);
expect(root).toMatchRenderedOutput('145');

// We've now rendered values 1, 2, 3, 4, 5, over our limit of 3. The least
Expand All @@ -253,7 +251,7 @@ describe('ReactCache', () => {
</Suspense>,
);

expect(Scheduler).toFlushAndYield([
await waitForAll([
// 1 is still cached
1,
// 2 and 3 suspend because they were evicted from the cache
Expand All @@ -262,11 +260,8 @@ describe('ReactCache', () => {
'Loading...',
]);
jest.advanceTimersByTime(100);
expect(Scheduler).toHaveYielded([
'Promise resolved [2]',
'Promise resolved [3]',
]);
expect(Scheduler).toFlushAndYield([1, 2, 3]);
assertLog(['Promise resolved [2]', 'Promise resolved [3]']);
await waitForAll([1, 2, 3]);
expect(root).toMatchRenderedOutput('123');
});

Expand All @@ -287,18 +282,15 @@ describe('ReactCache', () => {
},
);

expect(Scheduler).toFlushAndYield(['Loading...']);
await waitForAll(['Loading...']);

jest.advanceTimersByTime(1000);
expect(Scheduler).toHaveYielded([
'Promise resolved [B]',
'Promise resolved [A]',
]);
expect(Scheduler).toFlushAndYield(['Result']);
assertLog(['Promise resolved [B]', 'Promise resolved [A]']);
await waitForAll(['Result']);
expect(root).toMatchRenderedOutput('Result');
});

it('if a thenable resolves multiple times, does not update the first cached value', () => {
it('if a thenable resolves multiple times, does not update the first cached value', async () => {
let resolveThenable;
const BadTextResource = createResource(
([text, ms = 0]) => {
Expand Down Expand Up @@ -349,7 +341,7 @@ describe('ReactCache', () => {
},
);

expect(Scheduler).toFlushAndYield(['Suspend! [Hi]', 'Loading...']);
await waitForAll(['Suspend! [Hi]', 'Loading...']);

resolveThenable('Hi');
// This thenable improperly resolves twice. We should not update the
Expand All @@ -365,8 +357,8 @@ describe('ReactCache', () => {
},
);

expect(Scheduler).toHaveYielded([]);
expect(Scheduler).toFlushAndYield(['Hi']);
assertLog([]);
await waitForAll(['Hi']);
expect(root).toMatchRenderedOutput('Hi');
});

Expand Down
15 changes: 9 additions & 6 deletions packages/react-client/src/__tests__/ReactFlight-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ let ReactNoopFlightClient;
let ErrorBoundary;
let NoErrorExpected;
let Scheduler;
let assertLog;

describe('ReactFlight', () => {
beforeEach(() => {
Expand All @@ -33,6 +34,8 @@ describe('ReactFlight', () => {
ReactNoopFlightClient = require('react-noop-renderer/flight-client');
act = require('jest-react').act;
Scheduler = require('scheduler');
const InternalTestUtils = require('internal-test-utils');
assertLog = InternalTestUtils.assertLog;

ErrorBoundary = class extends React.Component {
state = {hasError: false, error: null};
Expand Down Expand Up @@ -808,13 +811,13 @@ describe('ReactFlight', () => {
const ClientDoublerModuleRef = clientReference(ClientDoubler);

const transport = ReactNoopFlightServer.render(<App />);
expect(Scheduler).toHaveYielded([]);
assertLog([]);

await act(async () => {
ReactNoop.render(await ReactNoopFlightClient.read(transport));
});

expect(Scheduler).toHaveYielded(['ClientDoubler']);
assertLog(['ClientDoubler']);
expect(ReactNoop).toMatchRenderedOutput(
<>
<div prop=":S1:">:S1:</div>
Expand Down Expand Up @@ -997,15 +1000,15 @@ describe('ReactFlight', () => {

const transport = ReactNoopFlightServer.render(<Foo />);

expect(Scheduler).toHaveYielded(['suspended']);
assertLog(['suspended']);

await act(async () => {
resolve();
await promise;
jest.runAllImmediates();
});

expect(Scheduler).toHaveYielded(['rendered']);
assertLog(['rendered']);

await act(async () => {
ServerContext._currentRenderer = null;
Expand Down Expand Up @@ -1045,7 +1048,7 @@ describe('ReactFlight', () => {

const transport = ReactNoopFlightServer.render(model);

expect(Scheduler).toHaveYielded([]);
assertLog([]);

await act(async () => {
ServerContext._currentRenderer = null;
Expand All @@ -1054,7 +1057,7 @@ describe('ReactFlight', () => {
ReactNoop.render(flightModel.foo);
});

expect(Scheduler).toHaveYielded(['ClientBar']);
assertLog(['ClientBar']);
expect(ReactNoop).toMatchRenderedOutput(<span>hi this is server</span>);

expect(() => {
Expand Down
6 changes: 5 additions & 1 deletion packages/react-debug-tools/src/__tests__/ReactDevToolsHooksIntegration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('React hooks DevTools integration', () => {
let overrideHookState;
let scheduleUpdate;
let setSuspenseHandler;
let waitForAll;

global.IS_REACT_ACT_ENVIRONMENT = true;

Expand All @@ -41,6 +42,9 @@ describe('React hooks DevTools integration', () => {
ReactTestRenderer = require('react-test-renderer');
Scheduler = require('scheduler');

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

act = ReactTestRenderer.act;
});

Expand Down Expand Up @@ -256,7 +260,7 @@ describe('React hooks DevTools integration', () => {
),
);

expect(Scheduler).toFlushAndYield([]);
await waitForAll([]);
// Ensure we timeout any suspense time.
jest.advanceTimersByTime(1000);
const fiber = renderer.root._currentFiber().child;
Expand Down
Loading

0 comments on commit ce8a72f

Please sign in to comment.