diff --git a/adopt-project.py b/adopt-project.py index a7e0f635..6302c214 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1919,7 +1919,9 @@ def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: would build it from this plan, FOR VALIDATION AND NEVER FOR WRITING; None where the plan's names are not ones the naming policy classifies, which `check` and `execute` report in their own words (`_check_names`) and the - manifest cannot be rendered without. + manifest cannot be rendered without, and None where its `elected_on:` is + a date `election_date` refuses, which `_elected_on_findings` reports in + the plan key's own words (#294). `_template_table` is asked, not copied, and `execute`'s `_template_values` is that same table with the real commit and digest, so the table cannot @@ -1929,7 +1931,8 @@ def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: `git` runs here and a checkout that cannot name its revision is asked the same question as any other. A refusal of the plan's own names, id, organisation or leg paths is raised here as it is by `check` and `execute` - a few lines later. + a few lines later. The organisation and the id are asked BEFORE the date + is: a bad date does not change which problem either command names first. """ if not isinstance(plan.legs, dict): # The doctor's PLACEMENT plan writes `legs:` as a list (see @@ -1942,7 +1945,21 @@ def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None: _check_names(naming, names, pins) except Refusal: return None + # The organisation and the id are asked BEFORE the date's guard below. + # `checked_value` refuses an `org:` this tool will not put on a command + # line (`unsafe-value`; `execute` meets it first, in `_repository_urls`), + # and `Plan.project_id` refuses an `id:` that is not a project id + # (`plan-bad-id`; `_template_table` reads it again). After the guard, + # neither was asked of a plan whose date is a finding: `check` listed the + # date where `execute` refused the organisation, and refused the id where + # `execute` refused the date. org = checked_value("org", plan.get("org")) + _ = plan.project_id + if _elected_on_findings(plan): + # Reported in the plan key's own words (#294), and the reference the + # manifest records is chosen BY that date when the plan names none: + # there is no manifest to ask about until it is one. + return None repositories = {role: f"{org}/{name}" for role, name in names.items()} (_, spec_path), (_, code_path) = _plan_leg_paths(plan) return _template_table( @@ -1955,8 +1972,8 @@ def _manifest_findings(plan: Plan, misreads_elsewhere: bool = False ) -> tuple[list[str], list[str]]: """What the project's own validator would refuse of this plan's values, as `check` prints them (one FINDING each), and what it prints once after - them. `[]` and `[]` when it would pass, or when the plan's names are - refused in their own words (`_manifest_values`). + them. `[]` and `[]` when it would pass, or when the plan's names, or its + date, are refused in their own words (`_manifest_values`). `misreads_elsewhere` is for a caller whose list ALSO holds a misread rule's finding for each value the project's reader reads back as another @@ -3474,6 +3491,163 @@ def _refused_by_policy(path: str) -> str | None: return None +# --------------------------------------------------------------------------- +# Plan values `execute` read late, asked by `check` before any leg exists +# --------------------------------------------------------------------------- +# +# Each value below was read by `execute` after `check` had said `plan ok`, and +# nothing in between asked what it held. `check` now asks, and `execute` +# refuses what it finds through `_refuse_what_check_finds`, before any leg +# exists. +# +# `follow_ups:` (#293). `plan` writes it as a block list, and every reader +# iterated the value with no test that it was one. A hand-edited +# `follow_ups: 5` (or `true`, `1.5`, `false`) ended `check` in +# `TypeError: 'int' object is not iterable`, a traceback with no FINDING +# naming the key, and on main ended `execute` the same way in +# `_commit_the_split`, after both legs were pushed; `abc` was written into the +# split commit's message as three follow-ups, `a`, `b` and `c`, and an item +# `- true` as `True`. +# +# `elected_on:` (#294). Nothing asked whether it was a date but +# `_template_values`, and only for a plan with no `reference:`: +# `default_reference` asked `election_date` after `_create_leg_remotes` had +# made both legs, and the refusal named `--elected-on`, a flag neither `check` +# nor `execute` takes. With `reference:` kept the date was never asked, and +# `execute` wrote `someday` into the manifest. THE DATE IS ASKED OF +# `election_date`, the rule `plan` asks of `--elected-on` and +# `_template_values` of the plan's date, and of nothing else: what that rule +# admits is #261's to decide, and a tighter rule there is asked here with no +# edit. Only the words are this caller's, because the date came from the plan +# file and not from a flag: the refusal `election_date` raises names +# `--elected-on`, which is right for `plan` and wrong here. + +#: `check`'s finding for a `follow_ups:` that is not a list of text, and the +#: code `execute` refuses it under (#293). +BAD_FOLLOW_UPS = "plan-bad-follow-ups" + +#: Said once after the `follow_ups:` findings. +FOLLOW_UPS_REMEDIATION = ( + "Remediation: write `follow_ups:` in the plan file as `plan` writes it, " + "a list with one item of text per follow-up, each on its own line as " + "` - \"\"`, or `follow_ups: []` for none. `execute` " + "writes each item as one line of the split commit's message, so it takes " + "nothing else: a number or a boolean is not a list, text on the key's own " + "line would be written one character per follow-up, and an item that is " + "not text would be written as Python spells it (`True`, `None`).") + + +def _read_as(value) -> str: + """What the plan's reader handed back for a value, in words, spelled as a + YAML plan spells it where that differs from Python (`true`, `null`).""" + if isinstance(value, bool): + return f"the boolean {str(value).lower()}" + if isinstance(value, (int, float)): + return f"the number {value!r}" + if isinstance(value, str): + return f"the text {value!r}" + if value is None: + return "null" + if isinstance(value, dict): + return f"a mapping of {len(value)} key(s)" + if isinstance(value, list): + return f"a list of {len(value)} item(s)" + return f"a value of type {type(value).__name__}" + + +def _plan_follow_ups(plan: Plan) -> list[str]: + """The plan's follow-ups as `execute` writes them into the split commit's + message: each item of a `follow_ups:` LIST that is text, and nothing for a + `follow_ups:` that is absent, bare or anything but a list. + + THE ONE READ, for the three readers (`_plan_values`, + `_print_what_will_happen`, `_commit_the_split`), so that none of them + iterates a number or spells a string out one character at a time. What it + leaves out is never silently dropped from a run: `_follow_ups_findings` + reports it in `check`, and `execute` has refused it through + `_refuse_what_check_finds` before the split commit is written. + """ + value = plan.get("follow_ups", []) + if not isinstance(value, list): + return [] + return [item for item in value if isinstance(item, str)] + + +def _follow_ups_findings(plan: Plan) -> list[str]: + """A FINDING for a `follow_ups:` that is present and not a list, or for + each item of one that is not text; `[]` for a list of text, an empty + list and a bare key (#293).""" + value = plan.get("follow_ups") + if value is None: + return [] + if not isinstance(value, list): + return [f"FINDING {BAD_FOLLOW_UPS}: `follow_ups:` is " + f"{_read_as(value)}, not a list: `execute` writes each item " + "of a list into the split commit's message as one follow-up"] + return [f"FINDING {BAD_FOLLOW_UPS}: item {number} of `follow_ups:` is " + f"{_read_as(item)}, not text: `execute` writes each item into the " + "split commit's message as one line of text" + for number, item in enumerate(value, 1) + if not isinstance(item, str)] + + +#: `check`'s finding for an `elected_on:` that `election_date` does not read, +#: and the code `execute` refuses it under: the code `plan` refuses a bad +#: `--elected-on` under, said here of the plan's key (#294). +ELECTION_DATE_MALFORMED = "election-date-malformed" + +#: Said once after the `elected_on:` finding. It names the plan file's key and +#: no flag: `check` and `execute` take none for the date. +ELECTED_ON_REMEDIATION = ( + "Remediation: edit `elected_on:` in the plan file to the day the shape " + "was elected, written as a zero-padded ISO date in quotes, e.g. " + "`elected_on: \"2026-09-02\"`: not 2026-9-2, and not a word like " + "'yesterday'. `execute` writes it into the project's `project.yaml`, and " + "a plan with no `reference:` has its reference chosen BY that date, so " + "it is read before anything is created.") + + +def _elected_on_findings(plan: Plan) -> list[str]: + """A FINDING for an `elected_on:` that `election_date` does not read, + whether or not the plan has a `reference:` (#294); `[]` for a date it + reads and for no `elected_on:` at all, which `execute` fills with today's. + + The value asked is the one `_template_values` takes, `str` of what the + plan holds, so the two cannot answer differently. `election_date`'s own + refusal is not passed on, because its words are `plan`'s: they name + `--elected-on`, which neither `check` nor `execute` takes. Only THAT + refusal is a finding about the date: any other `election_date` raises + is about something else (a rule it could not ask), and is raised as it + is. + """ + elected_on = plan.get("elected_on") + if elected_on is None: + return [] + try: + election_date(str(elected_on)) + except Refusal as refused: + if refused.code != ELECTION_DATE_MALFORMED: + raise + chosen = ("" if plan.get("reference") else + ", and, the plan naming no `reference:`, chooses the " + "reference by it") + return [f"FINDING {ELECTION_DATE_MALFORMED}: the plan's `elected_on:` " + f"is {_read_as(elected_on)}, which is not a day written " + "YYYY-MM-DD, the form `execute` writes into the project's " + f"`project.yaml`{chosen}"] + return [] + + +def _plan_field_findings(plan: Plan) -> tuple[list[str], list[str]]: + """`_elected_on_findings` and `_follow_ups_findings`, in the order `plan` + writes the two keys (`elected_on:` before `follow_ups:`), and what `check` + prints once after them.""" + dated, followed = _elected_on_findings(plan), _follow_ups_findings(plan) + return (dated + followed, + ([ELECTED_ON_REMEDIATION] if dated else []) + + ([FOLLOW_UPS_REMEDIATION] if followed else [])) + + #: What a plan whose entry `entry_path_problem` refuses is told to do. Said by #: `check`, once after its findings, and by `execute`'s refusal. UNSAFE_PATH_REMEDIATION = ( @@ -3636,8 +3810,9 @@ def _entry_findings(plan: Plan, source: Source, source's own submodules, then each value `execute` writes into a file that no file can hold (#255), then what the project's own `validate-manifest.py` would refuse of the values `execute` writes into - its manifest (#261), and last each relative url of a kept submodule - that names another repository from its leg (#190). + its manifest (#261), then each relative url of a kept submodule that + names another repository from its leg (#190), and last each plan value + `execute` reads late and `_plan_field_findings` asks about (#293, #294). ONE LIST, read by `check` and by `execute`'s `_refuse_what_check_finds`. #168 and #166 each added findings to `check` and a refusal to `execute` @@ -3673,7 +3848,10 @@ def _entry_findings(plan: Plan, source: Source, validator says is said of the manifest the value would be written into, which is the value's consequence; the byte, the break or the control is what the person retypes. Both are listed, and both refuse before any leg - exists. The relative urls of #190 still come after both, last. + exists. The relative urls of #190 still come after both, and last the + plan values `execute` reads late (#293, #294): an `elected_on:` that + `election_date` refuses is said there in the plan key's own words, and + `_manifest_values` does not ask the validator about it as well. """ tree = source.tree() unsafe, unsafe_remediation = _source_path_findings(plan, tree) @@ -3686,9 +3864,11 @@ def _entry_findings(plan: Plan, source: Source, _submodule_findings(plan, source, tree, local_remote_dir) unwritable_findings, unwritable_remediation = _value_findings(plan) value_findings, value_remediation = _manifest_findings(plan) - return (findings + split + unwritable_findings + value_findings + moved, + field_findings, field_remediation = _plan_field_findings(plan) + return (findings + split + unwritable_findings + value_findings + moved + + field_findings, unsafe_remediation + split_remediation + unwritable_remediation - + value_remediation + moved_remediation, notes) + + value_remediation + moved_remediation + field_remediation, notes) # --------------------------------------------------------------------------- @@ -4078,8 +4258,8 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, f"{len(drops)} dropped") print(" the source is never deleted, never renamed, never force-pushed") print(_topics_line(topic, local)) - for item in plan.get("follow_ups", []): - print(f" follow-up: {_shown(str(item))}") + for item in _plan_follow_ups(plan): + print(f" follow-up: {_shown(item)}") def cmd_check(args) -> int: @@ -5621,7 +5801,7 @@ def _commit_the_split(plan: Plan, source: Source, assembly: Path, print(f" beside {actual} (the source already has {intended}; " "nothing was overwritten)") - follow_ups = [str(f) for f in plan.get("follow_ups", [])] + follow_ups = _plan_follow_ups(plan) message = _split_message(names, paths_for, leg_commits, spec_path, code_path, follow_ups, materialized.collisions, seeded) @@ -5686,10 +5866,12 @@ def cmd_execute(args) -> int: # rendered from `templates/-root/` and needs it. The four leg # commit/digest values are the only ones that cannot be known yet; they # are filled in below, before the assembly root is materialized. And it - # is built before the legs' REPOSITORIES too: it can refuse (an - # `elected_on` that is not a date, a checkout of this standard that is - # not a git repository), and a refusal here used to leave both of them - # behind with nothing saying so (#188). It prints nothing. + # is built before the legs' REPOSITORIES too: it can refuse (a + # checkout of this standard that is not a git repository), and a + # refusal here used to leave both of them behind with nothing saying + # so (#188). An `elected_on:` that is not a date no longer reaches it: + # `_refuse_what_check_finds` has refused it above, as `check` finds it + # (#294). It prints nothing. shape_commit = git_out(["rev-parse", "HEAD"], cwd=SHAPE_ROOT).lower() values = _template_values(plan, names, repositories, urls, spec_path, code_path, tracking, {}, {}, @@ -5836,13 +6018,16 @@ def _plan_values(plan: Plan) -> list[tuple[str, str, str]]: """`(what, value, where)` for each value of the plan's that `execute` writes into a file, as `str` makes it, which is how `_template_values` and `_commit_the_split` take it. A key the plan does not carry is filled - by this tool (`private`, today's date) and is left out.""" + by this tool (`private`, today's date) and is left out. The follow-ups + are `_plan_follow_ups`', the items of text `_commit_the_split` writes; a + `follow_ups:` that is not a list of text is `plan-bad-follow-ups` (#293), + and is never iterated here.""" fields = [(key, str(plan.get(key)), where) for key, where in WRITTEN_PLAN_VALUES.items() if plan.get(key) is not None] fields += [("a `pins:` item", pin, ROOT_MANIFEST) for pin in plan.pins] - return fields + [("a `follow_ups:` item", str(item), SPLIT_MESSAGE) - for item in plan.get("follow_ups", [])] + return fields + [("a `follow_ups:` item", item, SPLIT_MESSAGE) + for item in _plan_follow_ups(plan)] def _value_findings(plan: Plan) -> tuple[list[str], list[str]]: diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index 4c4e5b79..a1f5318a 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -21,8 +21,8 @@ themselves; #168's submodule cases are refused by the same function under the same two codes, `plan-uncovered` and `plan-bad-leg`. -NO NETWORK, NO GITHUB, AND NO `git filter-repo` except in the one end-to-end -test at the bottom. The rest load `adopt-project.py` through importlib and call +NO NETWORK, NO GITHUB, AND NO `git filter-repo` except in the end-to-end tests +at the bottom. The rest load `adopt-project.py` through importlib and call `cmd_execute` in this process. Its probe for `git filter-repo` is stubbed, because every refusal under test is raised BEFORE the tool would be used. A `cmd_execute` that stopped refusing would carry on and make the legs, then @@ -35,6 +35,7 @@ import importlib.util import io +import re import shutil import sys from argparse import Namespace @@ -432,6 +433,381 @@ def test_a_path_that_is_a_git_option_is_still_unsafe_not_uncovered(adopter, no_leg_was_created(world.tmp) +# --- plan values `execute` read late --------------------------------------- + +#: `follow_ups:` and every item under it, as `plan` writes the block; the +#: edits below replace the whole of it. +FOLLOW_UPS_BLOCK = re.compile(r"^follow_ups:\n(?: - .*\n)*", re.M) +BAD_FOLLOW_UPS_CODE = "plan-bad-follow-ups" + +#: What a hand edit can leave `follow_ups:` holding that is not a list of +#: text, by test id: the block it is written as, and what the finding must say +#: of it (#293). The scalars ended `check` in `TypeError: ... is not iterable` +#: with no FINDING, and on main ended `execute` the same way after both legs +#: were pushed; `abc` went into the split commit's message as three +#: follow-ups, `a`, `b` and `c`, and an item `true` as `True`. +BAD_FOLLOW_UPS = { + "int": ("follow_ups: 5\n", "`follow_ups:` is the number 5, not a list"), + "true": ("follow_ups: true\n", + "`follow_ups:` is the boolean true, not a list"), + "false": ("follow_ups: false\n", + "`follow_ups:` is the boolean false, not a list"), + "float": ("follow_ups: 1.5\n", + "`follow_ups:` is the number 1.5, not a list"), + "text": ("follow_ups: abc\n", + "`follow_ups:` is the text 'abc', not a list"), + "mapping": ("follow_ups: {a: 1}\n", + "`follow_ups:` is a mapping of 1 key(s), not a list"), + "item-int": ("follow_ups:\n - 5\n", + "item 1 of `follow_ups:` is the number 5, not text"), + "item-true": ('follow_ups:\n - "keep"\n - true\n', + "item 2 of `follow_ups:` is the boolean true, not text"), + "item-null": ("follow_ups:\n - null\n", + "item 1 of `follow_ups:` is null, not text"), +} + +#: What `check` and `execute` still take, by test id, and the follow-ups each +#: reads: an empty list, a bare key, and a list of text as `plan` writes it. +GOOD_FOLLOW_UPS = { + "empty-list": ("follow_ups: []\n", []), + "bare-key": ("follow_ups:\n", []), + "text-items": ('follow_ups:\n - "Re-point CI at code/"\n' + ' - "merge shape/README.md"\n', + ["Re-point CI at code/", "merge shape/README.md"]), +} + + +def with_follow_ups(text: str, block: str) -> str: + """`text` with its `follow_ups:` block replaced by `block`.""" + edited, done = FOLLOW_UPS_BLOCK.subn(lambda _: block, text, count=1) + assert done == 1, "the plan has no `follow_ups:` block to edit" + return edited + + +def assert_no_traceback(result) -> None: + for output in (result.stdout, result.stderr): + assert "Traceback" not in output, output + + +#: What `execute`'s refusal says of a plan `check` rejects, made before any +#: leg exists, and the first words of the notice #445 prints after a failure +#: once one does. +NO_LEG_MADE = "no leg repository was created and nothing was pushed" +LATE_NOTICE = "NOTHING has been rolled back" + + +def assert_refused_before_any_leg(result) -> None: + """The refusal is `_refuse_what_check_finds`', made before any leg + exists: it says no leg was made, and no notice of what the run left + follows it, as one does a failure once a leg exists (#445).""" + assert NO_LEG_MADE in result.stderr, result.stderr + assert LATE_NOTICE not in result.stderr, result.stderr + + +def findings_of(result) -> list[str]: + return [line for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + + +@pytest.mark.parametrize("edit", sorted(BAD_FOLLOW_UPS)) +def test_check_finds_a_follow_ups_that_is_not_a_list_of_text(world, edit): + """#293: a FINDING that names the key and what it holds, exit 1, and no + traceback. At #270's head `check` read the value in `_plan_values` and + ended in `TypeError: 'int' object is not iterable`; on main it did so in + `_print_what_will_happen`. `abc` printed three follow-ups, `a`, `b` and + `c`, which the summary no longer reads.""" + block, said = BAD_FOLLOW_UPS[edit] + result = run_check(write_edited(world, + with_follow_ups(world.answered, block))) + assert result.returncode == 1, result.stderr + result.stdout + assert_no_traceback(result) + found = findings_of(result) + assert len(found) == 1, found + assert found[0].startswith(f"FINDING {BAD_FOLLOW_UPS_CODE}: {said}: "), \ + found + assert "Remediation: write `follow_ups:` in the plan file" in \ + result.stderr + shown = [line for line in result.stdout.splitlines() + if line.startswith(" follow-up: ")] + assert shown == ([" follow-up: keep"] if '"keep"' in block else []), \ + shown + + +@pytest.mark.parametrize("edit", sorted(BAD_FOLLOW_UPS)) +def test_execute_refuses_a_follow_ups_that_is_not_a_list_of_text_and_creates_no_leg( # noqa: E501 + adopter, world, edit): + """Through `_refuse_what_check_finds`, before any leg exists, under the + finding's own code. At #270's head this was the same `TypeError` raised + from the same list; on main it was raised in `_commit_the_split`, after + both legs were pushed.""" + block, said = BAD_FOLLOW_UPS[edit] + world.plan.write_text(with_follow_ups(world.answered, block), + encoding=UTF8) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == BAD_FOLLOW_UPS_CODE + assert f"FINDING {BAD_FOLLOW_UPS_CODE}: {said}" in refusal.detail + assert "write `follow_ups:` in the plan file" in refusal.remediation + no_leg_was_created(world.tmp) + assert source_branches(world.source) == ["main"] + + +@pytest.mark.parametrize("edit", sorted(GOOD_FOLLOW_UPS)) +def test_a_list_of_text_an_empty_list_and_a_bare_key_still_pass(adopter, + world, edit): + """The controls: what `plan` writes, and the two ways of writing none, + pass `check` and `execute`'s refusal, and the follow-ups read are the + items as written.""" + block, items = GOOD_FOLLOW_UPS[edit] + text = with_follow_ups(world.answered, block) + checked = run_check(write_edited(world, text)) + assert checked.returncode == 0, checked.stderr + checked.stdout + for item in items: + assert f" follow-up: {item}" in checked.stdout + plan, source = open_plan(adopter, world, text) + assert adopter._refuse_what_check_finds(plan, source) is None + + +DATE_CODE = "election-date-malformed" +DATE_FLAG = "--elected-on" + +#: An `elected_on:` that `election_date` refuses on every Python, by test id: +#: as the plan holds it, as the reader reads it, and whether the plan keeps +#: its `reference:` (#294). Without one, `execute` made both legs and only +#: then refused, naming `--elected-on`; with one it never read the date and +#: wrote the word into the manifest. `empty-text` is falsy and not null: only +#: `elected_on:` ABSENT or null is "no date" (`execute` fills in today's), so +#: `""` is a date `election_date` cannot read and is refused like any other. +BAD_DATES = { + "word": ("someday", "someday", False), + "notadate": ("notadate", "notadate", False), + "unpadded": ('"2026-9-2"', "2026-9-2", False), + "not-a-day": ('"2026-02-30"', "2026-02-30", False), + "empty-text": ('""', "", False), + "word-reference-kept": ("someday", "someday", True), +} +ONE_PER_BAD_DATE = pytest.mark.parametrize("edit", sorted(BAD_DATES)) + + +def with_elected_on(text: str, value: str, reference: bool) -> str: + """`text` with `elected_on:` set to `value` by hand, and its `reference:` + line deleted unless `reference`.""" + if not reference: + text, done = re.subn(r"^reference:.*\n", "", text, count=1, + flags=re.M) + assert done == 1, "the plan has no `reference:` line to delete" + edited, done = re.subn(r"^elected_on: .*$", + lambda _: f"elected_on: {value}", text, count=1, + flags=re.M) + assert done == 1, "the plan has no `elected_on:` line to edit" + return edited + + +@ONE_PER_BAD_DATE +def test_check_finds_an_elected_on_it_cannot_read_under_the_plans_key(world, + edit): + """#294: a FINDING `election-date-malformed` that names the plan's + `elected_on:` and no flag, whether or not the plan has a `reference:`. + `check` used to say `plan ok`: it never read the date.""" + value, read, reference = BAD_DATES[edit] + result = run_check(write_edited( + world, with_elected_on(world.answered, value, reference))) + assert result.returncode == 1, result.stderr + result.stdout + assert_no_traceback(result) + found = findings_of(result) + assert len(found) == 1, found + assert found[0].startswith( + f"FINDING {DATE_CODE}: the plan's `elected_on:` is the text " + f"{read!r}, which is not a day written YYYY-MM-DD"), found + assert ("the plan naming no `reference:`" in found[0]) is not reference + # the claim is about what `execute` WRITES, not "the one form it reads": + # on Python 3.11+ `election_date` reads `20261011` too, until #314 lands + assert ", the form `execute` writes into the project's `project.yaml`" in \ + found[0], found + assert "the one form" not in found[0], found + assert "Remediation: edit `elected_on:` in the plan file" in \ + result.stderr + assert DATE_FLAG not in result.stderr + result.stdout + + +@pytest.mark.parametrize("value,read_as", [ + ("false", "the boolean false"), ("0", "the number 0"), + ("[]", "a list of 0 item(s)"), ("{}", "a mapping of 0 key(s)")]) +def test_check_finds_a_falsy_elected_on_that_is_not_null(world, value, + read_as): + """#294: only an `elected_on:` that is absent or null is "no date". Any + other falsy value is a date `election_date` cannot read, so it is a + FINDING that says what the plan holds; `if not elected_on:` in place of + `is None` would skip it and `check` would say `plan ok`. It is not a + `BAD_DATES` row because that test says "the text".""" + result = run_check(write_edited( + world, with_elected_on(world.answered, value, False))) + assert result.returncode == 1, result.stderr + result.stdout + assert_no_traceback(result) + found = findings_of(result) + assert len(found) == 1, found + assert found[0].startswith( + f"FINDING {DATE_CODE}: the plan's `elected_on:` is {read_as}, which " + "is not a day written YYYY-MM-DD"), found + + +@ONE_PER_BAD_DATE +def test_execute_refuses_an_elected_on_it_cannot_read_before_any_leg( + adopter, world, edit): + """The same finding, refused through `_refuse_what_check_finds` before + `_create_leg_remotes`. With no `reference:`, `execute` used to create + both legs and then refuse from `default_reference`, in words that named + `--elected-on`; with one, it went on and wrote the word into the + manifest.""" + value, read, reference = BAD_DATES[edit] + world.plan.write_text(with_elected_on(world.answered, value, reference), + encoding=UTF8) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == DATE_CODE + no_leg_was_created(world.tmp) + assert source_branches(world.source) == ["main"] + assert f"the plan's `elected_on:` is the text {read!r}" in refusal.detail + assert "edit `elected_on:` in the plan file" in refusal.remediation + assert DATE_FLAG not in str(refusal) + + +#: An `org:` or `id:` the plan refuses in its own words, by test id: the line +#: `plan` writes, the hand edit, and the code `check` and `execute` both +#: refuse it under. Beside an `elected_on:` that is a finding, the date's +#: guard in `_manifest_values` used to return before either was asked, so +#: the two commands named different first problems. +REFUSED_BESIDE_A_BAD_DATE = { + "org-unsafe": ("org: testorg", 'org: "-evil"', "unsafe-value"), + "id-bad": ("id: northwind", 'id: "North Wind"', "plan-bad-id"), +} + + +@pytest.mark.parametrize("reference", [True, False], + ids=["reference-kept", "no-reference"]) +@pytest.mark.parametrize("edit", sorted(REFUSED_BESIDE_A_BAD_DATE)) +def test_a_bad_date_does_not_change_what_check_and_execute_refuse_first( + adopter, world, edit, reference): + """A plan with `elected_on: someday` AND an `org:` or `id:` it refuses: + `check` and `execute` refuse it under the same code, the organisation's + or the id's, as they do with no date guard, and no leg is created. With + both asked after the guard, `check` listed only the date (exit 1) where + `execute` refused `unsafe-value`, and `check` refused `plan-bad-id` where + `execute` refused the date.""" + line, edited, code = REFUSED_BESIDE_A_BAD_DATE[edit] + text = with_elected_on(world.answered, "someday", reference) + assert line in text, f"`plan` wrote no `{line}` line" + plan = write_edited(world, text.replace(line, edited, 1)) + + checked = run_check(plan) + assert checked.returncode == 2, checked.stderr + checked.stdout + assert_no_traceback(checked) + first = next((said for said in checked.stderr.splitlines() + if said.startswith("REFUSED ")), "") + assert first.startswith(f"REFUSED {code}: "), checked.stderr + + refusal = refused_by(adopter, execute_args(world.tmp, plan)) + assert refusal.code == code + no_leg_was_created(world.tmp) + assert source_branches(world.source) == ["main"] + + +@pytest.mark.parametrize("value,reference", [ + ('"2026-09-02"', False), ('"2026-06-01"', False), ('"2026-09-02"', True), + ("null", False)]) +def test_an_elected_on_election_date_reads_still_passes(adopter, world, value, + reference): + """The controls: a date, with or without the `reference:` it chooses, and + no date at all, which `execute` fills with today's.""" + text = with_elected_on(world.answered, value, reference) + checked = run_check(write_edited(world, text)) + assert checked.returncode == 0, checked.stderr + checked.stdout + plan, source = open_plan(adopter, world, text) + assert adopter._refuse_what_check_finds(plan, source) is None + + +@pytest.mark.parametrize("value", ["2026-W41-6", "20261011", "2026-09-02", + "someday"]) +def test_check_asks_election_date_itself_of_the_plans_date(adopter, world, + value): + """ONE QUESTION, `plan`'s. `check` asks the plan's `elected_on:` what + `plan` asks `--elected-on`, `election_date` on this interpreter, and + nothing of its own: `2026-W41-6` and `20261011` are dates to + `date.fromisoformat` from Python 3.11 and not to the project's own + validator, which is #261's to rule on. Whatever `election_date` answers, + `check` answers the same, so `plan`, `check` and `execute` cannot + disagree about one value.""" + try: + adopter.election_date(value) + refused = False + except adopter.Refusal: + refused = True + result = run_check(write_edited( + world, with_elected_on(world.answered, f'"{value}"', False))) + assert result.returncode == (1 if refused else 0), \ + result.stderr + result.stdout + assert (f"FINDING {DATE_CODE}:" in result.stderr) is refused + + +def test_a_tighter_election_date_is_asked_with_no_edit(adopter, world, + monkeypatch): + """`election_date` held to `YYYY-MM-DD` on every Python, as #261 asks of + it, stood in for here: `check`'s list follows it, and `execute` refuses + `2026-W41-6` before any leg. The stand-in's own words name + `--elected-on`; the refusal says the plan's key and not them.""" + real = adopter.election_date + + def held_to_the_form(elected_on): + if not re.fullmatch(r"\d{4}-\d{2}-\d{2}", str(elected_on)): + raise adopter.Refusal( + DATE_CODE, f"{DATE_FLAG} {elected_on!r} is not YYYY-MM-DD") + return real(elected_on) + + monkeypatch.setattr(adopter, "election_date", held_to_the_form) + world.plan.write_text( + with_elected_on(world.answered, '"2026-W41-6"', False), encoding=UTF8) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == DATE_CODE + assert "the plan's `elected_on:` is the text '2026-W41-6'" in \ + refusal.detail + assert DATE_FLAG not in str(refusal) + no_leg_was_created(world.tmp) + + +def test_another_refusal_from_election_date_is_not_a_date_finding( + adopter, world, monkeypatch): + """Only `election-date-malformed` is a finding about the date. Anything + else `election_date` raises -- a rule it could not ask, as when the + validator it may consult is missing -- is raised as it is, and a date it + never judged is not reported as malformed.""" + def cannot_ask(elected_on): + raise adopter.Refusal("rule-unusable", "the rule could not be asked") + + monkeypatch.setattr(adopter, "election_date", cannot_ask) + plan, source = open_plan(adopter, world, world.answered) + with pytest.raises(adopter.Refusal) as caught: + adopter._refuse_what_check_finds(plan, source) + assert caught.value.code == "rule-unusable" + + +def test_both_findings_are_listed_and_the_code_is_the_first(adopter, world): + """Both keys wrong at once: one list, the date first, as `plan` writes the + two keys, and each remediation once.""" + text = with_follow_ups(with_elected_on(world.answered, "someday", False), + "follow_ups: 5\n") + printed = check_findings(write_edited(world, text)) + assert [line.split(":", 1)[0] for line in printed] == [ + f"FINDING {DATE_CODE}", f"FINDING {BAD_FOLLOW_UPS_CODE}"] + plan, source = open_plan(adopter, world, text) + with pytest.raises(adopter.Refusal) as caught: + adopter._refuse_what_check_finds(plan, source) + assert caught.value.code == DATE_CODE + for finding in printed: + assert finding in caught.value.detail + assert caught.value.remediation.count("Remediation: edit `elected_on:`") \ + == 1 + assert caught.value.remediation.count( + "Remediation: write `follow_ups:`") == 1 + + # --- a plan that is not UTF-8 (#192) ---------------------------------------- #: The byte `0xff` is in no UTF-8 text, so wherever it lands the file cannot be @@ -541,3 +917,61 @@ def test_an_uncovered_plan_creates_no_leg_and_the_corrected_plan_then_runs( assert "adoption verified" in corrected.stdout for role in ROLES: assert (remotes / f"{PROJECT}-{role}.git").is_dir() + + +@needs_filter_repo +def test_a_follow_ups_that_is_not_a_list_creates_no_leg_and_the_corrected_plan_runs( # noqa: E501 + world): + """#293 as a person met it, through `main`: exit 2, a REFUSED line, no + traceback and no leg, so the corrected plan does not meet + `leg-remote-exists`.""" + remotes, work = world.tmp / REMOTES, world.tmp / WORK + flags = ("--plan", str(world.plan), "--yes", "--local-remote-dir", + str(remotes), "--work-dir", str(work)) + world.plan.write_text(with_follow_ups(world.answered, "follow_ups: 5\n"), + encoding=UTF8) + refused = run_script(ADOPT, "execute", *flags) + assert refused.returncode == 2, refused.stderr + refused.stdout + assert f"REFUSED {BAD_FOLLOW_UPS_CODE}: " in refused.stderr + assert_no_traceback(refused) + assert_refused_before_any_leg(refused) + assert "creating the leg repositories" not in refused.stdout + assert not remotes.exists(), "not even the directory the legs go in" + assert source_branches(world.source) == ["main"] + + world.plan.write_text(world.answered, encoding=UTF8) + corrected = run_script(ADOPT, "execute", *flags) + assert corrected.returncode == 0, corrected.stderr + corrected.stdout + assert "adoption verified" in corrected.stdout + + +@needs_filter_repo +def test_an_elected_on_it_cannot_read_creates_no_leg_and_the_corrected_plan_runs( # noqa: E501 + adopter, world): + """#294 as a person met it, through `main`: exit 2, a REFUSED line that + names no flag, no traceback and no leg, so the corrected plan does not + meet `leg-remote-exists`. The corrected plan has no `reference:`, and the + manifest records the one its date chooses.""" + remotes, work = world.tmp / REMOTES, world.tmp / WORK + flags = ("--plan", str(world.plan), "--yes", "--local-remote-dir", + str(remotes), "--work-dir", str(work)) + world.plan.write_text(with_elected_on(world.answered, "someday", False), + encoding=UTF8) + refused = run_script(ADOPT, "execute", *flags) + assert refused.returncode == 2, refused.stderr + refused.stdout + assert f"REFUSED {DATE_CODE}: " in refused.stderr + assert_no_traceback(refused) + assert_refused_before_any_leg(refused) + assert DATE_FLAG not in refused.stderr + assert "creating the leg repositories" not in refused.stdout + assert not remotes.exists(), "not even the directory the legs go in" + assert source_branches(world.source) == ["main"] + + world.plan.write_text( + with_elected_on(world.answered, '"2026-09-02"', False), encoding=UTF8) + corrected = run_script(ADOPT, "execute", *flags) + assert corrected.returncode == 0, corrected.stderr + corrected.stdout + assert "adoption verified" in corrected.stdout + manifest = adopter.load_yaml(work / PROJECT / "project.yaml") + assert manifest["reference"] == adopter.default_reference("2026-09-02") + assert str(manifest["elected_on"]) == "2026-09-02" diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index 491ada1d..9c3711db 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -89,12 +89,30 @@ id="elected-on-basic-format"), ] +#: An `elected_on:` that `election_date` refuses (`2026-W41-6` and `20261011` +#: since #261) is ONE finding, #294's `election-date-malformed`, in the plan +#: key's own words: `_manifest_values` does not ask the validator about a +#: manifest whose reference that date chooses. The rows below that edit +#: `elected_on:` expect it, and not the validator's `plan-manifest-invalid`. +BAD_DATE = "election-date-malformed" + + +def said_of(new: str) -> tuple[str, str, str]: + """`(the FINDING, the REFUSED prefix, the remediation)` that `check` and + `execute` give the value a row's edited line `new` holds.""" + if new.startswith("elected_on:"): + return (f"FINDING {BAD_DATE}", f"REFUSED {BAD_DATE}", + "edit `elected_on:` in the plan file") + return (FINDING, "REFUSED plan-manifest-", + "correct each one named here in the plan") + + #: `(old line, new line, what `check` names)`: the same values, edited into a #: plan by hand, which is what a plan is for. #: A text value the reader reads as a number (`elected_on: "20261011"`) is #: not here but in `TEXT_THE_READER_MISREADS` below: where a misread rule is #: also in the list (#181) it is that rule's finding, and these rows name the -#: validator's. +#: validator's, or `said_of`'s for the date. PLAN_EDITS = [ pytest.param("visibility: private", "visibility: secret", ("visibility is 'secret'", "manifest-visibility"), @@ -112,7 +130,8 @@ 'reference: " "', ("reference is ' '", "manifest-reference"), id="reference-blank"), pytest.param('elected_on: "2026-09-02"', 'elected_on: "2026-W41-6"', - ("elected_on is '2026-W41-6'", "manifest-elected-on"), + ("the plan's `elected_on:` is the text '2026-W41-6'", + "not a day written YYYY-MM-DD"), id="elected-on-iso-week"), pytest.param("org: testorg", "org: test+org", ("org is 'test+org'", "manifest-leg-repository"), @@ -250,15 +269,16 @@ def test_a_plan_with_no_such_edit_still_passes_check(world): @pytest.mark.parametrize("old, new, named", PLAN_EDITS) def test_check_finds_a_value_the_validator_refuses(world, old, new, named): + finding, _, remediation = said_of(new) plan = edited(world, old, new) result = run_script(ADOPT, "check", "--plan", str(plan)) assert result.returncode == 1, result.stderr + result.stdout findings = [line for line in result.stderr.splitlines() if line.startswith("FINDING ")] - assert [f for f in findings if f.startswith(FINDING)], result.stderr + assert [f for f in findings if f.startswith(finding)], result.stderr for fragment in named: assert fragment in result.stderr, result.stderr - assert "correct each one named here in the plan" in result.stderr + assert remediation in result.stderr @needs_filter_repo @@ -266,6 +286,7 @@ def test_check_finds_a_value_the_validator_refuses(world, old, new, named): def test_execute_refuses_it_before_any_leg_exists(world, old, new, named): """Each row ended `adoption verified` on main, with both legs pushed and a first bootstrap that failed.""" + finding, refused, _ = said_of(new) plan = edited(world, old, new) result = execute(world, plan) assert result.returncode == 2, result.stderr + result.stdout @@ -273,8 +294,8 @@ def test_execute_refuses_it_before_any_leg_exists(world, old, new, named): # value another rule also judges (a number-looking `elected_on` is a # misread on #181) may be listed before this one, so what is asked is # that this finding is in it. - assert "REFUSED plan-manifest-" in result.stderr - assert f"{FINDING}: " in result.stderr + assert refused in result.stderr + assert f"{finding}: " in result.stderr for fragment in named: assert fragment in result.stderr, result.stderr assert "adoption verified" not in result.stdout @@ -508,7 +529,7 @@ def test_check_finds_it_from_a_tree_with_no_git_as_in_a_checkout( "--plan", str(plan)) assert without_git.returncode == 1, (without_git.stderr + without_git.stdout) - assert FINDING in without_git.stderr, without_git.stderr + assert said_of(new)[0] in without_git.stderr, without_git.stderr for fragment in named: assert fragment in without_git.stderr, without_git.stderr in_checkout = run_script(ADOPT, "check", "--plan", str(plan)) @@ -664,6 +685,8 @@ def test_a_value_the_reader_reads_as_another_kind_is_not_this_questions( #: which the template writes bare and the project's reader reads as a number #: or a boolean. The validator refuses each; where a misread rule is also in #: the list (#181) it refuses the same value, and the sentence should be one. +#: `20261011` is also a date `election_date` refuses, so its one sentence is +#: #294's (`said_of`), and the validator is not asked about it. TEXT_THE_READER_MISREADS = [ pytest.param("id: northwind", 'id: "007"', "manifest-id: id is 7", id="id-007"), @@ -671,7 +694,7 @@ def test_a_value_the_reader_reads_as_another_kind_is_not_this_questions( "manifest-visibility: visibility is True", id="visibility-true"), pytest.param('elected_on: "2026-09-02"', 'elected_on: "20261011"', - "manifest-elected-on: elected_on is 20261011", + "the plan's `elected_on:` is the text '20261011'", id="elected-on-basic-format"), ] @@ -699,7 +722,7 @@ def test_execute_refuses_a_text_value_the_reader_misreads_before_any_leg( plan = edited(world, old, new) result = execute(world, plan) assert result.returncode == 2, result.stderr + result.stdout - assert "REFUSED plan-manifest-" in result.stderr + assert said_of(new)[1] in result.stderr assert new.split(": ", 1)[1].strip('"') in result.stderr assert "adoption verified" not in result.stdout assert "Traceback" not in result.stderr @@ -711,11 +734,17 @@ def test_a_text_value_the_reader_misreads_is_one_finding_on_this_rule_alone( adopter, world, old, new, named): """With nothing else asking the misread question the validator's finding is the only one, so `check` and `execute` still refuse the value: it must - not be dropped for want of a rule that is not here.""" + not be dropped for want of a rule that is not here. A date `election_date` + refuses is the one finding of #294's rule, which the list always holds, + and the validator is not asked about it as well.""" plan = adopter.Plan.load(edited(world, old, new)) findings, remediation = adopter._manifest_findings(plan) + finding = said_of(new)[0] + if finding != FINDING: + assert (findings, remediation) == ([], []) + findings, remediation = adopter._plan_field_findings(plan) assert len(findings) == 1, findings - assert findings[0].startswith(FINDING) + assert findings[0].startswith(finding) assert named in findings[0] assert remediation