-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
ci: Fix workflow to update screenshots #4993
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Say, the first two steps of this workflow are the exact same as the steps of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, those are separate jobs. Each job runs with a clean slate. You're thinking of "steps" within jobs. Calling another workflow (run-lab-tests) requires its own job. So the jobs are: 1. Set pending status, 2. run test workflow, 3. update PR, 4. set final status |
||
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 }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description of
ignore_test_status
says "If true, ignore test failures and treat the workflow as a success"However, this seems to be treating the workflow as a failure. Is the description inaccurate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means the job runs on failure, or if ignore_test_status is true. Normally, we would only upload screenshot artifacts if the tests failed. But if we're running the workflow specifically to get the screenshots updated, we upload them no matter what.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment updated. ignore_test_status modifies many things that normally depend on test status.