From 96a0ca05396f900db9dc49de67d961d8abc7cac7 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Tue, 27 Feb 2024 15:24:47 -0800 Subject: [PATCH] ci: Improve CI status setting (#6306) - Link CI status to specific jobs, not just the run - Split selenium job status flag away from ignore_test_status (which is for getting screenshots) - Remove redundant status setting step in update-screenshots - Fix final status computation in update-screenshots to account for edge cases and old typo --- .../set-commit-status/action.yaml | 36 +++++++++++++++---- .github/workflows/selenium-lab-tests.yaml | 22 +++++++++--- .github/workflows/update-screenshots.yaml | 32 +++++++++-------- 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/.github/workflows/custom-actions/set-commit-status/action.yaml b/.github/workflows/custom-actions/set-commit-status/action.yaml index c2da72bba4..7b16edadf7 100644 --- a/.github/workflows/custom-actions/set-commit-status/action.yaml +++ b/.github/workflows/custom-actions/set-commit-status/action.yaml @@ -14,6 +14,9 @@ inputs: token: description: A GitHub access token. required: true + job_name: + description: A job name, so that the status' target URL can point to a specific job. + required: false runs: using: composite @@ -21,17 +24,36 @@ runs: - name: Report Commit Status shell: bash run: | - # This is the URL to view this workflow run on GitHub. It will be - # attached to the commit status, so that when someone clicks "details" - # next to the status on the PR, it will link to this run where they can - # see the logs. - RUN_URL="https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}" + export GITHUB_TOKEN="${{ inputs.token }}" + + # Here we compute a "target URL". It will be attached to the commit + # status, so that when someone clicks "details" next to the status on + # the PR, it will link to the appropriate logs. + if [[ "${{ inputs.job_name }}" != "" ]]; then + # There are three identifiers for the job: + # - The job's key in YAML, which is "github.job" + # - The job's name, which is not provided by any runner environment + # - The job's numerical ID, which is not provided either + # To link to this specific job in the status, we need the numerical + # job ID. The GH API provides a list of jobs, which contain + # numerical IDs and names, but not keys. So the caller of this + # action must provide the string name, and then we look up the + # numerical ID in the API. "github.job" is useless here. + job_id=$( + gh api /repos/${{github.repository}}/actions/runs/${{github.run_id}}/jobs \ + | jq '.jobs | map(select(.name=="${{ inputs.job_name }}")) | .[].id' + ) + TARGET_URL="https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}/job/$job_id" + else + # Generic link to the run, without any specific job. + TARGET_URL="https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}" + fi + SHA1=$(git rev-parse HEAD) - GITHUB_TOKEN=${{ inputs.token }} \ gh api \ -X POST \ -F "context=${{ inputs.context }}" \ -F "state=${{ inputs.state }}" \ - -F "target_url=$RUN_URL" \ + -F "target_url=$TARGET_URL" \ "repos/${{ github.repository }}/statuses/$SHA1" diff --git a/.github/workflows/selenium-lab-tests.yaml b/.github/workflows/selenium-lab-tests.yaml index d1bfa4f287..f0ea6ed884 100644 --- a/.github/workflows/selenium-lab-tests.yaml +++ b/.github/workflows/selenium-lab-tests.yaml @@ -31,9 +31,17 @@ on: required: false type: string ignore_test_status: - description: "If true, ignore test success or failure, never set the commit status, and always upload screenshots." + description: "If true, ignore test success or failure, and always upload screenshots." required: false type: boolean + skip_commit_status: + description: "If true, skip the commit status." + required: false + type: boolean + job_name_prefix: + description: "A prefix added to the job name when setting commit status, needed to correctly link to each job. Use when skip_commit_status is false, and set to the name of the parent job, plus space-slash-space." + required: false + type: string schedule: # Runs every night at 2am PST / 10am UTC, testing against the main branch. - cron: '0 10 * * *' @@ -138,10 +146,11 @@ jobs: ref: ${{ needs.compute-ref.outputs.REF }} - name: Set commit status to pending - if: ${{ inputs.ignore_test_status == false }} + if: ${{ inputs.skip_test_status == false }} uses: ./.github/workflows/custom-actions/set-commit-status with: context: Selenium / Build + job_name: "${{ inputs.job_name_prefix }}Pre-build Player" state: pending token: ${{ secrets.GITHUB_TOKEN }} @@ -182,10 +191,11 @@ jobs: - name: Report final commit status # 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 }} + if: ${{ (success() || failure()) && inputs.skip_commit_status == false }} uses: ./.github/workflows/custom-actions/set-commit-status with: context: Selenium / Build + job_name: "${{ inputs.job_name_prefix }}Pre-build Player" state: ${{ job.status }} token: ${{ secrets.GITHUB_TOKEN }} @@ -206,10 +216,11 @@ jobs: ref: ${{ needs.compute-ref.outputs.REF }} - name: Set commit status to pending - if: ${{ inputs.ignore_test_status == false }} + if: ${{ inputs.skip_commit_status == false }} uses: ./.github/workflows/custom-actions/set-commit-status with: context: Selenium / ${{ matrix.browser }} + job_name: "${{ inputs.job_name_prefix }}${{ matrix.browser }}" state: pending token: ${{ secrets.GITHUB_TOKEN }} @@ -362,9 +373,10 @@ jobs: - name: Report final commit status # 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 }} + if: ${{ (success() || failure()) && inputs.skip_commit_status == false }} uses: ./.github/workflows/custom-actions/set-commit-status with: context: Selenium / ${{ matrix.browser }} + job_name: "${{ inputs.job_name_prefix }}${{ matrix.browser }}" state: ${{ job.status }} token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/update-screenshots.yaml b/.github/workflows/update-screenshots.yaml index 14496765d5..7b998612b1 100644 --- a/.github/workflows/update-screenshots.yaml +++ b/.github/workflows/update-screenshots.yaml @@ -35,6 +35,7 @@ jobs: pr: ${{ inputs.pr }} test_filter: layout ignore_test_status: true + job_name_prefix: "Get Selenium Lab Screenshots / " update-pr: name: Update PR @@ -45,13 +46,6 @@ jobs: 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 }} - - name: Get artifacts uses: actions/download-artifact@v4 with: @@ -108,15 +102,23 @@ jobs: - 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 + # The final status must be one of: success, failure, error, pending. + # However, the status from "result" from an earlier job is one of: + # success, failure, cancelled, skipped. + # We start by mapping those. + LAB_TEST_RESULT=$(echo "${{ needs.run-lab-tests.result }}" \ + | sed -Ee 's/(cancelled|skipped)/error/') + UPDATE_PR_RESULT=$(echo "${{ needs.update-pr.result }}" \ + | sed -Ee 's/(cancelled|skipped)/error/') + + if [[ "$LAB_TEST_RESULT" == "success" ]]; then + # If run-lab-tests succeeded, use the status of update-pr, which + # comes after that. If that is blank, default to "error". + echo "status=${UPDATE_PR_RESULT:-error}" >> $GITHUB_OUTPUT else - echo "status=$LAST_TEST_STATUS" >> $GITHUB_OUTPUT + # If run-lab-tests failed, use that. If that is blank, default to + # "error". + echo "status=${LAB_TEST_RESULT:-error}" >> $GITHUB_OUTPUT fi - name: Report final status