Skip to content

Commit

Permalink
ci: make required checks passed if skipped (#37)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sripwoud authored Aug 8, 2024
1 parent 8bd1e14 commit 597e070
Showing 1 changed file with 23 additions and 4 deletions.
27 changes: 23 additions & 4 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

0 comments on commit 597e070

Please sign in to comment.