diff --git a/scripts/family.py b/scripts/family.py index 318bec02..b33f24a0 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -2319,19 +2319,41 @@ 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)) + (#1127, RV1117's N1): `bump`'s `member-unknown` and `add`'s + `member-already-present` in either run, and the two + `member-duplicate-id` remedies in a dry run, which asks no branch + (#1228; a real `add` or `bump` asks them after `_checked_branch`). 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".""" + return _branch_clause(_switch_command(family.root, suggested), refuses) + + +def _branch_clause(switch: str, refuses: str) -> str: + """`_off_the_tracking_branch`'s words, around a `switch -c` line already + spelled: `init`'s `family-holder-already-here` names a holder it has + not opened, by its path, and its branch is `init`'s placeholder + (`_add_branch_switch`), which `_switch_command` would quote (#1228).""" return ("on a branch other than the holder's tracking branch, which " - f"{refuses} ({switch} makes one), ") + f"{refuses} ({_cited(switch)} makes one), ") + + +def _add_branch_switch(holder: Path) -> str: + """`git -C switch -c family/add-`: the branch `init` + says to make before the `add` it prints, in its NEXT STEPS block and in + the remedy of `family-holder-already-here` (#487, #1228). The holder is + spelled by `shell_quoted` for the reader's shell (#486). The branch is + left bare: its `` is the placeholder the `--member` of that + `add` says (#607), and quoted it would read as a value (`_family_line`). + `add`'s own refusal names the branch lowercased.""" + return (shell_line(["git", "-C", holder, "switch", "-c"]) + + " family/add-") def _add_command(holder: Path, org: str) -> str: @@ -2500,15 +2522,24 @@ def _landing(args, family: str) -> tuple[Path | None, Path]: # names it (#609's rule: a refusal by name, never a traceback). The two # paths are named as `shown_path` names a path (#766): `--into`, or the # directory the person stands in, can hold an ESC or a line break. + # THE `add` THE REMEDY NAMES RUNS AS PRINTED (#1228): after the branch + # it needs, as `init`'s NEXT STEPS print the two, since a holder `init` + # made stands on its tracking branch, where `add` refuses; and `_cited`, + # so under a folder named with a backtick the line is printed whole + # (#1127, RV1117's N6). if into.name == family and os.path.isfile(into / family / MANIFEST): + clause = _branch_clause(_add_branch_switch(into / family), + "`add` refuses") raise Refusal( "family-holder-already-here", f"{shown_path(into)} is already the {family} family folder and " f"its holder is at {shown_path(into / family)}, so there is " "nothing to init", - f"Remediation: `{_add_command(into / family, args.org)}` grows " - "it, and `make siblings` in the holder puts each member's " - "working clone beside it. There is no --force.") + f"Remediation: {clause}" + + _cited(_add_command(into / family, args.org)) + + " grows it, and `make siblings` in the holder puts each " + "member's working clone beside it. There is no --force. " + + BRANCH_LEFT) folder, holder = ((into, into / family) if into.name == family else (into / family, into / family / family)) if args.into is None: @@ -3500,7 +3531,6 @@ def _print_next_steps(args, work: Path, folder: Path | None) -> None: clones anything in a holder whose parent folder is not named after the family (#543), so there the block says so instead of promising clones. """ - switch = shell_line(["git", "-C", work, "switch", "-c"]) beside = ("`make siblings` then puts each member's WORKING clone\n" "beside the holder, in the family folder, on its tracking " "branch.") @@ -3516,7 +3546,7 @@ def _print_next_steps(args, work: Path, folder: Path | None) -> None: print(f""" NEXT STEPS - {switch} family/add- + {_add_branch_switch(work)} {_add_command(work, args.org)} cd {shell_quoted(work)} make siblings @@ -6646,7 +6676,7 @@ def _row_recording(family: Family, project_id: str, def _checked_member_id(family: Family, repository: str, project: str, - project_id: str) -> None: + project_id: str, *, dry_run: bool = False) -> None: """Refused when a row already records the `id` the member's own `project.yaml` does (#330): the project is a member under another name. @@ -6671,6 +6701,11 @@ def _checked_member_id(family: Family, repository: str, project: str, and nothing it would pass. The validator keeps its own spelling of the rule, as it keeps `member-path`'s: it ships INTO every holder, and a shared copy would move every holder's pin. + + `dry_run` IS `add --dry-run`'s ASK (#1228), which reads no branch: its + remedy says the branch its two lines need, as `member-already-present` + says it. A real `add` asks this after `_checked_branch`, so its remedy + names none. """ row = _row_recording(family, project_id) if row is not None: @@ -6678,7 +6713,16 @@ def _checked_member_id(family: Family, repository: str, project: str, # THE `remove` AND THE `bump` IT NAMES RUN AS PRINTED (#1005, #927), # 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. + # `add` asks this after `_checked_branch`, which refuses the + # tracking branch, so no branch is named. A dry run asks it before + # the branch is read, so it names the branch first: the one the + # `remove` line's own refusal would name, and `BRANCH_LEFT` (#1228). + branch, left = "", "" + if dry_run: + branch = _off_the_tracking_branch( + family, "`bump` and `remove` refuse", + _suggested_branch("remove", str(row.get("id") or name))) + left = " " + BRANCH_LEFT raise Refusal( "member-duplicate-id", f"{project} is already a member of {family.data.get('name')} " @@ -6688,6 +6732,7 @@ 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, " + + branch + _cited(_family_line("remove", family.root, "--member", name)) + " takes the old row out and this `add` then pins it under the " "new name; " @@ -6695,7 +6740,7 @@ def _checked_member_id(family: Family, repository: str, project: str, + " --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.") + "mended in that project, not in the family." + left) def _probe_member(url: str, protocol: list[str], repository: str, @@ -7609,10 +7654,13 @@ def cmd_add(args) -> int: print(UNREADABLE_HINT, file=sys.stderr) return 2 if probed_id is not None: - _checked_member_id(family, repository, project, probed_id) + _checked_member_id(family, repository, project, probed_id, + dry_run=args.dry_run) # Every refusal above is the same under `--dry-run`, which leaves here, - # before `MountRollback` is taken and anything is mounted (#326). + # before `MountRollback` is taken and anything is mounted (#326), but + # that `member-duplicate-id`'s remedy there names a branch first: the + # real run has read the branch by then, and the dry run has not (#1228). if args.dry_run: return _add_dry_run(family, repository, project, path, url, protocol, commit, at, args.trailer, suggested) @@ -8782,7 +8830,7 @@ def _member_id_at(member: Path, commit: str, project: str, def _checked_bumped_id(family: Family, row: dict, commit: str, - project_id: str) -> None: + project_id: str, *, dry_run: bool = False) -> None: """`member-duplicate-id` for a `bump` whose `--to` records, in the member's own `project.yaml`, the `id` another row already records (#541): the question `add` asks since #330 (`_checked_member_id`), asked @@ -8802,6 +8850,10 @@ def _checked_bumped_id(family: Family, row: dict, commit: str, bump replaces it, so the id it records today is no other project's. Every other row is asked as `add` asks it, so `bump` refuses what the gate would refuse and nothing it would pass. + + `dry_run` IS `bump --dry-run`'s ASK (#1228), which reads no branch + first: its remedy says the branch its `remove` line needs. A real + `bump` asks this after `_checked_branch`, so its remedy names none. """ project = str(row["project"]) held = _row_recording(family, project_id, besides=project) @@ -8811,7 +8863,17 @@ def _checked_bumped_id(family: Family, row: dict, commit: str, # THE `remove` IT NAMES RUNS AS PRINTED (#1005): `remove` requires # `--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. + # real `bump` asks this after `_checked_branch`, which refuses the + # tracking branch, so no branch is named. A dry run asks it before the + # branch is read, so it names the branch first: the one the `remove` + # line's own refusal would name, and `BRANCH_LEFT` (#1228). + branch, left = "", "" + if dry_run: + branch = _off_the_tracking_branch( + family, "`remove` refuses", + _suggested_branch("remove", + str(held.get("id") or held["project"]))) + left = " " + BRANCH_LEFT raise Refusal( "member-duplicate-id", f"{repository} at {commit[:12]} is already a member of " @@ -8823,11 +8885,12 @@ def _checked_bumped_id(family: Family, row: dict, commit: str, f"of {repository} whose project.yaml records an id no other row " f"records. If {repository} and {held.get('repository')} are one " "project, " + + branch + _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.") + "project, not in the family." + left) def _head_oid(root: Path, path: str) -> str | None: @@ -9300,10 +9363,12 @@ def _bump_dry_run(family: Family, row: dict, path: str, commit: str, resolved as the checkout would take it (`^{commit}`, which is what `git checkout` accepts). The manifest is read by `_member_id_at`, its id is asked against the other rows by `_checked_bumped_id` as the real run - asks it (#541), and the digest is computed by `tree_digest`, which reads - objects and never a checkout, so the digest printed is the one the real - run computes, never a guess. The rows are `_bumped_rows`', and - `family.rewritten` asks `family-members-unrecognised` without writing. + asks it (#541), its remedy naming a branch first, which the real run + has read by then (#1228), and the digest is computed by `tree_digest`, + which reads objects and never a checkout, so the digest printed is the + one the real run computes, never a guess. The rows are `_bumped_rows`', + and `family.rewritten` asks `family-members-unrecognised` without + writing. `nothing to do` IS THE REAL RUN'S OWN RULE (#210), asked without the write: the commit would record nothing when HEAD's gitlink is already @@ -9331,7 +9396,7 @@ def _bump_dry_run(family: Family, row: dict, path: str, commit: str, if linked: project_id = str(row.get("id") or "") else: - _checked_bumped_id(family, row, commit, project_id) + _checked_bumped_id(family, row, commit, project_id, dry_run=True) digest = tree_digest(member, commit) new, rows = _bumped_rows(family, row, project_id, commit, digest) text = family.rewritten(rows) diff --git a/tests/test_family_bump_to_checks.py b/tests/test_family_bump_to_checks.py index ea27ca98..d9b513ae 100644 --- a/tests/test_family_bump_to_checks.py +++ b/tests/test_family_bump_to_checks.py @@ -62,6 +62,8 @@ #: The shell's reading of a printed line, and of a refusal's `switch`. from test_family_printed_quoting import family_lead, host_words from test_family_tracking_branch import remedy_switch +#: The sentence every remedy that names a `switch -c` ends with (#607). +from test_validate_family_branch_step import BRANCH_LEFT PATH = f"members/{MEMBER}" @@ -80,12 +82,25 @@ def records_the_neighbours_id(manifest_path: Path) -> None: OWN_ID_LINE, f'\nid: "{OTHER_ID}"\n'.encode("ascii"))) -def duplicate_id_refusal(to: str, holder: Path) -> tuple[str, str]: +def duplicate_id_refusal(to: str, holder: Path, + dry_run: bool = False) -> 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), after the interpreter and the path to `family.py` - from where the run stood (#1127).""" + from where the run stood (#1127). + + `dry_run` FOR `bump --dry-run`'s (#1228), which reads no branch before + it asks the id, where the real run has passed `_checked_branch`: the + `remove` comes after the branch it needs, the one `remove`'s own + refusal names, and the remedy ends with `BRANCH_LEFT`.""" + root = shell_quoted(holder.resolve()) + branch, left = "", "" + if dry_run: + branch = ("on a branch other than the holder's tracking branch, " + f"which `remove` refuses (`git -C {root} switch -c " + f"family/remove-{OTHER_ID}` makes one), ") + left = f" {BRANCH_LEFT}" return ( f"REFUSED member-duplicate-id: {ORG}/{MEMBER} at {to[:12]} is " f"already a member of {NAME} under another name: its project.yaml " @@ -95,11 +110,30 @@ 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_lead()}remove --family-root " - f"{shell_quoted(holder.resolve())} " + f"{branch}`{family_lead()}remove --family-root {root} " 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.") + "`project.yaml` files, mended in that project, not in the family." + f"{left}") + + +def both_refused(holder: Path, env: dict, to: str, + stdout: str = "") -> tuple[str, str]: + """`both_runs`, for a refusal whose words the two runs say apart: + `bump --to ` with `--dry-run` and then for real, over the same + holder, both exit 2 with `stdout` and nothing else on stdout and + nothing in the holder moved. Returns `(the dry run's stderr, the real + run's)`.""" + before = state(holder, env) + + dry = bump(holder, env, to, "--dry-run") + real = bump(holder, env, to) + + assert_as_it_was(holder, env, before, BUMP_ORDER) + assert dry.returncode == real.returncode == 2, ( + dry.stdout + dry.stderr + real.stdout + real.stderr) + assert dry.stdout == real.stdout == stdout + return dry.stderr, real.stderr # --- the duplicate id -------------------------------------------------------- @@ -109,15 +143,17 @@ def test_bump_to_a_commit_recording_another_rows_id_is_refused_alike( """THE ISSUE'S FIRST LINE. The member's `project.yaml` at `--to` records the neighbour's id. `bump` committed it, exit 0, and the holder's own gate refused `member-duplicate-id`. Now both runs refuse it under that - code, in the same two lines, before the checkout, and the holder is as - it was and still passes its gate.""" + code, in the same detail, before the checkout, and the holder is as it + was and still passes its gate. The dry run's remedy names the branch + its `remove` needs first, which the real run has read (#1228).""" env = isolated(offline, tmp_path) to = commit_in_mount(holder, env, lambda mount: records_the_neighbours_id( mount / "project.yaml")) - stderr = both_runs(holder, env, to) + dry, real = both_refused(holder, env, to) - assert refusal(stderr) == duplicate_id_refusal(to, holder) + assert refusal(real) == duplicate_id_refusal(to, holder) + assert refusal(dry) == duplicate_id_refusal(to, holder, dry_run=True) assert validate(holder).returncode == 0 offline.assert_left_nothing() @@ -163,15 +199,17 @@ def test_a_duplicate_id_behind_a_tag_is_refused_after_the_peel_line( commit whose `project.yaml` records the neighbour's id. Both runs peel it and say so, in the line that says what WOULD be pinned, then refuse `member-duplicate-id` for the commit the tag names, exit 2, and pin - nothing: the holder is as it was and still passes its gate.""" + nothing: the holder is as it was and still passes its gate. The dry + run's remedy names the branch its `remove` needs first (#1228).""" env = isolated(offline, tmp_path) made = commit_in_mount(holder, env, lambda mount: records_the_neighbours_id(mount / "project.yaml")) to = annotated(holder / PATH, env, "v-duplicate", made) - stderr = both_runs(holder, env, to, peeled_line(to, made) + "\n") + dry, real = both_refused(holder, env, to, peeled_line(to, made) + "\n") - assert refusal(stderr) == duplicate_id_refusal(made, holder) + assert refusal(real) == duplicate_id_refusal(made, holder) + assert refusal(dry) == duplicate_id_refusal(made, holder, dry_run=True) assert validate(holder).returncode == 0 offline.assert_left_nothing() diff --git a/tests/test_family_dry_run.py b/tests/test_family_dry_run.py index 8d2662b6..f06cf9c5 100644 --- a/tests/test_family_dry_run.py +++ b/tests/test_family_dry_run.py @@ -51,6 +51,7 @@ from conftest import (appended_to_command_scope, cloned_just_pushed, git, run_script) +from repo_shape import shell_quoted #: The trailer lines a lane's run lands, from the file that owns them. from test_commit_trailers import TRAILERS, trailers_of #: The family fixture, its names and the offline environment, imported from @@ -71,6 +72,8 @@ GLOB, MEMBER, OTHER, hand_edit, older, raw, recorded, ) from test_family_member_path import isolated as member_path_env +#: The sentence every remedy that names a `switch -c` ends with (#607). +from test_validate_family_branch_step import BRANCH_LEFT #: GIT'S AUTOMATIC MAINTENANCE, OFF for the gits `isolated()` below hands an #: environment to: `family.py`'s children and the snapshot's gits. A `git @@ -1082,10 +1085,12 @@ def a_renamed_copy_of_a_member(root: Path) -> Path: #: Each refusal `add` makes before its first write that a fixture provokes #: cheaply, in `add`'s own order, as `(--member, more arguments, set-up, #: what stderr starts with)`: the holder's own refusals first, then the -#: member's, which need the probe's throwaway clone, and last the member's -#: `id:` against the rows (#330), which needs what that clone read. A -#: set-up that needs the member read from somewhere other than the family's -#: remotes returns that directory of remotes; the others return None. +#: member's, which need the probe's throwaway clone. A set-up that needs the +#: member read from somewhere other than the family's remotes returns that +#: directory of remotes; the others return None. The last refusal, the +#: member's `id:` against the rows (#330), which needs what that clone read, +#: is the one whose remedy the dry run says apart, and has a row of its own +#: after these (#1228). ADD_REFUSALS = { "malformed": (SPARE, (), None, "REFUSED member-malformed: "), "malformed-dot": (f"{ORG}/..", (), None, "REFUSED member-malformed: "), @@ -1129,8 +1134,6 @@ def a_renamed_copy_of_a_member(root: Path) -> Path: "REFUSED member-not-a-project: "), "unreadable": (f"{ORG}/IRNOPE", (), None, f"REFUSED reading {ORG}/IRNOPE. THE EXACT COMMAND WAS:"), - "duplicate-id": (f"{ORG}/{RENAMED}", (), a_renamed_copy_of_a_member, - "REFUSED member-duplicate-id: "), } @@ -1152,7 +1155,8 @@ def test_a_refusal_add_makes_before_it_mounts_is_the_same_under_dry_run( of the probe's throwaway clone). The real run and the dry run, over the same holder, print the same stdout and the same stderr, and leave the holder as it was, every hand edit a set-up made still in place, and no - temporary clone behind.""" + temporary clone behind. The one refusal whose remedy the two say apart + has the next row (#1228).""" env = isolated(offline, tmp_path) remotes = family["remotes"] if set_up is not None: @@ -1170,6 +1174,39 @@ def test_a_refusal_add_makes_before_it_mounts_is_the_same_under_dry_run( offline.assert_left_nothing() +def test_add_dry_runs_duplicate_id_names_the_branch_the_real_run_has_read( + family, holder, offline, tmp_path): + """THE RULING'S ONE DIFFERENCE (#1228). The member's `id:` against the + rows (#330) is asked last, of what the probe's clone read, and the dry + run refuses it as the real run does: the same code, exit 2, the same + detail and stdout, the holder as it was and no temporary clone behind. + A real `add` asks it after `_checked_branch`, so its remedy names no + branch; the dry run reads no branch, so its remedy says, before the two + lines, the branch they need, the one the `remove` line's own refusal + names, and ends with `BRANCH_LEFT`.""" + env = isolated(offline, tmp_path) + remotes = a_renamed_copy_of_a_member(holder) + before = add_state(holder, env) + + real = add(holder, env, remotes, member=f"{ORG}/{RENAMED}") + dry = add(holder, env, remotes, "--dry-run", member=f"{ORG}/{RENAMED}") + + assert dry.returncode == real.returncode == 2, dry.stdout + dry.stderr + assert dry.stdout == real.stdout + said, remedy = refusal(real.stderr) + assert said.startswith("REFUSED member-duplicate-id: "), said + forge = " renamed at the forge, " + assert remedy.count(forge) == 1, remedy + clause = ("on a branch other than the holder's tracking branch, which " + f"`bump` and `remove` refuse (`git -C " + f"{shell_quoted(holder.resolve())} switch -c " + f"family/remove-{PINNED.lower()}` makes one), ") + assert refusal(dry.stderr) == ( + said, f"{remedy.replace(forge, forge + clause)} {BRANCH_LEFT}") + assert_add_as_it_was(holder, env, before) + offline.assert_left_nothing() + + def test_add_dry_run_prints_the_trailers_the_real_commit_ends_with( family, holder, offline, tmp_path): """`--trailer` with `--dry-run` prints the lines, in order, after the row diff --git a/tests/test_family_remedy_lines.py b/tests/test_family_remedy_lines.py index b2f2ee20..002920ce 100644 --- a/tests/test_family_remedy_lines.py +++ b/tests/test_family_remedy_lines.py @@ -38,6 +38,25 @@ 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. + +Three more lines, from lane 5's review of #1201 (#1228), are measured the +same way. Each was reached with no branch read, and named none: + +* `add --dry-run`'s `member-duplicate-id`, for `IRRS2`, a renamed copy of + the pinned `IRRS` (`tests/test_family_add_member_id.py`'s), and +* `bump --dry-run`'s, for a `--to` whose `project.yaml` records the + neighbour's id (`tests/test_family_bump_to_checks.py`'s). A dry run asks + no branch, so each now names the one its lines need, the branch the + `remove` line's own refusal names, then `BRANCH_LEFT`; the real runs ask + them after `_checked_branch`, and their words are pinned unchanged in + those two files. +* `init`'s `family-holder-already-here`, whose `add` stands on the holder + `init` made, on its tracking branch. It now comes after `init`'s own + `switch -c family/add-`, and is `_cited` (N6). + +The branch each dry run's clause names is made from the row's `id:`, and +not from its name: a row renamed by hand, whose name is not its id +lowercased, tells the two apart (RV1264's NIT-2 on #1264). """ from __future__ import annotations @@ -47,7 +66,7 @@ import pytest -from conftest import git, run_script +from conftest import git, refusal_detail, 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 @@ -55,8 +74,12 @@ from test_family import ( # noqa: F401 FAMILY, MEMBERS, NAME, ORG, SPARE, family, manifest, validate, ) +from test_family_add_member_id import DUPLICATE_ID, RENAMED, renamed_copy from test_family_add_refusals import offline, refusal # noqa: F401 -from test_family_dry_run import isolated +from test_family_bump_to_checks import ( + duplicate_id_refusal, records_the_neighbours_id, +) +from test_family_dry_run import commit_in_mount, 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). @@ -66,8 +89,9 @@ #: before `--to` is. COMMIT = "0123456789abcdef0123456789abcdef01234567" -#: The pinned member `add` is refused for, by the row's name. -PINNED = MEMBERS[0] +#: The pinned member `add` is refused for, by the row's name, and its +#: neighbour, whose id `bump --dry-run`'s `--to` records (#1228). +PINNED, NEIGHBOUR = MEMBERS #: What the remedies say before the line of `family.py` they name, with the #: verbs the tracking branch is refused by. @@ -136,6 +160,58 @@ def present_remediation(holder: Path, lead: str, ticked: bool = True) -> str: f"twice is not a way to move it. {BRANCH_LEFT}") +def add_dry_duplicate_remediation(holder: Path, lead: str) -> str: + """What `add --dry-run` of `IRRS2`, whose `project.yaml` records the id + the row `IRRS` pins, is told (#1228): the branch first, the one + `remove`'s own refusal names, then the `remove` and the `bump` the real + run names, then `BRANCH_LEFT`.""" + name = shell_quoted(PINNED) + remove = family_line(lead, "remove", holder, name) + bump = family_line(lead, "bump", holder, name) + " --to " + switch = switch_line(holder, f"family/remove-{PINNED.lower()}") + clause = OFF_TRACKING.format(refuses="`bump` and `remove` refuse", + switch=f"`{switch}`") + return (f"Remediation: a family pins each project once. If " + f"{ORG}/{RENAMED} is {ORG}/{PINNED} renamed at the forge, " + f"{clause}`{remove}` takes the old row out and this `add` then " + f"pins it under the new name; `{bump}` 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 " + f"in the family. {BRANCH_LEFT}") + + +def bump_dry_duplicate_remediation(holder: Path, lead: str) -> str: + """What `bump --dry-run` of `IRRS` to a commit whose `project.yaml` + records the id of `IRSS` is told (#1228): the branch first, the one + `remove`'s own refusal names, then the `remove` the real run names, + then `BRANCH_LEFT`.""" + remove = family_line(lead, "remove", holder, shell_quoted(NEIGHBOUR)) + switch = switch_line(holder, f"family/remove-{NEIGHBOUR.lower()}") + clause = OFF_TRACKING.format(refuses="`remove` refuses", + switch=f"`{switch}`") + return (f"Remediation: a family pins each project once. Pass `--to` a " + f"commit of {ORG}/{PINNED} whose project.yaml records an id no " + f"other row records. If {ORG}/{PINNED} and {ORG}/{NEIGHBOUR} are " + f"one project, {clause}`{remove}` 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 " + f"family. {BRANCH_LEFT}") + + +def holder_here_remediation(holder: Path, lead: str, + ticked: bool = True) -> str: + """What `init` run again over the family folder it made is told + (#1228): `init`'s own branch, its `` bare as the NEXT STEPS + print it, then `init`'s own `add`, then `BRANCH_LEFT`.""" + switch = switch_line(holder, "family/add-") + add = family_line(lead, "add", holder, f"{shell_quoted(ORG)}/") + clause = OFF_TRACKING.format(refuses="`add` refuses", + switch=cited(switch, ticked)) + return (f"Remediation: {clause}{cited(add, ticked)} grows it, and `make " + "siblings` in the holder puts each member's working clone beside " + f"it. There is no --force. {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), @@ -151,7 +227,13 @@ def add_present(family, holder: Path, stand: Path, env: dict): 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.""" + interpreter it names, and the script by the path it names from there. + + THE INTERPRETER IS ASSERTED, NOT LOOKED UP (RV1201's NIT-2 on #1201): + the line's first word must be `PYTHON`, and the script then runs under + this test's own interpreter (`run_script`). So a row proves the words a + shell reads and the script they reach, not that the reader's PATH has + a `PYTHON`.""" words = host_words(line) assert words[0] == PYTHON, line return run_script(Path(words[1]), *words[2:], *extra, cwd=stand, env=env) @@ -315,21 +397,34 @@ def test_member_unknowns_add_names_the_owner_where_it_is_known( # --- 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") +def renamed_by_hand(holder: Path, project: str, name: str) -> None: + """Edit the `project:` of the row that pins `project` to `name` and + commit it, as a person editing `family.yaml` by hand would. The row + keeps its `id:`, which is `project` lowercased: every other row's id + is its name lowercased, so only a row like this tells the two apart.""" path = holder / "family.yaml" text = path.read_text(encoding="utf-8") - edited = re.sub(rf'(?m)^( - project: )"?{PINNED}"?$', r'\1"IR RS"', + edited = re.sub(rf'(?m)^( - project: )"?{project}"?$', rf'\1"{name}"', 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) + assert rows_of(holder)[name]["id"] == project.lower(), ( + "premise: the row keeps its id") + + +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. The branch the clause names + is made from the row's `id:`, which is still `irrs`, and not from its + name, which would make `family/bump-member` (RV1201's NIT-1 on #1201: + every other row's id is its name lowercased).""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + renamed_by_hand(holder, PINNED, "IR RS") refused = add_present(family, holder, stand, env) assert refused.returncode == 2, refused.stdout + refused.stderr assert refused.stderr.startswith("REFUSED member-already-present: ") @@ -340,6 +435,8 @@ def test_a_rows_name_with_a_space_is_one_word_in_both_lines( ["bump", "--family-root", str(holder), "--member", "IR RS", "--to", ""], ["remove", "--family-root", str(holder), "--member", "IR RS"]] + assert re.findall(r"`(git -C [^`]*)`", refused.stderr) == [ + switch_line(holder, f"family/bump-{PINNED.lower()}")] # --- N6: a holder under a folder named with a backtick ----------------------- @@ -371,3 +468,201 @@ def test_a_command_holding_a_backtick_is_printed_bare( assert done.returncode == 0, done.stdout + done.stderr assert SPARE in rows_of(holder) assert git("status", "--porcelain", cwd=holder).stdout == "" + + +# --- #1228: the lines reached with no branch read ---------------------------- + +def add_renamed(holder: Path, remotes: Path, stand: Path, env: dict, + *extra: str): + """`add` of `IRRS2`, the pinned `IRRS` renamed at the forge, typed in + `stand`, reading the member from `remotes`.""" + return run_script(FAMILY, "add", *extra, "--family-root", str(holder), + "--member", f"{ORG}/{RENAMED}", "--local-remote-dir", + str(remotes), cwd=stand, env=env) + + +@pytest.mark.parametrize("verb", ["remove", "bump"]) +def test_add_dry_runs_duplicate_id_lines_run_from_the_tracking_branch( + family, offline, stand, tmp_path, verb): + """(a) `add --dry-run` reads no branch, so on the tracking branch it is + refused `member-duplicate-id` for `IRRS2`. Its remediation, word for + word: the branch the `remove` line's own refusal names, then the + `remove` and the `bump` lines, then `BRANCH_LEFT`. Typed alone there, + each line is refused `family-on-tracking-branch`; typed after the + `switch -c`, `remove` takes the old row out and the refused `add`, run + for real, then pins `IRRS2`, or `bump` (its `` filled in) moves + the row, and the holder is clean and its gate green.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + remotes = renamed_copy(family, tmp_path) + lead = family_lead(stand) + refused = add_renamed(holder, remotes, stand, env, "--dry-run") + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refusal(refused.stderr) == ( + DUPLICATE_ID.format(who=RENAMED, after=""), + add_dry_duplicate_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/remove-{PINNED.lower()}"), stand) + done = typed(line, stand, env) + assert done.returncode == 0, done.stdout + done.stderr + assert branch_of(holder) == f"family/remove-{PINNED.lower()}" + if verb == "bump": + assert rows_of(holder)[PINNED]["pin"]["commit"] == older + else: + added = add_renamed(holder, remotes, stand, env) + assert added.returncode == 0, added.stdout + added.stderr + assert sorted(rows_of(holder)) == sorted([RENAMED, NEIGHBOUR]) + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert validate(holder).returncode == 0 + + +def test_bump_dry_runs_duplicate_id_remove_runs_from_the_tracking_branch( + family, offline, stand, tmp_path): + """(b) `bump --dry-run` of `IRRS` to a pushed commit whose + `project.yaml` records the id of `IRSS` reads no branch before the id, + so on the tracking branch it is refused `member-duplicate-id`. Its + remediation, word for word: the branch the `remove` line's own refusal + names, then the `remove` line, then `BRANCH_LEFT`. Typed alone there, + the `remove` is refused `family-on-tracking-branch`; typed after the + `switch -c`, it takes `IRSS` out, the refused `bump`, run for real, + then pins that commit, and the holder is clean and its gate green.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + to = commit_in_mount(holder, env, lambda mount: records_the_neighbours_id( + mount / "project.yaml")) + lead = family_lead(stand) + + def bump(*extra: str): + return run_script(FAMILY, "bump", *extra, "--family-root", + str(holder), "--member", PINNED, "--to", to, + cwd=stand, env=env) + + refused = bump("--dry-run") + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refusal(refused.stderr) == ( + duplicate_id_refusal(to, holder)[0], + bump_dry_duplicate_remediation(holder, lead)) + line = family_line(lead, "remove", holder, NEIGHBOUR) + + alone = typed(line, stand, env) + assert alone.returncode == 2, alone.stdout + alone.stderr + assert alone.stderr.startswith("REFUSED family-on-tracking-branch: ") + assert sorted(rows_of(holder)) == sorted(MEMBERS) + + switched(switch_line(holder, f"family/remove-{NEIGHBOUR.lower()}"), stand) + done = typed(line, stand, env) + assert done.returncode == 0, done.stdout + done.stderr + assert branch_of(holder) == f"family/remove-{NEIGHBOUR.lower()}" + assert sorted(rows_of(holder)) == [PINNED] + bumped = bump() + assert bumped.returncode == 0, bumped.stdout + bumped.stderr + assert rows_of(holder)[PINNED]["pin"]["commit"] == to + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert validate(holder).returncode == 0 + + +def test_add_dry_runs_clause_branch_is_made_from_the_rows_id( + family, offline, stand, tmp_path): + """(a) The branch the clause names is the one `remove`'s own refusal + names, made from the row's `id:` and not from its name (RV1264's NIT-2 + on #1264: every other row's id is its name lowercased). The row `IRRS`, + renamed by hand to `Orrs`, still pins id `irrs`, so `add --dry-run` of + `IRRS2` is refused `member-duplicate-id`, and the clause's `switch -c` + names `family/remove-irrs`, not `family/remove-orrs`.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + renamed_by_hand(holder, PINNED, "Orrs") + refused = add_renamed(holder, renamed_copy(family, tmp_path), stand, env, + "--dry-run") + assert refused.returncode == 2, refused.stdout + refused.stderr + said, remedy = refusal(refused.stderr) + assert said.startswith("REFUSED member-duplicate-id: "), said + assert re.findall(r"`(git -C [^`]*)`", remedy) == [ + switch_line(holder, f"family/remove-{PINNED.lower()}")] + + +def test_bump_dry_runs_clause_branch_is_made_from_the_other_rows_id( + family, offline, stand, tmp_path): + """(b) The same for `bump --dry-run` of `IRRS` to a commit whose + `project.yaml` records the id of `IRSS`, the row the `remove` takes out + (RV1264's NIT-2 on #1264). `IRSS`, renamed by hand to `Osss`, still + pins id `irss`: the clause's `switch -c` names `family/remove-irss`, + not `family/remove-osss`.""" + env = isolated(offline, tmp_path) + holder = on_tracking(family, tmp_path / "holders") + renamed_by_hand(holder, NEIGHBOUR, "Osss") + to = commit_in_mount(holder, env, lambda mount: records_the_neighbours_id( + mount / "project.yaml")) + refused = run_script(FAMILY, "bump", "--dry-run", "--family-root", + str(holder), "--member", PINNED, "--to", to, + cwd=stand, env=env) + assert refused.returncode == 2, refused.stdout + refused.stderr + said, remedy = refusal(refused.stderr) + assert said.startswith("REFUSED member-duplicate-id: "), said + assert re.findall(r"`(git -C [^`]*)`", remedy) == [ + switch_line(holder, f"family/remove-{NEIGHBOUR.lower()}")] + + +#: `init`'s arguments, but where it lands, for a family of its own. +INIT = ("init", "--org", ORG, "--family", "Orbit", "--created-by", + "Test Human", "--created-on", "2026-10-12") + + +@pytest.mark.parametrize("folder", ["plain", "a`b"]) +def test_init_holder_already_heres_add_runs_as_printed_after_its_switch( + family, offline, stand, tmp_path, folder): + """(c) `init` run again over the family folder it made is refused + `family-holder-already-here`. Its remediation, word for word: `init`'s + own branch, `family/add-` bare, then `init`'s own `add`, then + `BRANCH_LEFT`; under `a`b/`, where both commands hold the backtick, + both are printed bare and the prose is the same. The holder `init` made + stands on its tracking branch: there the `add`, `` filled in, + is refused `family-on-tracking-branch`; typed after the `switch -c`, + `` filled in alike, it puts IRQS in the family on that branch, + and the holder is clean and its gate green.""" + env = isolated(offline, tmp_path) + into = tmp_path / folder + into.mkdir() + lands = ("--local-remote-dir", str(tmp_path / "init remotes")) + made = run_script(FAMILY, *INIT, *lands, "--into", str(into), cwd=stand, + env=env) + assert made.returncode == 0, made.stdout + made.stderr + holder = into.resolve() / "Orbit" / "Orbit" + assert branch_of(holder) == TRACKING, "premise: where `init` leaves it" + lead = family_lead(stand) + + again = run_script(FAMILY, *INIT, *lands, "--into", str(holder.parent), + cwd=stand, env=env) + assert again.returncode == 2, again.stdout + again.stderr + # By its code: `init` warns first where the checkout it copies from is + # dirty, as a developer's may be. + lines = again.stderr.splitlines() + said = refusal_detail(again.stderr, "family-holder-already-here") + assert len(said) == 1, again.stderr + remedy = lines[lines.index(said[0]) + 1] + assert remedy == holder_here_remediation(holder, lead, + ticked="`" not in folder) + add = family_line(lead, "add", holder, f"{shell_quoted(ORG)}/{SPARE}") + members = ("--local-remote-dir", str(family["remotes"])) + + alone = typed(add, stand, env, *members) + assert alone.returncode == 2, alone.stdout + alone.stderr + assert alone.stderr.startswith("REFUSED family-on-tracking-branch: ") + + switched(switch_line(holder, f"family/add-{SPARE}"), stand) + done = typed(add, stand, env, *members) + assert done.returncode == 0, done.stdout + done.stderr + assert branch_of(holder) == f"family/add-{SPARE}" + assert sorted(rows_of(holder)) == [SPARE] + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert validate(holder).returncode == 0 diff --git a/tests/test_readme_windows_quoting_sentence.py b/tests/test_readme_windows_quoting_sentence.py index 23b18c61..2bea9ba8 100644 --- a/tests/test_readme_windows_quoting_sentence.py +++ b/tests/test_readme_windows_quoting_sentence.py @@ -71,7 +71,10 @@ "_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` + "_print_next_steps": 1, # `init`'s block: its `cd` + # `init`'s `switch -c family/add-`, in its block and in + # `family-holder-already-here`'s remedy (#1228) + "_add_branch_switch": 1, "_checked_mount_path": 1, # the `git rm -r --cached -- ` remedy }, } @@ -88,7 +91,8 @@ "_family_line": None, "_add_command": {"add"}, "_unknown_members_repository": {"add"}, - "_print_next_steps": {"switch -c", "cd"}, + "_print_next_steps": {"cd"}, + "_add_branch_switch": {"switch -c"}, "_checked_mount_path": {"git rm -r --cached"}, }