Migrate Percy from CircleCI to Github Actions #5089

jmuzina · 2024-05-15T16:26:07Z

Done

Switches Percy integration from CircleCI to Github Actions

The problem

We are currently using CircleCI to execute Percy visual testing, because executing percy snapshot requires an environment variable PERCY_TOKEN to be set. However, repo secrets are not exposed to fork contributors for security reasons. CircleCI is used as a workaround for this, as it knows the Percy secret and is triggered on PR.

However, this disjoint approach causes issues with linking Percy build state to the PR. The PR CircleCI check success only means that the snapshots were uploaded to Percy; it does not necessarily mean that Percy diffs were approved.

The solution

This PR is the first step towards solving the above problem. This migrates us to using GHA to start Percy, allowing us to report Percy diff state in the PR at a later date and also run Percy more selectively (such as only when certain files change) at a later date.

The approach

Two workflows are used: pr-percy-copy-diff-files, and pr-percy-create-screenshots. This follows this approach by GH security team.

Copy diff files

Runs in an unprivileged context on pull_request to main. It checks out the templates/docs/examples/ and scss/ directories, notes the PR # & commit signature (as suggested by the GH security article above), and uploads it as an artifact to Github Actions associated with the workflow run.

Create screenshots

Runs in a privileged context on workflow_run trigger. Runs after completion of the copy diff files workflow. It downloads the artifacts from the previous workflow and replaces the matching files in main with the files from the artifacts. It then starts a local Flask server and uploads screenshots to Percy.

Note that for this workflow to run, pr-percy-create-screenshots.yml must be in the default branch of the target repo (canonical/vanilla-framework) at the time of the first workflow's completion. Thus, it will not run on this repo until this PR is merged and a second PR is raised to test its functionality.

QA

As mentioned above, screenshots will not be created by this PR as pr-percy-create-snapshots.yml is not yet in our main branch. However, the PR workflow will run and upload diff artifacts based on this PR commit. The following should occur after this PR is raised:

Copy diff files workflow is started & completed
After completion of the diff files workflow, an artifact is available for download in the workflow summary page with the name web-artifact-{workflow_run_id}. Opening this artifact will reveal a build.tar.gz file containing:
- examples/ (the templates/docs/examples/ folder from the PR code)
- scss/ (the scss/ folder from the PR code)
- pr_head_sha.txt (contains the signature of the last commit in the PR code
- pr_num.txt (contains the number of this PR, i.e., https://github.com/canonical/vanila-framework/pull/{pr_number})

To fully test this workflow in a fork PR context, a separate repository was created. An example workflow is shown:

PR created
Copy diff files workflow runs & creates artifact
Screenshot creation workflow runs
Status check percy_upload is applied to the PR, including a link to the Percy build

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

PR should have one of the following labels to automatically categorise it in release notes:
- Feature 🎁, Breaking Change 💣, Bug 🐛, Documentation 📝, Maintenance 🔨.
Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
- if CSS class names are not changed it can be bugfix relesase (x.x.X)
- if CSS class names are changed/added/removed it should be minor version (x.X.0)
- see the wiki for more details
Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.

Screenshots

Upload & Snapshot status checks on PR:

Percy diff:

Upload completed status check on PR with Percy build details:

webteam-app · 2024-05-15T16:26:14Z

Demo

Jenkins

demos.haus

bartaz

I left couple comments/questions, but overall impressive work!

bartaz · 2024-05-16T07:22:43Z

.github/workflows/pr-percy-copy-diff-files.yml

@@ -0,0 +1,42 @@
+name: "Upload artifacts for Percy screenshots"


Maybe just for simplicity (and potential future changes of what is done internally) we could rename this step to "Prepare Percy build" pr-percy-prepage.yml, and the other one "Percy snapshots" pr-percy-snapshots.yml.

Makes sense, renaming now

bartaz · 2024-05-16T07:27:59Z

.github/workflows/pr-percy-create-screenshots.yml

+ steps:
+ - name: Verify that triggering workflow run passed
+ run: |
+ if [[ "${{ github.event.workflow_run.conclusion }}" != "success" ]]; then


Is there any reason for this conditional check to be in steps, rather than on the job level?

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-based-on-the-conclusion-of-another-workflow

jobs: on-success: runs-on: ubuntu-latest if: ${{ github.event.workflow_run.conclusion == 'success' }} steps: - run: echo 'The triggering workflow passed' on-failure: runs-on: ubuntu-latest if: ${{ github.event.workflow_run.conclusion == 'failure' }} steps: - run: echo 'The triggering workflow failed'

Good call, moving that to a job level check

bartaz · 2024-05-16T07:30:05Z

.github/workflows/pr-percy-create-screenshots.yml

+
+ - name: Replace SCM files with artifact files
+ run: |
+ set -e


Nice touch, didn't know this before.

bartaz · 2024-05-17T11:55:48Z

.github/workflows/pr-percy-create-screenshots.yml

+ # This identifies the diff in the percy diffs list
+ PERCY_BRANCH: "pull/${{ steps.get_pr_data.outputs.pr }}"
+ #PERCY_BRANCH: "${{ github.event.workflow_run.head_repository.full_name }}/${{ github.event.workflow_run.head_branch }}"
+ #PERCY_PULL_REQUEST: ${{ steps.get_pr_data.outputs.pr }}


Are these intentionally commented out?

@bartaz Yes, I ran into a bug while I was working on #5092 where I noticed that all Percy builds were becoming the baseline because they defaulted to being "main" branch without me specifying PERCY_BRANCH.

This sets the diff branch name to a PR identifier, making sure that builds don't become the baseline unless they are approved.

The commented out code was doing the following:

PERCY_BRANCH - I was trying to set the branch name to <fork_repo_username>/<fork_repo_name>/<fork_pr_head_branch_name> in an attempt to make the diff branches unique. However I simplified this to the pull/ format as that is what is currently used in the project.

PERCY_PULL_REQUEST - links the diff to a specific PR by number. I think this requires the Percy->GH integration to be enabled, but I don't have permissions to enable it in the Percy portal.

See more about Percy CLI environment vars here.

I'll remove these in a commit shortly.

bartaz · 2024-05-17T12:00:23Z

.github/workflows/pr-percy-create-screenshots.yml

+ run: |
+ set -e
+ percy_output=$(yarn percy)
+ build_link=$(echo "$percy_output" | sed -n 's/.*Finalized build #.*: \(https:\/\/percy.io\/[^ ]*\).*/\1/p')


There is a lot of happening here in a bit cryptic way. Could be worth to add some comments to explain what data of Percy build is being extracted and why.

Added some new comments for this section

* Revert "Revert "break percy diff intentionally"" This reverts commit 20db9ce. * Revert "Revert "Revert "break percy diff intentionally""" This reverts commit 4e3ef26. * test for revisions to canonical/vanilla-framework#5089 * version bump for test pr * bump for test action * dedupe * test pr test pr * intentional fail * Revert "intentional fail" This reverts commit 20a3fb6.

bartaz · 2024-05-21T11:33:48Z

.github/workflows/pr-percy-snapshots.yml

+ # https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28#create-a-check-run
+ apply_pr_check:
+ name: Apply PR check
+ needs: upload


This will only get triggered once the upload is completed. Could we have some kind of "in progress" status as well to see that "Percy build" is running (and not only when it succeeds)?

bartaz

Great work, thanks. Let's try it out.

jmuzina added Review: Code needed Maintenance 🔨 Review: Percy needed This PR needs a review of Percy for visual regressions labels May 15, 2024

jmuzina self-assigned this May 15, 2024

jmuzina force-pushed the percy-gha-migration branch from a92ebf1 to 26ffa2c Compare May 15, 2024 17:05

Migrate Percy to Github Actions

9e31ec9

jmuzina force-pushed the percy-gha-migration branch from 26ffa2c to 9e31ec9 Compare May 15, 2024 17:48

jmuzina added 2 commits May 16, 2024 09:09

Enable Percy snapshots & embed percy buld in status check

4ad9e36

remove circleci from readme

15d477b

jmuzina marked this pull request as ready for review May 16, 2024 13:28

jmuzina removed the Review: Percy needed This PR needs a review of Percy for visual regressions label May 16, 2024

jmuzina requested a review from bartaz May 16, 2024 13:30

fix all percy builds becoming the new baseline even if they failed

d21ad12

jmuzina mentioned this pull request May 16, 2024

Percy GHA Migration - PR Status check reflects Percy diff state #5092

Closed

3 tasks

bartaz reviewed May 17, 2024

View reviewed changes

jmuzina added 4 commits May 17, 2024 09:09

remove commented Percy env vars

80ca065

Workflow verification step removed in favor of job check

17877f5

rename percy workflow jobs

8cfe675

percy snapshots extraction comments

a014316

jmuzina added a commit to jmuzina/test-web-percy-gha-migration that referenced this pull request May 17, 2024

test for revisions to canonical/vanilla-framework#5089

ce007cf

jmuzina added a commit to jmuzina/test-web-percy-gha-migration that referenced this pull request May 17, 2024

test for revisions to canonical/vanilla-framework#5089

e1a06f0

add momentary sleep to uptime wait per canonical#5092

ee88a68

jmuzina added a commit to jmuzina/vanilla-framework that referenced this pull request May 17, 2024

update per feedback on canonical#5089

fd458e3

bartaz reviewed May 21, 2024

View reviewed changes

bartaz added Review: Code +1 and removed Review: Code needed labels May 21, 2024

bartaz approved these changes May 21, 2024

View reviewed changes

bartaz merged commit e4e8a21 into canonical:main May 21, 2024
5 of 6 checks passed

jmuzina mentioned this pull request May 21, 2024

Percy GHA Migration - Baseline updates on push to main #5095

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Percy from CircleCI to Github Actions #5089

Migrate Percy from CircleCI to Github Actions #5089

jmuzina commented May 15, 2024 •

edited

webteam-app commented May 15, 2024

bartaz left a comment

bartaz May 16, 2024

jmuzina May 17, 2024

bartaz May 16, 2024

jmuzina May 17, 2024

bartaz May 16, 2024

bartaz May 17, 2024

jmuzina May 17, 2024

bartaz May 17, 2024

jmuzina May 17, 2024

bartaz May 21, 2024

bartaz left a comment

		@@ -0,0 +1,42 @@
		name: "Upload artifacts for Percy screenshots"

Migrate Percy from CircleCI to Github Actions #5089

Migrate Percy from CircleCI to Github Actions #5089

Conversation

jmuzina commented May 15, 2024 • edited

Done

The problem

The solution

The approach

Copy diff files

Create screenshots

QA

Check if PR is ready for release

Screenshots

webteam-app commented May 15, 2024

bartaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartaz left a comment

Choose a reason for hiding this comment

jmuzina commented May 15, 2024 •

edited