From 86f41d90b078045ba55a20cc962337f5bb203609 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 19:52:06 +0000 Subject: [PATCH 1/3] shape-doctor.py, bootstrap.py, bump-leg.py and update-shape.py: a `legs:` that is not a list, or holds an entry that is not a mapping, is said in #714's words, so the doctor's leg rows wait on the `manifest` row instead of calling the project one with no legs, bootstrap places no leg for it, and bump-leg.py and update-shape.py refuse it by name where each ended in a `TypeError` (#832) Four more readers of a project manifest's `legs:` iterated `legs or []`: the doctor's `Context.legs`, the assembly root's `scripts/bootstrap.py`, `bump-leg.py`'s `Project.legs` and `update-shape.py`'s `pull_request_repository`. `legs: 5` ended each in `TypeError: 'int' object is not iterable` (the doctor's whole report, in `declared_pins` under the `manifest kinds` row; `update-shape.py` on the re-run `--pr` recognises). A scalar `openXdox`, a mapping `{a: b}` or `[5]` was read as a project with no legs: the doctor's `legs` row said "a one-repository project has none", bootstrap's step (a) "no submodule legs declared", `bump-leg.py` refused `bump-leg-unknown-leg` with "it declares" and nothing after it, and `update-shape.py` "declares no assembly leg". Each now says #714's sentence for what it found, the value spelled by `repr` as the gates spell it: manifest-legs: legs is , expected a list of `role:`, `repository:` and `path:` mappings manifest-leg: a leg is not a mapping: - shape-doctor.py: `Context.legs` reads a list only, and `Context.legs_unread` says what names no leg. `legs_unread_row` makes the `legs`, `leg shape files` and `placement` rows `n/a`, says the sentence and names the `manifest` row in `waits_on`, as `waiting_row` does for a manifest that does not load (#205); `--placement-plan` refuses by name. A list holding legs beside such an entry still has its legs checked, and the entry is the `manifest` row's. - bootstrap.py (a pinned copy): `_legs_of` gives step (a) the legs and the sentences; it says each, places no leg for it and places the mappings beside it, and step (b)'s validators refuse. - bump-leg.py: `Project.legs` refuses `bump-leg-manifest-legs`, exit 2, before anything is fetched and under `--dry-run` as well. A list holding legs beside such an entry is refused too: its dry run previewed the move and the real run was refused `bump-leg-validators-red` on the project's own `validate-manifest.py`'s `manifest-leg` finding. - update-shape.py: `update-pr-no-repository` says the sentence. `templates/assembly-root/scripts/bootstrap.py` is in `shape_materialize.COPIED_VERBATIM`; no digest of it lives in this repository, so nothing is re-pinned here. In a project `update-shape.py check` reports it `upstream-changed` and `apply` re-copies and re-pins it. tests/test_legs_readers_not_a_list.py covers each site over `5`, `openXdox`, `{a: b}` and `[5]`, the entry rule (`[openXdox]`, `[null]`), a value holding ESC and a list mixing legs with such an entry. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 40 +- shape-doctor.py | 91 ++++- templates/assembly-root/scripts/bootstrap.py | 49 ++- tests/test_legs_readers_not_a_list.py | 399 +++++++++++++++++++ update-shape.py | 26 +- 5 files changed, 581 insertions(+), 24 deletions(-) create mode 100644 tests/test_legs_readers_not_a_list.py diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 226e7b0d..5238da0d 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -35,6 +35,11 @@ says so and moves the commit, exactly as the real run does (#534). A tree, a blob or a tag of either is refused `bump-leg-target-not-a-commit` by name, in both runs. + * a manifest whose `legs:` is not a list, or holds an entry that is not a + mapping: `bump-leg-manifest-legs`, naming each in the words of the + project's own `validate-manifest.py`, before anything is fetched and + under `--dry-run` as well (#832). That validator refuses either, so a + bump over one could never land. * a commit the leg's REMOTE does not have. A pin nobody else can fetch is a root nobody else can bootstrap. * a dirty root. The commit is made with explicit pathspecs, so unrelated @@ -202,8 +207,39 @@ def open(cls, root_spec) -> "Project": @property def legs(self) -> list[dict]: - return [row for row in (self.data.get("legs") or []) - if isinstance(row, dict)] + """The manifest's legs, in its own order. + + A `legs:` THAT IS NOT A LIST, OR AN ENTRY OF IT THAT IS NOT A MAPPING, + IS REFUSED `bump-leg-manifest-legs` (#832), naming each in + `validate-manifest.py`'s words for it (#714), before anything is + fetched or written and under `--dry-run` as well. Iterated as `legs + or []`, `legs: 5` ended the run in a `TypeError`, and a scalar + `openXdox`, a mapping or `[5]` was refused `bump-leg-unknown-leg`, + "it declares" with nothing after it, and the remedy of a `--leg` + that was not wrong. A list holding a leg beside such an entry was + previewed, exit 0, and the real run refused `bump-leg-validators-red` + on that validator's `manifest-leg` finding: a bump that cannot land + is refused before the dry run says it would. Absent or null declares + no leg, which `leg` refuses `bump-leg-unknown-leg` as it did. + """ + declared = self.data.get("legs") + if declared is None: + return [] + if not isinstance(declared, list): + said = [f"manifest-legs: legs is {declared!r}, expected a list of " + "`role:`, `repository:` and `path:` mappings"] + else: + said = [f"manifest-leg: a leg is not a mapping: {row!r}" + for row in declared if not isinstance(row, dict)] + if said: + raise Refusal( + "bump-leg-manifest-legs", f"{self.path}: " + "; ".join(said), + "Remediation: each entry of `legs:` is one leg, a mapping " + "with `role:`, `repository:` and `path:`. The project's own " + "`scripts/validate-manifest.py` names this same fault; a leg " + "is not pinned around a manifest defect, so that comes " + "first, then this command again.") + return declared def leg(self, role: str) -> dict: # `assembly` IS declared, at path `.`, and is refused BEFORE the diff --git a/shape-doctor.py b/shape-doctor.py index b7099555..a0585379 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -1178,10 +1178,37 @@ def _read(path: Path): return None def legs(self) -> list[dict]: - """The non-assembly legs `project.yaml` declares, in its own order.""" - return [leg for leg in ((self.manifest or {}).get("legs") or []) + """The non-assembly legs `project.yaml` declares, in its own order. + + A `legs:` THAT IS NOT A LIST DECLARES NO LEG THIS CAN NAME (#832), as + `validate-manifest.py` reads it (#714), and an entry that is not a + mapping is no leg; `legs_unread` says each. Iterated as `legs or []`, + `legs: 5` ended the whole report in a `TypeError`, in `declared_pins` + under the `manifest kinds` row, and a scalar `openXdox` or a mapping + was walked one character, or one key, at a time, so the `legs` row + called the project one with no legs. + """ + declared = (self.manifest or {}).get("legs") + if not isinstance(declared, list): + return [] + return [leg for leg in declared if isinstance(leg, dict) and leg.get("role") != "assembly"] + def legs_unread(self) -> list[str]: + """What in `legs:` names no leg `legs` can read, each as + `validate-manifest.py`'s finding, in its words (#714, #832): a + `legs:` that is not a list, or each entry of one that is not a + mapping. Absent or null is neither: it declares no leg, and + `validate-manifest.py` says `legs is missing or empty` of it.""" + declared = (self.manifest or {}).get("legs") + if declared is None: + return [] + if not isinstance(declared, list): + return [f"manifest-legs: legs is {declared!r}, expected a list of " + "`role:`, `repository:` and `path:` mappings"] + return [f"manifest-leg: a leg is not a mapping: {leg!r}" + for leg in declared if not isinstance(leg, dict)] + def members(self) -> list[dict]: return [row for row in ((self.manifest or {}).get("members") or []) if isinstance(row, dict)] @@ -2163,6 +2190,35 @@ def waiting_row(ctx: Context, check_id: str, label: str, needs: str, {**(detail or {}), "waits_on": owner}) +def legs_unread_row(ctx: Context, check_id: str, label: str, needs: str, + detail: dict | None = None) -> Row | None: + """The `n/a` a row that reads the LEGS gives where `legs:` names none it + can read (#832), or None: where it names a leg, and where it names none + at all (absent, null or `[]`), which the row's own sentence is about. + + A `legs:` that is not a list, or a list in which no leg but the assembly + leg is a mapping and some entry is not one, is not a project with no + legs, which is what these rows said of `openXdox`, `{a: b}` and `[5]` + ("a one-repository project has none"): it is a manifest nobody can read + legs out of. ONE FAULT, ONE FINDING, as `waiting_row` reasons: the + project's own `validate-manifest.py` says the fault, and the `manifest` + row runs it, so this row is `n/a`, says the same sentence + (`Context.legs_unread`) and names that row in `waits_on`. A list that + holds a leg as well as an entry that is not one is checked leg by leg, + as `validate-pins.py` checks the rest past such an entry, and the entry + is left to the `manifest` row. + """ + unread = ctx.legs_unread() + if not unread or ctx.legs(): + return None + what = ("holds an entry that is not a mapping" + if isinstance(ctx.manifest.get("legs"), list) else "is not a list") + return Row(check_id, label, NA, + f"{ctx.manifest_path.name}'s `legs:` {what} " + f"({'; '.join(unread)}), so {needs}; the `manifest` row names " + "the fix", None, {**(detail or {}), "waits_on": "manifest"}) + + # --------------------------------------------------------------------------- # The checks # --------------------------------------------------------------------------- @@ -5605,7 +5661,10 @@ def check_legs(ctx: Context) -> Row: THE LEGS ARE THE MANIFEST'S, and a manifest that does not load declares none that this row can name -- which is not "declares no non-assembly - leg", the sentence below, about a project that has none (#205). + leg", the sentence below, about a project that has none (#205). Nor is + a `legs:` that is not a list, or one holding only entries that are not + mappings: `legs_unread_row` says which, in `validate-manifest.py`'s + words, and waits on the `manifest` row (#832). THE NEXT IS CHOSEN BY WHAT IS WRONG, AND IT CURES THE FIRST KIND (#276). `make bootstrap` was named for all of it and does not answer it: it @@ -5673,9 +5732,10 @@ def check_legs(ctx: Context) -> Row: an unborn HEAD or a leg with no url gets the same sentence it gets with no line at all, which ends by saying to run the doctor again. """ - waiting = waiting_row(ctx, "legs", "legs", - "the legs it declares are not known and none is " - "checked against its pin", {"legs": []}) + needs = ("the legs it declares are not known and none is checked against " + "its pin") + waiting = (waiting_row(ctx, "legs", "legs", needs, {"legs": []}) + or legs_unread_row(ctx, "legs", "legs", needs, {"legs": []})) if waiting is not None: return waiting legs = ctx.legs() @@ -6029,8 +6089,11 @@ def check_leg_shape_files(ctx: Context) -> Row: compare every leg, say what was found, and offer a copy for the files a copy would fix. """ - waiting = waiting_row(ctx, "leg-shape-files", LABEL_LEG_SHAPE_FILES, - "there are no legs to compare", {"legs": []}) + needs = "there are no legs to compare" + waiting = (waiting_row(ctx, "leg-shape-files", LABEL_LEG_SHAPE_FILES, + needs, {"legs": []}) + or legs_unread_row(ctx, "leg-shape-files", + LABEL_LEG_SHAPE_FILES, needs, {"legs": []})) if waiting is not None: return waiting legs = ctx.legs() @@ -7205,11 +7268,13 @@ def check_placement(ctx: Context) -> Row: "a family holder has no legs of its own: each member is an " "assembly root and answers this in its own report") # Before the sentence below, which is about a project that declares no - # leg, not one whose manifest nobody could read (#205). And `n/a` is what - # makes `--placement-plan` refuse by name and write nothing: there is no - # leg to write a plan about until the manifest loads. - waiting = waiting_row(ctx, "placement", "placement", - "there is no leg to classify a path in") + # leg, not one whose manifest nobody could read (#205), nor one whose + # `legs:` names no leg that can be read (#832). And `n/a` is what makes + # `--placement-plan` refuse by name and write nothing: there is no leg to + # write a plan about until the manifest loads and names one. + needs = "there is no leg to classify a path in" + waiting = (waiting_row(ctx, "placement", "placement", needs) + or legs_unread_row(ctx, "placement", "placement", needs)) if waiting is not None: return placement_row(NA, waiting.reason, None, waiting.detail) legs = ctx.legs() diff --git a/templates/assembly-root/scripts/bootstrap.py b/templates/assembly-root/scripts/bootstrap.py index f92f8047..e9ed1dde 100755 --- a/templates/assembly-root/scripts/bootstrap.py +++ b/templates/assembly-root/scripts/bootstrap.py @@ -1042,18 +1042,53 @@ def _load_root_and_manifest(args) -> tuple[Path, dict | None]: return root, manifest +def _legs_of(manifest: dict | None) -> tuple[list[dict], list[str]]: + """`(the legs, what names no leg)`: each entry of `project.yaml`'s + `legs:` that is a mapping, and for what is not, `validate-manifest.py`'s + finding in its words (#714, #832). + + Iterated as `legs or []`, `legs: 5` ended `make bootstrap` in a + `TypeError` before step (a) printed a line, and a scalar `openXdox` or a + mapping was walked one character, or one key, at a time, so step (a) + said "no submodule legs declared", as of a one-repository project. A + `legs:` that is not a list names no leg; a list's entry that is not a + mapping is no leg, and the mappings beside it are still placed, as + `validate-pins.py` checks the rest past such an entry. Absent or null + declares no leg and says nothing here: step (b)'s manifest validator + says `legs is missing or empty`. + """ + declared = (manifest or {}).get("legs") + if declared is None: + return [], [] + if not isinstance(declared, list): + return [], [f"manifest-legs: legs is {declared!r}, expected a list of " + "`role:`, `repository:` and `path:` mappings; no leg is " + "placed, and the validators below refuse until `legs:` " + "is a list"] + return ([leg for leg in declared if isinstance(leg, dict)], + [f"manifest-leg: a leg is not a mapping: {leg!r}; it is not " + "placed, and the validators below refuse until it is a mapping" + for leg in declared if not isinstance(leg, dict)]) + + def _checkout_all_tracking_branches(root: Path, sub_legs: list[dict], manifest: dict | None, - branch_arg: str | None) -> None: + branch_arg: str | None, + unread: list[str] = ()) -> None: """Step (a): place every submodule leg on its tracking branch. Split from `main` for #132. Git that cannot be started is `git-not-runnable`, naming the leg it met it at (#694): one fact about - the machine, so no leg after it is tried. + the machine, so no leg after it is tried. `unread` is what `_legs_of` + found naming no leg, each said first, and a step with nothing else to + place is not a one-repository project's no-op (#832). """ + for line in unread: + print(f" {line}") if not sub_legs: - print(" no submodule legs declared. A one-repository project runs " - "the same command and this step is a no-op.") + if not unread: + print(" no submodule legs declared. A one-repository project " + "runs the same command and this step is a no-op.") return branch = branch_arg or (manifest or {}).get("tracking_branch") or "main" for leg in sub_legs: @@ -1108,8 +1143,7 @@ def main(argv: list[str] | None = None) -> int: print(str(exc), file=sys.stderr) return 2 - legs = [leg for leg in ((manifest or {}).get("legs") or []) - if isinstance(leg, dict)] + legs, unread = _legs_of(manifest) sub_legs = [leg for leg in legs if leg.get("role") != "assembly"] name = (manifest or {}).get("name") or root.name @@ -1117,7 +1151,8 @@ def main(argv: list[str] | None = None) -> int: try: print("\n(a) legs on tracking branches, pins untouched") - _checkout_all_tracking_branches(root, sub_legs, manifest, args.branch) + _checkout_all_tracking_branches(root, sub_legs, manifest, args.branch, + unread) print("\n(b) neutral validators") failed = _run_neutral_validators(root, manifest, args.skip_validators) diff --git a/tests/test_legs_readers_not_a_list.py b/tests/test_legs_readers_not_a_list.py new file mode 100644 index 00000000..e154d776 --- /dev/null +++ b/tests/test_legs_readers_not_a_list.py @@ -0,0 +1,399 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Four more readers of a project manifest's `legs:` read it as the three +gates read it since #714 (#826), and say #714's sentence for it (#832). + +EACH ITERATED `legs or []`: the doctor's `Context.legs`, the project's own +`scripts/bootstrap.py`, `bump-leg.py`'s `Project.legs` and `update-shape.py`'s +`pull_request_repository`. So `legs: 5` ended each in `TypeError: 'int' +object is not iterable` -- the doctor's whole report, in `declared_pins` +under its `manifest kinds` row -- and a scalar `openXdox`, a mapping `{a: b}` +or a list `[5]` was walked one character, one key or one entry at a time and +read as a project with no legs: the doctor's `legs` row said "a one-repository +project has none", bootstrap's step (a) "no submodule legs declared", +`bump-leg.py` refused `bump-leg-unknown-leg` with "it declares" and nothing +after it, and `update-shape.py`'s re-run `--pr` "declares no assembly leg". +NOW each says what it found in #714's words, the value spelled as the gates +spell it (`repr`): + + manifest-legs: legs is , expected a list of `role:`, + `repository:` and `path:` mappings + manifest-leg: a leg is not a mapping: + +- the doctor's `legs`, `leg shape files` and `placement` rows are `n/a`, say + the sentence and wait on the `manifest` row (`waits_on`), as a row waits on + a manifest that does not load (#205), and `--placement-plan` is refused by + name with nothing written; a list holding legs beside such an entry still + has its legs checked; +- bootstrap's step (a) says it, places no leg for it and places the legs + beside it, and step (b)'s validators refuse; +- `bump-leg.py` refuses `bump-leg-manifest-legs`, exit 2, in both runs and + before anything is fetched, a list holding legs beside such an entry + included: its own `validate-manifest.py` refuses that list, so the bump + could never land; +- `update-shape.py`'s `update-pr-no-repository` says it, where the re-run + `--pr` recognises crashed. + +THE GATES' OWN WORDS ARE #826's. A tree without it still has the naming and +pins gates crash on `5` and `validate-manifest.py` call it "missing or +empty", so this file asserts what the four readers say, from one table, and +nothing the gates print beside them; #826's own tests hold the gates to the +same sentence. + +THE HARNESS: conftest's `project` for bootstrap and `bump-leg.py`, +`tests/test_shape_doctor.py`'s session standard for the doctor, +`tests/test_update_shape.py`'s upstream for the re-run, and +`tests/test_update_shape_pr_base.py`'s in-process `update-shape.py`. +""" + +from __future__ import annotations + +import json +import re +import shutil +from types import SimpleNamespace + +import pytest + +from conftest import git, run_script +from test_bump_leg import bump +from test_manifest_validator import edit +from test_non_list_project_fields import doctored, spelled_as # noqa: F401 +from test_shape_doctor import ( # noqa: F401 + doctor, rows_of, scaffolded_here, standard, +) +from test_update_shape import apply, upstream_and_project # noqa: F401 +from test_update_shape_pr_base import ( # noqa: F401 + gh_calls, update_shape, without_its_words, +) + +#: The scaffold writes `legs:` as the manifest's last block. +LEGS_BLOCK = "\nlegs:\n" + +#: What #714 says after a `legs:` value that is not a list. +LEGS_TAIL = "expected a list of `role:`, `repository:` and `path:` mappings" + +#: `(YAML spelling, the sentence said)`: the issue's four, two more for the +#: entry rule, and a value holding ESC. The value is spelled as the gates +#: spell it, by `repr`, so no control character reaches a terminal raw. +CASES = [ + pytest.param("5", f"manifest-legs: legs is 5, {LEGS_TAIL}", id="number"), + pytest.param("openXdox", f"manifest-legs: legs is 'openXdox', {LEGS_TAIL}", + id="scalar-string"), + pytest.param("{a: b}", f"manifest-legs: legs is {{'a': 'b'}}, {LEGS_TAIL}", + id="mapping"), + pytest.param("[5]", "manifest-leg: a leg is not a mapping: 5", + id="list-number"), + pytest.param("[openXdox]", + "manifest-leg: a leg is not a mapping: 'openXdox'", + id="list-string"), + pytest.param("[null]", "manifest-leg: a leg is not a mapping: None", + id="list-null"), + pytest.param('"\\x1b[31mRED"', + f"manifest-legs: legs is '\\x1b[31mRED', {LEGS_TAIL}", + id="esc-string"), +] +#: The issue's own four. +ISSUE_CASES = CASES[:4] + +#: The entry the mixed case appends after the scaffold's three legs. +MIXED_ENTRY = " - 5\n" +MIXED_SAID = "manifest-leg: a leg is not a mapping: 5" + + +def is_a_list(said: str) -> bool: + """Is the sentence about an entry of a list, not about `legs:`?""" + return said.startswith("manifest-leg:") + + +def legs_are(project, spelled: str) -> None: + """`legs: ` in place of the scaffold's legs, the last block.""" + text = (project / "project.yaml").read_text() + edit(project, text[text.index(LEGS_BLOCK):], + "\n" + spelled_as("legs", spelled) + "\n") + + +def legs_and_an_entry_that_is_not_one(project) -> None: + """The scaffold's three legs, then an entry `5`.""" + text = (project / "project.yaml").read_text() + block = text[text.index(LEGS_BLOCK):] + edit(project, block, block + MIXED_ENTRY) + + +def no_traceback(result) -> None: + """No traceback, and no ESC: a value is spelled where it is said.""" + out = result.stdout + result.stderr + assert "Traceback" not in out, out + assert "\x1b" not in out, out + + +# --- the doctor -------------------------------------------------------------- + +#: The rows that read `Context.legs`, and what each says it cannot do. +LEG_ROWS = { + "legs": "the legs it declares are not known and none is checked against " + "its pin", + "leg-shape-files": "there are no legs to compare", + "placement": "there is no leg to classify a path in", +} + + +def waiting_reason(said: str, needs: str) -> str: + what = ("holds an entry that is not a mapping" if is_a_list(said) + else "is not a list") + return (f"project.yaml's `legs:` {what} ({said}), so {needs}; the " + "`manifest` row names the fix") + + +@pytest.mark.parametrize("spelled,said", CASES) +def test_the_doctors_rows_that_read_the_legs_say_the_sentence_and_wait( + standard, doctored, spelled, said): + """`5` ended the report in a `TypeError`; the others made the `legs` row + say the project is a one-repository one. Each row that reads the legs is + now `n/a`, says the sentence, and names the `manifest` row it waits on, + which is a FINDING; the row that crashed answers.""" + legs_are(doctored, spelled) + result = doctor(standard, doctored, "--json") + no_traceback(result) + payload = json.loads(result.stdout) + # A finding's exit. For `5` the standard's own naming gate crashes until + # #826 lands, which is CANNOT ANSWER about this checkout: never this row. + assert payload["exit"] == result.returncode, payload + assert result.returncode in ({1, 3} if spelled == "5" else {1}), payload + rows = rows_of(result) + assert rows["manifest-kinds"]["status"] == "ok", rows["manifest-kinds"] + assert rows["manifest"]["status"] == "FINDING", rows["manifest"] + for row_id, needs in LEG_ROWS.items(): + row = rows[row_id] + assert row["status"] == "n/a", row + assert row["reason"] == waiting_reason(said, needs), row + assert row["next"] is None, row + assert row["detail"]["waits_on"] == "manifest", row + assert row["detail"]["legs"] == [], row + assert "one-repository project has none" not in result.stdout + assert "declares no non-assembly leg" not in result.stdout + + +@pytest.mark.parametrize("spelled,said", ISSUE_CASES) +def test_a_placement_plan_is_refused_by_name_and_nothing_is_written( + standard, doctored, tmp_path, spelled, said): + """`--placement-plan` refuses an `n/a` placement row by name, exit 3, and + writes nothing: it says the row's sentence, not "declares no leg".""" + legs_are(doctored, spelled) + plan = tmp_path / "plan.yaml" + result = doctor(standard, doctored, "--placement-plan", str(plan)) + no_traceback(result) + assert result.returncode == 3, result.stdout + result.stderr + first = result.stderr.splitlines()[0] + assert first.startswith("REFUSED shape-doctor-no-placement-audit: "), first + assert first.endswith( + " has no placement audit to write a plan from: " + + waiting_reason(said, LEG_ROWS["placement"])), first + assert not plan.exists() + + +def test_the_doctor_checks_the_legs_beside_an_entry_that_is_not_one( + standard, doctored): + """THE OTHER SIDE: where the list holds legs too, the rows check them, as + `validate-pins.py` checks the rest past such an entry, and the entry is + the `manifest` row's alone.""" + legs_and_an_entry_that_is_not_one(doctored) + result = doctor(standard, doctored, "--json") + no_traceback(result) + rows = rows_of(result) + assert rows["manifest"]["status"] == "FINDING", rows["manifest"] + assert MIXED_SAID in rows["manifest"]["reason"], rows["manifest"] + legs = rows["legs"] + assert legs["status"] == "ok", legs + assert legs["reason"] == "2 leg(s) mounted and checked out at their pins" + assert [leg["role"] for leg in legs["detail"]["legs"]] == ["spec", "code"] + for row_id in LEG_ROWS: + assert "waits_on" not in rows[row_id]["detail"], rows[row_id] + + +# --- bootstrap.py, the project's own copy ------------------------------------ + +STEP_A = "\n(a) legs on tracking branches, pins untouched\n" +STEP_B = "\n(b) neutral validators\n" + + +def step_a(stdout: str) -> list[str]: + """The lines step (a) printed, whole.""" + _, opened, rest = stdout.partition(STEP_A) + assert opened, stdout + lines, closed, _ = rest.partition(STEP_B) + assert closed, stdout + return lines.splitlines() + + +def step_a_says(said: str) -> str: + if is_a_list(said): + return (f" {said}; it is not placed, and the validators below " + "refuse until it is a mapping") + return (f" {said}; no leg is placed, and the validators below refuse " + "until `legs:` is a list") + + +def bootstrap(project, *args: str): + return run_script(project / "scripts" / "bootstrap.py", *args, + cwd=project) + + +@pytest.mark.parametrize("spelled,said", CASES) +def test_bootstraps_step_a_says_the_sentence_and_places_no_leg(project, + spelled, said): + """`5` ended `make bootstrap` in a `TypeError` before step (a); the others + made step (a) say "no submodule legs declared", as of a one-repository + project. Step (a) is now the sentence alone. `--skip-validators`, so + nothing a gate prints is read.""" + legs_are(project, spelled) + result = bootstrap(project, "--skip-validators") + no_traceback(result) + assert result.returncode == 0, result.stdout + result.stderr + assert step_a(result.stdout) == [step_a_says(said)], result.stdout + assert "no submodule legs declared" not in result.stdout + + +#: Each gate's exit for these four: 1 with #826, which says the sentence, and +#: 2 or a traceback's 1 without it. Bootstrap reports each as it ends. +FAILED = re.compile(r"bootstrap FAILED: naming \(exit [12]\), manifest " + r"\(exit 1\), pins \(exit [12]\)") + + +@pytest.mark.parametrize("spelled,said", ISSUE_CASES) +def test_bootstrap_fails_on_its_validators_after_saying_the_sentence( + project, spelled, said): + """With the validators: step (a) says the sentence, step (b)'s gates + refuse, and bootstrap fails, exit 1, with no traceback of its own.""" + legs_are(project, spelled) + result = bootstrap(project) + assert result.returncode == 1, result.stdout + result.stderr + assert step_a(result.stdout) == [step_a_says(said)], result.stdout + assert 'bootstrap.py", line' not in result.stderr, result.stderr + last = result.stderr.splitlines()[-1] + assert FAILED.fullmatch(last), last + + +def test_bootstrap_places_the_legs_beside_an_entry_that_is_not_one(project): + """THE OTHER SIDE: the entry is said first, and the two legs beside it + are placed as ever.""" + legs_and_an_entry_that_is_not_one(project) + result = bootstrap(project, "--skip-validators") + no_traceback(result) + assert result.returncode == 0, result.stdout + result.stderr + said, spec, code = step_a(result.stdout) + assert said == step_a_says(MIXED_SAID) + assert spec.startswith(" [spec] spec: "), spec + assert code.startswith(" [code] code: "), code + + +# --- bump-leg.py ------------------------------------------------------------- + +BUMP_REMEDY = ( + "Remediation: each entry of `legs:` is one leg, a mapping with `role:`, " + "`repository:` and `path:`. The project's own " + "`scripts/validate-manifest.py` names this same fault; a leg is not " + "pinned around a manifest defect, so that comes first, then this " + "command again.\n") + +RUNS = [pytest.param(("--dry-run",), id="dry-run"), + pytest.param((), id="real-run")] + + +def assert_bump_refused(project, scaffolded, said: str, extra) -> None: + """`bump-leg-manifest-legs`, exit 2, before anything is fetched or + written: only the manifest edit the test made is in `git status`.""" + to = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + result = bump(project, scaffolded, to, *extra) + no_traceback(result) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "", result.stdout + manifest = project.resolve() / "project.yaml" + assert result.stderr == (f"REFUSED bump-leg-manifest-legs: {manifest}: " + f"{said}\n{BUMP_REMEDY}"), result.stderr + status = git("status", "--porcelain", cwd=project).stdout + assert status.splitlines() == [" M project.yaml"], status + + +@pytest.mark.parametrize("extra", RUNS) +@pytest.mark.parametrize("spelled,said", CASES) +def test_bump_leg_refuses_a_legs_it_cannot_read_by_name_in_both_runs( + project, scaffolded, spelled, said, extra): + """`5` was a `TypeError`; the others were `bump-leg-unknown-leg`, "it + declares" with nothing after it, and the remedy of a `--leg` that was + not wrong.""" + legs_are(project, spelled) + assert_bump_refused(project, scaffolded, said, extra) + + +@pytest.mark.parametrize("extra", RUNS) +def test_bump_leg_refuses_legs_beside_an_entry_that_is_not_one( + project, scaffolded, extra): + """The dry run previewed the move, exit 0, and the real run was refused + `bump-leg-validators-red` on `validate-manifest.py`'s `manifest-leg` + finding and rolled back: both runs are now refused first, alike.""" + legs_and_an_entry_that_is_not_one(project) + assert_bump_refused(project, scaffolded, MIXED_SAID, extra) + + +# --- update-shape.py --------------------------------------------------------- + +def why_none(said: str) -> str: + """`pull_request_repository`'s `why_none` for the sentence.""" + if is_a_list(said): + return ("project.yaml declares no assembly leg, so there is no " + f"repository to open a pull request against: {said}") + return ("project.yaml's `legs:` is not a list, so it names no assembly " + "leg and there is no repository to open a pull request against: " + f"{said}") + + +@pytest.mark.parametrize("spelled,said", CASES) +def test_update_shape_names_no_repository_in_the_sentence( + update_shape, tmp_path, capsys, gh_calls, spelled, said): + """`--pr` refuses `update-pr-no-repository` in the sentence's words, no + `gh` is asked, and the `NEXT` line keeps the placeholder it prints for + any manifest naming no repository.""" + (tmp_path / "project.yaml").write_text( + spelled_as("legs", spelled) + "\n", encoding="utf-8") + manifest = update_shape.load_yaml(tmp_path / "project.yaml") + assert update_shape.pull_request_repository(manifest) == ( + "", why_none(said)) + with pytest.raises(update_shape.Refusal) as raised: + update_shape.pull_request_target(tmp_path, update_shape.PROJECT_KIND) + assert str(raised.value) == f"REFUSED update-pr-no-repository: " \ + f"{why_none(said)}" + assert gh_calls == [] + args = SimpleNamespace(branch="shape/x", push=True, pr=False) + update_shape.next_line(tmp_path, args, [], "0" * 40, [], + update_shape.PROJECT_KIND) + assert without_its_words(capsys.readouterr().out.strip()) == ( + "NEXT gh pr create --repo / --base main --head shape/x") + + +def test_update_shape_reads_the_assembly_leg_beside_an_entry_that_is_not_one( + update_shape): + """THE OTHER SIDE: the assembly leg's repository is read past such an + entry; the validators `apply` runs before any landing say it.""" + manifest = {"legs": [5, {"role": "assembly", + "repository": "acme/Atlas"}]} + assert update_shape.pull_request_repository(manifest)[0] == "acme/Atlas" + + +@pytest.mark.parametrize("spelled,said", ISSUE_CASES) +def test_the_rerun_that_opens_the_missing_pull_request_refuses_by_name( + upstream_and_project, tmp_path, spelled, said): + """THE PATH THAT CRASHED: the re-run `--pr` recognises, with nothing to + write, reads the manifest's repository before `gh` is asked. `5` ended + it in a `TypeError`; now each value is refused in the sentence.""" + root = tmp_path / "Atlas" + shutil.copytree(upstream_and_project["clone"], root, symlinks=True) + first = apply(root, upstream_and_project, "--branch", "shape/x") + assert first.returncode == 0, first.stdout + first.stderr + legs_are(root, spelled) + git("commit", "-q", "-m", "legs", "--", "project.yaml", cwd=root) + result = apply(root, upstream_and_project, "--branch", "shape/x", "--pr") + no_traceback(result) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout.startswith("nothing to write: "), result.stdout + assert result.stderr == (f"REFUSED update-pr-no-repository: " + f"{why_none(said)}\n"), result.stderr diff --git a/update-shape.py b/update-shape.py index 63154d3e..411bbc29 100755 --- a/update-shape.py +++ b/update-shape.py @@ -2489,6 +2489,17 @@ def pull_request_repository(manifest: dict | None, after the commit and a manifest that names no repository is no reason to take that back; and `pull_request_line`'s bare form, the one a failed `--push` prints, leaves `--repo` off (#204). + + A `legs:` THAT IS NOT A LIST NAMES NO ASSEMBLY LEG (#832), as + `validate-manifest.py` reads it (#714), and an entry that is not a + mapping is no leg. Iterated as `legs or []`, `legs: 5` ended the re-run + `--pr` recognises (`_finish_landing`) in a `TypeError`, and a scalar + `openXdox` or a mapping was refused "declares no assembly leg", as of a + list of legs that leaves that one out. `why_none` now says what names + no leg, in that validator's words: the `manifest-legs` sentence, or + after "declares no assembly leg" each `manifest-leg` one. An entry that + is not a mapping beside the assembly leg does not stop its repository + being read; the validators `apply` runs before any landing say it. """ manifest = manifest or {} if kind is FAMILY_KIND: @@ -2501,11 +2512,22 @@ def pull_request_repository(manifest: dict | None, f"{kind.manifest} declares neither `repository:` nor an " "`org:`/`name:` pair, so there is no repository to open a " "pull request against") - legs = [leg for leg in manifest.get("legs") or [] + declared = manifest.get("legs") + if declared is not None and not isinstance(declared, list): + return "", ( + "project.yaml's `legs:` is not a list, so it names no assembly " + "leg and there is no repository to open a pull request against: " + f"manifest-legs: legs is {declared!r}, expected a list of " + "`role:`, `repository:` and `path:` mappings") + legs = [leg for leg in declared or [] if isinstance(leg, dict) and leg.get("role") == "assembly"] + unread = [] if legs else [f"manifest-leg: a leg is not a mapping: {leg!r}" + for leg in declared or [] + if not isinstance(leg, dict)] return (str(legs[0].get("repository") or "") if legs else ""), ( "project.yaml declares no assembly leg, so there is no " - "repository to open a pull request against") + "repository to open a pull request against" + + (f": {'; '.join(unread)}" if unread else "")) def commit_on_branch(root: Path, branch: str, paths: list[str], target: str, From bc83ac87f5286abe904c2daebfd757fcf4d1ec3f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 02:14:35 +0000 Subject: [PATCH 2/3] #862 round 1: absent, null and `[]` are held to each reader's old sentence, `[5, 6]` pins the `; ` join, `why_none` is asserted whole, the doctor's leg rows say the `manifest` row's command names the fault, and `pull_request_repository` drops a needless ternary (#832) RV862 (lane 5, #862 issuecomment-6242849328) found the code right and the tests short of it: - SHOULD-1: the `None` guards in the doctor's `Context.legs_unread`, `bump-leg.py`'s `Project.legs`, `update-shape.py`'s `pull_request_repository` and bootstrap's `_legs_of` could each be deleted with the new file still green, and an absent or null `legs:` would then read as `legs is None`. `NO_LEGS` (absent, null, `[]`) now holds the doctor's three rows to their old `n/a` sentences with no `waits_on`, bootstrap's step (a) to "no submodule legs declared", `bump-leg.py` to `bump-leg-unknown-leg` word for word in both runs, and `update-shape.py` to "declares no assembly leg" with nothing after it. - NIT-1: `[5, 6]` joins `CASES`, so each reader's `; ` join is pinned. - NIT-2: `why_none` and the `--pr` refusal are asserted whole for a list holding only a spec leg, so no stray `: ` can follow the sentence. - NIT-3: the doctor's three rows end "the `manifest` row's command names the fault": for a list, that row quotes the validator's first finding, and its command's output names the entry. - NIT-4: `pull_request_repository` names the entries that are not mappings whenever there are some, not only where no assembly leg is read; no caller reads `why_none` once a repository is. The one sentence that changes is an assembly leg with no `repository:` beside such an entry, which now names it, and a row pins it. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 5 +- tests/test_legs_readers_not_a_list.py | 218 +++++++++++++++++++++----- update-shape.py | 13 +- 3 files changed, 186 insertions(+), 50 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index a0585379..53a3e875 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2215,8 +2215,9 @@ def legs_unread_row(ctx: Context, check_id: str, label: str, needs: str, if isinstance(ctx.manifest.get("legs"), list) else "is not a list") return Row(check_id, label, NA, f"{ctx.manifest_path.name}'s `legs:` {what} " - f"({'; '.join(unread)}), so {needs}; the `manifest` row names " - "the fix", None, {**(detail or {}), "waits_on": "manifest"}) + f"({'; '.join(unread)}), so {needs}; the `manifest` row's " + "command names the fault", None, + {**(detail or {}), "waits_on": "manifest"}) # --------------------------------------------------------------------------- diff --git a/tests/test_legs_readers_not_a_list.py b/tests/test_legs_readers_not_a_list.py index e154d776..8b9d0673 100644 --- a/tests/test_legs_readers_not_a_list.py +++ b/tests/test_legs_readers_not_a_list.py @@ -33,6 +33,14 @@ - `update-shape.py`'s `update-pr-no-repository` says it, where the re-run `--pr` recognises crashed. +Two entries that are not mappings are said together, joined by `; `, in each +reader that says them on one line (`[5, 6]`, RV862's NIT-1). + +A MANIFEST THAT NAMES NO LEG -- `legs:` absent, null or `[]` -- IS LEFT AS IT +WAS, and `NO_LEGS` holds each reader to its old sentence word for word, so the +`None` guard each reader now has cannot go unseen (RV862's SHOULD-1): without +it, absent and null read as the `legs is None` mismatch #832 removes. + THE GATES' OWN WORDS ARE #826's. A tree without it still has the naming and pins gates crash on `5` and `validate-manifest.py` call it "missing or empty", so this file asserts what the four readers say, from one table, and @@ -72,44 +80,60 @@ #: What #714 says after a `legs:` value that is not a list. LEGS_TAIL = "expected a list of `role:`, `repository:` and `path:` mappings" -#: `(YAML spelling, the sentence said)`: the issue's four, two more for the +#: What #714 says of each entry of a list that is not a mapping. +NOT_A_MAPPING = "manifest-leg: a leg is not a mapping: " + +#: `(YAML spelling, the sentences said)`: the issue's four, three more for the #: entry rule, and a value holding ESC. The value is spelled as the gates #: spell it, by `repr`, so no control character reaches a terminal raw. CASES = [ - pytest.param("5", f"manifest-legs: legs is 5, {LEGS_TAIL}", id="number"), - pytest.param("openXdox", f"manifest-legs: legs is 'openXdox', {LEGS_TAIL}", + pytest.param("5", (f"manifest-legs: legs is 5, {LEGS_TAIL}",), + id="number"), + pytest.param("openXdox", + (f"manifest-legs: legs is 'openXdox', {LEGS_TAIL}",), id="scalar-string"), - pytest.param("{a: b}", f"manifest-legs: legs is {{'a': 'b'}}, {LEGS_TAIL}", + pytest.param("{a: b}", + (f"manifest-legs: legs is {{'a': 'b'}}, {LEGS_TAIL}",), id="mapping"), - pytest.param("[5]", "manifest-leg: a leg is not a mapping: 5", - id="list-number"), - pytest.param("[openXdox]", - "manifest-leg: a leg is not a mapping: 'openXdox'", + pytest.param("[5]", (f"{NOT_A_MAPPING}5",), id="list-number"), + pytest.param("[openXdox]", (f"{NOT_A_MAPPING}'openXdox'",), id="list-string"), - pytest.param("[null]", "manifest-leg: a leg is not a mapping: None", - id="list-null"), + pytest.param("[null]", (f"{NOT_A_MAPPING}None",), id="list-null"), + pytest.param("[5, 6]", (f"{NOT_A_MAPPING}5", f"{NOT_A_MAPPING}6"), + id="list-two-entries"), pytest.param('"\\x1b[31mRED"', - f"manifest-legs: legs is '\\x1b[31mRED', {LEGS_TAIL}", + (f"manifest-legs: legs is '\\x1b[31mRED', {LEGS_TAIL}",), id="esc-string"), ] #: The issue's own four. ISSUE_CASES = CASES[:4] +#: `legs:` absent (no key), null and `[]`: a manifest that names no leg, which +#: #832 leaves as it was and each reader still says in its old words. +NO_LEGS = [pytest.param(None, id="absent"), pytest.param("", id="null"), + pytest.param("[]", id="empty-list")] + #: The entry the mixed case appends after the scaffold's three legs. MIXED_ENTRY = " - 5\n" -MIXED_SAID = "manifest-leg: a leg is not a mapping: 5" +MIXED_SAID = (f"{NOT_A_MAPPING}5",) + + +def is_a_list(said: tuple) -> bool: + """Are the sentences about entries of a list, not about `legs:`?""" + return said[0].startswith(NOT_A_MAPPING) -def is_a_list(said: str) -> bool: - """Is the sentence about an entry of a list, not about `legs:`?""" - return said.startswith("manifest-leg:") +def joined(said: tuple) -> str: + """The sentences as a reader says them on one line.""" + return "; ".join(said) -def legs_are(project, spelled: str) -> None: - """`legs: ` in place of the scaffold's legs, the last block.""" +def legs_are(project, spelled: str | None) -> None: + """`legs: ` in place of the scaffold's legs, the last block, or + no `legs:` key at all for None.""" text = (project / "project.yaml").read_text() - edit(project, text[text.index(LEGS_BLOCK):], - "\n" + spelled_as("legs", spelled) + "\n") + after = "" if spelled is None else spelled_as("legs", spelled) + "\n" + edit(project, text[text.index(LEGS_BLOCK):], "\n" + after) def legs_and_an_entry_that_is_not_one(project) -> None: @@ -137,11 +161,22 @@ def no_traceback(result) -> None: } -def waiting_reason(said: str, needs: str) -> str: +#: The rows that read `Context.legs`, and what each said, and still says, of +#: a manifest that names no leg: an `n/a` that waits on no row. +NO_LEG_ROWS = { + "legs": "project.yaml declares no non-assembly leg; a one-repository " + "project has none and is not less governed for it", + "leg-shape-files": "no legs to compare", + "placement": "project.yaml declares no non-assembly leg, so there is no " + "leg for a path to be in the wrong one of", +} + + +def waiting_reason(said: tuple, needs: str) -> str: what = ("holds an entry that is not a mapping" if is_a_list(said) else "is not a list") - return (f"project.yaml's `legs:` {what} ({said}), so {needs}; the " - "`manifest` row names the fix") + return (f"project.yaml's `legs:` {what} ({joined(said)}), so {needs}; " + "the `manifest` row's command names the fault") @pytest.mark.parametrize("spelled,said", CASES) @@ -201,7 +236,7 @@ def test_the_doctor_checks_the_legs_beside_an_entry_that_is_not_one( no_traceback(result) rows = rows_of(result) assert rows["manifest"]["status"] == "FINDING", rows["manifest"] - assert MIXED_SAID in rows["manifest"]["reason"], rows["manifest"] + assert MIXED_SAID[0] in rows["manifest"]["reason"], rows["manifest"] legs = rows["legs"] assert legs["status"] == "ok", legs assert legs["reason"] == "2 leg(s) mounted and checked out at their pins" @@ -210,6 +245,26 @@ def test_the_doctor_checks_the_legs_beside_an_entry_that_is_not_one( assert "waits_on" not in rows[row_id]["detail"], rows[row_id] +@pytest.mark.parametrize("spelled", NO_LEGS) +def test_the_doctor_says_a_manifest_naming_no_leg_as_it_did(standard, + doctored, + spelled): + """THE OTHER SIDE (RV862's SHOULD-1): absent, null and `[]` keep the rows' + old sentences, waiting on no row; `validate-manifest.py`, under the + `manifest` row, says `legs is missing or empty` of each.""" + legs_are(doctored, spelled) + result = doctor(standard, doctored, "--json") + no_traceback(result) + assert result.returncode == 1, result.stdout + result.stderr + rows = rows_of(result) + assert rows["manifest"]["status"] == "FINDING", rows["manifest"] + for row_id, said in NO_LEG_ROWS.items(): + row = rows[row_id] + assert row["status"] == "n/a", row + assert row["reason"] == said, row + assert "waits_on" not in row["detail"], row + + # --- bootstrap.py, the project's own copy ------------------------------------ STEP_A = "\n(a) legs on tracking branches, pins untouched\n" @@ -225,12 +280,18 @@ def step_a(stdout: str) -> list[str]: return lines.splitlines() -def step_a_says(said: str) -> str: +def step_a_says(said: tuple) -> list[str]: + """Step (a)'s lines for the sentences: one for each.""" if is_a_list(said): - return (f" {said}; it is not placed, and the validators below " - "refuse until it is a mapping") - return (f" {said}; no leg is placed, and the validators below refuse " - "until `legs:` is a list") + return [f" {one}; it is not placed, and the validators below refuse " + "until it is a mapping" for one in said] + return [f" {one}; no leg is placed, and the validators below refuse " + "until `legs:` is a list" for one in said] + + +#: Step (a)'s one line over a manifest that names no leg, as ever. +NO_SUBMODULE_LEGS = (" no submodule legs declared. A one-repository project " + "runs the same command and this step is a no-op.") def bootstrap(project, *args: str): @@ -249,7 +310,7 @@ def test_bootstraps_step_a_says_the_sentence_and_places_no_leg(project, result = bootstrap(project, "--skip-validators") no_traceback(result) assert result.returncode == 0, result.stdout + result.stderr - assert step_a(result.stdout) == [step_a_says(said)], result.stdout + assert step_a(result.stdout) == step_a_says(said), result.stdout assert "no submodule legs declared" not in result.stdout @@ -267,7 +328,7 @@ def test_bootstrap_fails_on_its_validators_after_saying_the_sentence( legs_are(project, spelled) result = bootstrap(project) assert result.returncode == 1, result.stdout + result.stderr - assert step_a(result.stdout) == [step_a_says(said)], result.stdout + assert step_a(result.stdout) == step_a_says(said), result.stdout assert 'bootstrap.py", line' not in result.stderr, result.stderr last = result.stderr.splitlines()[-1] assert FAILED.fullmatch(last), last @@ -281,11 +342,23 @@ def test_bootstrap_places_the_legs_beside_an_entry_that_is_not_one(project): no_traceback(result) assert result.returncode == 0, result.stdout + result.stderr said, spec, code = step_a(result.stdout) - assert said == step_a_says(MIXED_SAID) + assert [said] == step_a_says(MIXED_SAID) assert spec.startswith(" [spec] spec: "), spec assert code.startswith(" [code] code: "), code +@pytest.mark.parametrize("spelled", NO_LEGS) +def test_bootstrap_says_a_manifest_naming_no_leg_as_it_did(project, spelled): + """THE OTHER SIDE (RV862's SHOULD-1): absent, null and `[]` are step (a)'s + no-op, word for word, as a one-repository project's is; step (b)'s + manifest validator says `legs is missing or empty`.""" + legs_are(project, spelled) + result = bootstrap(project, "--skip-validators") + no_traceback(result) + assert result.returncode == 0, result.stdout + result.stderr + assert step_a(result.stdout) == [NO_SUBMODULE_LEGS], result.stdout + + # --- bump-leg.py ------------------------------------------------------------- BUMP_REMEDY = ( @@ -299,19 +372,32 @@ def test_bootstrap_places_the_legs_beside_an_entry_that_is_not_one(project): pytest.param((), id="real-run")] -def assert_bump_refused(project, scaffolded, said: str, extra) -> None: - """`bump-leg-manifest-legs`, exit 2, before anything is fetched or - written: only the manifest edit the test made is in `git status`.""" +#: `bump-leg-unknown-leg`'s remedy, which a manifest naming no leg still gets. +UNKNOWN_LEG_REMEDY = ( + "Remediation: --leg takes a role this project's own manifest names. A " + "leg that is missing from the manifest is a manifest defect, not " + "something to pin around.\n") + + +def refused_bump(project, scaffolded, extra) -> tuple[str, object]: + """`(stderr, the manifest's path)` of a bump refused, exit 2, before + anything is fetched or written: only the manifest edit the test made is + in `git status`.""" to = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() result = bump(project, scaffolded, to, *extra) no_traceback(result) assert result.returncode == 2, result.stdout + result.stderr assert result.stdout == "", result.stdout - manifest = project.resolve() / "project.yaml" - assert result.stderr == (f"REFUSED bump-leg-manifest-legs: {manifest}: " - f"{said}\n{BUMP_REMEDY}"), result.stderr status = git("status", "--porcelain", cwd=project).stdout assert status.splitlines() == [" M project.yaml"], status + return result.stderr, project.resolve() / "project.yaml" + + +def assert_bump_refused(project, scaffolded, said: tuple, extra) -> None: + """`bump-leg-manifest-legs`, naming every sentence on one line.""" + stderr, manifest = refused_bump(project, scaffolded, extra) + assert stderr == (f"REFUSED bump-leg-manifest-legs: {manifest}: " + f"{joined(said)}\n{BUMP_REMEDY}"), stderr @pytest.mark.parametrize("extra", RUNS) @@ -335,16 +421,64 @@ def test_bump_leg_refuses_legs_beside_an_entry_that_is_not_one( assert_bump_refused(project, scaffolded, MIXED_SAID, extra) +@pytest.mark.parametrize("extra", RUNS) +@pytest.mark.parametrize("spelled", NO_LEGS) +def test_bump_leg_refuses_a_manifest_naming_no_leg_as_it_did( + project, scaffolded, spelled, extra): + """THE OTHER SIDE (RV862's SHOULD-1): absent, null and `[]` are still + `bump-leg-unknown-leg`, word for word, "it declares" with nothing after + it included; `validate-manifest.py` says `legs is missing or empty`.""" + legs_are(project, spelled) + stderr, manifest = refused_bump(project, scaffolded, extra) + assert stderr == (f"REFUSED bump-leg-unknown-leg: {manifest} declares no " + "leg with role 'spec'; it declares \n" + f"{UNKNOWN_LEG_REMEDY}"), stderr + + # --- update-shape.py --------------------------------------------------------- -def why_none(said: str) -> str: - """`pull_request_repository`'s `why_none` for the sentence.""" +#: `pull_request_repository`'s `why_none` where no assembly leg is read. +NO_ASSEMBLY_LEG = ("project.yaml declares no assembly leg, so there is no " + "repository to open a pull request against") + + +def why_none(said: tuple) -> str: + """`pull_request_repository`'s `why_none` for the sentences.""" if is_a_list(said): - return ("project.yaml declares no assembly leg, so there is no " - f"repository to open a pull request against: {said}") + return f"{NO_ASSEMBLY_LEG}: {joined(said)}" return ("project.yaml's `legs:` is not a list, so it names no assembly " "leg and there is no repository to open a pull request against: " - f"{said}") + f"{joined(said)}") + + +#: `(project.yaml, why_none)` where no repository is read and every entry +#: is a mapping, or none is there: the old sentence, whole, with nothing +#: after it (RV862's SHOULD-1 and NIT-2). Then an assembly leg with no +#: `repository:` beside an entry that is not a mapping, which names it. +NO_REPOSITORY = [ + pytest.param("name: Atlas\n", NO_ASSEMBLY_LEG, id="absent"), + pytest.param("legs:\n", NO_ASSEMBLY_LEG, id="null"), + pytest.param("legs: []\n", NO_ASSEMBLY_LEG, id="empty-list"), + pytest.param("legs:\n - role: spec\n repository: acme/AtlasSpec\n", + NO_ASSEMBLY_LEG, id="spec-leg-only"), + pytest.param("legs:\n - 5\n - role: assembly\n", + f"{NO_ASSEMBLY_LEG}: {NOT_A_MAPPING}5", + id="assembly-leg-with-no-repository-beside-5"), +] + + +@pytest.mark.parametrize("text,why", NO_REPOSITORY) +def test_update_shape_says_a_manifest_naming_no_repository_exactly( + update_shape, tmp_path, gh_calls, text, why): + """`why_none` and the `--pr` refusal, whole: no `legs is None` for an + absent or null `legs:`, and no stray `: ` after the sentence.""" + (tmp_path / "project.yaml").write_text(text, encoding="utf-8") + manifest = update_shape.load_yaml(tmp_path / "project.yaml") + assert update_shape.pull_request_repository(manifest) == ("", why) + with pytest.raises(update_shape.Refusal) as raised: + update_shape.pull_request_target(tmp_path, update_shape.PROJECT_KIND) + assert str(raised.value) == f"REFUSED update-pr-no-repository: {why}" + assert gh_calls == [] @pytest.mark.parametrize("spelled,said", CASES) diff --git a/update-shape.py b/update-shape.py index 411bbc29..7f024f2a 100755 --- a/update-shape.py +++ b/update-shape.py @@ -2497,9 +2497,11 @@ def pull_request_repository(manifest: dict | None, `openXdox` or a mapping was refused "declares no assembly leg", as of a list of legs that leaves that one out. `why_none` now says what names no leg, in that validator's words: the `manifest-legs` sentence, or - after "declares no assembly leg" each `manifest-leg` one. An entry that - is not a mapping beside the assembly leg does not stop its repository - being read; the validators `apply` runs before any landing say it. + after "declares no assembly leg" each `manifest-leg` one, which an + assembly leg with no `repository:` beside such an entry gets too. An + entry that is not a mapping beside the assembly leg does not stop its + repository being read, and no caller reads `why_none` then; the + validators `apply` runs before any landing say it. """ manifest = manifest or {} if kind is FAMILY_KIND: @@ -2521,9 +2523,8 @@ def pull_request_repository(manifest: dict | None, "`role:`, `repository:` and `path:` mappings") legs = [leg for leg in declared or [] if isinstance(leg, dict) and leg.get("role") == "assembly"] - unread = [] if legs else [f"manifest-leg: a leg is not a mapping: {leg!r}" - for leg in declared or [] - if not isinstance(leg, dict)] + unread = [f"manifest-leg: a leg is not a mapping: {leg!r}" + for leg in declared or [] if not isinstance(leg, dict)] return (str(legs[0].get("repository") or "") if legs else ""), ( "project.yaml declares no assembly leg, so there is no " "repository to open a pull request against" From b905bf8c7610a58b0a50256e6ca9409278e1eaaf Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 02:17:52 +0000 Subject: [PATCH 3/3] tests/test_legs_readers_not_a_list.py: with #826 merged, the doctor exits 1 for `legs: 5` too, and bootstrap's step (b) names each gate's exit 1 with no traceback anywhere (RV862's NIT-5, #832) Two checks allowed for gates without #826 (#714): the doctor test took exit 1 or 3 for `legs: 5`, because the standard's naming gate crashed on it (CANNOT ANSWER), and bootstrap's `FAILED` line took `exit [12]` for the naming and pins gates, which refused or crashed. Main c15a49e5 holds #826, merged into this branch, and every gate now reports such a `legs:` as a finding, exit 1. So the doctor's exit is 1 for every value, the `FAILED` line is asserted whole, and the validators run asserts no traceback at all, where it could assert only none of bootstrap's own. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- tests/test_legs_readers_not_a_list.py | 34 +++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/test_legs_readers_not_a_list.py b/tests/test_legs_readers_not_a_list.py index 8b9d0673..a96ca0a3 100644 --- a/tests/test_legs_readers_not_a_list.py +++ b/tests/test_legs_readers_not_a_list.py @@ -41,11 +41,12 @@ `None` guard each reader now has cannot go unseen (RV862's SHOULD-1): without it, absent and null read as the `legs is None` mismatch #832 removes. -THE GATES' OWN WORDS ARE #826's. A tree without it still has the naming and -pins gates crash on `5` and `validate-manifest.py` call it "missing or -empty", so this file asserts what the four readers say, from one table, and -nothing the gates print beside them; #826's own tests hold the gates to the -same sentence. +THE GATES' OWN WORDS ARE #826's, and #826's own tests hold the gates to the +same sentence, so this file asserts what the four readers say, from one +table, and of the gates beside them only how they end: since #826 every +gate reports such a `legs:` as a finding, exit 1, where the naming and pins +gates crashed on `5` or refused, so the doctor's verdict is a finding's, +exit 1, and bootstrap's step (b) names each gate's exit 1 (RV862's NIT-5). THE HARNESS: conftest's `project` for bootstrap and `bump-leg.py`, `tests/test_shape_doctor.py`'s session standard for the doctor, @@ -56,7 +57,6 @@ from __future__ import annotations import json -import re import shutil from types import SimpleNamespace @@ -190,10 +190,10 @@ def test_the_doctors_rows_that_read_the_legs_say_the_sentence_and_wait( result = doctor(standard, doctored, "--json") no_traceback(result) payload = json.loads(result.stdout) - # A finding's exit. For `5` the standard's own naming gate crashes until - # #826 lands, which is CANNOT ANSWER about this checkout: never this row. + # A finding's exit, for `5` too: since #826 the standard's own naming + # gate reports it, where it crashed and the verdict was CANNOT ANSWER. assert payload["exit"] == result.returncode, payload - assert result.returncode in ({1, 3} if spelled == "5" else {1}), payload + assert result.returncode == 1, payload rows = rows_of(result) assert rows["manifest-kinds"]["status"] == "ok", rows["manifest-kinds"] assert rows["manifest"]["status"] == "FINDING", rows["manifest"] @@ -314,24 +314,24 @@ def test_bootstraps_step_a_says_the_sentence_and_places_no_leg(project, assert "no submodule legs declared" not in result.stdout -#: Each gate's exit for these four: 1 with #826, which says the sentence, and -#: 2 or a traceback's 1 without it. Bootstrap reports each as it ends. -FAILED = re.compile(r"bootstrap FAILED: naming \(exit [12]\), manifest " - r"\(exit 1\), pins \(exit [12]\)") +#: Each gate's exit for these four: 1, a finding, since #826, where the +#: naming and pins gates refused (2) or crashed. Bootstrap reports each. +FAILED = "bootstrap FAILED: naming (exit 1), manifest (exit 1), pins (exit 1)" @pytest.mark.parametrize("spelled,said", ISSUE_CASES) def test_bootstrap_fails_on_its_validators_after_saying_the_sentence( project, spelled, said): - """With the validators: step (a) says the sentence, step (b)'s gates - refuse, and bootstrap fails, exit 1, with no traceback of its own.""" + """With the validators: step (a) says the sentence, each of step (b)'s + gates reports it, exit 1, and bootstrap fails, exit 1, with no + traceback anywhere.""" legs_are(project, spelled) result = bootstrap(project) + no_traceback(result) assert result.returncode == 1, result.stdout + result.stderr assert step_a(result.stdout) == step_a_says(said), result.stdout - assert 'bootstrap.py", line' not in result.stderr, result.stderr last = result.stderr.splitlines()[-1] - assert FAILED.fullmatch(last), last + assert last == FAILED, last def test_bootstrap_places_the_legs_beside_an_entry_that_is_not_one(project):