Skip to content

Conversation

thetaPC
Copy link
Contributor

@thetaPC thetaPC commented Feb 14, 2024

Issue number: N/A


What is the current behavior?

GitHub actions allow screenshots to be updated either by updating all the components or providing a component name.

If a component name is given, then the screenshots will only be updated on src/components/{component-name}. This isn't ideal when wanting to update a specific path.

What is the new behavior?

  • The script no longer strips out ion-. The developer needs to submit a valid component name or valid path.
  • The script no longer appends the value to src/components/
  • The script will run all instances of a component if only the name is given
  • The script will run tests within a given path

Does this introduce a breaking change?

  • Yes
  • No

Other information

How to test:

  1. Create a new branch based off this one
  2. Make a style change
  3. Push it to the new branch
  4. Update the reference images using the new branch
  5. This will show you all the options

Recommended to test:

  • a component name (chip)
  • a path (src/components/chip/test/a11y)
  • a component that doesn't exist (random-component)
  • no provided component, leave the first input blank

@thetaPC thetaPC marked this pull request as ready for review February 14, 2024 01:09
@thetaPC thetaPC requested a review from a team as a code owner February 14, 2024 01:09
@thetaPC thetaPC requested review from brandyscarney and removed request for a team February 14, 2024 01:09
Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

I ran the update screenshots action with the following inputs:

  • badge - Runs: npx playwright test src/components/badge
  • src/components/badge/test/basic - Runs: npx playwright test src/components/src/components/badge/test/basic
  • badge tabs - Runs: npx playwright test src/components/badge src/components/tabs
  • unknown - Runs: npx playwright test src/components/unknown
  • blank - Runs: npx playwright test

The only one that didn't work as expected is the 2nd one. It appended the entire path onto src/components.

@thetaPC thetaPC requested a review from brandyscarney March 8, 2024 18:31
Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

Running the update screenshots action with latest results in the following:

Looks good to me! 👍 Let me know if you need me to test it after the Docker update is merged in, but I don't think it should be affected.

@thetaPC thetaPC requested a review from brandyscarney March 18, 2024 22:51
@thetaPC
Copy link
Contributor Author

thetaPC commented Mar 18, 2024

@brandyscarney just to be safe, let's test it with the docker changes.

Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

Still works with Docker 👍

@thetaPC thetaPC added this pull request to the merge queue Mar 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 19, 2024
@thetaPC thetaPC added this pull request to the merge queue Mar 19, 2024
@thetaPC thetaPC removed this pull request from the merge queue due to a manual request Mar 19, 2024
@thetaPC thetaPC added this pull request to the merge queue Mar 19, 2024
Merged via the queue into main with commit 73d661a Mar 19, 2024
@thetaPC thetaPC deleted the gh-tests branch March 19, 2024 17:35
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.

2 participants