Skip to content

Commit

Permalink
Update more tests to not rely on sync queuing (#26358)
Browse files Browse the repository at this point in the history 
This fixes a handful of tests that were accidentally relying on React
synchronously queuing work in the Scheduler after a setState.

Usually this is because they use a lower level SchedulerMock method
instead of either `act` or one of the `waitFor` helpers. In some cases,
the solution is to switch to those APIs. In other cases, if we're
intentionally testing some lower level behavior, we might have to be a
bit more clever.

Co-authored-by: Tianyu Yao <skyyao@fb.com>
  • Loading branch information
@acdlite @tyao1
acdlite and tyao1 authored Mar 10, 2023
1 parent d1ad984 commit a8875ea
Show file tree
Hide file tree
Showing 25 changed files with 550 additions and 555 deletions.
51 changes: 24 additions & 27 deletions packages/react-debug-tools/src/__tests__/ReactDevToolsHooksIntegration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ describe('React hooks DevTools integration', () => {
let React;
let ReactDebugTools;
let ReactTestRenderer;
let Scheduler;
let act;
let overrideHookState;
let scheduleUpdate;
Expand All @@ -40,15 +39,14 @@ describe('React hooks DevTools integration', () => {
React = require('react');
ReactDebugTools = require('react-debug-tools');
ReactTestRenderer = require('react-test-renderer');
Scheduler = require('scheduler');

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

act = ReactTestRenderer.act;
});

it('should support editing useState hooks', () => {
it('should support editing useState hooks', async () => {
let setCountFn;

function MyComponent() {
Expand All @@ -70,14 +68,14 @@ describe('React hooks DevTools integration', () => {
expect(stateHook.isStateEditable).toBe(true);

if (__DEV__) {
act(() => overrideHookState(fiber, stateHook.id, [], 10));
await act(() => overrideHookState(fiber, stateHook.id, [], 10));
expect(renderer.toJSON()).toEqual({
type: 'div',
props: {},
children: ['count:', '10'],
});

act(() => setCountFn(count => count + 1));
await act(() => setCountFn(count => count + 1));
expect(renderer.toJSON()).toEqual({
type: 'div',
props: {},
Expand All @@ -86,7 +84,7 @@ describe('React hooks DevTools integration', () => {
}
});

it('should support editable useReducer hooks', () => {
it('should support editable useReducer hooks', async () => {
const initialData = {foo: 'abc', bar: 123};

function reducer(state, action) {
Expand Down Expand Up @@ -122,14 +120,14 @@ describe('React hooks DevTools integration', () => {
expect(reducerHook.isStateEditable).toBe(true);

if (__DEV__) {
act(() => overrideHookState(fiber, reducerHook.id, ['foo'], 'def'));
await act(() => overrideHookState(fiber, reducerHook.id, ['foo'], 'def'));
expect(renderer.toJSON()).toEqual({
type: 'div',
props: {},
children: ['foo:', 'def', ', bar:', '123'],
});

act(() => dispatchFn({type: 'swap'}));
await act(() => dispatchFn({type: 'swap'}));
expect(renderer.toJSON()).toEqual({
type: 'div',
props: {},
Expand All @@ -140,7 +138,7 @@ describe('React hooks DevTools integration', () => {

// This test case is based on an open source bug report:
// https://github.com/facebookincubator/redux-react-hook/issues/34#issuecomment-466693787
it('should handle interleaved stateful hooks (e.g. useState) and non-stateful hooks (e.g. useContext)', () => {
it('should handle interleaved stateful hooks (e.g. useState) and non-stateful hooks (e.g. useContext)', async () => {
const MyContext = React.createContext(1);

let setStateFn;
Expand Down Expand Up @@ -170,13 +168,13 @@ describe('React hooks DevTools integration', () => {
expect(stateHook.isStateEditable).toBe(true);

if (__DEV__) {
act(() => overrideHookState(fiber, stateHook.id, ['count'], 10));
await act(() => overrideHookState(fiber, stateHook.id, ['count'], 10));
expect(renderer.toJSON()).toEqual({
type: 'div',
props: {},
children: ['count:', '10'],
});
act(() => setStateFn(state => ({count: state.count + 1})));
await act(() => setStateFn(state => ({count: state.count + 1})));
expect(renderer.toJSON()).toEqual({
type: 'div',
props: {},
Expand All @@ -185,7 +183,7 @@ describe('React hooks DevTools integration', () => {
}
});

it('should support overriding suspense in legacy mode', () => {
it('should support overriding suspense in legacy mode', async () => {
if (__DEV__) {
// Lock the first render
setSuspenseHandler(() => true);
Expand All @@ -206,32 +204,32 @@ describe('React hooks DevTools integration', () => {
if (__DEV__) {
// First render was locked
expect(renderer.toJSON().children).toEqual(['Loading']);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Loading']);

// Release the lock
setSuspenseHandler(() => false);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Done']);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Done']);

// Lock again
setSuspenseHandler(() => true);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Loading']);

// Release the lock again
setSuspenseHandler(() => false);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Done']);

// Ensure it checks specific fibers.
setSuspenseHandler(f => f === fiber || f === fiber.alternate);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Loading']);
setSuspenseHandler(f => f !== fiber && f !== fiber.alternate);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Done']);
} else {
expect(renderer.toJSON().children).toEqual(['Done']);
Expand Down Expand Up @@ -267,33 +265,32 @@ describe('React hooks DevTools integration', () => {
if (__DEV__) {
// First render was locked
expect(renderer.toJSON().children).toEqual(['Loading']);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Loading']);

// Release the lock
setSuspenseHandler(() => false);
act(() => scheduleUpdate(fiber)); // Re-render
Scheduler.unstable_flushAll();
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Done']);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Done']);

// Lock again
setSuspenseHandler(() => true);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Loading']);

// Release the lock again
setSuspenseHandler(() => false);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Done']);

// Ensure it checks specific fibers.
setSuspenseHandler(f => f === fiber || f === fiber.alternate);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Loading']);
setSuspenseHandler(f => f !== fiber && f !== fiber.alternate);
act(() => scheduleUpdate(fiber)); // Re-render
await act(() => scheduleUpdate(fiber)); // Re-render
expect(renderer.toJSON().children).toEqual(['Done']);
} else {
expect(renderer.toJSON().children).toEqual(['Done']);
Expand Down
8 changes: 2 additions & 6 deletions packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

let React;
let ReactTestRenderer;
let Scheduler;
let ReactDebugTools;
let act;

Expand All @@ -21,7 +20,6 @@ describe('ReactHooksInspectionIntegration', () => {
jest.resetModules();
React = require('react');
ReactTestRenderer = require('react-test-renderer');
Scheduler = require('scheduler');
act = require('internal-test-utils').act;
ReactDebugTools = require('react-debug-tools');
});
Expand Down Expand Up @@ -890,10 +888,8 @@ describe('ReactHooksInspectionIntegration', () => {
</Suspense>,
);

await LazyFoo;

expect(() => {
Scheduler.unstable_flushAll();
await expect(async () => {
await act(async () => await LazyFoo);
}).toErrorDev([
'Foo: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
]);
Expand Down
Loading

0 comments on commit a8875ea

Please sign in to comment.