From f8650c849e9c23789688b0a4faedcfc31ed9eed1 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:44:08 +0000 Subject: [PATCH 1/2] validate-family.py: `member-duplicate` is decided from `members:` alone before the `--no-members` return and compares `project:` case-folded, so a `family.yaml` that names one row twice no longer passes the run a private family's CI falls back to, and rows `IRRS` and `irrs` are one finding naming both, since `members/IRRS` and `members/irrs` are one directory on macOS and Windows (#331) `member-duplicate` was raised in `_member_project_name`, which only the members-checked loop reaches, and `_check_members` returns before that loop under `--no-members`: the run a private family's CI falls back to when its members cannot be checked out. A `family.yaml` of two rows `project: IRRS` passed it (`family ok`, exit 0), so a duplicate row passed the one gate that ran. The comparison was exact besides, so rows `IRRS` and `irrs` with distinct ids passed in every mode, though `members/IRRS` and `members/irrs` are one directory on macOS and Windows. `family.py add` refuses a case variant now, so only a hand edit makes such rows, and this is the gate that finds the hand edit. THE FINDING IS ITS OWN FUNCTION. `_check_member_names_unique`, defined above `_check_member_ids_unique`, reads `members:` alone and is called from `_check_members` before the `--no-members` return, as `member-duplicate-id` (#206) is, so `--no-members` and `--pins` make it too. `_member_project_name` stops making the finding but still returns None for a name it has already seen, so that row's other checks stay skipped exactly as they were: the finding is made ONCE in every mode and no other finding moves. `_check_members`'s docstring and `member-duplicate-id` are untouched. IN ANY CASE. Two rows whose `project:` differs only in case are one finding, still `member-duplicate`, that names both rows as written and says they are one directory on a case-insensitive disk. An exact repeat keeps its text byte for byte (`tests/test_family.py` pins it). Each later row is one finding against the first row that holds its name. THE FOLD IS `str.lower()`, not `str.casefold()`: every name this standard admits for `project:` is ASCII (`family.py add` takes it through `checked_value`; a members-checked run refuses any other as `member-repository` and `member-repository-name`), and on ASCII the two agree. `Family.member` in `family.py` compares with `lower()` already (#206), so the tool that writes the rows and this gate cannot disagree about which rows are one. TESTS, in the new `tests/test_validate_family_duplicates.py` (15 rows): one row twice under `--no-members` (`member-duplicate`, exit 1, the finding alone); the same row twice with members checked and under `--pins` (one finding, not two); `IRRS` and `irrs` with distinct ids under `--no-members`, in both orders (both rows named as written, not the exact finding's text); three spellings plus a repeat (one finding per later row, in file order); distinct names, including `IRRS2` and `IRR`, ok in every mode; a row with no `project:` and a row that is no mapping skipped without a traceback; and, on a case-sensitive disk only, two real mounts `members/IRRS` and `members/irrs` (the second is IRQS, id `irqs`) in members-checked, `--pins` and `--no-members` runs, each one finding. macOS and Windows hold the two directories as one and skip that row; every other row builds its rows in `family.yaml` and runs there. MUTANTS, each against this file alone: the comparison exact again (7 of 15 fail: both case rows, the later-row row, the not-the-exact-finding row and the three mounted rows); the call moved after the `--no-members` return (6 fail: every `--no-members` row that expects a finding, the mounted one included, while the members-checked rows still pass); and the finding made in `_member_project_name` too (2 fail: the one-finding-not-two rows). Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- .../family-root/scripts/validate-family.py | 72 ++++- tests/test_validate_family_duplicates.py | 253 ++++++++++++++++++ 2 files changed, 322 insertions(+), 3 deletions(-) create mode 100644 tests/test_validate_family_duplicates.py diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 0991ba1b..201c99e5 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -216,15 +216,18 @@ def _member_project_name(row: dict, seen: dict, report: Report) -> str | None: """The row's own `project:`, or None when nothing else about this member should run — it has none, or its name was already seen once in `members:`. Split from `_check_member` for #137: the two ways this - member's checks stop before they start, in one place.""" + member's checks stop before they start, in one place. + + A name seen once already stops this row's checks and says nothing: the + finding for it is `_check_member_names_unique`'s (#331), made before any + row is checked so `--no-members` makes it too, and made once, not here as + well.""" project = row.get("project") if not isinstance(project, str) or not project: report.finding("member-project", f"a member row has no `project:`: " f"{row!r}") return None if project in seen: - report.finding("member-duplicate", - f"{project} appears more than once in `members:`") return None seen[project] = True return project @@ -522,6 +525,68 @@ def _check_shape_pin(root: Path, manifest: dict, report: Report) -> None: _check_shape_pin_files(root, pin, rel, report) +def _check_member_names_unique(members: list, report: Report) -> None: + """No two rows of `members:` name ONE project, in any case — the + `member-duplicate` finding (#331). + + It reads `members:` alone, so it runs under `--no-members` too, the run a + private family's CI falls back to when its members cannot be checked out, + and it runs before any row is checked, as `member-duplicate-id` does + (#206). `_member_project_name` used to make this finding, and only the + members-checked loop reaches that, so `--no-members` passed a + `family.yaml` that named one row twice. It is made HERE and once, in + every mode: `_member_project_name` still skips a row it has already seen, + and says nothing. + + Two spellings that differ only in case are one row: `members/IRRS` and + `members/irrs` are ONE directory on a case-insensitive disk (macOS, + Windows), where a holder can mount only one of them, whatever `id:` each + row records. The rows' ids are no help here and `member-duplicate-id`'s + question is a different one: it asks whether one PROJECT is mounted + twice, this asks whether one DIRECTORY is named twice. + + THE FOLD IS `str.lower()`, NOT `str.casefold()`, because every name this + standard admits for `project:` is ASCII, and on ASCII the two agree. + `family.py add` takes the name through `checked_value` + (`[A-Za-z0-9._/@+~-]`); a members-checked run refuses any other as + `member-repository` (`QUALIFIED_RE` holds the repository to ASCII) and + `member-repository-name` (the repository's name must BE the project). + `Family.member` in `family.py` compares with `lower()` (#206), so the tool + that writes the rows and this gate cannot disagree about which rows are + one. `casefold()` would only add folds (the German sharp s to `ss`) for + names no member can have. + + A row whose spelling an earlier row already has, to the byte, keeps the + finding it always had. A row that differs only in case names BOTH rows as + written. Either way each later row is one finding, against the first row + that holds its name. A row with no `project:` is `member-project`'s, and + a row that is no mapping is `member-row`'s: both are skipped here, as + `_check_member_ids_unique` skips them. + """ + spelled: set[str] = set() + first_folded: dict[str, str] = {} + for row in members: + if not isinstance(row, dict): + continue + project = row.get("project") + if not isinstance(project, str) or not project: + continue + if project in spelled: + report.finding("member-duplicate", + f"{project} appears more than once in `members:`") + continue + spelled.add(project) + first = first_folded.setdefault(project.lower(), project) + if first != project: + report.finding( + "member-duplicate", + f"{first} and {project} are two rows of `members:` whose " + f"`project:` differs only in case, so `members/{first}` and " + f"`members/{project}` are one directory on a case-insensitive " + "disk (macOS, Windows), where a holder can mount only one of " + "them. A family pins each project once.") + + def _check_member_ids_unique(root: Path, members: list, report: Report) -> None: """No two rows record ONE `id:` — that is one project pinned twice (#206). @@ -576,6 +641,7 @@ def _check_members(root: Path, members: list, no_members: bool, Split from `main` for #137. """ + _check_member_names_unique(members, report) _check_member_ids_unique(root, members, report) if no_members: print(f" -- {len(members)} member(s) SKIPPED (--no-members): " diff --git a/tests/test_validate_family_duplicates.py b/tests/test_validate_family_duplicates.py new file mode 100644 index 00000000..aa48dbf4 --- /dev/null +++ b/tests/test_validate_family_duplicates.py @@ -0,0 +1,253 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`validate-family.py` finds `member-duplicate` under `--no-members` too, and +in any case (#331). + +TWO FAULTS IN ONE FINDING. `member-duplicate` was raised in +`_member_project_name`, which only the members-checked loop reaches, and +`_check_members` returns before that loop under `--no-members`: the run a +private family's CI falls back to when its members cannot be checked out, so +a `family.yaml` that named one row twice passed the one gate that ran. And it +compared `project:` EXACTLY, so rows `IRRS` and `irrs` with distinct ids +passed everywhere, though `members/IRRS` and `members/irrs` are ONE directory +on macOS and Windows. `family.py add` refuses a case variant now, so only a +hand edit makes such rows; this is the gate that finds the hand edit. + +The finding is now its own function, `_check_member_names_unique`, which +reads `members:` alone and is called before the `--no-members` return, as +`member-duplicate-id` (#206, `tests/test_family.py`) is. It is made ONCE, +whatever the mode, and an exact repeat keeps the text it always had. + +The same family and the same offline rules as `tests/test_family.py`: the +fixture is imported rather than rebuilt, because a holder made any other way +would test a different holder. Members are scaffolded into local bare +repositories and nothing is created on GitHub. + +THE ROWS ARE BUILT IN `family.yaml` AND NEVER ON DISK, except in the one test +that mounts a second member at `members/irrs`: `--no-members` reads no +member, and a fixture with two directories whose names differ only in case +cannot exist on macOS or Windows (`mount_twice` spells its second row `IRRS2` +for that reason). That one test says so and skips there; every other row of +this module runs on all three platforms. +""" + +from __future__ import annotations + +import pytest + +from conftest import git +from test_family import ( # noqa: F401 + ORG, SPARE, family, family_module, holder, manifest, tree_digest, + validate, +) + +#: The finding an exact repeat has always had, byte for byte. A holder's CI +#: matches on text, so it is pinned here and not rebuilt from a template. +EXACT = "FINDING member-duplicate: IRRS appears more than once in `members:`" + + +def irrs_row(holder) -> dict: + """IRRS's row, as the manifest reads it back.""" + return next(row for row in manifest(holder)["members"] + if row["project"] == "IRRS") + + +def spelled(row: dict, project: str, member_id: str) -> dict: + """`row` under another spelling of the project, with an id of its own. + + The id is DISTINCT on purpose: `member-duplicate-id` asks whether one + project is mounted twice, and every row here is a different question — + whether one DIRECTORY is named twice. + """ + assert member_id != row["id"], "the point is a distinct id" + return {**row, "project": project, "id": member_id, + "repository": f"{ORG}/{project}", "path": f"members/{project}"} + + +def rewrite(holder, rows: list) -> None: + """The holder's `members:` block replaced by `rows`, and committed.""" + family_module().Family.open(holder).write_members(rows) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", + "Rewrite the members", cwd=holder) + + +def findings(result) -> list[str]: + """Every `FINDING` line, in the order they were printed.""" + return [line for line in result.stderr.splitlines() + if line.startswith("FINDING")] + + +def case_pair(first: str, second: str) -> str: + """The head of the finding for two rows that differ only in case: both + rows AS WRITTEN, the earlier one first, then what makes them one.""" + return (f"FINDING member-duplicate: {first} and {second} are two rows of " + "`members:` whose `project:` differs only in case, so " + f"`members/{first}` and `members/{second}` are one directory on " + "a case-insensitive disk") + + +# --- one row written twice --------------------------------------------------- + +def test_a_row_written_twice_is_found_under_no_members(holder): + """The run a private family's CI falls back to. The duplicate is a fact of + `family.yaml` alone, so it is found beside the members that run skips.""" + row = irrs_row(holder) + other = next(r for r in manifest(holder)["members"] + if r["project"] == "IRSS") + rewrite(holder, [row, dict(row), other]) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + assert findings(result) == [EXACT], result.stderr + assert "member(s) SKIPPED (--no-members)" in result.stdout + + +@pytest.mark.parametrize("extra", [(), ("--pins",)], + ids=["members-checked", "pins"]) +def test_with_members_checked_a_repeat_is_one_finding_not_two(holder, extra): + """`_member_project_name` used to make this finding and still stops that + row's checks, silently. A finding made in both places would be one fault + reported twice, and the count is what says so.""" + row = irrs_row(holder) + other = next(r for r in manifest(holder)["members"] + if r["project"] == "IRSS") + rewrite(holder, [row, dict(row), other]) + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + assert findings(result) == [EXACT], result.stderr + assert result.stderr.count("member-duplicate") == 1, result.stderr + + +# --- two rows that differ only in case --------------------------------------- + +@pytest.mark.parametrize("first,second", [("IRRS", "irrs"), ("irrs", "IRRS")], + ids=["upper-first", "lower-first"]) +def test_rows_that_differ_only_in_case_are_found_under_no_members( + holder, first, second): + """Both rows are named as WRITTEN, the earlier one first, whichever of + the two spellings that is, and the finding says why they are one.""" + base = irrs_row(holder) + rewrite(holder, [spelled(base, first, "irrs-first"), + spelled(base, second, "irrs-second")]) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + assert len(findings(result)) == 1, result.stderr + assert findings(result)[0].startswith(case_pair(first, second)), \ + result.stderr + assert "member-duplicate-id" not in result.stderr + assert "member(s) SKIPPED (--no-members)" in result.stdout + + +def test_a_case_variant_is_not_the_exact_finding(holder): + """The two are different faults in different words: an exact repeat says + it appears more than once, and a case variant says it is one directory on + some disks. A reader told the wrong one goes looking for a row that is + not there.""" + base = irrs_row(holder) + rewrite(holder, [base, spelled(base, "irrs", "irrs-twin")]) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + assert len(findings(result)) == 1, result.stderr + assert "appears more than once" not in result.stderr, result.stderr + + +def test_each_later_row_is_one_finding_against_the_first_that_holds_its_name( + holder): + """Three spellings of one name and one of them twice: every row after the + first is ONE finding, a case variant names the first row it collides with + and an exact repeat keeps its own text, in file order.""" + base = irrs_row(holder) + lower = spelled(base, "irrs", "irrs-two") + title = spelled(base, "Irrs", "irrs-three") + rewrite(holder, [base, lower, title, dict(lower)]) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + found = findings(result) + assert len(found) == 3, result.stderr + assert found[0].startswith(case_pair("IRRS", "irrs")), found + assert found[1].startswith(case_pair("IRRS", "Irrs")), found + assert found[2] == ("FINDING member-duplicate: irrs appears more than " + "once in `members:`"), found + + +def test_distinct_names_are_not_folded_into_one(holder): + """A name that merely starts with another's, or ends with a digit, is its + own row: the fold is CASE alone.""" + base = irrs_row(holder) + other = next(r for r in manifest(holder)["members"] + if r["project"] == "IRSS") + rewrite(holder, [base, spelled(base, "IRRS2", "irrs2"), + spelled(base, "IRR", "irr"), other]) + result = validate(holder, "--no-members") + assert result.returncode == 0, result.stderr + result.stdout + assert "family ok: InkRouter (inkrouter), 4 member(s)" in result.stdout + + +# --- a holder with nothing wrong --------------------------------------------- + +@pytest.mark.parametrize("extra", [(), ("--pins",), ("--no-members",)], + ids=["members-checked", "pins", "no-members"]) +def test_distinct_names_pass_in_every_mode(holder, extra): + """The family `init` and `add` made: two members, two names.""" + result = validate(holder, *extra) + assert result.returncode == 0, result.stderr + result.stdout + assert findings(result) == [] + assert "family ok: InkRouter (inkrouter), 2 member(s)" in result.stdout + + +def test_a_row_with_no_project_is_not_this_findings_to_make(holder): + """A row that is no mapping and a row with no `project:` are + `member-row`'s and `member-project`'s, which only the members-checked loop + asks. This one skips them rather than crash on them: a traceback exits 1, + which is a finding's code.""" + # BYTES, so Windows does not rewrite the whole manifest with CRLF. + text = (holder / "family.yaml").read_bytes().decode("utf-8") + (holder / "family.yaml").write_bytes( + (text + " - nameless\n - id: nameless\n").encode("utf-8")) + result = validate(holder, "--no-members") + assert result.returncode == 0, result.stderr + result.stdout + assert "Traceback" not in result.stderr + assert findings(result) == [] + + +# --- both rows mounted ------------------------------------------------------- + +@pytest.fixture +def case_sensitive(tmp_path): + """Skips on a disk that holds `Dir` and `dir` as one, as macOS and + Windows do by default: the second mount cannot exist there.""" + (tmp_path / "CaseProbe").mkdir() + if (tmp_path / "caseprobe").exists(): + pytest.skip("this disk is case-insensitive, so members/IRRS and " + "members/irrs cannot both be mounted; every other row " + "of this module builds its rows in family.yaml and runs " + "here") + + +@pytest.mark.parametrize("extra", [(), ("--pins",), ("--no-members",)], + ids=["members-checked", "pins", "no-members"]) +def test_two_mounted_rows_that_differ_only_in_case_are_one_finding( + family, holder, case_sensitive, extra): + """BOTH ROWS REAL, in every way the gate asks about: mounted at the path + their name makes, pinned at the commit mounted there, digested, and the + `project.yaml` inside says the id the row says — `irrs` mounts a + different member (IRQS, id `irqs`) so the ids are distinct and the only + thing wrong is the name. Members checked or not, it is ONE finding.""" + git("-c", "protocol.file.allow=always", "submodule", "add", "-q", "--", + str(family["remotes"] / f"{SPARE}.git"), "members/irrs", cwd=holder) + mounted = holder / "members" / "irrs" + commit = git("rev-parse", "HEAD", cwd=mounted).stdout.strip() + base = irrs_row(holder) + twin = spelled(base, "irrs", SPARE.lower()) + twin["pin"] = {**base["pin"], "commit": commit, + "tree_sha256": tree_digest(mounted, commit)} + family_module().Family.open(holder).write_members( + [*manifest(holder)["members"], twin]) + git("add", "--", ".gitmodules", "members/irrs", "family.yaml", + cwd=holder) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Mount a second member that differs from IRRS only in case", + cwd=holder) + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + assert len(findings(result)) == 1, result.stderr + assert findings(result)[0].startswith(case_pair("IRRS", "irrs")), \ + result.stderr From 3d8d65ed223b0ac9d4f61928125d29ce0380ed14 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:49:36 +0000 Subject: [PATCH 2/2] validate-family.py: fix round on the review of f8650c8: `member-duplicate` folds with `str.casefold()` so the long s and the sigmas count, its case finding no longer says a family pins each project once, the exact-repeat skip in `_member_project_name` is held by a test, and the WHAT IS CHECKED list names it (#331) SHOULD-1, test only. `_member_project_name` still skips a name it has already seen, and on main that line also MADE the finding, so deleting it turned the duplicate tests red; this change moved the finding to `_check_member_names_unique` and left the skip unguarded, and a mutant that deleted it passed every test. The tests only ever repeated a row byte for byte, and a copy checks out as cleanly as the row it copies. The one-finding-not-two rows now run a second repeat whose `pin.commit` is 40 zeros: with the skip gone that row is checked, refused `member-commit-unresolvable` (exit 2) and the duplicate finding is lost, so both modes go red. NIT-1, the coordinator's ruling. The fold is `str.casefold()`, as `shape_materialize.py` and `adopt-project.py` fold every other same-path-in- any-case check. `--no-members` admits any string as a `project:` (the checks that hold a name to ASCII run only with members checked), and a disk folds more than ASCII case: `IRRS` and `irrs` spelt with the long s (U+017F), or a name in capital sigmas against the same name in small ones, passed under `--no-members` with `lower()`. The comment now says that, says `casefold()` is no one disk's own fold (NTFS does not fold the German sharp s to `ss`; APFS also ignores normalization), and still says the two agree on every name `family.py` can write, which is why `Family.member`'s `lower()` is not at odds with it. Two new rows (long s, sigma) run in UTF-8 mode. NIT-5. The case finding's closing sentence "A family pins each project once" assumed one project; rows with distinct ids may be two. It now says a holder on a case-insensitive disk can keep only one of the two directories, that whether the rows pin one project or two one of them has to go, and to keep the row whose `repository:` is the member's and drop the other. It names no command: `family.py remove` cannot take out a row that was never mounted, which is the only kind a macOS or Windows holder can hold. A new row holds the wording. The exact-repeat text is unchanged byte for byte. The module docstring's WHAT IS CHECKED list now names the check, as its own bullet ahead of EVERY MEMBER, so it does not sit on the lines #317 edits. `_check_members`'s docstring is untouched. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- .../family-root/scripts/validate-family.py | 49 +++++++---- tests/test_validate_family_duplicates.py | 84 +++++++++++++++++-- 2 files changed, 111 insertions(+), 22 deletions(-) diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 201c99e5..ed3a2b9c 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -23,6 +23,10 @@ terms. - `shape`: a commit-and-digest pin of the openRepoShape revision this root was cut from, `revision_kind: commit`, never a tag + - across the rows, that no two name ONE `project:`, in any case: + `members/IRRS` and `members/irrs` are one directory on macOS and Windows, + whatever `id:` each row records (#331). This one reads `family.yaml` + alone, so `--no-members` asks it too. - EVERY MEMBER, three ways: 1. the GITLINK this repository records at `members/` equals `members[].pin.commit` — the same lockstep rule an assembly root @@ -545,22 +549,31 @@ def _check_member_names_unique(members: list, report: Report) -> None: question is a different one: it asks whether one PROJECT is mounted twice, this asks whether one DIRECTORY is named twice. - THE FOLD IS `str.lower()`, NOT `str.casefold()`, because every name this - standard admits for `project:` is ASCII, and on ASCII the two agree. - `family.py add` takes the name through `checked_value` - (`[A-Za-z0-9._/@+~-]`); a members-checked run refuses any other as - `member-repository` (`QUALIFIED_RE` holds the repository to ASCII) and - `member-repository-name` (the repository's name must BE the project). - `Family.member` in `family.py` compares with `lower()` (#206), so the tool - that writes the rows and this gate cannot disagree about which rows are - one. `casefold()` would only add folds (the German sharp s to `ss`) for - names no member can have. + THE FOLD IS `str.casefold()`, as the standard's other same-path-in-any-case + checks use it (`shape_materialize.py`, `adopt-project.py`). `family.py add` + only writes ASCII names (`checked_value`: `[A-Za-z0-9._/@+~-]`), and a + members-checked run refuses any other as `member-repository` + (`QUALIFIED_RE`) or `member-repository-name`. Under `--no-members` neither + runs, so a hand edit can put ANY string in `project:`, and a disk folds + more than ASCII case: the long s (U+017F) folds to `s`, and a final sigma + (U+03C2) to a small sigma (U+03C3), so `IRRS` and `irrs` spelt with the + long s, or a name in capital sigmas and the same name in small ones, are + one directory there, and `lower()` keeps each pair apart. On every name + `family.py` can write the two folds agree, and `Family.member` in + `family.py` compares with `lower()` (#206), so the tool that writes the + rows and this gate cannot disagree about a row the tool wrote. + `casefold()` is no one disk's own fold either: NTFS's table does not fold + the German sharp s (U+00DF) to `ss`, so for such a pair this may name a + collision that disk does not make, and APFS also ignores normalization, + which no fold here handles. It errs on the side of a gate that says one + row has to go. A row whose spelling an earlier row already has, to the byte, keeps the - finding it always had. A row that differs only in case names BOTH rows as - written. Either way each later row is one finding, against the first row - that holds its name. A row with no `project:` is `member-project`'s, and - a row that is no mapping is `member-row`'s: both are skipped here, as + finding it always had. A row that differs only in case, as a + case-insensitive disk folds it, names BOTH rows as written. Either way + each later row is one finding, against the first row that holds its name. + A row with no `project:` is `member-project`'s, and a row that is no + mapping is `member-row`'s: both are skipped here, as `_check_member_ids_unique` skips them. """ spelled: set[str] = set() @@ -576,15 +589,17 @@ def _check_member_names_unique(members: list, report: Report) -> None: f"{project} appears more than once in `members:`") continue spelled.add(project) - first = first_folded.setdefault(project.lower(), project) + first = first_folded.setdefault(project.casefold(), project) if first != project: report.finding( "member-duplicate", f"{first} and {project} are two rows of `members:` whose " f"`project:` differs only in case, so `members/{first}` and " f"`members/{project}` are one directory on a case-insensitive " - "disk (macOS, Windows), where a holder can mount only one of " - "them. A family pins each project once.") + "disk (macOS, Windows), and a holder there can keep only one " + "of them. Whether the rows pin one project or two, one of " + "them has to go: keep the row whose `repository:` is the " + "member's and drop the other.") def _check_member_ids_unique(root: Path, members: list, diff --git a/tests/test_validate_family_duplicates.py b/tests/test_validate_family_duplicates.py index aa48dbf4..ac71aa92 100644 --- a/tests/test_validate_family_duplicates.py +++ b/tests/test_validate_family_duplicates.py @@ -15,7 +15,9 @@ The finding is now its own function, `_check_member_names_unique`, which reads `members:` alone and is called before the `--no-members` return, as `member-duplicate-id` (#206, `tests/test_family.py`) is. It is made ONCE, -whatever the mode, and an exact repeat keeps the text it always had. +whatever the mode, and an exact repeat keeps the text it always had. Case is +folded with `str.casefold()`, so a pair a disk holds as one directory and +`str.lower()` keeps apart (the long s, the sigmas) is found too. The same family and the same offline rules as `tests/test_family.py`: the fixture is imported rather than rebuilt, because a holder made any other way @@ -34,7 +36,7 @@ import pytest -from conftest import git +from conftest import git, run_script from test_family import ( # noqa: F401 ORG, SPARE, family, family_module, holder, manifest, tree_digest, validate, @@ -44,6 +46,23 @@ #: matches on text, so it is pinned here and not rebuilt from a template. EXACT = "FINDING member-duplicate: IRRS appears more than once in `members:`" +#: A commit nothing can resolve. A repeated row pinned to it is what makes the +#: members-checked loop refuse (`member-commit-unresolvable`, exit 2, and the +#: finding lost) if `_member_project_name` ever stops skipping a name it has +#: already seen: a repeat copied byte for byte checks out as cleanly as the +#: row it repeats, so it would never notice. +NOWHERE = "0" * 40 + +#: Pairs one case-insensitive disk holds as one directory and `str.lower()` +#: keeps apart, which only `str.casefold()` finds: IRRS and IRR with the long s +#: (U+017F, folds to `s`), and capital sigmas against small ones (`lower()` +#: gives the final sigma U+03C2 where the small spelling has U+03C3). Written as +#: escapes so no editor or console re-encodes them. +FOLDS = { + "long-s": ("IRRS", "irr\u017f"), + "sigma": ("\u03a3\u0391\u03a3", "\u03c3\u03b1\u03c3"), +} + def irrs_row(holder) -> dict: """IRRS's row, as the manifest reads it back.""" @@ -102,18 +121,30 @@ def test_a_row_written_twice_is_found_under_no_members(holder): @pytest.mark.parametrize("extra", [(), ("--pins",)], ids=["members-checked", "pins"]) -def test_with_members_checked_a_repeat_is_one_finding_not_two(holder, extra): +@pytest.mark.parametrize("repeat", ["identical", "other-pin"]) +def test_with_members_checked_a_repeat_is_one_finding_not_two(holder, extra, + repeat): """`_member_project_name` used to make this finding and still stops that row's checks, silently. A finding made in both places would be one fault - reported twice, and the count is what says so.""" + reported twice, and the count is what says so. + + THE REPEAT IS ALSO PINNED TO A COMMIT NOWHERE (`other-pin`), because a + repeat that is byte for byte the row it repeats passes every check the + first one did, so deleting the skip would cost it nothing: it is the row + that would be checked, refused `member-commit-unresolvable` (exit 2) and + lose the finding, that holds the skip in place. + """ row = irrs_row(holder) + again = (dict(row) if repeat == "identical" else + {**row, "pin": {**row["pin"], "commit": NOWHERE}}) other = next(r for r in manifest(holder)["members"] if r["project"] == "IRSS") - rewrite(holder, [row, dict(row), other]) + rewrite(holder, [row, again, other]) result = validate(holder, *extra) assert result.returncode == 1, result.stderr + result.stdout assert findings(result) == [EXACT], result.stderr assert result.stderr.count("member-duplicate") == 1, result.stderr + assert "REFUSED" not in result.stderr, result.stderr # --- two rows that differ only in case --------------------------------------- @@ -149,6 +180,49 @@ def test_a_case_variant_is_not_the_exact_finding(holder): assert "appears more than once" not in result.stderr, result.stderr +@pytest.mark.parametrize("pair", FOLDS.values(), ids=FOLDS.keys()) +def test_rows_that_a_disk_folds_but_lower_does_not_are_found_under_no_members( + holder, pair): + """`--no-members` reads any string as a `project:` (the checks that hold a + name to ASCII run only with members checked), so a hand edit can write a + pair that `str.lower()` keeps apart and a case-insensitive disk does not. + The fold is `casefold()`, as the standard's other same-path-in-any-case + checks use. The child is run in UTF-8 mode so the names print as written + on every platform.""" + first, second = pair + assert first.lower() != second.lower(), "the point is that lower() fails" + base = irrs_row(holder) + rewrite(holder, [spelled(base, first, "irrs-first"), + spelled(base, second, "irrs-second")]) + result = run_script(holder / "scripts" / "validate-family.py", + "--no-members", cwd=holder, + env={"PYTHONUTF8": "1"}) + assert result.returncode == 1, result.stderr + result.stdout + assert len(findings(result)) == 1, result.stderr + assert findings(result)[0].startswith(case_pair(first, second)), \ + result.stderr + + +def test_the_case_finding_does_not_assume_one_project(holder): + """Rows with distinct ids may be two projects (`OrgA/IRRS` and + `OrgB/irrs`): the fault is one DIRECTORY, not one project pinned twice, so + the finding says so and says one row has to go. It names no command, + because `family.py remove` cannot take out a row that was never mounted, + which is the only kind a macOS or Windows holder can hold.""" + base = irrs_row(holder) + rewrite(holder, [base, spelled(base, "irrs", "irrs-twin")]) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + (line,) = findings(result) + assert line.startswith(case_pair("IRRS", "irrs")), line + assert ("and a holder there can keep only one of them. Whether the rows " + "pin one project or two, one of them has to go: keep the row " + "whose `repository:` is the member's and drop the other." + ) in line, line + assert "pins each project once" not in result.stderr + assert "family.py" not in line + + def test_each_later_row_is_one_finding_against_the_first_that_holds_its_name( holder): """Three spellings of one name and one of them twice: every row after the