timothy/ersatztv
1
0
Fork 0
Code Issues 58 Pull Requests 8 Actions Packages Projects Releases 9 Wiki Activity

probe(756): DO NOT MERGE — measure a dropped step on the BUILD lane #765

Closed
timothy wants to merge 2 commits from probe756/dropped-step-build-lane into main
pull from: probe756/dropped-step-build-lane
merge into: :main
Conversation 1 Commits 2 Files Changed 3
+809 -18
3 changed files with 809 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.gitea/workflows/docker-build.yml
+136 -18
View File
@@ -122,14 +122,26 @@ jobs:
# ersatztv#416: is this a docs-only change? If so, every heavy step below is skipped and this
# REQUIRED job reports success in seconds. It still RUNS (never `if:`-skipped) so the required
# context keeps reporting — see the workflow header and docs/ci-cd.md -> "Docs-only skip".
# EVERY consequential `run:` step in this job marks itself as its FIRST act (ersatztv#756),
# and the trailing `Assert every expected step executed` guard fails the job when one is
# missing. This is a REQUIRED context on `main`, and a step the runner drops takes the job
# GREEN having done no work — see scripts/ci-step-ran.sh for why that is fail-OPEN here while
# the same drop in review-verdict.yml is fail-CLOSED.
- name: Detect docs-only changes
id: detect
run: scripts/ci-detect-docs-only.sh
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark detect
scripts/ci-detect-docs-only.sh
- name: Detect already-validated tree (#420)
id: revalidate
env:
ETV_STATUS_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}
run: scripts/ci-detect-already-validated.sh
run: |
# PROBE ONLY (ersatztv#756) - reintroduces the #751 defect: `pr number` is not a valid
# expression, so the runner fails to interpolate this whole scalar and DROPS the step:
# ${{ pr number }}
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark revalidate
scripts/ci-detect-already-validated.sh
- name: Cache NuGet packages
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
@@ -141,7 +153,9 @@ jobs:
- name: Restore
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
run: dotnet restore
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark restore
dotnet restore
# Replaces setup-node's built-in `cache: npm`. The toolchain image supplies node/npm, but
# the SPA's package downloads are project deps, so they stay cached per lockfile.
@@ -156,36 +170,50 @@ jobs:
- name: Install SPA dependencies
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
working-directory: web
run: npm ci
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark npm-ci
npm ci
- name: Check generated SPA API client
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
working-directory: web
run: npm run check:api
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark check-api
npm run check:api
- name: Lint SPA
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
working-directory: web
run: npm run lint
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark lint
npm run lint
- name: Typecheck SPA
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
working-directory: web
run: npm run typecheck
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark typecheck
npm run typecheck
- name: Test SPA
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
working-directory: web
run: npm test -- --run
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark web-test
npm test -- --run
- name: Build SPA
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
working-directory: web
run: npm run build
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark web-build
npm run build
- name: Strip Scanner project ref (matches Docker build)
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
run: sed -i '/Scanner/d' ErsatzTV/ErsatzTV.csproj
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark strip-scanner
sed -i '/Scanner/d' ErsatzTV/ErsatzTV.csproj
# Start the true peak-anon sampler just before the memory-heavy dotnet Build/Test/Coverage so
# its high-water mark spans them (SPA build/test above are comparatively light). Paired with the
@@ -199,13 +227,16 @@ jobs:
- name: Build
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
run: dotnet build --configuration Release --no-restore
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark build
dotnet build --configuration Release --no-restore
- name: Test
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
run: >-
dotnet test --configuration Release --no-build --blame-hang-timeout "2m" --verbosity normal
--collect:"XPlat Code Coverage" --settings coverlet.runsettings --results-directory ./coverage
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark dotnet-test
dotnet test --configuration Release --no-build --blame-hang-timeout "2m" --verbosity normal \
--collect:"XPlat Code Coverage" --settings coverlet.runsettings --results-directory ./coverage
# Coverage reporting (ersatztv#15 scope item 4): coverlet.collector emits a Cobertura report
# per test project (via --collect above); ReportGenerator merges them into a human-readable
@@ -258,6 +289,43 @@ jobs:
continue-on-error: true
run: scripts/ci-peak-anon.sh report
# THE DROPPED-STEP GUARD (ersatztv#756). Every `run:` step above records that it began; this
# asserts the whole expected SET was recorded. A step the runner declines to interpolate is
# DROPPED and still concludes `success` (ersatztv#751), so without this a REQUIRED context
# reports green having done no work — fail-OPEN, and strictly worse than the fail-CLOSED
# version of the same bug that #751 fixed in review-verdict.yml.
#
# NO `if:` HERE, WHICH IS A DELIBERATE DEPARTURE FROM THE #751 GUARD and the one decision in
# this block that is easy to "fix" wrongly. That guard uses `if: always()` because its job has
# exactly one real step, so there is no ordinary red for it to talk over. Here there are
# twelve, and a genuine failure in an early one (a lint error, a failing test) SKIPS every
# later step — an `always()` guard would then announce "these steps never executed: typecheck
# web-test build dotnet-test" on top of every normal red build. That is not a dropped step, it
# is the runner doing what it is told, and a guard that cries wolf on every red build is a
# guard that gets deleted.
#
# The default `if:` is `success()`, which is exactly the condition wanted, and the invariant it
# rests on is worth stating because it is what makes the omission safe rather than lucky: this
# step is skipped ONLY when an earlier step failed, and an earlier step failing already fails
# the job. So `guard skipped => job red`, and the only path to a green job runs the guard. A
# dropped step is invisible precisely because it concludes `success`, which keeps the job green
# and therefore reaches here.
#
# ITS OWN BODY CANNOT BE DROPPED BY THE MECHANISM IT GUARDS AGAINST: it is a single command
# with no expression delimiter anywhere in the scalar, so the runner has nothing to rewrite.
# The two gate values come in through `env:`, which is interpolated PER VALUE — a bad payload
# there cannot take the body with it (`ci.workflow-run-body-no-expressions`), and both paths
# are held to naming a real context by
# test_every_workflow_expression_names_a_REAL_context_or_function.
- name: Assert every expected step executed (ersatztv#756)
env:
ETV_DOCS_ONLY: ${{ steps.detect.outputs.docs_only }}
ETV_REVALIDATE_SKIP: ${{ steps.revalidate.outputs.skip }}
run: >-
scripts/ci-step-ran.sh assert
--always detect revalidate
--gated restore npm-ci check-api lint typecheck web-test web-build strip-scanner build dotnet-test
migrations:
name: EF migration integrity (SQLite + MySql)
runs-on: ubuntu-latest
@@ -328,14 +396,21 @@ jobs:
# ersatztv#416: docs-only? Skip the build + migration replay; the job still reports success in
# seconds. REQUIRED context, so it always RUNS (never `if:`-skipped). See the workflow header.
# Same per-step marker contract as the `test` job above (ersatztv#756) — this is the other
# REQUIRED context, so a dropped migration-replay step would report EF integrity green having
# replayed nothing.
- name: Detect docs-only changes
id: detect
run: scripts/ci-detect-docs-only.sh
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark detect
scripts/ci-detect-docs-only.sh
- name: Detect already-validated tree (#420)
id: revalidate
env:
ETV_STATUS_AUTH: ${{ secrets.REGISTRY_USER }}:${{ secrets.REGISTRY_PASSWORD }}
run: scripts/ci-detect-already-validated.sh
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark revalidate
scripts/ci-detect-already-validated.sh
- name: Cache NuGet packages
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
@@ -347,11 +422,15 @@ jobs:
- name: Restore
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
run: dotnet restore
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark restore
dotnet restore
- name: Build
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
run: dotnet build --configuration Release --no-restore
run: |
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark build
dotnet build --configuration Release --no-restore
# dotnet-ef is baked into the CI toolchain image (docker/ci/Dockerfile) and already on PATH
# — no per-run `dotnet tool install`. Bump its version there (ersatztv#390).
@@ -361,6 +440,7 @@ jobs:
if: steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
run: |
set -euo pipefail
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark sqlite
echo "::group::SQLite model drift (has-pending-model-changes)"
dotnet ef migrations has-pending-model-changes --no-build --configuration Release \
--context TvContext --startup-project ErsatzTV --project ErsatzTV.Infrastructure.Sqlite -- --provider Sqlite
@@ -384,6 +464,7 @@ jobs:
MySql__ConnectionString: "Server=mysql;Port=3306;Database=ersatztv_migrations;Uid=root;Pwd=ersatztv;DefaultCommandTimeout=300;"
run: |
set -euo pipefail
"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark mysql
echo "::group::MySql model drift (has-pending-model-changes)"
dotnet ef migrations has-pending-model-changes --no-build --configuration Release \
--context TvContext --startup-project ErsatzTV --project ErsatzTV.Infrastructure.MySql -- --provider MySql
@@ -418,6 +499,43 @@ jobs:
# how the original defects escaped. The fixture itself is retained and is opt-in via
# ETV_TEST_MYSQL_CONNECTION (skipped, visibly, without it). Re-arming it here is tracked by #627.
# THE DROPPED-STEP GUARD (ersatztv#756). Every `run:` step above records that it began; this
# asserts the whole expected SET was recorded. A step the runner declines to interpolate is
# DROPPED and still concludes `success` (ersatztv#751), so without this a REQUIRED context
# reports green having done no work — fail-OPEN, and strictly worse than the fail-CLOSED
# version of the same bug that #751 fixed in review-verdict.yml.
#
# NO `if:` HERE, WHICH IS A DELIBERATE DEPARTURE FROM THE #751 GUARD and the one decision in
# this block that is easy to "fix" wrongly. That guard uses `if: always()` because its job has
# exactly one real step, so there is no ordinary red for it to talk over. Here there are
# twelve, and a genuine failure in an early one (a lint error, a failing test) SKIPS every
# later step — an `always()` guard would then announce "these steps never executed: typecheck
# web-test build dotnet-test" on top of every normal red build. That is not a dropped step, it
# is the runner doing what it is told, and a guard that cries wolf on every red build is a
# guard that gets deleted.
#
# The default `if:` is `success()`, which is exactly the condition wanted, and the invariant it
# rests on is worth stating because it is what makes the omission safe rather than lucky: this
# step is skipped ONLY when an earlier step failed, and an earlier step failing already fails
# the job. So `guard skipped => job red`, and the only path to a green job runs the guard. A
# dropped step is invisible precisely because it concludes `success`, which keeps the job green
# and therefore reaches here.
#
# ITS OWN BODY CANNOT BE DROPPED BY THE MECHANISM IT GUARDS AGAINST: it is a single command
# with no expression delimiter anywhere in the scalar, so the runner has nothing to rewrite.
# The two gate values come in through `env:`, which is interpolated PER VALUE — a bad payload
# there cannot take the body with it (`ci.workflow-run-body-no-expressions`), and both paths
# are held to naming a real context by
# test_every_workflow_expression_names_a_REAL_context_or_function.
- name: Assert every expected step executed (ersatztv#756)
env:
ETV_DOCS_ONLY: ${{ steps.detect.outputs.docs_only }}
ETV_REVALIDATE_SKIP: ${{ steps.revalidate.outputs.skip }}
run: >-
scripts/ci-step-ran.sh assert
--always detect revalidate
--gated restore build sqlite mysql
functional-e2e:
name: Functional E2E (curl + UI contracts)
runs-on: ubuntu-latest
scripts/ci-step-ran.sh
Executable
+161
View File
@@ -0,0 +1,161 @@
#!/usr/bin/env bash
# Per-step execution markers for the two REQUIRED docker-build.yml jobs (ersatztv#756).
#
# WHY THIS EXISTS. A `run:` body the runner declines to interpolate is DROPPED, and the job still
# concludes `success` (ersatztv#751, `ci.workflow-run-body-no-expressions`). In
# `review-verdict.yml` that is fail-CLOSED — the required `review-verdict/h10` is simply absent and
# the merge is blocked. In `docker-build.yml` it is fail-OPEN: `Build & test (.NET)` and
# `EF migration integrity (SQLite + MySql)` are the other two required contexts on `main`, so a
# dropped step there sends a required check GREEN having done no work. #751 guarded the safe
# direction because that is where the live bug was, not because these were checked.
#
# WHY PER STEP, NOT PER JOB, which is what #756 proposed. A marker written by the job's FIRST step
# only proves the job started. The dangerous drop is not step 1 — it is `Test`, or the migration
# replay: the job runs everything around them, reports green, and nothing ran that anyone cared
# about. A guard that cannot see the fail-open case it was built for is the "guard that never
# executed" failure one level up. So every consequential step marks itself and the trailing
# `if: always()` guard asserts the whole expected SET.
#
# WHY A SCRIPT AND NOT AN INLINE BODY, unlike the #751 guard. Two reasons, and the second is the
# load-bearing one:
#
# * The path literal exists ONCE. The #751 guard carries it twice (write + assert) and its tests
# spend real effort proving the two copies agree, because a divergence reddens every run and
# then gets deleted as broken. Here they cannot diverge.
# * A one-line `run: scripts/ci-step-ran.sh …` CANNOT CONTAIN AN EXPRESSION DELIMITER, so the
# mechanism this guards against cannot drop the guard itself. #751's own record names this as
# the stronger construction ("the body would have had to move into scripts/, where a one-line
# run: makes the class unreachable") and settled for inline only because the measurement showed
# it was not required there.
#
# WHY A SCRIPT IS ACCEPTABLE HERE THOUGH IT WOULD NOT BE IN review-verdict.yml. That workflow
# checks out the PR's BASE precisely so a PR cannot supply the code that judges it. `docker-build.yml`
# is head-resolved by design — a PR already supplies every test this job runs — so calling a script
# from the head adds no authority a PR did not already have. This is a CORRECTNESS gate against
# silent no-ops, not a security gate against a hostile PR; that job belongs to `review-verdict/h10`.
# Do not copy this reasoning back into the gate workflow.
#
# THE MARKER FILE IS KEYED ON THE RUN, not a fixed name. `RUNNER_TEMP` was measured to be `/tmp` on
# this instance (ersatztv#751) — a shared directory, not a private per-job dir — so a fixed name
# lives where another job can also write it, and a STALE marker would satisfy the guard on a run
# whose step was dropped: a silent PASS, the exact failure being removed. Job id, run id and attempt
# together mean no other run or job can answer for this one. If the runner exports none of them the
# path degrades to a fixed name, which is no worse than having no key at all.
set -euo pipefail
usage() {
cat >&2 <<'EOF'
usage:
ci-step-ran.sh mark <key>
Record that this step began executing. Call it as the step's FIRST act, before
anything in the body can fail.
ci-step-ran.sh assert --always <key>... [--gated <key>...]
Fail unless every expected key was marked. --always keys are always required.
--gated keys are required only when the job's skip gates did NOT fire, read from
ETV_DOCS_ONLY / ETV_REVALIDATE_SKIP so this mirrors the steps' own `if:`.
EOF
exit 2
}
marker_path() {
printf '%s/etv-ci-steps-ran-%s-%s-%s' \
"${RUNNER_TEMP:-${GITHUB_WORKSPACE:-/tmp}}" \
"${GITHUB_JOB:-nojob}" \
"${GITHUB_RUN_ID:-norunid}" \
"${GITHUB_RUN_ATTEMPT:-1}"
}
cmd_mark() {
[ "$#" -eq 1 ] && [ -n "$1" ] || usage
# Appended, never truncated: every step in the job shares one file, and a `>` here would erase
# its predecessors and make the guard red on every run.
#
# A failure to write is NOT swallowed. The step is running under `bash -e`, so a non-zero here
# fails the step and reddens the job — which is the same direction the guard would take a moment
# later, but with a message pointing at the real cause instead of at a missing marker.
printf '%s\n' "$1" >> "$(marker_path)"
}
cmd_assert() {
local -a always=() gated=()
local bucket=""
while [ "$#" -gt 0 ]; do
case "$1" in
--always) bucket=always ;;
--gated) bucket=gated ;;
-*) usage ;;
*)
case "$bucket" in
always) always+=("$1") ;;
gated) gated+=("$1") ;;
*) usage ;;
esac ;;
esac
shift
done
# ANTI-VACUITY, at runtime rather than only in the test suite. An `assert` called with no
# expectations passes unconditionally and reports "every expected step executed" — a guard that
# proves nothing while looking like it proved everything. Refuse instead.
if [ "${#always[@]}" -eq 0 ] && [ "${#gated[@]}" -eq 0 ]; then
echo "::error::ci-step-ran.sh assert was called with no expected keys, so it would pass unconditionally. This is a workflow bug, not a build failure." >&2
exit 2
fi
# The skip gates, mirroring the `if:` every gated step carries:
# steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
# Anything other than the exact string `true` means the step was expected to run — including the
# EMPTY string, which is what these read as when the detect step itself was dropped. That
# direction is deliberate: a dropped detect step must widen what is required, never narrow it.
local skipped=no
if [ "${ETV_DOCS_ONLY:-}" = "true" ] || [ "${ETV_REVALIDATE_SKIP:-}" = "true" ]; then
skipped=yes
fi
local marker
marker="$(marker_path)"
# `${arr[@]+"${arr[@]}"}` rather than a bare `"${arr[@]}"`: under `set -u` bash 3.2 (the system
# bash on the Macs this suite also runs on) treats expanding an EMPTY array as an unbound
# variable and aborts. The CI image ships bash 5, where it is fine — which is exactly the kind of
# difference that makes a guard pass locally and die on the runner, or the reverse.
local -a expected=(${always[@]+"${always[@]}"})
if [ "$skipped" = no ]; then
expected+=(${gated[@]+"${gated[@]}"})
else
echo "Skip gate fired (docs_only='${ETV_DOCS_ONLY:-}', already_validated='${ETV_REVALIDATE_SKIP:-}') — the gated steps were not expected to run."
fi
local -a missing=()
local key
for key in "${expected[@]}"; do
# `grep -qxF` over a FILE, never a pipeline: `grep -q` exits at its first match and would
# SIGPIPE a producer, which under `set -o pipefail` inverts the result for large inputs
# (ersatztv#698). Reading the file directly has no producer to kill. `-x` so a key cannot be
# satisfied by another key that contains it, `-F` so a key is never read as a pattern.
if ! grep -qxF "$key" "$marker" 2>/dev/null; then
missing+=("$key")
fi
done
if [ "${#missing[@]}" -gt 0 ]; then
echo "::error::These steps of job '${GITHUB_JOB:-?}' never executed: ${missing[*]}. The runner DROPPED them (an interpolation failure over a run: body does this and still reports the job GREEN — ersatztv#751/#756) or their \`if:\` no longer matches the guard's expectations. This job is a REQUIRED check, so a green here would mean a required context passed having done no work. Failing the job so it is visible."
if [ -f "$marker" ]; then
echo "Marker file ${marker} recorded:"
sed 's/^/ /' "$marker"
else
echo "There is no marker file at ${marker} at all — not one step of this job executed."
fi
exit 1
fi
echo "All ${#expected[@]} expected step(s) executed: ${expected[*]}"
}
[ "$#" -ge 1 ] || usage
sub="$1"
shift
case "$sub" in
mark) cmd_mark "$@" ;;
assert) cmd_assert "$@" ;;
*) usage ;;
esac
scripts/tests/test_ci_dropped_step_guard.py
+512
View File
@@ -0,0 +1,512 @@
"""The dropped-step guard on docker-build.yml's two REQUIRED jobs (ersatztv#756).
WHAT THIS IS PROTECTING. A `run:` body the runner declines to interpolate is DROPPED, and the job
still concludes `success` (ersatztv#751, `ci.workflow-run-body-no-expressions`). #751 fixed that in
`review-verdict.yml`, where the consequence is fail-CLOSED — `review-verdict/h10` is absent and the
merge is blocked. It left the two places where the same drop is fail-OPEN: `Build & test (.NET)` and
`EF migration integrity (SQLite + MySql)` are the other two required contexts on `main`, so a dropped
step there sends a required check green having done no work.
THE TESTS COME IN THREE KINDS AND NONE SUBSTITUTES FOR ANOTHER, which is the lesson #751 paid for:
* STATIC — the marker set and the guard's expectations agree, and the guard is positioned so it
can actually run. Cheap, and the only kind that catches a NEW step added without a marker.
* BEHAVIOURAL — the guard's real command line is EXECUTED against markers written by the steps'
real marker lines, both extracted from the parsed workflow. A structural test cannot prove an
exit code, and `exit 1` in a body is satisfiable by dead code.
* A LIVE PROBE — that the runner still executes a LATER step after dropping an earlier one, on the
BUILD lane rather than the `small` lane #751 measured. That is the premise the whole guard rests
on and no test here can establish it; it is recorded in docs/ci-cd.md and on the issue.
"""
from __future__ import annotations
import os
import re
import subprocess
from pathlib import Path
import pytest
import yaml
REPO_ROOT = Path(__file__).resolve().parents[2]
WORKFLOW = REPO_ROOT / ".gitea" / "workflows" / "docker-build.yml"
SCRIPT = REPO_ROOT / "scripts" / "ci-step-ran.sh"
# The jobs whose contexts branch protection REQUIRES on `main`. Read live on 2026-08-10:
# Build ErsatzTV Image / Build & test (.NET) (pull_request)
# Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request)
# review-verdict/h10
# The third is guarded by test_pr_changed_files.py; these two are this file's subject. `build`,
# `api-docs` and `format` are deliberately NOT here — they are not required, and all three
# legitimately interpolate into a `run:` body, so extending the absolute rule to them would be false.
REQUIRED_JOBS = ("test", "migrations")
_EXPR = re.compile(r"\$\{\{(.*?)\}\}", re.S)
_MARK = re.compile(r'ci-step-ran\.sh"?\s+mark\s+(\S+)')
# ONE parse, shared. `yaml.safe_load` per call returns a fresh object graph, so an identity test
# across two helpers (`steps[-1] is guard`) would compare structurally-equal but distinct dicts and
# fail — or, worse in the other direction, an `is not` filter would exclude nothing and a step would
# match as its own guard. That is not hypothetical: test_pr_changed_files.py records exactly this
# going wrong in the #751 guard test, where the assertions then ran against the wrong step.
_DOC = yaml.safe_load(WORKFLOW.read_text())
def _doc():
return _DOC
def _steps(job: str):
return _doc()["jobs"][job]["steps"]
def _run_steps(job: str):
return [s for s in _steps(job) if s.get("run")]
def _guard(job: str):
"""The trailing assert step. Located by CONTENT, never by index.
Locating it as `steps[-1]` here and then asserting it is last elsewhere would be circular — the
position test would hold by construction. This finds the step that invokes the assert
sub-command, and `test_the_guard_is_the_LAST_step` independently checks where it sits.
"""
hits = [s for s in _run_steps(job) if "ci-step-ran.sh assert" in s["run"]]
assert len(hits) == 1, f"job '{job}' has {len(hits)} assert steps, expected exactly 1"
return hits[0]
def _marked(job: str):
"""[(step, key)] for every step that records its own execution, in declaration order."""
out = []
for s in _run_steps(job):
m = _MARK.search(s["run"])
if m:
out.append((s, m.group(1)))
return out
def _guard_buckets(job: str):
"""(always_keys, gated_keys) as the guard's own argv spells them."""
argv = _guard(job)["run"].split()
assert "--always" in argv and "--gated" in argv, argv
a, g = argv.index("--always"), argv.index("--gated")
return argv[a + 1:g], argv[g + 1:]
# Mirrors the `if:` every gated step in these jobs carries. Compared as a normalised string rather
# than by parsing the expression: what matters is that a step's gating and the guard's bucketing are
# the SAME condition, and any rewrite of one that is not mirrored in the other should be loud.
SKIP_GATE = ("steps.detect.outputs.docs_only!='true'&&steps.revalidate.outputs.skip!='true'")
def _is_gated(step) -> bool:
return re.sub(r"\s+", "", str(step.get("if", ""))) == SKIP_GATE
# ------------------------------------------------------------------------------------------------
# STATIC
# ------------------------------------------------------------------------------------------------
@pytest.mark.parametrize("job", REQUIRED_JOBS)
def test_the_required_jobs_have_NO_expression_delimiter_in_any_run_body(job):
"""The absolute rule from `review-verdict.yml`, extended to the two required build jobs.
This is the cheaper and more general half of #756: the drop mechanism REQUIRES a delimiter in
the scalar, so a job with none is immune by construction and the runtime markers are a backstop
rather than the only line of defence. It is enforceable today because both jobs are already
delimiter-free (measured 2026-08-10: `test` 0, `migrations` 0, while `build` has 2, `api-docs` 1
and `format` 1 — which is exactly why the rule is scoped to the required pair and not repo-wide).
The escape hatch when a value really is needed is the step's `env:` block, which is interpolated
PER VALUE, so a payload that does not evaluate cannot take the body with it.
The `run:` SCALAR AS PARSED, comments and all. A shell comment inside a `run:` body is NOT inert
— that is the whole #751 defect — so this must never filter comments out. Ordinary YAML comments
outside a `run:` body ARE inert and are not read here.
"""
offenders = [
f"{s.get('name', '?')}: {m.group(1).strip()!r}"
for s in _run_steps(job)
for m in _EXPR.finditer(s["run"])
]
assert not offenders, (
f"job '{job}' of docker-build.yml has an expression delimiter inside a run: body — "
f"{offenders}. This job writes a REQUIRED status context, and even in a comment a delimiter "
"is unsafe: the runner rewrites the WHOLE body into a format(...) call, and if the payload "
"does not parse it DROPS THE STEP and reports the job green — so a required check would "
"pass having done no work (ersatztv#751/#756). Pass the value in through the step's `env:` "
"block instead; to describe an expression in prose, name it rather than quoting the "
"delimiters."
)
# ANTI-VACUITY. A walk that reached no bodies, or only the trivial ones, would make the
# assertion above green while proving nothing. Counted against the job's own step list read
# here, so a helper that silently stopped yielding steps is caught rather than rewarded.
declared = sum(1 for s in _steps(job) if isinstance(s, dict) and s.get("run"))
assert len(_run_steps(job)) == declared > 5, (
f"the walk reached {len(_run_steps(job))} run: bodies but job '{job}' declares {declared}"
)
@pytest.mark.parametrize("job", REQUIRED_JOBS)
def test_every_consequential_run_step_marks_itself_as_its_FIRST_act(job):
"""The completeness half — and the only test that catches a NEWLY ADDED step with no marker.
A guard that checks a fixed list can go quietly incomplete: someone adds a `Test SPA (part 2)`
step, it is never marked, the guard never expects it, and a drop of exactly that step is
invisible again. So the expectation is DERIVED from the workflow rather than written down twice.
EXEMPT: steps carrying `continue-on-error: true`. Those are advisory by construction (the
peak-anon sampler, the coverage summary) — the workflow already declares that their failure must
not redden the job, so their non-execution cannot be a fail-open either. Making them mandatory
would be asserting the opposite of what `continue-on-error` means.
FIRST ACT, not merely present. A marker written at the END of a body records completion, not
execution — and this repo has legitimate early-exit paths. More importantly a marker further down
can be skipped by an early `exit 0` while the step did nothing, which is the fail-open again one
line lower. `set -euo pipefail` is allowed to precede it: it cannot fail, and it is what makes
the rest of the body honest.
"""
missing, late = [], []
for s in _run_steps(job):
if s.get("continue-on-error") is True or "ci-step-ran.sh assert" in s["run"]:
continue
m = _MARK.search(s["run"])
if not m:
missing.append(s.get("name", "?"))
continue
# By LINE, not by byte offset. The marker sits mid-line (the command is quoted and
# prefixed with $GITHUB_WORKSPACE), so slicing at `m.start()` counts the marker's OWN line
# prefix as a preceding command and reddens every correctly-written step.
lines = s["run"].splitlines()
at = next(i for i, ln in enumerate(lines) if _MARK.search(ln))
preceding = [
ln.strip() for ln in lines[:at]
if ln.strip() and not ln.strip().startswith("#")
]
if [ln for ln in preceding if not ln.startswith("set -")]:
late.append((s.get("name", "?"), preceding))
assert not missing, (
f"these run: steps of the REQUIRED job '{job}' do not record that they executed: {missing}. "
"A step the runner drops concludes success, so without a marker its non-execution takes the "
"whole required context green having done no work (ersatztv#756). Add "
'`\"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh\" mark <key>` as the step\'s first line and '
"the key to the guard step's --always/--gated list."
)
assert not late, (
f"these steps of '{job}' mark themselves only after other commands have run: {late}. The "
"marker must be the first act, or a body that exits early records nothing while the guard "
"still expects it — or worse, records success for work that did not happen."
)
@pytest.mark.parametrize("job", REQUIRED_JOBS)
def test_the_guard_expects_EXACTLY_the_set_of_marked_keys_in_the_right_bucket(job):
"""Set equality in BOTH directions, plus the bucket, because each failure is silent differently.
A key marked but not expected → the guard never notices that step being dropped: a fail-open
that looks fully guarded. A key expected but not marked → the guard reddens on every single run,
which is fail-closed but reads as "this guard is broken" and is how a correct guard gets deleted.
The BUCKET has to match the step's own `if:`. A gated step listed under `--always` reddens every
docs-only and already-validated run — the two paths whose entire purpose is to report green in
seconds. An always-run step listed under `--gated` stops being checked the moment either skip
gate fires, which is a fail-open on precisely the runs where least else is happening.
"""
marked = _marked(job)
keys = [k for _, k in marked]
assert len(keys) == len(set(keys)), (
f"job '{job}' reuses a marker key: {[k for k in keys if keys.count(k) > 1]}. Two steps "
"sharing a key means either one satisfies the guard for both, so dropping one is invisible."
)
always, gated = _guard_buckets(job)
assert sorted(always + gated) == sorted(keys), (
f"job '{job}': the guard expects {sorted(always + gated)} but the steps mark "
f"{sorted(keys)}. Keys marked-but-unexpected are unguarded drops; keys "
"expected-but-unmarked redden every run."
)
for step, key in marked:
want = "gated" if _is_gated(step) else "always"
got = "gated" if key in gated else "always"
assert want == got, (
f"job '{job}': step {step.get('name')!r} is {want} (if: {step.get('if')!r}) but the "
f"guard lists its key {key!r} under --{got}."
)
@pytest.mark.parametrize("job", REQUIRED_JOBS)
def test_the_guard_is_the_LAST_step_carries_no_if_and_is_not_advisory(job):
"""Position and condition, which together are what make the guard reachable and quiet.
LAST, because a guard placed before a marked step reads a marker not yet written and fails on
every run.
NO `if:` — a deliberate departure from the #751 guard's `if: always()`, and the thing most likely
to be "corrected" back. That job has one real step, so `always()` costs nothing. These jobs have
a dozen, and a genuine failure in an early one SKIPS every later step: an `always()` guard would
then report "these steps never executed: typecheck web-test build dotnet-test" on top of every
ordinary red build. That is the runner obeying its own gating, not a dropped step, and a guard
that cries wolf on every red build gets deleted.
The default `if:` is `success()`, and the invariant that makes relying on it safe rather than
lucky: this step is skipped only when an earlier step FAILED, and that failure already fails the
job. So `guard skipped => job red`, and every path to a green job runs the guard. A dropped step
is invisible precisely because it concludes `success` — which keeps the job green and therefore
reaches here.
NOT `continue-on-error`, which would let it observe the failure and go green anyway — the whole
defect, one attribute over.
"""
steps = _steps(job)
guard = _guard(job)
assert steps[-1] is guard, (
f"the dropped-step guard is not the last step of '{job}' — it is at index "
f"{steps.index(guard)} of {len(steps)}, so any marked step after it would be unguarded and "
"the guard would read a marker that has not been written yet."
)
assert "if" not in guard, (
f"the '{job}' guard carries `if: {guard.get('if')!r}`. It must have none: the default "
"`success()` is what keeps it silent on ordinary red builds, and `always()` would make it "
"announce a false 'these steps never executed' on every failing run. See the comment above "
"the step for why this is a deliberate departure from the #751 guard."
)
assert guard.get("continue-on-error") is not True, (
f"the '{job}' guard is continue-on-error, so it detects the dropped step and lets the job go "
"green regardless — which is the defect it exists to remove."
)
@pytest.mark.parametrize("job", REQUIRED_JOBS)
def test_the_guards_OWN_body_cannot_be_dropped_by_the_mechanism_it_guards_against(job):
"""A guard the guarded mechanism can silently delete is worse than no guard.
Its absence is silent too: the job simply goes green with nothing checked, which is
indistinguishable from a clean run. #751 states the rule; here it is stronger than there,
because the body is a single command with no delimiter possible rather than 20 lines of prose
that must be kept clean by hand.
The gate VALUES arrive through `env:`, which the runner interpolates per value — a bad payload
there fails that value, not the body. Both are additionally held to naming a real context by
test_every_workflow_expression_names_a_REAL_context_or_function in test_pr_changed_files.py.
"""
guard = _guard(job)
assert not _EXPR.search(guard["run"]), (
f"the '{job}' guard's own run body contains an expression delimiter, so the runner can drop "
"the guard the same way it drops the steps the guard is watching — and that absence is "
"silent as well."
)
assert guard["run"].strip().startswith("scripts/ci-step-ran.sh assert"), (
f"the '{job}' guard is no longer a bare invocation: {guard['run']!r}. Keeping it to one "
"command is what makes a delimiter impossible rather than merely absent."
)
assert set(guard.get("env", {})) == {"ETV_DOCS_ONLY", "ETV_REVALIDATE_SKIP"}, (
f"the '{job}' guard's env: is {guard.get('env')} — it must pass exactly the two skip-gate "
"values the script reads, or the gated bucket is evaluated against stale/absent state."
)
# ------------------------------------------------------------------------------------------------
# BEHAVIOURAL — the guard's real command line, against markers written by the steps' real lines
# ------------------------------------------------------------------------------------------------
def _mark_line(step) -> str:
"""The step's OWN marker line, verbatim from the workflow.
Extracted rather than rebuilt in Python ON PURPOSE. A test that composed the command itself
would keep passing after the workflow and the script drifted apart on the path, the quoting or
the sub-command — and that divergence is exactly the failure that makes the guard fail on every
run and then get deleted as broken. Running the real line proves the two agree by construction.
"""
line = next(ln for ln in step["run"].splitlines() if _MARK.search(ln))
return line.strip()
def _env(tmp_path, **extra):
env = {
"PATH": os.environ["PATH"],
"GITHUB_WORKSPACE": str(REPO_ROOT),
"RUNNER_TEMP": str(tmp_path),
"GITHUB_JOB": "test",
"GITHUB_RUN_ID": "424242",
"GITHUB_RUN_ATTEMPT": "7",
}
env.update(extra)
return env
def _run(script: str, env):
return subprocess.run(["bash", "-c", script], cwd=REPO_ROOT, env=env,
capture_output=True, text=True)
@pytest.mark.parametrize("job", REQUIRED_JOBS)
def test_the_guard_PASSES_when_every_step_marked_itself(job, tmp_path):
"""The positive control. Without it, a guard that always failed would satisfy every case below.
`GITHUB_JOB` is set to the job under test, so this also covers the marker file being keyed per
job: if it were not, the two jobs would share a file and one job's markers would answer for the
other's dropped steps.
"""
marks = [_mark_line(s) for s, _ in _marked(job)]
guard = _guard(job)["run"]
env = _env(tmp_path, GITHUB_JOB=job)
r = _run("\n".join(["set -e", *marks, guard]), env)
assert r.returncode == 0, (
f"the '{job}' guard rejected a run in which every step marked itself — the steps and the "
f"guard disagree, so this would fail on every run.\n{r.stdout}\n{r.stderr}"
)
assert "All" in r.stdout and "executed" in r.stdout, r.stdout
@pytest.mark.parametrize("job", REQUIRED_JOBS)
def test_dropping_ANY_single_step_FAILS_the_guard(job, tmp_path):
"""Every marked step, one at a time — not a sample.
An arbitrary sample gives false negatives here: the interesting drop is `Test` or the migration
replay, and a test that only omitted the first step would prove the guard catches the one case
that was never fail-open anyway. Dropping each key in turn is the only version that establishes
the property the issue asks for.
"""
marked = _marked(job)
guard = _guard(job)["run"]
for dropped_step, dropped_key in marked:
d = tmp_path / dropped_key
d.mkdir()
marks = [_mark_line(s) for s, k in marked if k != dropped_key]
r = _run("\n".join(["set -e", *marks, guard]), _env(d, GITHUB_JOB=job))
assert r.returncode != 0, (
f"job '{job}': the guard went GREEN with {dropped_step.get('name')!r} "
f"(key {dropped_key!r}) never having executed. That is a REQUIRED context reporting "
f"success having skipped that work — the exact fail-open of ersatztv#756.\n{r.stdout}"
)
assert dropped_key in (r.stdout + r.stderr), (
f"the guard failed but did not name the missing step {dropped_key!r}: {r.stdout}"
)
@pytest.mark.parametrize("gate", ["ETV_DOCS_ONLY", "ETV_REVALIDATE_SKIP"])
def test_a_fired_skip_gate_does_not_require_the_gated_steps(gate, tmp_path):
"""The docs-only and already-validated paths must still report green in seconds.
They are the reason these jobs are never `if:`-skipped at the JOB level (a skipped required
context is a state this repo deliberately does not rely on — ersatztv#416/#418), so a guard that
reddened them would make every docs-only PR unmergeable. Which is #751's user-visible symptom
arriving from the opposite direction, and worth a test rather than a comment.
"""
job = "test"
marks = [_mark_line(s) for s, k in _marked(job) if k in _guard_buckets(job)[0]]
guard = _guard(job)["run"]
r = _run("\n".join(["set -e", *marks, guard]), _env(tmp_path, GITHUB_JOB=job, **{gate: "true"}))
assert r.returncode == 0, (
f"with {gate}=true the guard still demanded the gated steps, so every docs-only / "
f"already-validated run of a REQUIRED job would be red.\n{r.stdout}\n{r.stderr}"
)
assert "Skip gate fired" in r.stdout, r.stdout
def test_a_fired_skip_gate_STILL_requires_the_ALWAYS_steps(tmp_path):
"""The negative control for the test above — otherwise `ETV_DOCS_ONLY=true` would be a blanket
off-switch and the previous test would be passing for the wrong reason.
This is the case that matters most on a docs-only run: the detect steps are the only things that
execute, so if their drop were unguarded the skip path would be entirely unchecked.
"""
guard = _guard("test")["run"]
r = _run("\n".join(["set -e", guard]), _env(tmp_path, ETV_DOCS_ONLY="true"))
assert r.returncode != 0, (
"with ETV_DOCS_ONLY=true and NO steps marked at all, the guard passed — the skip gate is "
"acting as a blanket off-switch rather than as a narrowing of what is expected."
)
assert "detect" in (r.stdout + r.stderr), r.stdout
def test_an_EMPTY_gate_value_requires_the_gated_steps(tmp_path):
"""A dropped `detect` step leaves its outputs EMPTY, not 'false'.
Reading empty as "skipped" would mean the one drop that disables the detect step also disables
the guard for everything downstream — the guard switching itself off in response to the very
failure it exists to catch. The direction has to be: anything that is not exactly `true` widens
what is required.
"""
guard = _guard("test")["run"]
r = _run("\n".join(["set -e", guard]), _env(tmp_path, ETV_DOCS_ONLY="", ETV_REVALIDATE_SKIP=""))
assert r.returncode != 0
assert "dotnet-test" in (r.stdout + r.stderr), (
f"empty gate values were read as a skip, so the gated steps went unchecked: {r.stdout}"
)
def test_a_STALE_marker_from_another_run_cannot_satisfy_the_guard(tmp_path):
"""RUNNER_TEMP is `/tmp` on these runners, not a private per-job directory (measured in #751).
So a fixed marker name would live where another run can also write it, and a leftover file would
satisfy the guard on a run whose step was dropped — a silent PASS, the exact failure mode being
removed. The key is run id + attempt + job.
"""
marks = [_mark_line(s) for s, _ in _marked("test")]
guard = _guard("test")["run"]
# Run 1 marks everything.
first = _env(tmp_path, GITHUB_JOB="test", GITHUB_RUN_ID="111", GITHUB_RUN_ATTEMPT="1")
assert _run("\n".join(["set -e", *marks]), first).returncode == 0
# Run 2 shares RUNNER_TEMP but marks nothing. It must NOT inherit run 1's markers.
second = _env(tmp_path, GITHUB_JOB="test", GITHUB_RUN_ID="222", GITHUB_RUN_ATTEMPT="1")
r = _run(guard, second)
assert r.returncode != 0, (
"a marker file left by a DIFFERENT run satisfied the guard, so a run whose steps were all "
f"dropped would pass silently.\n{r.stdout}"
)
# ...and a RETRY of run 1 must not inherit run 1's either.
retry = _env(tmp_path, GITHUB_JOB="test", GITHUB_RUN_ID="111", GITHUB_RUN_ATTEMPT="2")
assert _run(guard, retry).returncode != 0, (
"a re-run inherited the first attempt's markers, so a step dropped only on the retry passes"
)
# ...nor may the OTHER job in the same run inherit them.
sibling = _env(tmp_path, GITHUB_JOB="migrations", GITHUB_RUN_ID="111", GITHUB_RUN_ATTEMPT="1")
assert _run(_guard("migrations")["run"], sibling).returncode != 0, (
"the two required jobs share one marker file, so one job's markers answer for the other's "
"dropped steps"
)
def test_assert_with_no_expected_keys_REFUSES_instead_of_passing(tmp_path):
"""The script's own anti-vacuity check, exercised rather than trusted.
`assert` with an empty expectation list would print "All 0 expected step(s) executed" and exit 0
— a guard that proves nothing while reporting that it proved everything. That is how a guard
ends up shipped and dead, which this repo has now done twice (#751's fence, #751's own guard).
"""
r = _run(f"{SCRIPT} assert", _env(tmp_path))
assert r.returncode == 2, f"expected a usage refusal, got {r.returncode}: {r.stdout} {r.stderr}"
assert "no expected keys" in (r.stdout + r.stderr)
def test_mark_APPENDS_so_one_step_does_not_erase_its_predecessors(tmp_path):
"""`>` instead of `>>` in the script would leave only the last step's key.
The guard would then redden on every run — fail-closed, but it would look like the guard is
broken rather than like a real drop, and that is the state in which a correct guard gets removed.
"""
env = _env(tmp_path)
assert _run(f"{SCRIPT} mark alpha && {SCRIPT} mark beta", env).returncode == 0
r = _run(f"{SCRIPT} assert --always alpha beta", env)
assert r.returncode == 0, f"the second mark erased the first: {r.stdout} {r.stderr}"
def test_a_key_is_matched_WHOLE_not_as_a_substring(tmp_path):
"""`build` must not be satisfied by `web-build`, and `test` not by `web-test`.
Both pairs are live key names in the `test` job, so a substring match would mean dropping the
real `Build` or `Test` step — the two most consequential steps in the whole workflow — is
invisible because an SPA step of a similar name ran.
"""
env = _env(tmp_path)
assert _run(f"{SCRIPT} mark web-build && {SCRIPT} mark web-test", env).returncode == 0
r = _run(f"{SCRIPT} assert --always build", env)
assert r.returncode != 0, (
"the key 'build' was satisfied by a marker for 'web-build' — a dropped `dotnet build` would "
"pass unnoticed"
)