diff --git a/adopt-project.py b/adopt-project.py index b558bf61..ec5ca2b7 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -90,9 +90,17 @@ that clone is still on this machine, with no network: a partial clone (`git clone --filter`), whose `git` would fetch what it lacks, is not asked, and a NOTE names each pin it was not - rechecked for (#449). It prints what will happen and changes - nothing. - execute creates the two legs, extracts them with `git filter-repo`, makes + rechecked for (#449). Last, it asks what `execute` asks before + it makes either leg (#798): given `--local-remote-dir`, whether + each leg's bare repository is already there and whether that + directory can be made and written in, and without it, with `gh + repo view --json nameWithOwner`, whether GitHub already has + either leg's name, where a name GitHub only redirects (a + repository renamed or moved away) is free -- a NOTE and no + finding where gh cannot answer, so that `check` stays usable + offline. It prints what will happen and changes nothing. + execute asks about both legs before it makes either (#798), then + creates the two legs, extracts them with `git filter-repo`, makes the one split commit on a branch of the source, sets the `xf-project-` topic on all three and then opens the split's pull request against the tracking branch (both skipped for local @@ -118,6 +126,11 @@ whether it answers, and the place outside the assembly that a symlink kept at a path the shape writes leads to asked again of the disk, a climb from its own work directory. +And last, before it makes either leg, it refuses at once everything that +stands in their way (#798), as `check` finds it: a leg repository already +there, on the disk or on GitHub, a `--local-remote-dir` that cannot hold +them, and a `gh repo view` that fails otherwise than gh's not-found answer +or answers with no `nameWithOwner` to read. THE NEW PROJECT MUST BE ABLE TO HOLD WHAT `execute` WRITES INTO IT (#261). `execute` puts the plan's values into the root's `project.yaml`, and the @@ -177,6 +190,7 @@ import datetime as _dt import errno import itertools +import json import os import re import shlex @@ -269,6 +283,14 @@ from shape_materialize import ( # noqa: E402 free_plan_secrets_note, visibility_in, ) +# What `execute` asks of both leg repositories before it makes either, and +# `check` with it (#798): the folder `--local-remote-dir` names, asked as the +# scaffold and `family.py` ask theirs, and gh's not-found answer, the one +# failure of `gh repo view` that means a repository is absent (#609); the +# other is an answer naming another repository, a redirect. +from shape_materialize import ( # noqa: E402 + CANNOT_WRITE_IN, GH_NOT_FOUND_WORDS, folder_in_the_way, +) #: The naming policy this tool classifies leg names against. One constant, #: because three spellings of the same path is how the second one goes stale. @@ -10236,6 +10258,18 @@ def cmd_check(args) -> int: # and `check` never printed it (#716). if renamed is not None and exc.remediation == renamed: remediation.append(renamed) + else: + # LAST, as `execute` asks it last, right before it makes either leg + # (#798), and only of names `execute` would get that far with: what + # stands in the way of the two leg repositories, of the disk under + # `--local-remote-dir` and otherwise of GitHub, with `gh repo view + # --json nameWithOwner`. + # A question gh cannot answer is a NOTE here, not a finding. + legs, fixes, unasked = _leg_remote_findings(plan, names, + local_remote_dir) + findings.extend(legs) + remediation.extend(fixes) + notes = [*notes, *unasked] _print_what_will_happen(plan, source, names, naming.topic_for(plan.project_id), @@ -10482,9 +10516,9 @@ def _nothing_rolled_back(urls: dict, pushed: list, work_root: Path, source: Source | None = None) -> str: """The one notice for a failure once a leg repository exists. - `_create_leg_remotes` refuses a re-run while they exist: - `leg-remote-exists` under `--local-remote-dir`, and a `gh repo create` - that finds the name taken against GitHub. Every failure after it + `execute` refuses a re-run while they exist, before it makes either + (#798): `leg-remote-exists`, under `--local-remote-dir` and against + GitHub alike. Every failure after it therefore names the leg repositories this run `made` (both, unless the second could not be created, #188 N4), says which legs are already pushed (`pushed`, the roles in the order they were pushed), and gives the way @@ -10536,9 +10570,9 @@ def _nothing_rolled_back(urls: dict, pushed: list, work_root: Path, notice = (f"NOTHING has been rolled back: {named} {exist}, and {state}. " f"{rerun} the adoption with {them} deleted or with {fresh} (a " "new --local-remote-dir, or new leg repositories): a re-run " - f"that meets {them} is refused, as `leg-remote-exists` under " - "--local-remote-dir, and against GitHub by `gh repo create`, " - "which finds the name already taken.") + f"that meets {them} is refused as `leg-remote-exists` before " + "it makes either leg, under --local-remote-dir and against " + "GitHub alike.") built = (_cloned_here(source, work_root) + [name for name, _ in _built_in(work_root, names)]) if built: @@ -10817,25 +10851,305 @@ def _refuse_a_split_branch_left_behind(source: Source, urls: dict, + _branch_deletion(source, urls, branch)) +# --------------------------------------------------------------------------- +# Both leg repositories, asked about before either is made (#798) +# --------------------------------------------------------------------------- +# +# `_create_leg_remotes` made the spec leg's repository and asked about the +# code leg's only as it made that one: `leg-remote-exists` under +# `--local-remote-dir`, and `gh repo create` finding the name taken against +# GitHub. So a code leg that already existed was refused once the spec leg +# had been made, which #188 N4's notice named and nothing prevented; on +# GitHub that left a stray private `

-spec`, whose removal needs the +# `delete_repo` scope. And a `--local-remote-dir` that could not hold them +# ended in a traceback (a file at the path, `FileExistsError`; a folder above +# it this user cannot write in, `PermissionError`) or in `git init`'s raw +# failure (a folder this user cannot write in), each after `check` had said +# `plan ok`. +# +# BOTH ARE ASKED ABOUT FIRST, and everything that stands in their way is +# refused at once (`_refuse_what_stands_in_the_legs_way`): +# +# - under `--local-remote-dir`, whether each bare repository's path is taken +# (`os.path.lexists`, so a dangling link is taken too), the spec leg's and +# then the code leg's, and then whether the directory can be made and +# written in, asked as the scaffold and `family.py` ask theirs +# (`folder_in_the_way`, #609), which makes nothing; +# - against GitHub, `gh repo view --json nameWithOwner` of the spec +# leg's name and then the code leg's. A NAME IS TAKEN ONLY WHEN GITHUB +# ANSWERS WITH IT: gh's GraphQL follows a rename or a transfer, so the +# answer counts only when its `nameWithOwner` is the name asked, compared +# without case as GitHub compares names; an answer naming another +# repository is a redirect, which `gh repo create` takes over, and the +# name is free. A name that exists is `leg-remote-exists` before any `gh +# repo create`. Gh's not-found answer means absent, as #609 has it for the +# scaffold; a view that fails otherwise, or answers with no +# `nameWithOwner` to read, is `adopt-remote-unreadable`, naming the +# command and what it said. +# +# WHERE: right before `_create_leg_remotes`, after the yes, where +# `leg-remote-exists` was always refused. Every refusal before it keeps its +# order, and its precedence over this one: `adopt-branch-exists` and +# `plan-split-branch-taken` already defer to a leg still on disk. And GitHub +# is asked as close to `gh repo create` as it can be. +# +# `check` ASKS THE SAME, under the same flags and in the same order (#417's +# one verdict): given `--local-remote-dir`, of the disk, and without it, of +# GitHub; its findings come last, as `execute` asks them last. One +# difference: where gh cannot answer -- no login, a bad token, a rate limit, +# no network -- `check` says what it could not ask in a NOTE and still +# passes, so that it stays usable offline, where `execute` refuses. +# +# WHAT CAN STILL FAIL AFTER THE FIRST MAKE keeps #188 N4's notice +# (`_create_leg_remotes`): GitHub answers a repository the token cannot see +# as not found, and `gh repo create` then finds the name taken; a rate limit +# or an outage between the two creates; and under `--local-remote-dir` a +# path made since the question, or a disk that fills, at the code leg's +# `git init`. + +#: `check`'s finding, and `execute`'s refusal, of a leg repository that +#: already exists (#174, #798). +LEG_REMOTE_EXISTS = "leg-remote-exists" +#: ... of a `--local-remote-dir` that cannot hold them, under the name the +#: scaffold and `family.py` give theirs (#609). +LEG_FOLDER_REFUSED = "adopt-folder-cannot-be-made" +#: What each says to do: `check` after `Remediation:`, and `execute` after +#: `NO_LEG_YET`. +ON_DISK_FIX = ( + "Choose an empty --local-remote-dir, and a --work-dir the run that made " + "it did not clone into (a fresh one, or none): `git clone` refuses a " + "directory it already cloned into. There is no --force: re-running over " + "a live leg is not an adoption.") +ON_GITHUB_FIX = ( + "A leg repository is created new, never over one that exists, and there " + "is no --force: re-running over a live leg is not an adoption. If an " + "earlier `execute` made it and its adoption is not one to keep, delete " + "it (`gh repo delete `, which needs the `delete_repo` " + "scope); another repository of that name is renamed on GitHub first. " + "Then run `check` and `execute` again.") +UNREAD_FIX = ( + "Only gh's own answer that a repository does not exist (exit 1, " + f"`{GH_NOT_FOUND_WORDS}`), or one naming another repository that the " + "name redirects to, lets `execute` create one; any other answer says " + "nothing about whether it is there. Check `gh auth status` (`gh " + "auth login` where it names a token that is missing, bad or expired), " + "or let a rate limit or an outage pass, then run `execute` again.") +FOLDER_FIX = ( + "Move what stands in the way aside, or pass a --local-remote-dir where " + "a folder can be made. There is no --force.") +UNWRITABLE_FOLDER_FIX = ( + "Give this user write and search permission on that folder, or pass " + "another --local-remote-dir.") + + +def _leg_remote_answers(repositories: dict, names: dict, + local_remote_dir: Path | None) -> list[tuple]: + """`(code, detail, fix, note)` for each thing that stands in the way of + making the two leg repositories, in the order they are asked (#798): + nothing when both can be made. It makes nothing. + + Under `local_remote_dir` the disk is asked (`_in_the_way_on_disk`), and + otherwise GitHub, of `repositories` (`_in_the_way_on_github`). `note` is + what `check` prints instead of a finding, for a question gh could not + answer, and "" for every finding.""" + if local_remote_dir is not None: + return _in_the_way_on_disk(names, local_remote_dir) + asked = [_in_the_way_on_github(role, repositories[role]) + for role in EXTRACTED_LEGS] + return [answer for answer in asked if answer] + + +def _the_remote_dir(local_remote_dir: Path) -> Path: + """`--local-remote-dir` as each leg's url is made of it (`_remote_urls`) + and asked about: resolved, and made absolute as given where it cannot be + resolved. `Path.resolve` raised `RuntimeError` out of `execute` for a + link loop, a traceback before anything was asked (#798); + `folder_in_the_way` names it now, in `check` and `execute` alike.""" + try: + return local_remote_dir.resolve() + except (OSError, RuntimeError): + return local_remote_dir.absolute() + + +def _in_the_way_on_disk(names: dict, local_remote_dir: Path) -> list[tuple]: + """The disk's answers under `--local-remote-dir`: each leg's bare + repository path that something already takes, the spec leg's and then + the code leg's, and then the directory, where it cannot be made and + written in. Every path `_spelled`, as the notice spells it (#212).""" + remote_dir = _the_remote_dir(local_remote_dir) + found = [] + for role in EXTRACTED_LEGS: + bare = remote_dir / f"{names[role]}.git" + if os.path.lexists(bare): + found.append((LEG_REMOTE_EXISTS, + f"{_spelled(str(bare))}, the {role} leg's bare " + "repository, already exists", ON_DISK_FIX, "")) + in_the_way = folder_in_the_way(remote_dir) + if in_the_way: + here, why = in_the_way + found.append(( + LEG_FOLDER_REFUSED, + f"--local-remote-dir {_spelled(str(remote_dir))} cannot hold the " + f"two leg repositories: {_spelled(str(here))} {why}", + UNWRITABLE_FOLDER_FIX if why == CANNOT_WRITE_IN else FOLDER_FIX, + "")) + return found + + +def _first_said(output: str) -> str: + """`: `, each character a terminal would act on + spelled as `_spelled` spells it; "" when it said nothing.""" + lines = [line.strip() for line in output.splitlines() if line.strip()] + if not lines: + return "" + return ": " + "".join(char if char.isprintable() + else _spelled_character(char) for char in lines[0]) + + +def _name_with_owner(answer: str) -> str | None: + """The `nameWithOwner` a `gh repo view --json nameWithOwner` answer + names, or None for anything else: not JSON, not an object, no such + field, or one that is not a name.""" + try: + value = json.loads(answer) + except ValueError: + return None + value = value.get("nameWithOwner") if isinstance(value, dict) else None + return value if isinstance(value, str) and value else None + + +def _in_the_way_on_github(role: str, repository: str) -> tuple | None: + """GitHub's answer for one leg's `repository`, by `gh repo view --json + nameWithOwner`: `leg-remote-exists` when GitHub answers with that very + name, compared without case as GitHub compares names; None when gh + says it does not exist (#609), or answers with another repository's + name, which is a redirect left by a rename or a transfer, a name `gh + repo create` takes over; and `adopt-remote-unreadable` when gh failed + otherwise, could not be run or answered with no `nameWithOwner` to + read, which says nothing either way.""" + command = ["gh", "repo", "view", repository, "--json", "nameWithOwner"] + shown = " ".join(command) + try: + answer = run(command) + except CommandFailed as exc: + if exc.code == 1 and GH_NOT_FOUND_WORDS in exc.output: + return None + how = f"`{shown}` exited {exc.code}{_first_said(exc.output)}" + except OSError as exc: + how = f"`gh` could not be run: {exc}" + else: + found = _name_with_owner(answer) + if found is None: + how = (f"`{shown}` exited 0, and what it printed holds no " + f"`nameWithOwner`{_first_said(answer)}" if answer.strip() + else f"`{shown}` exited 0 and printed nothing") + elif found.lower() == repository.lower(): + return (LEG_REMOTE_EXISTS, + f"{repository}, the {role} leg's repository, already " + f"exists on GitHub: `{shown}` finds it", ON_GITHUB_FIX, + "") + else: + # GitHub followed a rename or a transfer away from this name: + # the name itself is free, and `gh repo create` takes it over. + return None + named = f"{repository}, the {role} leg's repository," + return (REMOTE_UNREADABLE, + f"whether {named} already exists on GitHub cannot be told: {how}", + UNREAD_FIX, + f"NOTE `check` could not ask GitHub whether {named} already " + f"exists: {how}. It is no finding here, so that `check` stays " + "usable offline; `execute` asks again before it makes either " + f"leg, and refuses as `{REMOTE_UNREADABLE}` while gh cannot " + "answer.") + + +def _refuse_what_stands_in_the_legs_way(repositories: dict, names: dict, + local_remote_dir: Path | None + ) -> None: + """Refuse every answer `_leg_remote_answers` gives at once, BEFORE + either leg repository is made (#798): the code is the first one's own, + the detail lists each as `check` prints it, and the remediation says + what each asks for, once.""" + answers = _leg_remote_answers(repositories, names, local_remote_dir) + if not answers: + return + raise Refusal( + answers[0][0], + "\n".join([f"{len(answers)} finding(s) about the two leg " + "repositories, asked before either is made:", + *(f" FINDING {code}: {detail}" + for code, detail, _fix, _note in answers)]), + f"Remediation: {NO_LEG_YET}" + " ".join( + dict.fromkeys(fix for _code, _detail, fix, _note in answers))) + + +def _leg_remote_findings(plan: Plan, names: dict, + local_remote_dir: Path | None + ) -> tuple[list[str], list[str], list[str]]: + """`check`'s `(findings, remediation, notes)` about the two leg + repositories: `execute`'s answers (#798), each finding as its refusal + lists it, and a NOTE for each question gh could not answer. GitHub is + asked only of an `org:` `execute` would put on a `gh` command line; + another is `execute`'s to refuse first, as `unsafe-value`.""" + repositories = {} + if local_remote_dir is None: + try: + org = checked_value("org", _plan_org(plan)) + except Refusal: + return [], [], [] + repositories = {role: f"{org}/{names[role]}" + for role in EXTRACTED_LEGS} + findings, fixes, notes = [], [], [] + for code, detail, fix, note in _leg_remote_answers( + repositories, names, local_remote_dir): + if note: + notes.append(note) + else: + findings.append(f"FINDING {code}: {detail}") + fixes.append(f"Remediation: {fix}") + return findings, list(dict.fromkeys(fixes)), notes + + +def _make_the_remote_dir(remote_dir: Path) -> None: + """`--local-remote-dir`'s `mkdir`, after the question said it can be + made (`folder_in_the_way`): what fails here anyway -- a file put there + since, a permission `os.access` did not foresee -- is refused by name, + before any leg exists, and is no `OSError` traceback (#798).""" + try: + remote_dir.mkdir(parents=True, exist_ok=True) + except OSError as exc: + raise Refusal( + LEG_FOLDER_REFUSED, + f"--local-remote-dir {_spelled(str(remote_dir))} cannot be made: " + f"{exc.strerror or exc}", + f"Remediation: {NO_LEG_YET}" + + (UNWRITABLE_FOLDER_FIX if isinstance(exc, PermissionError) + else FOLDER_FIX)) from exc + + def _create_leg_remotes(plan: Plan, names: dict, repositories: dict, urls: dict, tracking: str, local: bool, work_root: Path, *, source: Source | None = None) -> None: """(a) The two NEW repositories. The assembly root is never created here. - The spec leg's repository is created first, so a failure creating the - code leg's leaves one repository behind, and the re-run meets it: - against GitHub as `gh repo create` finding its name taken. That failure - therefore gets the notice too, naming the one that exists (#188 N4), and - is re-raised `reported`: `main` exits 2 on any of `LATE_FAILURES` without - printing it twice, and anything else (Ctrl-C) keeps its traceback. A - failure on the first leg leaves nothing, and `main` reports it as before. - `source` is handed on for the notice: nothing is built in the work - directory yet, but an `org/repo` source is already cloned there. + Both were asked about first (`_refuse_what_stands_in_the_legs_way`, + #798), so a name or a path already taken, and a `--local-remote-dir` + that cannot hold them, are refused before this runs. The spec leg's + repository is created first, so a failure creating the code leg's + that the question could not foresee -- a name GitHub hides from this + token, a path made since -- leaves one repository behind, and the re-run + meets it. That failure therefore gets the notice too, naming the one + that exists (#188 N4), and is re-raised `reported`: `main` exits 2 on + any of `LATE_FAILURES` without printing it twice, and anything else + (Ctrl-C) keeps its traceback. A failure on the first leg leaves nothing, + and `main` reports it as before. `source` is handed on for the notice: + nothing is built in the work directory yet, but an `org/repo` source is + already cloned there. """ print("\ncreating the leg repositories") if local: - Path(urls["spec"]).parent.mkdir(parents=True, exist_ok=True) + _make_the_remote_dir(Path(urls["spec"]).parent) made: list[str] = [] for role in EXTRACTED_LEGS: try: @@ -10938,9 +11252,10 @@ def _create_leg_remote(plan: Plan, names: dict, repositories: dict, print(f" gh {repositories[role]}") return bare = Path(urls[role]) + # Asked before either leg was made (#798); one there now was made since. if bare.exists(): raise Refusal( - "leg-remote-exists", f"{bare} already exists", + LEG_REMOTE_EXISTS, f"{bare} already exists", "Remediation: choose an empty --local-remote-dir, and a " "--work-dir the run that made it did not clone into (a fresh " "one, or none): `git clone` refuses a directory it already " @@ -11073,7 +11388,7 @@ def _push_the_leg(step: str, work: Path, url: str, tracking: str, "keeps there on the way to them. Then run `execute` again with fresh " "leg repositories (delete the two this run made, or choose an empty " "`--local-remote-dir`: a leg that already exists meets " - "`leg-remote-exists`, or a `gh repo create` that finds the name taken) " + "`leg-remote-exists`, on the disk or on GitHub) " "and an empty `--work-dir`, or none: a `--work-dir` this run was given " "still holds its clones of both legs, and the next run cannot clone " "into them.") @@ -11358,8 +11673,9 @@ def _remote_urls(org: str, names: dict, local_remote_dir: Path | None, for role, name in names.items()} # NOT created yet: nothing exists on disk until the human has said yes, # so a refused run leaves the directory it would have used absent rather - # than empty. - remote_dir = local_remote_dir.resolve() + # than empty. One that cannot be resolved (a link loop) is kept as given, + # for the question about it to refuse by name (#798). + remote_dir = _the_remote_dir(local_remote_dir) urls = {role: str(remote_dir / f"{name}.git") for role, name in names.items()} urls["assembly"] = str(source.path) @@ -12267,6 +12583,11 @@ def cmd_execute(args) -> int: plan.entries, source.tree(), source.commit) # ---- (a) the two legs' remotes ---------------------------------------- + # Both asked about first, and everything in their way refused at once, + # before either is made (#798): where `leg-remote-exists` always came, + # so every refusal above keeps its order. + _refuse_what_stands_in_the_legs_way(repositories, names, + args.local_remote_dir) _create_leg_remotes(plan, names, repositories, urls, tracking, local, work_root, source=source) @@ -14186,8 +14507,17 @@ def main(argv: list[str] | None = None) -> int: "submodule url the plan keeps in a leg is " "resolved from the bare repository `execute` " "would create there, as that run resolves it, " - "and not from GitHub's. Nothing is created " - "there.") + "and not from GitHub's, and `check` asks, as " + "`execute` asks before it makes either leg, " + "whether each leg's bare repository is already " + "there and whether this directory can be made " + "and written in. Nothing is created there. " + "Without it, `check` asks GitHub with `gh repo " + "view --json nameWithOwner` whether either " + "leg's name exists (a name GitHub only " + "redirects to a repository renamed or moved " + "away does not), and a `gh` that cannot answer " + "is a NOTE.") check.add_argument( "--work-dir", type=Path, default=None, help=_work_dir_help( @@ -14207,7 +14537,12 @@ def main(argv: list[str] | None = None) -> int: execute.add_argument("--source", default=None, help=SOURCE_OVERRIDE_HELP) execute.add_argument("--local-remote-dir", type=Path, default=None, help="create the legs as bare repositories here " - "instead of calling `gh` (the TEST path)") + "instead of calling `gh` (the TEST path). Both " + "are asked about before either is made: one " + "already there, or a directory that cannot be " + "made and written in, is refused, as `gh repo " + "view` finding either leg's name is without " + "it.") execute.add_argument("--allow-empty-leg", action="append", default=[], choices=EXTRACTED_LEGS, help="proceed even though NO path is assigned to this " diff --git a/docs/cli.md b/docs/cli.md index 663a0d8d..23b2522d 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -585,8 +585,15 @@ options: given (the TEST path): each relative submodule url the plan keeps in a leg is resolved from the bare repository `execute` would create there, as that run - resolves it, and not from GitHub's. Nothing is created - there. + resolves it, and not from GitHub's, and `check` asks, + as `execute` asks before it makes either leg, whether + each leg's bare repository is already there and + whether this directory can be made and written in. + Nothing is created there. Without it, `check` asks + GitHub with `gh repo view --json nameWithOwner` + whether either leg's name exists (a name GitHub only + redirects to a repository renamed or moved away does + not), and a `gh` that cannot answer is a NOTE. --work-dir WORK_DIR where `check` works: an `org/repo` --source, or the plan's `source.repository` where the clone `plan` made is gone or no longer whole, is cloned there. Give the @@ -626,7 +633,11 @@ options: inside a repository, as `source-not-top-level`. --local-remote-dir LOCAL_REMOTE_DIR create the legs as bare repositories here instead of - calling `gh` (the TEST path) + calling `gh` (the TEST path). Both are asked about + before either is made: one already there, or a + directory that cannot be made and written in, is + refused, as `gh repo view` finding either leg's name + is without it. --allow-empty-leg {spec,code} proceed even though NO path is assigned to this leg, seeding it from the shape's template. Unioned with the diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index c3af9884..5d75ea1b 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1539,9 +1539,10 @@ def _not_a_folder(path: Path) -> str | None: else "is not a folder") -def folder_cannot_be_made(path: Path) -> str | None: - """Why `path` cannot be made as a folder, with every folder above it, and - written in, or None when it can (#609). It makes nothing. +def folder_in_the_way(path: Path) -> tuple[Path, str] | None: + """`(where, why)` when `path` cannot be made as a folder, with every + folder above it, and written in: the one that stands in the way, and + why. None when it can (#609). It makes nothing. The question `Path.mkdir(parents=True, exist_ok=True)` and the first write into the folder ask, put without making anything: the nearest of @@ -1551,24 +1552,36 @@ def folder_cannot_be_made(path: Path) -> str | None: says yes for root, and for any folder on Windows, as `mkdir` would). A `path` nothing stands at is asked of the folder above it. - THE FOLDER IS NAMED AS `shown_source` NAMES A PATH (#692): as it is, and - as Python's `repr` spells it when a character in it is not printable. - Printed raw, an ESC in a folder's name reached the terminal and a line - break began a line of the person's choosing. A printable path is named - byte for byte, so `why.endswith(CANNOT_WRITE_IN)` still holds. + `folder_cannot_be_made` says it as one sentence, naming the folder as + `shown_source` names a path (#692). `adopt-project.py` asks it of + `--local-remote-dir` (#798), and names `where` its own way. """ for here in (path, *path.parents): if not os.path.lexists(here): continue why = _not_a_folder(here) if why: - return f"{shown_source(here)} {why}" + return here, why if not os.access(here, os.W_OK | os.X_OK): - return f"{shown_source(here)} {CANNOT_WRITE_IN}" + return here, CANNOT_WRITE_IN return None return None +def folder_cannot_be_made(path: Path) -> str | None: + """Why `path` cannot be made as a folder, with every folder above it, and + written in, or None when it can (#609): `folder_in_the_way`'s answer, as + ` `. It makes nothing. + + THE FOLDER IS NAMED AS `shown_source` NAMES A PATH (#692): as it is, and + as Python's `repr` spells it when a character in it is not printable. + Printed raw, an ESC in a folder's name reached the terminal and a line + break began a line of the person's choosing. A printable path is named + byte for byte, so `why.endswith(CANNOT_WRITE_IN)` still holds.""" + found = folder_in_the_way(path) + return f"{shown_source(found[0])} {found[1]}" if found else None + + def _unwritable_remediation(choose: str) -> str: """What a refusal of a folder this user cannot write in says to do, for a folder to make and for a working tree alike: give this user write and diff --git a/tests/gh_guard.py b/tests/gh_guard.py index 35c9f7a9..022c7c34 100644 --- a/tests/gh_guard.py +++ b/tests/gh_guard.py @@ -36,6 +36,20 @@ status`. Any other argv -- `gh version`, `gh auth status --hostname x` -- is not a probe and is refused. +ONE MORE CALL IS ANSWERED (#798): `gh repo view / --json +nameWithOwner`, which `adopt-project.py check` asks of each leg repository it +would create, as `execute` does before it makes either. Every `check` in this +suite that is given no `--local-remote-dir` asks it twice, and refused, each +such test would fail. It is answered as a `gh` with no login answers it, the +state the other two answers describe: exit 4 and gh's own words +(`NEEDS_A_LOGIN`), so `check` prints, on every machine, the NOTE that it +could not ask, and still passes. It is logged as a probe. Only that exact +shape is answered: `--json name`, `--json visibility` or any other field, +and any other `repo view`, are refused. SO IT ANSWERS ADOPT'S CALL ALONE: +`scaffold-project.py` and `family.py` ask whether their own repositories +exist with `--json name`, so a test of either that reaches gh with no stub +of its own is still failed at teardown (`tests/test_gh_guard.py`). + STANDARD LIBRARY ONLY, and importable: `tests/conftest.py` and `tests/test_gh_guard.py` read the same table, the same record format and the same decision from here, so the guard and the check cannot disagree about @@ -46,6 +60,7 @@ import json import os +import re import shlex import sys from pathlib import Path @@ -77,6 +92,36 @@ ("auth", "status"): (1, "", NOT_LOGGED_IN + "\n"), } +#: gh's own words, on stderr with exit 4, for a command that needs a login on +#: a machine that has none (gh 2.x, outside CI): how `ASKS_WHETHER_IT_EXISTS` +#: is answered (#798). +NEEDS_A_LOGIN = ( + "To get started with GitHub CLI, please run: gh auth login\n" + "Alternatively, populate the GH_TOKEN environment variable with a GitHub " + "API authentication token.") +NO_LOGIN_EXIT = 4 + +#: `repo view / --json nameWithOwner`: one owner and one name, as +#: a plan names them (an `org:` GitHub would refuse, `a+b`, included), with no +#: space or further `/`, and no option. +ASKS_WHETHER_IT_EXISTS = re.compile(r"[^\s/-][^\s/]*/[^\s/]+") + + +def answer(argv) -> tuple | None: + """`(exit status, stdout, stderr)` for a call the guard answers: one of + the two `PROBES`, or whether a repository exists, as a `gh` with no + login answers it (#798). None for anything else, which is refused.""" + argv = list(argv) + probe = PROBES.get(tuple(argv)) + if probe is not None: + return probe + if (len(argv) == 5 and argv[:2] == ["repo", "view"] + and argv[3:] == ["--json", "nameWithOwner"] + and ASKS_WHETHER_IT_EXISTS.fullmatch(argv[2])): + return NO_LOGIN_EXIT, "", NEEDS_A_LOGIN + "\n" + return None + + #: The refusal, ONE line on stderr. `{argv}` is the call as `shown` spells it. REFUSAL = ("gh guard (tests/conftest.py): no test may run the operator's gh, " "so `gh {argv}` was refused and logged; a test that means to run " @@ -88,8 +133,10 @@ def is_probe(argv) -> bool: - """Whether `argv` is one of the two `PROBES`, exactly.""" - return tuple(argv) in PROBES + """Whether `argv` is a call the guard answers (`answer`): one of the two + `PROBES`, exactly, or `repo view / --json nameWithOwner`. + """ + return answer(argv) is not None def shown(argv) -> str: @@ -133,9 +180,11 @@ def failure(lines) -> str | None: if not named: return None return ("tests/conftest.py's gh guard logged " - f"{len(named)} line(s) that are not the machine row's two probes " - "-- calls to gh made with no stub of the test's own ahead of the " - "guard on PATH, each refused:\n" + "\n".join(named) + "\n" + f"{len(named)} line(s) that are not a call it answers (the " + "machine row's two probes, and `repo view / --json " + "nameWithOwner`) -- calls to gh made with no stub of the test's " + "own ahead of the guard on PATH, each refused:\n" + + "\n".join(named) + "\n" "No test may reach the operator's gh. A test that means to run " "gh puts a stub of its own first on PATH, as " "tests/test_neutral_pin_validator.py does.") @@ -147,9 +196,9 @@ def main(argv) -> int: with open(log, "a", encoding="utf-8", newline="\n") as handle: handle.write(record(argv, os.environ.get("PYTEST_CURRENT_TEST", "")) + "\n") - answer = PROBES.get(tuple(argv)) - if answer is not None: - code, out, err = answer + answered = answer(argv) + if answered is not None: + code, out, err = answered sys.stdout.write(out) sys.stderr.write(err) return code diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index 7c767af6..18ea8eaf 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -35,9 +35,13 @@ import importlib.util import io +import json +import os import re +import shlex import shutil import sys +import textwrap from argparse import Namespace from dataclasses import dataclass from pathlib import Path @@ -45,10 +49,11 @@ import pytest from conftest import (ADOPT, git, make_source_repo, resolve, run_script, - write_plan) + spelled, write_plan) PROJECT = "Northwind" UTF8 = "utf-8" +CHECK = "check" UNCOVERED = "plan-uncovered" REMOTES = "remotes" WORK = "work" @@ -196,7 +201,7 @@ def open_plan(adopter, world: World, text: str): def run_check(plan: Path): """`check --plan `, run as a person would.""" - return run_script(ADOPT, "check", "--plan", str(plan)) + return run_script(ADOPT, CHECK, "--plan", str(plan)) def check_findings(plan: Path) -> list[str]: @@ -964,7 +969,7 @@ def test_a_wrong_source_is_named_when_source_says_where_to_read_it(world): and its `commit:` is what `check` holds the tree to. On 5a5011d this was `AttributeError` from `source_commit`.""" text = with_key(world.answered, "source", "source: 5\n") - result = run_script(ADOPT, "check", "--plan", + result = run_script(ADOPT, CHECK, "--plan", str(write_edited(world, text)), "--source", str(world.source)) assert result.returncode == 1, result.stderr + result.stdout @@ -1314,6 +1319,565 @@ def test_execute_raises_the_refusal_not_the_decode_error(adopter, world): no_leg_was_created(world.tmp) +# --- both leg repositories, asked about before either is made (#798) -------- +# +# `execute` made the spec leg's repository and asked about the code leg's only +# as it made that one. So a code leg that already existed, on disk or on +# GitHub, was refused once the spec leg had been made, and a +# `--local-remote-dir` that could not hold them ended in a traceback (a file +# at the path, a folder above it this user cannot write in) or in a raw +# `git init` failure (a folder this user cannot write in), each after `check` +# had said `plan ok`. Both are asked about now before either is made, every +# finding is refused at once, and `check` prints the same findings under the +# same flags. + +LEG_EXISTS = "leg-remote-exists" +FOLDER_REFUSED = "adopt-folder-cannot-be-made" +REMOTE_UNREADABLE = "adopt-remote-unreadable" +LEG_CODES = (LEG_EXISTS, FOLDER_REFUSED, REMOTE_UNREADABLE) +ORG = "testorg" +#: What `_create_leg_remotes` prints once it starts making them. +CREATING = "creating the leg repositories" +#: How `execute`'s refusal of them begins its remediation. +REMEDIATION_FIRST = f"Remediation: {NO_LEG_MADE}." +#: The two answers a folder can give that refuse it. +A_FILE = "is a file, not a folder" +CANNOT_WRITE_IN = "is a folder this user cannot write in" +#: gh's own words, with exit 4, for a command that needs a login, on a +#: machine with none. +NO_LOGIN = "To get started with GitHub CLI, please run: gh auth login" +#: What `check` prints for a repository gh could not answer about. +COULD_NOT_ASK = "NOTE `check` could not ask GitHub whether " + + +def bare(tmp_path: Path, role: str) -> Path: + """Where `execute --local-remote-dir /remotes` makes `role`'s + leg, as it names it: under the directory resolved.""" + return (tmp_path / REMOTES).resolve() / f"{PROJECT}-{role}.git" + + +def repository(role: str) -> str: + """`role`'s leg repository on GitHub, as `execute` names it.""" + return f"{ORG}/{PROJECT}-{role}" + + +def view(role: str) -> list[str]: + """The `gh` call that asks whether `role`'s leg repository exists.""" + return ["repo", "view", repository(role), "--json", "nameWithOwner"] + + +def leg_findings(text: str) -> list[str]: + """Each FINDING about the leg repositories in `text`, as `check` prints + it: the lines of its stderr, or of `execute`'s refusal, indented.""" + return [line.strip() for line in text.splitlines() + if line.strip().startswith(tuple(f"FINDING {code}:" + for code in LEG_CODES))] + + +def on_disk(tmp_path: Path, role: str) -> str: + return (f"FINDING {LEG_EXISTS}: {spelled(bare(tmp_path, role))}, the " + f"{role} leg's bare repository, already exists") + + +def on_github(role: str) -> str: + return (f"FINDING {LEG_EXISTS}: {repository(role)}, the {role} leg's " + "repository, already exists on GitHub: `gh repo view " + f"{repository(role)} --json nameWithOwner` finds it") + + +def told(role: str, what: str) -> str: + """How a `gh repo view` that failed otherwise than not-found, or + answered with no `nameWithOwner` to read, is named: the command, then + `what` it did.""" + return f"`gh repo view {repository(role)} --json nameWithOwner` {what}" + + +def exited(code: int, said: str) -> str: + """`told`'s `what` for a `gh` that exited `code`, saying `said` first.""" + return f"exited {code}: {said}" + + +def unread(role: str, what: str) -> str: + return (f"FINDING {REMOTE_UNREADABLE}: whether {repository(role)}, the " + f"{role} leg's repository, already exists on GitHub cannot be " + f"told: {told(role, what)}") + + +def as_named(path: Path) -> Path: + """`path` as `adopt-project.py` names a `--local-remote-dir`: resolved, + or made absolute where it cannot be (a link loop).""" + try: + return path.resolve() + except (OSError, RuntimeError): + return path.absolute() + + +def cannot_hold(local_remote_dir: Path, here: Path, why: str) -> str: + return (f"FINDING {FOLDER_REFUSED}: --local-remote-dir " + f"{spelled(as_named(local_remote_dir))} cannot hold the two leg " + f"repositories: {spelled(as_named(here))} {why}") + + +@pytest.fixture +def unwritable(): + """A maker of folders this user cannot write in: `make(path)` makes + `path` and sets it `0o555`, and each is made writable again for pytest + to delete. Skipped where a folder's mode refuses this user nothing: + root, and Windows.""" + made: list[Path] = [] + + def make(path: Path) -> Path: + if os.name == "nt" or os.geteuid() == 0: + pytest.skip("a folder's mode refuses nothing to this user here") + path.mkdir(parents=True) + path.chmod(0o555) + made.append(path) + return path + yield make + for path in made: + path.chmod(0o755) + + +def a_file(tmp_path: Path, _unwritable) -> tuple[Path, Path, str]: + """`(--local-remote-dir, what stands in the way, why)`: a file at it.""" + path = tmp_path / REMOTES + path.write_text("not a folder\n", encoding=UTF8) + return path, path, A_FILE + + +def under_an_unwritable_parent(tmp_path: Path, unwritable_folder): + parent = unwritable_folder(tmp_path / "read-only") + return parent / REMOTES, parent, CANNOT_WRITE_IN + + +def an_unwritable_directory(tmp_path: Path, unwritable_folder): + path = unwritable_folder(tmp_path / REMOTES) + return path, path, CANNOT_WRITE_IN + + +def under_a_link_loop(tmp_path: Path, _unwritable): + """A folder above it that is a link loop: `Path.resolve` raised + `RuntimeError` out of `execute` before anything was asked. The reason is + the system's own, read from the loop rather than spelled here.""" + loop, back = tmp_path / "loop", tmp_path / "back" + try: + loop.symlink_to(back) + back.symlink_to(loop) + except OSError: + pytest.skip("this machine cannot make a symlink") + with pytest.raises(OSError) as looped: + os.stat(loop) + return (loop / REMOTES, loop, + f"cannot be used as a folder: {looped.value.strerror}") + + +#: L1-72's three `--local-remote-dir`s, each refused by name, and a link +#: loop above one. +FOLDERS = {"a-file": a_file, + "under-an-unwritable-parent": under_an_unwritable_parent, + "an-unwritable-directory": an_unwritable_directory, + "under-a-link-loop": under_a_link_loop} +EACH_FOLDER = pytest.mark.parametrize("layout", sorted(FOLDERS)) +#: The legs that already exist, each refused by name. +EACH_EXISTING = pytest.mark.parametrize("existing", [(CODE,), (SPEC,), ROLES], + ids=["code", "spec", "both"]) + + +def made_bare(tmp_path: Path, existing: tuple) -> None: + for role in existing: + git("init", "-q", "--bare", str(bare(tmp_path, role)), cwd=tmp_path) + + +def execute_here(world: World, local_remote_dir: Path) -> list[str]: + """`main`'s argv for `execute --yes` of the world's plan into + `local_remote_dir`.""" + return ["execute", "--plan", str(world.plan), "--yes", + "--local-remote-dir", str(local_remote_dir), "--work-dir", + str(world.tmp / WORK)] + + +@EACH_EXISTING +def test_a_leg_on_disk_is_refused_before_either_is_made(adopter, world, + capsys, existing): + """L1-73, local: with `

-code.git` there, `execute` made + `

-spec.git` and was then refused at the code leg. Both are asked + about first, each one there is named, and none is made.""" + made_bare(world.tmp, existing) + refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) + assert refusal.code == LEG_EXISTS + assert leg_findings(refusal.detail) == [on_disk(world.tmp, role) + for role in existing] + assert refusal.remediation.startswith(REMEDIATION_FIRST) + for role in ROLES: + assert bare(world.tmp, role).exists() == (role in existing), role + assert CREATING not in capsys.readouterr().out + + +@EACH_FOLDER +def test_a_local_remote_dir_that_cannot_hold_the_legs_is_refused_by_name( + adopter, world, capsys, unwritable, layout): + """L1-72: a file at the path ended in a `FileExistsError` traceback, a + path under a folder this user cannot write in in a `PermissionError` + one, and a folder this user cannot write in in `git init`'s raw + failure. Each is refused by name now, exit 2, before anything is + made.""" + local_remote_dir, here, why = FOLDERS[layout](world.tmp, unwritable) + assert adopter.main(execute_here(world, local_remote_dir)) == 2 + err = capsys.readouterr().err + assert f"REFUSED {FOLDER_REFUSED}: " in err + assert leg_findings(err) == [cannot_hold(local_remote_dir, here, why)] + assert not list(world.tmp.rglob(f"{PROJECT}-*.git")) + + +def local_case(world: World, unwritable_folder, case: str) -> Path: + """The `--local-remote-dir` of `case`: one of `FOLDERS`, or legs on disk + under `/remotes`.""" + if case in FOLDERS: + return FOLDERS[case](world.tmp, unwritable_folder)[0] + made_bare(world.tmp, (CODE,) if case == "code-exists" else ROLES) + return world.tmp / REMOTES + + +@pytest.mark.parametrize("case", + ["code-exists", "both-exist", *sorted(FOLDERS)]) +def test_check_given_the_same_local_remote_dir_prints_what_execute_refuses( + adopter, world, unwritable, case): + """#417's one verdict: `check --local-remote-dir` said `plan ok` in + every case `execute` then refused. It prints each finding now, word for + word as `execute`'s refusal lists it, and fails.""" + local_remote_dir = local_case(world, unwritable, case) + checked = run_script(ADOPT, CHECK, "--plan", str(world.plan), + "--local-remote-dir", str(local_remote_dir)) + assert checked.returncode == 1, checked.stderr + checked.stdout + args = execute_args(world.tmp, world.plan) + args.local_remote_dir = local_remote_dir + refusal = refused_by(adopter, args) + printed = leg_findings(checked.stderr) + assert printed, checked.stderr + assert printed == leg_findings(refusal.detail) + + +#: A `gh` that logs each call, as JSON, to $GH_STUB_LOG and answers as GitHub +#: would for the repositories in $GH_STUB_EXISTING: `repo view` of one there +#: finds it, answering with its `nameWithOwner`, and of any other says gh's +#: not-found words, exit 1; `repo create` makes any other, and of one there, +#: or of one in $GH_STUB_HIDDEN (a repository the token cannot see), says the +#: name is taken. With $GH_STUB_VIEW_FAILS (` `) every `repo +#: view` fails so, and $GH_STUB_VIEW_ANSWERS (a JSON object) maps a +#: repository to what its `repo view` prints, exit 0. +GH_STUB = textwrap.dedent("""\ + import json, os, sys + argv = sys.argv[1:] + with open(os.environ["GH_STUB_LOG"], "a", encoding="utf-8") as log: + log.write(json.dumps(argv) + "\\n") + existing = os.environ.get("GH_STUB_EXISTING", "").split() + failing = os.environ.get("GH_STUB_VIEW_FAILS") + answers = json.loads(os.environ.get("GH_STUB_VIEW_ANSWERS", "{}")) + if argv[:2] == ["repo", "view"] and failing: + code, said = failing.split(" ", 1) + sys.stderr.write(said + "\\n") + sys.exit(int(code)) + if argv[:2] == ["repo", "view"] and argv[2] in answers: + sys.stdout.write(answers[argv[2]] + "\\n") + sys.exit(0) + if argv[:2] == ["repo", "view"] and argv[2] in existing: + print(json.dumps({"nameWithOwner": argv[2]})) + sys.exit(0) + if argv[:2] == ["repo", "view"]: + sys.stderr.write("GraphQL: Could not resolve to a Repository with " + f"the name '{argv[2]}'. (repository)\\n") + sys.exit(1) + hidden = os.environ.get("GH_STUB_HIDDEN", "").split() + if argv[:2] == ["repo", "create"] and argv[2] in existing + hidden: + sys.stderr.write("GraphQL: Name already exists on this account " + "(createRepository)\\n") + sys.exit(1) + if argv[:2] == ["repo", "create"]: + print(f"https://github.com/{argv[2]}") + sys.exit(0) + sys.stderr.write("gh stub: a call no test here expects\\n") + sys.exit(97) + """) + + +@dataclass(frozen=True) +class GitHub: + """What `on_github` set up: the log its `gh` keeps.""" + + log: Path + + def calls(self, words: int = 0) -> list[list[str]]: + """Each call the stub was run with, in order: its whole argv, or + its first `words`.""" + if not self.log.exists(): + return [] + return [json.loads(line)[:words or None] + for line in self.log.read_text(encoding=UTF8).splitlines()] + + +@pytest.fixture +def github(world, monkeypatch) -> GitHub: + """`execute` and `check` against GitHub, with no network: `GH_STUB` + first on PATH, ahead of the session's guard, with a token no `gh` takes + and an empty config directory, and every `https://github.com/` url git + reads rewritten to a directory that holds the assembly, a bare clone of + the source. A `#!` launcher, so not on Windows.""" + if os.name == "nt": + pytest.skip("the stub gh is a #! script") + tmp = world.tmp + stand_in = tmp / "github" + git("clone", "-q", "--bare", str(world.source), + str(stand_in / ORG / f"{PROJECT}.git"), cwd=tmp) + config = tmp / "gitconfig" + config.write_text(f'[url "{stand_in.as_posix()}/"]\n' + "\tinsteadOf = https://github.com/\n", encoding=UTF8) + bin_dir = tmp / "bin" + bin_dir.mkdir() + script = bin_dir / "gh_stub.py" + script.write_text(GH_STUB, encoding=UTF8) + launcher = bin_dir / "gh" + launcher.write_bytes(f"#!/bin/sh\nexec {shlex.quote(sys.executable)} " + f"{shlex.quote(str(script))} \"$@\"\n".encode(UTF8)) + launcher.chmod(0o755) + (tmp / "gh-config").mkdir() + for name, value in { + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "GH_STUB_LOG": str(tmp / "gh.log"), + "GH_TOKEN": "bogus-token-for-tests", + "GH_CONFIG_DIR": str(tmp / "gh-config"), + "GIT_CONFIG_GLOBAL": str(config)}.items(): + monkeypatch.setenv(name, value) + return GitHub(tmp / "gh.log") + + +def against_github(world: World) -> Namespace: + """`execute --yes` of the world's plan with no `--local-remote-dir`.""" + args = execute_args(world.tmp, world.plan) + args.local_remote_dir = None + return args + + +@pytest.mark.parametrize("existing", [(CODE,), ROLES], ids=["code", "both"]) +def test_a_leg_name_github_has_is_refused_before_any_repo_create( + adopter, world, github, monkeypatch, existing): + """L1-73, GitHub: the only `gh` calls were `repo create

-spec`, then + `repo create

-code`, which found the name taken, and a private + `

-spec` stayed behind. Both names are asked with `gh repo view` + first, and a name that exists is refused with no `repo create`.""" + monkeypatch.setenv("GH_STUB_EXISTING", + " ".join(repository(role) for role in existing)) + refusal = refused_by(adopter, against_github(world)) + assert refusal.code == LEG_EXISTS + assert leg_findings(refusal.detail) == [on_github(role) + for role in existing] + assert refusal.remediation.startswith(REMEDIATION_FIRST) + assert github.calls() == [view(SPEC), view(CODE)] + + +#: gh's failures that are not its answer that a repository does not exist, +#: `(exit, first line)`: neither says whether it does. +OTHER_FAILURES = { + "no-login": (4, NO_LOGIN), + "bad-credentials": (1, "HTTP 401: Bad credentials " + "(https://api.github.com/graphql)"), +} + + +@pytest.mark.parametrize("failure", sorted(OTHER_FAILURES)) +def test_a_view_that_fails_otherwise_is_refused_by_name( + adopter, world, github, monkeypatch, failure): + """Only gh's not-found answer means absent, as #622 has it for the + scaffold: any other failure of `gh repo view` is refused by name, the + command, its exit and gh's first line, before any `repo create`.""" + code, said = OTHER_FAILURES[failure] + monkeypatch.setenv("GH_STUB_VIEW_FAILS", f"{code} {said}") + refusal = refused_by(adopter, against_github(world)) + assert refusal.code == REMOTE_UNREADABLE + assert leg_findings(refusal.detail) == [unread(role, exited(code, said)) + for role in ROLES] + assert github.calls() == [view(SPEC), view(CODE)] + + +def test_a_gh_that_cannot_be_run_is_refused_by_name(adopter, world, github, + monkeypatch): + """A `gh` that cannot be started at all says nothing of either name + either: refused by name, not a `FileNotFoundError` traceback at the first + `repo create`.""" + real_run = adopter.run + + def no_gh(args, *rest, **named): + if args[0] == "gh": + raise FileNotFoundError(2, "No such file or directory", "gh") + return real_run(args, *rest, **named) + + monkeypatch.setattr(adopter, "run", no_gh) + refusal = refused_by(adopter, against_github(world)) + assert refusal.code == REMOTE_UNREADABLE + assert ("cannot be told: `gh` could not be run: [Errno 2] No such file or " + "directory: 'gh'") in refusal.detail + assert github.calls() == [] + + +def test_check_finds_a_leg_name_github_has_as_execute_refuses_it( + adopter, world, github, monkeypatch): + """`check`, given no `--local-remote-dir`, asks GitHub the same two + questions, and a name that exists is a FINDING, word for word as + `execute` refuses it.""" + monkeypatch.setenv("GH_STUB_EXISTING", repository(CODE)) + checked = run_check(world.plan) + assert checked.returncode == 1, checked.stderr + checked.stdout + assert leg_findings(checked.stderr) == [on_github(CODE)] + assert github.calls() == [view(SPEC), view(CODE)] + refusal = refused_by(adopter, against_github(world)) + assert leg_findings(refusal.detail) == leg_findings(checked.stderr) + + +@pytest.mark.parametrize("failure", sorted(OTHER_FAILURES)) +def test_check_notes_a_view_that_fails_otherwise_and_still_passes( + world, github, monkeypatch, failure): + """Where gh cannot answer -- no login, a bad token, no network -- + `check` names what it could not ask in a NOTE and still passes, so it + stays usable offline; `execute` refuses the same (above).""" + code, said = OTHER_FAILURES[failure] + monkeypatch.setenv("GH_STUB_VIEW_FAILS", f"{code} {said}") + checked = run_check(world.plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert leg_findings(checked.stderr) == [] + notes = [line for line in checked.stdout.splitlines() + if line.startswith(COULD_NOT_ASK)] + assert [role for role in ROLES + if any(told(role, exited(code, said)) in note + for note in notes)] == \ + list(ROLES), notes + assert len(notes) == len(ROLES), notes + assert github.calls() == [view(SPEC), view(CODE)] + + +def answering(monkeypatch, role: str, printed: str) -> None: + """`gh repo view` of `role`'s leg repository exits 0 and prints + `printed`; the other leg's is not found.""" + monkeypatch.setenv("GH_STUB_VIEW_ANSWERS", + json.dumps({repository(role): printed})) + + +def named_as(name_with_owner: str) -> str: + """GitHub's answer to `--json nameWithOwner`, naming `name_with_owner`.""" + return json.dumps({"nameWithOwner": name_with_owner}) + + +#: Names GitHub answers a view of the code leg's name with when it only +#: redirects that name: a repository renamed away from it, and one moved +#: to another owner. +REDIRECTS = { + "renamed": f"{ORG}/{PROJECT}-code-old", + "transferred": f"elsewhere/{PROJECT}-code", +} + + +class LegsMade(Exception): + """Raised as `_create_leg_remotes` returns, to stop `execute` there.""" + + +@pytest.mark.parametrize("redirect", sorted(REDIRECTS)) +def test_a_name_github_only_redirects_is_free(adopter, world, github, + monkeypatch, redirect): + """gh's GraphQL follows a rename or a transfer, so `gh repo view` of a + name a repository was renamed or moved away from exits 0 and answers + with the name it has now. That name is free, and `gh repo create` takes + it over: `check` finds nothing, and `execute` goes on to both `repo + create` calls. Exit 0 alone is not "taken".""" + answering(monkeypatch, CODE, named_as(REDIRECTS[redirect])) + checked = run_check(world.plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert leg_findings(checked.stderr) == [] + assert COULD_NOT_ASK not in checked.stdout, checked.stdout + assert github.calls() == [view(SPEC), view(CODE)] + github.log.unlink() + real = adopter._create_leg_remotes + + def then_stop(*args, **named): + real(*args, **named) + raise LegsMade + + monkeypatch.setattr(adopter, "_create_leg_remotes", then_stop) + monkeypatch.setattr(adopter, "_free_plan_hint_by_the_root", + lambda *_: None) + github_args = against_github(world) + with pytest.raises(LegsMade): + adopter.cmd_execute(github_args) + assert github.calls()[:2] == [view(SPEC), view(CODE)] + assert github.calls(3)[2:] == [["repo", "create", repository(SPEC)], + ["repo", "create", repository(CODE)]] + + +def test_a_name_github_answers_in_another_case_is_taken(adopter, world, + github, monkeypatch): + """GitHub compares names without case, so an answer naming the leg's + repository in other letters is that repository: taken, in `check` and + `execute` alike, with no `repo create`.""" + answering(monkeypatch, CODE, + named_as(f"{ORG.upper()}/{PROJECT.lower()}-CODE")) + checked = run_check(world.plan) + assert checked.returncode == 1, checked.stderr + checked.stdout + assert leg_findings(checked.stderr) == [on_github(CODE)] + refusal = refused_by(adopter, against_github(world)) + assert refusal.code == LEG_EXISTS + assert leg_findings(refusal.detail) == [on_github(CODE)] + assert github.calls() == [view(SPEC), view(CODE)] * 2 + + +#: Answers, exit 0, that hold no `nameWithOwner` to read, and how each is +#: named after the command: `(printed, what)`. +UNREADABLE_ANSWERS = { + "not-json": (f"{PROJECT}-code", "exited 0, and what it printed holds no " + f"`nameWithOwner`: {PROJECT}-code"), + "no-field": (json.dumps({"name": f"{PROJECT}-code"}), + "exited 0, and what it printed holds no `nameWithOwner`: " + + json.dumps({"name": f"{PROJECT}-code"})), + "nothing": ("", "exited 0 and printed nothing"), +} + + +@pytest.mark.parametrize("answer", sorted(UNREADABLE_ANSWERS)) +def test_an_answer_with_no_name_to_read_is_unreadable(adopter, world, github, + monkeypatch, answer): + """An answer that is not JSON, or holds no `nameWithOwner`, says + nothing about the name: `execute` refuses it as + `adopt-remote-unreadable`, naming the command and what it printed, and + `check` notes it and passes, as for any view gh could not answer.""" + printed, what = UNREADABLE_ANSWERS[answer] + answering(monkeypatch, CODE, printed) + checked = run_check(world.plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert leg_findings(checked.stderr) == [] + notes = [line for line in checked.stdout.splitlines() + if line.startswith(COULD_NOT_ASK)] + assert len(notes) == 1, notes + assert told(CODE, what) in notes[0], notes + refusal = refused_by(adopter, against_github(world)) + assert refusal.code == REMOTE_UNREADABLE + assert leg_findings(refusal.detail) == [unread(CODE, what)] + assert github.calls() == [view(SPEC), view(CODE)] * 2 + + +def test_a_name_github_hides_still_gets_the_notice_once_the_spec_leg_exists( + adopter, world, github, monkeypatch, capsys): + """THE NOTICE OF #188 N4 STAYS for what can still fail after the first + `repo create`: GitHub answers a repository the token cannot see as not + found, so both questions pass, and `repo create` then finds the code + leg's name taken once the spec leg's repository exists.""" + monkeypatch.setenv("GH_STUB_HIDDEN", repository(CODE)) + github_args = against_github(world) + with pytest.raises(adopter.CommandFailed): + adopter.cmd_execute(github_args) + assert github.calls()[:2] == [view(SPEC), view(CODE)] + assert github.calls(3)[2:] == [["repo", "create", repository(SPEC)], + ["repo", "create", repository(CODE)]] + err = capsys.readouterr().err + assert (f"{LATE_NOTICE}: `https://github.com/{repository(SPEC)}.git` " + "exists, and nothing is pushed to it.") in err + + # --- end to end ------------------------------------------------------------ @needs_filter_repo diff --git a/tests/test_adopt_filter_repo_failure.py b/tests/test_adopt_filter_repo_failure.py index 3aee0a04..b52c5759 100644 --- a/tests/test_adopt_filter_repo_failure.py +++ b/tests/test_adopt_filter_repo_failure.py @@ -169,10 +169,10 @@ def test_a_failed_leg_is_printed_with_its_command_and_its_output( def test_a_failed_leg_names_both_leg_repositories_and_what_to_do( adopter, monkeypatch, capsys, tmp_path, seeded, command): """Both leg repositories exist by now, and one may even be pushed. The - human reads the failure, fixes it and re-runs; the re-run is refused by - `_create_leg_remotes`, so the notice says so BEFORE it happens, for both - kinds of remote, and names the two repositories to delete or to - replace. It says to fix what failed first: a cause that is still there + human reads the failure, fixes it and re-runs; the re-run is refused as + `leg-remote-exists` before it makes either leg, on the disk and on + GitHub alike (#798), so the notice says so BEFORE it happens, and names + the two repositories to delete or to replace. It says to fix what failed first: a cause that is still there fails the re-run the same way, after two more leg repositories (#188's review, F2).""" refuse_every_leg(adopter, monkeypatch, command, tmp_path) @@ -191,8 +191,8 @@ def test_a_failed_leg_names_both_leg_repositories_and_what_to_do( assert spelled(arguments["urls"][role]) in notice assert "--local-remote-dir" in notice assert LIVE_LEG in notice - assert "gh repo create" in notice - assert "already taken" in notice + assert ("before it makes either leg, under --local-remote-dir and " + "against GitHub alike") in notice assert "Once what failed above is fixed, re-run the adoption" in notice # The work directory too, which holds what this run built: a re-run that # kept it looped (#184 P2-1), and `execute` now refuses it. diff --git a/tests/test_adopt_github_exits.py b/tests/test_adopt_github_exits.py index 2d34927d..e63b56eb 100644 --- a/tests/test_adopt_github_exits.py +++ b/tests/test_adopt_github_exits.py @@ -121,15 +121,18 @@ #: `gh` with no network. `repo create` makes a bare repository under #: $FAKE_GH_ROOT, which is what the rewritten `https://github.com/` URL then -#: names. Each command $FAKE_GH_REFUSES names (`|` between them; `pr create`, -#: or `repo edit ` for one repository alone) is refused the way a -#: token without the permission is, after writing the arguments it was given, -#: NUL-separated, to $FAKE_GH_ROOT/refused-arguments. `pr create` is refused -#: as GitHub refuses a base that is no branch of the repository (#479), and -#: records ` ` in $FAKE_GH_ROOT/opened when it opens one. -#: Every call records the token and the config directory it was run with in -#: $FAKE_GH_ROOT/environment, which are what a real `gh` would have been run -#: with, and its first three arguments, a line each, in $FAKE_GH_ROOT/calls. +#: names, and `repo view --json nameWithOwner`, which `execute` +#: asks of each leg before it makes either (#798), finds one there and gives +#: gh's not-found answer for any other. Each command $FAKE_GH_REFUSES names +#: (`|` between them; `pr create`, or `repo edit ` for one +#: repository alone) is refused the way a token without the permission is, after writing +#: the arguments it was given, NUL-separated, to +#: $FAKE_GH_ROOT/refused-arguments. `pr create` is refused as GitHub refuses a +#: base that is no branch of the repository (#479), and records ` +#: ` in $FAKE_GH_ROOT/opened when it opens one. Every call records the +#: token and the config directory it was run with in $FAKE_GH_ROOT/environment, +#: which are what a real `gh` would have been run with, and its first three +#: arguments, a line each, in $FAKE_GH_ROOT/calls. GH_SHIM = textwrap.dedent("""\ #!/bin/sh printf '%s\\n%s\\n' "$GH_TOKEN" "$GH_CONFIG_DIR" > "$FAKE_GH_ROOT/environment" @@ -147,6 +150,17 @@ done echo "HTTP 403: Resource not accessible by integration" >&2; exit 1 fi + case "$1 $2 $4 $5" in + "repo view --json nameWithOwner") + if [ -d "$FAKE_GH_ROOT/$3.git" ]; then + printf '{"nameWithOwner": "%s"}\\n' "$3" + else + said="GraphQL: Could not resolve to a Repository" + echo "$said with the name '$3'. (repository)" >&2 + exit 1 + fi + exit 0;; + esac case "$1 $2" in "repo create") git init -q --bare -b main "$FAKE_GH_ROOT/$3.git" && echo "https://github.com/$3";; diff --git a/tests/test_adopt_late_failure.py b/tests/test_adopt_late_failure.py index d8cce966..261f4c98 100644 --- a/tests/test_adopt_late_failure.py +++ b/tests/test_adopt_late_failure.py @@ -94,6 +94,9 @@ COPY_TREE = "copy_tree" TREE_OF = "_tree_of" PLAN_FLAG = "--plan" +LEG_REMOTE_EXISTS = "leg-remote-exists" +BARE_FLAG = "--bare" +EXECUTE = "execute" #: A source small enough that `plan` asks no question, so `execute` runs on #: the plan as written. @@ -375,7 +378,7 @@ def test_a_split_that_fails_names_its_step_and_both_pushed_legs( for role in LEGS: assert spelled(arguments["urls"][role]) in notice assert BOTH_PUSHED in notice - assert "leg-remote-exists" in notice + assert LEG_REMOTE_EXISTS in notice assert FIX_FIRST in notice # Read off the disk: the split's clone is there, and no leg's, because # the legs here are stand-ins that were never built in it. @@ -919,7 +922,7 @@ def unreadable(*_args): arguments = legs_arguments(tmp_path, source=source_of(adopter, tmp_path)) arguments["seeded"] = seeded for role in LEGS: - git("init", "-q", "--bare", "-b", "main", arguments["urls"][role], + git("init", "-q", BARE_FLAG, "-b", "main", arguments["urls"][role], cwd=tmp_path) assert adopter._build_the_legs(**arguments) is None @@ -1002,7 +1005,7 @@ def test_main_flushes_stdout_before_a_failure_no_step_reported( events, stderr = recorded_streams(monkeypatch) said, unsaid = scenario(adopter, monkeypatch, tmp_path) - assert adopter.main(["execute", PLAN_FLAG, "adoption-plan.yaml"]) == 2 + assert adopter.main([EXECUTE, PLAN_FLAG, "adoption-plan.yaml"]) == 2 err = "".join(stderr.written) assert flushed_first(events) @@ -1132,7 +1135,7 @@ def test_a_refused_leg_push_offers_an_adoption_its_one_exit( `main` as a ruleset does.""" monkeypatch.setattr(adopter, COPY_TREE, seed_one_file) remote = tmp_path / REMOTES / f"{ASSEMBLY}-{SPEC}.git" - git("init", "-q", "--bare", "-b", "main", str(remote), cwd=tmp_path) + git("init", "-q", BARE_FLAG, "-b", "main", str(remote), cwd=tmp_path) write_refusing_hook(remote / "hooks", MAIN_REF) with pytest.raises(adopter.CommandFailed): @@ -1261,13 +1264,13 @@ def test_a_live_leg_keeps_its_own_refusal_over_the_split_branch( with pytest.raises(adopter.Refusal) as caught: adopter.cmd_execute(execute_args(tmp_path, plan)) - assert caught.value.code == "leg-remote-exists" + assert caught.value.code == LEG_REMOTE_EXISTS def test_against_github_the_split_branch_is_asked_of_the_assembly( adopter, tmp_path): """Against GitHub nothing is on disk to look at, and the leg names are - taken or not by `gh repo create`; the branch is asked of the URL the + asked with `gh repo view` (#798); the branch is asked of the URL the split is pushed to, which a local path stands in for here.""" source = source_of(adopter, tmp_path) urls = {ROOT: str(source.path), @@ -1301,6 +1304,73 @@ def test_a_branch_the_split_push_fast_forwards_is_not_refused( BRANCH, True) is None +def after_the_question(adopter, monkeypatch, then) -> None: + """`_leg_remote_answers`, which asks about both leg repositories before + `execute` makes either (#798), answers as it does, and then `then()` + runs: what another run or a person does between that question and the + first make.""" + asked = adopter._leg_remote_answers + + def answered_then(*args): + answers = asked(*args) + then() + return answers + + monkeypatch.setattr(adopter, "_leg_remote_answers", answered_then) + + +def test_a_code_leg_made_after_the_question_still_gets_the_notice( + adopter, monkeypatch, capsys, tmp_path): + """#188 N4's notice STAYS for a failure that still comes after the + first make (#798). Both leg repositories are asked about first now, so + a `

-code.git` already there is refused before either is made; one + made after that question meets `_create_leg_remote`'s own + `leg-remote-exists` once the spec leg's exists, and the notice names + that one.""" + no_filter_repo_probe(adopter, monkeypatch) + plan = planned(tmp_path, make_source_repo(tmp_path / SOURCE, + tree=SMALL_TREE, edits=())) + code_leg = tmp_path / REMOTES / f"{ASSEMBLY}-{CODE}.git" + after_the_question(adopter, monkeypatch, lambda: git( + "init", "-q", BARE_FLAG, str(code_leg), cwd=tmp_path)) + + run_args = execute_args(tmp_path, plan) + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(run_args) + + assert caught.value.code == LEG_REMOTE_EXISTS + err = capsys.readouterr().err + assert f"{REFUSED} creating the {CODE} leg repository. " in err + spec_leg = (tmp_path / REMOTES).resolve() / f"{ASSEMBLY}-{SPEC}.git" + assert the_notice(err).startswith( + f"{ROLLED_BACK}: {spelled(spec_leg)} exists, and nothing is pushed " + "to it."), err + + +def test_a_local_remote_dir_that_cannot_be_made_after_the_question_is_refused( + adopter, monkeypatch, capsys, tmp_path): + """The `mkdir` of `--local-remote-dir` that follows the question raised + `FileExistsError` or `PermissionError` out of `main`, a traceback, for + what the question could not foresee (a file put there since). It is + refused by name, exit 2, before any leg exists, so with no notice.""" + no_filter_repo_probe(adopter, monkeypatch) + plan = planned(tmp_path, make_source_repo(tmp_path / SOURCE, + tree=SMALL_TREE, edits=())) + remotes = tmp_path / REMOTES + after_the_question(adopter, monkeypatch, lambda: remotes.write_text( + "put here since\n", encoding=UTF8)) + + assert adopter.main([EXECUTE, PLAN_FLAG, str(plan), "--yes", + "--local-remote-dir", str(remotes), "--work-dir", + str(tmp_path / WORK)]) == 2 + + err = capsys.readouterr().err + assert (f"{REFUSED} adopt-folder-cannot-be-made: --local-remote-dir " + f"{spelled(remotes.resolve())} cannot be made: ") in err, err + assert ROLLED_BACK not in err + assert remotes.is_file() + + def test_a_refusal_from_the_substitution_table_leaves_no_leg_repository( adopter, monkeypatch, tmp_path): """The table can refuse -- an `elected_on` that is not a date, a @@ -1541,7 +1611,7 @@ def adopt_with_hook(tmp_path: Path, ref_pattern: str): def execute(plan: Path, remotes: Path, work: Path, env: dict | None = None, merge_stderr: bool = False) -> subprocess.CompletedProcess: - return run_script(ADOPT, "execute", PLAN_FLAG, str(plan), "--yes", + return run_script(ADOPT, EXECUTE, PLAN_FLAG, str(plan), "--yes", "--local-remote-dir", str(remotes), "--work-dir", str(work), env=env, merge_stderr=merge_stderr) diff --git a/tests/test_adopt_leg_keys.py b/tests/test_adopt_leg_keys.py index 6f81afac..6d95a339 100644 --- a/tests/test_adopt_leg_keys.py +++ b/tests/test_adopt_leg_keys.py @@ -68,9 +68,20 @@ #: A `gh` that creates a bare repository for `repo create`, takes `repo edit`, #: and logs every argument of a `pr create`, NUL-separated, so the body is read -#: back as it was given. Anything else is refused: it reaches nothing. +#: back as it was given. `repo view --json nameWithOwner`, which +#: `execute` asks of each leg before it makes either (#798), finds one under +#: $FAKE_GH_ROOT, answering with its name as GitHub does, and gives gh's +#: not-found answer for any other. Anything else is refused: it reaches +#: nothing. STUB_GH = textwrap.dedent("""\ #!/bin/sh + if [ "$1 $2 $4 $5" = "repo view --json nameWithOwner" ]; then + if [ -d "$FAKE_GH_ROOT/$3.git" ]; then + printf '{"nameWithOwner": "%s"}\\n' "$3"; exit 0 + fi + echo "GraphQL: Could not resolve to a Repository with the name '$3'." >&2 + exit 1 + fi case "$1 $2" in "repo create") git init -q --bare -b main "$FAKE_GH_ROOT/$3.git" \\ diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index ba2b6d64..a89a60b2 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -148,6 +148,32 @@ def gh_calls(env: dict) -> str: return log.read_text(encoding=UTF8) if log.exists() else "" +def check_plan(plan: Path, **named) -> subprocess.CompletedProcess: + """`check --plan ` given the `--local-remote-dir` `execute` is + given here, `remotes` beside the plan: the test path, on which `check` + asks the disk and not GitHub whether each leg repository exists (#798). + So the stub's log holds only what a pin made `gh` asked, which is what + these tests read: `check` reads no network for a pin. One test runs + `check` as people run it, with no flag (`check_on_github`).""" + return run_script(ADOPT, "check", "--plan", str(plan), + "--local-remote-dir", str(plan.parent / "remotes"), + **named) + + +def check_on_github(plan: Path, **named) -> subprocess.CompletedProcess: + """`check --plan ` with no `--local-remote-dir`, as people run it: + it asks GitHub whether each leg repository exists (#798), the two calls + `LEG_VIEWS` names, and no more for a pin.""" + return run_script(ADOPT, "check", "--plan", str(plan), **named) + + +#: The two `gh` calls, as the stub logs them, that a `check` with no +#: `--local-remote-dir` makes for a plan of `PROJECT`: whether each leg's +#: repository exists, the spec leg's first. +LEG_VIEWS = [f"repo view {ORG}/{PROJECT}-{role} --json nameWithOwner" + for role in ("spec", "code")] + + def forge_objects(path: Path, objects: dict) -> Path: """`objects`, each oid and what the forge's GraphQL `object(oid:)` answers for it, written at `path` for the stub `gh` @@ -269,7 +295,7 @@ def adopted(request, tmp_path_factory, glass): for path, leg in ANSWERS: resolve(plan, path, leg) rmtree(readable) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=env) + checked = check_plan(plan, env=env) executed = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", "--local-remote-dir", str(base / "remotes"), "--work-dir", str(base / "work"), env=env) @@ -433,7 +459,7 @@ def adopted_as_text(request, tmp_path_factory): assert planned_run.returncode == 0, planned_run.stderr + planned_run.stdout for path, leg in ANSWERS: resolve(plan, path, leg) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=env) + checked = check_plan(plan, env=env) executed = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", "--local-remote-dir", str(base / "remotes"), "--work-dir", str(base / "work"), env=env) @@ -1054,7 +1080,7 @@ def test_check_passes_the_clone_a_plan_records_and_writes_no_path( with_pins(plan, "pins:\n" + pin_entry( commit=commit, digest=digest, source="local-clone (open\\u00a0Glass)")) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert [line for line in checked.stdout.splitlines() if line.startswith(" write ")] == [ @@ -1075,7 +1101,9 @@ def test_the_pin_and_write_lines_say_how_and_not_where(source_repo, tmp_path, digest was read, `local-clone`, and not where, because the path is this machine's own and `execute` writes into the pin file what `check` prints there (#540). The one line of either that names the clone is - #449's NOTE that the digest recomputes there.""" + #449's NOTE that the digest recomputes there. `check` runs here as + people run it, with no `--local-remote-dir`, and asks `gh` the two leg + questions alone (#798).""" path, commit, _ = glass digest = tree_digest(path, commit) out = tmp_path / "plan.yaml" @@ -1091,7 +1119,7 @@ def test_the_pin_and_write_lines_say_how_and_not_where(source_repo, tmp_path, assert str(path) not in result.stdout + result.stderr for answered, leg in ANSWERS: resolve(out, answered, leg) - checked = run_script(ADOPT, "check", "--plan", str(out), env=gh_env) + checked = check_on_github(out, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert [line for line in checked.stdout.splitlines() if line.startswith(" write ")] == [ @@ -1102,7 +1130,9 @@ def test_the_pin_and_write_lines_say_how_and_not_where(source_repo, tmp_path, f"NOTE {ENTRY}: digest recomputes at {commit[:12]} in the clone " f"`plan` read it from, {path}."], checked.stdout assert str(path) not in checked.stderr - assert gh_calls(gh_env) == "" + # The two leg questions, and nothing a pin asked: `check` reads no + # network for a pin when it runs as people run it, with no flag. + assert gh_calls(gh_env).splitlines() == LEG_VIEWS # --- what `check` finds, and `execute` refuses before any leg exists --------- @@ -1313,7 +1343,7 @@ def test_a_plan_whose_pin_execute_cannot_write_is_found_and_refused( block, said = cases[code][name] plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, block) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 1, checked.stdout + checked.stderr lines = [line for line in checked.stderr.splitlines() if line.startswith(f"FINDING {code}: ")] @@ -1413,7 +1443,7 @@ def test_a_pin_edited_after_plan_is_found_and_refused( commit, digest, code, recomputed = edits(glass)[edit] plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, read_from(recorded_clone, commit, digest)) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 1, checked.stdout + checked.stderr lines = [line for line in checked.stderr.splitlines() if line.startswith("FINDING ")] @@ -1457,7 +1487,7 @@ def test_a_pin_the_clone_recomputes_checks_ok(adopter, source_repo, tmp_path, typed = (commit.upper(), digest.upper()) if upper else (commit, digest) plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, read_from(recorded_clone, *typed)) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert pin_notes(checked) == [ f"NOTE {ENTRY}: digest recomputes at {commit[:12]} in the clone " @@ -1482,7 +1512,7 @@ def test_a_pin_whose_clone_is_gone_is_said_not_rechecked(source_repo, with_pins(plan, read_from(recorded_clone, second, tree_digest(path, first))) rmtree(recorded_clone) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert pin_notes(checked) == [ f"{NOT_RECHECKED}the clone `plan` read it from is not at " @@ -1509,7 +1539,7 @@ def test_a_pin_read_from_no_clone_is_said_not_rechecked(source_repo, with_pins(plan, "pins:\n" + pin_entry(commit=second, digest=tree_digest(path, first), source=source)) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert pin_notes(checked) == [ f"{NOT_RECHECKED}{why}. {GATE_RECOMPUTES}"], checked.stdout @@ -1530,7 +1560,7 @@ def test_a_pin_read_from_a_partial_clone_is_said_not_rechecked( plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, read_from(recorded_clone, second, tree_digest(path, first))) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert pin_notes(checked) == [ f"{NOT_RECHECKED}the clone at {recorded_clone} is a partial clone " @@ -1560,8 +1590,7 @@ def test_a_relative_clone_is_read_from_where_check_runs(source_repo, with_pins(plan, "pins:\n" + pin_entry(commit=second if edited else first, digest=tree_digest(path, first), source=f"local-clone ({name})")) - checked = run_script(ADOPT, "check", "--plan", str(plan), cwd=tmp_path, - env=gh_env) + checked = check_plan(plan, cwd=tmp_path, env=gh_env) assert checked.returncode == int(edited), checked.stdout + checked.stderr said = [line for line in (checked.stderr if edited else checked.stdout) .splitlines() if line.startswith(("FINDING ", f"NOTE {ENTRY}"))] @@ -1654,7 +1683,7 @@ def test_a_plan_with_a_whole_pin_checks_ok(source_repo, tmp_path, glass, and `execute` read no network for a pin, by design; #449).""" plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, "pins:\n" + a_good_pin(glass)) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert f" write {PIN_FILE}: opensoft/openGlass @ {glass[1][:12]}" \ in checked.stdout @@ -1670,7 +1699,7 @@ def test_a_plan_with_no_pin_says_nothing_of_pins(source_repo, tmp_path, """What `check` says of a pin's digest is said of pins, and only where there is one.""" plan = planned(source_repo, tmp_path, PLANNED) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert " write contracts/" not in checked.stdout assert "rechecked" not in checked.stdout @@ -1690,7 +1719,7 @@ def test_an_upper_cased_hand_edit_is_written_lower_cased(adopter, source_repo, plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, "pins:\n" + pin_entry(commit=commit.upper(), digest=digest.upper())) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr assert (f" write {PIN_FILE}: opensoft/openGlass @ {commit[:12]} tree " f"{digest[:12]}… (local-clone)") in checked.stdout diff --git a/tests/test_adopt_split_branch.py b/tests/test_adopt_split_branch.py index 17d103dc..22dde1af 100644 --- a/tests/test_adopt_split_branch.py +++ b/tests/test_adopt_split_branch.py @@ -515,6 +515,13 @@ def a_github(tmp_path: Path, monkeypatch, branches=(SPLIT_BRANCH,)) -> Path: return bare +#: The first `gh` command a run that went on to the legs gives: whether the +#: spec leg's repository exists, asked before either is made (#798), whole, +#: so that another `repo view` asked earlier cannot pass for it. +TO_THE_LEGS = ["gh", "repo", "view", f"{ORG}/{PROJECT}-spec", "--json", + "nameWithOwner"] + + def no_gh(adopter, monkeypatch) -> list: """`run`, with every `gh` command it is given recorded and refused as `Proceeded`: nothing here may reach GitHub.""" @@ -560,7 +567,7 @@ def test_execute_goes_on_to_the_legs_when_github_has_no_such_branch( asked = no_gh(adopter, monkeypatch) with pytest.raises(Proceeded): adopter.cmd_execute(execute_args(plan, tmp_path, local=False)) - assert asked[0][:3] == ["gh", "repo", "create"] + assert asked[0] == TO_THE_LEGS def test_execute_refuses_a_repository_it_cannot_list(adopter, tmp_path, @@ -668,7 +675,7 @@ def test_a_github_run_is_not_refused_for_the_split_branch_in_the_source( asked = no_gh(adopter, monkeypatch) with pytest.raises(Proceeded): adopter.cmd_execute(execute_args(plan, tmp_path, local=False)) - assert asked[0][:3] == ["gh", "repo", "create"] + assert asked[0] == TO_THE_LEGS def test_a_plan_that_seeds_both_legs_is_not_asked_about_the_extraction( @@ -693,7 +700,7 @@ def test_a_plan_that_seeds_both_legs_is_not_asked_about_the_extraction( args.allow_empty_leg = ["spec", "code"] with pytest.raises(Proceeded): adopter.cmd_execute(args) - assert asked[0][:3] == ["gh", "repo", "create"] + assert asked[0] == TO_THE_LEGS def with_origin(source: Path, bare: Path) -> None: @@ -843,7 +850,7 @@ def test_a_github_run_is_not_refused_for_a_branch_the_source_hides( args = execute_args(plan, tmp_path, local=False) with pytest.raises(Proceeded): adopter.cmd_execute(args) - assert asked[0][:3] == ["gh", "repo", "create"] + assert asked[0] == TO_THE_LEGS def test_an_earlier_split_branch_the_source_hides_is_found_and_refused( diff --git a/tests/test_adopt_temp_work_roots.py b/tests/test_adopt_temp_work_roots.py index 076fb358..61cc54cc 100644 --- a/tests/test_adopt_temp_work_roots.py +++ b/tests/test_adopt_temp_work_roots.py @@ -139,14 +139,21 @@ def gh_clones(adopter, monkeypatch, origin: Path) -> list: """`gh repo clone REPOSITORY ` stood in for by a `git clone` of `origin`, its `origin` then set to the GitHub URL, as gh leaves it; a repository gh's own parser refuses -- a path -- fails as gh fails it, - and any other repository is not found. Returns the list each `gh` - call's arguments are recorded in.""" + and any other repository is not found. `gh repo view + --json nameWithOwner`, which `check` and `execute` ask of each leg + repository before either is made (#798), finds none, as GitHub answers + for a name it does not have, and is not recorded. Returns the list each + other `gh` call's arguments are recorded in.""" real = adopter.run calls: list = [] def stand_in(args, *rest, **keywords): if args[0] != "gh": return real(args, *rest, **keywords) + if args[1:3] == ["repo", "view"] and \ + args[4:] == ["--json", "nameWithOwner"]: + raise adopter.CommandFailed( + args, None, 1, "GraphQL: Could not resolve to a Repository") calls.append(list(args)) if not GH_REPOSITORY.fullmatch(args[3]): raise adopter.CommandFailed(args, None, 1, diff --git a/tests/test_gh_guard.py b/tests/test_gh_guard.py index 11a1014a..93daaa28 100644 --- a/tests/test_gh_guard.py +++ b/tests/test_gh_guard.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 -"""The session's `gh` guard (#296): it is reached, it answers the two probes, -its check fails a test that ran `gh`, and the doctor's `machine` row meets it. +"""The session's `gh` guard (#296): it is reached, it answers the two probes +and whether a repository exists (#798), its check fails a test that ran `gh`, +and the doctor's `machine` row meets it. `tests/conftest.py` puts the guard first on PATH for every child and checks, after each test, that the guard refused none of that test's calls. Each test @@ -28,8 +29,8 @@ from pathlib import Path from conftest import REPO, WINDOWS_SKIP, run_script -from gh_guard import (GUARD_EXIT, NOT_LOGGED_IN, VERSION_LINE, failure, - record) +from gh_guard import (GUARD_EXIT, NEEDS_A_LOGIN, NO_LOGIN_EXIT, + NOT_LOGGED_IN, VERSION_LINE, failure, record) DOCTOR = REPO / "shape-doctor.py" @@ -111,6 +112,62 @@ def test_the_two_probes_answer_as_a_gh_with_no_login(gh_guard, request): assert failure(lines) is None +def test_whether_a_repository_exists_is_answered_as_a_gh_with_no_login( + gh_guard, request): + """`adopt-project.py check` asks `gh repo view / --json + nameWithOwner` of each leg it would create (#798). The guard answers as + a `gh` with no login does, exit 4 and gh's own words, logs it as a + probe, and the per-test check passes it. Any other `repo view` is still + refused.""" + asked = ["repo", "view", "testorg/Northwind-spec", "--json", + "nameWithOwner"] + result = run([guard_launcher(gh_guard), *asked]) + assert result.returncode == NO_LOGIN_EXIT, result.stdout + result.stderr + assert result.stdout == "" + assert result.stderr == NEEDS_A_LOGIN + "\n" + lines = gh_guard.take() + assert [(kind, argv) for kind, argv, _ in calls(lines)] == [ + ("probe", asked)] + assert calls(lines)[0][2].startswith(this_test(request)) + assert failure(lines) is None + for near in (["repo", "view", "testorg/Northwind-spec"], + ["repo", "view", "testorg/Northwind", "--json", + "visibility"], + ["repo", "view", "testorg/Northwind-spec", "--json", + "name"], + ["repo", "view", "--web", "--json", "nameWithOwner"], + ["repo", "view", "testorg/a/b", "--json", "nameWithOwner"], + ["repo", "create", "testorg/Northwind-spec", "--json", + "nameWithOwner"]): + assert failure([record(near, "")]) is not None, near + + +#: The argv `scaffold-project.py` (`remote_is_empty`) and `family.py` run to +#: ask whether a repository of their own exists, as each source spells it. +THEIR_VIEW = '["gh", "repo", "view", repository, "--json", "name"]' + + +def test_the_view_scaffold_and_family_ask_is_still_refused(gh_guard, + request): + """The guard answers adopt's `--json nameWithOwner` alone (#798). + `scaffold-project.py` and `family.py` ask `gh repo view / + --json name`, so a test of either that reaches gh with no stub of its + own is refused, exit 97, and failed at teardown, as before #798.""" + for tool in (REPO / "scaffold-project.py", REPO / "scripts" / "family.py"): + assert THEIR_VIEW in tool.read_text(encoding="utf-8"), tool + asked = ["repo", "view", "acme/Atlas", "--json", "name"] + result = run([guard_launcher(gh_guard), *asked]) + assert result.returncode == GUARD_EXIT, result.stdout + result.stderr + assert "tests/conftest.py" in result.stderr, result.stderr + lines = gh_guard.take() + assert [(kind, argv) for kind, argv, _ in calls(lines)] == [ + ("refused", asked)] + assert calls(lines)[0][2].startswith(this_test(request)) + message = failure(lines) + assert message is not None + assert "gh repo view acme/Atlas --json name " in message, message + + def test_the_check_fails_on_a_refused_call_and_passes_on_probes_alone(): """The per-test check's decision, called directly on log lines. diff --git a/tests/test_pin_annotated_tag.py b/tests/test_pin_annotated_tag.py index 330e489c..d6ae7449 100644 --- a/tests/test_pin_annotated_tag.py +++ b/tests/test_pin_annotated_tag.py @@ -34,12 +34,12 @@ import pytest -from conftest import (ADOPT, SCAFFOLD, WINDOWS_SKIP, git, planned, +from conftest import (SCAFFOLD, WINDOWS_SKIP, git, planned, refused_before_any_leg, run_script) from test_adopt_pins import ( # noqa: F401 - ENTRY, ORG, PIN_FILE, PLANNED, PROJECT, adopter, asked_what, forge_objects, - gh_calls, gh_env, make_product, plan_with, read_from, shape_materialize, - tree_listing, with_pins, + ENTRY, ORG, PIN_FILE, PLANNED, PROJECT, adopter, asked_what, check_plan, + forge_objects, gh_calls, gh_env, make_product, plan_with, read_from, + shape_materialize, tree_listing, with_pins, ) from test_scaffold_pin_and_reuse import assert_pin_digest_recomputed from repo_shape import load_yaml, tree_digest @@ -288,7 +288,7 @@ def test_check_finds_a_recorded_id_that_is_no_commit( plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, read_from(tagged_clone, named, tree_digest(tagged.path, tagged.commit))) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 1, checked.stdout + checked.stderr lines = [line for line in checked.stderr.splitlines() if line.startswith("FINDING ")] diff --git a/tests/test_pin_source_top_level.py b/tests/test_pin_source_top_level.py index 405e7fcb..5608d92a 100644 --- a/tests/test_pin_source_top_level.py +++ b/tests/test_pin_source_top_level.py @@ -36,8 +36,8 @@ from conftest import (ADOPT, ORG, SCAFFOLD, WINDOWS_SKIP, git, planned, refused_before_any_leg, run_script) from test_adopt_pins import ( # noqa: F401 - ENTRY, PIN_FILE, PLANNED, adopter, gh_calls, gh_env, read_from, - with_pins, + ENTRY, PIN_FILE, PLANNED, adopter, check_plan, gh_calls, gh_env, + read_from, with_pins, ) from test_pin_source_unreadable import RUNS, adoptable, made, run # noqa: F401 from repo_shape import Refusal, load_yaml, parse_yaml, tree_digest @@ -275,7 +275,7 @@ def test_check_finds_a_recorded_path_that_is_no_clone_s_top_level( plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, read_from(path, stand_ins.commit, recorded_on_main(path, stand_ins))) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 1, checked.stdout + checked.stderr lines = [line for line in checked.stderr.splitlines() if line.startswith("FINDING ")] @@ -309,7 +309,7 @@ def test_check_asks_a_recorded_top_level_as_it_always_did( path = stand_ins.base / where plan = planned(source_repo, tmp_path, PLANNED) with_pins(plan, read_from(path, stand_ins.commit, stand_ins.digest)) - checked = run_script(ADOPT, "check", "--plan", str(plan), env=gh_env) + checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr said = (f"digest NOT rechecked here: the clone at {path} is a partial " "clone" if where == PARTIAL else diff --git a/tests/test_template_values_single_pass.py b/tests/test_template_values_single_pass.py index 3c113449..3f642b72 100644 --- a/tests/test_template_values_single_pass.py +++ b/tests/test_template_values_single_pass.py @@ -434,7 +434,11 @@ def test_adopt_records_each_free_text_value_as_typed(kind, tmp_path, gh_stub): "--elected-on", "2026-09-02", "--out", str(plan), env=env) assert planned.returncode == 0, planned.stderr + planned.stdout - checked = run_script(ADOPT, "check", "--plan", str(plan), env=env) + # Given the `--local-remote-dir` `execute` is given, `check` asks the + # disk and not GitHub whether each leg repository exists (#798). + checked = run_script(ADOPT, "check", "--plan", str(plan), + "--local-remote-dir", str(tmp_path / "remotes"), + env=env) assert checked.returncode == 0, checked.stderr + checked.stdout result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", "--local-remote-dir", str(tmp_path / "remotes"),