From 4a674377046820aaefa38a4b6c9f940eccd73fe7 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Fri, 22 Mar 2024 19:25:13 +0100 Subject: [PATCH] devops(gha): move from always() to !cancelled() (#30060) **Investigation:** - According to [here](https://github.com/microsoft/playwright/actions/runs/8351676198), the job got cancelled, because someone force-pushed and another commit took priority. - We have `if: always()` for uploading the blobs, even if they are cancelled. We expect, that the task before (`npx playwright test`) finished writing the blob, but it didn't - cancelled in between. We still upload - a broken one. - We download the broken zip and it breaks from there on. Proposed change: Instead of uploading always, upload if it did not get cancelled. Quoting from the GitHub Actions docs about `always()`: > Warning: Avoid using always for any task that could suffer from a critical failure, for example: getting sources, otherwise the workflow may hang until it times out. If you want to run a job or step regardless of its success or failure, use the recommended alternative: if: `${{ !cancelled() }}` This is phase 1/2 where it changes our code to use this new condition. The actual roll-out happens once it works for us. Relates https://github.com/microsoft/playwright/issues/29451 --- .github/actions/download-artifact/action.yml | 2 - .../action.yml | 3 - .github/actions/upload-blob-report/action.yml | 8 +- .github/workflows/publish_release_npm.yml | 4 +- .github/workflows/tests_electron.yml | 4 +- .github/workflows/tests_primary.yml | 24 ++-- .github/workflows/tests_secondary.yml | 114 +++++++++--------- .github/workflows/tests_service.yml | 4 +- .github/workflows/tests_stress.yml | 20 +-- .github/workflows/tests_video.yml | 2 +- .github/workflows/tests_webview2.yml | 2 +- 11 files changed, 90 insertions(+), 97 deletions(-) diff --git a/.github/actions/download-artifact/action.yml b/.github/actions/download-artifact/action.yml index d279df5f8d..3ab38a4c17 100644 --- a/.github/actions/download-artifact/action.yml +++ b/.github/actions/download-artifact/action.yml @@ -4,12 +4,10 @@ inputs: namePrefix: description: 'Name prefix of the artifacts to download' required: true - type: string default: 'blob-report' path: description: 'Directory with downloaded artifacts' required: true - type: string default: '.' runs: using: "composite" diff --git a/.github/actions/download-blob-report-from-azure/action.yml b/.github/actions/download-blob-report-from-azure/action.yml index 8026de64ea..848b98cfbc 100644 --- a/.github/actions/download-blob-report-from-azure/action.yml +++ b/.github/actions/download-blob-report-from-azure/action.yml @@ -4,16 +4,13 @@ inputs: blob_prefix: description: 'Name of the Azure blob storage directory containing blob report' required: true - type: string output_dir: description: 'Output directory where downloaded blobs will be stored' required: true - type: string default: 'blob-report' connection_string: description: 'Azure connection string' required: true - type: string runs: using: "composite" steps: diff --git a/.github/actions/upload-blob-report/action.yml b/.github/actions/upload-blob-report/action.yml index 87eda3baf8..43ef983e27 100644 --- a/.github/actions/upload-blob-report/action.yml +++ b/.github/actions/upload-blob-report/action.yml @@ -4,29 +4,27 @@ inputs: report_dir: description: 'Directory containing blob report' required: true - type: string default: 'test-results/blob-report' job_name: description: 'Unique job name' required: true - type: string default: '' runs: using: "composite" steps: - name: Upload blob report to GitHub - if: always() && github.event_name == 'pull_request' + if: ${{ !cancelled() && github.event_name == 'pull_request' }} uses: actions/upload-artifact@v4 with: name: blob-report-${{ inputs.job_name }} path: ${{ inputs.report_dir }}/** retention-days: 7 - name: Write triggering pull request number in a file - if: always() && github.event_name == 'pull_request' + if: ${{ !cancelled() && github.event_name == 'pull_request' }} shell: bash run: echo '${{ github.event.number }}' > pull_request_number.txt; - name: Upload artifact with the pull request number - if: always() && github.event_name == 'pull_request' + if: ${{ !cancelled() && github.event_name == 'pull_request' }} uses: actions/upload-artifact@v4 with: name: pull-request-${{ inputs.job_name }} diff --git a/.github/workflows/publish_release_npm.yml b/.github/workflows/publish_release_npm.yml index 3b92a01b14..46b5816834 100644 --- a/.github/workflows/publish_release_npm.yml +++ b/.github/workflows/publish_release_npm.yml @@ -25,10 +25,10 @@ jobs: - run: npm run build - run: npx playwright install-deps - run: utils/publish_all_packages.sh --release-candidate - if: "github.event.release.prerelease" + if: ${{ github.event.release.prerelease }} env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} - run: utils/publish_all_packages.sh --release - if: "!github.event.release.prerelease" + if: ${{ !github.event.release.prerelease }} env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/.github/workflows/tests_electron.yml b/.github/workflows/tests_electron.yml index 4427cc5e9c..05f7302747 100644 --- a/.github/workflows/tests_electron.yml +++ b/.github/workflows/tests_electron.yml @@ -42,7 +42,7 @@ jobs: - run: npm run etest if: matrix.os != 'ubuntu-latest' - run: node tests/config/checkCoverage.js electron - if: always() && matrix.os == 'ubuntu-latest' + if: ${{ !cancelled() && matrix.os == 'ubuntu-latest' }} - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash diff --git a/.github/workflows/tests_primary.yml b/.github/workflows/tests_primary.yml index 8c491514ae..5b4b080287 100644 --- a/.github/workflows/tests_primary.yml +++ b/.github/workflows/tests_primary.yml @@ -58,10 +58,10 @@ jobs: - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- npm run test -- --project=${{ matrix.browser }}-* - run: node tests/config/checkCoverage.js ${{ matrix.browser }} - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -92,10 +92,10 @@ jobs: PWTEST_CHANNEL: chromium-tip-of-tree PWTEST_BOT_NAME: "${{ matrix.os }}-chromium-tip-of-tree" - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -145,10 +145,10 @@ jobs: - run: xvfb-run npm run ttest -- --shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }} if: matrix.os == 'ubuntu-latest' - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -172,18 +172,18 @@ jobs: env: PWTEST_BOT_NAME: "web-components-html-reporter" - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: packages/html-reporter/blob-report job_name: "web-components-html-reporter" - run: npm run test-web - if: always() + if: ${{ !cancelled() }} env: PWTEST_BOT_NAME: "web-components-web" - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: packages/web/blob-report @@ -219,7 +219,7 @@ jobs: run: npm run test -- --workers=1 working-directory: ./playwright-vscode - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: playwright-vscode/blob-report @@ -255,10 +255,10 @@ jobs: - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- npm run itest if: matrix.os == 'ubuntu-latest' - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report diff --git a/.github/workflows/tests_secondary.yml b/.github/workflows/tests_secondary.yml index 7659c338bc..0ba6b9d848 100644 --- a/.github/workflows/tests_secondary.yml +++ b/.github/workflows/tests_secondary.yml @@ -45,10 +45,10 @@ jobs: - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- npm run test -- --project=${{ matrix.browser }}-* - run: node tests/config/checkCoverage.js ${{ matrix.browser }} - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -77,10 +77,10 @@ jobs: - run: npx playwright install --with-deps ${{ matrix.browser }} chromium - run: npm run test -- --project=${{ matrix.browser }}-* - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -113,10 +113,10 @@ jobs: if: matrix.browser != 'firefox' shell: bash - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -151,7 +151,7 @@ jobs: - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- npm run itest if: matrix.os == 'ubuntu-latest' - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash headed_tests: @@ -176,14 +176,14 @@ jobs: - run: npm run build - run: npx playwright install --with-deps ${{ matrix.browser }} chromium - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- npm run test -- --project=${{ matrix.browser }}-* --headed - if: always() && startsWith(matrix.os, 'ubuntu-') + if: ${{ !cancelled() && startsWith(matrix.os, 'ubuntu-') }} - run: npm run test -- --project=${{ matrix.browser }}-* --headed - if: always() && !startsWith(matrix.os, 'ubuntu-') + if: ${{ !cancelled() && !startsWith(matrix.os, 'ubuntu-') }} - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -213,10 +213,10 @@ jobs: env: PWTEST_MODE: ${{ matrix.mode }} - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -252,10 +252,10 @@ jobs: PWTEST_TRACE: 1 PWTEST_CHANNEL: ${{ matrix.channel }} - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -280,10 +280,10 @@ jobs: env: PWTEST_CHANNEL: chrome - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -309,10 +309,10 @@ jobs: env: PWTEST_CHANNEL: chrome - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -337,10 +337,10 @@ jobs: env: PWTEST_CHANNEL: chrome - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -371,10 +371,10 @@ jobs: - run: npm run ctest if: matrix.os != 'ubuntu-20.04' - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -405,10 +405,10 @@ jobs: - run: npm run ctest -- --headed if: matrix.os != 'ubuntu-latest' - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -433,10 +433,10 @@ jobs: env: PWTEST_CHANNEL: firefox-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -462,10 +462,10 @@ jobs: env: PWTEST_CHANNEL: firefox-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -490,10 +490,10 @@ jobs: env: PWTEST_CHANNEL: firefox-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -518,10 +518,10 @@ jobs: env: PWTEST_CHANNEL: msedge - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -547,10 +547,10 @@ jobs: env: PWTEST_CHANNEL: msedge - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -575,10 +575,10 @@ jobs: env: PWTEST_CHANNEL: msedge - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -603,10 +603,10 @@ jobs: env: PWTEST_CHANNEL: msedge-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -632,10 +632,10 @@ jobs: env: PWTEST_CHANNEL: msedge-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -660,10 +660,10 @@ jobs: env: PWTEST_CHANNEL: msedge-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -688,10 +688,10 @@ jobs: env: PWTEST_CHANNEL: msedge-dev - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -717,10 +717,10 @@ jobs: env: PWTEST_CHANNEL: msedge-dev - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -745,10 +745,10 @@ jobs: env: PWTEST_CHANNEL: msedge-dev - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -773,10 +773,10 @@ jobs: env: PWTEST_CHANNEL: chrome-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -802,10 +802,10 @@ jobs: env: PWTEST_CHANNEL: chrome-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -830,10 +830,10 @@ jobs: env: PWTEST_CHANNEL: chrome-beta - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report @@ -873,10 +873,10 @@ jobs: PLAYWRIGHT_CHROMIUM_USE_HEADLESS_NEW: 1 - run: node tests/config/checkCoverage.js chromium - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash - name: Upload blob report - if: always() + if: ${{ !cancelled() }} uses: ./.github/actions/upload-blob-report with: report_dir: blob-report diff --git a/.github/workflows/tests_service.yml b/.github/workflows/tests_service.yml index b8c192f988..2739680712 100644 --- a/.github/workflows/tests_service.yml +++ b/.github/workflows/tests_service.yml @@ -33,7 +33,7 @@ jobs: PLAYWRIGHT_SERVICE_OS: ${{ matrix.service-os }} PLAYWRIGHT_SERVICE_RUN_ID: ${{ github.run_id }}-${{ github.run_attempt }}-${{ github.sha }} - name: Upload blob report to GitHub - if: always() + if: ${{ !cancelled() }} uses: actions/upload-artifact@v3 with: name: all-blob-reports @@ -43,7 +43,7 @@ jobs: merge_reports: name: "Merge reports" needs: [test] - if: always() + if: ${{ !cancelled() }} runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/tests_stress.yml b/.github/workflows/tests_stress.yml index 7c2ce79088..d0409cd585 100644 --- a/.github/workflows/tests_stress.yml +++ b/.github/workflows/tests_stress.yml @@ -37,22 +37,22 @@ jobs: - run: npx playwright install firefox-asan if: matrix.os != 'windows-latest' - run: npm run stest contexts -- --project=chromium - if: always() + if: ${{ !cancelled() }} - run: npm run stest browsers -- --project=chromium - if: always() + if: ${{ !cancelled() }} - run: npm run stest frames -- --project=chromium - if: always() + if: ${{ !cancelled() }} - run: npm run stest contexts -- --project=webkit - if: always() + if: ${{ !cancelled() }} - run: npm run stest browsers -- --project=webkit - if: always() + if: ${{ !cancelled() }} - run: npm run stest frames -- --project=webkit - if: always() + if: ${{ !cancelled() }} - run: npm run stest contexts -- --project=firefox - if: always() + if: ${{ !cancelled() }} - run: npm run stest browsers -- --project=firefox - if: always() + if: ${{ !cancelled() }} - run: npm run stest frames -- --project=firefox - if: always() + if: ${{ !cancelled() }} - run: npm run stest heap -- --project=chromium - if: always() + if: ${{ !cancelled() }} diff --git a/.github/workflows/tests_video.yml b/.github/workflows/tests_video.yml index 3acda7cec7..b51c0e4e76 100644 --- a/.github/workflows/tests_video.yml +++ b/.github/workflows/tests_video.yml @@ -36,5 +36,5 @@ jobs: env: PWTEST_VIDEO: 1 - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash diff --git a/.github/workflows/tests_webview2.yml b/.github/workflows/tests_webview2.yml index 26c97f6062..cf9a2ef0e9 100644 --- a/.github/workflows/tests_webview2.yml +++ b/.github/workflows/tests_webview2.yml @@ -46,5 +46,5 @@ jobs: Start-Process -FilePath setup.exe -Verb RunAs -Wait - run: npm run webview2test - run: ./utils/upload_flakiness_dashboard.sh ./test-results/report.json - if: always() + if: ${{ !cancelled() }} shell: bash