From 430c13f68548a0a89f7003832a2e11f07f0a9ce7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 17:37:39 +0000 Subject: [PATCH] shape-doctor.py: a validator's verdict is a printed `FINDING` or `REFUSED` and any other non-zero exit is a crash, so a gate killed by a signal, ended by `os._exit(1)`, holding conflict markers or failing argparse no longer "reports a finding", a crash of the standard's own naming script is CANNOT ANSWER instead of `INVALID (naming)`, and a pass that printed `WARNING` or ` note ` lines is a `note` quoting them while a red row quotes its finding and never a warning (#410, #448) One rule for every child the doctor reads, from the v1.1.0 triage of #410 that Brett Heap approved at 14:31:28Z on 2026-10-14: a verdict is a printed `FINDING` or `REFUSED`, anything else non-zero is a crash, and a WARNING or note line on a pass is a `note`. `crashed()` no longer asks for a `Traceback (most recent call last):` line. Every finding and refusal the shipped validators and the naming script choose to report is a line starting `FINDING` or `REFUSED`, printed before the exit 1 or 2 that goes with it, so a non-zero exit with neither line is a child that stopped before it said anything. That brings in #447: a negative exit (SIGKILL -9, SIGTERM -15), `os._exit(1)` with nothing printed, a `SyntaxError` from merge-conflict markers (no traceback header), `can't open file` and an argparse `usage:` error, each of which read "reports a finding" or "refuses". The quoted line is the output's last, or, with no output, words saying so that name the signal. A FINDING printed before the child died still stands, as #357 ruled. Both `naming` rows now ask `crashed()` (#410). The naming script is always this checkout's, so a crash carries `detail.crashed` and `detail.environment`, like a crashed template validator: an assembly root is CANNOT ANSWER, exit 3, and a directory that is not a root stays NOT A SHAPE ROOT, exit 2, because `verdict_for` decides that first. The text report prints its `shape-doctor-cannot-answer` refusal only under a CANNOT ANSWER verdict, so none appears under NOT A SHAPE ROOT. `check_naming` keeps `--quiet`: the `manifest` row now carries the same chain WARNING. On exit 0, `validator_row` reads ` skip `, `WARNING ` and ` note ` blocks with one reader (`reported`), each into its own `detail` key (`skipped`, `warnings`, `notes`), quoted in the reason in the order printed. So validate-manifest.py's `WARNING naming-referent-chain` (#448) and validate-pins.py's ` note neutral-pin-source-collides` (#460's pins-note line, comment 6136245436) reach the report as a `note` that moves no verdict. `REPORT_ITEM` learns ` note `, so a note after a skip is no longer read as that skip's text. A red row's quoted line is the first FINDING or REFUSED, else the first WARNING, else the last line, so a manifest row no longer quotes an earlier leg's WARNING as its reason (#448's decision 4). `with_gitlink_repairs` and `with_merged_away_rows` read the whole output and skip a crashed row, so the wider crash rule only keeps them off rows that printed no finding. Measured on 5ed3e5d: the stock naming script still raises `UnicodeEncodeError` printing a name that is not UTF-8 under the doctor's `PYTHONIOENCODING=utf-8`, so #410's case (1) is still there. The doctor's row now calls it a crash of this checkout's script; the script itself is unchanged, by the brief's ruling. Tests: tests/test_doctor_validator_verdicts.py, new, on #357's harness. tests/test_pin_source_env_collisions.py's colliding-pair test expected the `pins` row to be `ok` over that note; it now expects a `note`. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 312 ++++++++++--- tests/test_doctor_validator_verdicts.py | 557 ++++++++++++++++++++++++ tests/test_pin_source_env_collisions.py | 26 +- 3 files changed, 821 insertions(+), 74 deletions(-) create mode 100644 tests/test_doctor_validator_verdicts.py diff --git a/shape-doctor.py b/shape-doctor.py index 343a83ba..83ff637c 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -285,6 +285,7 @@ import os import re import shutil +import signal import subprocess import sys from pathlib import Path @@ -1150,72 +1151,164 @@ def offline(env: dict) -> dict: return env -#: The line Python prints first when an exception reaches the top of a -#: program -- and one a validator that reached a verdict never prints (#278). -TRACEBACK_HEADER = "Traceback (most recent call last):" +#: The two words every verdict a validator reaches starts with: each finding +#: it reports and each refusal it makes is a line that begins with one of +#: them (#278, #410). +VERDICT_WORDS = ("FINDING", "REFUSED") + + +def signal_name(number: int) -> str: + """`SIGKILL` for 9, where this platform names the signal, else `signal + ` (#447).""" + try: + return signal.Signals(number).name + except ValueError: + return f"signal {number}" def crashed(code: int, whole: str) -> str | None: - """The line to quote when a validator CRASHED before reaching a verdict, - or None when it reached one (#278). - - THE EXIT CODE CANNOT SAY. The validators' protocol is `0 valid, 1 - findings, 2 refusal`, and an exception that reaches the top of a Python - program exits 1 too -- so a validator that could not import `repo_shape`, - or raised on a line of its own, read as a project with a finding, quoting - the exception. A crash is told by three things together: a non-zero - exit, no line starting `FINDING` or `REFUSED` (every finding and refusal - a validator prints starts with one), and a `TRACEBACK_HEADER` line. The - line quoted is the output's last, which for a traceback is the exception - itself -- what the row quoted before, kept so a reader still sees why. + """What to quote when a validator ended WITHOUT REACHING A VERDICT, or + None when it reached one (#278, #410, #448, #447). + + A VERDICT IS A PRINTED `FINDING` OR `REFUSED`; ANYTHING ELSE NON-ZERO IS + A CRASH. The validators' protocol is `0 valid, 1 findings, 2 refusal`, + and the exit code alone cannot say which of those a child reached: an + exception that reaches the top of a Python program exits 1 too, so a + validator that could not import `repo_shape`, or raised on a line of its + own, read as a project with a finding (#278). What CAN say is the + output. Every finding and every refusal `validate-pins.py`, + `validate-manifest.py`, `validate-family.py` and the naming script + choose to report is a line starting `FINDING` or `REFUSED`, printed + before the exit 1 or 2 that goes with it, so a non-zero exit with + neither line is a child that stopped before it had anything to say. + + THE TRACEBACK IS NO LONGER ASKED FOR (#447). #357 required a + `Traceback (most recent call last):` line as well, and so missed every + crash that prints none: a child killed by a signal (a NEGATIVE exit, + `-9` for the OOM killer's SIGKILL, `-15` for a timeout's SIGTERM, which + no Python program can choose for itself); `os._exit(1)` with nothing + printed; a validator holding merge-conflict markers, whose `SyntaxError` + CPython prints with no traceback header; a file the interpreter `can't + open`, exit 2; and an argparse `usage:` error, exit 2. Each of those read + as "reports a finding" or "refuses", a statement about the project made + by a child that never read it. One rule covers them all where a test for + a traceback covered none. + + A FINDING PRINTED BEFORE THE CHILD DIED IS STILL A VERDICT (#357, + R278-1): the line is there, and an exception after it does not take it + back, so any `FINDING` or `REFUSED` line anywhere in the output answers + None. The NAMING rows ask this too (#410), through `naming_crashed_row`. + + THE QUOTED LINE is the output's last, which for a traceback is the + exception itself, for a `SyntaxError` its `SyntaxError:` line and for + argparse its `error:` line -- what a reader needs to see why. With no + output at all it is words that say so, naming the signal for a negative + exit (`signal_name`), so a row never ends on an empty quote. """ lines = [line.strip() for line in whole.splitlines() if line.strip()] - if code == 0 or TRACEBACK_HEADER not in lines: - return None - if any(line.startswith(("FINDING", "REFUSED")) for line in lines): + if code == 0 or any(line.startswith(VERDICT_WORDS) for line in lines): return None - return lines[-1][:200] + if lines: + return lines[-1][:200] + if code < 0: + return f"it printed nothing and was ended by {signal_name(-code)}" + return "it printed nothing" + + +#: The `why` of every crash row, the validators' and the naming rows' alike +#: (#278, #410): the NEXT is the child's own line, run to see where it +#: stopped, and the row quotes only the last line of that. +CRASH_WHY = "read its output in full, it shows where the validator stopped" #: The first line of an ITEM in a validator's report, as against a line that -#: CONTINUES the item above it: an ` ok`, ` skip` or ` --` line, a -#: `FINDING`, `REFUSED` or `WARNING`, and the success summary a validator ends -#: on (`pins ok`, `manifest ok: ...`, `family ok: ...`). +#: CONTINUES the item above it: an ` ok`, ` skip`, ` note` or ` --` line, +#: a `FINDING`, `REFUSED` or `WARNING`, and the success summary a validator +#: ends on (`pins ok`, `manifest ok: ...`, `family ok: ...`). ` note` is an +#: item since #448: `validate-pins.py` prints its notes beside its skips, and +#: a note printed after a skip was read as the rest of that skip's text. REPORT_ITEM = re.compile( - r" (?:ok|skip|--)\s|(?:FINDING|REFUSED|WARNING)\b|[a-z]+ ok\b") + r" (?:ok|skip|note|--)\s|(?:FINDING|REFUSED|WARNING)\b|[a-z]+ ok\b") + + +#: What a validator that PASSED can still have said, by the prefix it says it +#: with, as `(prefix, the row's detail key, the words the reason quotes it +#: after)` (#297, #448). Each kind is a `note` on a row whose child exited 0: +#: `validate-pins.py`'s skip of a digest it had no source to recompute, +#: `validate-manifest.py`'s WARNING about a chain link whose tree is not on +#: this disk, and `validate-pins.py`'s note, such as +#: `neutral-pin-source-collides` (#460's pins-note line). +PASS_REPORTS = ( + (" skip ", "skipped", "did not recheck"), + ("WARNING ", "warnings", "warns"), + (" note ", "notes", "notes"), +) -def skipped(whole: str) -> list[str]: - """What each ` skip ` block a validator printed did not check, as - `` (#297): `validate-pins.py`'s line for a neutral product's digest - it found no source to recompute, which it reports and still exits 0. +def reported(whole: str) -> list[tuple[str, str]]: + """`[(detail key, text)]` for every ` skip `, `WARNING ` and ` note ` + block a validator printed, in the order printed, each `` the block + less its prefix (#297, #448). - THE WHOLE BLOCK, NOT ITS FIRST LINE: the skip embeds `gh`'s own stderr + THE WHOLE BLOCK, NOT ITS FIRST LINE: a skip embeds `gh`'s own stderr verbatim, and `gh` prints its not-logged-in help over two lines (four under `GITHUB_ACTIONS`), so the text `validate-pins.py` meant to say runs on past the first newline -- to the `--pin-source`, `SHAPE_PIN_SOURCE_` and `../` ways to give it a source, - which are why the row has no NEXT. Every line after the ` skip ` line is + which are why the row has no NEXT. Every line after the block's first is kept, up to the next `REPORT_ITEM` or the end, and the lines are joined - with a space. Flattened by `ascii_text`, as the machine row's `report` - is, so `detail` says what the reason says.""" - skips: list[list[str]] = [] + with a space. ONE READER FOR THE THREE KINDS, AND `REPORT_ITEM` IS ITS + ONE DEFINITION OF AN ITEM: every line it matches ends the block before + it, and a block starts only at one of them. So a note printed after a + skip, or a skip after a note, starts a block of its own and neither + swallows the other, because ` note ` is an item. Flattened by + `ascii_text`, as the machine row's `report` is, so `detail` says what + the reason says.""" + blocks: list[tuple[str, list[str]]] = [] inside = False for line in whole.splitlines(): - if line.startswith(" skip "): - skips.append([line.strip()[len("skip "):]]) - inside = True - elif REPORT_ITEM.match(line): - inside = False + if REPORT_ITEM.match(line): + kind = next(((prefix, key) for prefix, key, _words in PASS_REPORTS + if line.startswith(prefix)), None) + inside = kind is not None + if kind is not None: + prefix, key = kind + blocks.append((key, + [line.strip()[len(prefix.strip()) + 1:]])) elif inside and line.strip(): - skips[-1].append(line.strip()) - return [ascii_text(" ".join(lines)) for lines in skips] + blocks[-1][1].append(line.strip()) + return [(key, ascii_text(" ".join(lines))) for key, lines in blocks] + + +def skipped(whole: str) -> list[str]: + """What each ` skip ` block a validator printed did not check, as + `` (#297): `reported`'s skips alone.""" + return [text for key, text in reported(whole) if key == "skipped"] + + +def passed_but(said: list[tuple[str, str]]) -> str: + """The words after "passes, but" for what a passing validator `reported` + (#297, #448): each kind's words from `PASS_REPORTS`, then its texts, in + the order printed. A RUN of one kind shares its words, so a validator + that only skipped reads `did not recheck: ; `, as it always did, + and a skip, a note and a skip read as three runs.""" + words = {key: said_as for _prefix, key, said_as in PASS_REPORTS} + runs: list[tuple[str, list[str]]] = [] + for key, text in said: + if runs and runs[-1][0] == key: + runs[-1][1].append(text) + else: + runs.append((key, [text])) + return "; ".join(f"{words[key]}: {'; '.join(texts)}" + for key, texts in runs) def run_validator(ctx: Context, script: Path, args: list[str], pythonpath: Path | None = None, *, reach_network: bool = False) -> tuple: - """`(exit code, first line worth quoting, whole output)`. + """`(exit code, the line worth quoting, whole output)`: the first + `FINDING` or `REFUSED` line, else the first `WARNING`, else the last + (#448). THE CHILD'S `PYTHONPATH` IS EXACTLY WHAT THE ROW'S NEXT PRINTS, AND THE DOCTOR'S OWN IS NEVER PART OF IT (#306). `pythonpath` is the directory to @@ -1313,15 +1406,18 @@ def run_validator(ctx: Context, script: Path, args: list[str], stdin=subprocess.DEVNULL, env=env, start_new_session=True) whole = (proc.stdout or "") + (proc.stderr or "") - quoted = "" - for line in whole.splitlines(): - stripped = line.strip() - if stripped.startswith(("FINDING", "REFUSED", "WARNING")): - quoted = stripped - break - if not quoted: - lines = [line.strip() for line in whole.splitlines() if line.strip()] - quoted = lines[-1] if lines else "" + # THE QUOTED LINE OF A RED ROW IS A VERDICT LINE (#448, decision 4): the + # first `FINDING` or `REFUSED`, else the first `WARNING`, else the last + # line. `validate-manifest.py` prints leg by leg, so an earlier leg's + # WARNING came before a later leg's FINDING, and a row whose gate went + # red quoted the note that moved nothing instead of the finding that + # failed it. A WARNING is still quoted where nothing else would say + # more: an exit 0 never quotes, and a crash quotes its own last line. + lines = [line.strip() for line in whole.splitlines() if line.strip()] + quoted = next( + (line for line in lines if line.startswith(VERDICT_WORDS)), + next((line for line in lines if line.startswith("WARNING")), + lines[-1] if lines else "")) return proc.returncode, quoted[:200], whole @@ -1443,14 +1539,19 @@ def validator_row(ctx: Context, check_id: str, label: str, own: str, A VALIDATOR THAT CRASHED HAS NOT ANSWERED (#278). Exit 1 is a finding AND an exception that reached the top, so the row asks `crashed` before - it believes the code. The root's OWN copy crashing is still a FINDING -- - a project whose gate cannot run has a broken gate, which is a fact about - the project (#237 draws the same line from the other side) -- but it - says it crashed, carries `detail.crashed`, and says when the cause is - the `repo_shape.py` it imports (`imports_repo_shape`). The TEMPLATE copy + it believes the code -- and since #447 a crash is ANY non-zero exit that + printed no `FINDING` or `REFUSED` line: a signal, `os._exit(1)`, a + `SyntaxError`, an argparse `usage:` error. The verb a verdict row picks + by its code, "refuses" for 2 and "reports a finding" otherwise, is then + reached only by a child that printed one of those lines. The root's OWN + copy crashing is still a FINDING -- a project whose gate cannot run has + a broken gate, which is a fact about the project (#237 draws the same + line from the other side) -- but it says it crashed, carries + `detail.crashed`, and says when the cause is the `repo_shape.py` it + imports (`imports_repo_shape`). The TEMPLATE copy crashing is this checkout's failure and not the project's: it carries `detail.environment`, so the verdict is CANNOT ANSWER, exit 3. Either - way the NEXT is the validator's own line, which shows the traceback. + way the NEXT is the validator's own line, which shows where it stopped. AND A TEMPLATE THIS CHECKOUT CANNOT GIVE ITS `PYTHONPATH` IS NOT RUN (#278): `pythonpath_unnamable` says why, the row is an environment row @@ -1464,6 +1565,18 @@ def validator_row(ctx: Context, check_id: str, label: str, own: str, reason quotes what was not rechecked, because "passes" alone said a digest was verified that nobody verified. The skip line already names the three ways to give it a source, so there is no NEXT. + + AND SO DOES A PASS THAT WARNED OR NOTED (#448, #460's pins-note line). + `validate-manifest.py` prints `WARNING naming-referent-chain: ...` and + exits 0 for a chain link whose tree is not on this disk -- "printed and + then forgotten by the exit code, deliberately" -- and `validate-pins.py` + prints ` note neutral-pin-source-collides: ...` beside its skips. Both + were dropped, so a chain nothing here could check read as checked under + COMPLIANT, and the one line saying which variable is read for neither + pin reached nobody. Each is a `note` the same way: its own `detail` key + (`skipped`, `warnings`, `notes`, from `PASS_REPORTS`), quoted in the + reason in the order printed (`passed_but`), and no NEXT -- each names + its own remedies, and which declaration stays is the human's call. """ used, which, own_copy = validator_for(ctx, own, template) if not used.is_file(): @@ -1491,12 +1604,13 @@ def validator_row(ctx: Context, check_id: str, label: str, own: str, detail = {"validator": used.as_posix(), "own_copy": own_copy, "exit": code} if code == 0: - skips = skipped(whole) - if not skips: + said = reported(whole) + if not said: return Row(check_id, label, OK, f"{which} passes", None, detail) - detail["skipped"] = skips - return Row(check_id, label, NOTE, f"{which} passes, but did not " - f"recheck: {'; '.join(skips)}", None, detail) + for key, text in said: + detail.setdefault(key, []).append(text) + return Row(check_id, label, NOTE, + f"{which} passes, but {passed_but(said)}", None, detail) crash = crashed(code, whole) if crash is not None: detail["crashed"] = True @@ -1513,8 +1627,7 @@ def validator_row(ctx: Context, check_id: str, label: str, own: str, "so nothing here is a statement about the project") command = validator_command(ctx.shape, used, own_copy, args) return Row(check_id, label, FINDING, reason, command, detail, - why="read its output in full, it shows where the " - "validator stopped") + why=CRASH_WHY) verb = "refuses" if code >= 2 else "reports a finding" reason = f"{which} {verb} (exit {code})" + (f": {quoted}" if quoted else "") # A COMMAND THAT RUNS WHEN IT IS PASTED. This row used to print @@ -1704,13 +1817,23 @@ def check_naming(ctx: Context) -> Row: The names are READ OUT OF `project.yaml`, so a manifest that does not load leaves none to classify: `waiting_row` (#205). + + A SCRIPT THAT CRASHED HAS NOT ANSWERED (#410). This row read the exit + code alone, so a checkout of the standard whose own naming script raised + said `INVALID (naming)`, exit 1, about a compliant project: a statement + about the project caused by a fault in the checkout asking. It asks + `crashed` as `validator_row` does, and a crash is `naming_crashed_row`, + CANNOT ANSWER, exit 3. `--quiet` stays: the script's WARNINGs are about + a chain link it could not read, and the `manifest` row now carries + `validate-manifest.py`'s WARNING for that same link (#448), so the fact + is said once. """ waiting = waiting_row(ctx, "naming", "naming", "there are no leg names to classify") if waiting is not None: return waiting script = ctx.shape / "scripts" / "validate-repository-naming.py" - code, quoted, _ = run_validator( + code, quoted, whole = run_validator( ctx, script, ["--project", str(ctx.root / PROJECT_MANIFEST), "--quiet"]) detail = {"policy": (ctx.shape / "contracts" / @@ -1719,11 +1842,46 @@ def check_naming(ctx: Context) -> Row: return Row("naming", "naming", OK, "every leg name classifies, and each declared role agrees " "with its form", None, detail) + command = (f"{PYTHON} {quote_arg(script)} --explain --project " + f"{quote_arg(ctx.root / PROJECT_MANIFEST)}") + crash = crashed(code, whole) + if crash is not None: + return naming_crashed_row(code, crash, command, detail) return Row("naming", "naming", FINDING, f"the naming policy is not satisfied (exit {code})" - + (f": {quoted}" if quoted else ""), - f"{PYTHON} {quote_arg(script)} --explain --project " - f"{quote_arg(ctx.root / PROJECT_MANIFEST)}", detail) + + (f": {quoted}" if quoted else ""), command, detail) + + +def naming_crashed_row(code: int, crash: str, command: str, detail: dict, + names: list[str] | None = None) -> Row: + """The `naming` row for a naming script that ended without a verdict + (#410): `check_naming`'s, or, given the `names` it classified, + `check_not_a_root_naming`'s. + + THE NAMING SCRIPT IS ALWAYS THIS CHECKOUT'S, never the project's, so its + crash is what a crash of the standard's TEMPLATE validator is in + `validator_row` (#278): `detail.crashed`, and `detail.environment`, so + an assembly root's verdict is CANNOT ANSWER, exit 3, and the reason + says nothing here is a statement about the project. In a directory that + is not a root the same marking leaves the verdict NOT A SHAPE ROOT, exit + 2, because `verdict_for` decides that before `cannot_answer`; what the + marking changes there is the row, which used to quote the script's + exception as a fact about the name. The NEXT is the script's own line, + as before, and its `why` is a crash row's: read the output in full. + """ + detail["crashed"] = True + detail["environment"] = True + if names is None: + lead, whose, about = "", "the project's", "the project" + else: + lead = f"{', '.join(names)}: " + whose, about = "this directory's", "what it is called" + return Row("naming", "naming", FINDING, + f"{lead}the standard's scripts/validate-repository-naming.py " + f"crashed before reaching a verdict (exit {code}): {crash}; " + f"that script is this checkout's, not {whose}, so nothing " + f"here is a statement about {about}", command, detail, + why=CRASH_WHY) def check_manifest(ctx: Context) -> Row: @@ -5853,13 +6011,21 @@ def check_not_a_root_naming(ctx: Context) -> Row: ["--explain", "--", *names]) detail = {"names": names, "origin": remote, "exit": code, "explain": ascii_text(whole).strip().splitlines()[:40]} + spelled = " ".join(quote_arg(name) for name in names) + command = f"{PYTHON} {quote_arg(script)} --explain -- {spelled}" + # A SCRIPT THAT CRASHED SAID NOTHING ABOUT THE NAME (#410). A directory + # whose name is not UTF-8 made the script raise as it printed that name, + # and this row quoted `UnicodeEncodeError: ...` as the name's finding. + # The verdict stays NOT A SHAPE ROOT; the row says the checkout's script + # stopped, as `check_naming`'s does. + crash = crashed(code, whole) + if crash is not None: + return naming_crashed_row(code, crash, command, detail, names) reason = f"{', '.join(names)}: " + ( "classifies under the naming policy" if code == 0 else (quoted or "matches no family in the policy")) - spelled = " ".join(quote_arg(name) for name in names) return Row("naming", "naming", OK if code == 0 else FINDING, reason, - None if code == 0 else - f"{PYTHON} {quote_arg(script)} --explain -- {spelled}", detail) + None if code == 0 else command, detail) def check_what_is_here(ctx: Context) -> Row: @@ -6771,7 +6937,13 @@ def report(ctx: Context, rows: list[Row], verdict: str, code: int, note = verdict_note(ctx) if note: print(note) - blocked = cannot_answer(rows) + # ONLY UNDER THE VERDICT IT EXPLAINS (#410). A crashed naming script in + # a directory that is not a root marks its row `environment` as it does + # in a root, and the verdict there is still NOT A SHAPE ROOT, exit 2: + # a `shape-doctor-cannot-answer` refusal under it would contradict the + # line above it. + blocked = (cannot_answer(rows) if verdict.startswith(V_CANNOT_ANSWER) + else None) if blocked is not None: print(f"\nREFUSED shape-doctor-cannot-answer: {blocked.reason}", file=sys.stderr) diff --git a/tests/test_doctor_validator_verdicts.py b/tests/test_doctor_validator_verdicts.py new file mode 100644 index 00000000..bddcd57c --- /dev/null +++ b/tests/test_doctor_validator_verdicts.py @@ -0,0 +1,557 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`shape-doctor.py` reads a validator's output under ONE rule: a verdict is +a printed `FINDING` or `REFUSED`, anything else non-zero is a crash, and a +pass that printed `WARNING` or ` note ` lines is a `note` (#410, #448, with +#447 and #460's pins-note line riding along). + +FOUR WAYS A ROW STILL SAID MORE, OR LESS, THAN THE CHILD DID. + +- #447: `crashed()` needed a `Traceback` line, so a validator killed by a + signal (a negative exit), ended by `os._exit(1)` with nothing printed, + holding merge-conflict markers (a `SyntaxError`, which prints no traceback + header) or failing argparse (`usage:`, exit 2) "reports a finding" or + "refuses": a statement about the project from a child that never read it. +- #410: both `naming` rows read the exit code alone, so a checkout of the + standard whose own naming script crashed made a compliant project + `INVALID (naming)`, and a directory whose name is not UTF-8 got the + script's `UnicodeEncodeError` quoted as a finding about the name. +- #448: `validate-manifest.py`'s `WARNING naming-referent-chain` on exit 0 + appeared nowhere in the report, and a red manifest row quoted that WARNING + as its reason when an earlier leg printed it before a later leg's FINDING. +- #460's pins-note line: `validate-pins.py`'s ` note ` lines on exit 0 were + dropped, and a note printed after a ` skip ` was read as that skip's text. + +THE HARNESS IS #357's (`tests/test_doctor_validator_rows.py`), imported: its +crash builder, its copy of the standard and its `gh` stub, over the session's +standard and project from `tests/test_shape_doctor.py`. Every assertion is on +what a command does -- a row's status, its `detail`, the verdict, a NEXT that +is run -- and on the words a row quotes from its child, never on a `why`. + +NOTHING HERE REACHES A NETWORK. The scaffolds use local bare remotes; a run +whose validator asks `gh api` for a neutral product's tree has #357's +logging stub first on `PATH`, ahead of the session guard, and is +`WINDOWS_SKIP` for its shebang. +""" + +from __future__ import annotations + +import os +import shutil +import signal +import subprocess +from pathlib import Path + +import pytest + +from conftest import WINDOWS_SKIP, clone_just_pushed, run_script +#: The layer stand-in a chain descendant pins, from the module that scaffolds +#: one: a second way of building it would test a different one. +from test_scaffold_pin_and_reuse import layer_repo +#: #357's harness, and through it the session's standard and project. Each +#: fixture imported gives this module its own instance of it. +from test_doctor_validator_rows import ( # noqa: F401 + CRASH, DOCTOR, TEMPLATE_FALLBACK, assert_next_runs_the_roots_own, + crashing, doctor, doctor_module, logging_gh, payload_of, project, + rows_of, scaffolded_here, standard, standard_under, +) + +NAMING_SCRIPT = "scripts/validate-repository-naming.py" +#: What every crash row says, the validators' and the naming rows' alike. +CRASHED = "crashed before reaching a verdict" +#: The tail #410's ruling gives a crashed naming script's row in a root. +NOT_THE_PROJECTS = ("that script is this checkout's, not the project's, so " + "nothing here is a statement about the project") + + +def command_of(row: dict) -> list: + """The row's NEXT as the argv it runs; the paths here need no quoting.""" + return row["next"].split() + + +def run_argv(command: list) -> subprocess.CompletedProcess: + return subprocess.run(command, capture_output=True, text=True, + check=False, stdin=subprocess.DEVNULL, + env={**os.environ, "PYTHONDONTWRITEBYTECODE": "1"}) + + +# --- #447: a crash is told by what the child printed ------------------------- + +#: The ways a root's own `validate-pins.py` ends with no verdict, as +#: `(source, expected exit, words the reason quotes)`. The signals are named, +#: not numbered, and read inside the test: Windows has no SIGKILL. +MERGE_MARKERS = "<<<<<<< HEAD\nimport sys\n=======\nimport os\n>>>>>>> theirs\n" +ENDINGS = { + "sigkill": ("import os, signal\nos.kill(os.getpid(), signal.SIGKILL)\n", + "SIGKILL", "it printed nothing and was ended by SIGKILL"), + "sigterm": ("import os, signal\nos.kill(os.getpid(), signal.SIGTERM)\n", + "SIGTERM", "it printed nothing and was ended by SIGTERM"), + "os-exit": ("import os\nos._exit(1)\n", 1, "it printed nothing"), + "merge-markers": (None, 1, "SyntaxError"), + "usage": ("import sys\n" + "print('usage: validate-pins.py [-h]', file=sys.stderr)\n" + "print('validate-pins.py: error: unrecognized arguments: x', " + "file=sys.stderr)\n" + "sys.exit(2)\n", + 2, "validate-pins.py: error: unrecognized arguments: x"), +} + + +def ending(how: str, own: Path) -> tuple: + """`(source, exit code, words)` for `how`, the source of a merge left + unresolved being the root's own file with the markers appended.""" + source, code, words = ENDINGS[how] + if source is None: + source = own.read_text(encoding="utf-8") + MERGE_MARKERS + if isinstance(code, str): + code = -int(getattr(signal, code)) + return source, code, words + + +@pytest.mark.parametrize("how", [ + pytest.param("sigkill", marks=WINDOWS_SKIP), + pytest.param("sigterm", marks=WINDOWS_SKIP), + "os-exit", "merge-markers", "usage"]) +def test_a_gate_that_ended_without_a_verdict_is_a_crash_whatever_it_printed( + standard, project, tmp_path, how): + """#447: the root's own `validate-pins.py` ended by a signal, by + `os._exit(1)`, by a `SyntaxError` or by argparse is a CRASH row: it says + it crashed, carries `detail.crashed`, quotes what the child last said + (or that it said nothing, naming the signal), and never "reports a + finding" or "refuses". + + THE ROOT'S OWN COPY IS STILL A FINDING about the project's gate, #357's + line: a gate that cannot run is a broken gate. So the verdict is the one + an edited copy gives, DRIFTED, exit 1, never CANNOT ANSWER, and the NEXT + is that gate's own line. `WINDOWS_SKIP` for the two signals only: a + process there is ended with the code its killer chose, not a negative + one. + """ + own = project / TEMPLATE_FALLBACK["pins"]["own"] + source, code, words = ending(how, own) + here, root, row_id, own_copy = crashing("own-pins", standard, project, + None, tmp_path, source) + result = doctor(here, root, "--json") + payload = payload_of(result) + row = rows_of(result)[row_id] + assert row_id == "pins" and own_copy is True + assert row["status"] == "FINDING", row + assert row["detail"]["crashed"] is True, row + assert row["detail"]["own_copy"] is True, row + assert row["detail"]["exit"] == code, row + assert "environment" not in row["detail"], row + assert f"{CRASHED} (exit {code}): " in row["reason"], row + assert words in row["reason"], row + assert "reports a finding" not in row["reason"], row + assert "refuses" not in row["reason"], row + assert row["why"].startswith("read its output in full"), row + assert_next_runs_the_roots_own(row, root, "validate-pins.py") + assert result.returncode == 1, result.stdout + result.stderr + assert payload["verdict"].startswith("DRIFTED"), payload + + +@pytest.mark.parametrize("how", [pytest.param("sigkill", marks=WINDOWS_SKIP), + "os-exit"]) +def test_the_standards_template_ended_without_a_verdict_cannot_answer( + standard, project, tmp_path, how): + """#447's other half: the standard's TEMPLATE copy, for a root with none + of its own, killed or ended by `os._exit(1)` is this checkout's failure: + `detail.environment`, CANNOT ANSWER, exit 3 -- as #357 already ruled for + a template that raised, and as it did not for one that printed nothing. + """ + template = standard / TEMPLATE_FALLBACK["manifest"]["template"] + source, code, words = ending(how, template) + here, root, row_id, own_copy = crashing("template", standard, project, + None, tmp_path, source) + result = doctor(here, root, "--json") + row = rows_of(result)[row_id] + assert own_copy is False + assert row["status"] == "FINDING", row + assert row["detail"]["crashed"] is True, row + assert row["detail"]["environment"] is True, row + assert row["detail"]["exit"] == code, row + assert words in row["reason"], row + assert "reports a finding" not in row["reason"], row + assert result.returncode == 3, result.stdout + result.stderr + assert payload_of(result)["verdict"].startswith( + f"CANNOT ANSWER ({row['label']}: "), result.stdout + + +def test_crashed_reads_the_output_and_never_a_traceback(standard): + """`crashed` on constructed outputs, on every platform: None for exit 0 + and for any output holding a `FINDING` or `REFUSED` line, a negative + exit included (a finding printed before the child died still stands); + otherwise the last non-empty line, or words naming what was printed and, + for a negative exit, the signal.""" + crashed = doctor_module(standard).crashed + assert crashed(0, "") is None + assert crashed(0, "Traceback (most recent call last):\nE: x\n") is None + assert crashed(1, "FINDING pin-x: a finding\nTraceback (most recent " + "call last):\nRuntimeError: boom\n") is None + assert crashed(-9, " ok a\nFINDING pin-x: printed, then killed\n") \ + is None + assert crashed(2, "REFUSED pin-y: a refusal\n") is None + assert crashed(1, "") == "it printed nothing" + assert crashed(-int(signal.SIGTERM), "") == \ + "it printed nothing and was ended by SIGTERM" + assert crashed(-200, "") == "it printed nothing and was ended by " \ + "signal 200" + assert crashed(-int(signal.SIGTERM), " ok a\n ok b\n\n") == "ok b" + assert crashed(1, ' File "x.py", line 3\n <<<<<<< HEAD\n ^^\n' + "SyntaxError: invalid syntax\n") == \ + "SyntaxError: invalid syntax" + assert crashed(2, "usage: v [-h]\nv: error: the following arguments " + "are required: --root\n") == \ + "v: error: the following arguments are required: --root" + assert crashed(4, "WARNING w: something\n") == "WARNING w: something" + + +# --- #410: the naming rows ask `crashed` too -------------------------------- + +def naming_crashes(standard: Path, tmp_path: Path) -> Path: + """A COPY of the standard, history and all, whose own naming script + raises: the case #410 was filed for, built as #357 builds its template + crash.""" + here = standard_under(standard, tmp_path, "naming-crash") + (here / NAMING_SCRIPT).write_text(CRASH, encoding="utf-8") + return here + + +def test_a_crashed_naming_script_cannot_answer_for_a_compliant_project( + standard, project, tmp_path): + """(2) of the issue: a compliant project asked from a checkout whose + naming script raises is CANNOT ANSWER, exit 3, never `INVALID (naming)`. + + The row carries `detail.crashed` and `detail.environment`, as a crashed + TEMPLATE validator's does, says the script crashed, quotes the exception + and ends saying nothing here is about the project. Its NEXT is still the + script's own `--explain` line, and run it shows the exception the row + quoted, with the exit the row records. THE CONTROL is the same project + from the session's standard: `ok`. + """ + root = project.resolve() + control = rows_of(doctor(standard, root, "--json"))["naming"] + assert control["status"] == "ok", control + here = naming_crashes(standard, tmp_path) + result = doctor(here, root, "--json") + payload = payload_of(result) + row = rows_of(result)["naming"] + assert row["status"] == "FINDING", row + assert row["detail"]["crashed"] is True, row + assert row["detail"]["environment"] is True, row + assert row["detail"]["exit"] == 1, row + assert (f"the standard's {NAMING_SCRIPT} {CRASHED} (exit 1): " + "RuntimeError: boom; ") in row["reason"], row + assert row["reason"].endswith(NOT_THE_PROJECTS), row + assert "naming policy is not satisfied" not in row["reason"], row + assert result.returncode == 3, result.stdout + result.stderr + assert payload["verdict"].startswith("CANNOT ANSWER (naming: "), payload + module = doctor_module(standard) + assert row["next"] == ( + f"{module.PYTHON} {module.quote_arg(here.resolve() / NAMING_SCRIPT)} " + f"--explain --project {module.quote_arg(root / 'project.yaml')}"), \ + row["next"] + assert row["why"].startswith("read its output in full"), row + ran = run_argv(command_of(row)) + assert ran.returncode == row["detail"]["exit"], ran.stdout + ran.stderr + assert "RuntimeError: boom" in ran.stdout + ran.stderr + text = doctor(here, root) + assert text.returncode == 3, text.stdout + text.stderr + assert "REFUSED shape-doctor-cannot-answer: " in text.stderr, text.stderr + assert f"\n Remediation: {row['next']}\n" in text.stderr, text.stderr + + +def test_a_crashed_naming_script_leaves_a_directory_not_a_shape_root( + standard, tmp_path): + """The same checkout asked about a directory that is NOT a root: the + verdict stays NOT A SHAPE ROOT, exit 2, because `verdict_for` decides + that before `cannot_answer`, and the row is the crash row, marked as in + a root, not a FINDING quoting the exception as a fact about the name. + + AND THE TEXT REPORT PRINTS NO `shape-doctor-cannot-answer` refusal: one + under a NOT A SHAPE ROOT verdict would contradict the line above it. + """ + here = naming_crashes(standard, tmp_path) + away = tmp_path / "elsewhere" / "Atlas" + away.mkdir(parents=True) + result = doctor(here, away, "--json") + payload = payload_of(result) + row = rows_of(result)["naming"] + assert result.returncode == 2, result.stdout + result.stderr + assert payload["verdict"] == "NOT A SHAPE ROOT", payload + assert row["detail"]["crashed"] is True, row + assert row["detail"]["environment"] is True, row + assert row["reason"].startswith( + f"Atlas: the standard's {NAMING_SCRIPT} {CRASHED} (exit 1): " + "RuntimeError: boom; "), row + assert not row["reason"].startswith("Atlas: RuntimeError"), row + assert command_of(row)[-3:] == ["--explain", "--", "Atlas"], row["next"] + assert row["why"].startswith("read its output in full"), row + text = doctor(here, away) + assert text.returncode == 2, text.stdout + text.stderr + assert "NOT A SHAPE ROOT (exit 2)" in text.stdout, text.stdout + assert "shape-doctor-cannot-answer" not in text.stderr, text.stderr + + +@WINDOWS_SKIP +def test_a_name_that_is_not_utf8_is_never_an_exception_quoted_as_a_finding( + standard, tmp_path): + """(1) of the issue, with the stock standard: a directory whose name + holds the byte `\\xff`. The row is never a FINDING quoting the script's + exception about the name. + + MEASURED ON MAIN 5ed3e5d (Python 3.12.3, `LC_ALL=C.UTF-8`): the stock + script, under the `PYTHONIOENCODING=utf-8` every validator child gets, + still raises `UnicodeEncodeError` printing the name in its `--explain` + block, and the row said `FINDING | Atl?as: UnicodeEncodeError: ...`. So + today the row is the crash row; a script that prints such a name will + make it the `naming-unclassified` FINDING, and either is a true row -- + what this test holds is that the exception is never the finding. + `WINDOWS_SKIP`: Windows has no name that is not UTF-8, and a disk that + refuses one (APFS) skips. + """ + name = os.fsdecode(b"Atl\xffas") + here = tmp_path / "elsewhere" / name + try: + here.mkdir(parents=True) + except OSError as exc: + pytest.skip(f"this filesystem refuses a name that is not UTF-8: " + f"{exc}") + result = doctor(standard, here, "--json") + payload = payload_of(result) + row = rows_of(result)["naming"] + assert result.returncode == 2, result.stdout + result.stderr + assert payload["verdict"] == "NOT A SHAPE ROOT", payload + assert row["status"] == "FINDING", row + assert row["detail"]["names"] == [name], row + if row["detail"].get("crashed"): + assert row["detail"]["environment"] is True, row + assert f"{CRASHED} (exit 1): UnicodeEncodeError" in row["reason"], row + assert "this checkout's, not this directory's" in row["reason"], row + else: + assert "FINDING naming-unclassified" in row["reason"], row + assert "Error" not in row["reason"], row + assert not row["reason"].startswith("Atl?as: UnicodeEncodeError"), row + + +# --- #448: a pass that warned is a note, and a red row quotes its verdict --- + +@pytest.fixture(scope="module") +def chain_descendant(standard, tmp_path_factory) -> dict: + """#448's repro: `codexDox` pins the LAYER `openXdox` and records the + chain `openXdox,openDox`, scaffolded from the session's standard with + the layer's stand-in as `--pin-source`, then cloned AWAY from it, so no + tree answers for the chain's middle link and nothing beside the clone + holds one.""" + base = tmp_path_factory.mktemp("chain-descendant") + layer, commit = layer_repo(base / "standin") + made = run_script( + standard / "scaffold-project.py", "--org", "codexSoft", "--project", + "codexDox", "--elected-by", "Test Human", "--elected-on", + "2026-09-05", "--pin", f"openXdox@{commit}", "--referent-chain", + "openXdox,openDox", "--pin-source", str(layer), + "--local-remote-dir", str(base / "remotes"), + "--work-dir", str(base / "work")) + assert made.returncode == 0, made.stderr + made.stdout + clone = base / "away" / "codexDox" + clone.parent.mkdir() + cloned = clone_just_pushed(base / "remotes" / "codexDox.git", clone) + assert cloned.returncode == 0, cloned.stderr + assert not (clone.parent / "openXdox").exists() + return {"clone": clone, "layer": layer} + + +def doctor_offline(standard: Path, root: Path, tmp_path: Path): + """The doctor with #357's logging `gh` first on PATH, for the run whose + `validate-pins.py` asks `gh api` for the layer's tree: it answers 97, + and the validator prints its `skip`.""" + bin_dir, _calls = logging_gh(tmp_path / "gh") + return run_script(standard / DOCTOR, "--root", str(root), "--json", + env={"PATH": f"{bin_dir}{os.pathsep}" + f"{os.environ['PATH']}"}) + + +@WINDOWS_SKIP +def test_a_chain_nothing_here_could_check_is_a_note_and_still_compliant( + standard, chain_descendant, tmp_path): + """#448: the root's own `validate-manifest.py` prints `WARNING + naming-referent-chain` and exits 0, and the `manifest` row is a `note` + whose reason quotes it, with `detail.warnings` holding it and no NEXT. + A note moves no verdict: COMPLIANT, exit 0, as before.""" + result = doctor_offline(standard, chain_descendant["clone"], tmp_path) + payload = payload_of(result) + row = rows_of(result)["manifest"] + assert result.returncode == 0, result.stdout + result.stderr + assert payload["verdict"] == "COMPLIANT", payload + assert row["status"] == "note", row + assert row["next"] is None, row + assert row["detail"]["exit"] == 0, row + (warning,) = row["detail"]["warnings"] + assert warning.startswith( + "naming-referent-chain: leg 'assembly': declared-unverified: "), row + assert "SHAPE_PIN_SOURCE_OPENXDOX" in warning, row + assert row["reason"] == ( + "the project's own scripts/validate-manifest.py passes, but warns: " + + warning), row + assert "skipped" not in row["detail"], row + assert "notes" not in row["detail"], row + + +def test_a_chain_whose_link_is_read_is_ok(standard, chain_descendant): + """The control, on every platform: `SHAPE_PIN_SOURCE_OPENXDOX` names the + layer's stand-in, so the link is read, the validator prints no WARNING + and the row is `ok` -- and no `gh` is asked, the pin's digest + recomputing from the same tree.""" + result = run_script( + standard / DOCTOR, "--root", str(chain_descendant["clone"]), + "--json", + env={"SHAPE_PIN_SOURCE_OPENXDOX": str(chain_descendant["layer"])}) + payload = payload_of(result) + rows = rows_of(result) + assert result.returncode == 0, result.stdout + result.stderr + assert payload["verdict"] == "COMPLIANT", payload + assert rows["manifest"]["status"] == "ok", rows["manifest"] + assert "warnings" not in rows["manifest"]["detail"], rows["manifest"] + assert rows["pins"]["status"] == "ok", rows["pins"] + + +@WINDOWS_SKIP +def test_a_red_manifest_row_quotes_its_finding_and_never_the_warning( + standard, chain_descendant, tmp_path): + """#448's decision 4: the same root with the spec leg's `naming.role` + set to `code`, a true `naming-role` finding. The validator prints the + assembly leg's WARNING FIRST and the spec leg's FINDING after it (held + below, so the test means something), and the row quotes the FINDING: + a row that fails never names a note as its reason, and collects none.""" + root = tmp_path / "away" / "codexDox" + shutil.copytree(chain_descendant["clone"], root, symlinks=True) + manifest = root / "project.yaml" + body = manifest.read_bytes() + assert body.count(b" role: spec\n") == 1, body + manifest.write_bytes(body.replace(b" role: spec\n", + b" role: code\n")) + gate = run_script(root / "scripts" / "validate-manifest.py", cwd=root) + assert gate.returncode == 1, gate.stdout + gate.stderr + said = gate.stderr + assert 0 <= said.index("WARNING naming-referent-chain") \ + < said.index("FINDING naming-role"), said + result = doctor_offline(standard, root.resolve(), tmp_path) + row = rows_of(result)["manifest"] + assert row["status"] == "FINDING", row + assert row["detail"]["exit"] == 1, row + assert "crashed" not in row["detail"], row + assert "warnings" not in row["detail"], row + assert ("reports a finding (exit 1): FINDING naming-role: leg 'spec': " + in row["reason"]), row + assert "WARNING" not in row["reason"], row + assert "naming-referent-chain" not in row["reason"], row + + +@pytest.mark.parametrize("output, quoted", [ + ("WARNING w: a note\nFINDING f: the finding\n", "FINDING f: the finding"), + ("WARNING w: a note\n ok a\nREFUSED r: a refusal\n", + "REFUSED r: a refusal"), + (" ok a\nWARNING w: a note\nWARNING v: another\nlast line\n", + "WARNING w: a note"), + (" ok a\nlast line\n\n", "last line"), + ("", ""), +]) +def test_run_validator_quotes_a_verdict_line_before_any_warning( + standard, tmp_path, output, quoted): + """`run_validator`'s quoted line, asked of a child that prints `output` + on stderr and exits 1, on every platform: the first `FINDING` or + `REFUSED`, else the first `WARNING`, else the last line (#448).""" + module = doctor_module(standard) + root = tmp_path / "root" + root.mkdir() + probe = root / "probe.py" + probe.write_text("import sys\n" + f"sys.stderr.write({output!r})\n" + "sys.exit(1)\n", encoding="utf-8") + code, said, whole = module.run_validator(module.Context(root), probe, []) + assert code == 1, whole + assert said == quoted, whole + + +# --- #460's pins-note line: a note is its own item --------------------------- + +SKIP_LINES = (" skip openGlass: pin commit 0123456789ab digest NOT rechecked " + "(gh-unreadable: one\n" + "two). A --pin-source would verify it offline.\n") +SKIP_TEXT = ("openGlass: pin commit 0123456789ab digest NOT rechecked " + "(gh-unreadable: one two). A --pin-source would verify it " + "offline.") +NOTE_LINE = (" note neutral-pin-source-collides: project.yaml declares " + "'open-glass' and 'open_glass'.\n") +NOTE_TEXT = ("neutral-pin-source-collides: project.yaml declares " + "'open-glass' and 'open_glass'.") + + +@pytest.mark.parametrize("order", ["skip-then-note", "note-then-skip"]) +def test_a_note_and_a_skip_each_land_in_their_own_key(standard, project, + tmp_path, order): + """The root's own `validate-pins.py` replaced by a stub that passes after + printing an ` ok `, a two-line ` skip ` and a ` note `, in each order, + on every platform: the row is a `note`, the skip is `detail.skipped` + WHOLE, the note is `detail.notes`, neither swallows the other, the + reason quotes them in the order printed, and the `pins ok` summary is in + neither.""" + blocks = [SKIP_LINES, NOTE_LINE] + if order == "note-then-skip": + blocks.reverse() + stub = ("import sys\n" + "sys.stdout.write(" + repr(" ok spec: pin commit " + "0123456789ab digest recomputes\n" + + "".join(blocks) + "pins ok\n") + + ")\n") + here, root, row_id, _own = crashing("own-pins", standard, project, None, + tmp_path, stub) + row = rows_of(doctor(here, root, "--json"))[row_id] + assert row["status"] == "note", row + assert row["next"] is None, row + assert row["detail"]["exit"] == 0, row + assert row["detail"]["skipped"] == [SKIP_TEXT], row + assert row["detail"]["notes"] == [NOTE_TEXT], row + lead = "the project's own scripts/validate-pins.py passes, but " + said = [f"did not recheck: {SKIP_TEXT}", f"notes: {NOTE_TEXT}"] + if order == "note-then-skip": + said.reverse() + assert row["reason"] == lead + "; ".join(said), row + for text in row["detail"]["skipped"] + row["detail"]["notes"]: + assert "pins ok" not in text, row + assert " ok " not in text, row + + +def test_reported_reads_every_kind_in_the_order_printed(standard): + """`reported` on a constructed output, on every platform: each ` skip `, + `WARNING ` and ` note ` block whole, less its prefix, up to the next + item; and a line before any block, an ` ok` and the summary are none + of them.""" + module = doctor_module(standard) + whole = ("Notice: not part of a block\n" + " ok a: fine\n" + " note n1: one\n" + " more of n1\n" + " skip s1: NOT rechecked (gh-unreadable: x\n" + "y). A --pin-source.\n" + "WARNING w1: a link not read\n" + " note n2: two\n" + " skip s2: NOT rechecked.\n" + "pins ok\n") + said = module.reported(whole) + assert said == [ + ("notes", "n1: one more of n1"), + ("skipped", "s1: NOT rechecked (gh-unreadable: x y). A --pin-source."), + ("warnings", "w1: a link not read"), + ("notes", "n2: two"), + ("skipped", "s2: NOT rechecked.")] + assert module.skipped(whole) == [ + "s1: NOT rechecked (gh-unreadable: x y). A --pin-source.", + "s2: NOT rechecked."] + assert module.passed_but(said) == ( + "notes: n1: one more of n1; did not recheck: s1: NOT rechecked " + "(gh-unreadable: x y). A --pin-source.; warns: w1: a link not read; " + "notes: n2: two; did not recheck: s2: NOT rechecked.") + assert module.passed_but([("skipped", "a"), ("skipped", "b")]) == \ + "did not recheck: a; b" + assert module.reported(" ok a\nmanifest ok: x\n") == [] + diff --git a/tests/test_pin_source_env_collisions.py b/tests/test_pin_source_env_collisions.py index 52207870..4108d5a7 100644 --- a/tests/test_pin_source_env_collisions.py +++ b/tests/test_pin_source_env_collisions.py @@ -854,6 +854,21 @@ def doctor_of(behind, root: Path) -> dict: "rows": {row["id"]: row for row in payload["rows"]}} +def pins_row_says_the_collision(row: dict, names) -> None: + """The doctor's `pins` row over a `validate-pins.py` that passed and + printed the `neutral-pin-source-collides` note: a `note`, with the note + in `detail.notes` and quoted in the reason, and no NEXT, because which + declaration stays is the human's decision (#460's pins-note line). The + row used to say `ok`, and the note reached nobody.""" + assert row["status"] == "note", row + assert row["next"] is None, row + (note,) = row["detail"]["notes"] + assert note.startswith("neutral-pin-source-collides: "), row + assert "SHAPE_PIN_SOURCE_OPEN_GLASS" in note, row + assert all(repr(name) in note for name in names), row + assert row["reason"].endswith(f"passes, but notes: {note}"), row + + @pytest.mark.parametrize("names", [ pytest.param(["open-glass", "open_glass"], id="hyphen-underscore"), pytest.param(["open-glass", "open.glass", "open_glass"], id="three"), @@ -868,14 +883,16 @@ def test_a_project_declaring_a_colliding_pair_takes_a_shape_update( rolled back, for this update and every later one, until a person dropped a true declaration of descent. Now it is a note: the update applies, the project's validator passes and says the note, each pin still verifies from - its own checkout, and the doctor's `pins` row stays `ok` before and after - and moves no verdict a project declaring no pair does not have.""" + its own checkout, and the doctor's `pins` row carries the note before and + after, as a `note` row (#460's pins-note line, #448's ruling), which moves + no verdict a project declaring no pair does not have.""" root = declared_project(behind, tmp_path, names) control = declared_project(behind, tmp_path / "control", ["openGlass"]) before, before_control = doctor_of(behind, root), doctor_of(behind, control) - assert before["rows"]["pins"]["status"] == "ok", before["rows"]["pins"] + pins_row_says_the_collision(before["rows"]["pins"], names) + assert before_control["rows"]["pins"]["status"] == "ok", before_control assert before["rows"]["shape-currency"]["status"] == "FINDING" assert (before["verdict"], before["exit"]) == ( before_control["verdict"], before_control["exit"]), (before, @@ -912,7 +929,8 @@ def test_a_project_declaring_a_colliding_pair_takes_a_shape_update( controlled = update(behind, control, "apply", "--yes") assert controlled.returncode == 0, controlled.stdout + controlled.stderr after, after_control = doctor_of(behind, root), doctor_of(behind, control) - assert after["rows"]["pins"]["status"] == "ok", after["rows"]["pins"] + pins_row_says_the_collision(after["rows"]["pins"], names) + assert after_control["rows"]["pins"]["status"] == "ok", after_control assert after["rows"]["shape-currency"]["status"] == "ok" assert (after["verdict"], after["exit"]) == ( after_control["verdict"], after_control["exit"]), (after,