fix(887): the image build builds the SPA and does not test it (#899)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 20s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m24s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m14s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m5s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m1s
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 20s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m24s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m14s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m5s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m1s
`docker/Dockerfile`'s web-build stage ran the SPA vitest suite with two hand-written `--exclude`d spec paths. The stage is gitless twice over, and the suite has members needing a git checkout or the git binary, so that list was a population nothing derives. #883 added a third member without updating it; because `Build & push image (amd64)` is `if: github.event_name != 'pull_request'`, the red was unreachable on a PR and landed on `main` and the `v*` tag path. Every image build has failed since. The list is removed rather than extended: the stage builds the SPA and does not test it, and the suite runs once, unfiltered, in the `test` job that `build` already `needs:`. `scripts/tests/test_image_build_delegates_the_spa_suite.py` holds the invariant in three parts, because the first two together still certify a publish on which the suite never ran. It PINS command text rather than parsing it: three earlier versions asked what a command MEANS and were wrong nine times, and a partial match of `web/vite.config.ts` was then defeated seven more ways, so both mechanisms were withdrawn rather than respelled. The transferable rule, recorded in the guard and the decision record: a pin assumes it is pinning the artifact that still DECIDES. Every route found was authority moving where the pin was not looking — another file, another occurrence, another workflow, or a hook the pinned command invokes. Nine independent cold-review rounds, eight BLOCKED. 73-mutant development battery, 0 missed; one declared clause mutation harness-executed per suite. fixes #887
This commit was merged in pull request #899.
This commit is contained in:
+35
-31
@@ -9,37 +9,41 @@ WORKDIR /source
|
||||
COPY design-system/. ./design-system/
|
||||
COPY web/. ./web/
|
||||
WORKDIR /source/web
|
||||
# The SPA suite runs here except for two files (ersatztv#819), excluded for OVERLAPPING reasons —
|
||||
# one needs the git binary, the other needs the binary AND a checkout. Reading them as two separate
|
||||
# reasons is what broke this stage once already:
|
||||
# * `web/src/api/pageSizeCallSites.guard.test.ts` needs a git CHECKOUT — and, through it, the
|
||||
# binary. It derives its file population from `git ls-files` rather than a directory walk, and
|
||||
# refuses rather than falling back. This stage's context is `web/` + `design-system/` only, so
|
||||
# there is no `.git`.
|
||||
# * `web/vite-plugins/trackedSourceFiles.realgit.test.ts` needs the BINARY but no checkout: it
|
||||
# builds its own temp repository to prove the derivation by executing it.
|
||||
# `node:22-bookworm-slim` ships no git (`command -v git` -> not found), so it dies with
|
||||
# `spawnSync git ENOENT`.
|
||||
# So this is NOT checkout-versus-binary, and adding a `COPY .git` would not let either run here —
|
||||
# the binary would still be missing. Excluding only the first is not enough either, and a replica
|
||||
# that merely deletes `.git` cannot show that: verify any change here with the git binary off
|
||||
# `PATH`, not just with the directory absent.
|
||||
# Everything else — all but those two files — runs fine gitless and is kept, rather than dropping
|
||||
# the whole suite for one file as an earlier fix here did.
|
||||
# The excluded pair is not skipped overall: `docker-build.yml`'s `Build & test (.NET)` job runs the
|
||||
# whole suite on a real checkout, and `build` (the job that invokes this Dockerfile) carries
|
||||
# `needs: [test, migrations, scan]`. State that chain precisely, because the `needs:` edge is not
|
||||
# all of it: `Test SPA` is also gated on `docs_only` and on the #420 revalidate skip, and `build` is
|
||||
# not gated on `revalidate`. On a push whose tree is byte-identical to an already-green head the
|
||||
# suite is skipped and the image still builds — carried there by #420's byte-identical-tree
|
||||
# argument. The `docs_only` arm cannot ship an image at all (`Build and push` is gated on it too),
|
||||
# and `ci-detect-docs-only.sh` classifies by PATH SHAPE (`docs/` or `*.md`), not by directory, so a
|
||||
# `web/*.md` would count as docs — there are none today, but do not restate it as "any `web/**`".
|
||||
RUN npm run lint && npm run typecheck && \
|
||||
npm test -- --run \
|
||||
--exclude 'src/api/pageSizeCallSites.guard.test.ts' \
|
||||
--exclude 'vite-plugins/trackedSourceFiles.realgit.test.ts' && \
|
||||
npm run build
|
||||
# THE VITEST SUITE IS NOT RUN IN THIS STAGE, AND MUST NOT BE RE-ADDED (ersatztv#887).
|
||||
# This stage is gitless in both senses at once. It copies only `web/` and `design-system/`, so it
|
||||
# holds no `.git` — a property of the STAGE, not of the context, which is the repository root
|
||||
# (`context: .`) and whose `.dockerignore` does not exclude `.git`: the directory is there to be
|
||||
# copied and simply is not. And `node:22-bookworm-slim` ships no git binary (`command -v git` -> not
|
||||
# found), which is why a `COPY .git` would not help either. Members of the SPA suite need one or the
|
||||
# other: `pageSizeCallSites.guard.test.ts` and `completeAnnotations.guard.test.ts` derive their file
|
||||
# population from `git ls-files` and refuse to fall back to a directory walk
|
||||
# (`testing.guard-derives-population-from-source`), and `trackedSourceFiles.realgit.test.ts` builds
|
||||
# its own temp repository. (Not every `*.guard.test.ts` — `completeRequest.guard.test.ts` needs no
|
||||
# git at all.)
|
||||
#
|
||||
# Running the suite here anyway therefore costs a hand-maintained list of the members that cannot
|
||||
# run — and that list is a population nothing derives. It went stale the first time a guard was
|
||||
# added without updating it, and because `Build & push image (amd64)` is `if: github.event_name !=
|
||||
# 'pull_request'`, the resulting red is unreachable on a PR and lands on `main` and on the `v*` tag
|
||||
# path: every image build failed and `:latest` stopped being republished. Excluding one more file
|
||||
# re-arms that; the list is removed instead.
|
||||
#
|
||||
# WHAT STILL VALIDATES THE SUITE. `docker-build.yml`'s `test` job runs it UNFILTERED on a real
|
||||
# checkout, and `build` — the job that invokes this Dockerfile — carries `needs: [test, migrations,
|
||||
# scan]`, so an image cannot be published past a red suite. Two skips exist inside `test` and
|
||||
# neither leaves an image unvalidated: `docs_only` also gates `Build and push`, so that arm ships
|
||||
# nothing — and that arm IS asserted, by `test_the_DOCS_ONLY_arm_cannot_publish_an_image`. The #420
|
||||
# revalidate skip fires only on a tree byte-identical to a head that already carried a green combined
|
||||
# status; that arm is NOT asserted here — it rests on `scripts/ci-detect-already-validated.sh`, which
|
||||
# carries no mutation proof of its own, so it is a dependency rather than a check. Held by
|
||||
# `scripts/tests/test_image_build_delegates_the_spa_suite.py`. That guard PINS the command line
|
||||
# below as text: any edit to it reddens, whatever the edit says. Update the pin in the same commit,
|
||||
# and do not use it to re-add a suite run.
|
||||
#
|
||||
# `lint` and `typecheck` stay. They are gitless-safe with no member that is not, so they carry no
|
||||
# list and no trap; whether the image build should run them at all is a separate question this
|
||||
# does not answer.
|
||||
RUN npm run lint && npm run typecheck && npm run build
|
||||
|
||||
FROM --platform=linux/amd64 192.168.1.95:3000/timothy/ersatztv-ffmpeg:8.1.2 AS runtime-base
|
||||
COPY --from=dotnet-runtime /usr/share/dotnet /usr/share/dotnet
|
||||
|
||||
@@ -54,6 +54,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
|
||||
| `ci.gate-trigger-base-resolved` | The workflow that writes the branch-protection-required `review-verdict/h10` status triggers on `pull_request_target` with `branches: [main]`, never on plain `pull_request`. Gitea resolves a `pull_request` workflow DEFINITION from the PR's own head commit, so under that trigger a PR editing `.gitea/workflows/review-verdict.yml` ran its own rewritten copy and could post `h10=success` for itself; `pull_request_target` resolves the definition from the base instead. The `branches: [main]` filter is part of the rule, not a refinement of it: base resolution only relocates the rewrite from the head to the base, so without the filter a PR opened into an attacker-pushed base branch runs that branch's gate. `pull_request_target` is safe HERE only because this job never checks out or executes head-supplied code — it checks out `base.sha` and runs only that tree's scripts (`ci.shared-pr-file-enumeration`); reintroducing a head checkout under this trigger would be worse than the bug it fixed. This closes the rewrite route through THIS workflow and does NOT close the class. Gitea injects `GITEA_TOKEN` into EVERY job and it USED to be write-capable everywhere, so any ref-resolved workflow could forge `review-verdict/h10`; since #748 (2026-08-27) all six workflows here declare `permissions:` — five `code: read`, and `review-verdict.yml`'s gate job `code: write`, deliberately, because it IS the gate (the five rest on the TOP-LEVEL form, which is INFERRED rather than probed — the gate job's JOB-LEVEL form is the one measured; see `ci.actions-credential-scoping`). That narrows the INJECTED token and only that: deliberately NO "only X can post a status" summary is stated here, because any such summary generalizes from the injected token, which `permissions:` scopes, to every credential a job can reach, which it does not touch — `RENOVATE_TOKEN` is in the same secret store and is referenced by `renovate.yml`, one of the five holding `code: read`. `ci-image.yml` was the last holdout — its unfiltered `push:` made it the worst-placed of the six — and #744 closed that trigger route (`ci.toolchain-image-publish-is-a-dispatch`) while #748 scoped its token. So the route is NARROWED, not closed, and the SHAPE is open — a newly added workflow declaring write, or omitting a declaration and inheriting the owner default, is a route again, which is why `test_pr_changed_files.py` asserts every tracked workflow declares a block. And a collaborator's own API token still can, since branch protection binds the context and not its issuer. The credential half is now RESOLVED in `ci.actions-credential-scoping` (#697): CI's registry secret was the ADMIN account's basic auth and is now a PAT that cannot post a status, which removes the ADMIN escalation and that credential's route (an ALLOW-LISTED user credential's forgery carries a matching `creator` and is inherited as a verdict; an Actions job's carries `creator: null` and is re-derived, as since #742 is every account outside `H10_REVIEWERS` — but do NOT read that asymmetry as protection: re-derivation fires only on the trigger's `types`, and posting a status is not one of them, so a POST timed after the last PR event simply stands). It does not remove EVERY route: `RENOVATE_TOKEN` is a `write:repository` bot PAT in the same secret store, reachable by any PR-added workflow. The owner-level Restricted default (server-management#714) remains a separate OPEN lever — its effect on a declared `code: write` is unmeasured, and the version half of that condition has been satisfied since the 1.25.4 -> 1.27.1 upgrade on 2026-08-05, so it is not something to wait on a Gitea release for. A collaborator's own token remains unfixable; the exemption path has its own separate defects in #698. | 2026-07-28 | [link](records/ci/gate-trigger-base-resolved.md) |
|
||||
| `ci.gitea-milestone-filter-noop` | Never filter issues with the server-side `?milestones=<name>` parameter — fetch all open issues once and filter LOCALLY on each issue's `.milestone.title`. | 2026-07-21 | [link](records/ci/gitea-milestone-filter-noop.md) |
|
||||
| `ci.grep-q-pipefail-inversion` | In any script running under `set -o pipefail`, a security or classification predicate of the form `producer \| grep -q…` is FORBIDDEN: `grep -q` exits at its first match, the producer then takes SIGPIPE and exits 141 once the data exceeds the pipe buffer (~64K), so `pipefail` reports the pipeline as FAILED even though grep MATCHED — inverting the predicate exactly when the input is large. A here-string (`grep -q… <<< "$data"`) is ALSO forbidden: bash materialises a large here-string via temporary storage, so it fails when temp space is full or unwritable, and inside an `if`/`!` that failure flips the predicate the same way. COUNT instead — `n=$(printf '%s\n' "$data" \| grep -cE "$re")` — because `grep -c` drains stdin (no early exit, no SIGPIPE) over an ordinary pipe (no temp file). Read grep's status honestly: exit 1 means a zero count and is a legitimate answer, anything >1 is a real error. Evaluate the counts ONCE at TOP LEVEL, never inline inside an `if`/`elif` condition: inside `$( )` an `exit` leaves only the subshell and `set -e` does not fire, so an error silently reads as "no match". Validate that each result is numeric and fail closed if not. This applies to both the enforced gate `.gitea/workflows/review-verdict.yml` and the advisory hook `.claude/hooks/pretooluse-merge-consent.sh`. | 2026-07-29 | [link](records/ci/grep-q-pipefail-inversion.md) |
|
||||
| `ci.image-build-delegates-the-spa-suite` | `docker/Dockerfile` runs no vitest suite. Its web-build stage lints, typechecks and BUILDS the SPA; the suite runs once, unfiltered, in `docker-build.yml`'s `test` job on a real checkout, and `build` carries `needs: [test, migrations, scan]` so no image is published past a red suite. THAT EDGE IS NOW THE ONLY LAYER, so the guard checks it is real and not merely present — and it does so by PINNING TEXT rather than parsing it. The commands each SPA-carrying Dockerfile stage runs, and the gating step's `run:` body and `if:`, are compared as strings against a declared pin; the step and its job must carry no `continue-on-error` in any spelling, the job no job-level `if:`, and the publish step must keep its own `docs_only` gate. A guard that asks what a command MEANS was wrong nine times in three review rounds. A pin is immune to a different SPELLING of the command, which is that whole class; it is not immune to the same text meaning something else, so the routes to that are pinned or refused too: `working-directory` and the step's own `shell:`, the workflow `defaults.run.shell` and the job-level `defaults` overriding it, a stage `SHELL`, `web/package.json`'s script map (pinned WHOLE — selecting on the literal `vitest` missed `npm run test`, `npm t` and the `prebuild`/`preinstall` lifecycle hooks), `web/vite.config.ts` PINNED WHOLE — it decides what the suite collects and what `vite build` loads, and pinning a BLOCK of it was defeated seven measured ways across three rounds, so the partial match was withdrawn rather than respelled an eighth time; and the ABSENCE of any config file that outranks it — `vitest.config.*` for vitest, and `vite.config.js`/`.mjs` for `vite build`, whose `DEFAULT_CONFIG_FILES` lists them BEFORE `.ts`. Pinning one file is worthless while a second can outrank it, and that family went short twice. The gating job's `container:` image is left to `ci.image-pin-population` rather than guarded twice. What is unmodelled is a LIST, not an "only": an `ENV` changing what a pinned `RUN` resolves, the plugin BODIES, and a publish through another action. The count of such routes lives in the `docs/guard-inventory.md` row and nowhere else — it briefly existed in three places with two different values, which is how a number stays wrong. It is a running total, wrong at every count so far. They share one shape, which is the transferable part: A PIN ASSUMES IT IS PINNING THE ARTIFACT THAT STILL DECIDES, and every route found so far is authority moving to another file, another occurrence, another workflow, or a hook the pinned command invokes. Do NOT restore a filtered in-image run by naming the specs that cannot run there: that list is a population nothing derives, and the red it produces when it goes stale is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != 'pull_request'` — so it lands on `main` and on the `v*` tag path, where it fails the release cut. Held in both directions by `scripts/tests/test_image_build_delegates_the_spa_suite.py`. | 2026-08-30 | [link](records/ci/image-build-delegates-the-spa-suite.md) |
|
||||
| `ci.infra-shaped-red-under-load` | When a job dies inside a setup/cache step before your code compiles, check the runner host's load before diagnosing the diff, and never file a CI bug off one sample under pressure. | 2026-07-21 | [link](records/ci/infra-shaped-red-under-load.md) |
|
||||
| `ci.jq-version-contract` | Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset, because the CI runner ships jq 1.6 while every developer Mac ships 1.8.x. `scripts/jq-preflight.sh` (no args) prints the parsed version and asserts a floor of 1.6 in every gate job's log; `scripts/jq-preflight.sh --expect 1.6` additionally pins and fails loudly, but ONLY in the `script-tests` job. `review-verdict.yml` never pins — it writes the branch-protection-required `review-verdict/h10` status, so a hard pin there would turn any jq bump into a repo-wide merge deadlock. | 2026-07-26 | [link](records/ci/jq-version-contract.md) |
|
||||
| `ci.killed-job-triage` | Never trust a job's `conclusion` field alone — read the log tail and require an `❌ Failure - Main …` marker before treating a red as a real failure. | 2026-07-21 | [link](records/ci/killed-job-triage.md) |
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
---
|
||||
key: ci.image-build-delegates-the-spa-suite
|
||||
title: '2026-08-30 — the image build builds the SPA and does not test it (#887)'
|
||||
status: active
|
||||
since: '2026-08-30'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: '`docker/Dockerfile` runs no vitest suite. Its web-build stage lints, typechecks and BUILDS the SPA; the suite runs once, unfiltered, in `docker-build.yml`''s `test` job on a real checkout, and `build` carries `needs: [test, migrations, scan]` so no image is published past a red suite. THAT EDGE IS NOW THE ONLY LAYER, so the guard checks it is real and not merely present — and it does so by PINNING TEXT rather than parsing it. The commands each SPA-carrying Dockerfile stage runs, and the gating step''s `run:` body and `if:`, are compared as strings against a declared pin; the step and its job must carry no `continue-on-error` in any spelling, the job no job-level `if:`, and the publish step must keep its own `docs_only` gate. A guard that asks what a command MEANS was wrong nine times in three review rounds. A pin is immune to a different SPELLING of the command, which is that whole class; it is not immune to the same text meaning something else, so the routes to that are pinned or refused too: `working-directory` and the step''s own `shell:`, the workflow `defaults.run.shell` and the job-level `defaults` overriding it, a stage `SHELL`, `web/package.json`''s script map (pinned WHOLE — selecting on the literal `vitest` missed `npm run test`, `npm t` and the `prebuild`/`preinstall` lifecycle hooks), `web/vite.config.ts` PINNED WHOLE — it decides what the suite collects and what `vite build` loads, and pinning a BLOCK of it was defeated seven measured ways across three rounds, so the partial match was withdrawn rather than respelled an eighth time; and the ABSENCE of any config file that outranks it — `vitest.config.*` for vitest, and `vite.config.js`/`.mjs` for `vite build`, whose `DEFAULT_CONFIG_FILES` lists them BEFORE `.ts`. Pinning one file is worthless while a second can outrank it, and that family went short twice. The gating job''s `container:` image is left to `ci.image-pin-population` rather than guarded twice. What is unmodelled is a LIST, not an "only": an `ENV` changing what a pinned `RUN` resolves, the plugin BODIES, and a publish through another action. The count of such routes lives in the `docs/guard-inventory.md` row and nowhere else — it briefly existed in three places with two different values, which is how a number stays wrong. It is a running total, wrong at every count so far. They share one shape, which is the transferable part: A PIN ASSUMES IT IS PINNING THE ARTIFACT THAT STILL DECIDES, and every route found so far is authority moving to another file, another occurrence, another workflow, or a hook the pinned command invokes. Do NOT restore a filtered in-image run by naming the specs that cannot run there: that list is a population nothing derives, and the red it produces when it goes stale is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != ''pull_request''` — so it lands on `main` and on the `v*` tag path, where it fails the release cut. Held in both directions by `scripts/tests/test_image_build_delegates_the_spa_suite.py`.'
|
||||
signals: 'image build fails on main only · every image build fails · latest not republished · release cut fails at the image build · gitless build STAGE not context · node:22-bookworm-slim ships no git · virtual:etv-tracked-source-files could not read the git index · hand-maintained exclude list · exclusion list is the underived population · suite members needing the git binary vs a checkout · COPY .git would not help · needs edge is the only gate · unfiltered gating run · paths: `docker/Dockerfile`, `.gitea/workflows/docker-build.yml`, `scripts/tests/test_image_build_delegates_the_spa_suite.py`, `web/vite-plugins/trackedSourceFiles.ts` · issues: #887, #883, #819, #806, #420'
|
||||
mechanics: 'Populations are DERIVED from the git index (tracked Dockerfiles and workflows, and the stages within them carrying the SPA source); the command lines in those places are PINNED as text. A population decides what is CHECKED, so a hand-written one goes silently short; a pin decides what is EXPECTED, so a stale one goes loudly red. Publishing jobs come from the `docker/build-push-action` step and the Dockerfile each builds from that step''s `file:` input, so `ci-image.yml` is out of scope by derivation rather than by an exemption. 73 mutants, 0 missed (development battery).'
|
||||
---
|
||||
|
||||
The web-build stage is gitless twice over. It copies only `web/` and `design-system/`, so it holds
|
||||
no `.git`. That is a property of the STAGE, not of the build context — the context is the repository
|
||||
root (`context: .`) and `.dockerignore` does not exclude `.git`, so the directory is available to be
|
||||
copied and simply is not; stating it the other way invites a reader to check, find `.git` in the
|
||||
context, and conclude the whole note is stale. And `node:22-bookworm-slim` ships no git binary. Members of the SPA
|
||||
suite need one or the other: `pageSizeCallSites.guard.test.ts` and
|
||||
`completeAnnotations.guard.test.ts` derive their file population from `git ls-files`
|
||||
and refuse to fall back to a directory walk (`testing.guard-derives-population-from-source`), and
|
||||
`trackedSourceFiles.realgit.test.ts` builds a real temporary repository. Because the two
|
||||
prerequisites OVERLAP rather than divide, adding a `COPY .git` fixes neither: the binary is still
|
||||
missing.
|
||||
|
||||
So running the suite there at all costs a list of the members that cannot run, maintained by hand
|
||||
beside a suite that grows. That list went stale the first time a guard was added without updating
|
||||
it, and the failure it produced was invisible where it could have been cheap and fatal where it was
|
||||
not: green on every PR, red on `main` and on the release tag.
|
||||
|
||||
**Why not the alternatives.** Each was considered and rejected on its own evidence, not by analogy:
|
||||
|
||||
* **Add a third `--exclude`.** Re-arms the same trap for the next guard, which is what the issue was
|
||||
filed to prevent.
|
||||
* **Derive the exclusions from source.** Buildable, but it requires a predicate for "needs git" over
|
||||
spec text — two unrelated shapes today (a virtual-module import, a `git` spawn) and no reason to
|
||||
think that stays two. A string predicate over source is the wrong instrument for a set whose
|
||||
members are defined by what they DO at runtime.
|
||||
* **Give the container git** (`apt-get install git` plus a real `.git`). It works, and it costs a
|
||||
cache-busting `COPY` of VCS metadata on every commit, so the whole lint/typecheck/test/build layer
|
||||
re-runs for changes that do not touch `web/`. Synthesising a repository in the image instead —
|
||||
`git init && git add -A` — is worse than it looks: the resulting index is a filesystem walk with
|
||||
extra steps, which is precisely the population the guards refuse.
|
||||
|
||||
**What the removal gives up, enumerated rather than waved through**
|
||||
(`process.enumerate-workaround-behaviors-before-deleting`):
|
||||
|
||||
1. *Blocking a publish on a red suite* — retained, by the `needs:` edge. Two skips exist inside
|
||||
`test`. `docs_only` also gates `Build and push`, so that arm ships nothing — ASSERTED, by
|
||||
`test_the_DOCS_ONLY_arm_cannot_publish_an_image`, after cold review pointed out it had been
|
||||
stated as fact with nothing checking it. The #420 revalidate skip fires only on a tree
|
||||
byte-identical to a head that already carried a green combined status; that arm is a DEPENDENCY
|
||||
on `scripts/ci-detect-already-validated.sh` rather than something asserted here, and that script
|
||||
is graded `MUTATION: NONE`. Said plainly because #887 is what made the second arm load-bearing:
|
||||
before it, the in-image run covered that case.
|
||||
2. *Running the suite under `node:22-bookworm-slim` specifically* — **lost**, and this is the real
|
||||
cost. It is small because the suite is jsdom and pure JS, and because the artifact the image
|
||||
actually ships is the output of `vite build`, which still runs there: a test that passes in the
|
||||
CI toolchain image and fails in bookworm-slim would be telling us about the test environment, not
|
||||
about the image.
|
||||
3. *`docker build` running the suite for a developer locally* — lost; `npm test -- --run` is the
|
||||
direct route and is what `docs/testing.md` documents.
|
||||
|
||||
**The guard's own mechanism was replaced once, and the reason belongs here rather than only in the commit log, because the next person to widen it needs it.** Three versions asked whether a given command runs the suite and whether its failure can be swallowed. That predicate was wrong nine times across three cold-review rounds — executed heredocs treated as data, `#` truncating a command mid-word (including the live `${#reports[@]}` idiom), compound punctuation welding commands, `npm t` and `./node_modules/.bin/vitest` and `timeout`/`su -c` wrappers unrecognised, `true || npm test` counted as a run it never performs, and `continue-on-error: ${{ … }}` slipping a two-literal check. Twice, a clause added to remove a FALSE RED opened a FALSE GREEN on the guard's headline assertion. The mechanism was withdrawn rather than patched a tenth time: the risky command lines are now compared against a pin, so no spelling has to be recognised in order to be rejected. The cost is real and is the point — a legitimate edit to those lines reddens the guard and its author updates the pin deliberately.
|
||||
|
||||
A fourth cold review then attacked the pin itself and found the honest limit: the replacement removes the SPELLING problem outright, but the same pinned text can be made to mean something else, and two mutants re-armed this very defect through `web/package.json` — `RUN npm run build` executes whatever that file says — with every pin still matching. A fifth round then found the same mistake inside the fix: the package.json check SELECTED on the literal `vitest` where a PIN was available, and four one-line edits that never spell it — `npm run test`, `npm t`, and the `prebuild`/`preinstall` lifecycle hooks — each re-armed the defect with every other pin matching. The whole script map is pinned now, and so is `web/vite.config.ts` — WHOLE, after three further
|
||||
rounds each defeated one spelling of a partial match of it, and two more routes bypassed the marker
|
||||
entirely because `defineConfig` is the identity function and a later spread replaces what was
|
||||
matched.
|
||||
|
||||
The general lesson is worth more than any of the fixes: replacing a predicate with a pin moves the problem from "can the checker recognise this?" to "is the checked text still the whole story?". The second question has a short, workable answer where the first did not — but it is still a question, the route list has been wrong at every count so far, and the honest form of that is a running total with its history attached rather than a closure claim.
|
||||
|
||||
`lint` and `typecheck` stay in the stage. They are gitless-safe with no member that is not, so they
|
||||
carry no list and no trap. Whether the image build should run them at all is a separate question
|
||||
this does not answer, and leaving them is not an argument that it should.
|
||||
File diff suppressed because one or more lines are too long
+1
-1
@@ -20,7 +20,7 @@ before adding tests, not just `docs/contributing.md` §8 (which now just points
|
||||
| `ErsatzTV.Scanner.Tests` | Library scanning: scan handlers, folder scanners, NFO readers | Handler tests substitute the folder scanners + `ILibraryRepository` and assert the resulting repository writes (e.g. `ScanLocalLibraryHandlerTests` pins which `LastScan` levels a scan records — ersatztv#264). Fakes/`Testably` back the file-system-facing scanners. ~1,485 tests (approximate on purpose — an exact count goes stale on every PR that adds one). Additionally contains `Core/FFmpeg/TranscodingTests` — `[Explicit]` + `[Combinatorial]`, so it never runs in CI or a plain `dotnet test` (it needs real ffmpeg/hardware) and contributes 0 to that count; run it by name when touching the transcoding pipeline. |
|
||||
| `ErsatzTV.Architecture.Tests` | Layering rules via NetArchTest.eNhancedEdition | Core↛Infra/App/EF; FFmpeg↛all; App↛concrete providers. 5 tests. See `docs/contributing.md` §1. |
|
||||
| `ErsatzTV.FFmpeg.Tests` | FFmpeg command construction | Build a pipeline, assert the exact rendered arg string (`PipelineBuilderBaseTests.cs`). |
|
||||
| `web/` (vitest) | React SPA unit tests | Run alongside typecheck + build (see below). Collects `src/**`, `web/scripts/**` *and* `web/vite-plugins/**`, but deliberately **excludes** `web/e2e/**` (the Playwright specs — vitest's default `**/*.spec.*` glob would otherwise run them under jsdom). Two files have git prerequisites since ersatztv#819. `web/src/api/pageSizeCallSites.guard.test.ts` needs a git **checkout** AND, through it, the **binary** — it derives its file population from `git ls-files` via `web/vite-plugins/trackedSourceFiles.ts` rather than a directory walk, and refuses rather than falling back. `web/vite-plugins/trackedSourceFiles.realgit.test.ts` needs the **binary** but no checkout: it builds its own temp repository to prove that derivation by executing it. So it is not checkout-versus-binary — supplying a `.git` alone would not let either run. Every other file runs fine with neither. `docker/Dockerfile`'s web-build stage runs the suite with exactly those two `--exclude`d, because its context carries no `.git` and `node:22-bookworm-slim` ships no git — and the two exclusions overlap rather than divide — both files need the binary. |
|
||||
| `web/` (vitest) | React SPA unit tests | Run alongside typecheck + build (see below). Collects `src/**`, `web/scripts/**` *and* `web/vite-plugins/**`, but deliberately **excludes** `web/e2e/**` (the Playwright specs — vitest's default `**/*.spec.*` glob would otherwise run them under jsdom). Some files have git prerequisites since ersatztv#819, and the set is not fixed — ersatztv#883 added one. `web/src/api/pageSizeCallSites.guard.test.ts` and `web/src/api/completeAnnotations.guard.test.ts` need a git **checkout** AND, through it, the **binary**: they derive their file population from `git ls-files` via `web/vite-plugins/trackedSourceFiles.ts` rather than a directory walk, and refuse rather than falling back. `web/vite-plugins/trackedSourceFiles.realgit.test.ts` needs the **binary** but no checkout — it builds its own temp repository to prove that derivation by executing it. So it is not checkout-versus-binary: supplying a `.git` alone would not let any of them run. **The suite therefore runs only where git is present, and `docker/Dockerfile` is not such a place** — its web-build stage builds the SPA and does not test it (ersatztv#887). Enumerating the git-dependent files as Docker `--exclude`s was tried and REVERSED: that list is a population nothing derives, it went stale the first time a guard was added, and the resulting red is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != 'pull_request'` — so it landed on `main` and on the release tag instead. The image is gated on `docker-build.yml`'s `test` job running the whole suite on a real checkout, held by `scripts/tests/test_image_build_delegates_the_spa_suite.py`. |
|
||||
| `web/e2e/` (Playwright) | UI-interactive E2E flows against a **live** instance | Not a unit suite and **not** part of `npm test` — needs a running server, so it runs via `scripts/e2e-ui.sh` (boots its own fresh instance) and in CI as a step of the `functional-e2e` job. Headless Chromium, `serial`, `retries: 0`. Scope rule: assert only what the curl harness structurally cannot. See `docs/e2e-local.md` → "UI-E2E harness". |
|
||||
|
||||
## Golden-file nets
|
||||
|
||||
@@ -403,6 +403,26 @@ MUTATIONS: tuple[Mutation, ...] = (
|
||||
"that it moved). Disarming the comparison inside the script instead would prove the second "
|
||||
"and say nothing about the first, which is the half that was missing.",
|
||||
),
|
||||
Mutation(
|
||||
guard="scripts/tests/test_image_build_delegates_the_spa_suite.py",
|
||||
target="docker/Dockerfile",
|
||||
clause="RUN npm run lint && npm run typecheck && npm run build",
|
||||
replacement="RUN npm run lint && npm run typecheck && npm test -- --run && npm run build",
|
||||
proof="test_image_build_delegates_the_spa_suite.py::test_every_SPA_CARRYING_STAGE_runs_exactly_its_pinned_commands",
|
||||
granularity=CLAUSE,
|
||||
expect="docker/Dockerfile::web-build does not run its pinned commands",
|
||||
why="The target is not the guard, per the checker-guard exception: the guard IS a test, so "
|
||||
"disarming it makes it absent rather than red, and the mutation goes into the guarded "
|
||||
"ARTIFACT. The clause is the web-build stage's whole command line, and the replacement is "
|
||||
"the defect itself rather than a caricature of it — ersatztv#883 put a vitest run back into "
|
||||
"a gitless stage, and every image build failed from that commit until #887. WHAT THIS PROVES "
|
||||
"IS NARROWER THAN IT LOOKS, and saying so is the point: the guard no longer decides whether "
|
||||
"a command RUNS the suite (that predicate was wrong nine times across three review rounds), "
|
||||
"it compares the stage's commands against a pin. So this mutation proves the pin is "
|
||||
"compared and reported — not that any particular spelling is recognised, because none needs "
|
||||
"to be. The mutant is deliberately the UNFILTERED spelling: the filtered one is what broke, "
|
||||
"but a pin rejects both identically and choosing the narrower case would suggest otherwise.",
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -43,6 +43,7 @@ from scripts.tests import test_ci_image_pin_population as image_pin
|
||||
from scripts.tests import test_ci_status_context_uniqueness as ctx_unique
|
||||
from scripts.tests import test_guard_inventory as guard_inventory
|
||||
from scripts.tests import test_hook_fire_log as hook_fire
|
||||
from scripts.tests import test_image_build_delegates_the_spa_suite as image_build
|
||||
from scripts.tests import test_pr_changed_files as pr_changed
|
||||
from scripts.tests import test_remote_state_inventory as remote_state
|
||||
from scripts.tests import test_workflow_job_guards as job_guards
|
||||
@@ -108,6 +109,8 @@ def _as_relative_strings(members) -> set[str]:
|
||||
DERIVATIONS = (
|
||||
("test_guard_inventory.derived_guard_files", guard_inventory.derived_guard_files, 25),
|
||||
("test_hook_fire_log.hook_scripts", hook_fire.hook_scripts, 10),
|
||||
("test_image_build_delegates_the_spa_suite.dockerfiles", image_build.dockerfiles, 4),
|
||||
("test_image_build_delegates_the_spa_suite.workflow_files", image_build.workflow_files, 5),
|
||||
("test_ci_image_pin_population.workflow_files", image_pin.workflow_files, 5),
|
||||
("test_ci_status_context_uniqueness.workflow_files", ctx_unique.workflow_files, 5),
|
||||
("test_remote_state_inventory.derived_population", remote_state.derived_population, 40),
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -13,10 +13,11 @@ import { type GitRunner, resolveTrackedSourceFiles } from './trackedSourceFiles'
|
||||
* plugin's refusal. Reading the two exclusions as checkout-versus-binary is what broke the Docker
|
||||
* stage once already; see `docker/Dockerfile`, which retracts that framing.
|
||||
*
|
||||
* `docker/Dockerfile`'s web-build stage runs on `node:22-bookworm-slim`, which ships no git at all,
|
||||
* so this file is excluded there alongside the guard. Keeping them apart means
|
||||
* that exclusion costs two files rather than the thirteen injected-runner tests in
|
||||
* `trackedSourceFiles.test.ts`, which run fine with no git and no repository.
|
||||
* Keeping these apart from `trackedSourceFiles.test.ts` confines the git prerequisite to one file:
|
||||
* its thirteen injected-runner siblings run with no git and no repository. The suite as a whole is
|
||||
* therefore run only where git is present. `docker/Dockerfile`'s web-build stage is not such a
|
||||
* place — `node:22-bookworm-slim` ships no git at all — and since ersatztv#887 it does not run the
|
||||
* suite there rather than naming the files that cannot run, which was a list nothing derived.
|
||||
*
|
||||
* Do NOT make these skip when git is missing. A check that answers "could not tell" by passing is a
|
||||
* permanent no-op, which is the whole failure mode this plugin exists to avoid.
|
||||
@@ -74,7 +75,9 @@ describe('resolveTrackedSourceFiles against a REAL repository (#819)', () => {
|
||||
|
||||
// Residual: this asserts the message the MISSING-BINARY path also emits, so in an environment with
|
||||
// no git it passes for the wrong reason. Not silent overall — its two siblings fail ENOENT there,
|
||||
// loudly — and that is exactly the environment this file is excluded from in `docker/Dockerfile`.
|
||||
// loudly. Since ersatztv#887 no such environment runs this suite: `docker/Dockerfile` stopped
|
||||
// running it rather than excluding the files that cannot run, so the residual is now reachable
|
||||
// only on a developer machine with no git installed.
|
||||
it('really throws outside a repository, rather than returning an empty population', () => {
|
||||
const notARepo = mkdtempSync(join(tmpdir(), 'etv-819-bare-'));
|
||||
repos.push(notARepo);
|
||||
|
||||
@@ -150,7 +150,9 @@ export function trackedSourceFilesPlugin(runGit?: GitRunner, fileExists?: FileEx
|
||||
return id === TRACKED_SOURCE_FILES_ID ? RESOLVED_ID : undefined;
|
||||
},
|
||||
// Deliberately LAZY: git runs only when something imports the virtual module. `vite build`
|
||||
// never resolves this id and so never shells out to git.
|
||||
// never resolves this id and so never shells out to git. LOAD-BEARING FOR THE IMAGE BUILD since
|
||||
// ersatztv#887: `docker/Dockerfile`'s web-build stage runs `npm run build` in a stage with no git
|
||||
// binary, so making this eager would break every image build for #887's original root cause.
|
||||
load(id) {
|
||||
if (id !== RESOLVED_ID) {
|
||||
return undefined;
|
||||
|
||||
Reference in New Issue
Block a user