From 24273dd47e428af3e31f7404acae7f2090daa332 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 20 Oct 2026 17:58:16 +0000 Subject: [PATCH] tests/test_adopt_ignored_shape_files.py, tests/test_adopt_pins.py, tests/test_adopt_submodule_url_answers.py, tests/test_adopt_plan_yaml_quoting.py and tests/test_naming_policy.py: each of five adopt tests now fails when the thing it guards is removed: a link's target reaches git as bytes, the stub gh keeps its #! line where text mode writes CRLF, a caller's GIT_ALLOW_PROTOCOL cannot hide a lost guard, each guard of two pairs is reached alone, and the naming test asks three name shapes (#916) L4-54: split_commit_of feeds `git hash-object --stdin` bytes, and a test watches what git is given and what the tree holds, for a target with a line break, a carriage return and a non-ASCII character. L1-60: stub_gh writes its script with write_bytes, and a test gives Path.write_text Windows' CRLF and reads the script back. L1-62: test_git_is_held_to_the_disk_when_it_asks clears GIT_ALLOW_PROTOCOL. L1-K6: a test for each of _execute_gets_to_write's visibility clause, `left = judged | ...` and the two questions of _pushed_in_every_mode, each reached with the other out of the way; no code changed. L2-91: the naming test is asked over North{c}Wind, openDo{c}x and Med{c}xScribe, and a second test says each shape is one a form takes. Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_ignored_shape_files.py | 41 +++++++- tests/test_adopt_pins.py | 32 +++++- tests/test_adopt_plan_yaml_quoting.py | 120 ++++++++++++++++++++++ tests/test_adopt_submodule_url_answers.py | 6 +- tests/test_naming_policy.py | 37 ++++++- 5 files changed, 228 insertions(+), 8 deletions(-) diff --git a/tests/test_adopt_ignored_shape_files.py b/tests/test_adopt_ignored_shape_files.py index 29992959..a51acb3d 100644 --- a/tests/test_adopt_ignored_shape_files.py +++ b/tests/test_adopt_ignored_shape_files.py @@ -290,11 +290,15 @@ def split_commit_of(tmp_path: Path, files: dict, ignore: str = IGNORE_DOTFILES, (repo / IGNORE_FILE).write_text(ignore) git("add", "-A", "--", ".", cwd=repo) for link, target in (links or {}).items(): + # BYTES on stdin and out, never text (#916, L4-54): a text-mode pipe + # encodes with the locale and, on Windows, gives each line break a + # `\r`, and a link's target is the blob's bytes exactly. blob = subprocess.run( ["git", "hash-object", "-w", "--stdin"], cwd=str(repo), - input=target, capture_output=True, text=True, check=True) + input=target.encode("utf-8"), capture_output=True, check=True) git("update-index", "--add", "--cacheinfo", - f"{SYMLINK},{blob.stdout.strip()},{link}", cwd=repo) + f"{SYMLINK},{blob.stdout.decode('utf-8').strip()},{link}", + cwd=repo) commit_index(repo, "The split") write_files(repo, disk_only) return repo, git("rev-parse", "HEAD", cwd=repo).stdout.strip() @@ -422,6 +426,39 @@ def test_a_pinned_path_is_found_through_the_split_trees_symlinks( assert adopter._shape_file_findings(repo, commit) == [] +#: A link target no text-mode pipe passes on as it is: a line break (a +#: Windows pipe writes `\r\n` for it), a bare carriage return, and a character +#: the locale's encoding may not have. `chr` keeps this source ASCII. +AWKWARD_TARGET = "up\r\nand\rover/caf" + chr(0xE9) + "/" + + +def test_split_commit_of_gives_git_a_links_target_as_bytes(tmp_path, + monkeypatch): + """`split_commit_of` feeds `git hash-object --stdin` the link's target as + BYTES, the rule this suite keeps for what goes to git's stdin (#511's + NIT-6; #916's L4-54). Fed as text, a Windows pipe gave the target a + `\\r` for each line break and a non-UTF-8 locale a different encoding of + the last character, and the tree held another link than the one asked + for. On a POSIX UTF-8 run both reach git alike, so the call is watched: + what git is given, and then what the tree holds.""" + fed = [] + real_run = subprocess.run + + def watching(argv, *args, **named): + if "hash-object" in argv: + fed.append(named.get("input")) + return real_run(argv, *args, **named) + + monkeypatch.setattr(subprocess, "run", watching) + repo, commit = split_commit_of(tmp_path, {MAKEFILE: MAKEFILE_BODY}, + links={"link": AWKWARD_TARGET}) + target = AWKWARD_TARGET.encode("utf-8") + assert fed == [target], fed + held = real_run(["git", "cat-file", "blob", f"{commit}:link"], + cwd=str(repo), capture_output=True, check=True) + assert held.stdout == target + + def findings_within(adopter, repo: Path, commit: str, seconds: float = 60) -> list: """`_shape_file_findings`, or a FAILURE if it has not answered within diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 33d7094e..c68675c8 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -119,7 +119,9 @@ def stub_gh(directory: Path) -> dict: bin_dir = directory / "fakebin" bin_dir.mkdir() gh = bin_dir / "gh" - gh.write_text(textwrap.dedent("""\ + # BYTES, never `write_text` (#916, L1-60): text mode gives each `\n` a + # `\r` on Windows, which would end the `#!` line in `python3\r`. + gh.write_bytes(textwrap.dedent("""\ #!/usr/bin/env python3 import json, os, sys with open(os.environ["GH_STUB_LOG"], "a", encoding="utf-8") as log: @@ -137,7 +139,7 @@ def stub_gh(directory: Path) -> dict: sys.exit(0) print("HTTP 404: Not Found", file=sys.stderr) sys.exit(97) - """), encoding=UTF8) + """).encode(UTF8)) gh.chmod(gh.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) return {"PATH": f"{bin_dir}{os.pathsep}{os.environ.get('PATH', '')}", "GH_STUB_LOG": str(directory / "gh.log")} @@ -740,6 +742,32 @@ def test_with_no_pin_source_plan_reads_the_digest_from_gh(source_repo, assert not (tmp_path / "unanswered.yaml").exists() +def windows_write_text(path: Path, data: str, encoding: str | None = None, + errors: str | None = None, + newline: str | None = None) -> int: + """`Path.write_text` as Windows runs it: text mode writes `\\r\\n` for + each `\\n`, unless `newline` says otherwise.""" + if newline is None: + data = data.replace("\n", "\r\n") + with path.open("wb") as handle: + return handle.write(data.encode(encoding or UTF8, errors or "strict")) + + +def test_the_stub_gh_is_written_as_bytes_so_a_crlf_cannot_break_its_shebang( + tmp_path, monkeypatch): + """#916's L1-60: `stub_gh` wrote its `#!` script with `Path.write_text`, + and where text mode writes CRLF (Windows, or an emulation of it) the + first line became `#!/usr/bin/env python3\\r`, which `env` cannot find, + so `test_with_no_pin_source_plan_reads_the_digest_from_gh` failed. The + script is the same bytes where text mode is CRLF as where it is not, + asked here on every OS by giving `Path.write_text` Windows' behaviour.""" + monkeypatch.setattr(Path, "write_text", windows_write_text) + stub_gh(tmp_path) + script = (tmp_path / "fakebin" / "gh").read_bytes() + assert script.startswith(b"#!/usr/bin/env python3\n"), script[:40] + assert b"\r" not in script, script + + def test_pin_owner_resolves_an_unqualified_name(source_repo, tmp_path, glass, gh_env): """`--pin-owner` means what it means for the scaffold: the owner an diff --git a/tests/test_adopt_plan_yaml_quoting.py b/tests/test_adopt_plan_yaml_quoting.py index e1f42c47..fc54b009 100644 --- a/tests/test_adopt_plan_yaml_quoting.py +++ b/tests/test_adopt_plan_yaml_quoting.py @@ -1253,6 +1253,100 @@ def test_the_misread_rule_asks_nothing_of_a_value_execute_never_writes( assert adopter._misread_findings(adopter.Plan.load(plan)) == [] +# --- #916's L1-K6: the two clauses that keep a value out of the misread rule, +# --- each reached with the other out of the way ------------------------------ +# +# `_execute_gets_to_write` (a visibility outside the three choices is not +# written in every mode) and `left = judged | ...` in `_misread_findings` (a +# value the project's own validator judges is its alone) keep the SAME +# visibilities out, so reverting either one alone failed no test: the first +# test above asks both at once. Each of these asks one. + + +@pytest.mark.parametrize(KEY_SPELLED, NEVER_WRITTEN) +def test_what_execute_writes_alone_keeps_a_value_it_never_writes_out( + adopter, answered, bare_template, tmp_path, monkeypatch, key, spelled): + """`_execute_gets_to_write` with no help from the validator's clause: the + validator is made to judge no value (`_validator_judged` answers an empty + set, not None), so a visibility outside the three choices that the loader + misreads on a bare line (`bare_template`) stays out of the rule only + because `_plan_fields` leaves it out. A rule that asked about it would + name it misread, with a remediation saying the run ends `adoption + verified`, where `execute` refuses it before any leg exists.""" + monkeypatch.setattr(adopter, "_validator_judged", + lambda _plan: frozenset()) + _, text = answered + plan = tmp_path / PLAN_FILE + carrying(plan, text, key, spelled) + assert adopter._misread_findings(adopter.Plan.load(plan)) == [] + + +@pytest.fixture +def bare_elector(adopter, bare_template) -> None: + """`bare_template` with the manifest's `elected_by:` and `reference:` + lines written bare too. Neither was bare before #259 (the elector was + quoted by #284 and `reference:` from the start), so `bare_template` + leaves both between quotes, where no value is misread. Bare, a blank one + reads as no value, and nothing but the validator's clause then keeps the + rule from naming what the validator names.""" + written = "" + for rel in adopter.TEMPLATED_YAML: + path = bare_template / rel + text = path.read_text(encoding=UTF8) + for placeholder in (ELECTED_BY_PLACEHOLDER, REFERENCE_PLACEHOLDER): + text = text.replace(f'"{{{{{placeholder}}}}}"', + f"{{{{{placeholder}}}}}") + path.write_text(text, encoding=UTF8) + written += text + for key, placeholder in ((ELECTED_BY_KEY, ELECTED_BY_PLACEHOLDER), + (REFERENCE_KEY, REFERENCE_PLACEHOLDER)): + assert f"{key}: {{{{{placeholder}}}}}\n" in written, key + + +#: `(plan key, as a plan edited by hand spells it, its placeholder)`: a blank +#: value on a line written bare reads as no value, which the validator refuses +#: (`manifest-elected-by`, `manifest-reference`). `_execute_gets_to_write` +#: leaves out only a visibility and a tracking branch, so these are asked. +JUDGED_AND_MISREAD = [ + pytest.param(ELECTED_BY_KEY, '" "', ELECTED_BY_PLACEHOLDER, + id="elector-blank"), + pytest.param(ELECTED_BY_KEY, '""', ELECTED_BY_PLACEHOLDER, + id="elector-empty"), + pytest.param(REFERENCE_KEY, '" "', REFERENCE_PLACEHOLDER, + id="reference-blank"), + pytest.param(REFERENCE_KEY, '""', REFERENCE_PLACEHOLDER, + id="reference-empty"), +] + + +@pytest.mark.parametrize("key, spelled, placeholder", JUDGED_AND_MISREAD) +def test_the_validators_judgement_alone_keeps_a_value_it_judges_out( + adopter, answered, bare_elector, tmp_path, capsys, key, spelled, + placeholder): + """`left = judged | ...` with no help from `_execute_gets_to_write`: these + values are asked about (it leaves out only a visibility outside the three + choices and a tracking branch `execute` never writes), the loader misreads + each on its bare line, and the validator judges it, so the validator's + finding is the only one, as `JUDGED_ONCE` has it on the shipped + templates. Both the rule and `check` in this process are asked: a rule + that did not leave the value to the validator would print it twice.""" + _, text = answered + plan = tmp_path / PLAN_FILE + carrying(plan, text, key, spelled) + loaded = adopter.Plan.load(plan) + asked = dict(adopter._plan_fields(loaded)) + assert placeholder in asked, "`_execute_gets_to_write` left it out" + assert adopter.manifest_misreads([asked[placeholder]]), ( + "the loader reads the bare line back, so it is nobody's to judge") + assert placeholder in adopter._validator_judged(loaded) + assert adopter._misread_findings(loaded) == [] + code, _, stderr = run_here(adopter, capsys, "check", PLAN_OPTION, + str(plan)) + assert code == 1, stderr + (line,) = judged_lines(stderr) + assert line.startswith(INVALID_FINDING), stderr + + #: A tracking branch the loader reads as no value, and git will not create. BRANCH_GIT_REFUSES = "~" @@ -1527,6 +1621,32 @@ def test_the_source_says_whether_execute_pushes_the_legs_to_a_branch( assert adopter._pushed_in_every_mode(opened, "true") is pushed +@pytest.mark.parametrize("own, origin, clone_says_no, source_says_no", [ + pytest.param(True, False, True, False, id="the-clones-memory-alone"), + pytest.param(False, True, False, True, id="the-sources-own-alone"), +]) +def test_each_question_pushed_in_every_mode_asks_is_asked_alone( + adopter, tmp_path, own, origin, clone_says_no, source_says_no): + """#916's L1-K6: `_pushed_in_every_mode` says no where EITHER of its two + questions does, so a branch both refuse (`the-source-lacks-it` in + `BRANCH_CASES`) is no test of one: take either question out and the + other still says no. These are the two branches ONE question refuses, + with the other's answer stated as a premise, so that taking out either + question alone fails here. The first is what the source's clone + remembers of `origin` (`_tracking_branch_problem`, gh mode): the source + has `true` and `origin` lacks it. The second is the source's own + branches as `git ls-remote` lists them (`--local-remote-dir`): `origin` + has `true` and the source lacks it.""" + opened = adopter.Source.open(str(branch_source(tmp_path, own, origin)), + tmp_path / "work") + problem = adopter._tracking_branch_problem( + opened, adopter.TRACKING_KEY, "true") + listed = adopter._branches_a_clone_gets(str(opened.path), opened.path) + assert (problem is not None) is clone_says_no, problem + assert ("true" not in listed) is source_says_no, listed + assert adopter._pushed_in_every_mode(opened, "true") is False + + def test_plan_asks_the_validator_before_the_misread_rule( adopter, answered, bare_template, tmp_path, monkeypatch, capsys): """rv181f's G1 at `plan`: the project's own validator is asked first, diff --git a/tests/test_adopt_submodule_url_answers.py b/tests/test_adopt_submodule_url_answers.py index 4440f75a..2f35260d 100644 --- a/tests/test_adopt_submodule_url_answers.py +++ b/tests/test_adopt_submodule_url_answers.py @@ -303,8 +303,12 @@ def test_git_is_held_to_the_disk_when_it_asks(adopter, tmp_path, monkeypatch): `_on_this_disk` were wrong about still reaches no network: with that judgement forced for an `ssh://` url, git itself refuses the transport, the answer is what git said, and the `ssh` git would start is never - started. No global config rewrites the url on the way.""" + started. No global config rewrites the url on the way, and the caller's + own `GIT_ALLOW_PROTOCOL` is cleared (#916's L1-62): were it `file`, git + would refuse the transport for the test and hide a guard `_answers` + had lost.""" started = a_recording_ssh(tmp_path, monkeypatch) + monkeypatch.delenv("GIT_ALLOW_PROTOCOL", raising=False) empty = tmp_path / "gitconfig" empty.write_text("", encoding="utf-8") monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(empty)) diff --git a/tests/test_naming_policy.py b/tests/test_naming_policy.py index eef53fa1..9fc1f8c7 100644 --- a/tests/test_naming_policy.py +++ b/tests/test_naming_policy.py @@ -241,19 +241,50 @@ def test_those_are_the_characters_the_two_patterns_disagree_on(): assert between == sorted(NO_ID_CHARACTERS) +#: The shapes of a root or family name, one for each form a name can take +#: that holds a run of letters and digits (#916's L2-91): `{c}` is where the +#: character goes. A plain CamelCase name (`project-leg`); `neutral-product`, +#: `open` and a capital, then a tail; and `domain-descendant`, a stem, then +#: `x` and a capital, with the character in the stem. A policy that let one of +#: those two patterns take `_` passed the first shape alone. +NAME_SHAPES = ("North{c}Wind", "openDo{c}x", "Med{c}xScribe") +#: The neutral products the descendant shape pins, so that a descendant's +#: referent is REACHED (`classify` calls a name a descendant only then). +REFERENTS = {"openScribe", "openxScribe"} + + +@pytest.mark.parametrize("shape", NAME_SHAPES) @pytest.mark.parametrize("character", NO_ID_CHARACTERS) def test_no_root_or_family_name_holds_a_character_no_id_takes(policy, - character): + character, + shape): """#658 and #659 tell a derived id refused for one of these characters to change `--project` or `--family` to one CamelCase token, never to pass an `--id`, and, in place, that the source cannot be adopted under that name. That holds while the policy classifies no such name as an - assembly root or a family holder.""" - name = f"North{character}Wind" + assembly root or a family holder, whichever of the forms it is spelled + like: no form takes the name at all, and none classifies it with its + referent reached or declared.""" + name = shape.format(c=character) + assert policy.matches(name) == [], name + assert policy.matches(name, "assembly") == [], name assert policy.classify(name, "assembly", set()) is None, name + assert policy.classify(name, "assembly", REFERENTS) is None, name assert policy.classify(name, "family") is None, name +@pytest.mark.parametrize("shape, form", [ + ("North{c}Wind", "project-leg"), ("openDo{c}x", "neutral-product"), + ("Med{c}xScribe", "domain-descendant")]) +def test_each_name_shape_is_one_a_form_takes_without_the_character( + policy, shape, form): + """The three shapes above are live: with no character in them each is + taken by the form it is there to ask about, so the test above cannot + pass for want of a name any form would take.""" + name = shape.format(c="") + assert form in {family for family, _ in policy.matches(name, "assembly")} + + def test_overlaps_are_reported_not_hidden(policy): """`openxFactory` satisfies three forms; precedence picks one, --explain shows all three."""