diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0091a9c..7f79508 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,11 +19,14 @@ on: permissions: contents: read -# Cancel superseded runs of a pull request; runs for pushes get their own +# Cancel superseded runs of a pull request, but only for a new head commit +# (not for a rerun, which may be of an older commit), so that a newer run on +# the same commit doesn't leave a failed gate (see +# "Cancelled runs" in actions/gate/README.md); runs for pushes get their own # group, so that queued runs for successive pushes are not dropped concurrency: group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.sha || github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + cancel-in-progress: ${{ github.event_name == 'pull_request' && github.event.action == 'synchronize' && github.run_attempt == 1 }} jobs: lint: @@ -224,6 +227,7 @@ jobs: name: CI passed runs-on: ubuntu-latest timeout-minutes: 5 + # Not `!cancelled()`: see "Cancelled runs" in actions/gate/README.md if: always() needs: [lint, test] permissions: {} diff --git a/actions/gate/README.md b/actions/gate/README.md index a2b9f3d..584d58a 100644 --- a/actions/gate/README.md +++ b/actions/gate/README.md @@ -37,6 +37,22 @@ passed: allow-skipped: ${{ github.event_name == 'pull_request' && 'push' || '' }} ``` +Let only a new head commit of a pull request cancel its runs in progress +(see [Cancelled runs](#cancelled-runs)), as in this repository's +`ci.yml`: + +```yaml +concurrency: + # Runs for pushes get their own group, so that none is dropped + group: >- + ${{ github.workflow }}-${{ github.event_name == 'push' + && github.sha || github.ref }} + cancel-in-progress: >- + ${{ github.event_name == 'pull_request' + && github.event.action == 'synchronize' + && github.run_attempt == 1 }} +``` + ## Inputs - `needs` (required): the results of the needed jobs, as @@ -45,9 +61,53 @@ passed: separated by whitespace; by default none. Compute it in the expression, e.g. from the event, so that a skip is allowed only where it is intended. +## Cancelled runs + +The gate job runs `always()`, so that it fails whenever a needed job failed +or was cancelled, whether by its `timeout-minutes`, by `fail-fast` in a +matrix, by hand or with the whole run. A cancelled run thus leaves a failed +`CI passed` on its commit, and as the newest result of a check on a commit +counts, that failure blocks the merge until a newer run on the same commit +passes. + +That is harmless when the cancelled run was testing an old commit, but it +blocks a pull request when a newer run on the **same** commit cancels it, +e.g. a run for editing the description, marking the pull request ready or +reopening it (event `pull_request`, actions `edited`, `ready_for_review`, +`reopened`): the cancelled run's failed gate is then the newest result on +the commit until the newer run's gate reports, and stays so if it never +does. Therefore only a new head commit (action `synchronize`, first +attempt) may cancel the runs of a pull request in progress +(`cancel-in-progress` above). A rerun keeps the event of its run, and a +rerun of an older commit's run must not cancel the run on the head commit, +so reruns never cancel. A run for another action, or a rerun, waits until +the run in progress has finished, and replaces a waiting one. A waiting +run that is replaced never starts any job, so it leaves no `CI passed` at +all. + +The gate job doesn't use `if: ${{ !cancelled() }}`, which would skip it in +cancelled runs: GitHub counts a skipped required check as passed +([Troubleshooting required status +checks](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks)), +so a run cancelled by hand, or replaced by a newer run that hasn't reported +yet, would allow the merge without tests. + +A run cancelled by hand fails the gate: rerun it, or push, before you merge. + +The price: after an event other than a push, e.g. marking a pull request +ready, its result comes only after the run in progress and then a whole new +run. Trigger CI only on the `pull_request` actions that it needs (`types`). + ## Tests -`tests/gate/test.sh` runs the script on sets of job results (all success, -a failure, a cancel, allowed and unexpected skips, empty `needs`); the -`test` job of this repository's CI also runs it, and the action itself on -passing and failing results. +`tests/gate/test.sh`: + +- runs the script on sets of job results (all success, a failure, a + cancel, allowed and unexpected skips, empty `needs`); +- checks that the gate jobs of this repository's CI and of the example + above run `always()`, and that only a new head commit of a pull request + cancels runs in progress. + +The `test` job of this repository's CI runs it, and the action itself on +passing and failing results. How GitHub applies the conditions and the +concurrency group shows only in real runs. diff --git a/tests/gate/test.sh b/tests/gate/test.sh index 4fcc270..d300abe 100755 --- a/tests/gate/test.sh +++ b/tests/gate/test.sh @@ -31,6 +31,7 @@ expect() { ok='{"result":"success","outputs":{}}' expect pass "{\"lint\":${ok},\"test\":${ok}}" expect fail "{\"lint\":${ok},\"test\":{\"result\":\"failure\"}}" "" "test: failure" +# Cancelled jobs fail the gate, whatever cancelled them (see the README) expect fail "{\"lint\":{\"result\":\"cancelled\"},\"test\":${ok}}" "" "lint: cancelled" expect pass "{\"lint\":${ok},\"push\":{\"result\":\"skipped\"}}" "push publish" # IDs on separate lines (e.g. a YAML block scalar) @@ -64,6 +65,85 @@ else echo "ok: step summary" fi +# The gate jobs of this repository's CI and of the README's example run +# `always()` (not `!cancelled()`), and only a new head commit of a pull +# request cancels runs in progress (see "Cancelled runs" in the README) +# gate_if : the job-level `if:` of the job `passed` +gate_if() { + awk ' + # The indent of the first non-blank character + { indent = match($0, /[^ ]/) } + /^ *passed:$/ { job = indent; next } + !job || !/^ *[^ #]/ { next } + indent <= job { exit } + # The indent of the job keys, from the first one + !key { key = indent } + indent == key && /^ *if:/ { sub(/^ *if: */, ""); print; exit } + ' "$1" +} +# The parser finds the job-level condition of `passed` only, not those of +# other jobs, comments or steps +fixture="$(mktemp)" +trap 'rm -f "${summary}" "${fixture}"' EXIT +printf 'jobs:\n lint:\n if: a\n passed:\n # if: b\n needs: [lint]\n if: c\n next:\n if: d\n' > "${fixture}" +if [[ "$(gate_if "${fixture}")" == c ]]; then + echo "ok: gate condition parser" +else + echo "NOT OK: gate condition parser: $(gate_if "${fixture}")" + failures=$((failures + 1)) +fi +printf 'jobs:\n passed:\n steps:\n - if: s\n run: x\n next:\n if: d\n' > "${fixture}" +if [[ -z "$(gate_if "${fixture}")" ]]; then + echo "ok: gate condition parser, no job-level condition" +else + echo "NOT OK: gate condition parser took a step's or job's condition: $(gate_if "${fixture}")" + failures=$((failures + 1)) +fi +# shellcheck disable=SC2016 # a GitHub expression, not a shell one +cancel_expected='${{ github.event_name == '\''pull_request'\'' && github.event.action == '\''synchronize'\'' && github.run_attempt == 1 }}' +for file in .github/workflows/ci.yml actions/gate/README.md; do + condition="$(gate_if "${file}")" + if [[ "${condition}" == "always()" ]]; then + echo "ok: gate condition in ${file}" + else + echo "NOT OK: gate condition in ${file}: ${condition@Q}" + failures=$((failures + 1)) + fi +done +# cancel_in_progress : `cancel-in-progress` of the workflow-level +# `concurrency` (not of a job's), also folded (`>-`) over several lines as +# in the README +cancel_in_progress() { + awk ' + /^concurrency:/ { top = 1; next } + top && /^[^ ]/ { exit } + top && /^ cancel-in-progress:/ { + sub(/^ cancel-in-progress: */, "") + if ($0 != ">-") { print; exit } + folded = 1; next + } + folded && /^ [^ ]/ { sub(/^ +/, ""); out = out (out ? " " : "") $0; next } + folded { exit } + END { if (out) print out } + ' "$1" +} +printf 'concurrency:\n group: g\njobs:\n a:\n concurrency:\n cancel-in-progress: x\n' > "${fixture}" +if [[ -z "$(cancel_in_progress "${fixture}")" ]]; then + echo "ok: cancel-in-progress parser, workflow level only" +else + echo "NOT OK: cancel-in-progress parser took a job's setting" + failures=$((failures + 1)) +fi +for file in .github/workflows/ci.yml actions/gate/README.md; do + cancel="$(cancel_in_progress "${file}")" + if [[ "${cancel}" == "${cancel_expected}" ]]; then + echo "ok: cancel-in-progress in ${file}" + else + echo "NOT OK: cancel-in-progress in ${file}: ${cancel@Q}" + failures=$((failures + 1)) + fi +done + if ((failures > 0)); then echo "${failures} test(s) failed" exit 1