Skip to content

Commit

Permalink
ci: Fix workflow to update screenshots (#4993)
Browse files Browse the repository at this point in the history
The initial commit was untested, but having the workflow in the main
branch allowed me to iterate on the workflow and test it in a branch.

The following changes were made:
- Standardize how workflow inputs are read (using inputs, which works
for all triggers, instead of github.event.inputs, which only works for
some triggers)
 - Improve the description of test_filter in selenium-lab-tests.
- Add missing fields in the workflow_call trigger of
selenium-lab-tests.yaml. (Unfortunately, the two triggers need to have
the inputs defined separately, and with slightly different fields.)
 - Log the chosen ref in selenium-lab-tests, to help debug inputs.
- Fix variable expansion in selenium-lab-tests so that test_filter can
contain spaces.
- Fix ignore_test_status, which did not work at all and broken the
nightly tests last night.
 - Fix dependencies between jobs in update-screenshots.
- Do not set status for the subordinate selenium-lab-tests job if
started from update-screenshots.
- Always upload screenshots from selenium-lab-tests if started from
update-screenshots (normally only uploaded on failure).
 - Set the PR status from upload-screenshots.
 - Fix unpacking of screenshots from artifacts in upload-screenshots.
 - Fix updating of changed screenshots.
 - Fix handling of a lack of screenshot changes.
 - Split up the jobs for easier debugging in upload-screenshots.

Still to do:
 - Fix setting of final PR status
- Use a personal access token so that tests can be triggered from the PR
push
  • Loading branch information
joeyparrish authored Feb 23, 2023
1 parent 541badc commit 25fc81e
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 43 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/build-and-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ on:
# old one. If a PR is updated and a new test run is started, the old test run
# will be cancelled automatically to conserve resources.
concurrency:
group: ${{ github.workflow }}-${{ github.event.inputs.ref || github.ref }}
group: ${{ github.workflow }}-${{ inputs.ref || github.ref }}
cancel-in-progress: true

jobs:
Expand All @@ -27,7 +27,7 @@ jobs:
- name: Checkout code
uses: actions/checkout@v3
with:
ref: ${{ github.event.inputs.ref || github.ref }}
ref: ${{ inputs.ref || github.ref }}

- name: Lint
run: python build/check.py
Expand Down Expand Up @@ -103,7 +103,7 @@ jobs:
- name: Checkout code
uses: actions/checkout@v3
with:
ref: ${{ github.event.inputs.ref || github.ref }}
ref: ${{ inputs.ref || github.ref }}

# Older versions of Safari can be installed, but not to the root, and it
# can't replace the standard version, at least not on GitHub's VMs. If
Expand Down Expand Up @@ -213,7 +213,7 @@ jobs:
- name: Checkout code
uses: actions/checkout@v3
with:
ref: ${{ github.event.inputs.ref || github.ref }}
ref: ${{ inputs.ref || github.ref }}

- name: Docker
run: |
Expand Down
68 changes: 45 additions & 23 deletions .github/workflows/selenium-lab-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
description: "A PR number to build and test in the lab. If empty, will build and test from main."
required: false
test_filter:
description: "A filter to run a subset of the tests. If empty, all tests will run."
description: "A regex filter to run a subset of the tests. If empty, all tests will run."
required: false
workflow_call:
# Allows for reuse from other workflows, such as "Update All Screenshots"
Expand All @@ -18,11 +18,15 @@ on:
pr:
description: "A PR number to build and test in the lab. If empty, will build and test from main."
required: false
type: string
test_filter:
description: "A filter to run a subset of the tests. If empty, all tests will run."
description: "A regex filter to run a subset of the tests. If empty, all tests will run."
required: false
type: string
ignore_test_status:
description: "If true, ignore test failures and treat the workflow as a success."
description: "If true, ignore test success or failure, never set the commit status, and always upload screenshots."
required: false
type: boolean
schedule:
# Runs every night at 2am PST / 10am UTC, testing against the main branch.
- cron: '0 10 * * *'
Expand All @@ -42,12 +46,12 @@ jobs:
- name: Compute ref
id: compute
run: |
if [[ "${{ github.event.inputs.pr }}" != "" ]]; then
LAB_TEST_REF="refs/pull/${{ github.event.inputs.pr }}/head"
if [[ "${{ inputs.pr }}" != "" ]]; then
LAB_TEST_REF="refs/pull/${{ inputs.pr }}/head"
else
LAB_TEST_REF="main"
fi
echo "REF=$LAB_TEST_REF" >> $GITHUB_OUTPUT
echo "REF=$LAB_TEST_REF" | tee -a $GITHUB_OUTPUT
# Configure the build matrix based on our grid's YAML config.
# The matrix contents will be computed by this first job and deserialized
Expand Down Expand Up @@ -112,6 +116,7 @@ jobs:
ref: ${{ needs.compute-ref.outputs.REF }}

- name: Set commit status to pending
if: ${{ inputs.ignore_test_status == false }}
uses: ./.github/workflows/custom-actions/set-commit-status
with:
context: Selenium / Build
Expand All @@ -129,8 +134,9 @@ jobs:
retention-days: 1

- name: Report final commit status
# Will run on success or failure, but not if the workflow is cancelled.
if: ${{ success() || failure() }}
# Will run on success or failure, but not if the workflow is cancelled
# or if we were asked to ignore the test status.
if: ${{ (success() || failure()) && inputs.ignore_test_status == false }}
uses: ./.github/workflows/custom-actions/set-commit-status
with:
context: Selenium / Build
Expand All @@ -154,6 +160,7 @@ jobs:
ref: ${{ needs.compute-ref.outputs.REF }}

- name: Set commit status to pending
if: ${{ inputs.ignore_test_status == false }}
uses: ./.github/workflows/custom-actions/set-commit-status
with:
context: Selenium / ${{ matrix.browser }}
Expand Down Expand Up @@ -197,25 +204,28 @@ jobs:
# the container.
- name: Test Player
run: |
# Use of an array keeps elements intact, and allows an element to
# contain spaces without being expanded into multiple arguments in a
# shell command.
extra_flags=()
# Generate a coverage report from uncompiled code on ChromeLinux.
# It should be the uncompiled build, or else we won't execute any
# coverage instrumentation on full-stack player integration tests.
if [[ "${{ matrix.browser }}" == "ChromeLinux" ]]; then
extra_flags="--html-coverage-report --uncompiled"
else
extra_flags=""
extra_flags+=(--html-coverage-report --uncompiled)
fi
filter="${{ github.event.inputs.test_filter }}"
if [[ "$filter" != "" ]]; then
extra_flags="$extra_flags --filter $filter"
fi
ignore_test_status="${{ github.event.inputs.ignore_test_status }}"
if [[ "$ignore_test_status" == "true" ]]; then
extra_flags="$extra_flags || true"
if [[ "${{ inputs.test_filter }}" != "" ]]; then
echo "Adding filter: ${{ inputs.test_filter }}"
extra_flags+=(--filter "${{ inputs.test_filter }}")
fi
# Do not automatically fail when a command fails. This allows us to
# implement the ignore_test_status input by capturing the exit code
# and examining it.
set +e
# Run the tests with any extra flags.
python3 build/test.py \
--no-build \
--reporters spec --spec-hide-passed \
Expand All @@ -225,7 +235,18 @@ jobs:
--grid-config build/shaka-lab.yaml \
--grid-address selenium-grid.lab:4444 \
--browsers ${{ matrix.browser }} \
$extra_flags
"${extra_flags[@]}"
# Capture the test exit code immediately after running the tests.
# There cannot be any other command between test.py and here.
exit_code=$?
# If ignoring test status, treat this as an exit code of 0 (success).
if [[ "${{ inputs.ignore_test_status }}" == "true" ]]; then
exit_code=0
fi
# Report the captured (and possibly overridden) exit status.
exit $exit_code
- name: Find coverage report (ChromeLinux only)
id: coverage
Expand Down Expand Up @@ -269,7 +290,7 @@ jobs:
# Upload new screenshots and diffs on failure; ignore if missing
- name: Upload screenshots
uses: actions/upload-artifact@v3
if: ${{ failure() }}
if: ${{ failure() || inputs.ignore_test_status }}
with:
# In this workflow, "browser" is the selenium node name, which can
# contain both browser and OS, such as "ChromeLinux".
Expand All @@ -281,8 +302,9 @@ jobs:
retention-days: 5

- name: Report final commit status
# Will run on success or failure, but not if the workflow is cancelled.
if: ${{ success() || failure() }}
# Will run on success or failure, but not if the workflow is cancelled
# or if we were asked to ignore the test status.
if: ${{ (success() || failure()) && inputs.ignore_test_status == false }}
uses: ./.github/workflows/custom-actions/set-commit-status
with:
context: Selenium / ${{ matrix.browser }}
Expand Down
111 changes: 95 additions & 16 deletions .github/workflows/update-screenshots.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,52 +12,131 @@ on:
required: true

jobs:
set-pending-status:
name: Set Pending Status
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
ref: refs/pull/${{ inputs.pr }}/head

- name: Set commit status to pending
uses: ./.github/workflows/custom-actions/set-commit-status
with:
context: Update All Screenshots
state: pending
token: ${{ secrets.GITHUB_TOKEN }}

run-lab-tests:
name: Get Selenium Lab Screenshots
needs: [set-pending-status]
uses: ./.github/workflows/selenium-lab-tests.yaml
with:
pr: ${{ github.event.inputs.pr }}
test_filter: 'layout'
pr: ${{ inputs.pr }}
test_filter: layout
ignore_test_status: true

update-pr:
name: Update PR
runs-on: ubuntu-latest
needs: [run-lab-tests]
steps:
- uses: actions/checkout@v3
with:
ref: refs/pull/${{ github.event.inputs.pr }}/head
ref: refs/pull/${{ inputs.pr }}/head

- name: Set commit status to pending
uses: ./.github/workflows/custom-actions/set-commit-status
with:
context: Update All Screenshots
state: pending
token: ${{ secrets.GITHUB_TOKEN }}

- name: Get artifacts
uses: actions/download-artifact@v3
with:
path: new-screenshots

- name: Update screenshots
- name: Move screenshots
run: |
# Unpack screenshots from the lab.
for i in screenshots-*.zip; do
unzip -d test/test/assets/screenshots/ "$i"
# The download-artifact action puts zip file contents into folders by
# the name of the artifact, so we can't have them downloaded directly
# to the screenshots folder. Here we move them into the correct
# locations.
cd new-screenshots
# "$i" is a folder for an artifact (eg "screenshots-ChromeAndroid"),
# and it contains a single folder with the name we use in our
# screenshots (eg "chrome-Android"). We want the contents of that
# inner folder to be copied to test/test/assets/screenshots/, where a
# folder with the same name (eg "chrome-Android") already exists.
for i in screenshots*; do
cp -a $i/* ../test/test/assets/screenshots/
done
# Update the official screenshots for any that has visibly changed.
# This is not a byte-for-byte comparison.
- name: Update screenshots
run: |
# Install prerequisites.
npm ci
# Update the official screenshots for any that have visibly changed.
# This is not a byte-for-byte comparison, but based on pixel diffs.
./build/updateScreenshots.py
# Emulate the actions bot.
git config user.name "github-actions[bot]"
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
# Update the PR.
git add test/test/assets/screenshots/*/*.png
git commit -m ':robot: Update all screenshots'
# Commit the changes. Ignore failure, in case there are no changes.
git add test/test/assets/screenshots/*/*.png || true
git commit -m ':robot: Update all screenshots' || true
PR_API_URL="/repos/${{ github.repository }}/pulls/${{ github.event.inputs.pr }}"
REMOTE=$(gh api $GH_API_URL | jq -r .head.repo.html_url)
BRANCH=$(gh api $GH_API_URL | jq -r .head.ref)
- name: Update PR
env:
GH_TOKEN: ${{ github.token }}
run: |
# Update the PR.
PR_API_URL="/repos/${{ github.repository }}/pulls/${{ inputs.pr }}"
REMOTE=$(gh api $PR_API_URL | jq -r .head.repo.html_url)
BRANCH=$(gh api $PR_API_URL | jq -r .head.ref)
git push "$REMOTE" "$BRANCH"
# If there were no changes, this will do nothing, but succeed.
git push "$REMOTE" HEAD:"$BRANCH"
- name: Debug
uses: mxschmitt/[email protected]
with:
limit-access-to-actor: true
if: failure()

set-final-status:
name: Set Final Status
runs-on: ubuntu-latest
needs: [run-lab-tests, update-pr]
# Will run on success or failure, but not if the workflow is cancelled.
if: ${{ success() || failure() }}
steps:
- uses: actions/checkout@v3
with:
ref: refs/pull/${{ inputs.pr }}/head

- name: Compute final status
id: compute
run: |
LAB_TEST_STATUS="${{ needs.run-lab-tests.status }}"
UPDATE_PR_STATUS="${{ needs.update-pr.status }}"
# If run-lab-tests succeeded, use the status of update-pr, otherwise
# use run-lab-tests (which is "failed" or "error").
if [ "$LAB_TEST_STATUS" == "success" ]; then
echo "status=$UPDATE_PR_STATUS" >> $GITHUB_OUTPUT
else
echo "status=$LAST_TEST_STATUS" >> $GITHUB_OUTPUT
fi
- name: Report final status
uses: ./.github/workflows/custom-actions/set-commit-status
with:
context: Update All Screenshots
state: ${{ steps.compute.outputs.status }}
token: ${{ secrets.GITHUB_TOKEN }}

0 comments on commit 25fc81e

Please sign in to comment.