diff --git a/scripts/family.py b/scripts/family.py index f2a4e925..7a46a4a9 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -198,6 +198,13 @@ from shape_materialize import ( # noqa: E402 FAMILY_TEMPLATED, PLACEHOLDER_RE, render, ) +# Where the holder's own reader refuses the manifest `init` would write: the +# sentences that name the value on the line it refuses, and the first +# sentence of their remedy, the scaffold's and `adopt-project.py`'s since +# #708, asked of `family.yaml` (#929). +from shape_materialize import ( # noqa: E402 + KIND_INVALID, KIND_UNREADABLE, UNREADABLE_CHOICE, _unread_sentences, +) # How every command this prints for a person to paste spells its paths # (#486), the quoting the holder's `scripts/siblings.py` uses too. from repo_shape import shell_line, shell_quoted # noqa: E402 @@ -3437,17 +3444,50 @@ def family_manifest_invalid(values: dict, policy: NamingPolicy, what: dict, relates to another is asked as the holder asks it. `values` must be the complete table and `policy` the naming policy the holder carries a copy of. The validator's `Report` is asked for its findings alone: its notes - are what it says when nothing is wrong. + are what it says when nothing is wrong. Where the holder's own YAML + reader refuses the file before the validator can be asked, each + sentence is the reader's, naming the value on the line it refuses + (`_manifest_problems`, #929). + """ + return [sentence for _, sentence in _manifest_problems( + values, policy, what, template_dir)] + + +#: The placeholders that carry a value of `what` into `family.yaml` where no +#: line holds that value's own placeholder (#929): the organisation is the +#: owner half of the holder's `repository:`, so the reader's refusal of that +#: line is `--org`'s. `shape_materialize.CARRIED_BY` is the project's. +CARRIED_BY = {"FAMILY_REPOSITORY": "ORG"} + + +def _manifest_problems(values: dict, policy: NamingPolicy, what: dict, + template_dir: Path) -> list[tuple[str, str]]: + """`family_manifest_invalid`'s sentences, each with its kind, as + `shape_materialize` gives a project's (#708): `invalid` for a finding of + the validator's, and `unreadable` where the holder's own YAML reader + refuses the file the validator would read, whose words are the + reader's and whose remedy is its own (`MANIFEST_UNREADABLE_REMEDIATION`). + + THE VALUE IS NAMED (#929). The reader refuses `repository: @x/Orbit`, + so `init --org @x --local-remote-dir ` said only that the manifest + was a file the reader refuses, under a remedy calling the words the + validator's. `_unread_sentences`, the scaffold's and `adopt-project.py`'s + since #708, reads each line of `family.yaml` that carries a value alone + and names the value whose line the reader refuses, by its own + placeholder or by the one `CARRIED_BY` names; where none is, the + sentence is the reader's refusal of the whole file, as before. """ template = (Path(template_dir) / MANIFEST).read_text(encoding="utf-8") try: manifest = parse_yaml(render(template, values, MANIFEST)) except YamlError as exc: - return ["the manifest this run would write is a file the holder's " - f"own YAML reader refuses: {exc}."] + return [(KIND_UNREADABLE, sentence) for sentence in _unread_sentences( + values, what, template_dir, exc, manifest=MANIFEST, + carried_by=CARRIED_BY, reader="the holder's own YAML reader")] if not isinstance(manifest, dict): - return ["the manifest this run would write is not a mapping, which " - "is what the holder's own YAML reader makes of it."] + return [(KIND_INVALID, "the manifest this run would write is not a " + "mapping, which is what the holder's own YAML reader makes " + "of it.")] validator = family_validator(template_dir) # THE VALIDATOR'S OWN DEFECT, NEVER A FINDING. One that raises, one that # EXITS (a `sys.exit(0)` would end this run with a zero, having printed @@ -3470,7 +3510,8 @@ def family_manifest_invalid(values: dict, policy: NamingPolicy, what: dict, raise _validator_unusable( "answered with a finding that is not text", f"its type is {type(finding).__name__}") - return _sentences_of(findings, values, what) + return [(KIND_INVALID, sentence) + for sentence in _sentences_of(findings, values, what)] #: The remediation of a value the reader turns into something else, without @@ -3495,6 +3536,20 @@ def family_manifest_invalid(values: dict, policy: NamingPolicy, what: dict, "it refuses is red from its first commit, because `make validate` and " "the `validate` workflow run it first, and `init` cannot be run again " "over a pushed holder: there is no --force.") +#: Where the holder's own YAML reader refuses the manifest before its +#: validator can be asked (#929), the words are the reader's and a sentence +#: may name no value, so the remedy opens as the project's does +#: (`UNREADABLE_CHOICE`, #708) and says the words are the reader's. +MANIFEST_UNREADABLE_REMEDIATION = ( + f"{UNREADABLE_CHOICE} The words are the holder's own YAML reader's, " + "which refuses the manifest this run would write before its " + "validate-family.py can be asked. A holder whose manifest it refuses is " + "red from its first commit, because `make validate` and the `validate` " + "workflow run that validator first, and `init` cannot be run again over " + "a pushed holder: there is no --force.") +#: The remedy for each kind of problem `_manifest_problems` names. +KIND_REMEDIATIONS = {KIND_INVALID: MANIFEST_INVALID_REMEDIATION, + KIND_UNREADABLE: MANIFEST_UNREADABLE_REMEDIATION} def _manifest_flags(args) -> dict: @@ -3527,6 +3582,9 @@ def refuse_unholdable_manifest(args, values: dict, and alone, because the validator's finding about it (`id is None`) is its consequence; one the validator refuses (`--created-by ' '`, `--name 'Contoso Group'`) is `family-manifest-invalid`, in the validator's words. + A manifest the reader refuses before the validator can be asked (`--org + @x`, `repository: @x/`) is `family-manifest-invalid` still, in + the reader's words and with the reader's remedy (#929). """ flags = _manifest_flags(args) nothing = " Nothing was created, and `--dry-run` refuses the same." @@ -3534,11 +3592,12 @@ def refuse_unholdable_manifest(args, values: dict, if problems: raise Refusal("family-manifest-misread", " ".join(problems), MANIFEST_MISREAD_REMEDIATION + nothing) - problems = family_manifest_invalid( - values, NamingPolicy.load(NAMING_POLICY), flags, template_dir) - if problems: - raise Refusal("family-manifest-invalid", " ".join(problems), - MANIFEST_INVALID_REMEDIATION + nothing) + found = _manifest_problems(values, NamingPolicy.load(NAMING_POLICY), flags, + template_dir) + if found: + raise Refusal("family-manifest-invalid", + " ".join(sentence for _, sentence in found), + KIND_REMEDIATIONS[found[0][0]] + nothing) def cmd_init(args) -> int: diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index c36ef75d..383c897f 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -5172,11 +5172,19 @@ def _invalid_problems(values: dict, policy: NamingPolicy, what: dict, def _unread_sentences(values: dict, what: dict, template_dir: Path, - refusal: YamlError) -> list[str]: + refusal: YamlError, manifest: str = MANIFEST_TEMPLATE, + carried_by: dict | None = None, + reader: str = "the project's own YAML reader" + ) -> list[str]: """A sentence for each value of `what` that a line of the manifest the project's own reader refuses carries, in the reader's words, and the reader's `refusal` of the whole file where no such line is found. + `manifest`, `carried_by` (`CARRIED_BY` when None) and `reader`, the + words that name the reader, are the project's unless the caller gives + its own: `family.py init` asks it of the holder's `family.yaml`, whose + `repository:` carries `--org` (#929). + THE VALUE IS NAMED AGAIN (#708). Until #392 and #393 the reader took `repository: @x/Atlas` for text, so `--org @x` reached the validator, whose `manifest-leg-repository` names `--org`. Since then the reader @@ -5188,24 +5196,23 @@ def _unread_sentences(values: dict, what: dict, template_dir: Path, `_misread_rows` reads one, and a value is named where the reader refuses one of its lines. """ + carried_by = CARRIED_BY if carried_by is None else carried_by said: dict[str, list[str]] = {} - template = (Path(template_dir) / MANIFEST_TEMPLATE).read_text( - encoding="utf-8") + template = (Path(template_dir) / manifest).read_text(encoding="utf-8") for line in template.splitlines(): - carried = {CARRIED_BY.get(key, key) for key in _placeholders_of(line)} + carried = {carried_by.get(key, key) for key in _placeholders_of(line)} named = [key for key in what if key in carried and key in values] if not named or line.lstrip().startswith("#"): continue - reading = _reading_of(render(line, values, MANIFEST_TEMPLATE)) + reading = _reading_of(render(line, values, manifest)) if isinstance(reading, YamlError): for key in named: said.setdefault(key, []).append(f"{reading}.") if not said: - return ["the manifest this run would write is a file the project's " - f"own YAML reader refuses: {refusal}."] - return [f"{what[key]} is {str(values[key])!r}, which the project's own " - f"YAML reader refuses: {' '.join(words)}" - for key, words in said.items()] + return ["the manifest this run would write is a file " + f"{reader} refuses: {refusal}."] + return [f"{what[key]} is {str(values[key])!r}, which {reader} refuses: " + f"{' '.join(words)}" for key, words in said.items()] def _validator_notes(values: dict, policy: NamingPolicy, template_dir: Path, @@ -5286,13 +5293,17 @@ def manifest_judged(values: dict, policy: NamingPolicy, #: Where the project's own YAML reader refuses the manifest before its #: validator can be asked (#708), the words are the reader's, and a sentence #: may name no value (`_unread_sentences`), so the remedy says neither that -#: they are the validator's nor that a value is always named. -MANIFEST_UNREADABLE_REMEDIATION = ( +#: they are the validator's nor that a value is always named. Its first +#: sentence is `UNREADABLE_CHOICE`, which `family.py init`'s remedy for the +#: holder's reader opens with too (#929). +UNREADABLE_CHOICE = ( "Remediation: choose another value for each one named; where none is " - "named, the text the reader's words quote holds it. The words are the " - "project's own YAML reader's, which refuses the manifest this run would " - "write before its validate-manifest.py can be asked, and a project " - "whose manifest it refuses is one whose first bootstrap would fail.") + "named, the text the reader's words quote holds it.") +MANIFEST_UNREADABLE_REMEDIATION = ( + f"{UNREADABLE_CHOICE} The words are the project's own YAML reader's, " + "which refuses the manifest this run would write before its " + "validate-manifest.py can be asked, and a project whose manifest it " + "refuses is one whose first bootstrap would fail.") #: The remedy for each kind of problem, so a refusal names it by its kind #: (#708), with no conditional nested in another. KIND_REMEDIATIONS = {KIND_MISREAD: MANIFEST_MISREAD_REMEDIATION, diff --git a/tests/test_family_init_yaml_words.py b/tests/test_family_init_yaml_words.py new file mode 100644 index 00000000..27128516 --- /dev/null +++ b/tests/test_family_init_yaml_words.py @@ -0,0 +1,152 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`family.py init`: where the holder's own YAML reader refuses the manifest +`init` would write, the refusal names the value on the line it refuses and +gives the reader's remedy (#929, #459's U119 and U151). + +#817 (#708) did this for the scaffold and `adopt-project.py`. The family half +was left: the reader refuses `repository: @x/`, so `init --org @x +--local-remote-dir ` said only "the manifest this run would write is a +file the holder's own YAML reader refuses", named no value, and its remedy +said "the words after each value are the validator's own", when the words +were the reader's and the validator was never asked. Without +`--local-remote-dir`, `--org @x` is refused first as `family-bad-org` (#871), +so this wording is reached where no GitHub repository is made, or by a value +that passes GitHub's checks and that the reader still refuses. + +`init` now asks `shape_materialize._unread_sentences`, the scaffold's and +`adopt-project.py`'s since #708, of `family.yaml`, with `--org` carried by +the holder's `repository:`, and gives the reader's remedy, which opens with +the sentence the project's does (`UNREADABLE_CHOICE`). The code is +`family-manifest-invalid` still, as #708 kept `-manifest-invalid` for the +other two. A finding of the validator's keeps the validator's remedy. + +NO NETWORK AND NO GITHUB: `--local-remote-dir`, a template copy and the +suite's `gh` guard. +""" + +from __future__ import annotations + +import sys + +import pytest + +from conftest import REPO +from test_family_init_values import ( + HOLDER, TEMPLATE, family_module, given, init, nothing_was_created, table, + template_copy, +) +from test_reader_refusals_named import NOT_THE_VALIDATORS, OPENS + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 + +#: The code all three cases are refused under. +INVALID = "family-manifest-invalid" + +#: The reader's remedy, as `init` gives it, before the sentence it ends with. +READER_REMEDY = ( + "Remediation: choose another value for each one named; where none is " + "named, the text the reader's words quote holds it. The words are the " + "holder's own YAML reader's, which refuses the manifest this run would " + "write before its validate-family.py can be asked. A holder whose " + "manifest it refuses is red from its first commit, because `make " + "validate` and the `validate` workflow run that validator first, and " + "`init` cannot be run again over a pushed holder: there is no --force.") + +#: The validator's remedy, as `init` gives it, before the sentence it ends +#: with: what a finding of the validator's is told, and what the manifest +#: that is not a mapping is still told (lane 5's review of #960, NIT 3). +VALIDATOR_REMEDY = ( + "Remediation: choose a value the holder's own validate-family.py " + "accepts; the words after each value are the validator's own. A holder " + "it refuses is red from its first commit, because `make validate` and " + "the `validate` workflow run it first, and `init` cannot be run again " + "over a pushed holder: there is no --force.") + +#: The sentence every `init` remedy ends with, nothing having been made. +NOTHING = " Nothing was created, and `--dry-run` refuses the same." + +#: How the reader's words begin where they are the holder's. +HOLDERS_READER_REFUSES = "which the holder's own YAML reader refuses: " + + +@pytest.mark.parametrize("dry_run", [True, False], ids=["dry-run", "run"]) +@pytest.mark.parametrize("org", ["@x", "@opensoft"]) +def test_init_names_org_with_the_readers_remedy(tmp_path, org, dry_run): + """The issue's probe, both runs: `--org` is named, with the reader's + words for the holder's `repository:` line, under the reader's remedy, + and nothing is created.""" + result = init(tmp_path, "--org", org, dry_run=dry_run, + without=("--org",)) + assert result.returncode == 2, result.stderr + result.stdout + assert result.stdout == "" + assert (f"REFUSED {INVALID}: --org is {org!r}, {HOLDERS_READER_REFUSES}" + f"{OPENS} `@`, which is reserved in YAML: '{org}/{HOLDER}'.\n" + f"{READER_REMEDY}{NOTHING}\n") in result.stderr + assert NOT_THE_VALIDATORS not in result.stderr + nothing_was_created(tmp_path) + + +def test_a_refused_line_that_carries_no_value_names_the_whole_file( + tmp_path): + """Where the reader refuses a line that carries none of the values, the + sentence is its refusal of the whole file, as it was, and the remedy is + the reader's, which says where none is named, the text its words quote + holds it.""" + template = template_copy(tmp_path, "unreadable", quoted=True) + manifest = template / "family.yaml" + manifest.write_bytes(manifest.read_bytes() + b"notes: @nobody\n") + module = family_module() + with pytest.raises(module.Refusal) as caught: + module.refuse_unholdable_manifest(given(), table(), template) + assert caught.value.code == INVALID + assert caught.value.detail == ( + "the manifest this run would write is a file the holder's own YAML " + f"reader refuses: {OPENS} `@`, which is reserved in YAML: " + "'@nobody'.") + assert caught.value.remediation == READER_REMEDY + NOTHING + + +def test_a_finding_of_the_validator_keeps_the_validators_remedy(): + """The control: `--created-by ' '` reaches the validator, whose finding + is named in its own words under its own remedy, as before.""" + module = family_module() + with pytest.raises(module.Refusal) as caught: + module.refuse_unholdable_manifest(given(created_by=" "), + table(CREATED_BY=" "), TEMPLATE) + assert caught.value.code == INVALID + assert caught.value.detail.startswith( + "--created-by is ' ', which the holder's own validate-family.py " + "refuses: family-created-by: "), caught.value.detail + assert caught.value.remediation == VALIDATOR_REMEDY + NOTHING + + +def test_a_manifest_that_is_not_a_mapping_keeps_its_kind_and_remedy( + tmp_path): + """LANE 5'S REVIEW OF #960, NIT 3. The manifest that is not a mapping + is still `invalid` in `_manifest_problems`, so it keeps main's words + and the validator's remedy, as the body says. The existing test of it + (`test_family_init_values.py`) holds only the sentence; this holds the + remedy too, so moving its kind to the reader's fails a test.""" + template = template_copy(tmp_path, "list", quoted=True) + (template / "family.yaml").write_bytes(b"- one\n- two\n") + module = family_module() + with pytest.raises(module.Refusal) as caught: + module.refuse_unholdable_manifest(given(), table(), template) + assert caught.value.code == INVALID + assert caught.value.detail == ( + "the manifest this run would write is not a mapping, which is what " + "the holder's own YAML reader makes of it.") + assert caught.value.remediation == VALIDATOR_REMEDY + NOTHING + + +def test_the_projects_remedy_keeps_its_words(): + """The project's remedy is built from the sentence the holder's opens + with now, and says what it said under #708, byte for byte.""" + assert shape_materialize.MANIFEST_UNREADABLE_REMEDIATION == ( + "Remediation: choose another value for each one named; where none is " + "named, the text the reader's words quote holds it. The words are the " + "project's own YAML reader's, which refuses the manifest this run " + "would write before its validate-manifest.py can be asked, and a " + "project whose manifest it refuses is one whose first bootstrap would " + "fail.")