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

Jump to bottom

Upgrade user event #4426

Merged
merged 24 commits into from
Sep 8, 2023
Merged

Upgrade user event #4426

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
25c5edd
Upgrade testing tools
snowystinger Apr 18, 2023
b8aa4ba
fix alphabetical order
snowystinger Apr 18, 2023
35a31af
remove all legacy timers
snowystinger Apr 18, 2023
e972690
fix lint
snowystinger Apr 18, 2023
cbf086f
fix lint for real
snowystinger Apr 18, 2023
030e4a9
fix test
snowystinger Apr 18, 2023
ac7dec8
fix snapshot tests
snowystinger Apr 18, 2023
543e19b
Fix 16 and 17 tests
snowystinger Apr 18, 2023
587ae76
fix some alphabet
snowystinger Apr 19, 2023
b6cf711
Upgrade user event
snowystinger Apr 22, 2023
599f183
Merge branch 'main' into upgrade-jest
snowystinger Apr 25, 2023
685a3bc
Merge branch 'upgrade-jest' into upgrade-user-event
snowystinger Apr 25, 2023
3d1827e
Merge branch 'main' into upgrade-user-event
snowystinger Jul 10, 2023
cf09d5d
Merge branch 'main' into upgrade-user-event
snowystinger Jul 10, 2023
5177271
Merge branch 'main' into upgrade-user-event
snowystinger Aug 17, 2023
55bdb71
Fix react 17 tests
snowystinger Aug 22, 2023
1c2b5c8
Merge branch 'main' into upgrade-user-event
snowystinger Aug 22, 2023
d2f735f
remove extra code
snowystinger Aug 24, 2023
5bb4fec
Merge branch 'main' into upgrade-user-event
snowystinger Aug 28, 2023
bf353e2
Merge branch 'main' into upgrade-user-event
snowystinger Aug 29, 2023
4412507
fix new test
snowystinger Aug 30, 2023
2a5058d
Merge branch 'main' into upgrade-user-event
snowystinger Sep 4, 2023
587d31b
Merge branch 'main' into upgrade-user-event
snowystinger Sep 6, 2023
870eb8d
Merge branch 'main' into upgrade-user-event
snowystinger Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
"@testing-library/dom": "^9.2.0",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^14.0.0",
"@testing-library/user-event": "^12.1.3",
"@testing-library/user-event": "^14.4.3",
"@types/react": "^18.2.7",
"@types/storybook__react": "^5.2.1",
"@typescript-eslint/eslint-plugin": "^5.40.0",
Expand Down
23 changes: 14 additions & 9 deletions packages/@react-aria/checkbox/test/useCheckboxGroup.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
* governing permissions and limitations under the License.
*/

import {act, render} from '@react-spectrum/test-utils';
import {AriaCheckboxGroupItemProps, AriaCheckboxGroupProps} from '@react-types/checkbox';
import {CheckboxGroupState, useCheckboxGroupState} from '@react-stately/checkbox';
import {pointerMap, render} from '@react-spectrum/test-utils';
import React, {useRef} from 'react';
import {useCheckboxGroup, useCheckboxGroupItem} from '../';
import userEvent from '@testing-library/user-event';
Expand All @@ -38,7 +38,12 @@ function CheckboxGroup({groupProps, checkboxProps}: {groupProps: AriaCheckboxGro
}

describe('useCheckboxGroup', () => {
it('handles defaults', () => {
let user;
beforeAll(() => {
user = userEvent.setup({delay: null, pointerMap});
Copy link
Member

Choose a reason for hiding this comment

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

wonder if we can handle this setup in the global beforeAll step or do this setup + return the user object via a util function call. Would make it easier for external users to do the same setup when we eventually expose these test-utils.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that, decided it was better to do it individually to make it easier to swap out users if we need to. Not saying this couldn't be improved, I'm just not sure setting up one globally beforeAll is the best way to go about it. A util function call seems fine, I was thinking something like

import {keyboardUser, mixedPointerKeyboardUser...} from 'whatever-util'

One interesting thing we could possibly do with this is enforce a specific interaction type for a set of tests. I think I we don't supply pointers to the pointerMap, then the user can't user mouse, so they would have to either use touch or keyboard only. But I haven't tried it yet.

});

it('handles defaults', async () => {
let onChangeSpy = jest.fn();
let {getByRole, getAllByRole, getByLabelText} = render(
<CheckboxGroup
Expand Down Expand Up @@ -68,7 +73,7 @@ describe('useCheckboxGroup', () => {
expect(checkboxes[2].checked).toBeFalsy();

let dragons = getByLabelText('Dragons');
userEvent.click(dragons);
await user.click(dragons);
expect(onChangeSpy).toHaveBeenCalledTimes(1);
expect(onChangeSpy).toHaveBeenCalledWith(['dragons']);

Expand Down Expand Up @@ -158,7 +163,7 @@ describe('useCheckboxGroup', () => {
expect(checkboxGroup).toHaveAttribute('data-testid', 'favorite-pet');
});

it('sets aria-disabled and makes checkboxes disabled when isDisabled is true', () => {
it('sets aria-disabled and makes checkboxes disabled when isDisabled is true', async () => {
let groupOnChangeSpy = jest.fn();
let checkboxOnChangeSpy = jest.fn();
let {getAllByRole, getByRole, getByLabelText} = render(
Expand All @@ -180,7 +185,7 @@ describe('useCheckboxGroup', () => {
expect(checkboxes[2]).toHaveAttribute('disabled');
let dragons = getByLabelText('Dragons');

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

expect(groupOnChangeSpy).toHaveBeenCalledTimes(0);
expect(checkboxOnChangeSpy).toHaveBeenCalledTimes(0);
Expand Down Expand Up @@ -227,7 +232,7 @@ describe('useCheckboxGroup', () => {
expect(checkboxes[2]).not.toHaveAttribute('disabled');
});

it('sets aria-readonly="true" on each checkbox', () => {
it('sets aria-readonly="true" on each checkbox', async () => {
let groupOnChangeSpy = jest.fn();
let checkboxOnChangeSpy = jest.fn();
let {getAllByRole, getByLabelText} = render(
Expand All @@ -247,14 +252,14 @@ describe('useCheckboxGroup', () => {
expect(checkboxes[2].checked).toBeFalsy();
let dragons = getByLabelText('Dragons');

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

expect(groupOnChangeSpy).toHaveBeenCalledTimes(0);
expect(checkboxOnChangeSpy).toHaveBeenCalledTimes(0);
expect(checkboxes[2].checked).toBeFalsy();
});

it('should not update state for readonly checkbox', () => {
it('should not update state for readonly checkbox', async () => {
let groupOnChangeSpy = jest.fn();
let checkboxOnChangeSpy = jest.fn();
let {getAllByRole, getByLabelText} = render(
Expand All @@ -270,7 +275,7 @@ describe('useCheckboxGroup', () => {
let checkboxes = getAllByRole('checkbox') as HTMLInputElement[];
let dragons = getByLabelText('Dragons');

userEvent.click(dragons);
await user.click(dragons);

expect(groupOnChangeSpy).toHaveBeenCalledTimes(0);
expect(checkboxOnChangeSpy).toHaveBeenCalledTimes(0);
Expand Down
22 changes: 12 additions & 10 deletions packages/@react-aria/color/test/useColorWheel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* governing permissions and limitations under the License.
*/

import {act, fireEvent, installMouseEvent, installPointerEvent, render} from '@react-spectrum/test-utils';
import {act, fireEvent, installMouseEvent, installPointerEvent, pointerMap, render} from '@react-spectrum/test-utils';
import {ColorWheelProps} from '@react-types/color';
import {parseColor, useColorWheelState} from '@react-stately/color';
import React, {useRef} from 'react';
Expand Down Expand Up @@ -50,8 +50,10 @@ function ColorWheel(props: ColorWheelProps) {

describe('useColorWheel', () => {
let onChangeSpy = jest.fn();
let user;

beforeAll(() => {
user = userEvent.setup({delay: null, pointerMap});
jest.useFakeTimers();
});

Expand All @@ -72,7 +74,7 @@ describe('useColorWheel', () => {
expect(slider).toHaveAttribute('value', '0');
});

it('the slider is focusable', () => {
it('the slider is focusable', async () => {
let {getAllByRole, getByRole} = render(<div>
<button>A</button>
<ColorWheel />
Expand All @@ -81,17 +83,17 @@ describe('useColorWheel', () => {
let slider = getByRole('slider');
let [buttonA, buttonB] = getAllByRole('button');

userEvent.tab();
await user.tab();
expect(document.activeElement).toBe(buttonA);
userEvent.tab();
await user.tab();
expect(document.activeElement).toBe(slider);
userEvent.tab();
await user.tab();
expect(document.activeElement).toBe(buttonB);
userEvent.tab({shift: true});
await user.tab({shift: true});
expect(document.activeElement).toBe(slider);
});

it('disabled', () => {
it('disabled', async () => {
let {getAllByRole, getByRole} = render(<div>
<button>A</button>
<ColorWheel isDisabled />
Expand All @@ -101,11 +103,11 @@ describe('useColorWheel', () => {
let [buttonA, buttonB] = getAllByRole('button');
expect(slider).toHaveAttribute('disabled');

userEvent.tab();
await user.tab();
expect(document.activeElement).toBe(buttonA);
userEvent.tab();
await user.tab();
expect(document.activeElement).toBe(buttonB);
userEvent.tab({shift: true});
await user.tab({shift: true});
expect(document.activeElement).toBe(buttonA);
});

Expand Down