fix(772,792): close the fix-round review — the doc still described the semantics the fix reversed

The re-review's one HIGH was mine and was the obvious one to miss: the previous commit changed
the preflight so an unverifiable answer FAILS, and left a `docs/ci-cd.md` paragraph two
screens away still saying "anything else is reported as could-not-tell". That paragraph is
the one an operator reads when the job goes red, and it would have talked them into
reinstating the defect. Replaced with the full arm table, including the two rows the first
draft got wrong and why.

  * "gates nothing" was false in the way this repo has recorded before (#598): the
    merge-consent hook reads the COMBINED status, so a red preflight blocks the merge like
    any other red job. It does not SKIP the jobs it diagnoses; that is the accurate claim,
    in ci-cd.md and in the remote-state row.
  * The production retry defaults were evaluated by nothing — every test overrode both
    knobs. A test now drops the overrides and measures three attempts and a real pause, so
    editing the default to 1/0 (which would falsify the "a blip does not redden a PR"
    argument) goes red.
  * `journalctl -u gitea | grep ExecuteCleanupRules` is not a reproduction: that identifier
    reaches the log only through slow-query warnings, so an empty grep on a healthy host
    reads as "the rule never ran" — the inverse. Replaced with the admin cron API, which
    answers deterministically.
  * The recovery recipe's `docker buildx use default` needs the containerd image store to
    `--push` (both named hosts have it, checked today) and mutated the operator's builder
    selection without restoring it.
  * The stub's comment claimed both halves of real curl's transport failure mattered; only
    the exit status is observable, because `|| resp=""` discards what curl printed.
  * The empty-half credential refusal echoed the username; it needs no value at all. The
    401/403 arm aborts the remaining pins while 404 continues — deliberate, now stated.
  * `curl -u "$VAR"` puts a credential in argv, and this job runs container-free on a shared
    host. NOT fixed here: it is the shape all five `scripts/` callers already use, so fixing
    one site leaves the class and splits the codebase. Filed as #821 and named at the site.

refs #772
refs #792
This commit is contained in:
2026-08-22 23:48:20 +02:00
parent 0cf355e494
commit 30640bb780
4 changed files with 72 additions and 9 deletions
@@ -14,6 +14,7 @@ from __future__ import annotations
import os
import subprocess
import time
from pathlib import Path
import pytest
@@ -46,8 +47,8 @@ if not user or not password:
codes = dict(pair.split("=", 1) for pair in (state / "codes").read_text().split() if pair)
code = codes.get(tag, codes.get("*", "200"))
if code == "TRANSPORT":
# Real curl prints the -w output and exits 7 when the connection fails; both halves matter.
print("000", end="")
# Only the EXIT STATUS is observable: the script's `|| resp=""` discards whatever curl printed,
# so what this reproduces is the non-zero exit, not the `\n000` real curl also emits.
sys.exit(7)
body = '{"schemaVersion": 2, "mediaType": "application/vnd.oci.image.manifest.v1+json"}'
if code == "200-NOT-A-MANIFEST":
@@ -274,3 +275,27 @@ def test_the_grep_line_cannot_match_ITSELF(preflight):
assert result.returncode != 0
assert "no ersatztv-ci pin found" in result.stderr
assert preflight.calls() == []
def test_the_PRODUCTION_retry_defaults_are_the_ones_that_run(preflight):
"""Every other test overrides the retry knobs, so nothing evaluated `${VAR:-default}` itself.
That matters because the defaults are the argument: "unknown fails" is only affordable if an
ordinary registry blip is absorbed first. Edited to 1 attempt / 0 seconds, this file would stay
green while a single transient 503 reddened every PR. So this one drops both overrides and
measures the real thing — three attempts, and a pause long enough to have actually happened.
"""
del preflight.env["ETV_CI_ATTEMPTS"]
del preflight.env["ETV_CI_RETRY_SECONDS"]
preflight.set_codes({"32747a0": "503"})
started = time.monotonic()
result = preflight.run()
elapsed = time.monotonic() - started
assert result.returncode != 0
assert len(preflight.calls()) == 3, f"the default attempt count is not 3 — got {len(preflight.calls())} call(s)"
assert elapsed >= 8, (
f"two pauses at the default 5s should take >=10s; took {elapsed:.1f}s, so the default pause "
"has been shortened out from under the 'a blip does not redden a PR' argument"
)