Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[assert helpers] react-reconciler #31986

Merged
merged 2 commits into from
Jan 6, 2025
Merged

[assert helpers] react-reconciler #31986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f67863c
[tests] fix hidden use() warnings
rickhanlonii Jan 6, 2025
c5e705f
[assert helpers] react-reconciler
rickhanlonii Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions packages/react-reconciler/src/__tests__/Activity-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ let startTransition;
let waitForPaint;
let waitFor;
let assertLog;
let assertConsoleErrorDev;

describe('Activity', () => {
beforeEach(() => {
Expand All @@ -37,6 +38,7 @@ describe('Activity', () => {
waitForPaint = InternalTestUtils.waitForPaint;
waitFor = InternalTestUtils.waitFor;
assertLog = InternalTestUtils.assertLog;
assertConsoleErrorDev = InternalTestUtils.assertConsoleErrorDev;
});

function Text(props) {
Expand Down Expand Up @@ -784,11 +786,14 @@ describe('Activity', () => {
// would be null because it was nulled out when it was deleted, but there
// was no null check before we accessed it. A weird edge case but we must
// account for it.
expect(() => {
setState('Updated');
}).toErrorDev(
"Can't perform a React state update on a component that hasn't mounted yet",
);
setState('Updated');
assertConsoleErrorDev([
"Can't perform a React state update on a component that hasn't mounted yet. " +
'This indicates that you have a side-effect in your render function that ' +
'asynchronously later calls tries to update the component. ' +
'Move this work to useEffect instead.\n' +
' in Child (at **)',
]);
});
expect(root).toMatchRenderedOutput(null);
});
Expand Down
153 changes: 120 additions & 33 deletions packages/react-reconciler/src/__tests__/ReactActWarnings-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ let Suspense;
let startTransition;
let getCacheForType;
let caches;
let assertConsoleErrorDev;

// These tests are mostly concerned with concurrent roots. The legacy root
// behavior is covered by other older test suites and is unchanged from
Expand All @@ -38,6 +39,7 @@ describe('act warnings', () => {
const InternalTestUtils = require('internal-test-utils');
waitForAll = InternalTestUtils.waitForAll;
assertLog = InternalTestUtils.assertLog;
assertConsoleErrorDev = InternalTestUtils.assertConsoleErrorDev;
});

function createTextCache() {
Expand Down Expand Up @@ -171,9 +173,21 @@ describe('act warnings', () => {

// Flag is true. Warn.
await withActEnvironment(true, async () => {
expect(() => setState(2)).toErrorDev(
'An update to App inside a test was not wrapped in act',
);
setState(2);
assertConsoleErrorDev([
'An update to App inside a test was not wrapped in act(...).\n' +
'\n' +
'When testing, code that causes React state updates should be wrapped into act(...):\n' +
'\n' +
'act(() => {\n' +
' /* fire events that update state */\n' +
'});\n' +
'/* assert on the output */\n' +
'\n' +
"This ensures that you're testing the behavior the user would see in the browser. " +
'Learn more at https://react.dev/link/wrap-tests-with-act\n' +
' in App (at **)',
]);
await waitForAll([2]);
expect(root).toMatchRenderedOutput('2');
});
Expand Down Expand Up @@ -202,12 +216,11 @@ describe('act warnings', () => {

// Default behavior. Flag is undefined. Warn.
expect(global.IS_REACT_ACT_ENVIRONMENT).toBe(undefined);
expect(() => {
act(() => {
setState(1);
});
}).toErrorDev(
'The current testing environment is not configured to support act(...)',
act(() => {
setState(1);
});
assertConsoleErrorDev(
['The current testing environment is not configured to support act(...)'],
{withoutStack: true},
);
assertLog([1]);
Expand All @@ -224,12 +237,13 @@ describe('act warnings', () => {

// Flag is false. Warn.
await withActEnvironment(false, () => {
expect(() => {
act(() => {
setState(1);
});
}).toErrorDev(
'The current testing environment is not configured to support act(...)',
act(() => {
setState(1);
});
assertConsoleErrorDev(
[
'The current testing environment is not configured to support act(...)',
],
{withoutStack: true},
);
assertLog([1]);
Expand All @@ -240,10 +254,23 @@ describe('act warnings', () => {
it('warns if root update is not wrapped', async () => {
await withActEnvironment(true, () => {
const root = ReactNoop.createRoot();
expect(() => root.render('Hi')).toErrorDev(
// TODO: Better error message that doesn't make it look like "Root" is
// the name of a custom component
'An update to Root inside a test was not wrapped in act(...)',
root.render('Hi');
assertConsoleErrorDev(
[
// TODO: Better error message that doesn't make it look like "Root" is
// the name of a custom component
'An update to Root inside a test was not wrapped in act(...).\n' +
'\n' +
'When testing, code that causes React state updates should be wrapped into act(...):\n' +
'\n' +
'act(() => {\n' +
' /* fire events that update state */\n' +
'});\n' +
'/* assert on the output */\n' +
'\n' +
"This ensures that you're testing the behavior the user would see in the browser. " +
'Learn more at https://react.dev/link/wrap-tests-with-act',
],
{withoutStack: true},
);
});
Expand All @@ -265,9 +292,21 @@ describe('act warnings', () => {
act(() => {
root.render(<App />);
});
expect(() => app.setState({count: 1})).toErrorDev(
'An update to App inside a test was not wrapped in act(...)',
);
app.setState({count: 1});
assertConsoleErrorDev([
'An update to App inside a test was not wrapped in act(...).\n' +
'\n' +
'When testing, code that causes React state updates should be wrapped into act(...):\n' +
'\n' +
'act(() => {\n' +
' /* fire events that update state */\n' +
'});\n' +
'/* assert on the output */\n' +
'\n' +
"This ensures that you're testing the behavior the user would see in the browser. " +
'Learn more at https://react.dev/link/wrap-tests-with-act\n' +
' in App (at **)',
]);
});
});

Expand All @@ -288,9 +327,21 @@ describe('act warnings', () => {

// Even though this update is synchronous, we should still fire a warning,
// because it could have spawned additional asynchronous work
expect(() => ReactNoop.flushSync(() => setState(1))).toErrorDev(
'An update to App inside a test was not wrapped in act(...)',
);
ReactNoop.flushSync(() => setState(1));
assertConsoleErrorDev([
'An update to App inside a test was not wrapped in act(...).\n' +
'\n' +
'When testing, code that causes React state updates should be wrapped into act(...):\n' +
'\n' +
'act(() => {\n' +
' /* fire events that update state */\n' +
'});\n' +
'/* assert on the output */\n' +
'\n' +
"This ensures that you're testing the behavior the user would see in the browser. " +
'Learn more at https://react.dev/link/wrap-tests-with-act\n' +
' in App (at **)',
]);

assertLog([1]);
expect(root).toMatchRenderedOutput('1');
Expand Down Expand Up @@ -322,12 +373,36 @@ describe('act warnings', () => {
expect(root).toMatchRenderedOutput('Loading...');

// This is a retry, not a ping, because we already showed a fallback.
expect(() => resolveText('Async')).toErrorDev(
resolveText('Async');
assertConsoleErrorDev(
[
'A suspended resource finished loading inside a test, but the event ' +
'was not wrapped in act(...)',

...(gate('enableSiblingPrerendering') ? ['not wrapped in act'] : []),
'A suspended resource finished loading inside a test, but the event was not wrapped in act(...).\n' +
'\n' +
'When testing, code that resolves suspended data should be wrapped into act(...):\n' +
'\n' +
'act(() => {\n' +
' /* finish loading suspended data */\n' +
'});\n' +
'/* assert on the output */\n' +
'\n' +
"This ensures that you're testing the behavior the user would see in the browser. " +
'Learn more at https://react.dev/link/wrap-tests-with-act',

...(gate('enableSiblingPrerendering')
? [
'A suspended resource finished loading inside a test, but the event was not wrapped in act(...).\n' +
'\n' +
'When testing, code that resolves suspended data should be wrapped into act(...):\n' +
'\n' +
'act(() => {\n' +
' /* finish loading suspended data */\n' +
'});\n' +
'/* assert on the output */\n' +
'\n' +
"This ensures that you're testing the behavior the user would see in the browser. " +
'Learn more at https://react.dev/link/wrap-tests-with-act',
]
: []),
],

{withoutStack: true},
Expand Down Expand Up @@ -363,9 +438,21 @@ describe('act warnings', () => {
expect(root).toMatchRenderedOutput('(empty)');

// This is a ping, not a retry, because no fallback is showing.
expect(() => resolveText('Async')).toErrorDev(
'A suspended resource finished loading inside a test, but the event ' +
'was not wrapped in act(...)',
resolveText('Async');
assertConsoleErrorDev(
[
'A suspended resource finished loading inside a test, but the event was not wrapped in act(...).\n' +
'\n' +
'When testing, code that resolves suspended data should be wrapped into act(...):\n' +
'\n' +
'act(() => {\n' +
' /* finish loading suspended data */\n' +
'});\n' +
'/* assert on the output */\n' +
'\n' +
"This ensures that you're testing the behavior the user would see in the browser. " +
'Learn more at https://react.dev/link/wrap-tests-with-act',
],
{withoutStack: true},
);
});
Expand Down
22 changes: 13 additions & 9 deletions packages/react-reconciler/src/__tests__/ReactAsyncActions-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ let useTransition;
let useState;
let useOptimistic;
let textCache;
let assertConsoleErrorDev;

describe('ReactAsyncActions', () => {
beforeEach(() => {
Expand All @@ -21,6 +22,8 @@ describe('ReactAsyncActions', () => {
Scheduler = require('scheduler');
act = require('internal-test-utils').act;
assertLog = require('internal-test-utils').assertLog;
assertConsoleErrorDev =
require('internal-test-utils').assertConsoleErrorDev;
useTransition = React.useTransition;
useState = React.useState;
useOptimistic = React.useOptimistic;
Expand Down Expand Up @@ -1231,15 +1234,16 @@ describe('ReactAsyncActions', () => {
assertLog(['A']);
expect(root).toMatchRenderedOutput(<div>A</div>);

await expect(async () => {
await act(() => {
setLoadingProgress('25%');
startTransition(() => setText('B'));
});
}).toErrorDev(
'An optimistic state update occurred outside a transition or ' +
'action. To fix, move the update to an action, or wrap ' +
'with startTransition.',
await act(() => {
setLoadingProgress('25%');
startTransition(() => setText('B'));
});
assertConsoleErrorDev(
[
'An optimistic state update occurred outside a transition or ' +
'action. To fix, move the update to an action, or wrap ' +
'with startTransition.',
],
{withoutStack: true},
);
assertLog(['Loading... (25%)', 'A', 'B']);
Expand Down
34 changes: 28 additions & 6 deletions packages/react-reconciler/src/__tests__/ReactFragment-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
let React;
let ReactNoop;
let waitForAll;
let assertConsoleErrorDev;

describe('ReactFragment', () => {
beforeEach(function () {
Expand All @@ -22,6 +23,7 @@ describe('ReactFragment', () => {

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

it('should render a single child via noop renderer', async () => {
Expand Down Expand Up @@ -740,9 +742,22 @@ describe('ReactFragment', () => {
await waitForAll([]);

ReactNoop.render(<Foo condition={false} />);
await expect(async () => await waitForAll([])).toErrorDev(
'Each child in a list should have a unique "key" prop.',
);
await waitForAll([]);
assertConsoleErrorDev([
gate('enableOwnerStacks')
? 'Each child in a list should have a unique "key" prop.\n' +
'\n' +
'Check the render method of `div`. ' +
'It was passed a child from Foo. ' +
'See https://react.dev/link/warning-keys for more information.\n' +
' in Foo (at **)'
: 'Each child in a list should have a unique "key" prop.\n' +
'\n' +
'Check the render method of `Foo`. ' +
'See https://react.dev/link/warning-keys for more information.\n' +
' in Stateful (at **)\n' +
' in Foo (at **)',
]);

expect(ops).toEqual([]);
expect(ReactNoop).toMatchRenderedOutput(
Expand Down Expand Up @@ -937,9 +952,16 @@ describe('ReactFragment', () => {
}

ReactNoop.render(<Foo condition={true} />);
await expect(async () => await waitForAll([])).toErrorDev(
'Each child in a list should have a unique "key" prop.',
);
await waitForAll([]);
assertConsoleErrorDev([
'Each child in a list should have a unique "key" prop.\n' +
'\n' +
'Check the top-level render call using <Foo>. ' +
'It was passed a child from Foo. ' +
'See https://react.dev/link/warning-keys for more information.\n' +
' in span (at **)\n' +
' in Foo (at **)',
]);

ReactNoop.render(<Foo condition={false} />);
// The key warning gets deduped because it's in the same component.
Expand Down
Loading
Loading