timothy/ersatztv
1
0
Fork 0
Code Issues 55 Pull Requests 6 Actions 2 Packages Projects Releases 9 Wiki Activity

ci(631): run scripts/tests in CI as its own job, not inside the flake-covered decisions-guard

Browse Source 
Nothing executed scripts/tests/. No workflow and no Husky hook invoked pytest:
decisions-guard runs decisions_validate.py and build_decisions_catalog.py directly
(the code, never its tests) and the `test` job is dotnet test only. The 111 tests
guarding the decision corpus, the #610 migration-equivalence harness, the
merge-consent exemption logic and the #622 review-verdict poster were enforced only
by whoever remembered to run pytest locally.

Adds a `script-tests` job to pr-checks.yml rather than the step inside decisions-guard
the issue proposed. decisions-guard is covered by ci.decisions-lifecycle-flake, a
standing instruction that a lone `decisions lifecycle` red is a known flake and must
not be investigated — so a genuine pytest regression there would surface as exactly
the red every session is told to wave through. A gate inherits the credibility of the
job it lives in.

Runs unconditionally: the suite executes the real post-review-verdict.sh and
pretooluse-merge-consent.sh, so a `scripts/**` path filter would miss a
.claude/hooks/** edit. ~10s, pure stdlib + pytest.

Negative control verified locally (broken test -> exit 1); CI-red measurement follows
on a scratch PR per the Done-when.

Refs #631
This commit is contained in:
timothy
2026-07-26 13:24:19 +02:00
parent eb339084f9
commit 69d8d3ccfe
4 changed files with 137 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.gitea/workflows/pr-checks.yml
+44 -3
View File
@@ -3,7 +3,8 @@ name: PR Gates
# Fast, git-only PR gates split out of docker-build.yml into a dedicated `on: pull_request`
# workflow (ersatztv#535) so they are NEVER created on a tag/main push.
#
# WHY THIS FILE EXISTS. These three checks are pure `checkout + git diff` gates: they carry no
# WHY THIS FILE EXISTS. These checks are cheap `checkout + git diff` (or, for `script-tests`,
# checkout + pytest) gates: they carry no
# `container:`, run on the `small` lane (git-only, 1 GiB; server-management#639), and are PR-only.
# While they lived in docker-build.yml — which also triggers on push to main and on `v*` tags —
# Gitea still DISPATCHED them as runner tasks on every such push to evaluate the `if:` skip, because
@@ -22,8 +23,9 @@ name: PR Gates
#
# These stay on `runs-on: small` and carry NO CI toolchain image pin, so `ci-image-pin`'s grep of
# docker-build.yml still validates the five pin-bearing jobs (test/migrations/functional-e2e/
# api-docs/format) that remain there. None of these three are required checks — branch protection
# requires only `Build & test (.NET)` and `EF migration integrity` — so relocating them (which
# api-docs/format) that remain there. None of these jobs are required checks — branch protection
# requires only `Build & test (.NET)`, `EF migration integrity` and `review-verdict/h10` — so
# relocating them (which
# changes their status-context prefix from "Build ErsatzTV Image / …" to "PR Gates / …") does not
# affect merges. See docs/ci-cd.md -> "PR gates workflow".
@@ -187,3 +189,42 @@ jobs:
run: PYTHONPATH=. python3 scripts/build_decisions_catalog.py --check
- name: Kickoff guard
run: bash scripts/check-kickoff-guard.sh
# FAILS THE RUN on a red (ersatztv#631) — like its sibling gates here it is not (yet) a required
# status check, so it reddens the PR without hard-blocking the merge button; see the header.
# Runs scripts/tests/ — the pytest suite covering the decision-corpus
# parser/validator/catalog builder, the #610 migration-equivalence harness, the merge-consent
# exemption logic and the #622 review-verdict poster. Until #631 NOTHING executed these: no
# workflow and no Husky hook invoked pytest, so the suite guarding our merge-gating machinery was
# local-only and a regression in it was caught only by luck. `decisions-guard` above runs that
# code, but never its tests.
#
# WHY ITS OWN JOB rather than a step inside decisions-guard (which the issue proposed as the
# cheapest home): `ci.decisions-lifecycle-flake` is a STANDING instruction that a lone
# `decisions lifecycle` red is a known infra flake to be ignored — "do not investigate". Folding
# the suite into that job would make a genuine pytest regression present as exactly the red every
# session is told to wave through, which is the same silently-green failure mode #631 exists to
# close. A distinct job name keeps a real failure unambiguous.
#
# Runs UNCONDITIONALLY on every PR rather than behind a `scripts/**` path filter. The suite's
# corpus tests are fixture/tmp-repo based, but test_post_review_verdict.py and
# test_merge_consent_exemption.py execute the REAL `scripts/post-review-verdict.sh` and
# `.claude/hooks/pretooluse-merge-consent.sh`, so its true input set spans at least two top-level
# directories. A `scripts/**` filter would silently miss a `.claude/hooks/**` edit — and at ~10s a
# filter buys nothing but drift. Pure-stdlib + pytest (the decisions read path is deliberately
# dependency-free — no PyYAML), so the only install is pytest itself.
script-tests:
name: Script tests (pytest)
runs-on: small
if: github.event_name == 'pull_request'
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.x'
- name: Install pytest
run: python3 -m pip install --disable-pip-version-check --quiet pytest
- name: Run scripts/tests
run: PYTHONPATH=. python3 -m pytest scripts/tests -q
docs/ci-cd.md
+44 -6
View File
@@ -149,8 +149,11 @@ rather than in `docker-build.yml` — see that section (ersatztv#535).
**`small` is git-only, and that is load-bearing (server-management#639).** Everything in
the lane is a checkout plus a `git diff`: `decisions-guard`, `ci-image-pin`,
`docs-reminder`. Nothing there runs a compiler or a `docker build`, which is why the lane
can be capped at 1 GiB per job. Route a heavy job here and it will OOM — give it
`docs-reminder` — plus `script-tests`, which is a checkout plus a pure-stdlib `pytest` run
(ersatztv#631). Nothing there runs a compiler or a `docker build`, which is why the lane
can be capped at 1 GiB per job. The lightweight-Python jobs are the deliberate edge of the
"git-only" rule, not an exception to it: `setup-python` + `pip install pytest` + a suite whose
heaviest allocation is a handful of temp-dir git repos stays far under the cap. Route a heavy job here and it will OOM — give it
`ubuntu-latest`, or its own label on `ci-runner`, the only host with no prod workload.
**Lane assignment (ersatztv#390).** *Slot counts below are as-of 2026-07-17; the table above is
@@ -507,8 +510,8 @@ expensive 787-migration replay is skipped; the service is capped and idle for se
`api-docs` and `format` already short-circuit on docs-only changes via their own path detection (no
API path / no `.cs` changed → they pass in ~5s), so they needed no change. `docs-reminder`,
`decisions-guard` and `ci-image-pin` keep running on docs-only changes — the first two are *about*
docs and must.
`decisions-guard`, `ci-image-pin` and `script-tests` keep running on docs-only changes — the first
two are *about* docs and must, and `script-tests` is unconditional by design (ersatztv#631).
Not in scope: the within-run triple `dotnet build` (ersatztv#398; measured and rejected as
build-once — see `docs/decisions.md`). The separate redundancy of running the **whole matrix on a
@@ -598,12 +601,47 @@ compiler/docker build), so it doesn't violate the "small is git-only" lane rule.
`docs-reminder`, otherwise a seconds-long `git diff` + parse with no dotnet/node setup
(`runs-on: small`).
### `script-tests` job (`Script tests (pytest)`, PR-only — in `pr-checks.yml`)
> Reddens the run on failure, but like the other `pr-checks.yml` gates it is **not** one of the
> three required status checks on `main` (`Build & test (.NET)`, `EF migration integrity`,
> `review-verdict/h10`). Promoting it to required is a branch-protection change, tracked separately.
Runs the repository's Python test suite: `PYTHONPATH=. python3 -m pytest scripts/tests -q`
(111 tests, ~10s). It covers the decision-corpus parser/validator/catalog builder, the ersatztv#610
migration-equivalence harness, the merge-consent exemption logic and the ersatztv#622 review-verdict
poster.
**Until ersatztv#631, nothing ran these tests.** No workflow and no Husky hook invoked `pytest`.
`decisions-guard` executes `decisions_validate.py` and `build_decisions_catalog.py` directly — it
exercises that *code* but never its *tests* — and the `test` job is `dotnet test` only. The suite
guarding our merge-gating machinery was therefore local-only, and a test added "for CI enforcement"
was decorative.
**Why it is its own job, not a step inside `decisions-guard`.** `decisions-guard` is covered by the
standing `ci.decisions-lifecycle-flake` rule: a lone `decisions lifecycle` red is a known infra
flake and sessions are instructed *not to investigate it*. Adding the suite there would make a
genuine pytest regression surface as precisely the red everyone is told to wave through — the same
"reports success while doing nothing" failure mode ersatztv#631 exists to close. A distinct job
name keeps a real failure unambiguous.
**Why it runs unconditionally** rather than behind a `scripts/**` path filter: the suite's true
input set spans more than one directory — `test_post_review_verdict.py` and
`test_merge_consent_exemption.py` execute the real `scripts/post-review-verdict.sh` and
`.claude/hooks/pretooluse-merge-consent.sh` — so a `scripts/**` filter would silently miss a
`.claude/hooks/**` edit. At ~10s, a filter buys nothing but drift. The suite is pure stdlib plus
`pytest` (the decisions read path is deliberately dependency-free — no PyYAML), so the only install
step is `pip install pytest`; like the other `small`-lane Python jobs it adds
`actions/setup-python@v5` first. Checkout is at default depth: every `git` call in the suite runs
against a temp repo it creates itself, never this repository's history.
## PR gates workflow
**File:** `.gitea/workflows/pr-checks.yml` — `on: pull_request` only.
The three git-only PR gates — `ci-image-pin`, `docs-reminder`, `decisions-guard` (described above)
— live here, **not** in `docker-build.yml`, and that separation is the fix for **ersatztv#535**.
The four git-only PR gates — `ci-image-pin`, `docs-reminder`, `decisions-guard`, `script-tests`
(all described above) — live here, **not** in `docker-build.yml`, and that separation is the fix
for **ersatztv#535**.
**Why they are split out.** All three are pure `checkout + git diff` gates on the `small` lane
(no `container:`) and are PR-only (`if: github.event_name == 'pull_request'`). While they lived in
docs/decisions/README.md
+1
View File
@@ -51,6 +51,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
| `ci.peak-anon-measurement` | The `test` job's headline memory figure is a sampled high-water mark of cgroup `anon`, produced by `scripts/ci-peak-anon.sh`; `memory.peak` and the end-of-job `anon`/`file` split are kept only as a cache-inflated reference. | 2026-07-19 | [link](records/ci/peak-anon-measurement.md) |
| `ci.root-screenshot-guard` | The Husky `pre-commit` hook refuses a staged root-level `*.png` (belt-and-suspenders with the `.gitignore` rule); nested `*.png` real assets are unaffected. | 2026-07-12 | [link](records/ci/root-screenshot-guard.md) |
| `ci.runner-placement` | No persistent Roslyn compiler server survives a CI build (`UseSharedCompilation=false` etc., runner env + Dockerfile `ENV`); every `services:` container gets its own explicit `--memory`/`--memory-swap`/`--cpus` cap (it does not inherit the job container's). | 2026-07-17 | [link](records/ci/runner-placement.md) |
| `ci.script-tests-job` | The `scripts/tests/` pytest suite runs on every PR as a dedicated `script-tests` job in `pr-checks.yml` (`runs-on: small`, `setup-python` + `pip install pytest`, `PYTHONPATH=. python3 -m pytest scripts/tests -q`), unconditionally rather than behind a `scripts/**` path filter, and **never as a step inside `decisions-guard`** — a job whose reds a standing rule instructs sessions to ignore must never host a gate whose reds are real. Any new CI gate must be reachable by a failure that is unambiguously attributable to it. | 2026-07-26 | [link](records/ci/script-tests-job.md) |
| `ci.small-lane-git-only` | `runs-on: small` is defined by what a job does (git-only), not its usual runtime; the two `docker build` jobs (docker-build.yml, ci-image.yml) move to `ubuntu-latest` because their worst-case memory, not median runtime, was pinning the small lane's per-slot cap. | 2026-07-20 | [link](records/ci/small-lane-git-only.md) |
| `ci.ui-e2e-harness` | The UI-interactive E2E flows run as headless Playwright specs (`web/e2e/*.spec.ts`, driven by `scripts/e2e-ui.sh`) in a **second step of the existing advisory `functional-e2e` job**, never their own job; the browser is `chromium-headless-shell` **baked into the CI toolchain image** (`docker/ci/Dockerfile`, `PLAYWRIGHT_VERSION` kept equal to `web/package.json`'s EXACT `@playwright/test` pin), never installed per run; specs are `serial` with `retries: 0` and assert only contracts the curl harness structurally cannot reach. | 2026-07-25 | [link](records/ci/ui-e2e-harness.md) |
| `ci.verify-locally-ci-confirms` | Treat the local build/verify/review pass as the decision point and CI as confirmation — don't idle waiting on a run you have no reason to doubt. | 2026-07-21 | [link](records/ci/verify-locally-ci-confirms.md) |
docs/decisions/records/ci/script-tests-job.md
+48
View File
@@ -0,0 +1,48 @@
---
key: ci.script-tests-job
title: '2026-07-26 — `scripts/tests/` runs in CI as its own `script-tests` job, never inside the flake-covered `decisions-guard` (#631)'
status: active
since: '2026-07-26'
supersedes: none
superseded-by: none
rule: 'The `scripts/tests/` pytest suite runs on every PR as a dedicated `script-tests` job in `pr-checks.yml` (`runs-on: small`, `setup-python` + `pip install pytest`, `PYTHONPATH=. python3 -m pytest scripts/tests -q`), unconditionally rather than behind a `scripts/**` path filter, and **never as a step inside `decisions-guard`** — a job whose reds a standing rule instructs sessions to ignore must never host a gate whose reds are real. Any new CI gate must be reachable by a failure that is unambiguously attributable to it.'
signals: 'scripts/tests never ran in CI, pytest not in any workflow, python test suite local-only, decorative test, decisions-guard runs the code not the tests, script-tests job, small lane pytest, negative control CI goes red · paths: `.gitea/workflows/pr-checks.yml`, `scripts/tests/`, `docs/ci-cd.md` · issues: #631, #610, #621, #622, #542'
mechanics: '`.gitea/workflows/pr-checks.yml` -> `script-tests`; `docs/ci-cd.md` -> "`script-tests` job"'
---
Until #631 **nothing executed `scripts/tests/`**. No workflow and no Husky hook invoked `pytest`.
`decisions-guard` runs `decisions_validate.py` and `build_decisions_catalog.py` directly — it
exercises that *code* but never its *tests* — and the `test` job is `dotnet test` only. So the 111
tests guarding the decision corpus, the #610 migration-equivalence harness, the merge-consent
exemption logic and the #622 review-verdict poster were caught only if someone happened to run
pytest locally. #622's suite was very nearly shipped in the belief that it was enforced.
**The durable part is where the job does NOT go.** The obvious home — a step inside `decisions-guard`,
which already has `setup-python` and the right lane — is the one place it must not live.
`ci.decisions-lifecycle-flake` is a standing instruction that a lone `decisions lifecycle` red is a
known infra flake and must **not** be investigated. Folding the suite in there would make a genuine
pytest regression present as precisely the red every session is told to wave through: the gate would
be enforced on paper and inert in practice, which is the same "reports success while doing nothing"
family as the #603 `stale-after` field that never fired, the #609 marker that printed `OK` while
doing nothing, and the #621 record that vanishes silently. A gate inherits the credibility of the job
it lives in, so **a job under a standing ignore-rule can host no real gate.**
This does not conflict with `ci.ui-e2e-harness` ("never their own job"). That record folds UI-E2E
into `functional-e2e` because the specs need an app the job has *already booted* — sharing expensive
setup. Here there is no shared setup to reuse (pytest needs only a checkout and `pip install pytest`),
and the sibling job carries an ignore-rule. Same question, opposite answers, for stated reasons.
**Unconditional, not path-filtered.** The suite's real input set spans more than `scripts/`:
`test_post_review_verdict.py` and `test_merge_consent_exemption.py` execute the actual
`scripts/post-review-verdict.sh` and `.claude/hooks/pretooluse-merge-consent.sh`. A `scripts/**`
filter would silently miss a `.claude/hooks/**` edit — and at ~10s a filter buys nothing but a
drift surface. Default checkout depth is sufficient: every `git` call in the suite runs against a
temp repo it creates itself, never this repository's history.
**Verified by measurement, not a green tick** (the #631 Done-when, and `ci.docs-only-detect-shallow-safe`'s
lesson): a deliberately-failing test was confirmed to exit non-zero locally and to turn the CI job
red on a scratch PR, before the passing state was accepted as meaningful.
It is not yet a *required* status check — `main` requires only `Build & test (.NET)`,
`EF migration integrity` and `review-verdict/h10`. It reddens the run; promoting it to required is a
branch-protection change left deliberately separate.