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

Build: Fix E2E and chromatic inconsistencies #23051

Merged
merged 6 commits into from
Jun 14, 2023
Merged

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Jun 13, 2023

Closes #

What I did

This PR solves a few sources of flakiness.

Viewports E2E test

It seemed to be testing the wrong element, which playwright assumed had the same dimensions either without or with viewports enabled:

image

image

Chromatic

The "Desktop" component stories had a counter which was causing inconsistencies, and now it has chromatic ignore regions to avoid it:
image

The "DocsPage" story has a very inconsistent args table row which keeps changing in order, therefore I added a play function which injects a chromatic ignore region to it. I did it in the play function and not in the component so that we don't always ignore snapshots for the argstable component, and we really just ignore in the place we know is inconsistent.

Additionally I made the "play function banner" (we display only when snapshotting stories in Chromatic that have play function) more prominent and explanatory

image

The "TS Argtypes" suffers a similar issue, where a minified function name keeps changing, so I added a similar mechanism using the play function.

How to test

Tests should work correctly

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@yannbf yannbf added build Internal-facing build tooling & test updates ci:merged Run the CI jobs that normally run when merged. and removed ci:merged Run the CI jobs that normally run when merged. labels Jun 13, 2023
@yannbf yannbf force-pushed the fix/viewports-e2e-test branch from 48abd77 to b2d03d5 Compare June 14, 2023 07:03
@yannbf yannbf changed the title Build: Fix addon viewports e2e test Build: Fix E2E and chromatic inconsistencies Jun 14, 2023
@yannbf yannbf added the ci:daily Run the CI jobs that normally run in the daily job. label Jun 14, 2023
Copy link
Contributor

@kasperpeulen kasperpeulen left a comment

Choose a reason for hiding this comment

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

Nice work man!

@yannbf yannbf force-pushed the fix/viewports-e2e-test branch from b2ccb65 to 8e239f1 Compare June 14, 2023 08:26
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Nice one @yannbf 👏

@shilman shilman merged commit d00fe88 into next Jun 14, 2023
@shilman shilman deleted the fix/viewports-e2e-test branch June 14, 2023 08:59
@yannbf yannbf added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Jun 14, 2023
shilman added a commit that referenced this pull request Jun 14, 2023
Build: Fix E2E and chromatic inconsistencies
(cherry picked from commit d00fe88)
kasperpeulen pushed a commit that referenced this pull request Jun 14, 2023
Build: Fix E2E and chromatic inconsistencies
(cherry picked from commit d00fe88)
kasperpeulen pushed a commit that referenced this pull request Jun 14, 2023
Build: Fix E2E and chromatic inconsistencies
(cherry picked from commit d00fe88)
@JReinhold JReinhold added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Internal-facing build tooling & test updates ci:daily Run the CI jobs that normally run in the daily job. patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants