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.

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

Refactor Button component test file to new format #3353

Merged
merged 48 commits into from
Nov 13, 2024

Conversation

SriHV
Copy link
Contributor

@SriHV SriHV commented Sep 13, 2024

What is the context of this PR?

139

Refactored the button macro.spec file as per the new format.

How to review this PR

Ask yourself....

  • Do we need any additional tests?
  • Are the tests descriptions clear?
  • Does the order of the test document make sense?
  • Is it internally consistent?
  • Is it consistent with the guidelines for test refactoring?

run this test command to view the test results:
yarn test --testNamePattern="FOR: button"

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

@SriHV SriHV self-assigned this Sep 13, 2024
Copy link

netlify bot commented Sep 13, 2024

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit 309688a
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/6734cb658d683200085fc51a
😎 Deploy Preview https://deploy-preview-3353--ons-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@SriHV SriHV linked an issue Sep 13, 2024 that may be closed by this pull request
@SriHV SriHV added the Tech improvements Tech debt, cleanup, code standardisation etc. label Sep 13, 2024
@SriHV SriHV requested a review from a team September 17, 2024 09:56
@SriHV SriHV marked this pull request as ready for review September 17, 2024 09:56
Copy link
Contributor

@rmccar rmccar left a comment

Choose a reason for hiding this comment

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

These are mainly just minor things but it would be good to be consistent with the way we are writing these tests. We need to make sure we use the correct names for params and the tests describe what they are testing

src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
src/components/button/_macro.spec.js Outdated Show resolved Hide resolved
@rmccar rmccar changed the title Refactor button test file to new format Refactor button component test file to new format Sep 19, 2024
@rmccar rmccar changed the title Refactor button component test file to new format Refactor Button component test file to new format Sep 27, 2024
@rmccar rmccar added the Do not merge Don't merge this PR until this label is removed label Oct 2, 2024
@rmccar rmccar removed the Do not merge Don't merge this PR until this label is removed label Oct 9, 2024
@rmccar
Copy link
Contributor

rmccar commented Nov 8, 2024

There are now a couple of new tests being added in #3418 that will need to be incorporated into this

@rmccar
Copy link
Contributor

rmccar commented Nov 8, 2024

Ive realised we are missing tests for the following params, I think we need to add them:

  • name
  • value
  • iconType
  • listeners

rmccar
rmccar previously approved these changes Nov 13, 2024
@rmccar rmccar dismissed their stale review November 13, 2024 09:31

One more minor comment

@SriHV SriHV merged commit 1f4b6bc into main Nov 13, 2024
25 checks passed
@SriHV SriHV deleted the enhancement/139/testing-refactor-button branch November 13, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tech improvements Tech debt, cleanup, code standardisation etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Testing refactor - Button
4 participants