Skip to content

Commit

Permalink
Upgrade user-event to v14 (#1605)
Browse files Browse the repository at this point in the history
* Upgrade user-event to v14

* Address breaking changes in Popover spec

- removed the mocked document.createRange. This isn't necessary anymore. See testing-library/user-event#902
- awaited userEvent calls, removed unnecessary act blocks
- fixed type errors around icon types
- note: there are still act warnings for some tests in the specs, even some not involving any state updates (style snapshots). Probably points to unwanted rerenders. Added a fixme comment.

* Address breaking changes in useEscapeKey

- exposed the renderHook method from @testing-library/react, meant to replace the onoe from @testing-library/react-hooks, which isn't compatible with React 18. For migration purposes, I'm naming it newRenderHook--I'll remove the legacy method and rename the new one after all specs have been migrated
- awaited userEvent calls and removed unnecessary act blocks

* Fix accessibility error in ToastContext

The live region wrapping toasts should not be a ul, because uls son't allow the status role (likely because it overrides list semantics anyways. Switched for divs.

Found through a better axe test

* Migrate user-event in NotificationToast spec

- await userEvent method calls
- added a new render helper to render a component tree including the ToastProvider, to reduce test boilerplate when testing business logic
- improved the accesibility test to cover an actual document with a toast instead of the toast UI in isolation

* WIP: upgrade user-event in SidePanel components

- awaited userEvent methods and removed unnecessary act blocks
- migrated useSidePanel to newRenderHook and removed unnecessary actHook blocks
- added ariaHideApp=false to default test props in SidePanelContext to silence react-modal warning about the undefined app element
- set up userEvent with delay=null in SidePanelContext to address this issue: testing-library/user-event#833
- TODO: two tests using userEvent.keyboard() are still failing. Marked as todo.
- TODO: there are two act() warnings left (likely from before) when running `yarn test sidepanel` (all SidePanel-related specs)

* Patch keyCode event in SidePanel spec

This also adds comments on accessibility tests that inconsistently trigger axe warnings. Will be investigated and addressed separately.

* Migrate Button, RadioButton, RadioButtonGroup, Header

Awaited userEvent calls.

* Migrate remaining input components

Checkbox, CurrencyInput, Selector.

Awaited userEvent method calls

* Migrate notification components

Awaited userEvent method calls

* Migrate modal components

- awaited userEvent method calls
- set up userEvent with delay=null in ModalContext spec (same as in SidePanelContext)

* Migrate more components

Hamburger, Pagination, PageList, UtilityLinks, useClickOutside.

Awaited userEvent method calls.

Does not include the migration to new renderHook for useClickOutside, will be handled separately.

* Use userEvent with delay=null in ToastProvider spec

Same as in the ModalProvider and SidePanelProvider

* Migrate userEvent in useAutoExpand

- awaited method calls
- migrated {space} to { }
- TODO: migrate to the new renderHook, will be addressed separately

* Migrate userEvent in remaining components

- await method calls
- fix some TS errors

* Migrate useStep to renderHook from RTL

- replace imports
- replace removed waitForNextUpdate by a more explicit waitFor
- remove tests covering error logic. Error testing was removed in the renderHook port to RTL and we're not covering this in other components, so I think it's fine to remove.

See testing-library/react-testing-library#991 for more context

* Migrate renderHook from RHTL to new RTL implementation

See testing-library/react-testing-library#991 for context.

- replaced import of renderHook to newRenderHook (will be renamed in a follow-up commit)
- replaced actHook by act
- (edge cases were handled in earlier commits)

* Rename newRenderHook to renderHook

* Await remaining userEvent promises

Not sure why these specs passed locally before

* Address review comment
  • Loading branch information
Robin Métral committed Jun 14, 2022
1 parent 2ac245b commit 384cedb
Show file tree
Hide file tree
Showing 62 changed files with 609 additions and 819 deletions.
5 changes: 5 additions & 0 deletions .changeset/fresh-wolves-beam.md
@@ -0,0 +1,5 @@
---
'@sumup/circuit-ui': patch
---

Switched the `ToastContext`'s live region element from a `ul` to a `div`: lists shouldn't have `role="status"` since this strips list semantics.
13 changes: 4 additions & 9 deletions packages/circuit-ui/components/Anchor/Anchor.spec.tsx
Expand Up @@ -22,7 +22,6 @@ import {
renderToHtml,
axe,
RenderFn,
act,
userEvent,
} from '../../util/test-utils';
import { ClickEvent } from '../../types/events';
Expand Down Expand Up @@ -82,7 +81,7 @@ describe('Anchor', () => {
expect(actual).toBeVisible();
});

it('should call the onClick handler when rendered as a link', () => {
it('should call the onClick handler when rendered as a link', async () => {
const props = {
...baseProps,
'href': 'https://sumup.com',
Expand All @@ -93,24 +92,20 @@ describe('Anchor', () => {
};
const { getByTestId } = renderAnchor(render, props);

act(() => {
userEvent.click(getByTestId('anchor'));
});
await userEvent.click(getByTestId('anchor'));

expect(props.onClick).toHaveBeenCalledTimes(1);
});

it('should call the onClick handler when rendered as a button', () => {
it('should call the onClick handler when rendered as a button', async () => {
const props = {
...baseProps,
'onClick': jest.fn(),
'data-testid': 'anchor',
};
const { getByTestId } = renderAnchor(render, props);

act(() => {
userEvent.click(getByTestId('anchor'));
});
await userEvent.click(getByTestId('anchor'));

expect(props.onClick).toHaveBeenCalledTimes(1);
});
Expand Down
7 changes: 2 additions & 5 deletions packages/circuit-ui/components/Button/Button.spec.tsx
Expand Up @@ -22,7 +22,6 @@ import {
renderToHtml,
axe,
RenderFn,
act,
userEvent,
} from '../../util/test-utils';

Expand Down Expand Up @@ -135,17 +134,15 @@ describe('Button', () => {
expect(getByText(loadingLabel)).toBeVisible();
});

it('should call the onClick handler when clicked', () => {
it('should call the onClick handler when clicked', async () => {
const props = {
...baseProps,
'onClick': jest.fn(),
'data-testid': 'link-button',
};
const { getByTestId } = renderButton(render, props);

act(() => {
userEvent.click(getByTestId('link-button'));
});
await userEvent.click(getByTestId('link-button'));

expect(props.onClick).toHaveBeenCalledTimes(1);
});
Expand Down
Expand Up @@ -51,7 +51,7 @@ describe('CardHeader', () => {
expect(closeButton).toHaveTextContent(closeButtonLabel);
});

it('should call the onClose prop when the close button is clicked', () => {
it('should call the onClose prop when the close button is clicked', async () => {
const onClose = jest.fn();

const { getByRole } = render(
Expand All @@ -61,7 +61,7 @@ describe('CardHeader', () => {
);
const closeButton = getByRole('button');

userEvent.click(closeButton);
await userEvent.click(closeButton);

expect(onClose).toHaveBeenCalledTimes(1);
});
Expand Down
7 changes: 2 additions & 5 deletions packages/circuit-ui/components/Checkbox/Checkbox.spec.tsx
Expand Up @@ -20,7 +20,6 @@ import {
render,
renderToHtml,
axe,
act,
userEvent,
} from '../../util/test-utils';

Expand Down Expand Up @@ -87,7 +86,7 @@ describe('Checkbox', () => {
expect(inputEl).not.toHaveAttribute('checked');
});

it('should call the change handler when clicked', () => {
it('should call the change handler when clicked', async () => {
const { getByLabelText } = render(
<Checkbox noMargin {...defaultProps}>
Label
Expand All @@ -97,9 +96,7 @@ describe('Checkbox', () => {
exact: false,
});

act(() => {
userEvent.click(inputEl);
});
await userEvent.click(inputEl);

expect(defaultProps.onChange).toHaveBeenCalledTimes(1);
});
Expand Down
6 changes: 3 additions & 3 deletions packages/circuit-ui/components/Checkbox/Checkbox.stories.tsx
Expand Up @@ -30,10 +30,10 @@ const interactionTasks: PublicInteractionTask[] = [
name: 'Tick checkbox',
description: 'Click the checkbox and wait for the label to change',
run: async ({ container }: InteractionTaskArgs): Promise<void> => {
const checkbox: HTMLElement | null = container.querySelector(
const checkbox = container.querySelector(
'input[type=checkbox]',
);
userEvent.click(checkbox);
) as HTMLInputElement;
await userEvent.click(checkbox);
await findByText(container, 'Checked');
},
},
Expand Down
Expand Up @@ -21,7 +21,6 @@ import {
render,
renderToHtml,
axe,
act,
userEvent,
} from '../../util/test-utils';
import { InputProps } from '../Input';
Expand Down Expand Up @@ -70,35 +69,31 @@ describe('CurrencyInput', () => {
expect(tref.current).toBe(input);
});

it('should format a en-GB amount correctly', () => {
it('should format a en-GB amount correctly', async () => {
const { getByLabelText } = render(
<CurrencyInput locale="en-GB" currency="EUR" label="Amount" noMargin />,
);

const input = getByLabelText(/Amount/) as HTMLInputElement;

act(() => {
userEvent.type(input, '1234.56');
});
await userEvent.type(input, '1234.56');

expect(input.value).toBe('1,234.56');
});

it('should format a de-DE amount correctly', () => {
it('should format a de-DE amount correctly', async () => {
const { getByLabelText } = render(
<CurrencyInput locale="de-DE" currency="EUR" label="Amount" noMargin />,
);

const input = getByLabelText(/Amount/) as HTMLInputElement;

act(() => {
userEvent.type(input, '1234,56');
});
await userEvent.type(input, '1234,56');

expect(input.value).toBe('1.234,56');
});

it('should format an amount in a controlled input with an initial numeric value', () => {
it('should format an amount in a controlled input with an initial numeric value', async () => {
const ControlledCurrencyInput = () => {
const [value, setValue] = useState<CurrencyInputProps['value']>(1234.5);
return (
Expand All @@ -119,10 +114,8 @@ describe('CurrencyInput', () => {
const input = getByLabelText(/Amount/) as HTMLInputElement;
expect(input.value).toBe('1.234,5');

act(() => {
userEvent.clear(input);
userEvent.type(input, '1234,56');
});
await userEvent.clear(input);
await userEvent.type(input, '1234,56');

expect(input.value).toBe('1.234,56');
});
Expand Down
7 changes: 2 additions & 5 deletions packages/circuit-ui/components/Hamburger/Hamburger.spec.tsx
Expand Up @@ -18,7 +18,6 @@ import {
renderToHtml,
axe,
render,
act,
userEvent,
RenderFn,
} from '../../util/test-utils';
Expand Down Expand Up @@ -58,17 +57,15 @@ describe('Hamburger', () => {
/**
* Logic tests.
*/
it('should call the onClick prop when clicked', () => {
it('should call the onClick prop when clicked', async () => {
const onClick = jest.fn();
const { getByTestId } = renderHamburger(render, {
...baseProps,
onClick,
'data-testid': 'hamburger',
});

act(() => {
userEvent.click(getByTestId('hamburger'));
});
await userEvent.click(getByTestId('hamburger'));

expect(onClick).toHaveBeenCalledTimes(1);
});
Expand Down
10 changes: 5 additions & 5 deletions packages/circuit-ui/components/ImageInput/ImageInput.spec.tsx
Expand Up @@ -148,7 +148,7 @@ describe('ImageInput', () => {
const { getByLabelText } = render(<StatefulInput />);
const inputEl = getByLabelText(defaultProps.label) as HTMLInputElement;

userEvent.upload(inputEl, file);
await userEvent.upload(inputEl, file);

await waitFor(() => {
expect(inputEl.files && inputEl.files[0]).toEqual(file);
Expand Down Expand Up @@ -188,7 +188,7 @@ describe('ImageInput', () => {
const inputEl = getByLabelText(defaultProps.label) as HTMLInputElement;
const imageEl = getByRole('img') as HTMLImageElement;

userEvent.upload(inputEl, file);
await userEvent.upload(inputEl, file);

await waitFor(() => {
expect(imageEl.src).toBe(
Expand All @@ -202,15 +202,15 @@ describe('ImageInput', () => {
const inputEl = getByLabelText(defaultProps.label) as HTMLInputElement;
const imageEl = getByRole('img') as HTMLImageElement;

userEvent.upload(inputEl, file);
await userEvent.upload(inputEl, file);

await waitFor(() => {
expect(imageEl.src).toBe(
'https://source.unsplash.com/EcWFOYOpkpY/200x200',
);
});

userEvent.click(
await userEvent.click(
getByRole('button', { name: defaultProps.clearButtonLabel }),
);

Expand All @@ -229,7 +229,7 @@ describe('ImageInput', () => {
const { getByLabelText, getByText } = render(<StatefulInput />);
const inputEl = getByLabelText(defaultProps.label) as HTMLInputElement;

userEvent.upload(inputEl, file);
await userEvent.upload(inputEl, file);

await waitFor(() => {
expect(getByText(errorMessage)).toBeVisible();
Expand Down
15 changes: 6 additions & 9 deletions packages/circuit-ui/components/ListItem/ListItem.spec.tsx
Expand Up @@ -13,16 +13,15 @@
* limitations under the License.
*/

import { createRef } from 'react';
import { SumUpCard } from '@sumup/icons';
import { createRef, FC } from 'react';
import { IconProps, SumUpCard } from '@sumup/icons';

import {
create,
render,
renderToHtml,
axe,
RenderFn,
act,
userEvent,
} from '../../util/test-utils';
import Body from '../Body';
Expand Down Expand Up @@ -56,7 +55,7 @@ describe('ListItem', () => {
it('should render a ListItem with a leading icon', () => {
const wrapper = renderListItem(create, {
...baseProps,
leadingComponent: SumUpCard,
leadingComponent: SumUpCard as FC<IconProps>,
});
expect(wrapper).toMatchSnapshot();
});
Expand Down Expand Up @@ -197,16 +196,14 @@ describe('ListItem', () => {
expect(wrapper).toMatchSnapshot();
});

it('should call the onClick handler when clicked', () => {
it('should call the onClick handler when clicked', async () => {
const props = {
...baseProps,
onClick: jest.fn(),
};
const { getByRole } = renderListItem(render, props);

act(() => {
userEvent.click(getByRole('button'));
});
await userEvent.click(getByRole('button'));

expect(props.onClick).toHaveBeenCalledTimes(1);
});
Expand All @@ -227,7 +224,7 @@ describe('ListItem', () => {
const wrapper = renderListItem(renderToHtml, {
...baseProps,
variant: 'navigation',
leadingComponent: SumUpCard,
leadingComponent: SumUpCard as FC<IconProps>,
details: 'Details',
trailingLabel: 'Trailing label',
trailingDetails: 'Trailing details',
Expand Down
Expand Up @@ -21,7 +21,6 @@ import {
renderToHtml,
axe,
RenderFn,
act,
userEvent,
} from '../../util/test-utils';
import Body from '../Body';
Expand Down Expand Up @@ -116,7 +115,7 @@ describe('ListItemGroup', () => {
expect(container).toMatchSnapshot();
});

it('should render the focused item in a ListItemGroup with interactive items', () => {
it('should render the focused item in a ListItemGroup with interactive items', async () => {
const { getAllByRole } = renderListItemGroup(render, {
...baseProps,
items: baseProps.items.map((item) => ({
Expand All @@ -125,10 +124,8 @@ describe('ListItemGroup', () => {
})),
});

act(() => {
userEvent.tab();
userEvent.tab(); // blur first and focus second item
});
await userEvent.tab();
await userEvent.tab(); // blur first and focus second item

expect(getAllByRole('button')[1]).toMatchSnapshot();
});
Expand Down
10 changes: 4 additions & 6 deletions packages/circuit-ui/components/Modal/Modal.spec.tsx
Expand Up @@ -13,7 +13,7 @@
* limitations under the License.
*/

import { render, act, userEvent, axe, waitFor } from '../../util/test-utils';
import { render, userEvent, axe, waitFor } from '../../util/test-utils';

import { Modal, ModalProps } from './Modal';

Expand All @@ -24,7 +24,7 @@ describe('Modal', () => {
closeButtonLabel: 'Close modal',
onClose: jest.fn(),
// eslint-disable-next-line react/prop-types, react/display-name
children: () => <p data-testid="children">Hello world!</p>,
children: <p data-testid="children">Hello world!</p>,
// Silences the warning about the missing app element.
// In user land, the modal is always rendered by the ModalProvider,
// which takes care of setting the app element.
Expand All @@ -45,12 +45,10 @@ describe('Modal', () => {
});
});

it('should call the onClose callback', () => {
it('should call the onClose callback', async () => {
const { getByRole } = render(<Modal {...defaultModal} />);

act(() => {
userEvent.click(getByRole('button'));
});
await userEvent.click(getByRole('button'));

expect(defaultModal.onClose).toHaveBeenCalled();
});
Expand Down

1 comment on commit 384cedb

@vercel
Copy link

@vercel vercel bot commented on 384cedb Jun 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.