Skip to content

chore(ActionList): update tests to new standards#7669

Merged
nicolethoen merged 1 commit intopatternfly:mainfrom
wise-king-sullyman:action-list-test-revamp
Jul 8, 2022
Merged

chore(ActionList): update tests to new standards#7669
nicolethoen merged 1 commit intopatternfly:mainfrom
wise-king-sullyman:action-list-test-revamp

Conversation

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator

What: Closes #7665

Additional issues:

@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Jul 7, 2022

test('Renders with class pf-c-action-list', () => {
render(<ActionList>Test</ActionList>);

expect(screen.getByText('Test')).toHaveClass('pf-c-action-list');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be reasonable to combine this test with the last since they are rendering the same thing?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and there are a couple other below that also render the same thing?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I definitely wouldn't call it unreasonable to combine them, but I personally would advocate for a testing approach that only makes assertions about one aspect of component behavior in each test.

The primary reason being that I believe single behavior tests provide maximum utility from the test suite, as with this approach a failing test (or combination of them) can often point you straight to where a regression was introduced.

Copy link
Copy Markdown
Contributor

@kmcfaul kmcfaul left a comment

Choose a reason for hiding this comment

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

like the descriptive test case names 👍

@nicolethoen nicolethoen merged commit 02da009 into patternfly:main Jul 8, 2022
@wise-king-sullyman wise-king-sullyman deleted the action-list-test-revamp branch July 8, 2022 20:11
@patternfly-build
Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • eslint-plugin-patternfly-react@4.63.11
  • @patternfly/react-catalog-view-extension@4.75.11
  • @patternfly/react-charts@6.77.11
  • @patternfly/react-code-editor@4.65.11
  • @patternfly/react-console@4.75.11
  • @patternfly/react-core@4.224.11
  • @patternfly/react-docs@5.85.11
  • @patternfly/react-icons@4.75.11
  • @patternfly/react-inline-edit-extension@4.69.11
  • demo-app-ts@4.184.11
  • @patternfly/react-integration@4.186.11
  • @patternfly/react-log-viewer@4.69.11
  • @patternfly/react-styles@4.74.11
  • @patternfly/react-table@4.93.11
  • @patternfly/react-tokens@4.76.11
  • @patternfly/react-topology@4.71.11
  • @patternfly/react-virtualized-extension@4.71.11
  • transformer-cjs-imports@4.62.11

Thanks for your contribution! 🎉

jenny-s51 pushed a commit to jenny-s51/patternfly-react that referenced this pull request Jul 26, 2022
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.

Action list test revamp

4 participants