diff --git a/scripts/family.py b/scripts/family.py index edffe014..fc334c77 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -2282,15 +2282,95 @@ def _family_script() -> str: return str(here) +def _family_line(verb: str, holder: Path, *words: str) -> str: + """` --family-root `: ONE line + of this file's own, as `init`'s NEXT STEPS and every remedy that names a + verb print it, each value spelled by `shell_quoted` for the reader's + shell (#486, #1127). + + THE ONE SPELLING OF THESE LINES (#1127, RV1117's N2 and N4 on #1117). + `_add_command` printed the interpreter and this file's path from where + the person stands (`_family_script`); the remedies that named `bump`, + `remove` and `add` said a bare `family.py`, which runs only from + `scripts/` and only where the file is executable, and printed a row's + `project:` raw, two words where it held a space. A placeholder + (``, `/...`) is the caller's to add after the line, bare: + quoted, it would read as a value. + + THE VERB IS A STRING AT EVERY CALL, and + `tests/test_readme_windows_quoting_sentence.py` reads each one: README + lists the verbs whose lines are quoted for the shell they are printed + in, and a call naming a verb it does not list fails there.""" + return " ".join([PYTHON, shell_quoted(_family_script()), verb, + "--family-root", shell_quoted(holder), + *(shell_quoted(word) for word in words)]) + + +def _cited(command: str) -> str: + """`command` between backticks, as a remedy's prose names a command, or + bare where the command holds a backtick of its own (#1127, RV1117's + N6): a holder under a folder named with one closed the quotation inside + its path, so the text between the marks was half the command. Every + shell reads the line the same either way.""" + return command if "`" in command else f"`{command}`" + + +def _off_the_tracking_branch(family: "Family", refuses: str, + suggested: str) -> str: + """The clause a remedy puts before the line of this file it names, where + the refusal is said before `_checked_branch` asks the holder's branch + (#1127, RV1117's N1): `bump`'s `member-unknown`, and `add`'s + `member-already-present`. On the tracking branch, where a fresh clone + stands, the line that followed was refused `family-on-tracking-branch`. + So the clause says where the line runs and names the `switch -c` that + makes a branch, as the holder's `validate-family.py` does before the + `remove` its `member-duplicate-id` prints; the remedy ends with + `BRANCH_LEFT` (#607). In words true on any branch: the refusal is said + before the branch is read, and reading it here could refuse in its + place. `refuses` names the verb or verbs the branch is refused by and + says so: "`add` refuses", or "`bump` and `remove` refuse".""" + switch = _cited(_switch_command(family.root, suggested)) + return ("on a branch other than the holder's tracking branch, which " + f"{refuses} ({switch} makes one), ") + + def _add_command(holder: Path, org: str) -> str: """The `add` that grows the holder at `holder`, as `init` prints it: ONE line, so it pastes into PowerShell too, with this file's path, the holder's and the organisation each spelled by `shell_quoted` for the - reader's shell (#486). A holder under `my work/` used to print - `--family-root .../my work/Orbit`, two arguments to argparse. - `` is left for the person to fill in.""" - return (f"{PYTHON} {shell_quoted(_family_script())} add --family-root " - f"{shell_quoted(holder)} --member {shell_quoted(org)}/") + reader's shell (#486), by `_family_line`, as every remedy's line is + (#1127). A holder under `my work/` used to print `--family-root .../my + work/Orbit`, two arguments to argparse. `` is left for the + person to fill in.""" + return (f"{_family_line('add', holder)} --member " + f"{shell_quoted(org)}/") + + +def _unknown_members_repository(family: "Family", typed: str, + project: str) -> str: + """The `--member` value of the `add` that `bump`'s `member-unknown` + names: `/` with the owner filled in where it is known + (#1127, RV1117's N3), as ONE word for the reader's shell. + + THE OWNER, IN THIS ORDER: the one the person typed, where `bump + --member` was given as `/`; else `family.yaml`'s `org:`, + the organisation `init` made the holder in and its NEXT STEPS' `add` + names; else the blank ``, which is left only where neither is + there. The blank pasted unfilled is a redirection in every shell, not a + value: `family.py` never runs, and where a file named `org` is in the + working directory the shell tries to create the path after it. A member + may live in another organisation than the family's (`MEMBER_RE`), so + the family's is the likeliest owner, not a certain one; an `add` of a + repository that is not there refuses before it touches the holder. + Either owner is used only where `/` is a `--member` + `add` takes.""" + typed = typed.strip() + org = family.data.get("org") + for owner in (typed.rpartition("/")[0], org): + if isinstance(owner, str) and MEMBER_RE.fullmatch( + f"{owner}/{project}"): + return shell_quoted(f"{owner}/{project}") + return f"/{shell_quoted(project)}" def _standard_checkout(start: Path) -> Path | None: @@ -6460,17 +6540,25 @@ def _checked_add_target(family: Family, repository: str, project: str, """ held = family.holding(repository, project) if held is not None: - # THE `bump` IT NAMES RUNS AS PRINTED (#927): `bump` requires - # `--family-root`, so the line names this holder, spelled by - # `shell_quoted` as `_add_command`'s is. + # THE `bump` AND THE `remove` IT NAMES RUN AS PRINTED (#927, #1127), + # each a `_family_line` with this holder's `--family-root` and the + # row's own name, quoted. Said before `_checked_branch`, so a person + # on the tracking branch is told the branch both verbs need first. + name = str(held.get("project")) + suggested = _suggested_branch("bump", str(held.get("id") or name)) raise Refusal( "member-already-present", f"{project} is already a member of {family.data.get('name')}: " f"the row {held.get('project')} pins {held.get('repository')}", - "Remediation: `family.py bump --family-root " - f"{shell_quoted(family.root)} --member {held.get('project')} " - "--to ` moves an existing member; `remove` takes it " - "out. Adding it twice is not a way to move it.") + "Remediation: " + + _off_the_tracking_branch(family, "`bump` and `remove` refuse", + suggested) + + _cited(_family_line("bump", family.root, "--member", name) + + " --to ") + + " moves an existing member and " + + _cited(_family_line("remove", family.root, "--member", name)) + + " takes it out. Adding it twice is not a way to move it. " + + BRANCH_LEFT) # Everything the mount's `--force` would otherwise let past git, but the # cached store, which is asked below (#242). _checked_mount_path(family.root, project, path, @@ -6588,8 +6676,9 @@ def _checked_member_id(family: Family, repository: str, project: str, if row is not None: name = row["project"] # THE `remove` AND THE `bump` IT NAMES RUN AS PRINTED (#1005, #927), - # each with this holder's `--family-root`, which both verbs require, - # as `member-already-present`'s `bump` does. + # each a `_family_line` with this holder's `--family-root`, which + # both verbs require, and the row's own name, quoted (#1127). A real + # `add` asks this after `_checked_branch`, so no branch is named. raise Refusal( "member-duplicate-id", f"{project} is already a member of {family.data.get('name')} " @@ -6599,14 +6688,14 @@ def _checked_member_id(family: Family, repository: str, project: str, "project twice, which `validate-family.py` refuses", "Remediation: a family pins each project once. If " f"{repository} is {row.get('repository')} renamed at the forge, " - "`family.py remove --family-root " - f"{shell_quoted(family.root)} --member {name}` takes the old row " - "out and this `add` then pins it under the new name; `family.py " - f"bump --family-root {shell_quoted(family.root)} --member {name} " - "--to ` moves the row that is there instead. Two " - "projects that record one id are a fault in one of their " - "`project.yaml` files, mended in that project, not in the " - "family.") + + _cited(_family_line("remove", family.root, "--member", name)) + + " takes the old row out and this `add` then pins it under the " + "new name; " + + _cited(_family_line("bump", family.root, "--member", name) + + " --to ") + + " moves the row that is there instead. Two projects that " + "record one id are a fault in one of their `project.yaml` files, " + "mended in that project, not in the family.") def _probe_member(url: str, protocol: list[str], repository: str, @@ -8683,8 +8772,9 @@ def _checked_bumped_id(family: Family, row: dict, commit: str, return repository = row.get("repository") # THE `remove` IT NAMES RUNS AS PRINTED (#1005): `remove` requires - # `--family-root`, so the line names this holder, spelled by - # `shell_quoted` as `member-already-present`'s `bump` is (#927). + # `--family-root`, so the line is a `_family_line` naming this holder and + # the other row's own name, quoted, as every remedy's line is (#1127). A + # real `bump` asks this after `_checked_branch`, so no branch is named. raise Refusal( "member-duplicate-id", f"{repository} at {commit[:12]} is already a member of " @@ -8695,11 +8785,12 @@ def _checked_bumped_id(family: Family, row: dict, commit: str, "Remediation: a family pins each project once. Pass `--to` a commit " f"of {repository} whose project.yaml records an id no other row " f"records. If {repository} and {held.get('repository')} are one " - "project, `family.py remove --family-root " - f"{shell_quoted(family.root)} --member {held['project']}` takes the " - "other row out first. Two projects that record one id are a fault in " - "one of their `project.yaml` files, mended in that project, not in " - "the family.") + "project, " + + _cited(_family_line("remove", family.root, "--member", + held["project"])) + + " takes the other row out first. Two projects that record one id " + "are a fault in one of their `project.yaml` files, mended in that " + "project, not in the family.") def _head_oid(root: Path, path: str) -> str | None: @@ -9383,15 +9474,20 @@ def cmd_bump(args) -> int: row = family.member(project) if row is None: # THE `add` IT NAMES RUNS AS PRINTED (#1005): `add` requires - # `--family-root`, so the line names this holder, spelled by - # `shell_quoted` as `_add_command`'s is (#486). `` is left for - # the person to fill in: nothing here says whose repository it is. + # `--family-root`, so the line is a `_family_line` naming this + # holder (#1127), with the member's owner where it is known + # (`_unknown_members_repository`). Said before `_checked_branch`, so + # a person on the tracking branch is told the branch `add` needs. raise Refusal( "member-unknown", f"{project} is not a member of {family.data.get('name')}", - "Remediation: `family.py add --family-root " - f"{shell_quoted(family.root)} --member /{project}` puts it " - "in the family first.") + "Remediation: " + + _off_the_tracking_branch(family, "`add` refuses", + _suggested_branch("add", project)) + + _cited(_family_line("add", family.root) + " --member " + + _unknown_members_repository(family, args.member, + project)) + + " puts it in the family first. " + BRANCH_LEFT) project = str(row["project"]) commit = checked_value("--to", args.to).lower() if not COMMIT_RE.match(commit): diff --git a/tests/test_bump_remedies_family_root.py b/tests/test_bump_remedies_family_root.py index e7499657..7c61e760 100644 --- a/tests/test_bump_remedies_family_root.py +++ b/tests/test_bump_remedies_family_root.py @@ -48,7 +48,7 @@ from test_family import FAMILY, family, manifest # noqa: F401 from test_family_add_member_id import PINNED, RENAMED, renamed_copy from test_family_add_refusals import offline # noqa: F401 -from test_family_printed_quoting import copied_holder, host_words +from test_family_printed_quoting import copied_holder, family_lead, host_words #: A commit id to put where a line says `` or `` when nothing is #: run with it. It is NOT a fixture's `advanced`: that one pushes a branch to @@ -108,12 +108,14 @@ def refused_as(code, family, holder, offline, tmp_path): return result.stderr -def the_bump_line(stderr: str, lead: str = "") -> str: - """The one `family.py bump ...` a remedy prints, from `bump` to the - closing backtick, with `lead` before `family.py` (a path, for - `bump-leg.py`'s).""" - found = re.findall(re.escape(f"`{lead}family.py ") + r"(bump [^`]*)`", - stderr) +def the_bump_line(stderr: str, lead: str | None = None) -> str: + """The one `bump ...` a remedy prints, from `bump` to the closing + backtick. `lead` is what comes before `bump`: `bump-leg.py`'s + `scripts/family.py `, or by default `family.py`'s own, the interpreter + and the path to the file from where the run stood (`family_lead`, + #1127).""" + lead = family_lead() if lead is None else lead + found = re.findall(re.escape(f"`{lead}") + r"(bump [^`]*)`", stderr) assert len(found) == 1, stderr return found[0] @@ -218,7 +220,7 @@ def test_a_bump_leg_remedy_that_names_family_bump_names_the_holder( result = BUMP_LEG_CASES[code](project, scaffolded, tmp_path) assert result.returncode == 2, result.stdout + result.stderr assert f"REFUSED {code}: " in result.stderr - line = the_bump_line(result.stderr, "scripts/") + line = the_bump_line(result.stderr, "scripts/family.py ") assert line == "bump --family-root --member --to " # The holder as a shell is handed it: quoted, so that a basetemp whose # path holds a space (`--basetemp="my work/bt"`) is still one word. diff --git a/tests/test_family.py b/tests/test_family.py index 0b78e50a..b1c31f45 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -48,7 +48,7 @@ sys.path.insert(0, str(REPO / "scripts")) from repo_shape import ( # noqa: E402 - Refusal, load_yaml, shell_quoted, tree_digest, + PYTHON, Refusal, load_yaml, shell_quoted, tree_digest, ) from shape_materialize import ( # noqa: E402 SHAPE_REPOSITORY, readme_shape_lines, readme_trailing_shape_line, @@ -540,7 +540,11 @@ def test_add_refuses_a_spelling_of_a_member_the_family_already_pins( assert result.returncode == 2, result.stderr + result.stdout assert "REFUSED member-already-present" in result.stderr assert "the row IRRS pins InkRouter/IRRS" in result.stderr - assert ("`family.py bump --family-root " + # The line as every remedy prints one of `family.py`'s own: the + # interpreter and the path that reaches the file from where the run + # stood, which is this process's (#1127). + script = shell_quoted(os.path.relpath(FAMILY.resolve())) + assert (f"`{PYTHON} {script} bump --family-root " f"{shell_quoted(holder.resolve())} --member IRRS --to " "`") in result.stderr assert "mounting" not in result.stdout diff --git a/tests/test_family_add_member_id.py b/tests/test_family_add_member_id.py index f5eb1f39..244182f6 100644 --- a/tests/test_family_add_member_id.py +++ b/tests/test_family_add_member_id.py @@ -53,7 +53,7 @@ ) #: The shell's reading of a printed line on this host: the holder `bump` #: names is one quoted word of it, whatever its path holds (#927). -from test_family_printed_quoting import host_words +from test_family_printed_quoting import family_lead, host_words #: The project the renamed copy is of, and the name it is added under. PINNED = "IRRS" @@ -71,10 +71,10 @@ DUPLICATE_ID_REMEDIATION = ( f"Remediation: a family pins each project once. If {ORG}/{{who}} is " - f"{ORG}/{PINNED} renamed at the forge, `family.py remove --family-root " + f"{ORG}/{PINNED} renamed at the forge, `{{lead}}remove --family-root " "{holder} --member " f"{PINNED}` takes the old row out and this `add` then pins it under the " - "new name; `family.py bump --family-root {holder} --member " + "new name; `{lead}bump --family-root {holder} --member " f"{PINNED} --to ` moves the row that is there instead. Two " "projects that record one id are a fault in one of their " "`project.yaml` files, mended in that project, not in the family.") @@ -83,9 +83,11 @@ def duplicate_id_remediation(who: str, holder: Path) -> str: """What `add` tells the human to do about `member-duplicate-id`: the `remove` and the `bump` it names each carry `--family-root` and the - holder, as both verbs require (#927, #1005).""" + holder, as both verbs require (#927, #1005), and each begins with the + interpreter and the path to `family.py` from where the run stood + (#1127).""" return DUPLICATE_ID_REMEDIATION.format( - who=who, holder=shell_quoted(holder.resolve())) + who=who, holder=shell_quoted(holder.resolve()), lead=family_lead()) def renamed_copy(family, tmp_path: Path) -> Path: @@ -174,7 +176,8 @@ def test_each_command_the_refusal_names_runs_on_the_row_it_names( remotes = renamed_copy(family, tmp_path) refused = offline.add(holder, RENAMED, remotes=remotes) assert refused.returncode == 2, refused.stdout + refused.stderr - commands = re.findall(rf"`family\.py ({verb} [^`]*)`", refused.stderr) + commands = re.findall(rf"`{re.escape(family_lead())}({verb} [^`]*)`", + refused.stderr) assert len(commands) == 1, refused.stderr older = git("rev-parse", "main~1", cwd=family["remotes"] / f"{PINNED}.git").stdout.strip() diff --git a/tests/test_family_bump_to_checks.py b/tests/test_family_bump_to_checks.py index 59cb1121..ea27ca98 100644 --- a/tests/test_family_bump_to_checks.py +++ b/tests/test_family_bump_to_checks.py @@ -60,7 +60,7 @@ from test_family_bump_to_a_tag import annotated, both_runs, peeled_line from test_family_member_path import MEMBER, OTHER, older, raw #: The shell's reading of a printed line, and of a refusal's `switch`. -from test_family_printed_quoting import host_words +from test_family_printed_quoting import family_lead, host_words from test_family_tracking_branch import remedy_switch PATH = f"members/{MEMBER}" @@ -84,7 +84,8 @@ def duplicate_id_refusal(to: str, holder: Path) -> tuple[str, str]: """The two lines `bump --to ` is refused with when the member's `project.yaml` at `to` records the neighbour's id. The `remove` the remedy names carries `--family-root` and the holder, as `remove` - requires (#1005).""" + requires (#1005), after the interpreter and the path to `family.py` + from where the run stood (#1127).""" return ( f"REFUSED member-duplicate-id: {ORG}/{MEMBER} at {to[:12]} is " f"already a member of {NAME} under another name: its project.yaml " @@ -94,7 +95,8 @@ def duplicate_id_refusal(to: str, holder: Path) -> tuple[str, str]: "Remediation: a family pins each project once. Pass `--to` a commit " f"of {ORG}/{MEMBER} whose project.yaml records an id no other row " f"records. If {ORG}/{MEMBER} and {ORG}/{OTHER} are one project, " - f"`family.py remove --family-root {shell_quoted(holder.resolve())} " + f"`{family_lead()}remove --family-root " + f"{shell_quoted(holder.resolve())} " f"--member {OTHER}` takes the other row out first. " "Two projects that record one id are a fault in one of their " "`project.yaml` files, mended in that project, not in the family.") diff --git a/tests/test_family_printed_quoting.py b/tests/test_family_printed_quoting.py index 787fc46b..896e40ce 100644 --- a/tests/test_family_printed_quoting.py +++ b/tests/test_family_printed_quoting.py @@ -105,6 +105,17 @@ def host_words(line: str) -> list[str]: else shlex.split(line)) +def family_lead(cwd: Path | None = None) -> str: + """How a line of `family.py`'s own begins where the tool prints one from + `cwd` (this process's where None): the interpreter, then the path that + reaches the file from there as one word, each followed by a space. It is + how `init`'s NEXT STEPS print their `add` (#488), and how every remedy + that names a verb prints its line (#1127).""" + here = Path.cwd() if cwd is None else Path(cwd) + script = os.path.relpath(FAMILY.resolve(), here.resolve()) + return f"{PYTHON} {shell_quoted(script)} " + + # --- the quoter --------------------------------------------------------------- #: What must survive UNQUOTED, on both platforms: a line whose whole job is diff --git a/tests/test_family_remedy_lines.py b/tests/test_family_remedy_lines.py new file mode 100644 index 00000000..b2f2ee20 --- /dev/null +++ b/tests/test_family_remedy_lines.py @@ -0,0 +1,373 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The lines of `family.py`'s own that `bump`'s `member-unknown` and `add`'s +`member-already-present` print run as printed, from the branch a fresh clone +stands on (#1127, RV1117's follow-ups on #1117). + +RV1117's review of #1117 found seven things about these remedies, and each +is measured here as the remedy now prints it: + +* N1: on the holder's tracking branch, where `init` and a fresh clone leave + a person, the `add` that `member-unknown` names was refused + `family-on-tracking-branch`. So was the `bump` that `member-already-present` + names (found by measuring it): both refusals are said before + `_checked_branch` reads the branch. Each remedy now says the line runs on + a branch other than the tracking branch and names the `git switch -c` that + makes one, as the holder's `validate-family.py` says before its `remove`, + then `BRANCH_LEFT` (#607). Typed in order from the tracking branch, the + switch, then the line, each act goes through; the line alone is refused. +* N2: the lines began with a bare `family.py`, which runs only from + `scripts/`. Each now begins as `init`'s NEXT STEPS' `add` does: the + interpreter, then the path that reaches the file from where the refused + run stood, and is typed here from that folder by that path. +* N3: `member-unknown`'s `add` left `` blank where `family.yaml`'s + `org:` is known, a redirection in every shell when pasted. The owner is + now the one `--member` was typed with, else the family's `org:`, else the + blank, each only where `/` is a `--member` `add` takes. +* N4: the row's `--member` name was printed raw: a row `IR RS` was two + words. It is now one, quoted for the reader's shell. +* N6: a holder under a folder whose name holds a backtick closed the + remedy's backticks inside its path. A command that holds one is now + printed bare. +* N7: `member-already-present` said "`remove` takes it out" with no line to + type. It now prints the `remove` line beside the `bump` line, and both are + typed here. + +N5, the README sentence's list of verbs tied to `NAMED`, is +`tests/test_readme_windows_quoting_sentence.py`'s. The holder is +`tests/test_family.py`'s family, copied and put back on its tracking branch +as `tests/test_family_tracking_branch.py` does; its spare member `IRQS` is +in its remotes and not in the holder. No network: `gh` is a shim that exits +non-zero. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +from conftest import git, run_script +from repo_shape import PYTHON, shell_quoted +#: The family fixture and its names, the offline environment and the +#: helpers of the tests that already trigger these codes, imported from the +#: modules that own them rather than rebuilt. +from test_family import ( # noqa: F401 + FAMILY, MEMBERS, NAME, ORG, SPARE, family, manifest, validate, +) +from test_family_add_refusals import offline, refusal # noqa: F401 +from test_family_dry_run import isolated +from test_family_printed_quoting import copied_holder, family_lead, host_words +from test_family_tracking_branch import TRACKING, older_commit +#: The sentence every remedy that names a `switch -c` ends with (#607). +from test_validate_family_branch_step import BRANCH_LEFT + +#: A commit id for a `--to` that is never read: `member-unknown` is said +#: before `--to` is. +COMMIT = "0123456789abcdef0123456789abcdef01234567" + +#: The pinned member `add` is refused for, by the row's name. +PINNED = MEMBERS[0] + +#: What the remedies say before the line of `family.py` they name, with the +#: verbs the tracking branch is refused by. +OFF_TRACKING = ("on a branch other than the holder's tracking branch, which " + "{refuses} ({switch} makes one), ") + + +def on_tracking(family, base: Path) -> Path: + """A private copy of the holder under `base`, back on its tracking + branch with every member the fixture added: where `init` leaves a + person, and where a fresh clone stands.""" + holder = copied_holder(family, base) + git("switch", "-q", "-C", TRACKING, cwd=holder) + assert git("status", "--porcelain", cwd=holder).stdout == "" + return holder + + +@pytest.fixture +def stand(tmp_path) -> Path: + """The folder a person runs the refused command from, and types its + remedy in: not the holder's, and not the standard's, with a space in its + name.""" + folder = tmp_path / "my desk" + folder.mkdir() + return folder.resolve() + + +def cited(command: str, ticked: bool) -> str: + return f"`{command}`" if ticked else command + + +def switch_line(holder: Path, branch: str) -> str: + return f"git -C {shell_quoted(holder)} switch -c {branch}" + + +def family_line(lead: str, verb: str, holder: Path, member: str) -> str: + """` --family-root --member `, where + `member` is already spelled for the shell.""" + return (f"{lead}{verb} --family-root {shell_quoted(holder)} " + f"--member {member}") + + +def unknown_remediation(holder: Path, lead: str, ticked: bool = True) -> str: + """What `bump --member IRQS` is told: make a branch, then the `add` that + puts IRQS in the family, owned by the family's `org:`.""" + add = family_line(lead, "add", holder, shell_quoted(f"{ORG}/{SPARE}")) + switch = switch_line(holder, f"family/add-{SPARE.lower()}") + clause = OFF_TRACKING.format(refuses="`add` refuses", + switch=cited(switch, ticked)) + return (f"Remediation: {clause}{cited(add, ticked)} puts it in the " + f"family first. {BRANCH_LEFT}") + + +def present_remediation(holder: Path, lead: str, ticked: bool = True) -> str: + """What `add` of the member the holder already pins is told: make a + branch, then the `bump` that moves the row, or the `remove` that takes + it out.""" + name = shell_quoted(PINNED) + bump = family_line(lead, "bump", holder, name) + " --to " + remove = family_line(lead, "remove", holder, name) + switch = switch_line(holder, f"family/bump-{PINNED.lower()}") + clause = OFF_TRACKING.format(refuses="`bump` and `remove` refuse", + switch=cited(switch, ticked)) + return (f"Remediation: {clause}{cited(bump, ticked)} moves an existing " + f"member and {cited(remove, ticked)} takes it out. Adding it " + f"twice is not a way to move it. {BRANCH_LEFT}") + + +def bump_unknown(holder: Path, stand: Path, env: dict, member: str = SPARE): + """`bump` of a member the holder does not pin, typed in `stand`.""" + return run_script(FAMILY, "bump", "--family-root", str(holder), + "--member", member, "--to", COMMIT, cwd=stand, env=env) + + +def add_present(family, holder: Path, stand: Path, env: dict): + """`add` of the member the holder already pins, typed in `stand`.""" + return run_script(FAMILY, "add", "--family-root", str(holder), + "--member", f"{ORG}/{PINNED}", "--local-remote-dir", + str(family["remotes"]), cwd=stand, env=env) + + +def typed(line: str, stand: Path, env: dict, *extra: str): + """`line` typed in `stand` as the reader's shell reads it: the + interpreter it names, and the script by the path it names from there.""" + words = host_words(line) + assert words[0] == PYTHON, line + return run_script(Path(words[1]), *words[2:], *extra, cwd=stand, env=env) + + +def switched(line: str, stand: Path) -> None: + """The remedy's `git switch -c`, typed in `stand` as the shell reads + it.""" + words = host_words(line) + assert words[:2] == ["git", "-C"], line + git(*words[1:], cwd=stand) + + +def branch_of(holder: Path) -> str: + return git("symbolic-ref", "--short", "HEAD", cwd=holder).stdout.strip() + + +def rows_of(holder: Path) -> dict: + return {row["project"]: row for row in manifest(holder)["members"]} + + +# --- N1, N2: member-unknown's `add`, from the tracking branch ---------------- + +def test_member_unknowns_add_runs_as_printed_from_the_tracking_branch( + family, offline, stand, tmp_path): + """The remediation, word for word: the branch first, then the `add` by + the interpreter and the path from where the refused `bump` stood, the + owner filled in. Typed alone on the tracking branch the `add` is refused + there, as the remedy says; typed after its `switch -c`, from the same + folder, it puts IRQS in the family on that branch, and the holder is + clean and its gate green.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + lead = family_lead(stand) + refused = bump_unknown(holder, stand, env) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refusal(refused.stderr) == ( + f"REFUSED member-unknown: {SPARE} is not a member of {NAME}", + unknown_remediation(holder, lead)) + switch = switch_line(holder, f"family/add-{SPARE.lower()}") + add = family_line(lead, "add", holder, shell_quoted(f"{ORG}/{SPARE}")) + remotes = ("--local-remote-dir", str(family["remotes"])) + + alone = typed(add, stand, env, *remotes) + assert alone.returncode == 2, alone.stdout + alone.stderr + assert alone.stderr.startswith("REFUSED family-on-tracking-branch: ") + assert SPARE not in rows_of(holder) + + switched(switch, stand) + done = typed(add, stand, env, *remotes) + assert done.returncode == 0, done.stdout + done.stderr + assert branch_of(holder) == f"family/add-{SPARE.lower()}" + rows = rows_of(holder) + assert sorted(rows) == sorted([*MEMBERS, SPARE]) + assert rows[SPARE]["repository"] == f"{ORG}/{SPARE}" + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert validate(holder).returncode == 0 + + +# --- N1b, N2, N7: member-already-present's `bump` and `remove` --------------- + +@pytest.mark.parametrize("verb", ["bump", "remove"]) +def test_member_already_presents_lines_run_as_printed_from_the_tracking_branch( + family, offline, stand, tmp_path, verb): + """The remediation, word for word: the branch first, then a `bump` line + and a `remove` line, each by the interpreter and the path from where the + refused `add` stood. Typed alone on the tracking branch each is refused + there; typed after the `switch -c`, `bump` (its `` filled in) + moves the row's pin and `remove` takes the row out, and the holder is + clean and its gate green.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + lead = family_lead(stand) + refused = add_present(family, holder, stand, env) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refusal(refused.stderr) == ( + f"REFUSED member-already-present: {PINNED} is already a member of " + f"{NAME}: the row {PINNED} pins {ORG}/{PINNED}", + present_remediation(holder, lead)) + line = family_line(lead, verb, holder, PINNED) + older = older_commit(family, PINNED) + if verb == "bump": + line += f" --to {older}" + before = rows_of(holder) + + alone = typed(line, stand, env) + assert alone.returncode == 2, alone.stdout + alone.stderr + assert alone.stderr.startswith("REFUSED family-on-tracking-branch: ") + assert rows_of(holder) == before + + switched(switch_line(holder, f"family/bump-{PINNED.lower()}"), stand) + done = typed(line, stand, env) + assert done.returncode == 0, done.stdout + done.stderr + assert branch_of(holder) == f"family/bump-{PINNED.lower()}" + rows = rows_of(holder) + if verb == "bump": + assert sorted(rows) == sorted(MEMBERS) + assert rows[PINNED]["pin"]["commit"] == older + else: + assert sorted(rows) == sorted(MEMBERS[1:]) + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert validate(holder).returncode == 0 + + +# --- N3: the owner of member-unknown's `add` --------------------------------- + +def without_org(text: str) -> str: + edited = re.sub(r"(?m)^org: .*\n", "", text, count=1) + assert edited != text, "premise: family.yaml has an `org:` line" + return edited + + +def org_of(value: str): + """An edit of `family.yaml` that gives `org:` the YAML `value`.""" + def edit(text: str) -> str: + edited = re.sub(r"(?m)^org: .*$", f"org: {value}", text, count=1) + assert edited != text, "premise: family.yaml has an `org:` line" + return edited + return edit + + +#: `(--member as typed, an edit of family.yaml, the --member the line names)`: +#: the typed owner first, then the family's `org:`, then the blank, each +#: only where `/` is a `--member` `add` takes. +OWNERS = { + "the-familys-org": (SPARE, None, shell_quoted(f"{ORG}/{SPARE}")), + "the-typed-owner": (f"OtherOrg/{SPARE}", None, + shell_quoted(f"OtherOrg/{SPARE}")), + "a-typed-owner-add-refuses": (f"_Other/{SPARE}", None, + shell_quoted(f"{ORG}/{SPARE}")), + "no-org": (SPARE, without_org, f"/{shell_quoted(SPARE)}"), + "an-empty-org": (SPARE, org_of('""'), f"/{shell_quoted(SPARE)}"), + "an-org-add-refuses": (SPARE, org_of('"Ink Router"'), + f"/{shell_quoted(SPARE)}"), + "no-org-but-typed": (f"OtherOrg/{SPARE}", without_org, + shell_quoted(f"OtherOrg/{SPARE}")), +} + + +@pytest.mark.parametrize("typed_member, edit, expected", OWNERS.values(), + ids=OWNERS) +def test_member_unknowns_add_names_the_owner_where_it_is_known( + family, offline, stand, tmp_path, typed_member, edit, expected): + """The `--member` of the `add` line: `` is a blank only where + neither the typed `--member` nor `family.yaml` gives an owner `add` + takes, and the value is one word for the reader's shell.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + if edit is not None: + path = holder / "family.yaml" + path.write_text(edit(path.read_text(encoding="utf-8")), + encoding="utf-8") + refused = bump_unknown(holder, stand, env, typed_member) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refused.stderr.startswith("REFUSED member-unknown: ") + lead = family_lead(stand) + found = re.findall(rf"`{re.escape(lead)}(add [^`]*)`", refused.stderr) + assert found == [f"add --family-root {shell_quoted(holder)} --member " + f"{expected}"] + + +# --- N4: a row's name is one word -------------------------------------------- + +def test_a_rows_name_with_a_space_is_one_word_in_both_lines( + family, offline, stand, tmp_path): + """A row whose `project:` holds a space, edited by hand and committed: + `add` of the repository it pins names it in its `bump` and `remove` + lines as ONE word, the row's name whole.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + path = holder / "family.yaml" + text = path.read_text(encoding="utf-8") + edited = re.sub(rf'(?m)^( - project: )"?{PINNED}"?$', r'\1"IR RS"', + text, count=1) + assert edited != text, "premise: the row's `project:` line" + path.write_text(edited, encoding="utf-8") + git("commit", "-q", "-m", "A row named by hand", "--", "family.yaml", + cwd=holder) + refused = add_present(family, holder, stand, env) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refused.stderr.startswith("REFUSED member-already-present: ") + lead = family_lead(stand) + lines = re.findall(rf"`{re.escape(lead)}((?:bump|remove) [^`]*)`", + refused.stderr) + assert [host_words(line) for line in lines] == [ + ["bump", "--family-root", str(holder), "--member", "IR RS", "--to", + ""], + ["remove", "--family-root", str(holder), "--member", "IR RS"]] + + +# --- N6: a holder under a folder named with a backtick ----------------------- + +def test_a_command_holding_a_backtick_is_printed_bare( + family, offline, stand, tmp_path): + """Under `a`b/`, every command each remedy names holds the backtick, so + each is printed bare and the prose around it is the same; the `add` + `member-unknown` names, typed after its switch, still puts IRQS in the + family.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "a`b") + assert "`" in str(holder), "premise: the holder's path holds a backtick" + lead = family_lead(stand) + + present = add_present(family, holder, stand, env) + assert present.returncode == 2, present.stdout + present.stderr + assert refusal(present.stderr)[1] == present_remediation( + holder, lead, ticked=False) + + unknown = bump_unknown(holder, stand, env) + assert unknown.returncode == 2, unknown.stdout + unknown.stderr + assert refusal(unknown.stderr)[1] == unknown_remediation( + holder, lead, ticked=False) + switched(switch_line(holder, f"family/add-{SPARE.lower()}"), stand) + done = typed(family_line(lead, "add", holder, + shell_quoted(f"{ORG}/{SPARE}")), + stand, env, "--local-remote-dir", str(family["remotes"])) + assert done.returncode == 0, done.stdout + done.stderr + assert SPARE in rows_of(holder) + assert git("status", "--porcelain", cwd=holder).stdout == "" diff --git a/tests/test_readme_windows_quoting_sentence.py b/tests/test_readme_windows_quoting_sentence.py index 695c2803..23b18c61 100644 --- a/tests/test_readme_windows_quoting_sentence.py +++ b/tests/test_readme_windows_quoting_sentence.py @@ -66,18 +66,32 @@ "scripts/family.py": { "_print_next": 1, # the `NEXT` line: `git push -u origin ...` "_switch_command": 1, # the `git switch -c ` line - "_add_command": 3, # the `family.py add --family-root ...` line + # every ` family.py --family-root ...` line (#1127) + "_family_line": 3, + "_add_command": 1, # `init`'s `add`: its `--member /...` + # `bump`'s `member-unknown`: its `add` line's `--member` (#1127) + "_unknown_members_repository": 2, "_print_next_steps": 2, # `init`'s block: its `switch -c` and `cd` "_checked_mount_path": 1, # the `git rm -r --cached -- ` remedy - "_checked_add_target": 1, # `member-already-present`'s `bump` line - # `add`'s `member-duplicate-id`: its `bump` line (#927) and its - # `remove` line (#1005) - "_checked_member_id": 2, - "_checked_bumped_id": 1, # `bump`'s `member-duplicate-id`: `remove` - "cmd_bump": 1, # `bump`'s `member-unknown`: its `add` line }, } +#: The kinds of line README's sentence lists for `family.py`, in its words, +#: and the functions of NAMED that print each (#1127, RV1117's N5 on #1117): +#: the sentence's list was tied to nothing, so a verb dropped from both docs +#: passed. `_family_line` prints every line of `family.py`'s own that a +#: remedy names, so its kinds are the verbs its calls name +#: (`family_line_verbs`), read from the source. +FAMILY_LINES = { + "_print_next": {"NEXT"}, + "_switch_command": {"switch -c"}, + "_family_line": None, + "_add_command": {"add"}, + "_unknown_members_repository": {"add"}, + "_print_next_steps": {"switch -c", "cd"}, + "_checked_mount_path": {"git rm -r --cached"}, +} + #: The quoters' own home is not a tool, and only these two bodies are what the #: sentence need not name: `shell_line` spells each word by `shell_quoted`, and #: `shell_quoted` is the host-dialect rule itself. Any other function of @@ -344,6 +358,63 @@ def test_the_functions_that_quote_for_the_host_are_the_ones_the_sentence_names( f"{NAMED[tool]}: change the sentence in both places, then NAMED here.") +def sentence_lists_for_family_py() -> set: + """The kinds of line README's sentence names for `family.py`, read from + README as written, its backticks kept: "`family.py`'s `NEXT`, ..., `bump` + and `remove` lines".""" + text = squashed((REPO / "README.md").read_text(encoding="utf-8")) + listed = re.search(r"`family\.py`'s (.*?) lines are quoted for the shell", + text) + assert listed is not None, "README names no `family.py` lines" + return set(re.findall(r"`([^`]+)`", listed.group(1))) + + +def family_line_verbs(source: str) -> list: + """The first argument of each call of `_family_line` in `source`: the + verb of the line it prints, or None where that is not a string constant, + which nobody can read without running the tool.""" + return [call.args[0].value + if call.args and isinstance(call.args[0], ast.Constant) + and isinstance(call.args[0].value, str) else None + for call in ast.walk(ast.parse(source)) + if isinstance(call, ast.Call) + and isinstance(call.func, ast.Name) + and call.func.id == "_family_line"] + + +def family_line_kinds(source: str) -> set: + """Every kind of line `FAMILY_LINES` says the functions of `family.py` + print, `_family_line`'s read from `source`.""" + kinds = set() + for function, said in FAMILY_LINES.items(): + kinds |= set(family_line_verbs(source)) if said is None else said + return kinds + + +def test_the_kinds_of_family_lines_are_the_ones_the_sentence_lists(): + """README lists the kinds of `family.py` line that are quoted for the + shell they are printed in, and NAMED pins the functions that print them: + each function's kinds, `_family_line`'s from the verb each call names, + are the sentence's list, no more and no fewer (#1127). A verb dropped + from the sentence, or a line of a new verb, fails here.""" + source = (REPO / "scripts" / "family.py").read_text(encoding="utf-8") + assert set(FAMILY_LINES) == set(NAMED["scripts/family.py"]) + verbs = family_line_verbs(source) + assert verbs, "`family.py` calls `_family_line` nowhere" + assert None not in verbs, verbs + assert family_line_kinds(source) == sentence_lists_for_family_py() + + +def test_the_kinds_follow_the_verbs_family_line_is_called_with(): + """`family_line_verbs` reads each call's verb, and a verb no one can read + is None, which the test of the kinds refuses.""" + source = ("def f(h, v):\n _family_line('bump', h)\n" + " _family_line(v, h)\n other('add', h)\n") + assert family_line_verbs(source) == ["bump", None] + assert "siblings" in family_line_kinds( + "def f(h):\n return _family_line('siblings', h)\n") + + def test_a_quoter_imported_under_another_name_is_a_host_call(): source = ("from repo_shape import shell_line as joined, shell_quoted\n" "spelled = joined\n" diff --git a/tests/test_remove_add_remedies_family_root.py b/tests/test_remove_add_remedies_family_root.py index adfd0ad5..5d052782 100644 --- a/tests/test_remove_add_remedies_family_root.py +++ b/tests/test_remove_add_remedies_family_root.py @@ -13,9 +13,8 @@ row out first. Both KNOW the holder, so each names it, in the one word `shell_quoted` makes of its path, as #927's `bump` line does. * `bump`'s `member-unknown`, which says `add` puts the member in the family - first. It names the holder the same way, and leaves `` for the person - to fill in, as the verbs' usage lines do: nothing there says whose - repository the project is. + first. It names the holder the same way, and the owner `family.yaml`'s + `org:` names (#1127; `tests/test_family_remedy_lines.py` has the rest). Each case is measured two ways, over a holder in a plain folder, under two with a space in their names (one of them with an apostrophe too: the paths a @@ -60,7 +59,7 @@ from test_family_bump_to_checks import records_the_neighbours_id from test_family_dry_run import commit_in_mount, isolated from test_family_member_path import MEMBER, OTHER -from test_family_printed_quoting import copied_holder, host_words +from test_family_printed_quoting import copied_holder, family_lead, host_words #: A commit id for a `--to` that is never read: `member-unknown` is said #: before `--to` is. @@ -208,8 +207,8 @@ def mend_bump_unknown(reach, family, env, kept): lambda line, kept: line, lambda family, kept: [], mend_bump_duplicate_id), "bump-member-unknown": Case( - "member-unknown", "add", f"/{SPARE}", refuse_bump_unknown, - lambda line, kept: line.replace("", ORG), + "member-unknown", "add", f"{ORG}/{SPARE}", refuse_bump_unknown, + lambda line, kept: line, # `add` reads the member from the family's own bare repositories, # as every `add` in this suite does: nothing here reaches a forge. lambda family, kept: ["--local-remote-dir", str(family["remotes"])], @@ -217,11 +216,14 @@ def mend_bump_unknown(reach, family, env, kept): } -def the_line(stderr: str, case: Case) -> str: - """The one `family.py ...` the refusal prints, from the verb to - the closing backtick, and the refusal checked to be the case's code.""" +def the_line(stderr: str, case: Case, reach: Reach) -> str: + """The one ` ...` the refusal prints, from the verb to the closing + backtick, after the interpreter and the path to `family.py` from where + the refused run stood (#1127), and the refusal checked to be the case's + code.""" assert f"REFUSED {case.code}: " in stderr, stderr - found = re.findall(rf"`family\.py ({case.verb} [^`]*)`", stderr) + lead = re.escape(f"`{family_lead(reach.cwd)}") + found = re.findall(rf"{lead}({case.verb} [^`]*)`", stderr) assert len(found) == 1, stderr return found[0] @@ -240,7 +242,7 @@ def test_the_line_a_remedy_names_carries_the_holder_whole( refused run was pointed at it.""" stderr, kept = case.refuse(reach, family, isolated(offline, tmp_path), tmp_path) - line = the_line(stderr, case) + line = the_line(stderr, case, reach) assert line == expected(case, reach) assert host_words(line) == [case.verb, "--family-root", str(reach.holder), "--member", case.member] @@ -253,7 +255,7 @@ def test_the_line_a_remedy_names_parses( words are accepted: no `the following arguments are required`.""" stderr, kept = case.refuse(reach, family, isolated(offline, tmp_path), tmp_path) - words = host_words(case.fill(the_line(stderr, case), kept)) + words = host_words(case.fill(the_line(stderr, case, reach), kept)) capsys.readouterr() args = parsed(family_module, words, capsys) assert args.family_root == str(reach.holder) @@ -270,7 +272,7 @@ def test_the_line_a_remedy_names_runs_as_printed_and_does_what_it_says( is in the family: whichever the remedy says it is for.""" env = isolated(offline, tmp_path) stderr, kept = case.refuse(reach, family, env, tmp_path) - words = host_words(case.fill(the_line(stderr, case), kept)) + words = host_words(case.fill(the_line(stderr, case, reach), kept)) done = run_script(FAMILY, *words, *case.extra(family, kept), cwd=elsewhere, env=env) assert "the following arguments are required" not in done.stderr