From 131845f3689d11760301efc0839e57453d7ebbbb Mon Sep 17 00:00:00 2001 From: Julie Muzina Date: Mon, 17 Jun 2024 09:06:27 -0400 Subject: [PATCH 1/5] Link percy builds with a specific pull request; remove unnecessary artifact compression; Note manual status check application as to-be-removed; run percy only when certain files are changed --- .github/workflows/percy-prepare.yml | 48 ++++++++++++++++++++ .github/workflows/percy-snapshot/action.yml | 5 ++ .github/workflows/pr-percy-prepare-label.yml | 22 +++++++++ .github/workflows/pr-percy-prepare-push.yml | 26 +++++++++++ .github/workflows/pr-percy-prepare.yml | 42 ----------------- .github/workflows/pr-percy-snapshots.yml | 27 +++++++---- 6 files changed, 118 insertions(+), 52 deletions(-) create mode 100644 .github/workflows/percy-prepare.yml create mode 100644 .github/workflows/pr-percy-prepare-label.yml create mode 100644 .github/workflows/pr-percy-prepare-push.yml delete mode 100644 .github/workflows/pr-percy-prepare.yml diff --git a/.github/workflows/percy-prepare.yml b/.github/workflows/percy-prepare.yml new file mode 100644 index 000000000..e1218f8aa --- /dev/null +++ b/.github/workflows/percy-prepare.yml @@ -0,0 +1,48 @@ +name: "Prepare Percy build" + +on: + workflow_call: + inputs: + pr_number: + required: true + type: number + repository: + required: true + type: string + commitsh: + required: true + type: string + +jobs: + copy_artifact: + name: Prepare build + runs-on: ubuntu-latest + steps: + - name: Checkout repo + uses: actions/checkout@v4 + with: + sparse-checkout: | + templates/docs/examples/ + scss/ + ref: ${{ github.event.inputs.commitsh }} + repository: ${{ github.event.inputs.repository }} + + - name: Populate artifact directory + run: | + mkdir -p artifact + cp -R templates/docs/examples/ scss/ artifact/. + + # Archive the PR number associated with this workflow since it won't be available in the base workflow context + # https://github.com/orgs/community/discussions/25220 + - name: Archive PR data + if: github.event_name=='pull_request' + working-directory: artifact + run: | + echo ${{ inputs.pr_number }} > pr_num.txt + echo ${{ inputs.commitsh }} > pr_head_sha.txt + + - name: Upload artifact + uses: actions/upload-artifact@v4 + with: + name: "percy-testing-web-artifact" + path: artifact/* \ No newline at end of file diff --git a/.github/workflows/percy-snapshot/action.yml b/.github/workflows/percy-snapshot/action.yml index ea4ff4abf..61e813ceb 100644 --- a/.github/workflows/percy-snapshot/action.yml +++ b/.github/workflows/percy-snapshot/action.yml @@ -1,6 +1,9 @@ name: Percy Snapshot description: Starts Vanilla local server, captures example snapshots, & uploads them to Percy inputs: + pr_number: + required: false + description: Identifier of a pull request to associate with this Percy build. I.E. github.com/{repo}/pull/{pr_number} commitsh: required: true description: SHA signature of commit triggering the build @@ -81,6 +84,8 @@ runs: PERCY_BRANCH: ${{ inputs.branch_name }} PERCY_COMMIT: ${{ inputs.commitsh }} PERCY_CLIENT_ERROR_LOGS: false + # If PR number is set & a Percy-GHA integration is set up, this allows Percy to set status on the PR + PERCY_PULL_REQUEST: ${{ inputs.pr_number }} run: | set -e # Start a percy build and capture the stdout diff --git a/.github/workflows/pr-percy-prepare-label.yml b/.github/workflows/pr-percy-prepare-label.yml new file mode 100644 index 000000000..e7c41cb3c --- /dev/null +++ b/.github/workflows/pr-percy-prepare-label.yml @@ -0,0 +1,22 @@ +# This workflow ensures Percy is executed against PRs with the Percy required label, regardless of which paths were changed +name: "Percy [labeled]" + +on: + pull_request: + branches: + - main + types: + # workflow runs after a label is added to the PR, or when a commit is added to a PR with the Percy label + - labeled + - ready_for_review + - synchronize + +jobs: + copy_artifact: + name: Prepare build + if: contains(github.event.pull_request.labels.*.name, vars.PERCY_TEST_REQUESTED_LABEL_NAME) + uses: ./.github/workflows/percy-prepare.yml + with: + pr_number: ${{ github.event.number }} + repository: ${{ github.repository }} + commitsh: ${{ github.event.pull_request.head.sha }} \ No newline at end of file diff --git a/.github/workflows/pr-percy-prepare-push.yml b/.github/workflows/pr-percy-prepare-push.yml new file mode 100644 index 000000000..650897db1 --- /dev/null +++ b/.github/workflows/pr-percy-prepare-push.yml @@ -0,0 +1,26 @@ +# This workflow runs Percy against non-draft PRs that have changes in relevant examples filepaths. +name: "Percy [pushed]" + +on: + pull_request: + branches: + - main + paths: + - "templates/docs/examples/**" + - "scss/**" + types: + - opened + - ready_for_review + - synchronize + +jobs: + copy_artifact: + name: Prepare build + # Ignore draft PRS and PRs with the Percy label + # If we run tests against PRs with the Percy label, we will run tests twice (test is also ran by the labelling workflow) + if: ${{ !github.event.pull_request.draft && !contains(github.event.pull_request.labels.*.name, vars.PERCY_TEST_REQUESTED_LABEL_NAME) }} + uses: ./.github/workflows/percy-prepare.yml + with: + pr_number: ${{ github.event.number }} + repository: ${{ github.repository }} + commitsh: ${{ github.event.pull_request.head.sha }} \ No newline at end of file diff --git a/.github/workflows/pr-percy-prepare.yml b/.github/workflows/pr-percy-prepare.yml deleted file mode 100644 index f00cc0931..000000000 --- a/.github/workflows/pr-percy-prepare.yml +++ /dev/null @@ -1,42 +0,0 @@ -name: "Prepare Percy build" - -on: - pull_request: - branches: - - main - -jobs: - copy_artifact: - name: Copy changed files to GHA artifact - runs-on: ubuntu-latest - steps: - - name: Checkout repo - uses: actions/checkout@v4 - with: - sparse-checkout: | - templates/docs/examples/ - scss/ - - - name: Make artifact directory - run: mkdir -p artifact - - # Archive the PR number associated with this workflow since it won't be available in the base workflow context - # https://github.com/orgs/community/discussions/25220 - - name: Archive PR data - if: github.event_name=='pull_request' - working-directory: artifact - run: | - echo ${{ github.event.number }} > pr_num.txt - echo ${{ github.event.pull_request.head.sha }} > pr_head_sha.txt - - - name: Compress artifact - working-directory: artifact - run: | - cp -R ../templates/docs/examples/ ../scss/ . - tar -czf build.tar.gz * - - - name: Upload artifact - uses: actions/upload-artifact@v4 - with: - name: "web-artifact-${{ github.run_id }}" - path: artifact/build.tar.gz \ No newline at end of file diff --git a/.github/workflows/pr-percy-snapshots.yml b/.github/workflows/pr-percy-snapshots.yml index aef7c93b2..b9970a5fe 100644 --- a/.github/workflows/pr-percy-snapshots.yml +++ b/.github/workflows/pr-percy-snapshots.yml @@ -3,7 +3,8 @@ name: "Percy screenshots" on: workflow_run: workflows: - - "Prepare Percy build" + - "Percy [pushed]" + - "Percy [labeled]" types: - completed @@ -14,6 +15,7 @@ jobs: runs-on: ubuntu-latest outputs: pr_head_sha: ${{ steps.get_pr_data.outputs.sha }} + pr_number: ${{ steps.get_pr_data.outputs.pr_number }} percy_build_link: ${{ steps.percy_snapshot.outputs.build_link }} percy_org_id: ${{ steps.percy_snapshot.outputs.org_id }} percy_build_id: ${{ steps.percy_snapshot.outputs.build_id }} @@ -21,20 +23,23 @@ jobs: - name: Checkout SCM uses: actions/checkout@v4 + - name: Remove SCM directories that will be replaced by artifact files + run: | + set -e + rm -rf templates/docs/examples/ scss/ + - name: Download artifact from workflow run uses: actions/download-artifact@v4 with: - name: "web-artifact-${{ github.event.workflow_run.id }}" + name: "percy-testing-web-artifact" github-token: ${{ secrets.GITHUB_TOKEN }} repository: ${{ github.event.workflow_run.repository.full_name }} run-id: ${{ github.event.workflow_run.id }} - - name: Replace SCM files with artifact files + - name: Move artifact files into place to replace SCM run: | set -e - rm -rf templates/docs/examples/ scss/ - tar xzf build.tar.gz - rm build.tar.gz + # artifact directory contains `scss/`, `/examples`, `pr_num.txt`, and `pr_head_sha.txt`. `/examples` must be moved to `templates/docs/`. mv examples/ templates/docs/. - name: Get PR data @@ -43,17 +48,19 @@ jobs: run: | set -e echo "sha=$(cat pr_head_sha.txt)" >> $GITHUB_OUTPUT - echo "pr=$(cat pr_num.txt)" >> $GITHUB_OUTPUT + echo "pr_number=$(cat pr_num.txt)" >> $GITHUB_OUTPUT - name: Take snapshots & upload to Percy id: percy_snapshot - uses: './.github/workflows/percy-snapshot' + uses: "./.github/workflows/percy-snapshot" with: - branch_name: "pull/${{ steps.get_pr_data.outputs.pr }}" + branch_name: "pull/${{ steps.get_pr_data.outputs.pr_number }}" + pr_number: ${{ steps.get_pr_data.outputs.pr_number }} commitsh: ${{ steps.get_pr_data.outputs.sha }} percy_token_write: ${{ secrets.PERCY_TOKEN_WRITE }} # Add a check to the PR to show that screenshots were sent to Percy + # Manual status check to be removed once IS-GHA integration is complete # https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28#create-a-check-run apply_pr_check: name: Apply PR check @@ -65,7 +72,7 @@ jobs: id: create_check_run uses: octokit/request-action@v2.x with: - route: POST /repos/${{github.repository}}/check-runs + route: POST /repos/${{ github.repository }}/check-runs owner: octokit repo: request-action name: "percy_upload" From cecce53e2e1560876455f4b083c9fe03d0627c36 Mon Sep 17 00:00:00 2001 From: Julie Muzina Date: Tue, 11 Jun 2024 09:55:51 -0400 Subject: [PATCH 2/5] Link percy builds with a specific pull request; remove unnecessary artifact compression; Note manual status check application as to-be-removed; run percy only when certain files are changed --- .github/workflows/percy-prepare.yml | 2 +- .github/workflows/pr-percy-prepare-label.yml | 4 ++-- .github/workflows/pr-percy-prepare-push.yml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/percy-prepare.yml b/.github/workflows/percy-prepare.yml index e1218f8aa..4ebd67d37 100644 --- a/.github/workflows/percy-prepare.yml +++ b/.github/workflows/percy-prepare.yml @@ -15,7 +15,7 @@ on: jobs: copy_artifact: - name: Prepare build + name: Copy changed files to GHA artifact runs-on: ubuntu-latest steps: - name: Checkout repo diff --git a/.github/workflows/pr-percy-prepare-label.yml b/.github/workflows/pr-percy-prepare-label.yml index e7c41cb3c..9f11b48d7 100644 --- a/.github/workflows/pr-percy-prepare-label.yml +++ b/.github/workflows/pr-percy-prepare-label.yml @@ -1,5 +1,5 @@ # This workflow ensures Percy is executed against PRs with the Percy required label, regardless of which paths were changed -name: "Percy [labeled]" +name: "Percy [labeled] / Prepare build" on: pull_request: @@ -13,7 +13,7 @@ on: jobs: copy_artifact: - name: Prepare build + name: Copy changed files to GHA artifact if: contains(github.event.pull_request.labels.*.name, vars.PERCY_TEST_REQUESTED_LABEL_NAME) uses: ./.github/workflows/percy-prepare.yml with: diff --git a/.github/workflows/pr-percy-prepare-push.yml b/.github/workflows/pr-percy-prepare-push.yml index 650897db1..4bfef3b8b 100644 --- a/.github/workflows/pr-percy-prepare-push.yml +++ b/.github/workflows/pr-percy-prepare-push.yml @@ -1,5 +1,5 @@ # This workflow runs Percy against non-draft PRs that have changes in relevant examples filepaths. -name: "Percy [pushed]" +name: "Percy [pushed] / Prepare build" on: pull_request: @@ -15,7 +15,7 @@ on: jobs: copy_artifact: - name: Prepare build + name: Copy changed files to GHA artifact # Ignore draft PRS and PRs with the Percy label # If we run tests against PRs with the Percy label, we will run tests twice (test is also ran by the labelling workflow) if: ${{ !github.event.pull_request.draft && !contains(github.event.pull_request.labels.*.name, vars.PERCY_TEST_REQUESTED_LABEL_NAME) }} From c0f0727192f047b2b86fd9004cbd0414a5f4aa4c Mon Sep 17 00:00:00 2001 From: Julie Muzina Date: Mon, 17 Jun 2024 10:59:17 -0400 Subject: [PATCH 3/5] Rename percy workflows --- .github/workflows/pr-percy-prepare-label.yml | 2 +- .github/workflows/pr-percy-prepare-push.yml | 2 +- .github/workflows/pr-percy-snapshots.yml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-percy-prepare-label.yml b/.github/workflows/pr-percy-prepare-label.yml index 9f11b48d7..3a81b8a45 100644 --- a/.github/workflows/pr-percy-prepare-label.yml +++ b/.github/workflows/pr-percy-prepare-label.yml @@ -1,5 +1,5 @@ # This workflow ensures Percy is executed against PRs with the Percy required label, regardless of which paths were changed -name: "Percy [labeled] / Prepare build" +name: "Percy (labeled)" on: pull_request: diff --git a/.github/workflows/pr-percy-prepare-push.yml b/.github/workflows/pr-percy-prepare-push.yml index 4bfef3b8b..9c0ecdf60 100644 --- a/.github/workflows/pr-percy-prepare-push.yml +++ b/.github/workflows/pr-percy-prepare-push.yml @@ -1,5 +1,5 @@ # This workflow runs Percy against non-draft PRs that have changes in relevant examples filepaths. -name: "Percy [pushed] / Prepare build" +name: "Percy (pushed)" on: pull_request: diff --git a/.github/workflows/pr-percy-snapshots.yml b/.github/workflows/pr-percy-snapshots.yml index b9970a5fe..982aab7e3 100644 --- a/.github/workflows/pr-percy-snapshots.yml +++ b/.github/workflows/pr-percy-snapshots.yml @@ -3,8 +3,8 @@ name: "Percy screenshots" on: workflow_run: workflows: - - "Percy [pushed]" - - "Percy [labeled]" + - "Percy (pushed)" + - "Percy (labeled)" types: - completed From 14445ead3edb989a369f0052a0ca8709f5089354 Mon Sep 17 00:00:00 2001 From: Julie Muzina Date: Mon, 17 Jun 2024 11:00:32 -0400 Subject: [PATCH 4/5] move percy-snapshot action into its own dir --- .github/{workflows => actions}/percy-snapshot/action.yml | 0 .github/workflows/percy-baseline.yml | 2 +- .github/workflows/pr-percy-snapshots.yml | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename .github/{workflows => actions}/percy-snapshot/action.yml (100%) diff --git a/.github/workflows/percy-snapshot/action.yml b/.github/actions/percy-snapshot/action.yml similarity index 100% rename from .github/workflows/percy-snapshot/action.yml rename to .github/actions/percy-snapshot/action.yml diff --git a/.github/workflows/percy-baseline.yml b/.github/workflows/percy-baseline.yml index bc7c1a9ca..5ef6b4611 100644 --- a/.github/workflows/percy-baseline.yml +++ b/.github/workflows/percy-baseline.yml @@ -13,7 +13,7 @@ jobs: - name: Checkout SCM uses: actions/checkout@v4 - - uses: ./.github/workflows/percy-snapshot + - uses: ./.github/actions/percy-snapshot with: branch_name: main commitsh: ${{ github.sha }} diff --git a/.github/workflows/pr-percy-snapshots.yml b/.github/workflows/pr-percy-snapshots.yml index 982aab7e3..446993362 100644 --- a/.github/workflows/pr-percy-snapshots.yml +++ b/.github/workflows/pr-percy-snapshots.yml @@ -52,7 +52,7 @@ jobs: - name: Take snapshots & upload to Percy id: percy_snapshot - uses: "./.github/workflows/percy-snapshot" + uses: "./.github/actions/percy-snapshot" with: branch_name: "pull/${{ steps.get_pr_data.outputs.pr_number }}" pr_number: ${{ steps.get_pr_data.outputs.pr_number }} From 9e60c58fdd4a217421cbb8814eb1865055af1e9d Mon Sep 17 00:00:00 2001 From: Julie Muzina Date: Tue, 18 Jun 2024 10:18:34 -0400 Subject: [PATCH 5/5] Add more docs to Percy workflows; add Percy testing section to pull requests guide --- .github/workflows/percy-prepare.yml | 2 ++ .github/workflows/pr-percy-prepare-label.yml | 5 ++-- .github/workflows/pr-percy-prepare-push.yml | 5 ++-- .github/workflows/pr-percy-snapshots.yml | 1 + guides/pull-requests.md | 28 ++++++++++++++++++++ 5 files changed, 37 insertions(+), 4 deletions(-) diff --git a/.github/workflows/percy-prepare.yml b/.github/workflows/percy-prepare.yml index 4ebd67d37..7b72ffe37 100644 --- a/.github/workflows/percy-prepare.yml +++ b/.github/workflows/percy-prepare.yml @@ -1,3 +1,5 @@ +# This workflow is triggered by the `pr-percy-prepare` workflows. +# It checks out the SCSS/example markup changes and uploads them to Github as an artifact. name: "Prepare Percy build" on: diff --git a/.github/workflows/pr-percy-prepare-label.yml b/.github/workflows/pr-percy-prepare-label.yml index 3a81b8a45..7f6a87d5d 100644 --- a/.github/workflows/pr-percy-prepare-label.yml +++ b/.github/workflows/pr-percy-prepare-label.yml @@ -8,7 +8,6 @@ on: types: # workflow runs after a label is added to the PR, or when a commit is added to a PR with the Percy label - labeled - - ready_for_review - synchronize jobs: @@ -19,4 +18,6 @@ jobs: with: pr_number: ${{ github.event.number }} repository: ${{ github.repository }} - commitsh: ${{ github.event.pull_request.head.sha }} \ No newline at end of file + commitsh: ${{ github.event.pull_request.head.sha }} + +# Completion should trigger `pr-percy-snapshots` workflow. diff --git a/.github/workflows/pr-percy-prepare-push.yml b/.github/workflows/pr-percy-prepare-push.yml index 9c0ecdf60..0fc6f5f73 100644 --- a/.github/workflows/pr-percy-prepare-push.yml +++ b/.github/workflows/pr-percy-prepare-push.yml @@ -10,7 +10,6 @@ on: - "scss/**" types: - opened - - ready_for_review - synchronize jobs: @@ -23,4 +22,6 @@ jobs: with: pr_number: ${{ github.event.number }} repository: ${{ github.repository }} - commitsh: ${{ github.event.pull_request.head.sha }} \ No newline at end of file + commitsh: ${{ github.event.pull_request.head.sha }} + +# Completion should trigger `pr-percy-snapshots` workflow. diff --git a/.github/workflows/pr-percy-snapshots.yml b/.github/workflows/pr-percy-snapshots.yml index 446993362..6cadda20e 100644 --- a/.github/workflows/pr-percy-snapshots.yml +++ b/.github/workflows/pr-percy-snapshots.yml @@ -1,3 +1,4 @@ +# This workflow listens for completion of the `pr-percy-prepare` workflows, picks up their outputs, and performs Percy testing. name: "Percy screenshots" on: diff --git a/guides/pull-requests.md b/guides/pull-requests.md index 449438e02..a7552bbe5 100644 --- a/guides/pull-requests.md +++ b/guides/pull-requests.md @@ -78,6 +78,34 @@ apply the relevant `-1` Labels. ### Submitting & merging a PR +#### Percy visual testing + +We use [Percy](https://percy.io) for visual testing. Percy tests are run against pull requests to +ensure that PRs to not introduce visual regressions. Your PR will be tested by Percy if it meets the following conditions: + +- PR is against the `main` branch +- One of the following is true: + - PR passes Percy selectivity filters + - PR changes files in the `scss/` or `templates/docs/examples/` directories + - PR is not a draft + - PR is labeled with "Review: Percy needed" + +To ensure optimal Percy usage, we suggest the following PR flow: + +1. Open the PR (against `main`) in such a way that it causes an initial Percy test to run. + - If your PR makes changes to files in the `scss/` or `templates/docs/examples/` directories, it will be automatically + tested as long as it is not marked as a draft. + - Applying the "Review: Percy needed" label to the PR ensures that it is always tested. +2. Review the initial [Percy build](https://percy.io/bb49709b/vanilla-framework). +3. If there are additional changes needed to the PR through the review process, you can remove the "Review: Percy needed" + label and mark the PR as a draft to prevent additional Percy tests from running. +4. Once the PR is ready for final review, remove the draft status and reapply the "Review: Percy needed" label to trigger + a final Percy test. +5. If the Percy test passes, apply "Review: Percy +1" to indicate that the PR has passed Percy testing. +6. If all other reviews have been completed, the PR is ready to be merged. + +#### Merging a PR + After the necessary review steps have been completed and the PR is ready to be merged, the creator of the PR should merge it themself. The type of merge to use should be decided using the following logic: