ci(631): install pyyaml too — the suite is not pure-stdlib, and CI proved it
First CI run went red on `ModuleNotFoundError: No module named 'yaml'` at collection: test_migration_equivalence.py imports scripts/migrate_decisions_split.py, which uses PyYAML by design. It passed locally only because this machine already had PyYAML. The original claim came from reading decisions_lib/decisions_validate/build_catalog and the test files themselves — which is precisely the set that does NOT import yaml. Replaced with an AST import scan over all of scripts/, which gives the complete third-party set: pytest and yaml. (etv_client is imported only by scripts/scripted-schedules/entrypoint.py, a container entrypoint pytest never collects.) This does not contradict the dependency-free decisions READ path: decisions_lib._read_frontmatter is hand-written exactly so validation runs where nothing is installed. The one-shot WRITE path is allowed PyYAML, and its test needs it. Verified the way it should have been the first time: a clean venv with pytest alone reproduces CI's collection error; adding pyyaml gives 111 passed. Comments, ci-cd.md and the decision record all corrected to drop the "pure stdlib" claim. Refs #631
This commit is contained in:
@@ -210,8 +210,7 @@ jobs:
|
||||
# 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.
|
||||
# filter buys nothing but drift.
|
||||
script-tests:
|
||||
name: Script tests (pytest)
|
||||
runs-on: small
|
||||
@@ -223,8 +222,16 @@ jobs:
|
||||
uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: '3.x'
|
||||
- name: Install pytest
|
||||
run: python3 -m pip install --disable-pip-version-check --quiet pytest
|
||||
# pytest + PyYAML. PyYAML is NOT a contradiction of the dependency-free decisions READ path:
|
||||
# `decisions_lib._read_frontmatter` is hand-written precisely so validation runs where nothing
|
||||
# is installed, but the one-shot WRITE path `migrate_decisions_split.py` uses PyYAML by
|
||||
# design — and `test_migration_equivalence.py` imports that module, so the suite needs it.
|
||||
# `pytest` and `yaml` are the complete third-party set, established by an AST import scan over
|
||||
# all of scripts/ rather than by reading the files that seemed relevant: the first cut of this
|
||||
# job claimed "pure stdlib", passed locally on a machine that happened to have PyYAML, and
|
||||
# went red in CI on a collection error.
|
||||
- name: Install test dependencies
|
||||
run: python3 -m pip install --disable-pip-version-check --quiet pytest pyyaml
|
||||
# Preflight, not an install (ersatztv#390 removed run-time `apt-get` from CI on purpose).
|
||||
# test_post_review_verdict.py and test_merge_consent_exemption.py exec the REAL
|
||||
# post-review-verdict.sh / pretooluse-merge-consent.sh, which shell out to `jq` ~26 times.
|
||||
|
||||
+11
-4
@@ -629,10 +629,17 @@ name keeps a real failure unambiguous.
|
||||
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
|
||||
`.claude/hooks/**` edit. At ~10s, a filter buys nothing but drift.
|
||||
|
||||
**Dependencies: `pytest` and `pyyaml`** — the complete third-party set across `scripts/`, established
|
||||
by an AST import scan rather than by reading the files that looked relevant. PyYAML does **not**
|
||||
contradict the dependency-free decisions *read* path: `decisions_lib._read_frontmatter` is
|
||||
hand-written exactly so validation runs where nothing is installed, but the one-shot *write* path
|
||||
`migrate_decisions_split.py` uses PyYAML by design, and `test_migration_equivalence.py` imports that
|
||||
module. (The first cut of this job claimed "pure stdlib + pytest", passed locally on a machine that
|
||||
happened to have PyYAML installed, and went red in CI on a `ModuleNotFoundError` at collection —
|
||||
which is itself a small demonstration of why the suite needed to run in CI at all.) 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.
|
||||
|
||||
A **preflight step** asserts `jq` and `git` are on PATH before running the suite. Those two tests
|
||||
|
||||
@@ -29,8 +29,8 @@ 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.
|
||||
setup. Here there is no shared setup to reuse (a checkout plus `pip install pytest pyyaml`), 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
|
||||
|
||||
Reference in New Issue
Block a user