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.

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

Skip “Percy screenshots” when secrets are unavailable on forks #3028

Merged
merged 2 commits into from
Nov 21, 2022
Merged

Skip “Percy screenshots” when secrets are unavailable on forks #3028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49aff04
Move “Percy Screenshots” to separate GitHub workflow
colinrotherham Nov 21, 2022
7a81c8c
Skip “Percy screenshots” when secrets are unavailable on forks
colinrotherham Nov 21, 2022
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
34 changes: 34 additions & 0 deletions .github/workflows/screenshots.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
name: Percy screenshots

on:
workflow_call:
workflow_dispatch:

concurrency:
group: screenshots-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

jobs:
screenshots:
name: Send screenshots
runs-on: ubuntu-latest

env:
PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }}
PORT: 8888

steps:
- name: Checkout
uses: actions/checkout@v3

- name: Install dependencies
uses: ./.github/workflows/actions/install-node

- name: Build
uses: ./.github/workflows/actions/build

- name: Start review application
run: npm run serve &

- name: Send screenshots to Percy
run: npx percy exec -- npm run test:screenshots
27 changes: 7 additions & 20 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,26 +162,13 @@ jobs:
run: ${{ matrix.run }}

regression:
name: Send screenshots to Percy
runs-on: ubuntu-latest
name: Percy
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any benefit to having this inside the tests workflow, rather than just having screenshots.yml trigger on the same pull_request and push events?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it gives the Install and Build steps chance to get cached

Then when we trigger the Percy workflow it can go super fast (already installed and built)

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense – but I think this might be worth adding as a comment somewhere as it's not obvious.

Copy link
Contributor Author

@colinrotherham colinrotherham Nov 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Sounds good, comment added

Similar to how we only install js-beautify in the Diff changes to dist workflow it would be nice in future (we're nearly there) to just install Percy with the Review app devDependencies:

npm ci --workspace app

needs: [install, build]

env:
PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }}
PORT: 8888

steps:
- name: Checkout
uses: actions/checkout@v3

- name: Restore dependencies
uses: ./.github/workflows/actions/install-node

- name: Restore build
uses: ./.github/workflows/actions/build

- name: Start review application
run: npm run serve &
# Run existing "Percy screenshots" workflow
# (after install and build have been cached)
uses: ./.github/workflows/screenshots.yml
secrets: inherit

- name: Send screenshots to Percy
run: npx percy exec -- npm run test:screenshots
# Skip when secrets are unavailable on forks
if: ${{ github.repository_owner == 'alphagov' }}