diff --git a/adopt-project.py b/adopt-project.py index d993354b..ffb4df0d 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -37,12 +37,13 @@ whole (#273). Where the source names a repository, `--org` and `--project` name that one, compared without case, or `plan` refuses them (`adopt-not-the-source`), and `check` finds and - `execute` refuses a plan edited to name another - (`plan-not-the-source`): in place, the source IS the assembly - root, and its own name is renamed first, on GitHub, never - retyped (#499). An `origin` that is a url names the - `org/repo` it ends in, with or without a trailing `/`; one that - is a path, in any separator, names none, and `plan` says it + `execute` refuses a plan edited to name another repository than + the source names, or, where it names none, than the plan + recorded for it (`plan-not-the-source`, #716): in place, the + source IS the assembly root, and its own name is renamed first, + on GitHub, never retyped (#499). An `origin` that is a url names + the `org/repo` it ends in, with or without a trailing `/`; one + that is a path, in any separator, names none, and `plan` says it compared nothing (#521). An organisation read from the url and refused is named by where it came from (#427). check validates a plan against the source: each entry path bound for @@ -10031,15 +10032,30 @@ def cmd_check(args) -> int: another = _not_the_source(plan, source, names) if another: findings.append(f"FINDING {PLAN_NOT_THE_SOURCE}: {another}") - remediation.append(_own_name_remediation(source, naming, pins, "")) + remediation.append(_own_name_remediation( + _held_repository(plan, source), naming, pins, "")) + # THE SOURCE IS HANDED TO `_check_names` WHERE `execute` HANDS IT (#716): + # when the plan does not name another repository, so that a refusal of + # the assembly root's own name says to rename it first, as `execute` + # says it on the same plan. Where the plan does name another, + # `execute` stops at `plan-not-the-source` and never asks the names, and + # a refusal of the plan's name is then about a name that is not the + # source's: `_own_name_remediation` has said what to do. + renamed = (_rename_first(source.repository) + if source.repository is not None and not another else None) try: - _check_names(naming, names, pins) + _check_names(naming, names, pins, None if another else source) for role, name in names.items(): found = naming.classify(name, role, pins) print(f" {role:<9} {name:<28} {found.family}" + (f"/{found.role}" if found.role else "")) except Refusal as exc: findings.append(f"FINDING {exc.code}: {exc.detail}") + # Only the rename-first remedy is printed, once after the findings: + # it is the one `check` was missing. `--project takes ...` is not, + # and `check` never printed it (#716). + if renamed is not None and exc.remediation == renamed: + remediation.append(renamed) _print_what_will_happen(plan, source, names, naming.topic_for(plan.project_id), @@ -13248,9 +13264,20 @@ def _check_names(policy: NamingPolicy, names: dict[str, str], # refuses before it asks the naming policy or any remote, a plan whose `org:` # names another with its `legs.assembly:`, which is what `execute` acts on, or # with its `project:`, as `plan-not-the-source`: a hand edit is caught too, -# whatever `org:` then holds, because it is read as `execute` reads it -# (`_plan_org`). A source that names no repository (no `origin`, or one that -# ends in no `org/repo`) is asked nothing, as before. +# because `org:` is read as `execute` reads it (`_plan_org`), as `str()` of +# what the file holds. One that `checked_value` refuses (a list, a mapping, +# an empty value, a space, a leading `-`, a character outside ASCII) is, in +# `execute`, `plan-not-the-source` where the plan is held to a repository +# (`_held_repository`, and the `org:` is not that repository's own) and +# `unsafe-value` where it is held to none. `check` asks `checked_value` of +# `org:` first (`_manifest_values`, and `_submodule_findings` where a kept +# submodule url is relative) and ends in `unsafe-value`, exit 2, before it +# compares anything; only where the plan's own names are refused, and no +# kept url is relative, does it reach the comparison with one (#716, +# L4-48). A source that names no repository (no `origin`, or one that ends +# in no `org/repo`) is asked nothing by `plan`, as before; `check` and +# `execute` hold the plan's names to what the plan recorded for it, +# `source: repository:`, where it recorded one (#716). # # AN `origin` THAT IS A PATH NAMES NO REPOSITORY (#521), in any separator # (`url_repository`). It was read as its last two directories, so a clone of @@ -13330,13 +13357,14 @@ def _path_origin_notes(args, source: Source) -> list[str]: "repository on GitHub."] -def _own_name_remediation(source: Source, naming: NamingPolicy, +def _own_name_remediation(repository: str, naming: NamingPolicy, pins: set[str], done: str) -> str: """What `adopt-not-the-source` and `plan-not-the-source` say to do: `plan` again under the source's own name, or, where the naming policy - refuses that name, `_rename_first`. `done` is what the refusing command - left as it was, said first.""" - repository = str(source.repository) + refuses that name, `_rename_first`. `repository` is the one the source + is held to (`_held_repository`), as text: where the live source names + none, the plan's record of it, and `str(None)` is no name (#716). `done` + is what the refusing command left as it was, said first.""" org, _, name = repository.partition("/") try: _check_names(naming, _names(name), pins) @@ -13373,24 +13401,116 @@ def _refuse_another_repository(args, source: Source, naming: NamingPolicy, "an in-place adoption keeps the source's own name, and `execute` " "pushes the split to the repository they name, opens its pull " "request there and sets its topic on it", - _own_name_remediation(source, naming, pins, "no plan was written. ")) + _own_name_remediation(repository, naming, pins, + "no plan was written. ")) + + +def _last_two_directories(path: str) -> str | None: + """`/` for a filesystem path, in either separator and + after a `:`, with one `.git` dropped from the name and the case kept; + None where the path has fewer than two (#716). + + A SUPERSET OF WHAT v1.0.0 AND v1.0.1 READ. Their `_remote_repository` + took `[:/]([^/:]+/[^/]+?)(?:\\.git)?$` of a path `origin` (until #521), + which is its last two segments, split at `/` or `:`, one `.git` dropped + from the second: `/srv/a:b/Atlas.git` was `b/Atlas`, and + `/srv/clones/Atlas.git.git` was `clones/Atlas.git`. It read none for a + trailing `/`, a backslash path, or a relative one that starts with no + separator; this reads each as its last two directories too, which only + holds a plan to nothing more often, and that is what main does.""" + segments = [part for part in re.split(r"[\\/:]", path) if part] + if segments and segments[-1].endswith(".git") \ + and len(segments[-1]) > len(".git"): + segments[-1] = segments[-1][:-len(".git")] + return "/".join(segments[-2:]) if len(segments) >= 2 else None + + +def _held_repository(plan: Plan, source: Source) -> str | None: + """The repository an in-place adoption by `plan` is held to, or None + where there is none (#716): the one the live source names + (`source.repository`), and where it names none, the one the plan + recorded for it, `source: repository:`. + + A SOURCE WITH NO `origin` ONCE HELD A PLAN TO NOTHING. `plan` records + `repository: null` for it and compares nothing, which is right; but a + plan for acme/Atlas whose names were edited to `Atlsa`, with `origin` + then removed from the source, or given a copy of it that has none by + `--source`, checked `plan ok`, and `execute` went on to ask GitHub about + `acme/Atlsa`. The record is read as `_github_name` reads a name, and + never raises: one that is no text, or names no `owner/repo`, holds the + plan to nothing, as a null does, and `Plan.open_source` is the reader + that refuses one it must clone (`_recorded_repository`). + + A RECORD THAT IS THE PATH'S OWN LAST TWO DIRECTORIES HOLDS NOTHING. v1.0.0 + and v1.0.1 read a path `origin` as those directories (#521), so `plan + --org acme --project Atlas` on a clone of `/srv/clones/Atlas.git` was + refused with "`--org clones`" and, where the plan was written all the + same, recorded `repository: clones/Atlas`. A person who put `org: acme` + back by hand holds a correct plan, and holding it to that record + refused it, with a remedy that is the advice #521 took out. Where the + source's `origin` is a path (`_origin_path`), a record that is its last + two directories, as `_last_two_directories` reads them without case, in + either separator, is the old reader's and not the source's name: the + plan is held to nothing. Any other record is held, so a `git clone` of + the source, whose `origin` is a path, still holds a plan edited to + `Atlsa` to `acme/Atlas`.""" + if source.repository is not None: + return source.repository + recorded = plan.source.get("repository") + held = _github_name(recorded) if isinstance(recorded, str) else None + path = _origin_path(source) if held is not None else None + if path is not None: + # Read as `held` was, so the two spellings are one name: v1.0.1 + # records `clones/Atlas.git` for `/srv/clones/Atlas.git.git`. + old = _last_two_directories(path) + old = None if old is None else _github_name(old) + if old is not None and old.casefold() == held.casefold(): + return None + return held + + +def _why_the_source_names_none(source: Source) -> str: + """Why the live source names no repository, as `_not_the_source` says it + beside the plan's record (#716): it has no `origin`, or its `origin` + names none (a path, a url that ends in no `org/repo`, or one that is not + UTF-8: `_remote_repository`).""" + try: + git_out(["remote", "get-url", "origin"], cwd=source.path, binary=True) + except Refusal: + return "it has no `origin`" + return "its `origin` names no repository" def _not_the_source(plan: Plan, source: Source, names: dict[str, str]) -> str | None: """`plan-not-the-source`'s detail, or None: the plan's `org:` with its `legs.assembly:` (`names`, read by `Plan.names`) or with its `project:` - names another repository than the one the source names (#499). - - `org:` is read as `execute` reads it (`_plan_org`), whatever the file - holds. While it was compared only when it was text, a number, a - boolean, a null or a deleted `org:` line checked `plan ok`, and - `execute` acted on `2026/Atlas`, `True/Atlas` or `None/Atlas` (#513's - review, SHOULD-1). Where it is not text, the detail says what the file - holds beside how `execute` spells it. `project:` is read only as text: - a misread one is not this question's to answer, and `execute` acts on - `legs.assembly:`.""" - repository = source.repository + names another repository than the one the source is held to + (`_held_repository`): the one it names (#499), or, where it names none, + the one the plan records for it (#716). + + `org:` is read as `execute` reads it (`_plan_org`), as whatever text + `str()` makes of what the file holds. While it was compared only when + it was text, a number, a boolean, a null or a deleted `org:` line + checked `plan ok`, and `execute` acted on `2026/Atlas`, `True/Atlas` or + `None/Atlas` (#513's review, SHOULD-1). Where it is not text, the detail + says what the file holds beside how `execute` spells it. `project:` is + read only as text: a misread one is not this question's to answer, and + `execute` acts on `legs.assembly:`. + + AN `org:` THAT `checked_value` REFUSES IS CAUGHT, BUT NOT ALWAYS AS + THIS (#716, L4-48): a list, a mapping, an empty value, one with a space, + a leading `-` or a character outside ASCII. `execute` asks this first, + so it refuses such an `org:` as `plan-not-the-source` where the plan is + held to a repository (`_held_repository`, and the `org:` is not that + repository's own), and as `unsafe-value`, from `_repository_urls`, + where it is held to none. `check` asks + `checked_value` of `org:` first (`_manifest_values`, and + `_submodule_findings` where a kept submodule url is relative) and ends + in `unsafe-value`, exit 2, before it compares anything; only where the + plan's own names are refused, and no kept url is relative, does it + reach this with one.""" + repository = _held_repository(plan, source) if repository is None: return None org, held = _plan_org(plan), plan.get("org") @@ -13407,8 +13527,14 @@ def _not_the_source(plan: Plan, source: Source, keys = " and ".join(f"`{key}: {_shown(value)}`" for key, value in wrong) named = _and_listed(sorted({f"{_shown(org)}/{_shown(value)}" for _, value in wrong})) + # Where the live source names none, the plan's own record is what it is + # held to, and the detail says so: the source did not say it (#716). + is_the = (f"the source is {_shown(repository)}" + if source.repository is not None else + f"the plan records the source as {_shown(repository)} " + f"({_why_the_source_names_none(source)})") return (f"the plan's {said} with its {keys} names {named}, " - f"and the source is {_shown(repository)}: an in-place adoption " + f"and {is_the}: an in-place adoption " "keeps the source's own name, and `execute` acts on the " "repository `org:` and `legs.assembly:` name") @@ -13426,7 +13552,7 @@ def _refuse_a_plan_for_another_repository( raise Refusal( PLAN_NOT_THE_SOURCE, f"{plan.path}: {detail}", _own_name_remediation( - source, naming, pins, + _held_repository(plan, source), naming, pins, "no leg repository was created and nothing was pushed. ")) diff --git a/tests/test_adopt_check_plan_name.py b/tests/test_adopt_check_plan_name.py new file mode 100644 index 00000000..4ec05aa6 --- /dev/null +++ b/tests/test_adopt_check_plan_name.py @@ -0,0 +1,494 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`check` says what `execute` says of a plan's name and of its source (#716). + +#499 made an in-place adoption keep the source's own name, and told +`check` and `execute` to say so alike. Two lines of that work were left, +both about a plan edited by hand, both refused by `execute` already, so that +nothing was lost, and `check` is the command that tells a person before any +leg exists: + +- L4-38: `check` asked `_check_names` about the plan's names without the + source, so a plan in the source's own name that the naming policy refuses + (`billing-service`) was `naming-unclassified` with no remedy, where + `execute` says that the source cannot be adopted in place under that name + and that renaming it on GitHub comes first. `check` now hands the source to + `_check_names` where `execute` does, and prints that remedy, once, after + its findings. +- L4-40: where the live source names no repository, the plan's own + `source: repository:` was asked nothing. A plan for acme/Atlas edited to + `Atlsa`, with `origin` then removed from the source, or run against a copy + that has none (`--source`), checked `plan ok`, and `execute` went on to ask + GitHub about `acme/Atlsa`. The plan's record holds it now, in both. +- L4-48: the comments on #499's `org:` read said that a hand edit is caught + "whatever `org:` then holds". An `org:` that `checked_value` refuses is + `unsafe-value` in `check`, which asks it first, and, in `execute`, + `plan-not-the-source` where the plan is held to a repository and + `unsafe-value` where it is held to none: caught in both, compared only + by one. The last tests here pin that, so the comments say what the + commands do. +- RV825's MUST-1: v1.0.0 and v1.0.1 read a path `origin` as its last two + directories (#521), so a plan they wrote can record `repository: + clones/Atlas` for a source whose `origin` is `/clones/Atlas.git`. + With the right `org:` put back by hand it is a correct plan, and holding + it to that record refused it and brought back the advice #521 took out + (`--org clones`). A record that is the path's own last two directories + holds nothing. + +Nothing here reaches GitHub or runs `gh`: each source's `origin` is an +`https://github.com/` URL that nothing fetches, and `execute` is run +in-process with every `git` and `gh` command it would run replaced by one +that fails the test (`refused_before_any_remote`). +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from conftest import ADOPT, git, run_script +from test_adopt_project_is_the_source import ( # noqa: F401 - a fixture + PLAN, PLAN_NOT_THE_SOURCE, RENAME_FIRST, RETYPE, adopter, check, edited, + findings, plan, refused_before_any_remote, source_of) +from test_adopt_relative_submodule_url import ( + HTTPS_ORIGIN, SIBLING, THING, planned as planned_with_submodule, + source_with as source_with_submodule) + +#: The organisation, the source's own name, and a typo of it. +ORG, OWN, TYPO = "acme", "Atlas", "Atlsa" +#: The remedy `plan-not-the-source` gives, up to where it names the source's +#: own name and no more. `{}` is what it says first, as `check` does not. +OWN_NAME = ("Remediation: {}Run `plan` again with `--project Atlas --org " + "acme`, the source's own name,") +#: Why `plan-not-the-source` refuses, after what the plan names and the +#: source is. +WHY = ("an in-place adoption keeps the source's own name, and `execute` acts " + "on the repository `org:` and `legs.assembly:` name") +#: Why the live source names no repository, as the finding says it: it has +#: no `origin` at all, or its `origin` names none (a path, here). +NO_ORIGIN = "it has no `origin`" +NAMES_NONE = "its `origin` names no repository" + + +def recorded(why: str) -> str: + """What `check` finds of the plan edited to `Atlsa` where the live + source names no repository, for `why`, and the plan's record of it is + the repository.""" + return (f"the plan's `org: {ORG}` with its `legs.assembly: {TYPO}` and " + f"`project: {TYPO}` names {ORG}/{TYPO}, and the plan records the " + f"source as {ORG}/{OWN} ({why}): {WHY}") + + +def renamed(plan_path: Path, name: str, was: str = "Northwind") -> None: + """The plan with `name` for its project and the three names that carry + it, as a hand edit makes them.""" + edited(plan_path, *[(f"{key}: {was}{suffix}", f"{key}: {name}{suffix}") + for key, suffix in (("project", ""), + (" assembly", ""), + (" spec", "-spec"), + (" code", "-code"))]) + + +def planned_for(base: Path, project: str, org: str = ORG) -> Path: + """A source with no `origin`, and the plan `plan` writes for it as + `project`, which the plan then records no repository for.""" + source = source_of(base, None) + written = plan(base, source, "--project", project, "--org", org) + assert written.returncode == 0, written.stdout + written.stderr + return source + + +def planned_from_origin(base: Path, repository: str = f"{ORG}/{OWN}") -> Path: + """A source whose `origin` is `repository` and the plan for it, which + records `repository` as the source's. Returns the source.""" + source = source_of(base, repository) + written = plan(base, source, "--project", repository.split("/")[1]) + assert written.returncode == 0, written.stdout + written.stderr + return source + + +# --- L4-38: `check` says what `execute` says of the source's own name ------- + +@pytest.mark.parametrize("repository, code", [ + ("acme/billing-service", "naming-unclassified"), + ("acme/Tools-Install", "naming-role-mismatch"), +], ids=["no-family", "not-a-project-leg"]) +def test_check_and_execute_say_the_same_remedy_of_the_sources_own_name( + adopter, tmp_path, monkeypatch, repository, code): + """A plan whose names are the source's own, in a name the policy + refuses, as `plan` wrote it before the source had an `origin` and a + hand edit made it: `execute` says the source cannot be adopted in place + under that name and that renaming it comes first. `check` said only + that the name matched no family; it says that too, once, and never to + retype `--project`.""" + org, _, name = repository.partition("/") + source = planned_for(tmp_path, "Northwind", org) + renamed(tmp_path / PLAN, name) + git("remote", "add", "origin", f"https://github.com/{repository}.git", + cwd=source) + + checked = check(tmp_path / PLAN) + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert [line.split(":")[0] for line in findings(checked.stderr)] == [ + f"FINDING {code}"], checked.stderr + assert refusal.code == code + assert refusal.remediation == f"Remediation: {repository} {RENAME_FIRST}" + # Said as `execute` says it, as a line of its own, after the findings. + assert checked.stderr.splitlines().count(refusal.remediation) == 1, \ + checked.stderr + assert RETYPE not in checked.stderr + + +def test_check_says_nothing_of_renaming_a_leg_whose_name_is_the_plans_own( + tmp_path): + """A leg's name is the plan's own to edit, and `_check_names` keeps the + remedy it gave for one, which `check` never printed: a leg the policy + refuses is a finding with no rename-first text, though the source is + named.""" + planned_from_origin(tmp_path) + edited(tmp_path / PLAN, (f" spec: {OWN}-spec", " spec: bad_name")) + + checked = check(tmp_path / PLAN) + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert findings(checked.stderr) == [ + "FINDING naming-unclassified: 'bad_name' matches no family in the " + "naming policy"], checked.stderr + assert "Remediation:" not in checked.stderr + assert "gh repo rename" not in checked.stderr + + +def test_check_does_not_tell_the_source_to_be_renamed_for_another_name( + adopter, tmp_path, monkeypatch): + """Where the plan names ANOTHER repository, `execute` stops at + `plan-not-the-source` and never asks the naming policy, and the name + the policy would refuse is not the source's. `check` finds both, and its + one remedy is `plan-not-the-source`'s; it does not tell a source named + `Atlas`, a name the policy takes, that it cannot be adopted under it.""" + planned_from_origin(tmp_path) + renamed(tmp_path / PLAN, "bad_name", OWN) + + checked = check(tmp_path / PLAN) + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert [line.split(":")[0] for line in findings(checked.stderr)] == [ + f"FINDING {PLAN_NOT_THE_SOURCE}", + "FINDING naming-unclassified"], checked.stderr + remedies = [line for line in checked.stderr.splitlines() + if line.startswith("Remediation:")] + assert len(remedies) == 1, checked.stderr + assert remedies[0].startswith(OWN_NAME.format("")), remedies + assert "cannot be adopted in place" not in checked.stderr + assert refusal.code == PLAN_NOT_THE_SOURCE + + +# --- L4-40: the plan's record holds it where the source names none ---------- + +def unreadable_origin(source: Path, origin: str | None = None) -> None: + """The source as `execute` meets it after `git remote remove origin`, or + with `origin` instead: no repository named.""" + if origin is None: + git("remote", "remove", "origin", cwd=source) + else: + git("remote", "set-url", "origin", origin, cwd=source) + + +@pytest.mark.parametrize("origin, why", [ + (None, NO_ORIGIN), + ("/srv/mirrors/Atlas.git", NAMES_NONE), + ("Atlas.git", NAMES_NONE), + ("https://example.invalid/Atlas", NAMES_NONE), +], ids=["no-origin", "path", "one-segment-path", "url-with-no-org"]) +def test_a_plan_edited_to_a_typo_is_found_where_the_origin_names_none( + adopter, tmp_path, monkeypatch, origin, why): + """The issue's own: a plan for acme/Atlas edited to `Atlsa`, then `git + remote remove origin`. On main `check` said `plan ok` and `execute` went + on to ask GitHub about `acme/Atlsa`. Both find `plan-not-the-source`, + naming what the plan records, and neither says to run `plan` with a + project that is not a name. A source with no `origin` says it has none + (RV825's NIT-3); one whose `origin` names no repository, a path or a url + that ends in no `org/repo`, says that. A record that is not the path's + own last two directories is held.""" + source = planned_from_origin(tmp_path) + unreadable_origin(source, origin) + assert check(tmp_path / PLAN).returncode == 0, "the plan, unedited" + renamed(tmp_path / PLAN, TYPO, OWN) + + checked = check(tmp_path / PLAN) + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert findings(checked.stderr) == [ + f"FINDING {PLAN_NOT_THE_SOURCE}: {recorded(why)}"], checked.stderr + assert checked.stderr.count(OWN_NAME.format("")) == 1, checked.stderr + assert refusal.code == PLAN_NOT_THE_SOURCE + assert refusal.detail == f"{tmp_path / PLAN}: {recorded(why)}" + assert refusal.remediation.startswith(OWN_NAME.format( + "no leg repository was created and nothing was pushed. ")), \ + refusal.remediation + + +@pytest.mark.parametrize("keep_origin, why", [ + (False, NO_ORIGIN), + (True, NAMES_NONE), +], ids=["no-origin", "origin-is-the-path-it-was-cloned-from"]) +def test_a_source_given_by_source_that_names_none_holds_the_plan_too( + adopter, tmp_path, monkeypatch, keep_origin, why): + """`--source `, which `check` and `execute` both take: a copy + with no `origin`, or one whose `origin` is the path it was cloned from + (which names no repository), names none, so the plan's record does. The + source the plan names has its `origin`, and is not the one read.""" + source = planned_from_origin(tmp_path) + copy = tmp_path / "copy" + git("clone", "-q", str(source), str(copy), cwd=tmp_path) + if not keep_origin: + unreadable_origin(copy) + renamed(tmp_path / PLAN, TYPO, OWN) + + checked = run_script(ADOPT, "check", "--plan", str(tmp_path / PLAN), + "--source", str(copy)) + real = adopter.Plan.open_source + # `refused_before_any_remote` passes no `--source`: every open of the + # source is handed the copy as one. + monkeypatch.setattr(adopter.Plan, "open_source", + lambda self, _, *rest, **keywords: real( + self, str(copy), *rest, **keywords)) + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert findings(checked.stderr) == [ + f"FINDING {PLAN_NOT_THE_SOURCE}: {recorded(why)}"], checked.stderr + assert (refusal.code, refusal.detail) == ( + PLAN_NOT_THE_SOURCE, f"{tmp_path / PLAN}: {recorded(why)}") + + +@pytest.mark.parametrize("held", ["acme/Atlas", "ACME/atlas", + "acme/Atlas.git"], + ids=["exact", "another-case", "dot-git"]) +def test_a_plan_that_names_the_recorded_repository_is_not_refused( + adopter, tmp_path, monkeypatch, held): + """Control: a record that the plan's names are, as GitHub resolves them + (without case, with or without `.git`), holds nothing against it. `check` + says `plan ok`, and `execute` goes on to the first command it runs + against GitHub, which the stand-in turns into the test's own failure.""" + source = planned_from_origin(tmp_path) + unreadable_origin(source) + edited(tmp_path / PLAN, (" repository: acme/Atlas", + f" repository: {held}")) + + checked = check(tmp_path / PLAN) + + assert checked.returncode == 0, checked.stdout + checked.stderr + assert "\nplan ok\n" in checked.stdout + with pytest.raises(AssertionError, match=r"execute ran \['git', " + r"'ls-remote', 'https://github" + r"\.com/acme/Atlas\.git'"): + refused_before_any_remote(adopter, monkeypatch, tmp_path / PLAN, + tmp_path / "work") + + +def test_a_plan_that_recorded_no_repository_holds_nothing( + adopter, tmp_path, monkeypatch): + """Control: a source with no `origin` when `plan` read it has no + repository to record (`repository: null`), so the plan's names are its + own, as ever: edited to another, `check` still says `plan ok`, and + `execute` goes on to ask about the name it was given.""" + planned_for(tmp_path, "Northwind") + renamed(tmp_path / PLAN, "Northwnd") + + checked = check(tmp_path / PLAN) + + assert checked.returncode == 0, checked.stdout + checked.stderr + assert "\nplan ok\n" in checked.stdout + with pytest.raises(AssertionError, match=r"execute ran \['git', " + r"'ls-remote', 'https://github" + r"\.com/acme/Northwnd\.git'"): + refused_before_any_remote(adopter, monkeypatch, tmp_path / PLAN, + tmp_path / "work") + + +@pytest.mark.parametrize("held", ["[acme, Atlas]", "Atlas", "2026", "~", + "acme/a/b"], + ids=["list", "no-owner", "number", "null", + "three-segments"]) +def test_a_record_that_names_no_repository_holds_nothing_and_raises_none( + tmp_path, held): + """A record that is no text, or that names no `owner/repo`, is read as + none, as `_github_name` reads a name: `check` says `plan ok` for the + plan edited to a typo, with no traceback, as it did.""" + source = planned_from_origin(tmp_path) + unreadable_origin(source) + edited(tmp_path / PLAN, (" repository: acme/Atlas", + f" repository: {held}")) + renamed(tmp_path / PLAN, TYPO, OWN) + + checked = check(tmp_path / PLAN) + + assert checked.returncode == 0, checked.stdout + checked.stderr + assert "Traceback" not in checked.stderr + assert "\nplan ok\n" in checked.stdout + + +def test_the_sources_own_origin_is_asked_before_the_plans_record(tmp_path): + """Control: where the source names a repository, that is the one the + plan is held to, as #499 made it; the plan's record of an older one is + never asked. A plan for the source as it is checks ok, though the record + names another.""" + planned_from_origin(tmp_path) + edited(tmp_path / PLAN, (" repository: acme/Atlas", + " repository: acme/Elsewhere")) + + checked = check(tmp_path / PLAN) + + assert checked.returncode == 0, checked.stdout + checked.stderr + assert "\nplan ok\n" in checked.stdout + + +# --- RV825's MUST-1: a plan v1.0.0 or v1.0.1 wrote for a path `origin` ------- + +@pytest.mark.parametrize("origin, record", [ + ("/srv/clones/Atlas.git", "clones/Atlas"), + ("/srv/clones/Atlas", "clones/Atlas"), + ("/srv/clones/Atlas.git/", "clones/Atlas"), + # The record in another case than the path, as a person may retype it, + # and as v1.0.1's own regex writes it, `Clones/ATLAS`. + ("/srv/Clones/ATLAS.git", "clones/Atlas"), + ("/srv/Clones/ATLAS.git", "Clones/ATLAS"), + ("C:\\srv\\clones\\Atlas.git", "clones/Atlas"), + ("\\\\server\\clones\\Atlas.git", "clones/Atlas"), + ("file:///srv/clones/Atlas.git", "clones/Atlas"), + ("clones/Atlas.git", "clones/Atlas"), + # Two that v1.0.1 really records an exotic name for: its regex takes a + # `:` for a separator and drops only the last `.git`. + ("/srv/clones/Atlas.git.git", "clones/Atlas.git"), + ("/srv/a:b/Atlas.git", "b/Atlas"), +], ids=["dot-git", "no-dot-git", "trailing-slash", "another-case", + "v1-0-1-case", "backslashes", "share", "file-url", "relative", + "two-dot-gits", "colon"]) +def test_a_plan_that_v1_0_x_wrote_for_a_path_origin_is_still_a_correct_plan( + adopter, tmp_path, monkeypatch, origin, record): + """v1.0.0 and v1.0.1 read a path `origin` as its last two directories + (#521): `plan --org acme --project Atlas` on a clone of + `/srv/clones/Atlas.git` was refused as `--org clones`, and the plan + written all the same recorded `repository: clones/Atlas`. With `org: + acme` put back by hand it is a correct plan, and `check` said `plan ok` + and `execute` went on to `acme/Atlas`. Held to that record it was + refused, with a remedy that brings `--org clones` back; the record is + the old reader's, not the source's name, so it holds nothing. The plan + here is the one this version writes, with its null edited to what + those versions recorded.""" + source = source_of(tmp_path, None) + git("remote", "add", "origin", origin, cwd=source) + written = plan(tmp_path, source, "--project", OWN, "--org", ORG) + assert written.returncode == 0, written.stdout + written.stderr + edited(tmp_path / PLAN, (" repository: null", + f" repository: {record}")) + + checked = check(tmp_path / PLAN) + + assert checked.returncode == 0, checked.stdout + checked.stderr + assert "\nplan ok\n" in checked.stdout + with pytest.raises(AssertionError, match=r"execute ran \['git', " + r"'ls-remote', 'https://github" + r"\.com/acme/Atlas\.git'"): + refused_before_any_remote(adopter, monkeypatch, tmp_path / PLAN, + tmp_path / "work") + + +# --- L4-48: what `check` does with an `org:` `checked_value` refuses -------- + +#: The value `execute` spells, and `check` asks `checked_value` about. +UNSAFE_ORGS = [("org: acme labs", "'acme labs'"), + ("org: [acme, labs]", "\"['acme', 'labs']\""), + ('org: ""', "''"), + ("org: -acme", "'-acme'"), + ("org: \u00e1cme", "'\u00e1cme'"), + ("org: {a: b}", "\"{'a': 'b'}\"")] + + +@pytest.mark.parametrize("org, spelled", UNSAFE_ORGS, + ids=["space", "list", "empty", "leading-dash", + "not-ascii", "mapping"]) +@pytest.mark.parametrize("held, code", [ + (True, PLAN_NOT_THE_SOURCE), + (False, "unsafe-value"), +], ids=["held-to-a-repository", "held-to-none"]) +def test_an_unsafe_org_is_unsafe_in_check_and_compared_by_execute_if_held( + adopter, tmp_path, monkeypatch, org, spelled, held, code): + """L4-48: the comments on #499's `org:` read said any `org:` is caught + "whatever the file holds". It is caught in both commands, and not by the + same one: `check` asks `checked_value` first and ends in `unsafe-value`, + exit 2, never comparing it with the repository. `execute` compares it + first, so it refuses it as `plan-not-the-source` where the plan is held + to a repository, and as `unsafe-value` where it is held to none (a source + that recorded no repository), as `check` does in both.""" + if held: + planned_from_origin(tmp_path) + else: + planned_for(tmp_path, OWN) + edited(tmp_path / PLAN, ("org: acme", org)) + + checked = check(tmp_path / PLAN) + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert checked.returncode == 2, checked.stdout + checked.stderr + unsafe = (f"REFUSED unsafe-value: org is {spelled}, which is not a value " + "this tool will put on a `git` or `gh` command line") + assert unsafe in checked.stderr.splitlines() + assert PLAN_NOT_THE_SOURCE not in checked.stderr + assert refusal.code == code + if not held: + assert unsafe[len("REFUSED "):] == f"{refusal.code}: {refusal.detail}" + + +def test_check_compares_an_unsafe_org_where_no_manifest_is_built( + tmp_path): + """Where `check` reaches the comparison with such an `org:`: where the + plan's own names are refused first, `_manifest_values` builds no + manifest and never asks `checked_value` (and no kept submodule url is + relative: the next test), so `check` finds the plan names another + repository, and the name, and exits 1.""" + planned_from_origin(tmp_path) + edited(tmp_path / PLAN, ("org: acme", "org: acme labs")) + renamed(tmp_path / PLAN, "bad_name", OWN) + + checked = check(tmp_path / PLAN) + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert [line.split(":")[0] for line in findings(checked.stderr)] == [ + f"FINDING {PLAN_NOT_THE_SOURCE}", + "FINDING naming-unclassified"], checked.stderr + assert "`org: acme labs`" in findings(checked.stderr)[0] + + +def test_check_asks_an_unsafe_org_first_where_a_kept_submodule_url_is_relative( + adopter, tmp_path, monkeypatch): + """RV825's SHOULD-1(b): `_submodule_findings` asks `checked_value` of + `org:` earlier than `_manifest_values` does, wherever a kept submodule's + url is relative. With the plan's names refused and `org: test org`, + `check` ends in `unsafe-value`, exit 2, and never compares it, as it + does with no relative url (the test above); `execute` says + `plan-not-the-source`.""" + source = source_with_submodule(tmp_path, SIBLING, HTTPS_ORIGIN) + made = planned_with_submodule(source, tmp_path) + edited(made, ("org: testorg", "org: test org")) + renamed(made, "bad_name", THING) + + checked = check(made) + refusal = refused_before_any_remote(adopter, monkeypatch, made, + tmp_path / "work") + + assert checked.returncode == 2, checked.stdout + checked.stderr + assert checked.stderr.splitlines()[0] == ( + "REFUSED unsafe-value: org is 'test org', which is not a value this " + "tool will put on a `git` or `gh` command line") + assert PLAN_NOT_THE_SOURCE not in checked.stderr + assert refusal.code == PLAN_NOT_THE_SOURCE