diff --git a/AGENTS.md b/AGENTS.md index e579245c..202ee37f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -211,13 +211,13 @@ checkout — answers ONE question about ONE repository: is it compliant with thi shape, and what is missing. Run it before you propose a change to a repository you did not scaffold, and again after any procedure above. It writes nothing into the repository and fetches nothing, so there is no confirmation to get and -no `--dry-run` to do first. The checks are a REGISTRY with an empty `fix` slot -in each of them, which is where a repair mode will hang; nothing in it writes -today. The one exception to "writes nothing" is `--placement-plan `, which -writes that file and nothing else — see 4. Installed, `openRepoShape --doctor` -takes `--json` and `--placement-plan ` as well, and KEEPS its temporary -clone of the standard, because the NEXT lines run from it: its last line names -that directory, which is the human's to delete once those commands have run. +no `--dry-run` to do first. The one exception to "writes nothing" is +`--placement-plan `, which writes that file and nothing else — see 4. +The checks are a REGISTRY with an empty `fix` slot in each of them, which is +where a repair mode will hang; nothing in it writes today. Installed, +`openRepoShape --doctor` takes `--json` and `--placement-plan ` as well, +and KEEPS its temporary clone of the standard for the NEXT lines that name it: +its last line names that directory, the human's to delete once those have run. 1. **Read the verdict, then read the rows.** `COMPLIANT`, `COMPLIANT, SHAPE BEHIND`, `DRIFTED`, `MISPLACED`, `INVALID`, `NOT A SHAPE ROOT` — exits 0, 1, diff --git a/README.md b/README.md index f6171a4c..633e2312 100644 --- a/README.md +++ b/README.md @@ -1114,11 +1114,11 @@ openRepoShape --doctor # the repository you are standing in ./shape-doctor.py --root ../MedxEHR --json ``` -It **writes nothing into the repository and fetches nothing from it**, and it -reimplements nothing: each row is a check this file documents. It compares -against the checkout it is RUN FROM, so the verdict is offline. Installed, it -clones the standard into a temporary directory and KEEPS it, because its NEXT -lines run from there: its last line names it, safe to delete after they run. +It **writes nothing into the repository but a `--placement-plan` file you name, +and fetches nothing from it**, and it reimplements nothing: each row is a check +this file documents. It compares against the checkout it is RUN FROM, so the +verdict is offline. Installed, it KEEPS its temporary clone of the standard for +the NEXT lines naming it: its last line names it, safe to delete after they run. For an assembly root the rows are **naming** (this standard's own `contracts/repository-naming.yaml`), **manifest** and **pins** (the project's diff --git a/docs/cli.md b/docs/cli.md index 7c95713b..70ecd3f2 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -34,7 +34,7 @@ one today, and the generator refuses to write this file if one ever appears. | [`scaffold-project.py`](#scaffold-projectpy) | Creates the three repositories and writes the pins. What the front door calls once the human has said yes. | | [`adopt-project.py`](#adopt-projectpy) | Converts an EXISTING repository into the shape, in place, keeping its name and its history. | | [`update-shape.py`](#update-shapepy) | Re-copies a project's shape files from this standard and re-pins them. It never merges. | -| [`shape-doctor.py`](#shape-doctorpy) | Answers ONE question about ONE repository: is it compliant with this shape, and what is missing. Writes nothing. | +| [`shape-doctor.py`](#shape-doctorpy) | Answers ONE question about ONE repository: is it compliant with this shape, and what is missing. Writes nothing but a `--placement-plan` file you name. | | [`bootstrap`](#bootstrap) | Runs this standard's canonical bootstrap against a project that was never scaffolded, so a repository that elected nothing still has the command. | | [`scripts/bump-leg.py`](#scriptsbump-legpy) | Advances ONE leg's pin — the gitlink, `contracts/-pin.yaml` and every workflow `@` — in one lockstep commit. | | [`scripts/family.py`](#scriptsfamilypy) | Creates a FAMILY holder and maintains its member pins. | @@ -123,6 +123,7 @@ Anything after `--` is passed straight through to scaffold-project.py. openRepoShape --install install (or update) this command into ~/.local/bin openRepoShape --preflight check this MACHINE and stop; creates nothing openRepoShape --doctor [] [--json] [--placement-plan ] + diagnose a REPOSITORY against this standard openRepoShape [--org ] [setup-project.py options] [-- ] openRepoShape --help | --version @@ -168,7 +169,7 @@ ran this from, even inside the repository), and moves nothing. Run beside a checkout of the standard, it uses that one. Run from anywhere else, it clones the standard into a temporary directory first and KEEPS it, -because its NEXT lines name files in it. The report's last line names +for the NEXT lines that name files in it. The report's last line names that directory, which is safe to delete once you are done with those commands; under `--json` that line goes to stderr, so stdout stays one JSON object. diff --git a/docs/handbook.html b/docs/handbook.html index 79a63b04..3e6d0164 100644 --- a/docs/handbook.html +++ b/docs/handbook.html @@ -1451,7 +1451,7 @@

Bootstrap is COPIED into the project, not fetched

- +

Reference

Keeping a project’s shape current

@@ -1513,7 +1513,7 @@

Keeping a project’s shape current

- +

Reference

Is this repository compliant? openRepoShape --doctor

@@ -1523,12 +1523,13 @@

Is this repository compliant? openRepoShape --doctor

openRepoShape --doctor ../MedxEHR      # or: ./shape-doctor.py --root ../MedxEHR
 openRepoShape --doctor                 # the repository you are standing in
 ./shape-doctor.py --root ../MedxEHR --json
-

It writes nothing into the repository and fetches nothing from it, and - it reimplements nothing. One flag writes at all, and it writes one file you named: - --placement-plan, below. It compares against the checkout it is RUN FROM, so - the verdict is offline. Installed, it clones the standard into a temporary directory and - KEEPS it, because its NEXT lines run from there: its last line names it, safe to delete - after they run.

+

It writes nothing into the repository but a --placement-plan file you + name, and fetches nothing from it, and it reimplements nothing. That flag, below, is + the one that writes at all, and it writes one file, at the path you give: a relative one + lands in the directory you stand in, inside the repository when you stand there. It + compares against the checkout it is RUN FROM, so the verdict is offline. Installed, it + clones the standard into a temporary directory and KEEPS it for the NEXT lines that name + it: its last line names it, safe to delete after they run.

For an assembly root the rows are naming, manifest and pins (the project’s OWN validators when it carries them — they are the pinned copies, which is the point — else the template copies, and the row says diff --git a/openRepoShape b/openRepoShape index 58bbc3f2..46cab822 100755 --- a/openRepoShape +++ b/openRepoShape @@ -52,16 +52,23 @@ say() { printf '%s\n' "$*"; } set_mode() { [ "$MODE" = "run" ] || [ "$MODE" = "$1" ] || die "--$MODE and --$1 are two different runs; pass one. - --install places this command, --doctor reads a repository and writes nothing." + --install places this command; --doctor reads a repository and writes + nothing into it but a --placement-plan you name." MODE="$1" } die() { printf '\nREFUSED: %s\n' "$1" >&2; exit "${2:-2}"; } +# THE DOCTOR'S DESCRIPTION SITS ON A LINE OF ITS OWN, IN THE SAME COLUMN +# (#546). Its full form outgrew that column when #500 added the two flags, +# and the description went with it; argparse puts help under an option too +# long for the column in just this way. The form itself stays one line, the +# one the refusal under `--doctor` prints. usage() { cat <<'USAGE' openRepoShape --install install (or update) this command into ~/.local/bin openRepoShape --preflight check this MACHINE and stop; creates nothing openRepoShape --doctor [] [--json] [--placement-plan ] + diagnose a REPOSITORY against this standard openRepoShape [--org ] [setup-project.py options] [-- ] openRepoShape --help | --version @@ -107,7 +114,7 @@ ran this from, even inside the repository), and moves nothing. Run beside a checkout of the standard, it uses that one. Run from anywhere else, it clones the standard into a temporary directory first and KEEPS it, -because its NEXT lines name files in it. The report's last line names +for the NEXT lines that name files in it. The report's last line names that directory, which is safe to delete once you are done with those commands; under `--json` that line goes to stderr, so stdout stays one JSON object. @@ -482,12 +489,19 @@ run_doctor() { # 9808aec, SHOULD-1). Whether the clone is kept does not change, only # what the line claims about it. # + # AND "THAT NAMES IT", NEVER "RUNS FROM IT" (#546). Not every NEXT needs + # the clone: a `pins` row's, where the project carries its own validator, + # runs that copy and names no file in here. And "runs from it" read as + # "cd into it", where the `placement` row's relative `--placement-plan + # placement-plan.yaml` would write the plan inside the directory this + # same line calls safe to delete. + # # IT NAMES THE DIRECTORY AND HANDS OUT NO `rm`. Every other line here # that reads like a command is one to run; an agent told to run what the # report names would delete the clone before the commands it holds ran. if [ "$cloned" -eq 1 ]; then trap - EXIT - kept="the standard this run used is kept at $WORKDIR, and any NEXT command above runs from it; it is safe to delete once you are done with them." + kept="the standard this run used is kept at $WORKDIR, for any NEXT command above that names it; it is safe to delete once you are done with them." if [ "$DOCTOR_JSON" -eq 1 ]; then say "$kept" >&2 else @@ -513,9 +527,14 @@ while [ $# -gt 0 ]; do # would otherwise be taken for the a scaffold run names. A # word beginning with a dash is NOT the path -- it is a flag, and the # two below are the only ones this mode takes. + # + # ONE PATH, HOWEVER MANY `--doctor`s (#546). A path after a second + # `--doctor` used to replace the first one silently; now it is left for + # the positional case below, which takes it for a second path, and the + # doctor refuses that as it refuses `--doctor . Atlas`. --doctor) set_mode doctor; shift - if [ $# -gt 0 ]; then + if [ $# -gt 0 ] && [ "$DOCTOR_PATH_GIVEN" -eq 0 ]; then case "$1" in -*) ;; *) DOCTOR_PATH="$1"; DOCTOR_PATH_GIVEN=1; shift ;; @@ -537,12 +556,19 @@ while [ $# -gt 0 ]; do # The separate word is refused when it begins with `-`, as argparse # refuses it, so `--placement-plan --json` is not a plan written to a # file called `--json`; the `=` form is the way to name such a file. + # + # ONE PLAN PER RUN, AND A SECOND IS A REFUSAL, NEVER LAST-WINS (#546). + # Both used to reach `shape-doctor.py`, whose argparse kept the last, so + # `--placement-plan a.yaml --placement-plan b.yaml` wrote b.yaml alone + # and said nothing about a.yaml. The doctor writes one plan; a person who + # named two meant one of them, as with two modes or two paths above. --json | --placement-plan | --placement-plan=*) if [ "$MODE" != "doctor" ]; then PASSTHROUGH+=("$1"); shift elif [ "$1" = "--json" ]; then DOCTOR_ARGS+=(--json); DOCTOR_JSON=1; shift else + PLAN_BEFORE="$PLAN_FILE" case "$1" in --placement-plan=*) PLAN_FILE="${1#--placement-plan=}"; shift ;; *) @@ -555,6 +581,8 @@ while [ $# -gt 0 ]; do esac [ -n "$PLAN_FILE" ] || die "--placement-plan needs the file to write the plan to: --placement-plan , or --placement-plan= for a name that begins with '-'." + [ -z "$PLAN_BEFORE" ] || + die "two placement plans, '$PLAN_BEFORE' and '$PLAN_FILE'. --doctor writes one: pass --placement-plan once." DOCTOR_ARGS+=("--placement-plan=$PLAN_FILE") fi ;; @@ -602,7 +630,7 @@ if [ "$MODE" = "doctor" ]; then # here too until #500, while the mode was one path wide; they are # `shape-doctor.py`'s own, and are handed on above. [ -z "$PROJECT" ] && [ -z "$ORG" ] && [ ${#PASSTHROUGH[@]} -eq 0 ] || - die "--doctor takes a path, --json and --placement-plan , and nothing else; it reads a repository and writes nothing into it. + die "--doctor takes a path, --json and --placement-plan , and nothing else; it reads a repository and writes nothing into it but that . openRepoShape --doctor [] [--json] [--placement-plan ] To scaffold, drop --doctor. To check this machine, run: openRepoShape --preflight" set +e diff --git a/scripts/render-cli-reference.py b/scripts/render-cli-reference.py index 44772b71..03cb489d 100755 --- a/scripts/render-cli-reference.py +++ b/scripts/render-cli-reference.py @@ -143,7 +143,8 @@ class Tool(NamedTuple): "them. It never merges."), Tool("shape-doctor.py", "python", "python3 shape-doctor.py", "Answers ONE question about ONE repository: is it compliant with " - "this shape, and what is missing. Writes nothing."), + "this shape, and what is missing. Writes nothing but a " + "`--placement-plan` file you name."), Tool("bootstrap", "python", "python3 bootstrap", "Runs this standard's canonical bootstrap against a project that was " "never scaffolded, so a repository that elected nothing still has " diff --git a/shape-doctor.py b/shape-doctor.py index aebbfb2a..a16ab055 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -18,22 +18,22 @@ `adopt-project.py plan` or `setup.sh`. This runs them in sequence, prints one row each, and then prints ONE verdict. Every row is somebody else's check. -WHAT IT NEVER DOES. It never writes into the repository it is pointed at and -it never fetches. ONE FLAG WRITES AT ALL, and it writes one file the caller -named and nothing else: `--placement-plan `, below. The upstream a -project's copies are compared against is THE CHECKOUT THIS FILE IS RUN FROM, -which is the whole of why the verdict is offline: no clone, no authenticated -call, and the same answer on a machine that has never spoken to github.com. -It runs the project's own validators OFFLINE: `run_validator` starts each -child with no token, no `gh` configuration and a proxy that goes nowhere -(#297), so they are offline whatever their own contract says. ONE ROW IS THE -EXCEPTION AND IT IS NAMED: `machine` asks `gh` who -it is logged in as (`setup-project.py --preflight` runs `gh --version`, `gh -auth status` and `gh api user`), which `gh` answers by talking to its host. That row -is `n/a` by status and cannot move the verdict -- a workstation missing `gh` -is not a repository being non-compliant -- so nothing this command CONCLUDES -depends on a network, and a machine with no `gh` at all is reported rather -than refused. +WHAT IT NEVER DOES. It writes nothing into the repository it is pointed at +but a `--placement-plan ` the caller names there, and it never fetches. +ONE FLAG WRITES AT ALL, and it writes one file the caller named and nothing +else: `--placement-plan `, below. The upstream a project's copies are +compared against is THE CHECKOUT THIS FILE IS RUN FROM, which is the whole of +why the verdict is offline: no clone, no authenticated call, and the same +answer on a machine that has never spoken to github.com. It runs the project's +own validators OFFLINE: `run_validator` starts each child with no token, no +`gh` configuration and a proxy that goes nowhere (#297), so they are offline +whatever their own contract says. ONE ROW IS THE EXCEPTION AND IT IS NAMED: +`machine` asks `gh` who it is logged in as (`setup-project.py --preflight` +runs `gh --version`, `gh auth status` and `gh api user`), which `gh` answers +by talking to its host. That row is `n/a` by status and cannot move the +verdict -- a workstation missing `gh` is not a repository being non-compliant +-- so nothing this command CONCLUDES depends on a network, and a machine with +no `gh` at all is reported rather than refused. THE CHECKS ARE A REGISTRY, not a function with eight paragraphs in it. Each one is a `Check` -- an `id`, the root kinds it `applies_to`, a `run(ctx)` that diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index d6b94f82..b0a1222a 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -141,6 +141,21 @@ def test_help_prints_every_usage_line(): assert line in lines, f"--help never printed:\n {line}" +def test_the_doctor_line_keeps_its_description_column(): + """#500 gave the doctor's synopsis its two flags, and the full form + filled the column the other lines describe themselves in, so its + description went (#546). It is back, on the line under the form and in + that same column, where argparse puts the help of an option too long for + it; the form itself stays one line.""" + lines = run_cmd("--help").stdout.splitlines() + column = USAGE_LINES[0].index("install (or update)") + assert USAGE_LINES[1].index("check this MACHINE") == column + doctor = lines.index(next(line for line in USAGE_LINES if "--doctor" in line)) + assert lines[doctor + 1] == \ + " " * column + "diagnose a REPOSITORY against this standard", ( + lines[doctor:doctor + 2]) + + def test_help_points_at_openrepotools(): """`--help` used to list the other two commands; it names their new home. @@ -591,6 +606,10 @@ def test_two_modes_is_a_refusal_not_last_wins(args, tmp_path): env={"OPENREPOSHAPE_BIN_DIR": str(bin_dir)}) assert result.returncode == 2, result.stdout + result.stderr assert "two different runs" in result.stderr + # "and writes nothing" was false of a `--placement-plan` typed inside the + # repository (#546): it says which one file the doctor does write. + assert "--doctor reads a repository and writes nothing into it but a " \ + "--placement-plan you name." in " ".join(result.stderr.split()) assert not bin_dir.exists(), "a refused run placed a command anyway" diff --git a/tests/test_repo_hygiene.py b/tests/test_repo_hygiene.py index 8df3a3e6..5a334bce 100644 --- a/tests/test_repo_hygiene.py +++ b/tests/test_repo_hygiene.py @@ -1491,6 +1491,90 @@ def test_the_openrepotools_install_line_is_the_same_everywhere(name): f" {OPENREPOTOOLS_INSTALL}") +#: Where each document describes `openRepoShape --doctor`: the file, the +#: line its passage begins with, what ends it, and whether it describes the +#: installed form's kept clone. `docs/cli.md` has two: the doctor's purpose in +#: its table (scripts/render-cli-reference.py's `TOOLS`) and the command's own +#: `--help`. +DOCTOR_PASSAGES = [ + pytest.param("README.md", "### Is this repository compliant?", "\n## ", + True, id="README.md"), + pytest.param("AGENTS.md", "## Checking a repository's compliance", + "\n## ", True, id="AGENTS.md"), + pytest.param("docs/handbook.html", '

', "
", + True, id="docs/handbook.html"), + pytest.param("docs/cli.md", "### `openRepoShape`", "\n### ", True, + id="docs/cli.md-help"), + pytest.param("docs/cli.md", "| [`shape-doctor.py`]", "\n", False, + id="docs/cli.md-purpose"), +] + +#: In any case: a sentence, or a table cell, can begin with it. +WRITES_NOTHING = re.compile(r"(?i)\bwrites nothing\b") + + +def writes_nothing_unexcepted(paragraph: str) -> list: + """The sentences of `paragraph` that say the doctor writes nothing with + no exception: neither the sentence itself nor the one right after it + names `--placement-plan` as what it does write. AGENTS.md says the + exception in a sentence of its own, 'The one exception to "writes + nothing" is `--placement-plan `', so the next sentence counts when + it says "writes nothing" too, and a mere mention of the flag further on + does not.""" + # A sentence also ends at `.)`, `."` or `.**`: the stop inside a bracket, + # a quotation or bold text. + sentences = re.split(r"(?<=[.!?])\s+|(?<=[.!?][)\"'*])\s+" + r"|(?<=[.!?][)\"'*]{2})\s+", paragraph) + + def excepted(sentence: str) -> bool: + return bool(WRITES_NOTHING.search(sentence)) \ + and "--placement-plan" in sentence + + return [sentence for here, sentence in enumerate(sentences) + if WRITES_NOTHING.search(sentence) and not excepted(sentence) + and not excepted((sentences[here + 1:] or [""])[0])] + + +@pytest.mark.parametrize(("name", "begins", "ends", "keeps"), + DOCTOR_PASSAGES) +def test_the_doctor_says_what_it_writes_and_why_it_keeps_its_clone( + name, begins, ends, keeps): + """Two things every description of the doctor said that it does not do + (#546). + + "It writes nothing into the repository" left out the one file it writes: + the `--placement-plan ` you name, at the path you give, so a + relative one typed inside the repository lands there. A sentence that + says the doctor writes nothing carries that exception, or is followed at + once by the sentence that does. + + And the installed form KEEPS its clone of the standard "because its NEXT + lines run from it". A `pins` row's NEXT can name no file in it, and "run + from it" reads as "cd into it", where the `placement` row's relative + `--placement-plan placement-plan.yaml` writes the plan into the directory + the same sentence calls safe to delete. It is kept for the NEXT lines that + name it, and every passage about the installed form says so. + """ + text = (REPO / name).read_text(encoding="utf-8") + start = text.index(begins) + passage = text[start:text.find(ends, start + len(begins))] + problems, kept = [], 0 + for paragraph in re.split(r"\n\s*\n|

", passage): + said = " ".join(re.sub(r"<[^>]+>", "", paragraph).split()) + problems += [f"says the doctor writes nothing, and not that it writes " + f"the --placement-plan file you name: {sentence}" + for sentence in writes_nothing_unexcepted(said)] + if "KEEPS" in said: + kept += 1 + if not re.search(r"for the NEXT lines (?:that name|naming)\b", said) \ + or re.search(r"NEXT (?:lines?|commands?) runs? from", said): + problems.append("gives a reason for the kept clone other than " + f"the NEXT lines that name it: {said}") + assert kept or not keeps, ( + f"{name} no longer says that the installed doctor KEEPS its clone") + assert not problems, f"{name}:\n" + "\n".join(problems) + + def test_nothing_here_installs_park_or_resume(): """The carve, asserted rather than remembered (#92). diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 2e34cfbb..b14b8c38 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2106,10 +2106,14 @@ def test_the_shim_runs_the_script_beside_it_and_fetches_nothing( #: The sentence the installed doctor ends on when it has kept its clone -#: (#500), cut where the directory's name begins and where it ends. +#: (#500), cut where the directory's name begins and where it ends. It is +#: kept for the NEXT lines that NAME it (#546): "any NEXT command above runs +#: from it" was false of a `pins` row's, which can name no file in it, and +#: read as "cd into it", where the `placement` row's relative plan would be +#: written into the directory this line calls safe to delete. KEPT = "the standard this run used is kept at " -KEPT_TAIL = (", and any NEXT command above runs from it; it is safe to delete " - "once you are done with them.") +KEPT_TAIL = (", for any NEXT command above that names it; it is safe to " + "delete once you are done with them.") def kept_by(text: str) -> Path: @@ -2288,24 +2292,29 @@ def test_the_installed_doctor_hands_on_json_and_keeps_stdout_one_object( @WINDOWS_SKIP @pytest.mark.skipif(shutil.which("bash") is None, reason="needs bash") -@pytest.mark.parametrize("args", [ +@pytest.mark.parametrize(("args", "inside"), [ pytest.param(("--doctor", "{project}", "--placement-plan", "plan.yaml"), - id="path-then-flag"), + False, id="path-then-flag"), pytest.param(("--doctor", "--placement-plan=plan.yaml", "{project}"), - id="flag-then-path"), + False, id="flag-then-path"), + pytest.param(("--doctor", "--placement-plan", "plan.yaml"), + True, id="standing-in-the-repository"), ]) def test_the_installed_doctors_placement_plan_lands_where_you_stand( - args, standard, project, tmp_path): + args, inside, standard, project, tmp_path): """`--placement-plan ` reaches `shape-doctor.py` from the installed form (#500) -- it is the `placement` row's own NEXT, which the installed form refused -- and a RELATIVE `` lands in the directory the - command was run from: not in the repository, and not in the kept clone. + command was run from: not in the kept clone, and in the repository only + when that is where you stand (#546), as with `openRepoShape --doctor + --placement-plan plan.yaml` typed there, the default path `.`. - The plan is the only thing written, as AGENTS.md's doctor section says; - the project's tree is compared file by file before and after. + The plan is the only thing written, as AGENTS.md's doctor section says, + and the doctor's own wording now says too: "writes nothing into it but" + that file. The project's tree is compared file by file before and after. """ - here = tmp_path / "here" - here.mkdir() + here = project if inside else tmp_path / "here" + here.mkdir(exist_ok=True) def snapshot(): return sorted( @@ -2317,10 +2326,13 @@ def snapshot(): standard, tmp_path, *[arg.format(project=project) for arg in args], cwd=here) assert result.returncode == 0, result.stdout + result.stderr - assert [entry.name for entry in here.iterdir()] == ["plan.yaml"] - assert "kind: placement-plan" in (here / "plan.yaml").read_text( - encoding="utf-8") + plan = here / "plan.yaml" + assert "kind: placement-plan" in plan.read_text(encoding="utf-8") assert "placement plan written to plan.yaml" in result.stdout + if inside: + before = sorted(before + [("plan.yaml", plan.stat().st_size)]) + else: + assert [entry.name for entry in here.iterdir()] == ["plan.yaml"] assert snapshot() == before assert not (kept_by(result.stdout) / "shape" / "plan.yaml").exists() @@ -2351,19 +2363,41 @@ def test_a_refused_doctor_still_names_its_kept_clone_without_next_lines( [kept.resolve()] +#: What `--doctor` says when it is handed something it does not take. Its +#: last clause said "and writes nothing into it" until #546, which was false +#: of a `--placement-plan ` named inside the repository. +NOTHING_ELSE = ("--doctor takes a path, --json and --placement-plan , " + "and nothing else; it reads a repository and writes nothing " + "into it but that .") +NO_PLAN_FILE = "--placement-plan needs the file" +TWO_PLANS = ("two placement plans, 'a.yaml' and 'b.yaml'. --doctor writes " + "one: pass --placement-plan once.") + + @WINDOWS_SKIP @pytest.mark.skipif(shutil.which("bash") is None, reason="needs bash") -@pytest.mark.parametrize("args", [ - pytest.param(("--doctor", "--org", "Foo"), id="a-scaffold-flag"), - pytest.param(("--doctor", ".", "Atlas"), id="a-second-path"), - pytest.param(("--json", "--doctor", "."), id="json-before-doctor"), - pytest.param(("--doctor", ".", "--placement-plan"), id="plan-no-file"), - pytest.param(("--doctor", ".", "--placement-plan="), id="plan-empty"), +@pytest.mark.parametrize(("args", "refusal"), [ + pytest.param(("--doctor", "--org", "Foo"), NOTHING_ELSE, + id="a-scaffold-flag"), + pytest.param(("--doctor", ".", "Atlas"), NOTHING_ELSE, id="a-second-path"), + pytest.param(("--doctor", ".", "--doctor", "Atlas"), NOTHING_ELSE, + id="a-second-path-after-a-second-doctor"), + pytest.param(("--json", "--doctor", "."), NOTHING_ELSE, + id="json-before-doctor"), + pytest.param(("--doctor", ".", "--placement-plan"), NO_PLAN_FILE, + id="plan-no-file"), + pytest.param(("--doctor", ".", "--placement-plan="), NO_PLAN_FILE, + id="plan-empty"), pytest.param(("--doctor", ".", "--placement-plan", "--json"), - id="plan-a-flag"), + NO_PLAN_FILE, id="plan-a-flag"), + pytest.param(("--doctor", ".", "--placement-plan", "a.yaml", + "--placement-plan", "b.yaml"), TWO_PLANS, id="two-plans"), + pytest.param(("--doctor", "--placement-plan=a.yaml", ".", + "--placement-plan=b.yaml"), TWO_PLANS, + id="two-plans-around-the-path"), ]) -def test_the_installed_doctor_refuses_before_it_clones(args, standard, - tmp_path): +def test_the_installed_doctor_refuses_before_it_clones(args, refusal, + standard, tmp_path): """What `--doctor` does not take is still refused by name, exit 2, before anything is cloned (#500 hands on two flags, and only those). @@ -2373,17 +2407,18 @@ def test_the_installed_doctor_refuses_before_it_clones(args, standard, with no file, an empty one, or a flag where the file goes is refused rather than handed to argparse or written to a file called `--json`. Nothing reaches stdout, `$TMPDIR` stays empty, and nothing is fetched. + + AND A SECOND OF EITHER IS A REFUSAL, NEVER LAST-WINS (#546), as two + modes are. Two plans both reached `shape-doctor.py`, whose argparse kept + the last, so `b.yaml` was written and `a.yaml` silently was not; a path + after a second `--doctor` replaced the first. Neither file is written. """ result = run_installed(standard, tmp_path, *args) assert result.returncode == 2, result.stdout + result.stderr assert result.stdout == "" - assert "REFUSED: " in result.stderr - if "--placement-plan" in " ".join(args): - assert "--placement-plan needs the file" in result.stderr - else: - assert "--doctor takes a path, --json and --placement-plan , " \ - "and nothing else" in result.stderr + assert "REFUSED: " + refusal in result.stderr assert list((tmp_path / "tmp").iterdir()) == [] + assert not list(tmp_path.glob("*.yaml")) fetched_nothing(tmp_path, doctor_ran=False) @@ -2395,8 +2430,7 @@ def test_the_shim_doctor_takes_a_path_and_nothing_else(standard, tmp_path): capture_output=True, text=True, check=False, input="", env=shim_env(tmp_path)) assert result.returncode == 2, result.stdout + result.stderr - assert "--doctor takes a path, --json and --placement-plan , and " \ - "nothing else" in result.stderr + assert NOTHING_ELSE in result.stderr assert "openRepoShape --preflight" in result.stderr, ( "the refusal must name the machine check, because `--doctor` meant " "that until 2026-09-10")