From 597e070158d9dc4d91c268b19603ee89bf98f056 Mon Sep 17 00:00:00 2001 From: sripwoud Date: Thu, 8 Aug 2024 17:26:59 +0200 Subject: [PATCH] ci: make required checks passed if skipped (#37) Fix #22 A [limitation of GitHub actions](https://github.com/orgs/community/discussions/13690) is that ci jobs that used as checks required to allow a PR to merge, are **always** required, even if they were skipped. In the case they were skipped...well the ci check isn't passed, not allowing a valid PR to merge. A [workaround](https://stackoverflow.com/a/77066140/9771158) is to use `${{ !(failure() || cancelled()) }}` as a condition, meaning: if the dependents jobs didn't fail or weren't cancelled, execute (especially: if dependents jobs were skipped, execute). This can be used as an `if` condition in a dummy job that will always be executed and successful, **unless the dependents jobs failed or were cancelled**. Which is what we want. The [workflow diagram](https://github.com/privacy-scaling-explorations/zk-kit.solidity/actions/runs/10297770771) illustrates well what is going on: ![image](https://github.com/user-attachments/assets/83d4728a-ee5f-45eb-9a40-ea7faf2e4bbe) This PR does not affect any `sol` files, so the `_tests` and `_slither` matrix jobs are all skipped. However `tests` and `slither` which depends on `_tests` and `_slither` respectively and are used as ci required checks, are still executed and pass, allowing the PR to merge. This will speed up even more the CI, and avoid ci checks being a stuck with `waiting for status to be reported` state. --- .github/workflows/main.yml | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a6736f0..4f5a56f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -45,7 +45,7 @@ jobs: id: changed-files uses: tj-actions/changed-files@v44 with: - files: packages/**/*.{sol,json,ts} + files: packages/**/*.{json,sol,ts} compile: if: needs.changed-files.outputs.any_changed == 'true' @@ -83,9 +83,10 @@ jobs: key: ${{ needs.deps.outputs.cache-key }} - run: yarn format - tests: + + _tests: if: needs.changed-files.outputs.any_changed == 'true' - needs: [changed-files, set-matrix, deps, compile] + needs: compile runs-on: ubuntu-latest strategy: matrix: @@ -119,6 +120,16 @@ jobs: parallel: true flag-name: run ${{ join(matrix.*, '-') }} + tests: + needs: _tests + # workaround for https://github.com/orgs/community/discussions/13690 + # https://stackoverflow.com/a/77066140/9771158 + if: ${{ !(failure() || cancelled()) }} + runs-on: ubuntu-latest + steps: + - name: Tests OK (passed or skipped) + run: true + set-matrix: if: needs.changed-files.outputs.any_changed == 'true' needs: changed-files @@ -133,7 +144,7 @@ jobs: matrix=$(ls -1 packages | jq -Rsc 'split("\n") | map(select(length > 0))') echo "matrix=$matrix" >> $GITHUB_OUTPUT - slither: + _slither: if: needs.changed-files.outputs.any_changed == 'true' needs: [changed-files, set-matrix, deps] runs-on: ubuntu-latest @@ -194,3 +205,11 @@ jobs: const header = '# Slither report' const body = process.env.REPORT await script({ github, context, header, body }) + + slither: + needs: _slither + if: ${{ !(failure() || cancelled()) }} + runs-on: ubuntu-latest + steps: + - name: Slither analysis OK (passed or skipped) + run: true