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

chore(Checkbox): updated tests #9756

Merged
merged 4 commits into from
Oct 27, 2023
Merged

chore(Checkbox): updated tests #9756

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
914e6d3
chore(Checkbox): updated tests
wise-king-sullyman Oct 17, 2023
437c1f8
Update packages/react-core/src/components/Checkbox/__tests__/Checkbox…
wise-king-sullyman Oct 23, 2023
6a9564e
chore(checkbox): address PR feedback
wise-king-sullyman Oct 23, 2023
edd9cdb
chore(checkbox): update default behavior tests to address feedback
wise-king-sullyman Oct 23, 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
292 changes: 223 additions & 69 deletions packages/react-core/src/components/Checkbox/__tests__/Checkbox.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,91 +6,245 @@ import userEvent from '@testing-library/user-event';
import { Checkbox } from '../Checkbox';
import styles from '@patternfly/react-styles/css/components/Check/check';

describe('Checkbox', () => {
test('controlled', () => {
const { asFragment } = render(<Checkbox isChecked id="check" aria-label="check" />);
expect(asFragment()).toMatchSnapshot();
});
test(`Renders with only the class ${styles.checkInput} on the check by default`, () => {
render(<Checkbox id="test-id" />);

test('controlled - 3rd state', () => {
const { asFragment } = render(<Checkbox isChecked={null} id="check" aria-label="check" />);
expect(asFragment()).toMatchSnapshot();
});
expect(screen.getByRole('checkbox')).toHaveClass(styles.checkInput, { exact: true });
});

test('uncontrolled', () => {
const { asFragment } = render(<Checkbox id="check" aria-label="check" />);
expect(asFragment()).toMatchSnapshot();
});
test(`Renders with only the classes ${styles.check} and ${styles.modifiers.standalone} on the check wrapper by default`, () => {
render(<Checkbox id="test-id" />);

test('isDisabled', () => {
const { asFragment } = render(<Checkbox id="check" isDisabled aria-label="check" />);
expect(asFragment()).toMatchSnapshot();
expect(screen.getByRole('checkbox').parentElement).toHaveClass(`${styles.check} ${styles.modifiers.standalone}`, {
exact: true
});
});

test('label is string', () => {
const { asFragment } = render(<Checkbox label="Label" id="check" isChecked aria-label="check" />);
expect(asFragment()).toMatchSnapshot();
});
test('Renders with additional classes passed via className', () => {
render(<Checkbox id="test-id" className="test-class" />);

test('label is function', () => {
const functionLabel = () => <h1>Header</h1>;
const { asFragment } = render(<Checkbox label={functionLabel()} id="check" isChecked aria-label="check" />);
expect(asFragment()).toMatchSnapshot();
});
expect(screen.getByRole('checkbox').parentElement).toHaveClass('test-class');
});

test('label is node', () => {
const { asFragment } = render(<Checkbox label={<h1>Header</h1>} id="check" isChecked aria-label="check" />);
expect(asFragment()).toMatchSnapshot();
});
test('Renders with additional classes passed via inputClassName', () => {
render(<Checkbox id="test-id" inputClassName="test-class" />);

test('passing class', () => {
const { asFragment } = render(
<Checkbox label="label" className="class-123" id="check" isChecked aria-label="check" />
);
expect(asFragment()).toMatchSnapshot();
});
expect(screen.getByRole('checkbox')).toHaveClass('test-class');
});

test('passing HTML attribute', () => {
const { asFragment } = render(
<Checkbox label="label" aria-labelledby="labelId" id="check" isChecked aria-label="check" />
);
expect(asFragment()).toMatchSnapshot();
});
test('Does not set the checkbox as invalid by default', () => {
render(<Checkbox id="test-id" />);

test('passing description', () => {
render(<Checkbox id="check" label="checkbox" description="Text description..." />);
expect(screen.getByText('Text description...')).toBeInTheDocument();
});
expect(screen.getByRole('checkbox')).toBeValid();
});

test('passing body', () => {
render(<Checkbox id="check" label="checkbox" body="This is where custom content goes." />);
test('Sets the checkbox as invalid when isValid is false', () => {
render(<Checkbox id="test-id" isValid={false} />);

expect(screen.getByText('This is where custom content goes.')).toBeInTheDocument();
});
expect(screen.getByRole('checkbox')).toBeInvalid();
});

test('checkbox onChange handler called when component is clicked', async () => {
const onChangeHandler = jest.fn();
const user = userEvent.setup();
test('Does not set the checkbox as disabled by default', () => {
render(<Checkbox id="test-id" />);

render(<Checkbox id="check" onChange={onChangeHandler} aria-label="check" isChecked={false} />);
expect(screen.getByRole('checkbox')).not.toBeDisabled();
});

await user.click(screen.getByLabelText('check'));
expect(onChangeHandler).toHaveBeenCalled();
});
test(`Sets the checkbox as disabled when isDisabled is passed`, () => {
thatblindgeye marked this conversation as resolved.
Show resolved Hide resolved
render(<Checkbox id="test-id" isDisabled />);

test('should throw console error when no id is given', () => {
const myMock = jest.fn();
global.console = { ...global.console, error: myMock };
expect(screen.getByRole('checkbox')).toBeDisabled();
});

render(<Checkbox id={undefined} />);
expect(myMock).toHaveBeenCalled();
});
test('Sets the label as disabled when isDisabled and label are passed', () => {
render(<Checkbox id="test-id" isDisabled label="test label" />);

test('renders component wrapper as span', () => {
const { container } = render(
<Checkbox component="span" label="label" aria-labelledby="labelId" id="check" isChecked aria-label="check" />
);
const span = container.querySelector('span');
expect(span).toHaveClass(styles.check);
});
expect(screen.getByLabelText('test label')).toBeDisabled();
});

test('Does not set the checkbox as required by default', () => {
render(<Checkbox id="test-id" />);

expect(screen.getByRole('checkbox')).not.toBeRequired();
});

test(`Sets the checkbox as required when isRequired is passed`, () => {
render(<Checkbox id="test-id" isRequired />);

expect(screen.getByRole('checkbox')).toBeRequired();
});

test('Does not set the checkbox as checked by default', () => {
render(<Checkbox id="test-id" />);

expect(screen.getByRole('checkbox')).not.toBeChecked();
});

test(`Sets the checkbox as checked when isChecked is passed`, () => {
render(<Checkbox id="test-id" isChecked />);

expect(screen.getByRole('checkbox')).toBeChecked();
});

test(`Sets the checkbox as checked when checked is passed`, () => {
render(<Checkbox id="test-id" checked />);

expect(screen.getByRole('checkbox')).toBeChecked();
});

test(`Calls onChange when the checkbox is clicked`, async () => {
const user = userEvent.setup();
const onChange = jest.fn();
render(<Checkbox id="test-id" onChange={onChange} />);

await user.click(screen.getByRole('checkbox'));

expect(onChange).toHaveBeenCalledTimes(1);
});

test('Does not call onChange when the checkbox is not clicked', () => {
const onChange = jest.fn();
render(<Checkbox id="test-id" onChange={onChange} />);

expect(onChange).not.toHaveBeenCalled();
});

test(`Calls onChange with the event and the checked value when the checkbox is clicked`, async () => {
const user = userEvent.setup();
const onChange = jest.fn();
render(<Checkbox id="test-id" onChange={onChange} />);

await user.click(screen.getByRole('checkbox'));

expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ type: 'change' }), true);
});

test(`Calls onChange with the event and the checked value when the checkbox is clicked and isChecked is passed`, async () => {
const user = userEvent.setup();
const onChange = jest.fn();
render(<Checkbox id="test-id" isChecked onChange={onChange} />);

await user.click(screen.getByRole('checkbox'));

expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ type: 'change' }), false);
});

test('Does not render a label by default', () => {
render(<Checkbox id="test-id" />);

expect(screen.queryByLabelText('test label')).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.

This would apply to the description and body tests as well, but I'm always unsure of what tests like this are expecting just because technically they're passing, but they'd also pass even if label="another test label" were passed in. So less that a label doesn't render and more that a label with this specific test doesn't render.

At the same time I'm not sure how pretty an alternative would be. Using something like getByRole(checkbox).parentElement.querySelector(.${styles.checkLabel}) may work but is lengthy.

Probably splitting hairs too much so I don't think I'd really block over it, but curious what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I don't have a hardline stance on this and happy to remove if you think they're unnecessary, but my reasoning for doing these kinds of tests is that it ensure that assertion in the following test (where we expect the label to exist) is actually valid and not giving a false positive.

Copy link
Contributor

@thatblindgeye thatblindgeye Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Since it'd accept a regex, what about expect(screen.queryByLabelText(/\w+/)).not.toBeInTheDocument();? We don't really care what the label text actually is, just whether it's rendered, so this would result in a failure if any sort of label text were to be found (rather than currently a failure only occurs if label text that is "test label" is found). Probably wouldn't work for the body and description tests, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm not against that idea at all.


test('Renders a label when label is passed', () => {
render(<Checkbox id="test-id" label="test label" />);

expect(screen.getByLabelText('test label')).toBeVisible();
});

test('Associates the label with the checkbox', () => {
render(<Checkbox id="test-id" label="test label" />);

expect(screen.getByRole('checkbox')).toHaveAccessibleName('test label');
});

test('Does not render an asterisk when a label is passed but isRequired is not', () => {
render(<Checkbox id="test-id" label="test label" />);

expect(screen.queryByText('*')).not.toBeInTheDocument();
});

test('Renders an asterisk when isRequired and a label are passed', () => {
render(<Checkbox id="test-id" isRequired label="test label" />);

expect(screen.getByText('*')).toBeVisible();
});

test(`Wraps the required asterisk in the ${styles.checkLabelRequired} className`, () => {
render(<Checkbox id="test-id" isRequired label="test label" />);

expect(screen.getByText('*')).toHaveClass(styles.checkLabelRequired, { exact: true });
});

test('Renders with the provided id', () => {
render(<Checkbox id="test-id" />);

expect(screen.getByRole('checkbox')).toHaveAttribute('id', 'test-id');
});

test('Does not render an aria-label by default', () => {
render(<Checkbox id="test-id" />);

expect(screen.getByRole('checkbox')).not.toHaveAttribute('aria-label');
});

test('Sets the name to the passed aria-label', () => {
render(<Checkbox id="test-id" aria-label="test aria-label" />);

expect(screen.getByRole('checkbox')).toHaveAccessibleName('test aria-label');
});

test('Does not render a description by default', () => {
render(<Checkbox id="test-id" />);

expect(screen.queryByText('test description')).not.toBeInTheDocument();
});

test('Renders a description when description is passed', () => {
render(<Checkbox id="test-id" description="test description" />);

expect(screen.getByText('test description')).toBeVisible();
});

test(`Renders the passed description with the ${styles.checkDescription} className`, () => {
render(<Checkbox id="test-id" description="test description" />);

expect(screen.getByText('test description')).toHaveClass(styles.checkDescription, { exact: true });
});

test('Does not render a body by default', () => {
render(<Checkbox id="test-id" />);

expect(screen.queryByText('test body')).not.toBeInTheDocument();
});

test('Renders a body when body is passed', () => {
render(<Checkbox id="test-id" body="test body" />);

expect(screen.getByText('test body')).toBeVisible();
});

test(`Renders the passed body with the ${styles.checkBody} className`, () => {
render(<Checkbox id="test-id" body="test body" />);

expect(screen.getByText('test body')).toHaveClass(styles.checkBody, { exact: true });
});

test('Renders the check wrapper as a div by default', () => {
render(<Checkbox id="test-id" />);

expect(screen.getByRole('checkbox').parentElement?.tagName).toBe('DIV');
});

test('Renders with the provided component', () => {
render(<Checkbox id="test-id" component="span" />);

expect(screen.getByRole('checkbox').parentElement?.tagName).toBe('SPAN');
});

test(`Spreads additional props`, () => {
render(<Checkbox id="test-id" data-testid="test-id" />);

expect(screen.getByTestId('test-id')).toBeInTheDocument();
});

test(`Sets the checkbox as checked by default when defaultChecked is passed`, () => {
render(<Checkbox id="test-id" defaultChecked />);

expect(screen.getByRole('checkbox')).toBeChecked();
});

test('Matches snapshot', () => {
const { asFragment } = render(<Checkbox id="test-id" ouiaId="ouia-id" />);

expect(asFragment()).toMatchSnapshot();
});
Loading
Loading