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

Enable throwSuggestions RTL configuration #3367

Merged
merged 7 commits into from
Oct 17, 2023
Merged

Enable throwSuggestions RTL configuration #3367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
69cc5e6
Enable `throwSuggestions`
amanmahajan7 Oct 16, 2023
d713b26
Fix tests
amanmahajan7 Oct 17, 2023
7a9a8bc
Fix sorting tests
amanmahajan7 Oct 17, 2023
f051f4d
Update test/setup.ts
amanmahajan7 Oct 17, 2023
6fec485
Merge branch 'main' into am-throwSuggestions
amanmahajan7 Oct 17, 2023
8aa06d0
Revert "Update test/setup.ts"
amanmahajan7 Oct 17, 2023
23ed47c
Use `toHaveTextContent`
amanmahajan7 Oct 17, 2023
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
12 changes: 8 additions & 4 deletions test/TreeDataGrid.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ test('should set aria-attributes', async () => {
test('should select rows in a group', async () => {
setup(['year', 'country']);

const headerCheckbox = screen.getByLabelText('Select All');
const headerCheckbox = screen.getByRole('checkbox', { name: 'Select All' });
expect(headerCheckbox).not.toBeChecked();

// expand group
Expand All @@ -249,7 +249,9 @@ test('should select rows in a group', async () => {
expect(screen.queryAllByRole('row', { selected: true })).toHaveLength(0);

// select parent row
await userEvent.click(within(groupCell1.parentElement!).getByLabelText('Select Group'));
await userEvent.click(
within(groupCell1.parentElement!).getByRole('checkbox', { name: 'Select Group' })
);
let selectedRows = screen.getAllByRole('row', { selected: true });
expect(selectedRows).toHaveLength(4);
expect(selectedRows[0]).toHaveAttribute('aria-rowindex', '6');
Expand All @@ -258,13 +260,15 @@ test('should select rows in a group', async () => {
expect(selectedRows[3]).toHaveAttribute('aria-rowindex', '10');

// unselecting child should unselect the parent row
await userEvent.click(within(selectedRows[3]).getByLabelText('Select'));
await userEvent.click(within(selectedRows[3]).getByRole('checkbox', { name: 'Select' }));
selectedRows = screen.getAllByRole('row', { selected: true });
expect(selectedRows).toHaveLength(1);
expect(selectedRows[0]).toHaveAttribute('aria-rowindex', '7');

// select child group
const checkbox = within(groupCell2.parentElement!).getByLabelText('Select Group');
const checkbox = within(groupCell2.parentElement!).getByRole('checkbox', {
name: 'Select Group'
});
await userEvent.click(checkbox);
selectedRows = screen.getAllByRole('row', { selected: true });
expect(selectedRows).toHaveLength(4);
Expand Down
46 changes: 23 additions & 23 deletions test/column/renderEditCell.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,25 @@ describe('Editor', () => {
it('should open editor on double click', async () => {
render(<EditorTest />);
await userEvent.click(getCellsAtRowIndex(0)[0]);
expect(screen.queryByLabelText('col1-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('spinbutton', { name: 'col1-editor' })).not.toBeInTheDocument();
Copy link
Contributor

Choose a reason for hiding this comment

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

What element are we actually querying here? I don't really understand what a spinbutton is

Copy link
Contributor Author

@amanmahajan7 amanmahajan7 Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

input type=number.

await userEvent.dblClick(getCellsAtRowIndex(0)[0]);
expect(screen.getByLabelText('col1-editor')).toHaveValue(1);
expect(screen.getByRole('spinbutton', { name: 'col1-editor' })).toHaveValue(1);
await userEvent.keyboard('2');
await userEvent.tab();
expect(screen.queryByLabelText('col1-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('spinbutton', { name: 'col1-editor' })).not.toBeInTheDocument();
expect(getCellsAtRowIndex(0)[0]).toHaveTextContent(/^21$/);
});

it('should open and commit changes on enter', async () => {
render(<EditorTest />);
await userEvent.click(getCellsAtRowIndex(0)[0]);
expect(screen.queryByLabelText('col1-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('spinbutton', { name: 'col1-editor' })).not.toBeInTheDocument();
await userEvent.keyboard('{enter}');
expect(screen.getByLabelText('col1-editor')).toHaveValue(1);
expect(screen.getByRole('spinbutton', { name: 'col1-editor' })).toHaveValue(1);
await userEvent.keyboard('3{enter}');
expect(getCellsAtRowIndex(0)[0]).toHaveTextContent(/^31$/);
expect(getCellsAtRowIndex(0)[0]).toHaveFocus();
expect(screen.queryByLabelText('col1-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('spinbutton', { name: 'col1-editor' })).not.toBeInTheDocument();
});

it('should open editor when user types', async () => {
Expand All @@ -47,17 +47,17 @@ describe('Editor', () => {
it('should close editor and discard changes on escape', async () => {
render(<EditorTest />);
await userEvent.dblClick(getCellsAtRowIndex(0)[0]);
expect(screen.getByLabelText('col1-editor')).toHaveValue(1);
expect(screen.getByRole('spinbutton', { name: 'col1-editor' })).toHaveValue(1);
await userEvent.keyboard('2222{escape}');
expect(screen.queryByLabelText('col1-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('spinbutton', { name: 'col1-editor' })).not.toBeInTheDocument();
expect(getCellsAtRowIndex(0)[0]).toHaveTextContent(/^1$/);
expect(getCellsAtRowIndex(0)[0]).toHaveFocus();
});

it('should commit changes and close editor when clicked outside', async () => {
render(<EditorTest />);
await userEvent.dblClick(getCellsAtRowIndex(0)[0]);
const editor = screen.getByLabelText('col1-editor');
const editor = screen.getByRole('spinbutton', { name: 'col1-editor' });
expect(editor).toHaveValue(1);
await userEvent.keyboard('2222');
await userEvent.click(screen.getByText('outside'));
Expand Down Expand Up @@ -103,9 +103,9 @@ describe('Editor', () => {

await scrollGrid({ scrollTop: 2000 });
expect(getCellsAtRowIndex(0)).toHaveLength(1);
expect(screen.queryByLabelText('col1-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('spinbutton', { name: 'col1-editor' })).not.toBeInTheDocument();
await userEvent.keyboard('123');
expect(screen.getByLabelText('col1-editor')).toHaveValue(1230);
expect(screen.getByRole('spinbutton', { name: 'col1-editor' })).toHaveValue(1230);
const spy = vi.spyOn(window.HTMLElement.prototype, 'scrollIntoView');
await userEvent.keyboard('{enter}');
expect(spy).toHaveBeenCalled();
Expand All @@ -117,38 +117,38 @@ describe('Editor', () => {
const cell = getCellsAtRowIndex(0)[1];
expect(cell).not.toHaveAttribute('aria-readonly');
await userEvent.dblClick(cell);
expect(screen.getByLabelText('col2-editor')).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: 'col2-editor' })).toBeInTheDocument();
});

it('should be editable if an editor is specified and editable is set to true', async () => {
render(<EditorTest editable />);
await userEvent.dblClick(getCellsAtRowIndex(0)[1]);
expect(screen.getByLabelText('col2-editor')).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: 'col2-editor' })).toBeInTheDocument();
});

it('should not be editable if editable is false', async () => {
render(<EditorTest editable={false} />);
const cell = getCellsAtRowIndex(0)[1];
expect(cell).toHaveAttribute('aria-readonly', 'true');
await userEvent.dblClick(cell);
expect(screen.queryByLabelText('col2-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('textbox', { name: 'col2-editor' })).not.toBeInTheDocument();
});

it('should not be editable if editable function returns false', async () => {
render(<EditorTest editable={(row) => row.col1 === 2} />);
await userEvent.dblClick(getCellsAtRowIndex(0)[1]);
expect(screen.queryByLabelText('col2-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('textbox', { name: 'col2-editor' })).not.toBeInTheDocument();

await userEvent.dblClick(getCellsAtRowIndex(1)[1]);
expect(screen.getByLabelText('col2-editor')).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: 'col2-editor' })).toBeInTheDocument();
});
});

describe('editorOptions', () => {
it('should detect outside click if editor is rendered in a portal', async () => {
render(<EditorTest createEditorPortal editorOptions={{ displayCellContent: true }} />);
await userEvent.dblClick(getCellsAtRowIndex(0)[1]);
const editor1 = screen.getByLabelText('col2-editor');
const editor1 = screen.getByRole('textbox', { name: 'col2-editor' });
expect(editor1).toHaveValue('a1');
await userEvent.keyboard('23');
// The cell value should update as the editor value is changed
Expand All @@ -164,7 +164,7 @@ describe('Editor', () => {
expect(getCellsAtRowIndex(0)[1]).not.toHaveFocus();

await userEvent.dblClick(getCellsAtRowIndex(0)[1]);
const editor2 = screen.getByLabelText('col2-editor');
const editor2 = screen.getByRole('textbox', { name: 'col2-editor' });
await userEvent.click(editor2);
await userEvent.keyboard('{enter}');
expect(getCellsAtRowIndex(0)[1]).toHaveFocus();
Expand All @@ -179,7 +179,7 @@ describe('Editor', () => {
/>
);
await userEvent.dblClick(getCellsAtRowIndex(0)[1]);
const editor = screen.getByLabelText('col2-editor');
const editor = screen.getByRole('textbox', { name: 'col2-editor' });
expect(editor).toBeInTheDocument();
await userEvent.click(screen.getByText('outside'));
await act(async () => {
Expand All @@ -205,7 +205,7 @@ describe('Editor', () => {
await userEvent.keyboard('yz{enter}');
expect(getCellsAtRowIndex(0)[1]).toHaveTextContent(/^a1yz$/);
await userEvent.keyboard('x');
expect(screen.queryByLabelText('col2-editor')).not.toBeInTheDocument();
expect(screen.queryByRole('textbox', { name: 'col2-editor' })).not.toBeInTheDocument();
});

it('should prevent navigation if onCellKeyDown prevents the default event', async () => {
Expand Down Expand Up @@ -283,9 +283,9 @@ describe('Editor', () => {
</>
);

const outerInput = screen.getByLabelText('outer-input');
const outerInput = screen.getByRole('textbox', { name: 'outer-input' });
await userEvent.dblClick(getCellsAtRowIndex(0)[0]);
const col1Input = screen.getByLabelText('col1-input');
const col1Input = screen.getByRole('textbox', { name: 'col1-input' });
expect(col1Input).toHaveFocus();
await userEvent.click(outerInput);
expect(outerInput).toHaveFocus();
Expand All @@ -295,7 +295,7 @@ describe('Editor', () => {
expect(outerInput).toHaveFocus();

await userEvent.dblClick(getCellsAtRowIndex(0)[1]);
const col2Input = screen.getByLabelText('col2-input');
const col2Input = screen.getByRole('textbox', { name: 'col2-input' });
expect(col2Input).toHaveFocus();
await userEvent.click(outerInput);
expect(outerInput).toHaveFocus();
Expand Down
4 changes: 2 additions & 2 deletions test/events.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe('Events', () => {
await userEvent.click(getCellsAtRowIndex(0)[0]);
expect(screen.queryByLabelText('col1-editor')).not.toBeInTheDocument();
await userEvent.click(getCellsAtRowIndex(0)[1]);
expect(screen.getByLabelText('col2-editor')).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: 'col2-editor' })).toBeInTheDocument();
});

it('should not open editor editor on double click if onCellDoubleClick prevents default', async () => {
Expand All @@ -100,7 +100,7 @@ describe('Events', () => {
await userEvent.dblClick(getCellsAtRowIndex(0)[0]);
expect(screen.queryByLabelText('col1-editor')).not.toBeInTheDocument();
await userEvent.dblClick(getCellsAtRowIndex(0)[1]);
expect(screen.getByLabelText('col2-editor')).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: 'col2-editor' })).toBeInTheDocument();
});

it('should call onCellContextMenu when cell is right clicked', async () => {
Expand Down
12 changes: 6 additions & 6 deletions test/rowSelection.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function testSelection(rowIdx: number, isSelected: boolean) {
}

async function toggleSelection(rowIdx: number, shift = false) {
const element = within(getCellsAtRowIndex(rowIdx)[0]).getByLabelText('Select');
const element = within(getCellsAtRowIndex(rowIdx)[0]).getByRole('checkbox', { name: 'Select' });
const user = userEvent.setup();
if (shift) await user.keyboard('{Shift>}');
await user.click(element);
Expand Down Expand Up @@ -81,7 +81,7 @@ test('toggle selection using keyboard', async () => {

test('select/deselect all rows when header checkbox is clicked', async () => {
setup();
const headerCheckbox = screen.getByLabelText('Select All');
const headerCheckbox = screen.getByRole('checkbox', { name: 'Select All' });
expect(headerCheckbox).not.toBeChecked();
await userEvent.click(headerCheckbox);
testSelection(0, true);
Expand All @@ -102,7 +102,7 @@ test('select/deselect all rows when header checkbox is clicked', async () => {

test('header checkbox is not checked when there are no rows', () => {
setup([]);
expect(screen.getByLabelText('Select All')).not.toBeChecked();
expect(screen.getByRole('checkbox', { name: 'Select All' })).not.toBeChecked();
});

test('header checkbox is not necessarily checked when selectedRows.size === rows.length', () => {
Expand All @@ -115,7 +115,7 @@ test('header checkbox is not necessarily checked when selectedRows.size === rows
/>
);

expect(screen.getByLabelText('Select All')).not.toBeChecked();
expect(screen.getByRole('checkbox', { name: 'Select All' })).not.toBeChecked();
});

test('header checkbox is not necessarily checked when selectedRows.size > rows.length', () => {
Expand All @@ -128,7 +128,7 @@ test('header checkbox is not necessarily checked when selectedRows.size > rows.l
/>
);

expect(screen.getByLabelText('Select All')).not.toBeChecked();
expect(screen.getByRole('checkbox', { name: 'Select All' })).not.toBeChecked();
});

test('extra keys are preserved when updating the selectedRows Set', async () => {
Expand Down Expand Up @@ -156,7 +156,7 @@ test('extra keys are preserved when updating the selectedRows Set', async () =>

render(<Test />);

const headerCheckbox = screen.getByLabelText('Select All');
const headerCheckbox = screen.getByRole('checkbox', { name: 'Select All' });

await toggleSelection(1);
expect(set).toStrictEqual(new Set([...initialSet, 2]));
Expand Down
3 changes: 3 additions & 0 deletions test/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { act } from 'react-dom/test-utils';
import { configure } from '@testing-library/react';

configure({ throwSuggestions: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

Duration 67.79s (transform 1.28s, setup 7.69s, collect 7.75s, tests 29.41s, environment 12.71s, prepare 2.69s)

The tests take over 1 minute now, I think it's fine to try it from time to time to improve tests, but I'm 👎 on keeping this enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How long does it take without this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

NVM I thought the GH CI was faster than that

Copy link
Contributor

Choose a reason for hiding this comment

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

We can add it back

amanmahajan7 marked this conversation as resolved.
Show resolved Hide resolved

// Allow node-environment tests to properly fail when accessing DOM APIs,
// as @testing-library/jest-dom may polyfill some DOM APIs like `window.CSS`
Expand Down
4 changes: 2 additions & 2 deletions test/sorting.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ function setup() {
}

function testSortColumns(expectedValue: readonly SortColumn[]) {
expect(JSON.parse(screen.getByTestId('sortColumnsValue').textContent!)).toStrictEqual(
expectedValue
expect(screen.getByTestId('sortColumnsValue', { suggest: false })).toHaveTextContent(
JSON.stringify(expectedValue)
);
}

Expand Down