Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,8 +1,74 @@
import * as React from 'react';
import { render } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import { Avatar } from '../Avatar';

test('simple avatar', () => {
const { asFragment } = render(<Avatar alt="avatar" src="test.png" border="light" />);
test('Renders simple avatar', () => {
render(
<div data-testid="avatar">
<Avatar alt="avatar" />
</div>
);
expect(screen.getByTestId('avatar').firstChild).toBeVisible();
});

test('Renders without any modifier class when border and size props are not passed', () => {
render(<Avatar alt="avatar" />);
expect(screen.getByRole('img')).toHaveClass('pf-c-avatar', { exact: true });
});

test('Renders with class name pf-m-light when "light" is passed as border prop', () => {
render(<Avatar alt="avatar" border="light" />);
expect(screen.getByRole('img')).toHaveClass('pf-m-light');
});

test('Renders with class name pf-m-dark when "dark" is passed as border prop', () => {
render(<Avatar alt="avatar" border="dark" />);
expect(screen.getByRole('img')).toHaveClass('pf-m-dark');
});

test('Renders with class name pf-m-xl when "xl" is passed as size prop', () => {
render(<Avatar alt="avatar" size="xl" />);
expect(screen.getByRole('img')).toHaveClass('pf-m-xl');
});

test('Renders with class name pf-m-lg when "lg" is passed as size prop', () => {
render(<Avatar alt="avatar" size="lg" />);
expect(screen.getByRole('img')).toHaveClass('pf-m-lg');
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Additional things I would like to see tested:

  • That it renders with pf-c-avatar
  • That the className prop functions
  • That the src prop functions
  • That the alt prop functions
  • That non-explicitly declared props (such as aria-label in this case) can be applied as expected
  • A snapshot test of the component to prevent unintended changes of the default behavior

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also one note: typically we want each test to ideally only include props that are either required or under test, so a lot of the props in use in the tests you have here can be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hopefully all the required changes have been resolved. Thanks for the feedback. I am not quite sure about the way the src test is carried out though, let me know what you think.

test('Renders with class name pf-m-md when "md" is passed as size prop', () => {
render(<Avatar alt="avatar" size="md" />);
expect(screen.getByRole('img')).toHaveClass('pf-m-md');
});

test('Renders with class name pf-m-sm when "sm" is passed as size prop', () => {
render(<Avatar alt="avatar" size="sm" />);
expect(screen.getByRole('img')).toHaveClass('pf-m-sm');
});

test('Renders with custom class name when className prop is passed', () => {
render(<Avatar alt="avatar" className="test-class" />);
expect(screen.getByRole('img')).toHaveClass('test-class');
});

test('Renders with passed src prop', () => {
render(<Avatar alt="avatar" src="test.png" />);
const image = screen.getByRole('img') as HTMLImageElement;
expect(image.src).toMatch('test.png');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this was a great way of testing that src works properly!

});

test('Renders with passed alt prop', () => {
render(<Avatar alt="avatar" />);
expect(screen.getByRole('img')).toHaveProperty('alt', 'avatar');
});

test('Renders with passed aria-label prop', () => {
render(<Avatar alt="avatar" aria-label="Avatar test" />);
expect(screen.getByRole('img')).toHaveAccessibleName('Avatar test');
});

test('Matches the snapshot', () => {
const { asFragment } = render(<Avatar alt="avatar" aria-label="Avatar test" src="test.png" className="test-class" />);

expect(asFragment()).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`simple avatar 1`] = `
exports[`Matches the snapshot 1`] = `
<DocumentFragment>
<img
alt="avatar"
class="pf-c-avatar pf-m-light"
aria-label="Avatar test"
class="pf-c-avatar test-class"
src="test.png"
/>
</DocumentFragment>
Expand Down