Skip to content

chore(percy): added automated targeted Percy build on PRs and releases #152

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.

Sign up for GitHub

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

Closed
ArtBlue wants to merge 8 commits into from
Closed

chore(percy): added automated targeted Percy build on PRs and releases #152

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
05791d7
chore(percy): added automated targeted Percy build on PRs and releases
ArtBlue May 29, 2025
a427f77
fix(percy): addressed PR feedback
ArtBlue May 30, 2025
4132876
fix(percy): added security measures per PR feedback
ArtBlue May 30, 2025
ca262d1
fix(percy): fixed errors
ArtBlue May 30, 2025
ba01d7a
fix(Percy): solved error issue
ArtBlue Jun 3, 2025
93a6d54
fix(Percy): installed missing husky to fix failure
ArtBlue Jun 3, 2025
f53ac16
fix(Percy): installing husky to fix the workflow automation that need…
ArtBlue Jun 3, 2025
f6a76a4
fix(Percy): removed node version from workflow
ArtBlue Jun 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions .github/workflows/baseline-visual-regression.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# This workflow runs Percy visual regression tests after a PR tagged with "package: skin"
# is merged into the main branch. It extracts the list of stories from the PR body
# and runs snapshots for those stories to update the visual regression baselines.

name: Percy Merged Visual Regression Baselines

on:
push:
branches:
- main

jobs:
post-merge-snapshots:
runs-on: ubuntu-latest

steps:
# Checkout the repository
- name: Checkout code
uses: actions/checkout@v3

# Set up Node.js
- name: Set up Node.js
uses: actions/setup-node@v3
with:
node-version: 22

# Install dependencies
- name: Install dependencies
run: npm install
working-directory: ./packages/skin

# Extract target stories from PR body
- name: Extract target stories
id: extract_stories
env:
PR_BODY: ${{ github.event.pull_request.body }}
run: |
# Use the PR_BODY environment variable securely
if [[ "$PR_BODY" == *"package: skin"* ]]; then
STORIES=$(echo "$PR_BODY" | awk '/Percy Stories/{getline; print}')
if [ -z "$STORIES" ]; then
echo "No Percy Stories found in PR body."
exit 0
fi
echo "stories=$STORIES" >> $GITHUB_ENV
else
echo "No relevant PR found for package: skin."
exit 0
fi
working-directory: ./packages/skin
Comment on lines +32 to +50
Copy link
Collaborator

@agliga agliga Jun 2, 2025

Choose a reason for hiding this comment

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

Is this section needed for the baesline? I assume when a PR merges it just gets a new baseline story right?

Copy link
Contributor Author

@ArtBlue ArtBlue Jun 3, 2025

Choose a reason for hiding this comment

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

Is this section needed for the baesline? I assume when a PR merges it just gets a new baseline story right?

I think there may be some confusion around how Percy works and the current process.

Doing baselines is currently a manual process. I've been running full baseline builds from master after releases to create new baselines. If Percy had the ability to automatically create new baselines by itself when we did releases, we would have definitely been doing that all of this time or at least thought about it. AFAIK, Percy has no awareness of repositories. It doesn't check for merges itself and run builds; don't think it's capable of doing that.

With our recent trunk based development, without automating this targeted build on releases, we would have to do that manually and since our releases are going to be smaller and more frequent, full builds on each release are going to be too much. Bottom line is we'd have to do this same thing manually - targeted Percy runs on each release.

This change basically automates that process. This runs targeted baseline builds to create baselines only on the components touched for the PR, which are only the ones that were touched by the PR. This streamlines visual regression with very little effort.

Copy link
Collaborator

@agliga agliga Jun 3, 2025

Choose a reason for hiding this comment

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

So here's my issue with the way this is
If I create a PR, merge it, and it runs, then it updates baselines for that PR. If that PR is missing those lines OR they are incorrect, it will only update a certain set of baselines.
The other issue is if we merge two PRs at the same time, only the latest one will run and update only those.
IMO, I think we should maybe run this once a day and update all snapshots. If that seems to be too much we can either run it once a week, or have a way to trigger it github manually when we want to update.

Copy link
Contributor Author

@ArtBlue ArtBlue Jun 3, 2025

Choose a reason for hiding this comment

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

If that PR is missing those lines OR they are incorrect, it will only update a certain set of baselines.

If this happens, then something terribly wrong has happened with the PR itself, because the whole point of the visual regression is to ensure we're covering all the changes to the appropriate stories. If those are not mentioned in the PR, that's already a failure at the first level. This simply creates baselines for the very first stories that were included, which is what should happen. That's typically all we care about. IMO, it's not a big deal to ensure the PR has the correct stories mentioned in the body.

The other issue is if we merge two PRs at the same time, only the latest one will run and update only those.

When would we merge two PRs at virtually the same time? If we have to concern ourselves with this possibility, then no automation will ever work.

IMO, I think we should maybe run this once a day and update all snapshots.

I don't this is a realistic or practical solution. The whole point is to automate visual regression as a whole. It's not practical to have someone run automation every single day. Also, if we did that, we would deplete the quota we have extremely quickly.

If that seems to be too much we can either run it once a week, or have a way to trigger it github manually when we want to update.

Once a week is better and may not deplete our quota completely. However, if there is more than one change at all touching components that are common in changesets during a single week, the diffs are going to be compared against obsolete baselines. This won't work IMO.


# Debug: Print extracted stories
- name: Debug extracted stories
run: |
STORIES=$(echo "$PR_BODY" | awk '/Percy Stories/{getline; print}')
Copy link
Collaborator

@agliga agliga Jun 2, 2025

Choose a reason for hiding this comment

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

Where is /Percy Stories/? Is that a url or directory?
Im mainly looking to see how this parses info so that if the PR_BODY has garbage it doesn't get hacked.

Copy link
Contributor Author

@ArtBlue ArtBlue Jun 3, 2025

Choose a reason for hiding this comment

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

/Percy Stories/ is just the regex for the string it's looking for in the PR body. If garbage is included, I believe Percy run should just fail.

echo "Extracted stories: $STORIES"
working-directory: ./packages/skin

# Run Percy for the extracted stories
- name: Run Percy visual tests
env:
PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }}
run: |
if [ -z "$stories" ]; then
echo "No target stories found. Skipping Percy tests."
exit 0
fi
echo "Running Percy for stories: $stories"
npm run snapshots "$stories"
working-directory: ./packages/skin
60 changes: 60 additions & 0 deletions .github/workflows/pr-visual-regression.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# This workflow runs Percy visual regression tests for pull requests. It extracts the list of stories
# from the PR body and runs snapshots for those stories to validate visual changes before merging.
# If Percy detects visual differences, the workflow fails to block merging.

name: Percy PR Visual Regression Tests

on:
pull_request:
types:
- opened
- synchronize
- reopened
- edited
- labeled

jobs:
visual-regression:
runs-on: ubuntu-latest

steps:
# Checkout the repository
- name: Checkout code
uses: actions/checkout@v3

# Set up Node.js
- name: Set up Node.js
uses: actions/setup-node@v3
with:
node-version: 22

# Install dependencies
- name: Install dependencies
run: npm install
working-directory: ./packages/skin
Copy link
Collaborator

@agliga agliga Jun 2, 2025

Choose a reason for hiding this comment

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

I believe all the dev depdencies are in the root. Might want to move that to the root.

Copy link
Contributor Author

@ArtBlue ArtBlue Jun 3, 2025

Choose a reason for hiding this comment

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

I thought about that before going in this direction. What benefits would that have vs. this option? I'm fine doing that, but it's a fairly large change... trying to make sure the benefits outweigh the cost.

Copy link
Collaborator

@agliga agliga Jun 3, 2025

Choose a reason for hiding this comment

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

Im just worried that some dev deps might not be added

Copy link
Collaborator

@agliga agliga Jun 3, 2025

Choose a reason for hiding this comment

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

Yeah, I think this CI issue is due to no dev deps being installed. Should probably remove working-directory

Error: Cannot find module 'glob'
Require stack:
- /home/runner/work/evo-web/evo-web/packages/skin/scripts/generate-bundle.js
- /home/runner/work/evo-web/evo-web/packages/skin/scripts/index.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1212:15)
    at Module._load (node:internal/modules/cjs/loader:1043:27)
    at Module.require (node:internal/modules/cjs/loader:1298:19)
    at require (node:internal/modules/helpers:182:18)
    at Object.<anonymous> (/home/runner/work/evo-web/evo-web/packages/skin/scripts/generate-bundle.js:3:14)
    at Module._compile (node:internal/modules/cjs/loader:1529:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1613:10)
    at Module.load (node:internal/modules/cjs/loader:1275:[32](https://github.com/eBay/evo-web/actions/runs/15423370047/job/43404096987#step:7:33))
    at Module._load (node:internal/modules/cjs/loader:1096:12)
    at Module.require (node:internal/modules/cjs/loader:1298:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/runner/work/evo-web/evo-web/packages/skin/scripts/generate-bundle.js',
    '/home/runner/work/evo-web/evo-web/packages/skin/scripts/index.js'
  ]
}

Copy link
Contributor Author

@ArtBlue ArtBlue Jun 3, 2025

Choose a reason for hiding this comment

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

Isn't it just missing glob? Aren't all the other dependencies in /package/skin? Are you saying move everything under skin to the root as a dependency?


# Extract target stories from PR body
- name: Extract target stories
id: extract_stories
env:
PR_BODY: ${{ github.event.pull_request.body }}
run: |
STORIES=$(echo "$PR_BODY" | awk '/Percy Stories/{getline; print}')
if [ -z "$STORIES" ]; then
echo "No Percy Stories found in PR body."
exit 0
fi
echo "stories=$STORIES" >> $GITHUB_ENV
working-directory: ./packages/skin

# Debug: Print extracted stories
- name: Debug extracted stories
run: echo "Extracted stories: $stories"
working-directory: ./packages/skin

# Run Percy for the extracted stories
- name: Run Percy visual tests
env:
PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }}
run: npm run snapshots --stories "$stories"
working-directory: ./packages/skin