Files
ersatztv/scripts/tests/test_docs_only_detector_clone_depth.py
T
timothyandClaude Opus 5 58d2c147a4
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 13s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 21s
review-verdict/h10 Review-verdict: MERGEABLE @ 58d2c14 (base: main)
PR Gates / Docs update reminder (pull_request) Successful in 22s
PR Gates / decisions lifecycle (pull_request) Successful in 17s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 18s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Failing after 8s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 12s
Review verdict / Set review-verdict status (pull_request_target) Successful in 16s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Canceled after 0s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Canceled after 0s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Canceled after 0s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Canceled after 0s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Canceled after 0s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Canceled after 0s
fix(836): never pass --depth to a checkout that may already be complete
`git fetch --depth=N` GRAFTS a complete clone shallow — it writes `.git/shallow`
and cuts history at N even though every object is already present.
`scripts/ci-detect-docs-only.sh` has four consumers in docker-build.yml and they
do not agree on checkout depth: test/migrations/functional-e2e check out
`fetch-depth: 2`, while `build` (and api-docs/format on the PR arm) check out
`fetch-depth: 0`. The script applied a depth chosen for the first group to all of
them.

For `build` that grafted its complete clone on every push to main, so the
`git describe --tags` in the immediately following `Compute version and tags`
step found no reachable tag, and a `|| echo v0.0.0` fallback that cannot fail
turned that into a version: every `:latest` image shipped
`InformationalVersion 0.0.0-<sha>`.

Both fetch sites now go through `fetch_ref`, which passes `--depth` only when
`git rev-parse --is-shallow-repository` answers `true`. That also closes the PR
arm's `--depth=200` — the same defect with a larger N, grafting api-docs/format.
An unknown answer passes no depth: the query fails only where there is no
readable repository, and a depth cannot help there either.

Second half: `Compute version and tags` no longer defaults. A failed
`git describe` on the non-tag path fails the job rather than stamping 0.0.0, so
no `:latest` is published instead of a mislabelled one. `build` is not a required
status context, so this cannot block a merge. The tag path never calls
`describe`, so `:prod`/`:<version>` releases are unaffected — which is also why
they were correctly versioned throughout.

Measured, not inferred:
- The live defect, from run 2416 / job 10345 (push to main 8aeacd534):
  `INFO_VERSION=0.0.0-8aeacd53`.
- Against a real anonymous clone of this repository: the predecessor detector
  grafts it and `describe` fails -> `0.0.0-1afad085`; the fixed detector leaves
  it complete -> `26.14.0-1afad085`.
- The workflow step body executed on both clones: grafted -> exit 1 with a
  diagnostic, complete -> `26.14.0-1afad085`, tag path -> `26.14.0` + `:prod`
  even on a grafted clone.

New guard `scripts/tests/test_docs_only_detector_clone_depth.py` drives the real
script over real `file://` clones (a plain path clone silently ignores --depth,
so it carries a negative control that requires the fixture to be able to graft at
all). It was witnessed red against the predecessor. Its clause mutation is
declared in mutation_manifest.py, which promotes the detector's guard-inventory
row from NONE to MUTATION; the stated reason it was undeclared — that a
skip-gate script's effect is visible only in a workflow run — was itself wrong.

fixes #836

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HETQK67q1bEpDZpHLZNsp9
2026-08-29 22:54:23 +02:00

358 lines
17 KiB
Python

"""`scripts/ci-detect-docs-only.sh` must never graft a COMPLETE clone shallow (ersatztv#836).
`git fetch --depth=N` writes `.git/shallow` and cuts history at N even when every object is already
present. Exactly four `docker-build.yml` jobs run the detector — `test`, `migrations` and
`functional-e2e` at `fetch-depth: 2`, and `build` at `fetch-depth: 0` — so `build` is the only
complete-clone consumer, and the detector asks the repository which shape it is in rather than
assuming. (`api-docs` and `format` have a step with `id: detect` too, but it runs their own inline
diff, not this script. Do not add them here.)
WHAT WENT WRONG WITHOUT THIS. `build`'s next step after the detector is `Compute version and tags`,
whose `git describe --tags --abbrev=0` needs a reachable tag. The graft cut it off, `describe`
failed, and a `|| echo v0.0.0` fallback that cannot fail turned that into a version string: every
`:latest` image published from `main` carried `InformationalVersion 0.0.0-<sha>` instead of
`26.x.y-<sha>`, from 2026-07-17 (#416) until #836, with nothing red anywhere. It was found by
reading the string out of
a running container. That is why the assertion here is on the RESULTING VERSION and on the shallow
flag — not on the detector's exit status, which was `0` throughout the defect.
WHY THE FIXTURE USES `file://`. `git clone /path` and `git fetch /path` use the local transport,
which IGNORES `--depth` outright. A fixture built on a plain path would therefore never graft, every
assertion below would hold, and the whole file would pass while testing nothing —
`testing.eight-ways-a-test-passes-for-the-wrong-reason`. `test_the_fixture_can_actually_graft` is the
negative control that keeps that honest: it performs the raw depth fetch the detector is forbidden to
perform and REQUIRES the graft to happen. If git or the transport ever stops grafting, that control
goes red and says so, instead of the other tests quietly becoming vacuous.
"""
from __future__ import annotations
import os
import subprocess
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parents[2]
DETECTOR = REPO_ROOT / "scripts" / "ci-detect-docs-only.sh"
TAG = "v26.3.1"
def _env(**extra: str) -> dict[str, str]:
"""Ambient `GIT_*` removed, everything else kept.
Exported `GIT_DIR`/`GIT_WORK_TREE`/`GIT_INDEX_FILE` override `cwd` and would aim these git calls
at the real repository. The rest of `os.environ` is carried deliberately rather than built from
scratch: `conftest.py`'s isolation puts `ETV_HOOK_FIRE_LOG_DIR` there, and a from-scratch
environment would drop it and write into the production hook-fire log.
`GITHUB_` is dropped alongside `GIT_` because it does NOT match the `GIT_` prefix
(`"GITHUB_OUTPUT".startswith("GIT_")` is False), and every `GITHUB_*` the detector reads decides
which arm it takes. Pinning only the one variable that bites today would leave the class open for
the next arm that starts reading a sibling — the whole prefix goes, and `_run_detector` puts back
exactly what each test means to set.
"""
env = {
k: v for k, v in os.environ.items() if not k.startswith(("GIT_", "GITHUB_"))
}
env.update(extra)
return env
def _git(args: list[str], cwd: Path) -> str:
r = subprocess.run(
["git", *args], cwd=str(cwd), env=_env(), capture_output=True, text=True
)
assert r.returncode == 0, f"git {' '.join(args)} failed: {r.stderr}"
return r.stdout.strip()
def _git_rc(args: list[str], cwd: Path) -> subprocess.CompletedProcess:
return subprocess.run(
["git", *args], cwd=str(cwd), env=_env(), capture_output=True, text=True
)
def _is_shallow(repo: Path) -> str:
return _git(["rev-parse", "--is-shallow-repository"], repo)
@pytest.fixture(scope="module")
def origin_url(tmp_path_factory) -> str:
"""An origin shaped like ours: the newest tag is well back, and the tip is a merge commit.
Both properties matter. The tag has to be further back than the fetch depth or the graft cannot
cut it off, and the tip has to be a merge commit or the detector's push arm reports
`non-merge push` and returns before reaching anything this file is about.
"""
root = tmp_path_factory.mktemp("detector-depth")
origin = root / "origin"
origin.mkdir()
_git(["init", "-q", "-b", "main", "."], origin)
_git(["config", "user.email", "test@example.com"], origin)
_git(["config", "user.name", "Test"], origin)
for i in range(1, 13):
(origin / f"f{i}.txt").write_text(f"{i}\n")
_git(["add", "-A"], origin)
_git(["commit", "-q", "-m", f"c{i}"], origin)
_git(["tag", TAG], origin)
for i in range(13, 31):
(origin / f"f{i}.txt").write_text(f"{i}\n")
_git(["add", "-A"], origin)
_git(["commit", "-q", "-m", f"c{i}"], origin)
# A docs-only branch merged with --no-ff, i.e. exactly how an update to `main` lands here.
_git(["checkout", "-q", "-b", "feature"], origin)
(origin / "docs").mkdir()
(origin / "docs" / "a.md").write_text("doc\n")
_git(["add", "-A"], origin)
_git(["commit", "-q", "-m", "docs change"], origin)
_git(["checkout", "-q", "main"], origin)
_git(["merge", "-q", "--no-ff", "feature", "-m", "merge feature (#1)"], origin)
return f"file://{origin}"
@pytest.fixture(scope="module")
def deep_origin_url(tmp_path_factory) -> str:
"""An origin with MORE than 200 commits after its newest tag, for the PR arm's `--depth=200`.
Built separately, and the extra ~3s is the price of a test that is not vacuous. The other
fixture's 31 commits all fit inside a depth of 200, so a `--depth=200` fetch there has nothing
to cut: no `.git/shallow` is written, `describe` keeps resolving, and a PR-arm test built on it
passes against the UNFIXED script — measured, which is how this fixture came to exist. A guard
whose positive case cannot occur is the defect it is meant to catch, one level up.
"""
root = tmp_path_factory.mktemp("detector-depth-deep")
origin = root / "origin"
origin.mkdir()
_git(["init", "-q", "-b", "main", "."], origin)
_git(["config", "user.email", "test@example.com"], origin)
_git(["config", "user.name", "Test"], origin)
(origin / "a.txt").write_text("a\n")
_git(["add", "-A"], origin)
_git(["commit", "-q", "-m", "c0"], origin)
_git(["tag", TAG], origin)
# Empty commits: the depth boundary counts commits, and 210 real ones would only be slower.
for i in range(1, 211):
_git(["commit", "-q", "--allow-empty", "-m", f"c{i}"], origin)
return f"file://{origin}"
def _clone(origin_url: str, dest: Path, depth: int | None = None) -> Path:
args = ["clone", "-q"]
if depth is not None:
args += [f"--depth={depth}"]
args += [origin_url, str(dest)]
_git(args, dest.parent)
return dest
def _run_detector(repo: Path, tmp_path: Path, **github_env: str) -> tuple[subprocess.CompletedProcess, str]:
"""Drive the real detector.
`GITHUB_REF_TYPE` is pinned to `branch` rather than left to the ambient environment. `_env()`
deliberately carries the rest of `os.environ` (for `ETV_HOOK_FIRE_LOG_DIR`), and the detector's
FIRST branch returns early on `GITHUB_REF_TYPE=tag` — before any fetch — so an exported `tag`
turns every test here red for a reason that has nothing to do with what they assert.
Not live today, stated precisely rather than dramatised: the `scan` job DOES run pytest on tag
pushes, but over two named files, and this is not one of them; `script-tests`, which does run
this file, is `on: pull_request`. Adding this file to `scan`'s argument list is the one edit that
would make it live. It is a default (`env.update(github_env)` follows), so a future test can
still drive the tag arm deliberately.
"""
out_file = tmp_path / f"gh-output-{repo.name}"
out_file.write_text("")
env = {"GITHUB_REF_TYPE": "branch"}
env.update(github_env)
r = subprocess.run(
["bash", str(DETECTOR)],
cwd=str(repo),
env=_env(GITHUB_OUTPUT=str(out_file), **env),
capture_output=True,
text=True,
)
return r, out_file.read_text()
# --------------------------------------------------------------------------------------------
# The negative control. Everything else here asserts that a graft does NOT happen, so it is all
# vacuous if the fixture cannot graft in the first place.
# --------------------------------------------------------------------------------------------
@pytest.mark.parametrize(
"fixture_name, depth",
[("origin_url", 2), ("deep_origin_url", 200)],
ids=["push-arm-depth-2", "pr-arm-depth-200"],
)
def test_the_fixture_can_actually_graft(request, fixture_name, depth, tmp_path):
"""The raw `--depth=N` fetch the detector must not perform DOES graft this fixture.
Parametrised over BOTH fixtures and BOTH depths, because a control that covers only one of
them leaves the other free to go vacuous unreported — which is what happened: the PR arm was
first written against the 31-commit origin, where a depth of 200 has nothing to cut, and it
passed against the unfixed script. One control per (fixture, depth) pair the file relies on.
"""
origin = request.getfixturevalue(fixture_name)
repo = _clone(origin, tmp_path / f"control-{depth}")
assert _is_shallow(repo) == "false", "a plain clone should be complete"
assert _git(["describe", "--tags", "--abbrev=0"], repo) == TAG
_git(["fetch", "--no-tags", f"--depth={depth}", "origin", "main"], repo)
assert _is_shallow(repo) == "true", (
f"a --depth={depth} fetch did not graft this fixture, so the test that relies on it is "
"vacuous — most likely the clone/fetch fell back to the local transport, which ignores "
"--depth (use file://), or the fixture has fewer commits than the depth"
)
described = _git_rc(["describe", "--tags", "--abbrev=0"], repo)
assert described.returncode != 0, (
"the graft happened but `git describe` still resolved, so the version this file protects "
"is not actually reachable from the shallow boundary — the fixture's tag is too close to "
f"the tip. stdout={described.stdout!r}"
)
# --------------------------------------------------------------------------------------------
# The guard proper.
# --------------------------------------------------------------------------------------------
def test_the_push_arm_leaves_a_COMPLETE_clone_complete(origin_url, tmp_path):
"""`build`'s shape: `fetch-depth: 0`, push to main. The version must survive the detector.
This is the proof named by `scripts/ci-detect-docs-only.sh`'s `MUTATION` row in
`docs/guard-inventory.md`; the declared mutation in `mutation_manifest.py` disarms the shallow
condition, which puts the `--depth=2` fetch back and reddens the two assertions below.
"""
repo = _clone(origin_url, tmp_path / "build-shape")
assert _is_shallow(repo) == "false"
r, output = _run_detector(
repo, tmp_path, GITHUB_EVENT_NAME="push", GITHUB_REF_NAME="main"
)
assert r.returncode == 0, f"detector failed: {r.stdout}{r.stderr}"
assert _is_shallow(repo) == "false", (
"the detector GRAFTED the complete clone shallow — this is ersatztv#836: the next step in "
"`build` is `Compute version and tags`, whose `git describe` then finds no reachable tag "
f"and the image ships InformationalVersion 0.0.0-<sha>.\ndetector said:\n{r.stdout}"
)
described = _git_rc(["describe", "--tags", "--abbrev=0"], repo)
assert described.returncode == 0 and described.stdout.strip() == TAG, (
"after the detector, `git describe --tags --abbrev=0` no longer resolves, so `build` would "
f"stamp this image 0.0.0. git said: {described.stdout.strip()!r} {described.stderr.strip()!r}"
)
# The detector still has to do its actual job on this shape.
assert "docs_only=true" in output, (
f"the merge tip is docs-only but the detector did not say so:\n{r.stdout}\n{output}"
)
def test_the_pull_request_arm_leaves_a_COMPLETE_clone_complete(deep_origin_url, tmp_path):
"""A complete checkout on the PR arm, N=200. NO PRODUCTION CONSUMER TODAY — deliberately.
Said plainly because the first version of this docstring got it wrong: on `pull_request` the only
jobs running this script are the three `fetch-depth: 2` ones, so the PR arm's `--depth=200` was
inert rather than latently firing. What this pins is the SHARED CLAUSE — the next `fetch-depth: 0`
consumer added to this arm must not silently inherit the graft that cost #836. It is a guard
against a future shape, not a reproduction of a shipped one, and grading it as the latter would
overstate what the file proves.
The deep fixture is what makes the case real at all: with the 32-commit origin a depth of 200 has
nothing to cut, and this test passed against the unfixed script.
"""
repo = _clone(deep_origin_url, tmp_path / "pr-shape")
assert _is_shallow(repo) == "false"
# A PR head: one docs commit on top of the base branch.
_git(["checkout", "-q", "-b", "pr-branch"], repo)
(repo / "docs").mkdir(exist_ok=True)
(repo / "docs" / "b.md").write_text("doc\n")
_git(["add", "-A"], repo)
_git(["commit", "-q", "-m", "docs on the PR head"], repo)
r, output = _run_detector(
repo, tmp_path, GITHUB_EVENT_NAME="pull_request", GITHUB_BASE_REF="main"
)
assert r.returncode == 0, f"detector failed: {r.stdout}{r.stderr}"
assert _is_shallow(repo) == "false", (
"the PR arm GRAFTED a complete clone shallow (ersatztv#836). No job runs this arm against a "
f"complete checkout today, so this is the shared clause regressing.\n{r.stdout}"
)
described = _git_rc(["describe", "--tags", "--abbrev=0"], repo)
assert described.returncode == 0 and described.stdout.strip() == TAG, (
"after the PR arm, `git describe --tags --abbrev=0` no longer resolves: "
f"{described.stdout.strip()!r} {described.stderr.strip()!r}"
)
# FETCH_HEAD is what the PR arm diffs against, so the depth-less fetch must still write it.
assert _git_rc(["rev-parse", "--verify", "-q", "FETCH_HEAD"], repo).returncode == 0, (
"the PR arm no longer writes FETCH_HEAD, so its base revision is unresolvable"
)
assert "docs_only=true" in output, (
f"the PR head is docs-only but the detector did not say so:\n{r.stdout}\n{output}"
)
def test_a_SHALLOW_checkout_still_gets_its_depth_and_still_decides(origin_url, tmp_path):
"""The other half: `test`/`migrations`/`functional-e2e` at `fetch-depth: 2` must be unchanged.
Removing the graft must not turn into removing the depth. A shallow checkout stays shallow, and
the detector still resolves `HEAD^1` and classifies the docs-only merge — the behaviour the
`--depth=2` was added for in the first place.
"""
repo = _clone(origin_url, tmp_path / "shallow-shape", depth=2)
assert _is_shallow(repo) == "true", "the fixture clone was meant to be shallow"
r, output = _run_detector(
repo, tmp_path, GITHUB_EVENT_NAME="push", GITHUB_REF_NAME="main"
)
assert r.returncode == 0, f"detector failed: {r.stdout}{r.stderr}"
assert _is_shallow(repo) == "true", "a shallow checkout was silently unshallowed"
assert "docs_only=true" in output, (
"the shallow checkout no longer classifies the docs-only merge — the depth stopped being "
f"applied where it IS needed:\n{r.stdout}\n{output}"
)
assert "fetching with --depth=2" in r.stdout, (
f"the shallow checkout did not take the depth branch:\n{r.stdout}"
)
assert "fetching without --depth" not in r.stdout, (
f"the shallow checkout took the complete-clone branch:\n{r.stdout}"
)
def test_an_UNKNOWN_shallow_answer_passes_no_depth(tmp_path):
"""Fail-direction, stated and executed: `--is-shallow-repository` unanswerable => no depth.
Defaulting the other way would re-graft on exactly the paths nobody can observe, so the branch
is pinned rather than argued. Running outside a repository is one way to make the query
unanswerable and the one this drives; it is NOT the only one — on a git older than 2.15 the flag
is unrecognised and rev-parse echoes it back verbatim with status 0. That shape is not covered
here (it needs an ancient git), and it reaches the same no-depth branch.
WHAT THIS DELIBERATELY DOES NOT ASSERT. Outside a repository the detector exits 128 with an
empty `$GITHUB_OUTPUT`, because `set -o pipefail` makes the `git rev-list ... | wc -w` assignment
below the fetch abort under `set -e`. That is PRE-EXISTING and unchanged here — measured against
`8aeacd534`, the commit this fix branched from, which exits 128 the same way. It is also not a
fail-open: the step goes red, so the job does, and no `docs_only=true` is ever emitted. Asserting
a clean exit here would be asserting a behaviour the script has never had.
"""
notarepo = tmp_path / "notarepo"
notarepo.mkdir()
r, _output = _run_detector(
notarepo, tmp_path, GITHUB_EVENT_NAME="push", GITHUB_REF_NAME="main"
)
assert "is-shallow=unknown" in r.stdout, (
f"the unknown answer was not reached or not reported:\n{r.stdout}"
)
assert "fetching without --depth" in r.stdout, (
f"an unknown shallow answer must not ask for a depth:\n{r.stdout}"
)