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

test(pie-link): DSW-2229 updated visual tests to split by size #1952

Merged
merged 7 commits into from
Oct 1, 2024

Conversation

JoshuaNg2332
Copy link
Contributor

@JoshuaNg2332 JoshuaNg2332 commented Sep 25, 2024

Describe your changes (can list changeset entries if preferable)

  • Updated Pie Link tests to split the variations up by size.
  • Screenshots should increase from 3 to 6 in number

This is to resolve issues such as:

  • Screenshotted variations being cropped due to too large of a screenshot
  • Improve screenshot readability and reviewability.

Author Checklist (complete before requesting a review)

  • I have performed a self-review of my code
  • I have reviewed visual test updates properly before approving

Reviewer checklists (complete before approving)

Reviewer 1 @maledr5

  • If there are visual test updates, I have reviewed them

Reviewer 2 @kevinrodrigues

  • If there are visual test updates, I have reviewed them

@JoshuaNg2332 JoshuaNg2332 requested review from a team as code owners September 25, 2024 15:52
Copy link

changeset-bot bot commented Sep 25, 2024

⚠️ No Changeset found

Latest commit: b0050c3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

maledr5
maledr5 previously approved these changes Sep 27, 2024
Copy link
Contributor

@maledr5 maledr5 left a comment

Choose a reason for hiding this comment

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

Change looks good. I checked in Percy and shows missing images that I assume are the old ones, but confused on why it shows as missing instead of removed. Is this a known issue to us?

@xander-marjoram
Copy link
Contributor

@maledr5 Yep "missing" is just the terminology used. As long as we're happy that only the removed screenshots are missing then that's all good.

Copy link
Contributor

@siggerzz siggerzz left a comment

Choose a reason for hiding this comment

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

Minor comment

@JoshuaNg2332 JoshuaNg2332 merged commit 8710b68 into main Oct 1, 2024
38 of 41 checks passed
@JoshuaNg2332 JoshuaNg2332 deleted the DSW-2229-split-link-screenshots branch October 1, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants