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

Be more strategic with our Percy snapshot quota #14779

Closed
2 of 3 tasks
tay1orjones opened this issue Oct 4, 2023 · 1 comment · Fixed by #14886
Closed
2 of 3 tasks

Be more strategic with our Percy snapshot quota #14779

tay1orjones opened this issue Oct 4, 2023 · 1 comment · Fixed by #14886

Comments

@tay1orjones
Copy link
Member

tay1orjones commented Oct 4, 2023

We have a very significant breadth of visual states within the system's components that we need to ensure remain stable and consistent from release to release. Additionally, PRs need to be reviewed to ensure that any visual changes are intentional and not a new defect.

To accomplish this we currently use Percy. Playwright is used to visit each story in the storybook, visually snapshot it, and send the snapshot to be diffed via Percy against baseline snapshots of what's currently in main.

The issue

We have a cap of how many screenshots we can use per month. We need to uncover ways we could be more strategic with the snapshot quota without increasing risk of defects, or negatively impacting the feedback loop of reviewing snapshots on every PR.

Right now, every time Percy diffs snapshots it's roughly 1000 snapshots towards our quota.

Ideas to explore

Ideas marked with ✅ are ones we think could be worth attempting

  • Create a testing story to test all variants together in one snapshot instead of disparate snapshots
    • This could make the diffs really difficult to parse and read when something goes wrong
    • Maybe we just test the playground stories instead of every story for every component
    • There's quite a bit of duplication between default stories and playground stories (usually)
  • ✅ Only test a subset of the themes
    • Unless we're changing tokens, the token ones aren't really that valuable. Do we need g90 and g100? There's more variants in the light theme. Could we get by with one light theme and one dark theme? Do we have usage metrics on which themes are used the least and would constitute the lowest risk of defects per usage?
  • Stop testing every PR and batch run VRT
    • This pretty well destorys the feedback loop on PRs
    • Makes it very very difficult to figure out why a change happened, and from what PR it resulted in.
  • ✅ Could we specify less breakpoints per theme?
    • Most components don't radically change their composition in different breakpoint sizes, using different tokens, etc. We're not changing colors or tokens per breakpoint.
    • Test in max width in all 4 themes and then only test mobile in the most used theme
      • This would reduce the snapshots per story down to 5 instead of 8
        • 4 themes, 2 breakpoints each down to 1 theme, 2 breakpoints + potentially 3 themes, 1 breakpoint = 5 total (37.5% less snaps just with this change)
        • Would be an easy win and wouldn't lose too much coverage
  • Selectively run VRT only for PRs that need it
    • docs changes don't need VRT, for instance
    • Not sure if GH Actions has anything like this built in
    • Could this easily scale to filetypes or files within certain sections of the codebase? (config, .github, etc)
  • Only run Percy once a label has been applied to the PR to prevent multiple builds
    • Every push to a PR counts against our snapshots
    • This will prevent percy catching things early and often though
    • Most PRs percy doesn't catch anything though
    • Maybe it's ran once initially and then isn't ran again until we add 'ready to merge' for a final check
      • How do we know if a PR has already had Percy ran once against it? The reported result of the percy status check?
      • One big hole with this idea is it opens up the door to potentially forgetting to run percy as a final check and defects could slip through. If/when percy supports the github merge queue this would be less of an issue as the "final check" would be done in the merge queue.
    • ✅ Could/should we avoid running percy on draft PRs?

Tasks

Preview Give feedback
@tay1orjones tay1orjones changed the title Improve efficient use of Percy snapshots Be more strategic with our Percy snapshot quota Oct 4, 2023
@tay1orjones tay1orjones moved this to 🪆 Needs Refined in Design System Oct 4, 2023
@tay1orjones tay1orjones added this to the 2023 Q4 milestone Oct 4, 2023
@tay1orjones tay1orjones moved this from 🪆 Needs Refined to 🏗 In Progress in Design System Oct 11, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In Progress to ✅ Done in Design System Oct 16, 2023
@tay1orjones
Copy link
Member Author

Changing the browser width matrix for a subset of the themes worked

#14886 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant