-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Fix #3577: Add jest unit testing to the Divider component / Add Rating and ToggleButton components Docs #3578
Conversation
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
import { Divider } from './Divider'; | ||
|
||
describe('Divider', () => { | ||
test('when tpe has not any property it returns with default class', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when component has no properties it returns with default class
const { container } = render(<Divider />); | ||
|
||
// Act + Assert | ||
expect(container.getElementsByClassName('p-divider p-component p-divider-horizontal').length).toBe(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better way to test for this is
expect(container).toHaveClass('p-divider p-component p-divider-horizontal');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make this change throughout this whole test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using https://jestjs.io/docs/snapshot-testing ?
I think it is more convenient, as the test code is separate from the expected output, so you do not need to maintain these classes in the test files. When something changes, it is usually more clear what the changes are when observing them in the snapshot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. I don't think it's a UI test. It tests the effect of certain logic on the UI and gets help from classes. For this reason, I could not associate it with snapshot tests. But if you'd like to send a sample, we'd be happy to review it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I will send you an example what I mean. It is capturing the generated markup and making sure it does not change unexpectedly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check here: #3579
expect(container.getElementsByClassName('p-divider p-component p-divider-vertical p-divider-bottom').length).toBe(1); | ||
}); | ||
|
||
test('when type has not any property it returns with default class', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when type has no property it returns with the default class
// Act + Assert | ||
expect(container.getElementsByClassName('p-divider p-component p-divider-solid').length).toBe(1); | ||
}); | ||
test('when type as property it returns with class', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when type is dashed it is styled as a dashed line
// Act + Assert | ||
expect(container.getElementsByClassName('p-divider p-component p-divider-dashed').length).toBe(1); | ||
}); | ||
test('when type as property it returns with class', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when type is dotted is is styled as a dotted line
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>. |
Feature Requests
Add jest unit testing to the Divider component