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

Divider: switch to snapshot tests #3579

Merged
merged 2 commits into from
Nov 8, 2022
Merged

Divider: switch to snapshot tests #3579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ffd10d
test: switch to snapshot tests
kalinkrustev Nov 4, 2022
30c544c
Merge pull request #8 from primefaces/master
kalinkrustev Nov 4, 2022
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
92 changes: 13 additions & 79 deletions components/lib/divider/Divider.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,84 +2,18 @@ import '@testing-library/jest-dom';
import { render } from '@testing-library/react';
import { Divider } from './Divider';

describe('Divider', () => {
test('when component has no properties it returns with default class', () => {
// Arrange
const { container } = render(<Divider />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-horizontal p-divider-solid p-divider-left').length).toBe(1);
});
test('when layout and align as property it returns with class', () => {
// Arrange
const { container } = render(<Divider layout={'horizontal'} align={'left'} />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-horizontal p-divider-left').length).toBe(1);
});
test('when layout and align as property it returns with class', () => {
// Arrange
const { container } = render(<Divider layout={'horizontal'} align={'right'} />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-horizontal p-divider-right').length).toBe(1);
});
test('when layout and align as property it returns with class', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you notice you repeated the same test message? It is good for the test message to reflect the case you are testing.

// Arrange
const { container } = render(<Divider layout={'horizontal'} align={'center'} />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-horizontal p-divider-center').length).toBe(1);
});

test('when tpe has not any property it returns with default class', () => {
// Arrange
const { container } = render(<Divider />);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original test is more complicated, than it need to be. Did you notice you have the same test multiple times?

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed this. But here I see the benefit of the function you wrote more than the snapshot benefit. You have created html code for all cases. The only thing we want to check is the existence of a class in render. If we created the same function for the old structure, which one would you think is advantageous? Please let me know if there's anything I missed. I will be happy to learn.

I am talking about this function:
const snapshot = (props, name) => expect(render(<Divider {...props} />).container).toMatchSnapshot(name);

Copy link
Member

Choose a reason for hiding this comment

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

So the real advantage of Snapshot testing over this current test. lets take for example this:

expect(container.getElementsByClassName('p-divider p-component p-divider-horizontal p-divider-center').length).toBe(1);

This test will NOT fail if the following is changed:

  1. If Divider is changed from a DIV to a SPAN this test would still pass and it should not the structure of the component has changed.
  2. If a new style class p-new-class was introduced in the component this test would still pass because the current query will still be true. And to me this test should fail if a NEW class is added to the component or an unexpected one is there like p-filled when it should not be etc.

So the benefits to me are tremendous.


// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-horizontal p-divider-solid').length).toBe(1);
});
test('when layout and align as property it returns with class', () => {
// Arrange
const { container } = render(<Divider layout={'vertical'} align={'center'} />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-vertical p-divider-center').length).toBe(1);
});
test('when layout and align as property it returns with class', () => {
// Arrange
const { container } = render(<Divider layout={'vertical'} align={'top'} />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-vertical p-divider-top').length).toBe(1);
});
test('when layout and align as property it returns with class', () => {
// Arrange
const { container } = render(<Divider layout={'vertical'} align={'bottom'} />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-vertical p-divider-bottom').length).toBe(1);
});

test('when type has no property it returns with the default class', () => {
// Arrange
const { container } = render(<Divider />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-solid').length).toBe(1);
});
test('when type is dashed it is styled as a dashed line', () => {
// Arrange
const { container } = render(<Divider type={'dashed'} />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-dashed').length).toBe(1);
});
test('when type is dotted is is styled as a dotted line', () => {
// Arrange
const { container } = render(<Divider type={'dotted'} />);

// Act + Assert
expect(container.getElementsByClassName('p-divider p-component p-divider-dotted').length).toBe(1);
const snapshot = (props, name) => expect(render(<Divider {...props} />).container).toMatchSnapshot(name);

describe('Divider Snapshot', () => {
test('check snapshots', () => {
snapshot({}, 'default');
snapshot({ layout: 'horizontal', align: 'left' }, 'horizontal left');
snapshot({ layout: 'horizontal', align: 'right' }, 'horizontal right');
snapshot({ layout: 'horizontal', align: 'center' }, 'horizontal center');
snapshot({ layout: 'vertical', align: 'center' }, 'vertical center');
snapshot({ layout: 'vertical', align: 'top' }, 'vertical top');
snapshot({ layout: 'vertical', align: 'bottom' }, 'vertical bottom');
snapshot({ type: 'dashed' }, 'dashed');
snapshot({ type: 'dotted' }, 'dotted');
});
});
118 changes: 118 additions & 0 deletions components/lib/divider/__snapshots__/Divider.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Divider Snapshot check snapshots: dashed 1`] = `
<div>
<div
class="p-divider p-component p-divider-horizontal p-divider-dashed p-divider-left"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;

exports[`Divider Snapshot check snapshots: default 1`] = `
<div>
<div
class="p-divider p-component p-divider-horizontal p-divider-solid p-divider-left"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;

exports[`Divider Snapshot check snapshots: dotted 1`] = `
<div>
<div
class="p-divider p-component p-divider-horizontal p-divider-dotted p-divider-left"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;

exports[`Divider Snapshot check snapshots: horizontal center 1`] = `
<div>
<div
class="p-divider p-component p-divider-horizontal p-divider-solid p-divider-center"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;

exports[`Divider Snapshot check snapshots: horizontal left 1`] = `
<div>
<div
class="p-divider p-component p-divider-horizontal p-divider-solid p-divider-left"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;

exports[`Divider Snapshot check snapshots: horizontal right 1`] = `
<div>
<div
class="p-divider p-component p-divider-horizontal p-divider-solid p-divider-right"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;

exports[`Divider Snapshot check snapshots: vertical bottom 1`] = `
<div>
<div
class="p-divider p-component p-divider-vertical p-divider-solid p-divider-bottom"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;

exports[`Divider Snapshot check snapshots: vertical center 1`] = `
<div>
<div
class="p-divider p-component p-divider-vertical p-divider-solid p-divider-center"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;

exports[`Divider Snapshot check snapshots: vertical top 1`] = `
<div>
<div
class="p-divider p-component p-divider-vertical p-divider-solid p-divider-top"
role="separator"
>
<div
class="p-divider-content"
/>
</div>
</div>
`;