Skip to content

test(Label): Refactor Label tests to use shallow rendering - #295

Merged
jeff-phillips-18 merged 1 commit into
patternfly:masterfrom
dmiller9911:Tests-DisposableLabel
Apr 9, 2018
Merged

jeff-phillips-18 merged 1 commit into
patternfly:masterfrom
dmiller9911:Tests-DisposableLabel

Conversation

@dmiller9911

Copy link
Copy Markdown
Contributor

What:

  • Modified tests under Label to use shallow rendering and targeted snapshots
  • Removed circular dependncy for Label and DisposableLabel. This fixes importing DisposableLabel directly.
  • Added tests for Label and RemoveButton

Link to Storybook:
N/A

Additional issues:
N/A

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 1097

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 71.812%

Totals Coverage Status
Change from base Build 1094: 0.1%
Covered Lines: 1256
Relevant Lines: 1583

💛 - Coveralls

@coveralls

coveralls commented Apr 2, 2018 •

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 1102

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 71.812%

Totals Coverage Status
Change from base Build 1098: 0.1%
Covered Lines: 1256
Relevant Lines: 1583

💛 - Coveralls

@jeff-phillips-18 jeff-phillips-18 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

expect(component.render()).toMatchSnapshot();
test('defaults props', () => {
const view = shallow(<DisposableLabel />);
expect(view).toMatchSnapshot();

@priley86 priley86 Apr 2, 2018 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do you want to start using names?
i.e. toMatchSnapshot('default props snapshot')); ? or fine w/ leaving those out? sorry was confused and started using them ;)

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.

It definitely does not hurt. The real benefit comes from when there are multiple snapshots in 1 test. By default, they will get the test's name. If there are multiple you just get a 1 2 3 etc. I think it would be a good idea though if we are going to request them for multiple to do them for single snapshots as well. That should cut down on the question of "when should I name the snapshot". Since it will be always it is easier to remember. Will update these.

* Modified tests under Label to use shallow rendering and targeted snapshots
* Removed circular dependncy for Label and DisposableLabel.  This fixes importing DisposableLabel directly.
* Added tests for Label and RemoveButton
@jeff-phillips-18
jeff-phillips-18 merged commit 77da4ac into patternfly:master Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants