Skip to content

Commit

Permalink
[tests] Fix assertions not flushed before act (#28745)
Browse files Browse the repository at this point in the history
Fixes some easy cases blocking
#28737, I'll follow up with more
complex/interesting cases in other PRs.
  • Loading branch information
rickhanlonii authored Apr 10, 2024
1 parent ed3c65c commit 42eff4b
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -913,15 +913,13 @@ describe('ReactCompositeComponent', () => {
await act(() => {
root.render(<Wrapper name="A" />);
});

assertLog(['A componentWillMount', 'A render', 'A componentDidMount']);
await act(() => {
root.render(<Wrapper name="B" />);
});

assertLog([
'A componentWillMount',
'A render',
'A componentDidMount',

'B componentWillMount',
'B render',
'A componentWillUnmount',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,11 @@ describe('ReactDOMFizzForm', () => {
await readIntoContainer(stream);
expect(container.textContent).toEqual('Loading...');

assertLog(['Loading...']);
// After hydration, it's updated to the final value
await act(() => ReactDOMClient.hydrateRoot(container, <App />));
expect(container.textContent).toEqual('Final');
assertLog(['Loading...', 'Final']);
},
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
});
expect(container.textContent).toEqual('not hovered');

assertLog(['not hovered']);
await act(async () => {
// Note: React does not use native mouseenter/mouseleave events
// but we should still correctly determine their priority.
Expand All @@ -308,7 +309,7 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
dispatchAndSetCurrentEvent(target.current, mouseEnterEvent);

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

await waitFor(['hovered']);
Expand Down
3 changes: 2 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,13 @@ describe('ReactDOMRoot', () => {
root.render(<Foo value="a" />);
});

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

await act(async () => {
root.render(<Foo value="b" />);

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

await waitFor(['b']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ let Suspense;
let Scheduler;
let act;
let textCache;
let assertLog;

describe('ReactDOMSuspensePlaceholder', () => {
let container;
Expand All @@ -31,6 +32,7 @@ describe('ReactDOMSuspensePlaceholder', () => {
ReactDOMClient = require('react-dom/client');
Scheduler = require('scheduler');
act = require('internal-test-utils').act;
assertLog = require('internal-test-utils').assertLog;
Suspense = React.Suspense;
container = document.createElement('div');
document.body.appendChild(container);
Expand Down Expand Up @@ -157,11 +159,11 @@ describe('ReactDOMSuspensePlaceholder', () => {
});

expect(container.textContent).toEqual('Loading...');

assertLog(['A', 'Suspend! [B]', 'Loading...']);
await act(() => {
resolveText('B');
});

assertLog(['A', 'B', 'C']);
expect(container.textContent).toEqual('ABC');
});

Expand Down
9 changes: 3 additions & 6 deletions packages/react-dom/src/__tests__/ReactEmptyComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,14 @@ describe('ReactEmptyComponent', () => {
root1.render(instance1);
});

assertLog(['mount undefined', 'update DIV']);

const root2 = ReactDOMClient.createRoot(container2);
await act(() => {
root2.render(instance2);
});

assertLog([
'mount undefined',
'update DIV',
'mount DIV',
'update undefined',
]);
assertLog(['mount DIV', 'update undefined']);
});

it('should be able to switch in a list of children', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,13 @@ describe('updaters', () => {
root.render(<Parent />);
});
expect(allSchedulerTags).toEqual([[HostRoot]]);
assertLog(['onCommitRoot']);

await act(() => {
root.render(<Parent />);
});
expect(allSchedulerTags).toEqual([[HostRoot], [HostRoot]]);
assertLog(['onCommitRoot']);
});

it('should report a function component as the scheduler for a hooks update', async () => {
Expand Down Expand Up @@ -148,12 +150,13 @@ describe('updaters', () => {
expect(scheduleForA).not.toBeNull();
expect(scheduleForB).not.toBeNull();
expect(allSchedulerTypes).toEqual([[null]]);
assertLog(['onCommitRoot']);

await act(() => {
scheduleForA();
});
expect(allSchedulerTypes).toEqual([[null], [SchedulingComponentA]]);

assertLog(['onCommitRoot']);
await act(() => {
scheduleForB();
});
Expand All @@ -162,6 +165,7 @@ describe('updaters', () => {
[SchedulingComponentA],
[SchedulingComponentB],
]);
assertLog(['onCommitRoot']);
});

it('should report a class component as the scheduler for a setState update', async () => {
Expand All @@ -180,7 +184,7 @@ describe('updaters', () => {
root.render(<Parent />);
});
expect(allSchedulerTypes).toEqual([[null]]);

assertLog(['onCommitRoot']);
expect(instance).not.toBeNull();
await act(() => {
instance.setState({});
Expand Down
10 changes: 3 additions & 7 deletions packages/use-subscription/src/__tests__/useSubscription-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,20 +338,16 @@ describe('useSubscription', () => {
observableB.next('b-3');
});

assertLog(['Grandchild: b-0', 'Child: b-3', 'Grandchild: b-3']);

// Update again
await act(() => root.render(<Parent observed={observableA} />));

// Flush everything and ensure that the correct subscribable is used
// We expect the last emitted update to be rendered (because of the commit phase value check)
// But the intermediate ones should be ignored,
// And the final rendered output should be the higher-priority observable.
assertLog([
'Grandchild: b-0',
'Child: b-3',
'Grandchild: b-3',
'Child: a-0',
'Grandchild: a-0',
]);
assertLog(['Child: a-0', 'Grandchild: a-0']);
expect(log).toEqual([
'Parent.componentDidMount',
'Parent.componentDidUpdate',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,14 +660,17 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
// Object.is algorithm to compare values.
await act(() => root.render(<App />));
expect(container.textContent).toEqual('NaN');
assertLog([NaN]);

// Update to real number
await act(() => store.set(123));
expect(container.textContent).toEqual('123');
assertLog([123]);

// Update back to NaN
await act(() => store.set('not a number'));
expect(container.textContent).toEqual('NaN');
assertLog([NaN]);
});

describe('extra features implemented in user-space', () => {
Expand Down Expand Up @@ -968,6 +971,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
),
);

assertLog(['A']);
expect(container.textContent).toEqual('A');

if (__DEV__ && gate(flags => flags.enableUseSyncExternalStoreShim)) {
Expand Down Expand Up @@ -1017,6 +1021,7 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
),
);

assertLog(['A']);
expect(container.textContent).toEqual('A');

if (__DEV__ && gate(flags => flags.enableUseSyncExternalStoreShim)) {
Expand Down

0 comments on commit 42eff4b

Please sign in to comment.