From c35c8964c523dc6627a788010a1d0eb04311f2ea Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:21:09 +0000 Subject: [PATCH] adopt-project.py: `plan` asks about every source path `execute` will not extract as it is named, with the reason and `root` or `drop` as its only answers, so `-notes.md` is no longer planned `leg: spec` for `check` to refuse, and #166's split question says the same of such an entry (#319) Since #196 `check` and `execute` ask one test of an entry's path, `entry_path_problem`, and `plan` asked it nothing. A source holding `-notes.md`, `notes\t.md`, `a\rb.md`, `a\nb.md`, `a==>b.md` or a name that is not UTF-8 was planned `leg: spec`, `review_required: false`, with no question; `check` then refused that answer as `plan-unsafe-path`, and `execute` refused it before any leg. `Icon\r` and ` `, which no rule places, were asked the policy's question, which offers `spec` and `code`. #166's split question named such a path as an entry to write, with any `leg:`. - `ask_about_refused_paths` runs after `ask_about_submodules`. Each entry the test refuses that is not answered `root` is written `leg: null`, `review_required: true`, confidence `review`, the classifier's rule and reason kept. Its `question:` says where the rules put it, or keeps the policy's question first; then that `execute` will not extract it into a leg as it is named, with `entry_path_problem`'s reason; that `root` or `drop` is the only answer `execute` takes; and that renaming it in the source and re-running `plan` is the way into a leg. - An entry that also holds a submodule is told `drop` is its only answer, because `root` is `plan-submodule-root`. - `_asked_about`'s split sentence names each replacement entry the test refuses, with its reason, and says `root` or `drop` (`drop` for a submodule). Past SPLIT_ENTRIES_NAMED of them it counts the rest. - What the rules imply (follow-ups, predicted collisions, the `seeding:` record) is read with such an entry asked, because no answer puts it in a leg. A leg only that path fed is recorded as seeded, with the `--allow-empty-leg` warning, which matches what `check` says of the plan as written and once it is answered. - `_asked_about` sets its question as written. `Verdict` collapsed each run of spaces, so `src/a b.md` was named `src/a b.md`, an entry that covers nothing. Unchanged: `check` and `execute` (no line of either is touched); `shape-doctor.py`'s placement plan (it calls `walk` only); `--help`. The plan file and `plan`'s report are byte-identical on main and here for the synthetic tree, a source with a registered submodule, and one holding admitted names (`My Notes.md`, `#notes.md`, `[ID].tsx`, ` lead.md`), diffed. Changed with it: #228's two tests of a spec directory that begins with `-` (`-notes/`, and `-Otouch pager-ran;/`, named like `git grep`'s pager option) read `leg: spec` and the follow-up from the plan. `plan` now asks about such a directory and writes no follow-up for it, because no answer `execute` takes moves it to the spec leg. Each test now asserts the plan's question and that the plan names no file for it, and still asks #228's literal `git grep`, of the rules' own answer `spec` through `cross_leg_references`: the same follow-up, and nothing run. Every other assertion is kept, the `leg: spec` one as the rules' answer. Left out: an entry the rules answer `root` whose path the test refuses stays `root` with no note, because `check` and `execute` accept it. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 154 +++++++++++++-- tests/test_adopt_entry_paths.py | 308 +++++++++++++++++++++++++++++- tests/test_adopt_literal_names.py | 84 +++++++- tests/test_adopt_plan_escapes.py | 6 +- tests/test_adopt_submodules.py | 135 ++++++++++++- 5 files changed, 648 insertions(+), 39 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index aaf2c5bc..cc8309d7 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -19,8 +19,9 @@ plan walks the source at its default branch, classifies every top-level path against `contracts/path-classification.yaml`, and writes - `adoption-plan.yaml`. Paths it cannot honestly call, and every - path holding one of the source's own submodules, carry + `adoption-plan.yaml`. Paths it cannot honestly call, every path + holding one of the source's own submodules, and every path + `execute` will not extract into a leg as it is named, carry `leg: null`, `review_required: true` and the QUESTION to ask. A leg path over a path it keeps in the root itself is a NOTE, not a refusal: the answer is the person's to change. @@ -993,20 +994,133 @@ def _asked_about(entry: Entry, held: list[str], asked: Verdict, own: list[str]) -> Entry: """`entry` as `ask_about_submodules` writes it: the `.gitmodules` verdict, whose `leg:` is null, and its question naming `held` and, when - it holds more than one path, the `own` entries that replace it. Each - path is `_spelled`, because the question is a line of the plan.""" - question = (f"{asked.question or ''} This entry holds the submodule(s) " - f"{_listed(held)}: it is asked this question, not " - f"classified by its rule `{entry.rule}`.") + it holds more than one path, the `own` entries that replace it, and + which of those `execute` will not extract as they are named + (`_refused_replacements`, #319). Each path is `_spelled`, because the + question is a line of the plan, and the question is set on the entry + as it is written: `Verdict` folds each run of spaces in a question to + one, and a name holding two, `src/a b.md`, was named `src/a b.md`, an + entry that covers nothing.""" + question = (f"{asked.question} " if asked.question else "") + ( + f"This entry holds the submodule(s) {_listed(held)}: it is asked " + f"this question, not classified by its rule `{entry.rule}`.") if len(own) > 1: question += (" Split the entry if its paths do not all belong in one " f"leg: replace it with " f"{_replacements(entry.path, held, own)}, each with its " "own `leg:`. An entry added beside this one for a path " - "it covers is `plan-covered-twice`.") - return Entry(entry.path, Verdict(None, asked.rule, asked.reason, - asked.confidence, question), - entry.files, entry.bytes) + "it covers is `plan-covered-twice`." + + _refused_replacements(own, held)) + out = Entry(entry.path, Verdict(None, asked.rule, asked.reason, + asked.confidence), entry.files, + entry.bytes) + out.question = question + return out + + +def _refused_replacements(own: list[str], held: list[str]) -> str: + """What the question `_asked_about` writes says of the entries `own` + that replace a directory entry, when `execute` will not extract one of + them into a leg as it is named (`entry_path_problem`, #319): each such + entry by name, with the reason, and that `root` or `drop` is its only + answer -- `drop` for a submodule, which the assembly root never keeps. + Past SPLIT_ENTRIES_NAMED of them the rest are counted. Empty when there + is none, so the question is what it was. + + Under the directory entry such a name is never written on its own, and + `check` judges nothing below it; as an entry of its own, the edit the + question names, it is judged, and `check` refused the answer `spec` or + `code` as `plan-unsafe-path` though this question had offered both.""" + refused = [(path, entry_path_problem(path)) for path in own] + refused = [(path, problem) for path, problem in refused if problem] + if not refused: + return "" + if len(refused) == 1: + [(path, problem)] = refused + answer = ("`drop` is the only answer for it: the assembly root keeps " + "no submodule of the source's" if path in held else + "`root` or `drop` is the only answer for it") + return (f" Of those entries, `execute` will not extract " + f"{_spelled(path)} into a leg as it is named, because " + f"{problem}; so {answer}.") + submodule = (", and `drop` for a submodule, which the assembly root " + "never keeps" if any(path in held for path, _ in refused) + else "") + named = "; ".join(f"{_spelled(path)}, because {problem}" + for path, problem in refused[:SPLIT_ENTRIES_NAMED]) + more = len(refused) - SPLIT_ENTRIES_NAMED + if more > 0: + named += (f"; and {more} more, each `plan-unsafe-path` in `check` " + "if it is answered `spec` or `code`") + return (" Of those entries, `execute` will not extract these into a leg " + "as they are named, so `root` or `drop` is the only answer for " + f"each{submodule}: {named}.") + + +def ask_about_refused_paths(entries: list[Entry], tree: list) -> list[Entry]: + """`entries`, with every one `execute` will not extract into a leg as it + is named ASKED, rather than answered `spec` or `code` (#319). + + `entry_path_problem` is the one test `check` and `execute` ask of an + entry's path (#196), and `plan` asked it nothing: a source holding + `-notes.md`, `notes\\t.md`, `a\\rb.md`, `a\\nb.md`, `a==>b.md` or a name + that is not UTF-8 was planned `leg: spec`, `review_required: false`, no + question, and the first anybody heard of it was `check`'s + `plan-unsafe-path`: an answer `plan` chose for the person, refused one + command later. `Icon\\r`, which no rule places, was asked the policy's + question, which offers `spec` and `code` as well. + + So each entry the test refuses that is not answered `root` -- the rules + put it in a leg, or ask -- gets `leg: null` and a question that says why + `execute` will not extract it, in the test's words, and that `root` or + `drop` is the only answer for it, as UNSAFE_PATH_REMEDIATION says after + `check`'s finding; renaming it in the source is the way into a leg. An + entry that holds a submodule is told `drop`: the assembly root keeps no + submodule of the source's (`plan-submodule-root`). A question the entry + carries already -- the policy's, or `.gitmodules`' -- is kept, and this + one follows it. An entry answered `root` is left as it is: it is never + extracted (NOT_EXTRACTED), and `check` and `execute` take it. The rule + and the reason stay the classifier's, so the plan still says where the + rules would have put it. + + `check` and `execute` are not changed: they ask the same test of + whatever the person answers, as they always did. The question is set on + the entry as it is written, as `_asked_about` sets one, so no run of + spaces in a name it spells is folded. + """ + gitlinks = [path for path, mode, _, _ in tree if mode == GITLINK_MODE] + out = [] + for entry in entries: + problem = (None if entry.leg in NOT_EXTRACTED + else entry_path_problem(entry.path)) + if problem is None: + out.append(entry) + continue + held = any(_covering([entry.path], path) for path in gitlinks) + out.append(_refused_as_named(entry, problem, held)) + return out + + +def _refused_as_named(entry: Entry, problem: str, held: bool) -> Entry: + """`entry` as `ask_about_refused_paths` writes it: `leg: null`, its rule + and reason, and the question, after any it carries already.""" + if entry.leg is not None: + lead = (f"The rules put this path in the {entry.leg} leg (rule " + f"`{entry.rule}`), but `execute` will not extract it") + else: + lead = (f"{entry.question} But " if entry.question else "") \ + + "`execute` will not extract this path" + answer = ("`drop` is the only answer `execute` takes for this entry as it " + "stands: `root` would keep a submodule in the assembly root, " + "which keeps none of the source's" if held else + "`root` or `drop` is the only answer `execute` takes for it") + out = Entry(entry.path, Verdict(None, entry.rule, entry.reason, + entry.confidence), entry.files, + entry.bytes) + out.question = (f"{lead} into a leg as it is named, because {problem}. " + f"So {answer}. To put it in a leg, rename it in the " + "source, commit, and re-run `plan`.") + return out def _replacements(entry: str, held: list[str], own: list[str]) -> str: @@ -1730,12 +1844,22 @@ def cmd_plan(args) -> int: # called the code leg empty, so the plan recorded it as seeded and told # the person to re-run with `--allow-empty-leg code`, which it never # needed (#166's NEW-1). A source with no submodule plans as it did. + # + # A path `execute` will not extract as it is named is the other way + # round (#319): `root` and `drop` are the only answers it takes, so the + # path is in no leg whatever the person says, and what the rules imply + # is read with it ASKED. A follow-up about a code path no leg will hold, + # or naming it as a spec directory, would be about nothing, and a leg it + # alone fed is seeded -- which the plan records, as `check` will say of + # the plan as written. A source with no such path plans as it did. ruled = walk(policy, [(path, size) for path, _, _, size in tree]) - materialized = _predict_collisions(ruled) - follow_ups = follow_ups_for(source, ruled, names, args.spec_path, + implied = ask_about_refused_paths(ruled, tree) + materialized = _predict_collisions(implied) + follow_ups = follow_ups_for(source, implied, names, args.spec_path, args.code_path, materialized) - seeded = seeded_legs(assigned_paths(ruled)) - entries = ask_about_submodules(policy, ruled, tree) + seeded = seeded_legs(assigned_paths(implied)) + entries = ask_about_refused_paths( + ask_about_submodules(policy, ruled, tree), tree) out = Path(args.out) write_lf(out, render_plan(args, source, entries, names, pins, follow_ups, seeded)) diff --git a/tests/test_adopt_entry_paths.py b/tests/test_adopt_entry_paths.py index 1a2e1d23..c5afddb2 100644 --- a/tests/test_adopt_entry_paths.py +++ b/tests/test_adopt_entry_paths.py @@ -11,6 +11,11 @@ outside ASCII, `#`, `?`, `[`, `;` -- is extracted, because every place such a path reaches `git` reads it literally. +`plan` asks the same test since #319: an entry it refuses is written +`leg: null` with a question giving the reason and the only two answers, +`root` and `drop`, where `plan` used to answer `spec` for it and leave +`check` to refuse that answer. + NO NETWORK AND NO GITHUB. `git filter-repo` is needed only by the end-to-end test at the bottom, which skips without it as `test_adopt_e2e.py` does; the Windows job installs none, so there it is the tests above it that run. They @@ -36,7 +41,8 @@ import pytest -from conftest import ADOPT, REPO, git, make_source_repo, run_script, write_plan +from conftest import (ADOPT, REPO, blank_unnamed_pin_sources, git, + make_source_repo, run_script, write_plan) PROJECT = "Northwind" UTF8 = "utf-8" @@ -307,15 +313,33 @@ def test_execute_refuses_it_in_checks_words_before_any_leg(adopter, tmp_path, no_leg_was_created(tmp_path) +def asked(tmp_path: Path, tree: dict, answers: dict) -> tuple[Path, Path]: + """`(source, plan)` for a source holding `tree`, with each entry + `answers` names, as the plan file spells it, answered as `answers` + says. Every other entry needs no answer: `plan` asks only about those + (#319).""" + source = make_source_repo(tmp_path / "Thing", tree, edits=()) + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + for spelled, leg in answers.items(): + answer(plan, spelled, "null", leg) + assert "leg: null" not in plan.read_text(encoding=UTF8) + return source, plan + + @pytest.mark.parametrize("answered", ["root", "drop"]) def test_an_entry_answered_root_or_drop_is_not_judged(adopter, tmp_path, answered): """A path that is never extracted reaches no line of the filter's list: `root` keeps it where it is and `drop` hands it to `git rm` after `--`. - So the remediation's `drop` is true; answered `spec` it is refused.""" - source, plan = planned(tmp_path, {"-notes.md": "notes\n", - "src/app.py": "VALUE = 1\n", - "docs/spec.md": "# spec\n"}) + So the remediation's `drop` is true; answered `spec` it is refused. + `plan` asks about it since #319, so `spec` is answered here by hand, as + `plan` used to write it.""" + source, plan = asked(tmp_path, {"-notes.md": "notes\n", + "src/app.py": "VALUE = 1\n", + "docs/spec.md": "# spec\n"}, + {'"-notes.md"': "spec"}) findings = [line for line in run_check(plan).stderr.splitlines() if line.startswith(f"FINDING {UNSAFE}:")] assert findings == [f"FINDING {UNSAFE}: `-notes.md` has leg: spec, and " @@ -334,10 +358,12 @@ def test_an_entry_answered_root_or_drop_is_not_judged(adopter, tmp_path, def test_a_control_character_in_the_source_is_found_and_refused(adopter, tmp_path): """Not a hand edit: the SOURCE holds `notes\\t.md`, `plan` writes it into - the plan as it stands, and the plan parses. `check` names it and - `execute` refuses it before any leg exists.""" - source, plan = planned(tmp_path, {"notes\t.md": "notes\n", - "src/app.py": "VALUE = 1\n"}) + the plan as it stands, and the plan parses. Answered `spec` -- what + `plan` wrote for it before #319 -- `check` names it and `execute` + refuses it before any leg exists.""" + source, plan = asked(tmp_path, {"notes\t.md": "notes\n", + "src/app.py": "VALUE = 1\n"}, + {'"notes\\t.md"': "spec"}) result = run_check(plan) assert result.returncode == 1, result.stderr + result.stdout assert f"FINDING {UNSAFE}: `notes\\t.md` has leg: spec" in result.stderr @@ -349,6 +375,227 @@ def test_a_control_character_in_the_source_is_found_and_refused(adopter, cwd=source).stdout.split() == ["main"] +# --- `plan` asks about them (#319) ---------------------------------------- +# +# On main at 71b58a1 `plan` asked `entry_path_problem` nothing: each name +# below that a rule places was written `leg: spec`, `review_required: +# false`, no question, and `check` then refused that answer as +# `plan-unsafe-path`; `Icon\r` and ` `, which no rule places, were asked +# the policy's question, which offers `spec` and `code` too. Now each is +# asked, with the reason, and offered the only two answers `execute` takes. + +#: `(the name as git records it, as the plan file spells it, a piece of the +#: question as the plan FILE shows it)`: one of each kind +#: `entry_path_problem` refuses that a source tree can hold, at the top +#: level, where it is an entry of its own. `.md` is spec by the extension +#: table; `Icon\r` and ` ` no rule places. The piece is the writer's +#: escaping measured (`repo_shape.double_quoted`): the question spells a +#: character as `_spelled` does, `\t` as a backslash and a `t`, and the +#: file doubles that backslash, so the line reads back as the question. +#: `{byte}` is the byte 0xE9 as `_spelled_character` spells it, doubled as +#: the file doubles it: `\xe9` at 71b58a1, and #329 respells it. +REFUSED_IN_THE_SOURCE = [ + pytest.param(b"-notes.md", '"-notes.md"', + "because it begins with `-`, which `execute` refuses as " + "policy", id="leading-dash"), + pytest.param(b"notes\t.md", '"notes\\t.md"', + "because it holds the control character `\\\\t`, which", + id="tab"), + pytest.param(b"a\rb.md", '"a\\rb.md"', + "because it holds the control character `\\\\r`, which", + id="carriage-return-inside"), + pytest.param(b"Icon\r", '"Icon\\r"', + "because it ends in a carriage return (`\\\\r`), which `git " + "filter-repo` strips from the end of each line of the path " + "list it reads, so its line would name `Icon`, another " + "path. So", id="carriage-return-at-the-end"), + pytest.param(b"a\nb.md", '"a\\nb.md"', + "because it holds a line feed (`\\\\n`), which ends its line", + id="line-feed"), + pytest.param(b"caf\xe9.md", '"caf\\udce9.md"', + "because it holds `{byte}`, which is not UTF-8", + id="not-utf8"), + pytest.param(b"a==>b.md", '"a==>b.md"', + "because it holds `==>`, which `git filter-repo " + "--paths-from-file` reads as a rename", id="rename-arrow"), + pytest.param(b" ", '" "', "because it is nothing but whitespace", + id="whitespace-only"), +] + +#: How the question `plan` asks about such a path ends, whatever it is. +ONLY_ROOT_OR_DROP = ( + "So `root` or `drop` is the only answer `execute` takes for it. To put " + "it in a leg, rename it in the source, commit, and re-run `plan`.") + +#: The files beside it, which `plan` places with no question. +BESIDE = {"README.md": "# Thing\n", "docs/guide.md": "# guide\n", + "src/main.py": "print(1)\n"} + + +def indexed_source(base: Path, name: bytes) -> Path: + """A source holding `BESIDE` and `name` at the top level, the name put + straight into git's index and committed as `git add` records it, so no + file system has to hold it (NTFS holds no tab or carriage return, APFS + no byte that is not UTF-8). `core.protectNTFS` is off for the two calls + that write it, as in `test_adopt_plan_escapes.py`: nothing here checks + it out. Skipped where git does not keep the name in the tree, which + only git can say: `update-index` ignores a path it will not record.""" + source = make_source_repo(base / "Thing", BESIDE, edits=()) + unguarded = ("-c", "core.protectNTFS=false") + blob = subprocess.run(["git", "hash-object", "-w", "--stdin"], + cwd=str(source), input=b"notes\n", + capture_output=True, check=True).stdout.strip() + added = subprocess.run( + ["git", *unguarded, "update-index", "--add", "-z", "--index-info"], + cwd=str(source), input=b"100644 " + blob + b"\t" + name + b"\0", + capture_output=True, check=False) + if added.returncode: + pytest.skip(f"git update-index refuses {name!r} here: " + f"{added.stderr!r}") + git(*unguarded, "-c", "user.name=Source Human", + "-c", "user.email=human@source.invalid", + "commit", "-q", "-m", "Add a name execute refuses", cwd=source) + held = subprocess.run(["git", "ls-tree", "-r", "-z", "--name-only", + "HEAD"], cwd=str(source), capture_output=True, + check=True).stdout.split(b"\0") + if name not in held: + pytest.skip(f"git does not keep {name!r} in a tree here") + return source + + +def plan_in_bytes(source: Path, plan: Path) -> subprocess.CompletedProcess: + """`plan`, as `conftest.write_plan` runs it, answered in BYTES and in + UTF-8 on every platform: a name printed raw, `Icon\\r`, would read as + `Icon\\n` through text mode, and the policy's question holds an em + dash.""" + env = blank_unnamed_pin_sources( + {**os.environ, "LANES_LANE": "", "PYTHONIOENCODING": UTF8}, {}) + return subprocess.run( + [sys.executable, str(ADOPT), "plan", "--source", str(source), + "--project", PROJECT, "--org", "testorg", "--elected-by", + "Test Human", "--elected-on", "2026-09-02", "--out", str(plan)], + capture_output=True, env=env, stdin=subprocess.DEVNULL, check=False) + + +def expected_question(adopter, name: str) -> str: + """The question `plan` asks about `name`: after the policy's own, when it + asks one, or after where the rules put it; then why `execute` will not + extract it, in `entry_path_problem`'s words, and the two answers.""" + verdict = adopter.PathPolicy.load(adopter.PATH_POLICY).classify_file(name) + tail = (f" into a leg as it is named, because " + f"{adopter.entry_path_problem(name)}. {ONLY_ROOT_OR_DROP}") + if verdict.leg: + return (f"The rules put this path in the {verdict.leg} leg (rule " + f"`{verdict.rule}`), but `execute` will not extract it" + + tail) + return f"{verdict.question} But `execute` will not extract this path" \ + + tail + + +@pytest.mark.parametrize("name, spelled, in_the_file", REFUSED_IN_THE_SOURCE) +def test_plan_asks_about_a_path_execute_refuses(adopter, tmp_path, name, + spelled, in_the_file): + """THE ISSUE. `plan` writes `leg: null`, `review_required: true` and a + question saying `execute` will not extract the path as it is named, why, + and that `root` or `drop` is the only answer; it prints that question + with the paths still to answer. `check` and `execute` say of the plan as + written what they say of any entry still asked: `plan-unsafe-path` + (`leg: null`, judged so the person knows what `spec` would meet) and + `plan-unresolved`, and `execute` refuses before any leg exists.""" + text_name = name.decode(UTF8, "surrogateescape") + in_the_file = in_the_file.replace( + "{byte}", adopter._spelled_character("\udce9").replace("\\", "\\\\")) + source = indexed_source(tmp_path, name) + plan = tmp_path / "adoption-plan.yaml" + written = plan_in_bytes(source, plan) + assert written.returncode == 0, written.stderr + written.stdout + question = expected_question(adopter, text_name) + assert in_the_file.replace("\\\\", "\\") in question + assert ONLY_ROOT_OR_DROP in question + + row = {str(row["path"]): row + for row in adopter.load_yaml(plan)["paths"]}[text_name] + assert (row["leg"], row["review_required"], row["confidence"]) == ( + None, True, "review") + assert row["question"] == question + raw = plan.read_bytes().decode(UTF8) + question_line = f" question: {adopter.y(question)}" + assert raw.count(f"\n - path: {spelled}\n") == 1 + assert raw.count(f"\n{question_line}\n") == 1 + assert question_line.isprintable() and in_the_file in question_line + + report = written.stdout.replace(os.linesep.encode(), b"\n").decode(UTF8) + if not text_name.isprintable(): + assert name not in written.stdout, written.stdout + assert f"\n {adopter._shown(text_name)}\n {question}\n" in report + + checked = run_check(plan) + assert checked.returncode == 1, checked.stderr + checked.stdout + found = [line for line in checked.stderr.splitlines() + if line.startswith("FINDING ")] + named = adopter._spelled(text_name) + assert found[0] == ( + f"FINDING {UNSAFE}: {named} has leg: null, and `execute` will not " + f"extract it into a leg: {adopter.entry_path_problem(text_name)}") + assert found[1].startswith(f"FINDING plan-unresolved: {named} still has " + "`leg: null`. The question was: ") + assert len(found) == 2, found + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(execute_args(tmp_path, plan)) + assert caught.value.code == "plan-unresolved" + no_leg_was_created(tmp_path) + + +@pytest.mark.parametrize("answered", ["root", "drop"]) +def test_the_answer_the_question_offers_checks_ok(adopter, tmp_path, + answered): + """The question's answers are answers: `-notes.md` answered `root` or + `drop` checks ok, and `execute` raises nothing over the plan before it + would make a leg. (`test_the_answers_it_offers_are_adopted_and_verified` + runs it to the end, where `git filter-repo` is installed.)""" + source = make_source_repo(tmp_path / "Thing", + {**BESIDE, "-notes.md": "notes\n"}, edits=()) + plan = tmp_path / "adoption-plan.yaml" + assert write_plan(source, plan, project=PROJECT).returncode == 0 + answer(plan, '"-notes.md"', "null", answered) + assert "leg: null" not in plan.read_text(encoding=UTF8) + checked = run_check(plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert "plan ok" in checked.stdout + loaded = adopter.Plan.load(plan) + assert adopter._refuse_what_check_finds( + loaded, loaded.open_source(str(source), tmp_path / "read")) is None + + +def test_a_leg_only_a_refused_path_fed_is_recorded_as_seeded(adopter, + tmp_path): + """What the rules imply is read with such a path ASKED: it is in no leg + whatever the answer, so a leg it alone fed is seeded, and the plan says + so -- the `seeding:` record and the warning naming `--allow-empty-leg` -- + as `check` says of the plan as written and once it is answered, with no + note that the record is stale. On main at 71b58a1 `plan` wrote + `-spec.md: spec`, recorded the spec leg as extracted, and warned of + nothing.""" + source = make_source_repo(tmp_path / "Thing", + {"-spec.md": "# spec\n", "README.md": "# T\n", + "src/main.py": "print(1)\n"}, edits=()) + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + assert adopter.load_yaml(plan)["seeding"]["spec"] == { + "seeded_from_template": True, "template": "templates/spec-root", + "reason": "no entry assigns a path to the spec leg"} + assert "WARNING the spec leg will be SEEDED" in written.stdout + assert "--allow-empty-leg spec" in written.stdout + for answered in (None, "drop"): + if answered: + answer(plan, '"-spec.md"', "null", answered) + checked = run_check(plan) + assert "WARNING the spec leg will be SEEDED" in checked.stdout + assert "the `seeding:` record says" not in checked.stdout + assert checked.returncode == 0, checked.stderr + checked.stdout + + # --- a byte that is not UTF-8 --------------------------------------------- #: A name holding the byte 0xE9 -- Latin-1's e-acute, and not UTF-8 -- below @@ -551,7 +798,7 @@ def test_a_source_with_such_names_is_adopted_and_verified(tmp_path): plan = tmp_path / "adoption-plan.yaml" written = write_plan(source, plan, project=PROJECT) assert written.returncode == 0, written.stderr + written.stdout - answer(plan, '"-notes.md"', "spec", "drop") + answer(plan, '"-notes.md"', "null", "drop") answer(plan, "D.tsx", "code", "root") if os.name != "nt": for spelled, old, new in POSIX_ANSWERS: @@ -589,3 +836,44 @@ def files(repo: Path, rev: str) -> set[str]: capture_output=True, check=True ).stdout.decode(UTF8).splitlines() assert log == [edited, "Initial import"], f"{path}: {log}" + + +@needs_filter_repo +@pytest.mark.parametrize("answered", ["root", "drop"]) +@pytest.mark.parametrize("name", [ + "-notes.md", pytest.param("notes\t.md", marks=POSIX_ONLY)]) +def test_the_answers_it_offers_are_adopted_and_verified(adopter, tmp_path, + name, answered): + """#319's ACCEPTANCE: `plan` asks about the path, the person gives one of + the two answers its question offers, `check` says `plan ok`, and + `execute` runs to `adoption verified`, the path where that answer put + it: in the root tree of the split, or nowhere. A tab is a name only a + POSIX disk holds, and `execute` checks the source out.""" + source = make_source_repo(tmp_path / "Thing", {**BESIDE, name: "n\n"}, + edits=()) + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + row = {str(row["path"]): row + for row in adopter.load_yaml(plan)["paths"]}[name] + assert row["leg"] is None and row["question"].endswith(ONLY_ROOT_OR_DROP) + answer(plan, adopter.y(name), "null", answered) + checked = run_check(plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert "plan ok" in checked.stdout + + remotes = tmp_path / REMOTES + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / WORK)) + assert result.returncode == 0, result.stderr + result.stdout + assert "adoption verified" in result.stdout + root = subprocess.run(["git", "ls-tree", "-r", "-z", "--name-only", + "adopt/three-repo-shape"], cwd=str(source), + capture_output=True, check=True).stdout + held = {path.decode(UTF8) for path in root.split(b"\0") if path} + assert (name in held) == (answered == "root") + spec = subprocess.run(["git", "ls-tree", "-r", "--name-only", "main"], + cwd=str(remotes / f"{PROJECT}-spec.git"), + capture_output=True, text=True, check=True).stdout + assert spec.split() == ["docs/guide.md"] diff --git a/tests/test_adopt_literal_names.py b/tests/test_adopt_literal_names.py index 763bd0b4..282a6566 100644 --- a/tests/test_adopt_literal_names.py +++ b/tests/test_adopt_literal_names.py @@ -15,6 +15,10 @@ itself except the failing grep, which `plan` cannot provoke, and the line feed, whose plan is #199's to write; each measured failing on main at a5b09c3, and `notes/` and `s?c/` are the controls that already passed there. +The two spec directories that begin with `-` are asked of `plan` AND of +`cross_leg_references`: since #319 `plan` asks about a path `execute` will +not extract as it is named, so it answers neither `spec` and greps for +neither, and the search #228 made literal is asked of the rules' own answer. The split's forced `git add -f` asks the same question of a path the shape wrote THROUGH a symlink the plan kept in the root: where it landed is the @@ -83,6 +87,40 @@ def legs(plan: dict) -> dict: return {str(entry["path"]): entry["leg"] for entry in plan["paths"]} +#: The spec directories `plan` ASKS about instead of answering `spec` (#319): +#: `execute` will not extract a path that begins with `-` into a leg as it is +#: named, so `root` or `drop` is the only answer it takes, no answer moves +#: the directory to the spec leg, and the plan names no file for it. The +#: rules still answer `spec`, and that is the answer #228's literal `git +#: grep` is asked of, through `cross_leg_references`. +ASKED = ("-notes/", "-Otouch pager-ran;/") + + +def assert_asked(plan: dict, path: str) -> None: + """`path` is written as #319 writes a path `execute` refuses: `leg: + null`, `review_required: true`, and a question that gives the reason + and the two answers.""" + [entry] = [entry for entry in plan["paths"] if str(entry["path"]) == path] + assert entry["leg"] is None, entry + assert entry["review_required"] is True, entry + question = str(entry["question"]) + assert ("The rules put this path in the spec leg (rule " + in question), question + assert ("but `execute` will not extract it into a leg as it is named, " + "because it begins with `-`" in question), question + assert "So `root` or `drop` is the only answer `execute` takes" \ + in question, question + + +def ruled(adopter, source: Path, base: Path): + """The source as `plan` opens it, and the rules' answer for each of its + paths: `walk`'s entries, before `plan` asks about any of them.""" + opened = adopter.Source.open(str(source), base / "work") + entries = adopter.walk(adopter.PathPolicy.load(adopter.PATH_POLICY), + [(name, size) for name, _, _, size in opened.tree()]) + return opened, entries + + @pytest.fixture(scope="module") def adopter(): spec = importlib.util.spec_from_file_location("adopt_literal_names", ADOPT) @@ -138,28 +176,47 @@ def adopter(): @pytest.mark.parametrize(("tree", "spec_dir", "code", "hits"), NAME_CLASSES) def test_each_file_that_names_a_spec_directory_is_a_follow_up( - tmp_path, tree, spec_dir, code, hits): + adopter, tmp_path, tree, spec_dir, code, hits): """Exactly the code files that name the spec directory, each spelled as the tree spells it. On main: `-notes/` was git's usage error (exit 129) and no follow-up; `:cache` was searched as the pathspec `cache` and found nothing; `*` also searched the spec leg and named `code/notes/two.md`; and `Übersicht.py` was written - `code/"src/\\303\\234bersicht.py"`. `?` never widened, and does not.""" + `code/"src/\\303\\234bersicht.py"`. `?` never widened, and does not. + + The search is asked of the rules' answer, through + `cross_leg_references`, and the plan writes what it found -- except for + `-notes/`, which `plan` asks about (#319, ASKED) and so writes with no + follow-up: no answer `execute` takes moves it to the spec leg.""" source = make_source_repo(tmp_path / "Thing", tree=tree, edits=()) plan = planned(source, tmp_path) rows = legs(plan) - assert rows[spec_dir] == "spec", rows + opened, entries = ruled(adopter, source, tmp_path) + assert {e.path: e.leg for e in entries}[spec_dir] == "spec" assert all(rows[entry] == "code" for entry in code), rows - assert named(plan) == [follow_up(hit, spec_dir) for hit in hits] - - -def test_a_spec_directory_named_like_the_pager_option_runs_nothing(tmp_path): + found = [follow_up(hit, spec_dir) for hit in hits] + assert adopter.cross_leg_references(opened, entries, SPEC_PATH, + CODE_PATH) == found + if spec_dir in ASKED: + assert_asked(plan, spec_dir) + assert named(plan) == [] + else: + assert rows[spec_dir] == "spec", rows + assert named(plan) == found + + +def test_a_spec_directory_named_like_the_pager_option_runs_nothing(adopter, + tmp_path): """`-O` is `git grep --open-files-in-pager`, which git runs through a shell. Read as that option, the directory's name made the planned commit's hash the PATTERN, the WORKTREE was searched for it, and a tracked file holding it was handed to ``: on main this created `pager-ran` in the source and wrote no follow-up. Now the name is the - pattern, the commit is the commit, and nothing runs.""" + pattern, the commit is the commit, and nothing runs. + + The name begins with `-`, so `plan` asks about it (#319, ASKED) and + searches for nothing; the search is asked of the rules' answer, `spec`, + through `cross_leg_references`, which is where the option was read.""" spec_dir = "-Otouch pager-ran;/" source = make_source_repo( tmp_path / "Thing", edits=(), @@ -178,8 +235,15 @@ def test_a_spec_directory_named_like_the_pager_option_runs_nothing(tmp_path): plan = planned(source, tmp_path) assert sorted(os.listdir(source)) == before, "git ran the pager" - assert legs(plan)[spec_dir] == "spec" - assert named(plan) == [follow_up("src/reader.py", spec_dir)] + assert_asked(plan, spec_dir) + assert named(plan) == [] + + opened, entries = ruled(adopter, source, tmp_path) + assert {e.path: e.leg for e in entries}[spec_dir] == "spec" + said = adopter.cross_leg_references(opened, entries, SPEC_PATH, CODE_PATH) + + assert sorted(os.listdir(source)) == before, "git ran the pager" + assert said == [follow_up("src/reader.py", spec_dir)] def test_a_colour_setting_in_the_source_is_not_in_a_follow_up(tmp_path): diff --git a/tests/test_adopt_plan_escapes.py b/tests/test_adopt_plan_escapes.py index e5a6d97f..0ae2a965 100644 --- a/tests/test_adopt_plan_escapes.py +++ b/tests/test_adopt_plan_escapes.py @@ -228,10 +228,12 @@ def test_a_value_spelled_as_an_escape_is_not_read_as_one(adopter): #: about it)`. The SAME name is put at the top level, where it is an entry of #: its own, and under `docs/`, which the rules send to the spec leg as one #: entry. A name with no extension no rule places, so `plan` asks; `.txt` is -#: code by the extension table. +#: code by the extension table, and `plan` asks about `caf\\xe9.txt` all the +#: same, because `execute` will not extract a name that is not UTF-8 into a +#: leg (#319; `test_adopt_entry_paths.py`). ODD_NAMES = [ pytest.param(b"Icon\r", '"Icon\\r"', True, id="carriage-return"), - pytest.param(b"caf\xe9.txt", '"caf\\udce9.txt"', False, id="not-utf8"), + pytest.param(b"caf\xe9.txt", '"caf\\udce9.txt"', True, id="not-utf8"), pytest.param("n\u0085el".encode(), '"n\\x85el"', True, id="next-line"), pytest.param("l\u2028s".encode(), '"l\\u2028s"', True, id="line-separator"), diff --git a/tests/test_adopt_submodules.py b/tests/test_adopt_submodules.py index d97d01cc..66331377 100644 --- a/tests/test_adopt_submodules.py +++ b/tests/test_adopt_submodules.py @@ -905,12 +905,20 @@ def test_a_newline_spelled_in_the_question_is_the_entry_to_write(tmp_path): entry is `plan-unsafe-path` and nothing else -- no file uncovered, no entry covering nothing -- where on main at e4decfb `check` said `plan ok` and `execute` refused the same plan as `unsafe-value`. Answered `drop`, - as the remediation offers, it checks ok.""" + as the remediation offers, it checks ok. Since #319 the question says + so itself: it names that entry, why, and its two answers.""" source = raw_source(tmp_path, b"src/vendored", b"src/a\nb.py") plan = planned(source, tmp_path)[0] - named = REPLACE_WITH_RE.search(plan_rows(plan)["src/"]["question"]) + question = plan_rows(plan)["src/"]["question"] + named = REPLACE_WITH_RE.search(question) own = re.findall(r"`([^`]+)`", named[1]) assert own == ["src/a\\nb.py", APP, VENDORED] + assert question.endswith( + " Of those entries, `execute` will not extract `src/a\\nb.py` into a " + "leg as it is named, because it holds a line feed (`\\n`), which " + "ends its line of the path list `git filter-repo` reads, so the rest " + "of the name would be read as a line of its own; so `root` or `drop` " + "is the only answer for it."), question split = answered(plan, tmp_path, {GITMODULES: "code"}) edited(split, "src/", {f'"{path}"': "code" for path in own}) result = check_plan(split) @@ -926,6 +934,129 @@ def test_a_newline_spelled_in_the_question_is_the_entry_to_write(tmp_path): assert result.returncode == 0, result.stderr +def test_the_split_names_each_entry_execute_refuses_and_its_answers( + adopter, tmp_path): + """#319, in #166's split. Of the entries that replace `src/`, a file + whose name holds a line feed and a SUBMODULE whose name holds a tab are + each named, with `entry_path_problem`'s reason, and told `root` or + `drop` -- `drop` for the submodule. On main at 71b58a1 the question + named both as entries to write and offered each any `leg:`, and `check` + refused `spec` or `code` for either as `plan-unsafe-path`. The edit the + question names, given those answers, checks ok.""" + source = raw_source(tmp_path, b"src/ven\tdored", b"src/a\nb.py") + plan = planned(source, tmp_path)[0] + question = plan_rows(plan)["src/"]["question"] + own = re.findall(r"`([^`]+)`", REPLACE_WITH_RE.search(question)[1]) + assert own == ["src/a\\nb.py", APP, "src/ven\\tdored"] + reasons = [adopter.entry_path_problem(path) + for path in ("src/a\nb.py", "src/ven\tdored")] + assert question.endswith( + " Of those entries, `execute` will not extract these into a leg as " + "they are named, so `root` or `drop` is the only answer for each, " + "and `drop` for a submodule, which the assembly root never keeps: " + f"`src/a\\nb.py`, because {reasons[0]}; `src/ven\\tdored`, because " + f"{reasons[1]}."), question + split = answered(plan, tmp_path, {GITMODULES: "code"}) + edited(split, "src/", {'"src/a\\nb.py"': "root", f'"{APP}"': "code", + '"src/ven\\tdored"': "drop"}) + result = check_plan(split) + assert result.returncode == 0, result.stderr + assert "plan ok" in result.stdout + + +#: A name `execute` will not extract, and why, as `entry_path_problem` says. +TAB_NAME = "d/a\tb.md" + + +@pytest.mark.parametrize("own, held, said", [ + # None refused: the question is what it was. + (["d/ok.md", "d/sub"], ["d/sub"], ""), + # One file: `root` or `drop`. + ([TAB_NAME, "d/sub"], ["d/sub"], + " Of those entries, `execute` will not extract `d/a\\tb.md` into a leg " + "as it is named, because {0}; so `root` or `drop` is the only answer " + "for it."), + # One submodule: `drop` alone. + (["d/ok.md", TAB_NAME], [TAB_NAME], + " Of those entries, `execute` will not extract `d/a\\tb.md` into a leg " + "as it is named, because {0}; so `drop` is the only answer for it: the " + "assembly root keeps no submodule of the source's."), +], ids=["none", "one-file", "one-submodule"]) +def test_the_split_sentence_says_each_refused_entry_s_answers(adopter, own, + held, said): + assert adopter._refused_replacements(own, held) == said.format( + adopter.entry_path_problem(TAB_NAME)) + + +def test_past_split_entries_named_the_rest_are_counted(adopter): + """Ten refused entries: the first SPLIT_ENTRIES_NAMED are named, each + with its reason, and the rest counted, as `_replacements` counts the + entries it does not name.""" + own = [f"d/f{i}\t.md" for i in range(10)] + said = adopter._refused_replacements(own, []) + cap = adopter.SPLIT_ENTRIES_NAMED + assert said.startswith( + " Of those entries, `execute` will not extract these into a leg as " + "they are named, so `root` or `drop` is the only answer for each: " + "`d/f0\\t.md`, because "), said + assert said.count(", because ") == cap + assert f"`d/f{cap - 1}\\t.md`" in said and f"`d/f{cap}\\t.md`" not in said + assert said.endswith( + f"; and {10 - cap} more, each `plan-unsafe-path` in `check` if it is " + "answered `spec` or `code`.") + + +@pytest.mark.parametrize("leg, expected", [ + ("drop", []), + # What `check` says of the answers the question rules out, unchanged. + ("root", [ROOT]), + ("code", ["plan-unsafe-path", UNREGISTERED]), +]) +def test_a_submodule_execute_refuses_as_named_is_told_drop(plan_of, tmp_path, + leg, expected): + """#319. An entry that holds a submodule AND that `execute` will not + extract as it is named -- the orphan `-dep`, at the top level -- is + asked `.gitmodules`' question and then told why, and that `drop` is its + only answer: `root` keeps a submodule in the assembly root, which + `check` refuses as `plan-submodule-root`.""" + plan = plan_of({"registered": (), "orphan": "-dep"})[0] + row = plan_rows(plan)["-dep"] + question = row["question"] + assert (row["leg"], row["review_required"]) == (None, True) + assert question.startswith( + "When a repository is adopted, a source's own `.gitmodules` goes to " + "the SAME leg as every submodule it registers;"), question + assert (" This entry holds the submodule(s) `-dep`: it is asked this " + "question, not classified by its rule `default`. But `execute` " + "will not extract this path into a leg as it is named, because " + "it begins with `-`, ") in question + assert question.endswith( + " So `drop` is the only answer `execute` takes for this entry as it " + "stands: `root` would keep a submodule in the assembly root, which " + "keeps none of the source's. To put it in a leg, rename it in the " + "source, commit, and re-run `plan`."), question + result = check_plan(answered(plan, tmp_path, {'"-dep"': leg})) + assert finding_codes(result.stderr) == expected, result.stderr + assert result.returncode == (1 if expected else 0) + + +def test_a_run_of_spaces_in_a_name_the_question_names_is_kept(tmp_path): + """`src/a b.md`, two spaces, beside a submodule. `Verdict` folds each + run of spaces in a question to one, so on main at 71b58a1 the question + named `src/a b.md`, and the split it named covered nothing and left the + file uncovered. The question is set as it is written now (#319, whose + sentences spell names too), and the split it names checks ok.""" + source = raw_source(tmp_path, b"src/vendored", b"src/a b.md") + plan = planned(source, tmp_path)[0] + named = REPLACE_WITH_RE.search(plan_rows(plan)["src/"]["question"]) + own = re.findall(r"`([^`]+)`", named[1]) + assert own == ["src/a b.md", APP, VENDORED] + split = answered(plan, tmp_path, {GITMODULES: "code"}) + edited(split, "src/", {f'"{path}"': "code" for path in own}) + result = check_plan(split) + assert result.returncode == 0, result.stderr + + def test_a_space_in_a_name_the_question_names_is_extracted(adopter, tmp_path): """#196's own case of #166's split: `docs/User Guide.md` beside a submodule at `docs/theme`. The question names it as an entry to write;