diff --git a/AGENTS.md b/AGENTS.md index d02c684d..1021e389 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -401,7 +401,7 @@ python3 scripts/family.py siblings --family-root `family.yaml`'s `members:` block, which the tool rewrites wholesale, but for a row repair `validate-family.py` names; the commit is then the person's. 4. **The member must be scaffolded or adopted FIRST.** A family cannot make a - project out of a repository; it can only pin one that already is. + project out of a repository; it can only pin one that already is. A row whose `id:` is no project id is `member-id-malformed`. 5. **`make siblings` in the holder — or `family.py siblings --family-root `, the same file — places the WORKSTATION layout**: each member cloned BESIDE the holder, in the family folder, on its tracking branch, diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 7c7c2bb1..41ed5053 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -29,6 +29,12 @@ alone, so `--no-members` asks it too. - that every entry of `members:` is a mapping, the `member-row` finding (#864). It reads `family.yaml` alone, so `--no-members` asks it too. + - that every row's `id:` is a project id, `PROJECT_ID_RE` asked of the + whole of it, the `member-id-malformed` finding (#1204): an id that ends + in a line feed or holds a space names no project. It reads + `family.yaml` alone, so `--no-members` asks it too; with the members + checked, each member's own `project.yaml` `id:` is asked as `family.py + add` asks it. - EVERY MEMBER, three ways: 1. the GITLINK this repository records at `members/` equals `members[].pin.commit` — the same lockstep rule an assembly root @@ -1619,12 +1625,12 @@ def _check_member_checkout(root: Path, asked: Gitlinked, report: Report, return _check_member_tree(member, asked.pin, asked.path, asked.commit, asked.project, report, asked.rule, stop) - _check_member_identity(member, asked.row, asked.project, asked.path, + _check_member_identity(root, member, asked.row, asked.project, asked.path, report) -def _check_member_identity(member: Path, row: dict, project: str, path: str, - report: Report) -> None: +def _check_member_identity(root: Path, member: Path, row: dict, project: str, + path: str, report: Report) -> None: """The mounted tree is the PROJECT this row claims, not just a commit. A member is an assembly root, so it carries `project.yaml`, and that file @@ -1642,6 +1648,17 @@ def _check_member_identity(member: Path, row: dict, project: str, path: str, (`load_yaml`'s `yaml-unreadable` line), and gave no cure. `path` is the row's path as `_check_member_repository_and_path` resolved it, so a row that gives none is named at `members/

`, not `None` (r956's NIT 1). + + THE MEMBER'S OWN `id:` IS ASKED AS `family.py add` ASKS IT (#1204), + `PROJECT_ID_RE` of the whole of `str(id or "")` (`_read_member_project`'s + `member-bad-id`), so the gate refuses, members checked, every mounted + member `add` would refuse: `member-id-malformed`, naming the file, as + the source, and the `bump` that pins it once it is mended. A row that + spelled the same malformed text was `ok`, since the two compared equal. + Where either id is malformed, `member-id-mismatch` is not said as well: + the row's own finding, made in every mode, or this one, is the fault, + and a mismatch would send the person to copy a malformed id. `root` is + the holder each remedy names. """ manifest_path = member / "project.yaml" named = f"{path}/project.yaml" @@ -1673,11 +1690,32 @@ def _check_member_identity(member: Path, row: dict, project: str, path: str, return recorded_id = row.get("id") actual_id = manifest.get("id") + source_bad = not _is_project_id(str(actual_id or "")) + if source_bad: + word = str(recorded_id or project) + report.finding( + "member-id-malformed", + f"{named}: the member's own `id:` is {actual_id!r}, which does " + f"not match {PROJECT_ID_RE.pattern}, so the tree mounted at " + f"{path} is no project, and `family.py add` and `bump` refuse " + "it as `member-bad-id`. That file is the source every row pins, " + "so it is mended in the member's own repository, whose " + "validate-manifest.py refuses it as `manifest-id`; then, from a " + "checkout of openRepoShape and on a branch other than the " + "holder's tracking branch, which `bump` refuses " + f"(`{_switch_line(root, 'bump', word)}` makes one), " + f"`{_bump_to(root, project, None)}` pins the mended commit and " + f"writes the row's `id:` from it. {BRANCH_LEFT}") if recorded_id is None: report.finding( "member-id-absent", f"{project}: the row records no `id:`, so nothing checks that the " "tree mounted here is the project this row claims") + elif source_bad or not _is_project_id(str(recorded_id)): + # ONE FINDING FOR ONE FAULT (#1204): the member's own id was named + # just above, or the row's by `_check_member_ids_well_formed`, in + # every mode, with the `bump` that writes the row from the source. + return elif str(recorded_id) != str(actual_id): report.finding( "member-id-mismatch", @@ -1944,6 +1982,101 @@ def _check_member_ids_unique(root: Path, members: list, f"name {first} there instead to keep {project}. {BRANCH_LEFT}") +def _is_project_id(text: str) -> bool: + """Whether `text` is a project id: `PROJECT_ID_RE` asked of the WHOLE + text (#1204). + + `fullmatch`, NOT `match` (#1139): the pattern ends in `$`, which also + matches just before a trailing line feed, so `match` takes `irrs\\n`. + The pattern is the holder's own `repo_shape.py`'s, whose text each + finding prints, so it is asked whole, not respelled. ONE reader for + the two places a member's id is asked, its row's `id:` + (`_check_member_ids_well_formed`) and its mounted `project.yaml`'s + (`_check_member_identity`), so the two cannot drift into two + questions.""" + return PROJECT_ID_RE.fullmatch(text) is not None + + +def _bump_to(root: Path, project: str, commit: str | None) -> str: + """`family.py bump` of `project` in this holder, `--to commit`, or `--to + ` where no commit can be named, as THE LOCKSTEP RULE says it: + POSIX-quoted, as every line this file prints is (#1204).""" + to = "" if commit is None else shlex.quote(commit) + return (f"{PYTHON} scripts/family.py bump --family-root " + f"{shlex.quote(str(root))} --member {shlex.quote(project)} " + f"--to {to}") + + +def _check_member_ids_well_formed(root: Path, members: list, + report: Report) -> None: + """Every row's `id:` is a project id: the `member-id-malformed` finding + (#1204). + + Nothing asked it. A row's id met two checks, the mounted `project.yaml` + compared with it as text and `_check_member_ids_unique`, and neither is + a pattern, so under `--no-members` an id that ended in a line feed, held + a space or opened with `-` was `family ok`, and with the members checked + so was one the mounted `project.yaml` spelled the same: the row a + `family.py add` older than #1166 wrote for a member whose own `id:` read + `"irqs\\n"`. It reads `family.yaml` alone, so it runs under + `--no-members` too, as `--pins` and a full run do. + + THE ROWS IT ASKS ARE `_check_member_ids_unique`'s: a row that is no + mapping (`member-row`), has no `project:` (`member-project`) or spells + an earlier row's `project:` exactly (`member-duplicate`) is skipped, so + every row `_check_member_identity` reaches has been asked here. A row + with no `id:`, or `id: null`, is `member-id-absent`'s, as before. Any + other `id:` is asked as its text, `str()`, the text the identity check + compares: a number reads as its digits, so a row a holder written before + #259 keeps as `id: 2026` is an id, as `family.py`'s `_member_id` says + it passes; `true`, `1.5`, `-3`, a list and a mapping read as text no + id is, and are refused, each named as the reader read it. + + THE REMEDY IS `family.py bump` TO THE COMMIT THE ROW PINS, runnable as + printed: `bump` reads the member's own `project.yaml` at that commit, + asks its `id:` as `add` does (`member-bad-id`) and writes the row from + it, and the pin does not move. Not `remove` and then `add`, which the + issue suggested: in a holder with the member checked out, `remove` + leaves the member's object store under `.git/modules/` on purpose, and + `add` then refuses `member-git-dir-cached`, so the two as printed did + not run (measured for #1204); and `add` would clone the member again + and move the pin to its tip without an `--at`. `bump` needs the member + checked out, and its own `member-uninitialized` names the line that + checks it out. A pin that is no 40-hex commit is named `--to `; + where members are checked, `member-pin-commit` refuses the run first, and + this finding follows once the pin is mended. + """ + projects: set[str] = set() + for row in members: + if not isinstance(row, dict): + continue + project = row.get("project") + if not isinstance(project, str) or not project or project in projects: + continue + projects.add(project) + member_id = row.get("id") + if member_id is None or _is_project_id(str(member_id)): + continue + pin = row.get("pin") + commit = pin.get("commit") if isinstance(pin, dict) else None + if not isinstance(commit, str) or not COMMIT_RE.fullmatch(commit): + commit = None + report.finding( + "member-id-malformed", + f"{project}: the row records id {member_id!r}, which does not " + f"match {PROJECT_ID_RE.pattern}, so it names no project. The " + "member's own project.yaml `id:` is the source this row pins. " + "From a checkout of openRepoShape, with the member checked out, " + "and on a branch other than the holder's tracking branch, which " + "`bump` refuses " + f"(`{_switch_line(root, 'bump', str(member_id))}` makes one), " + f"`{_bump_to(root, project, commit)}` writes the row's `id:` " + "from that file at the commit the row pins, and the pin stays " + "where it is. Where that file's `id:` is no project id either, " + "`bump` refuses it as `member-bad-id`: mend it in the member's " + f"own repository first, and give `--to` that commit. {BRANCH_LEFT}") + + def _gitmodules_sections(root: Path) -> dict[str, dict[str, str]]: """Every `submodule` section of `.gitmodules`, `{name: {key: value}}`, each key in the order git lists it: the file read ONCE for the two @@ -4468,7 +4601,8 @@ def _check_members(root: Path, members: list, no_members: bool, Split from `main` for #137. Every check that needs no member checked out comes first, so `--no-members` asks them too: the ones that read a - file alone, `family.yaml`'s names (#331) and ids (#206) and the paths + file alone, `family.yaml`'s names (#331), ids (#206) and each id's + form, `member-id-malformed` (#1204), and the paths `.gitmodules` gives twice (#300), then the gitlinks and sections no row names, which read the index and HEAD as well (#361), the gitlinks no section of HEAD's `.gitmodules` gives (#611) or gives a url, and a @@ -4513,6 +4647,7 @@ def _check_members(root: Path, members: list, no_members: bool, """ _check_member_names_unique(members, report) _check_member_ids_unique(root, members, report) + _check_member_ids_well_formed(root, members, report) # A ROW THAT IS NOT A MAPPING reads `family.yaml` alone too, so it is # found here, in both modes (#864): found only in the loop below, past # the `--no-members` return, it was counted as a member there, and the diff --git a/tests/test_validate_family_member_id_pattern.py b/tests/test_validate_family_member_id_pattern.py new file mode 100644 index 00000000..a704336b --- /dev/null +++ b/tests/test_validate_family_member_id_pattern.py @@ -0,0 +1,509 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`validate-family.py` asks whether each member row's `id:` is a project id, +and with the members checked each member's own `project.yaml` `id:` too +(#1204, the follow-up of #1166's `member-bad-id`). + +THE DEFECT, measured on #1227's head merged onto main `81eaf757` +(`ccc2c2d2`, Python 3.12.3, git 2.43.0). `PROJECT_ID_RE` was asked of one +id only, the family's own. A row's `id:` met two checks and neither is a +pattern: the mounted `project.yaml` compared with it as text +(`member-id-mismatch`), and `_check_member_ids_unique`. So: + +- under `--no-members`, the run a private family's CI falls back to, a row + `id:` of `"irrs\\n"`, `"irrs\\r"`, `"IRRS Capital"` or `"-x"` was `family + ok`, exit 0; with the members checked each was `member-id-mismatch`, of a + row that only had to copy the member's id; +- the holder a `family.py add` older than #1166 wrote for a member whose own + `project.yaml` reads `id: "irqs\\n"`, its row recording the same, was + `family ok` in every mode: the two compared equal. + +THE RULE NOW. `member-id-malformed`, a finding of its own: of a row's `id:` +in every mode, since it reads `family.yaml` alone, and of the mounted +`project.yaml`'s `id:`, asked exactly as `family.py add` asks it, so the gate +refuses the members `add` refuses (`member-bad-id`) and none it would pin. +Where either id is malformed, `member-id-mismatch` is not said as well. Each +remedy is a `bump`, typed as printed below: to the commit the row pins, which +writes the row from the member's own `project.yaml` and leaves the pin, or to +the commit that mends that file. A number reads as its digits, as the +identity check always compared it, so a row a holder written before #259 +keeps as `id: 2026` passes; `null` and no `id:` are `member-id-absent`'s, as +before. + +The family is `tests/test_family.py`'s offline one: members scaffolded into +local bare repositories, no network and no GitHub, a `gh` stub on PATH. +""" + +from __future__ import annotations + +import os +import re +import shlex +import shutil + +import pytest + +from conftest import REPO, git, run_script +from repo_shape import PYTHON, double_quoted, tree_digest +#: The family fixture, its names and the helpers that own a hand edit and a +#: member whose own `id:` is written by hand, imported from the modules that +#: own them rather than rebuilt. +from test_family import ( # noqa: F401 + ORG, SPARE, family, family_module, holder, manifest, validate, +) +from test_family_tracking_branch import on_main # noqa: F401 +from test_validate_family_branch_step import BRANCH_LEFT, switch_argv +from test_value_pattern_line_break import ( + ID_PATTERN, a_member_whose_id_is, findings, rewritten, +) + +LF = chr(10) +CR = chr(13) + +#: IRRS's row `id:` line, as the fixture's `add` wrote it. +IRRS_ID = " id: irrs\n" + +#: Table 3's four values that the pattern refuses, as the reader reads them. +REFUSED = [ + pytest.param("irrs" + LF, id="line-feed"), + pytest.param("irrs" + CR, id="carriage-return"), + pytest.param("IRRS Capital", id="space"), + pytest.param("-x", id="leading-dash"), +] + + +def with_row_id(holder, spelled: str) -> None: + """`family.yaml` with IRRS's `id:` written as `spelled`, as a hand edit + writes it.""" + rewritten(holder / "family.yaml", IRRS_ID, f" id: {spelled}\n") + + +def row_finding(project: str, value) -> str: + """How a row's finding starts, for `value` as the reader read it.""" + return (f"FINDING member-id-malformed: {project}: the row records id " + f"{value!r}, which does not match {ID_PATTERN}, so it names no " + "project.") + + +def source_finding(project: str, value) -> str: + """How a mounted member's own finding starts.""" + return (f"FINDING member-id-malformed: members/{project}/project.yaml: " + f"the member's own `id:` is {value!r}, which does not match " + f"{ID_PATTERN}, so the tree mounted at members/{project} is no " + "project, and `family.py add` and `bump` refuse it as " + "`member-bad-id`.") + + +def commands(line: str) -> list[str]: + """The `git -C` switch and the `family.py` line a finding names, in + order.""" + return [command for command in re.findall(r"`([^`]*)`", line) + if command.startswith(("git -C", PYTHON))] + + +def row_of(holder, project: str) -> dict: + return next(row for row in manifest(holder)["members"] + if row["project"] == project) + + +def mounted_as_add_wrote_it(family, holder, tmp_path, row_id: str, + own: str = 'id: "irqs\\n"\n') -> None: + """IRQS mounted from a copy of its remote whose `project.yaml` writes + its `id:` as `own`, `id: "irqs\\n"` unless said (`a_member_whose_id_is`, + which leaves its work clone at `tmp_path / "edit"`), and its row written + as a `family.py add` older than #1166 wrote it, with its `id:` spelled + `row_id` (#1166 refuses that member now, so the row is written by + hand). Committed, the pin and the digest those of the mount: the id is + the only thing wrong.""" + remotes = a_member_whose_id_is(family, tmp_path, own) + git("-c", "protocol.file.allow=always", "submodule", "add", "-q", "--", + str(remotes / f"{SPARE}.git"), f"members/{SPARE}", cwd=holder) + mounted = holder / "members" / SPARE + commit = git("rev-parse", "HEAD", cwd=mounted).stdout.strip() + text = (holder / "family.yaml").read_text(encoding="utf-8") + (holder / "family.yaml").write_bytes((text.rstrip("\n") + "\n" + "\n".join([ + f" - project: {SPARE}", + f" id: {row_id}", + f" repository: {ORG}/{SPARE}", + f" path: members/{SPARE}", + " pin:", + " revision_kind: commit", + f' commit: "{commit}"', + " digest_algorithm: sha256", + " digest_definition: sorted-ls-tree-r-v1", + f' tree_sha256: "{tree_digest(mounted, commit)}"', + ]) + "\n").encode("utf-8")) + git("add", "--", ".gitmodules", f"members/{SPARE}", "family.yaml", + cwd=holder) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Mount IRQS as add mounted it before #1166", cwd=holder) + + +# --- a row's `id:`, under --no-members --------------------------------------- + +@pytest.mark.parametrize("value", REFUSED) +def test_a_row_id_the_pattern_refuses_is_found_under_no_members(holder, + value): + """THE ISSUE'S ROW, table 3: each was `family ok`, exit 0. It reads + `family.yaml` alone, so the run that skips the members finds it, and + it is the one finding.""" + with_row_id(holder, double_quoted(value)) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(row_finding("IRRS", value) + " "), found[0] + assert "member(s) SKIPPED (--no-members)" in result.stdout + + +@pytest.mark.parametrize("spelled", ['"0"', "irrs"], ids=["zero", "irrs"]) +def test_a_well_formed_row_id_is_family_ok_under_no_members(holder, spelled): + """Table 3's controls: `0` is an id the pattern takes, and `irrs` is the + fixture's own. Each is `family ok`, before and after.""" + with_row_id(holder, spelled) + result = validate(holder, "--no-members") + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.splitlines()[-1].startswith("family ok: "), \ + result.stdout + + +@pytest.mark.parametrize("extra", [(), ("--no-members",), ("--pins",)], + ids=["members", "no-members", "pins"]) +def test_the_fixture_is_family_ok_in_every_mode(holder, extra): + """The well-formed fixture, untouched: no new finding in any mode.""" + result = validate(holder, *extra) + assert result.returncode == 0, result.stdout + result.stderr + assert "member-id-malformed" not in result.stderr + + +# --- a row's `id:`, with the members checked -------------------------------- + +@pytest.mark.parametrize("value", REFUSED) +def test_with_members_checked_it_is_the_one_finding_not_a_mismatch(holder, + value): + """With the members checked each value was `member-id-mismatch`, which + tells a person to copy the member's id and names no command. It is the + row's own finding now, in `--pins` too, and only that: one finding for + one fault.""" + with_row_id(holder, double_quoted(value)) + for extra in ((), ("--pins",)): + result = validate(holder, *extra) + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(row_finding("IRRS", value)), found[0] + + +def test_a_well_formed_row_id_that_is_not_the_members_is_still_a_mismatch( + holder): + """The control of the one above: `0` is an id, so `member-id-mismatch` + stays its finding, word for word.""" + with_row_id(holder, '"0"') + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result.stderr) == [ + "FINDING member-id-mismatch: members/IRRS: project.yaml declares id " + "'irrs' but family.yaml records '0' for IRRS. The project's own " + "manifest is the source; this row is a pin of it."], result.stderr + + +# --- a row `id:` that is not a string --------------------------------------- + +@pytest.mark.parametrize("spelled", ["true", "1.5", "-3", "[irrs]", + "{irrs: 1}"], + ids=["bool", "float", "negative", "list", "mapping"]) +def test_a_row_id_whose_text_is_no_id_is_found(holder, spelled): + """THE DECISION FOR A VALUE THAT IS NOT A STRING: it is asked as its + text, `str()`, the text the identity check compares, and none of these + reads as an id. Named as the reader read it.""" + with_row_id(holder, spelled) + value = row_of(holder, "IRRS")["id"] + assert not isinstance(value, str), value + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(row_finding("IRRS", value)), found[0] + + +@pytest.mark.parametrize("number", [2026, 0], ids=["2026", "zero"]) +def test_a_row_id_that_reads_as_a_number_is_its_digits(holder, number): + """A holder written before #259 keeps `id: 2026` for a member scaffolded + with `--id 2026`, which the reader reads as a number; `family.py`'s + `_member_id` says the validator passes it, and it still does. A bare + `id: 0` is the one number the two readings could part on: `add` asks the + member's own `id:` as `str(id or "")`, which makes `0` no id, and the row + is asked as `str()` alone, so `0` is the id `"0"`, an id the pattern + takes. Under `--no-members` it is `family ok`; with the members checked + it is the one `member-id-mismatch`, of a well-formed id that is not the + member's, with the number named as the reader read it.""" + with_row_id(holder, str(number)) + value = row_of(holder, "IRRS")["id"] + assert repr(value) == str(number), value + result = validate(holder, "--no-members") + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.splitlines()[-1].startswith("family ok: "), \ + result.stdout + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result.stderr) == [ + "FINDING member-id-mismatch: members/IRRS: project.yaml declares id " + f"'irrs' but family.yaml records {number} for IRRS. The project's " + "own manifest is the source; this row is a pin of it."], \ + result.stderr + + +def test_a_row_spelled_as_an_earlier_one_is_member_duplicates_alone(holder): + """THE ROWS ASKED ARE `_check_member_ids_unique`'s: a second row spelled + `IRRS`, its `id:` malformed, is `member-duplicate`'s, as the members + loop never reaches it either. One finding for one fault; whichever row + stays is asked on the next run.""" + family_root = family_module().Family.open(holder) + rows = family_root.members + irrs = next(row for row in rows if row["project"] == "IRRS") + family_root.write_members([*rows, {**irrs, "id": "irrs" + LF}]) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result.stderr) == [ + "FINDING member-duplicate: IRRS appears more than once in " + "`members:`"], result.stderr + + +@pytest.mark.parametrize("spelled", ["null", None], ids=["null", "no-id"]) +def test_a_row_with_no_id_is_still_member_id_absent_alone(holder, spelled): + """`id: null` reads as no `id:`, and a row with none is + `member-id-absent`'s where it was, members checked, and nothing under + `--no-members`, as before.""" + if spelled is None: + rewritten(holder / "family.yaml", IRRS_ID, "") + else: + with_row_id(holder, spelled) + assert validate(holder, "--no-members").returncode == 0 + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result.stderr) == [ + "FINDING member-id-absent: IRRS: the row records no `id:`, so nothing " + "checks that the tree mounted here is the project this row claims"], \ + result.stderr + + +# --- the remedy for a pin that is no commit --------------------------------- + +def test_a_pin_that_is_no_commit_is_named_to_commit(holder): + """The remedy's `--to` is the commit the row pins, and where the row + pins none (`abc` here, not 40 hex) it is the placeholder ``, the + literal word: `bump` refuses `--to abc` as `member-to-not-a-commit`, so + printing the pin would print a line that cannot run. With the members + checked, `member-pin-commit` refuses the run first, so the row's finding + follows once the pin is mended.""" + pinned = row_of(holder, "IRRS")["pin"]["commit"] + with_row_id(holder, double_quoted("irrs" + LF)) + rewritten(holder / "family.yaml", f' commit: "{pinned}"\n', + ' commit: "abc"\n') + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(row_finding("IRRS", "irrs" + LF) + " "), \ + found[0] + argv = shlex.split(commands(found[0])[1]) + assert argv[:3] == [PYTHON, "scripts/family.py", "bump"], argv + assert argv[argv.index("--member") + 1:] == ["IRRS", "--to", ""] + refused = validate(holder) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refused.stderr.startswith("REFUSED member-pin-commit: IRRS: "), \ + refused.stderr + assert "member-id-malformed" not in refused.stderr + + +# --- a mounted member's own `id:` -------------------------------------------- + +def test_the_holder_add_wrote_before_1166_is_refused_in_every_mode( + family, holder, tmp_path): + """THE ISSUE'S HOLDER. Its row and its member's own `project.yaml` both + read `id: "irqs\\n"`, so the two compared equal and every mode was + `family ok`, exit 0. The row is found in every mode; with the members + checked, the member's own file too, named as the source.""" + mounted_as_add_wrote_it(family, holder, tmp_path, + double_quoted("irqs" + LF)) + alone = validate(holder, "--no-members") + assert alone.returncode == 1, alone.stdout + alone.stderr + found = findings(alone.stderr) + assert len(found) == 1, alone.stderr + assert found[0].startswith(row_finding(SPARE, "irqs" + LF)), found[0] + for extra in ((), ("--pins",)): + result = validate(holder, *extra) + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 2, result.stderr + assert found[0].startswith(row_finding(SPARE, "irqs" + LF)), found[0] + assert found[1].startswith(source_finding(SPARE, "irqs" + LF)), \ + found[1] + assert "member-id-mismatch" not in result.stderr + + +@pytest.mark.parametrize("row_id", [double_quoted("irqs" + LF), "irqs"], + ids=["equal-to-its-row", "row-well-formed"]) +def test_a_mounted_member_whose_own_id_is_malformed_is_found( + family, holder, tmp_path, row_id): + """A mounted member whose own `id:` is malformed is found whatever its + row says: equal to it (both refused, no mismatch), or a well-formed + `irqs`, which was `member-id-mismatch`, a finding that told the person + to copy the malformed id into the row. Its text names the file as the + source and the `bump` that pins it once mended.""" + mounted_as_add_wrote_it(family, holder, tmp_path, row_id) + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + own = [line for line in found if line.startswith( + "FINDING member-id-malformed: members/")] + assert len(own) == 1, result.stderr + assert own[0].startswith(source_finding(SPARE, "irqs" + LF) + " "), own[0] + assert "That file is the source every row pins" in own[0] + argv = shlex.split(commands(own[0])[1]) + assert argv[:3] == [PYTHON, "scripts/family.py", "bump"], argv + assert os.path.samefile(argv[argv.index("--family-root") + 1], holder) + assert argv[argv.index("--member") + 1:] == [SPARE, "--to", ""] + assert own[0].endswith(". " + BRANCH_LEFT), own[0] + assert "member-id-mismatch" not in result.stderr + assert len(found) == (2 if row_id != "irqs" else 1), result.stderr + + +def test_the_members_own_id_is_asked_as_add_asks_it(family, holder, tmp_path): + """`add` reads the member's own `id:` as `str(id or "")` + (`_read_member_project`), so a `project.yaml` whose `id:` reads as the + number 0 is `member-bad-id`, and the member's own `validate-manifest.py` + refuses it too, as no string. The gate asks it the same way: a row + spelling it `"0"`, which compared equal as text and was `family ok`, is + refused by the member's own finding, and only that.""" + mounted_as_add_wrote_it(family, holder, tmp_path, '"0"', own="id: 0\n") + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(source_finding(SPARE, 0) + " "), found[0] + + +# --- the remedies, typed as printed ------------------------------------------ + +@pytest.mark.parametrize("where", [None, "it's a holder"], + ids=["plain", "space-and-apostrophe"]) +def test_the_rows_remedy_runs_as_printed_from_the_tracking_branch( + on_main, tmp_path, where): + """A row `id:` written by hand and committed, on the tracking branch, + where a person stands after `init`. The finding's two commands, typed as + printed from a checkout of openRepoShape: the switch makes a branch, + and the `bump` to the commit the row pins writes the row's `id:` from + the member's own `project.yaml`, the pin where it was. The gate is + green after it, in every mode. The holder is also run from a path that + holds a space and an apostrophe, as `test_validate_family_branch_step.py` + does for `_switch_line`: the `bump` line quotes it for the reader's + shell, or what is typed is not what is printed.""" + holder = on_main + if where is not None: + target = tmp_path / where + shutil.move(str(on_main), str(target)) + holder = target.resolve() + with_row_id(holder, double_quoted("irrs" + LF)) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", + "A row id written by hand", cwd=holder) + pinned = row_of(holder, "IRRS")["pin"]["commit"] + found = validate(holder, "--no-members") + assert found.returncode == 1, found.stdout + found.stderr + line = found.stderr.split("FINDING member-id-malformed: ", + 1)[1].splitlines()[0] + assert line.endswith(". " + BRANCH_LEFT), line + switch, bump = commands(line) + assert shlex.split(switch) == switch_argv(holder, "bump", "irrs" + LF) + argv = shlex.split(bump) + assert argv[:3] == [PYTHON, "scripts/family.py", "bump"], bump + assert argv[argv.index("--member") + 1] == "IRRS", bump + assert argv[argv.index("--to") + 1] == pinned, bump + assert os.path.samefile(argv[argv.index("--family-root") + 1], holder) + git(*shlex.split(switch)[1:], cwd=holder.parent) + done = run_script(REPO / argv[1], *argv[2:], cwd=REPO) + assert done.returncode == 0, done.stdout + done.stderr + row = row_of(holder, "IRRS") + assert (row["id"], row["pin"]["commit"]) == ("irrs", pinned) + assert git("status", "--porcelain", cwd=holder).stdout == "" + for extra in ((), ("--no-members",)): + assert validate(holder, *extra).returncode == 0 + + +def test_the_members_remedy_pins_the_mended_commit_as_printed( + family, holder, tmp_path): + """The member's own `id:` mended in its own repository, then the + finding's two commands typed as printed, `` the mended one: the + `bump` pins it and writes the row's `id:` from it, and the gate is + green.""" + mounted_as_add_wrote_it(family, holder, tmp_path, + double_quoted("irqs" + LF)) + found = validate(holder) + line = found.stderr.split( + f"FINDING member-id-malformed: members/{SPARE}/project.yaml: ", + 1)[1].splitlines()[0] + switch, bump = commands(line) + words = shlex.split(switch) + assert words[-2:] == ["-c", "family/bump-member"], switch + assert os.path.samefile(words[2], holder), switch + work = tmp_path / "edit" + rewritten(work / "project.yaml", 'id: "irqs\\n"\n', 'id: "irqs"\n') + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", + "The id, mended", cwd=work) + git("push", "-q", "origin", "HEAD", cwd=work) + mended = git("rev-parse", "HEAD", cwd=work).stdout.strip() + git(*shlex.split(switch)[1:], cwd=holder.parent) + argv = shlex.split(bump.replace("", mended)) + done = run_script(REPO / argv[1], *argv[2:], cwd=REPO) + assert done.returncode == 0, done.stdout + done.stderr + row = row_of(holder, SPARE) + assert (row["id"], row["pin"]["commit"]) == ("irqs", mended) + assert validate(holder).returncode == 0 + + +# --- `add` and the gate ask one question ------------------------------------ + +#: `(the id, whether it is malformed)`: table 3's values and its controls, +#: and the edges of the pattern either side. +ONE_QUESTION = [ + pytest.param("irrs" + LF, True, id="line-feed"), + pytest.param("irrs" + CR, True, id="carriage-return"), + pytest.param("IRRS Capital", True, id="space"), + pytest.param("-x", True, id="leading-dash"), + pytest.param("", True, id="empty"), + pytest.param(" irrs", True, id="leading-space"), + pytest.param("IRRS", True, id="capitals"), + pytest.param("irrs.docs", True, id="dot"), + pytest.param("0", False, id="zero"), + pytest.param("irrs", False, id="irrs"), + pytest.param("irrs-2", False, id="hyphen"), + pytest.param("9-lives", False, id="digit-first"), +] + + +@pytest.mark.parametrize("value, malformed", ONE_QUESTION) +def test_add_refuses_exactly_the_ids_the_holders_own_validator_finds( + holder, tmp_path, value, malformed): + """THE GATE'S OWN ANSWER, in `test_family_add_member_id.py`'s way. For + each id, `add`'s question (`_read_member_project`, of a `project.yaml` + recording it, no clone) and the holder's `validate-family.py + --no-members` over IRRS's row recording it: `add` refuses + `member-bad-id` exactly when the gate finds `member-id-malformed`, and + both exactly when the id is malformed.""" + member = tmp_path / "member" + member.mkdir() + (member / "project.yaml").write_bytes( + ("schema_version: 1\nkind: project-manifest\n" + f"id: {double_quoted(value)}\nname: IRRS\n").encode("utf-8")) + module = family_module() + try: + module._read_member_project(member, "IRRS", f"{ORG}/IRRS") + except module.Refusal as exc: + assert exc.code == "member-bad-id", str(exc) + refused = True + else: + refused = False + with_row_id(holder, double_quoted(value)) + result = validate(holder, "--no-members") + found = "FINDING member-id-malformed: IRRS: " in result.stderr + assert (refused, found) == (malformed, malformed), result.stderr + assert result.returncode == (1 if malformed else 0), result.stderr diff --git a/tests/test_value_pattern_line_break.py b/tests/test_value_pattern_line_break.py index 2cded3c8..53c2484c 100644 --- a/tests/test_value_pattern_line_break.py +++ b/tests/test_value_pattern_line_break.py @@ -164,7 +164,9 @@ def test_every_spelling_of_a_value_pattern_is_in_the_table(): ("templates/assembly-root/scripts/validate-manifest.py", "fullmatch"): ( 1, "the manifest's `id:`"), ("templates/family-root/scripts/validate-family.py", "fullmatch"): ( - 1, "the family's `id:`"), + 2, "the family's `id:`; and `_is_project_id`, the one reader of a " + "member's row `id:` and of its mounted `project.yaml` `id:` " + "(#1204)"), ("scripts/family.py", "fullmatch"): ( 1, "a member's `project.yaml` `id:`, as `add` mounts it"), ("scripts/family.py", "match"): (