diff --git a/docs/cli.md b/docs/cli.md index 79e1f7c4..6d8007c9 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -249,9 +249,7 @@ options: -h, --help show this help message and exit --org ORG --project PROJECT the assembly-root name, e.g. Atlas - --id ID lowercase project id (default: --project lowercased). - One the project's YAML reader would read as another - type -- `null`, `true`, `007` -- is refused. + --id ID lowercase project id (default: --project lowercased) --name NAME display name --visibility {private,public,internal} default: private for a fresh scaffold; with --reuse- @@ -282,9 +280,7 @@ options: exist yet. --tracking-branch TRACKING_BRANCH the branch every repository is created on (default: - main). One the project's YAML reader would read as - another type -- `null`, `true`, `007`, `2026` -- is - refused. + main) --spec-path SPEC_PATH where the spec leg is mounted, as a path in the assembly root (default: spec). A path that is, holds diff --git a/scaffold-project.py b/scaffold-project.py index 60b8b8ba..43ba56d6 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -39,13 +39,16 @@ WHAT THE NEW PROJECT MUST BE ABLE TO HOLD (#258, #261). The manifest is rendered from the values the run is about to use, read with the project's own YAML reader and put through the validator the template ships, BEFORE anything -is created and under `--dry-run` alike. A value the reader turns into something -else (`--id null`, `--tracking-branch true` or `007`: the template writes them -bare) is refused as `scaffold-manifest-misread`, and one the validator refuses -(a blank `--elected-by`, `--reference` or `--name`, an `--org` it cannot hold) -as `scaffold-manifest-invalid`, in the validator's own words; an -`--elected-on` is read in the validator's form only, on every Python. A name -that cannot be a directory's is refused as `scaffold-name-too-long`. +is created and under `--dry-run` alike. A value the reader would turn into +something else where a template line writes it bare is refused as +`scaffold-manifest-misread`. The template writes `id` and `tracking_branch` +between double quotes (#259), so `--id null` or `--tracking-branch 007` is +held as typed, and that refusal guards the template rather than the flags. A +value the validator refuses (a blank `--elected-by`, `--reference` or +`--name`, an `--org` it cannot hold) is refused as +`scaffold-manifest-invalid`, in the validator's own words; an `--elected-on` +is read in the validator's form only, on every Python. A name that cannot be a +directory's is refused as `scaffold-name-too-long`. IDEMPOTENCY. It refuses to write into a remote or a working directory that already exists. There is no --force: re-running over a live project is not a @@ -202,9 +205,7 @@ def main(argv: list[str] | None = None) -> int: help="the assembly-root name, e.g. Atlas") parser.add_argument("--id", default=None, help="lowercase project id (default: --project " - "lowercased). One the project's YAML reader " - "would read as another type -- `null`, `true`, " - "`007` -- is refused.") + "lowercased)") parser.add_argument("--name", default=None, help="display name") parser.add_argument("--visibility", choices=VISIBILITY_CHOICES, default=None, @@ -237,9 +238,7 @@ def main(argv: list[str] | None = None) -> int: "that did not exist yet.") parser.add_argument("--tracking-branch", default="main", help="the branch every repository is created on " - "(default: main). One the project's YAML reader " - "would read as another type -- `null`, `true`, " - "`007`, `2026` -- is refused.") + "(default: main)") parser.add_argument("--spec-path", default="spec", help="where the spec leg is mounted, as a path in " "the assembly root (default: spec). A path that " @@ -425,11 +424,13 @@ def _refuse_unholdable_manifest(args, values: dict, policy) -> None: `values` is the whole substitution table, so the project's own YAML reader and `validate-manifest.py` are asked about the `project.yaml` the - run is about to write: a value the reader turns into something else - (`--id null`, `--tracking-branch 007`) is `scaffold-manifest-misread`, and - one the validator refuses (a blank `--elected-by`, `--org test+org`) is - `scaffold-manifest-invalid`. `shape_materialize.refuse_manifest_values` - asks both, and says why each is refused. + run is about to write: a value the reader would turn into something else + where a template line writes it bare is `scaffold-manifest-misread` (the + template quotes `id` and `tracking_branch`, #259, so `--id null` and + `--tracking-branch 007` are held as typed), and one the validator refuses + (a blank `--elected-by`, `--org test+org`) is `scaffold-manifest-invalid`. + `shape_materialize.refuse_manifest_values` asks both, and says why each is + refused. A value a flag did not supply is named by where it came from, so the sentence does not point at a flag nobody typed. diff --git a/scripts/family.py b/scripts/family.py index d359f749..0017c3e1 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -132,8 +132,9 @@ from repo_shape import ( # noqa: E402 free_plan_secret_hint, COMMIT_RE, PROJECT_ID_RE, PYTHON, TREE_DIGEST_DEFINITION, - VISIBILITY_CHOICES, NamingPolicy, Refusal, checked_value, git_out, - load_yaml, recorded_gitlink, repo_basename, same_repository, tree_digest, + VISIBILITY_CHOICES, NamingPolicy, Refusal, YamlError, checked_value, + double_quoted, git_out, load_yaml, parse_yaml, recorded_gitlink, + repo_basename, same_repository, tree_digest, ) from shape_materialize import ( # noqa: E402 RULESET_HINT, SHAPE_REPOSITORY, CommandFailed, check_program, @@ -320,12 +321,46 @@ def rewritten(self, rows: list[dict]) -> str: return "\n".join(out) + "\n" +def _member_id(value) -> str: + """A row's `id:` as `_member_lines` spells it: bare where the holder's + reader takes the bare text back as that same text, and between double + quotes where it does not (#259). + + THE MEMBER'S OWN `project.yaml` QUOTES ITS ID NOW. A project scaffolded + with `--id 007` used to write `id: 007`, which every reader took as the + number 7; this row then recorded `str(7)`, and the two agreed on a + misread. The template writes `id: "007"` since #259, so the id read here + is `007`, and written bare it would come back as 7: `validate-family.py` + would report `member-id-mismatch` for a member it had just pinned, and so + would `2026`, `20261011`, `true` and `null`. The reader is asked, not a + list of the words it treats as something else, so a word it learns later + is quoted with no edit here. An id that reads back bare, which is every + ordinary one, is written as it always was, so a row whose id reads back + as text does not move. + + A ROW WHOSE BARE ID THE READER MISREADS MOVES, ONCE. A holder written + before #259 holds `id: 2026` for a member scaffolded with `--id 2026`, + and the next `bump`, `add` or `remove` of that holder rewrites the row as + `id: "2026"`. That is harmless: `validate-family.py` compares the id with + `str()`, and passes before and after. + """ + text = str(value) + try: + if parse_yaml(f"id: {text}\n") == {"id": text}: + return text + except YamlError: + # Bare, `[`, `{`, `&a`, `*a`, `!t` and an unclosed quote are not read + # as another type: the reader raises, and the holder would not load. + pass + return double_quoted(text) + + def _member_lines(row: dict) -> list[str]: """One member as the YAML subset `repo_shape.parse_yaml` reads back.""" pin = row.get("pin") or {} return [ f" - project: {row['project']}", - f" id: {row['id']}", + f" id: {_member_id(row['id'])}", f" repository: {row['repository']}", f" path: {row['path']}", " pin:", diff --git a/templates/assembly-root/contracts/code-pin.yaml b/templates/assembly-root/contracts/code-pin.yaml index aedb47cf..f498b7b2 100644 --- a/templates/assembly-root/contracts/code-pin.yaml +++ b/templates/assembly-root/contracts/code-pin.yaml @@ -19,7 +19,7 @@ kind: pinned_contract_manifest leg_role: code source_repository: {{CODE_REPOSITORY}} -submodule_path: {{CODE_PATH}} +submodule_path: "{{CODE_PATH}}" commit: "{{CODE_COMMIT}}" revision_kind: commit diff --git a/templates/assembly-root/contracts/spec-pin.yaml b/templates/assembly-root/contracts/spec-pin.yaml index 796ecd2d..418dd63a 100644 --- a/templates/assembly-root/contracts/spec-pin.yaml +++ b/templates/assembly-root/contracts/spec-pin.yaml @@ -19,7 +19,7 @@ kind: pinned_contract_manifest leg_role: spec source_repository: {{SPEC_REPOSITORY}} -submodule_path: {{SPEC_PATH}} +submodule_path: "{{SPEC_PATH}}" commit: "{{SPEC_COMMIT}}" revision_kind: commit diff --git a/templates/assembly-root/project.yaml b/templates/assembly-root/project.yaml index 75888d19..c4a4b0e0 100644 --- a/templates/assembly-root/project.yaml +++ b/templates/assembly-root/project.yaml @@ -28,7 +28,7 @@ kind: project-manifest # identically and owes no declaration. # =========================================================================== -id: {{PROJECT_ID}} +id: "{{PROJECT_ID}}" name: "{{PROJECT_NAME}}" # The elected schema, and the document the election followed. For a project @@ -44,7 +44,7 @@ reference: "{{REFERENCE}}" # Electing the shape is a human's act (a `PA` decision, `CA`-constrained and # `PM`-sequenced), so the manifest records whose act it was and when. elected_by: "{{ELECTED_BY}}" -elected_on: {{ELECTED_ON}} +elected_on: "{{ELECTED_ON}}" # The GitHub topic every one of this project's repositories carries. Names # carry the human eye, topics carry the organisation's search, and the @@ -59,12 +59,12 @@ topic: {{TOPIC}} # in-place adoption it is the value the two NEW legs were created with — the # assembly root IS the adopted source and keeps whatever visibility it # already had on GitHub, unchanged by this tool. -visibility: {{VISIBILITY}} +visibility: "{{VISIBILITY}}" # The branch `scripts/bootstrap.py` places each leg on, AT its pinned commit. # The pin stays exact: `.gitmodules` carries no `branch=`, so there is exactly # one authoritative answer to "what commit is this project". -tracking_branch: {{TRACKING_BRANCH}} +tracking_branch: "{{TRACKING_BRANCH}}" # The openRepoShape revision this project was scaffolded from, pinned by # commit and digest under the same rule the family's neutral products use: a @@ -156,9 +156,9 @@ legs: {{ASSEMBLY_NAMING}} - role: spec repository: {{SPEC_REPOSITORY}} - path: {{SPEC_PATH}} + path: "{{SPEC_PATH}}" {{SPEC_NAMING}} - role: code repository: {{CODE_REPOSITORY}} - path: {{CODE_PATH}} + path: "{{CODE_PATH}}" {{CODE_NAMING}} diff --git a/templates/family-root/family.yaml b/templates/family-root/family.yaml index 3600b9f8..0aff4ac8 100644 --- a/templates/family-root/family.yaml +++ b/templates/family-root/family.yaml @@ -30,25 +30,25 @@ kind: family-manifest # this file assert anything about a tree it does not own. # =========================================================================== -id: {{FAMILY_ID}} +id: "{{FAMILY_ID}}" name: "{{FAMILY_NAME}}" -org: {{ORG}} +org: "{{ORG}}" repository: {{FAMILY_REPOSITORY}} # Creating a family is a human's act, like electing the shape, so the manifest # records whose act it was and when. created_by: "{{CREATED_BY}}" -created_on: {{CREATED_ON}} +created_on: "{{CREATED_ON}}" # `private`, `public` or `internal`. NOT ENFORCED HERE — GitHub is the source # of truth for what a repository's visibility actually is — but recording it # is what lets a reader tell why the members need a credential in CI. -visibility: {{VISIBILITY}} +visibility: "{{VISIBILITY}}" # The branch `scripts/bootstrap.py` places each member on, AT its pinned # commit. `.gitmodules` carries no `branch=`: there is exactly one # authoritative answer to "what commit of this member is this family". -tracking_branch: {{TRACKING_BRANCH}} +tracking_branch: "{{TRACKING_BRANCH}}" # The directory every member is mounted under. One name, so a reader and a # validator cannot disagree about where to look. diff --git a/tests/test_adopt_github_exits.py b/tests/test_adopt_github_exits.py index 63725dcc..5c757f12 100644 --- a/tests/test_adopt_github_exits.py +++ b/tests/test_adopt_github_exits.py @@ -672,7 +672,7 @@ def test_a_source_on_another_branch_opens_its_pull_request_against_it( assert_pushed(github, branch) root = git("show", f"{SPLIT_BRANCH}:project.yaml", cwd=github / ORG / f"{NORTHWIND}.git").stdout - assert f"\ntracking_branch: {branch}\n" in root, root + assert f'\ntracking_branch: "{branch}"\n' in root, root assert "Base ref must be a branch" not in result.stderr diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index a573b122..cd45dbf9 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -109,10 +109,10 @@ def said_of(new: str) -> tuple[str, str, str]: #: `(old line, new line, what `check` names)`: the same values, edited into a #: plan by hand, which is what a plan is for. -#: A text value the reader reads as a number (`elected_on: "20261011"`) is -#: not here but in `TEXT_THE_READER_MISREADS` below: where a misread rule is -#: also in the list (#181) it is that rule's finding, and these rows name the -#: validator's, or `said_of`'s for the date. +#: A text value the reader read as a number while the template wrote it bare +#: (`elected_on: "20261011"`) is not here but in `TEXT_THE_VALIDATOR_REFUSES` +#: below, with the other values the template quotes now (#259). These rows +#: name the validator's finding, or `said_of`'s for the date. PLAN_EDITS = [ pytest.param("visibility: private", "visibility: secret", ("visibility is 'secret'", "manifest-visibility"), @@ -689,19 +689,31 @@ def test_a_value_the_reader_reads_as_another_kind_is_not_this_questions( assert FINDING not in result.stderr, result.stderr -# --- a text value the reader misreads: judged once, not twice --------------- +# --- a text value the template once wrote bare: judged once, not twice ------ +# +# Each value below is TEXT in the plan that the template wrote bare until #259, +# so the project's reader read it as a number or a boolean: the validator +# refused each in the retyped words (`id is 7`), and a misread rule (#181) +# would have refused the same value. The template quotes all three now, so the +# reader reads each back as typed, and no misread rule refuses any of them. +# What is left is the validator's own answer about the text itself, or, for +# `20261011`, a date `election_date` refuses, #294's (`said_of`). + +#: `(old line, new line)`: text the validator accepts once it reads it back as +#: typed, so `check` passes it and `execute` writes it into the manifest. +TEXT_THE_TEMPLATE_NOW_HOLDS = [ + pytest.param("id: northwind", 'id: "007"', id="id-007"), +] -#: `(old line, new line, what the validator says of it)`: TEXT in the plan, -#: which the template writes bare and the project's reader reads as a number -#: or a boolean. The validator refuses each; where a misread rule is also in -#: the list (#181) it refuses the same value, and the sentence should be one. -#: `20261011` is also a date `election_date` refuses, so its one sentence is -#: #294's (`said_of`), and the validator is not asked about it. -TEXT_THE_READER_MISREADS = [ - pytest.param("id: northwind", 'id: "007"', "manifest-id: id is 7", - id="id-007"), +#: `(old line, new line, what `check` says of it)`: text the validator +#: refuses as the text it is, so it is still refused, in words about that +#: text: `visibility is 'true'`, no longer `visibility is True`. `20261011` is +#: also a date `election_date` refuses (by the validator's own `DATE_RE`), so +#: its one sentence is #294's (`said_of`), `the text '20261011'`, and the +#: validator is not asked about it. +TEXT_THE_VALIDATOR_REFUSES = [ pytest.param("visibility: private", 'visibility: "true"', - "manifest-visibility: visibility is True", + "manifest-visibility: visibility is 'true'", id="visibility-true"), pytest.param('elected_on: "2026-09-02"', 'elected_on: "20261011"', "the plan's `elected_on:` is the text '20261011'", @@ -709,12 +721,11 @@ def test_a_value_the_reader_reads_as_another_kind_is_not_this_questions( ] -@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) -def test_check_refuses_a_text_value_the_reader_misreads(world, old, new, - named): - """Whichever rule says it: the validator's finding here, or a misread - rule's (#181) where one is in the list. What is asked is that `check` - finds the value, and names it.""" +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_check_refuses_a_text_value_the_validator_refuses(world, old, new, + named): + """`check` finds the value, and names it, in the validator's words, or in + #294's for a date `election_date` refuses (`said_of`).""" plan = edited(world, old, new) result = run_script(ADOPT, "check", "--plan", str(plan)) assert result.returncode == 1, result.stderr + result.stdout @@ -723,28 +734,60 @@ def test_check_refuses_a_text_value_the_reader_misreads(world, old, new, if line.startswith("FINDING ")] assert findings, result.stderr assert any(value in f for f in findings), result.stderr + assert any(f.startswith(said_of(new)[0]) and named in f + for f in findings), result.stderr + + +@pytest.mark.parametrize("old, new", TEXT_THE_TEMPLATE_NOW_HOLDS) +def test_check_passes_a_text_value_the_template_now_holds(world, old, new): + """`id: "007"` was the number 7 to the reader while the template wrote + `id:` bare, and the validator found `manifest-id: id is 7`. Quoted, it is + the id `007`, which the validator accepts, so the plan checks clean.""" + plan = edited(world, old, new) + result = run_script(ADOPT, "check", "--plan", str(plan)) + assert result.returncode == 0, result.stderr + result.stdout + assert "plan ok" in result.stdout + assert "FINDING " not in result.stderr @needs_filter_repo -@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) -def test_execute_refuses_a_text_value_the_reader_misreads_before_any_leg( +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_execute_refuses_a_text_value_the_validator_refuses_before_any_leg( world, old, new, named): plan = edited(world, old, new) result = execute(world, plan) assert result.returncode == 2, result.stderr + result.stdout assert said_of(new)[1] in result.stderr assert new.split(": ", 1)[1].strip('"') in result.stderr + assert named in result.stderr assert "adoption verified" not in result.stdout assert "Traceback" not in result.stderr no_leg_was_created(world) -@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) -def test_a_text_value_the_reader_misreads_is_one_finding_on_this_rule_alone( +@needs_filter_repo +@pytest.mark.parametrize("old, new", TEXT_THE_TEMPLATE_NOW_HOLDS) +def test_execute_writes_a_text_value_the_template_now_holds_as_typed( + adopter, world, old, new): + """`execute` refused this before any leg while the template wrote it + bare. It adopts it now, and the manifest it commits holds the text that + was in the plan.""" + plan = edited(world, old, new) + result = execute(world, plan) + assert result.returncode == 0, result.stderr + result.stdout + assert "adoption verified" in result.stdout + key, value = new.split(": ", 1) + shown = git("show", f"{adopter.ADOPT_BRANCH}:project.yaml", + cwd=world["source"]).stdout + assert repo_shape.parse_yaml(shown)[key] == value.strip('"') + + +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_a_text_value_the_validator_refuses_is_one_finding_on_this_rule_alone( adopter, world, old, new, named): - """With nothing else asking the misread question the validator's finding - is the only one, so `check` and `execute` still refuse the value: it must - not be dropped for want of a rule that is not here. A date `election_date` + """With nothing else asking about the value the validator's finding is the + only one, so `check` and `execute` still refuse the value: it must not be + dropped for want of a rule that is not here. A date `election_date` refuses is the one finding of #294's rule, which the list always holds, and the validator is not asked about it as well.""" plan = adopter.Plan.load(edited(world, old, new)) @@ -759,23 +802,48 @@ def test_a_text_value_the_reader_misreads_is_one_finding_on_this_rule_alone( assert remediation -@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) -def test_where_a_misread_rule_judges_it_the_validator_is_not_asked_again( - adopter, world, old, new, named): - """`misreads_elsewhere`: a list that holds the misread rule's finding for - such a value (#181's `plan-manifest-misread`) is not given a second one - about the same value, in the validator's words and with the retyped text - (`id is 7`) that the misread finding exists to avoid.""" +@pytest.mark.parametrize("old, new", TEXT_THE_TEMPLATE_NOW_HOLDS) +def test_a_text_value_the_template_now_holds_is_no_finding_either_way( + adopter, world, old, new): + """The validator accepts it, and the reader reads it back, so it is no + finding on this rule alone, and none where a misread rule judges the rest + (`misreads_elsewhere`).""" plan = adopter.Plan.load(edited(world, old, new)) + assert adopter._manifest_findings(plan) == ([], []) assert adopter._manifest_findings(plan, misreads_elsewhere=True) == ( [], []) +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_where_a_misread_rule_judges_the_rest_the_validator_still_answers( + adopter, world, old, new, named): + """`misreads_elsewhere` leaves out only what the project's reader would + misread, for a list that holds a misread rule's finding about it (#181's + `plan-manifest-misread`). While the template wrote these bare that was + each of them, and the validator was not asked again. It quotes them now + (#259): the reader reads each back as typed, no misread rule refuses it, + and the validator's finding, in words about the text, is the one with or + without such a rule in the list. A date `election_date` refuses is #294's + one finding either way, and the validator is not asked about it.""" + plan = adopter.Plan.load(edited(world, old, new)) + shared = adopter._manifest_findings(plan, misreads_elsewhere=True) + assert shared == adopter._manifest_findings(plan) + if said_of(new)[0] != FINDING: + assert shared == ([], []) + shared = adopter._plan_field_findings(plan) + (finding,), remediation = shared + assert finding.startswith(said_of(new)[0]) + assert named in finding + assert remediation + + def test_a_value_that_is_not_a_misread_is_still_asked_where_one_judges_the_rest( adopter, world): """Only what the reader misreads is left out. `visibility: secret` is text to the reader and the validator refuses it, so it is still a finding, and - so is a blank `elected_by`; `id: "007"` beside them is not.""" + so is a blank `elected_by`. `id: "007"` beside them was a misread while the + template wrote `id:` bare; quoted (#259) it is the id `007`, and no finding + on either list.""" text = world["text"].replace("id: northwind", 'id: "007"') text = text.replace("visibility: private", "visibility: secret") text = text.replace('elected_by: "Test Human"', 'elected_by: " "') @@ -785,21 +853,24 @@ def test_a_value_that_is_not_a_misread_is_still_asked_where_one_judges_the_rest( plan = adopter.Plan.load(path) alone, _ = adopter._manifest_findings(plan) shared, _ = adopter._manifest_findings(plan, misreads_elsewhere=True) - assert len(alone) == 3, alone - assert len(shared) == 2, shared + assert len(alone) == 2, alone + assert shared == alone assert any("visibility is 'secret'" in f for f in shared) assert any("elected_by is ' '" in f for f in shared) - assert not any("manifest-id" in f for f in shared) + assert not any("manifest-id" in f for f in alone) -@pytest.mark.parametrize("old, new, named", TEXT_THE_READER_MISREADS) -def test_a_text_value_the_reader_misreads_is_one_finding_in_the_list_check_prints( +@pytest.mark.parametrize("old, new, named", TEXT_THE_VALIDATOR_REFUSES) +def test_a_text_value_the_validator_refuses_is_one_finding_in_the_list_check_prints( adopter, world, old, new, named): """NOT ZERO AND NOT TWO, whatever rules the list holds. This rule alone, `_entry_findings` does not pass `misreads_elsewhere`, and the validator's - finding is the one. With #181 in the list as well, the call there must pass - it, or this is the pair the two rules each make of one value: this test - fails on a merge that has not.""" + finding is the one (#294's, for a date `election_date` refuses, which the + validator is then not asked about). A misread rule in the list as well + (#181) reads the template, which writes these values quoted (#259), so it + has nothing to say of them; one that said something would be the pair two + rules each make of one value, and this test fails on a merge that has + it.""" plan = adopter.Plan.load(edited(world, old, new)) source = plan.open_source(str(world["source"]), world["tmp"] / "read") findings, _, _ = adopter._entry_findings(plan, source) @@ -808,6 +879,17 @@ def test_a_text_value_the_reader_misreads_is_one_finding_in_the_list_check_print assert len(about) == 1, findings +@pytest.mark.parametrize("old, new", TEXT_THE_TEMPLATE_NOW_HOLDS) +def test_a_text_value_the_template_now_holds_is_no_finding_in_the_list_check_prints( + adopter, world, old, new): + """Not one finding but none: the whole list `check` prints is empty for a + plan whose only edit is a value the validator accepts.""" + plan = adopter.Plan.load(edited(world, old, new)) + source = plan.open_source(str(world["source"]), world["tmp"] / "read") + findings, _, _ = adopter._entry_findings(plan, source) + assert findings == [], findings + + # --- a validator that cannot be asked --------------------------------------- def a_copy_of_the_standard(base: Path, remake) -> Path: diff --git a/tests/test_bootstrap_after_bump.py b/tests/test_bootstrap_after_bump.py index 4f3da5cb..78063c4c 100644 --- a/tests/test_bootstrap_after_bump.py +++ b/tests/test_bootstrap_after_bump.py @@ -435,8 +435,8 @@ def test_the_branch_is_spelled_in_the_lines_that_name_it(root): `tests/test_bootstrap_spelling.py`: they are not what is asked.""" manifest = root / "project.yaml" text = manifest.read_bytes() - assert text.count(b"tracking_branch: main\n") == 1, text - manifest.write_bytes(text.replace(b"tracking_branch: main\n", + assert text.count(b'tracking_branch: "main"\n') == 1, text + manifest.write_bytes(text.replace(b'tracking_branch: "main"\n', b'tracking_branch: "tb\\u2028x"\n')) named = r"tb\u2028x" leg = root / "spec" @@ -476,8 +476,8 @@ def test_the_branch_and_its_upstream_are_spelled_in_the_move_back(root): tracking that ref, and the leg detached at the pin.""" manifest = root / "project.yaml" text = manifest.read_bytes() - assert text.count(b"tracking_branch: main\n") == 1, text - manifest.write_bytes(text.replace(b"tracking_branch: main\n", + assert text.count(b'tracking_branch: "main"\n') == 1, text + manifest.write_bytes(text.replace(b'tracking_branch: "main"\n', b'tracking_branch: "tb\\u2028x"\n')) raw = "tb\u2028x" leg = root / "spec" @@ -700,8 +700,9 @@ def test_the_command_that_brings_the_branch_forward_is_shell_quoted( git("mv", "spec", "my spec", cwd=fresh) manifest = fresh / "project.yaml" text = manifest.read_bytes() - assert text.count(b"path: spec\n") == 1, text - manifest.write_bytes(text.replace(b"path: spec\n", b'path: "my spec"\n')) + assert text.count(b'path: "spec"\n') == 1, text + manifest.write_bytes(text.replace(b'path: "spec"\n', + b'path: "my spec"\n')) leg = fresh / "my spec" pin = rev(leg) ahead = leg_main_ahead["ahead"] diff --git a/tests/test_bootstrap_spelling.py b/tests/test_bootstrap_spelling.py index 9c65ccf0..6cf55d0f 100644 --- a/tests/test_bootstrap_spelling.py +++ b/tests/test_bootstrap_spelling.py @@ -369,7 +369,7 @@ def test_a_legs_path_is_spelled_where_it_is_not_initialized( """A path the manifest holds that is no directory: step (a) says so, and says what to run, before any validator has had its say. A lone surrogate here raised from the `print`.""" - edit(project, "project.yaml", " path: spec", f" path: {path}") + edit(project, "project.yaml", ' path: "spec"', f" path: {path}") code, out, err = run(bootstrap_of(project), "--skip-validators", encoding="utf-8:strict", tmp_path=tmp_path, cwd=project) @@ -383,7 +383,7 @@ def test_a_legs_path_is_spelled_where_it_is_not_initialized( def test_a_tracking_branch_git_will_not_make_is_spelled(project, tmp_path): """Passes on origin/main too, and is here to keep it: that line prints the branch with `!r`, and a repr has always escaped a control.""" - edit(project, "project.yaml", "tracking_branch: main", + edit(project, "project.yaml", 'tracking_branch: "main"', r'tracking_branch: "a\x1bb"') code, out, err = run(bootstrap_of(project), "--skip-validators", encoding="utf-8:strict", tmp_path=tmp_path, @@ -401,7 +401,7 @@ def test_a_branch_name_is_spelled_in_every_line_that_names_it(project, """A branch git accepts and a terminal should not be handed raw: U+2028. The lines that name one: created (and where the remote's is), on, and the disagreement.""" - edit(project, "project.yaml", "tracking_branch: main", + edit(project, "project.yaml", 'tracking_branch: "main"', r'tracking_branch: "tb\u2028x"') named = r"tb\u2028x" spec = project / "spec" @@ -476,7 +476,8 @@ def test_a_register_and_its_path_are_spelled_when_it_cannot_be_read( separator, which does not parse: its path is on the `register:` line and again, with the reader's reason, on the `unreadable` one. Under a stdout that HOLDS a line separator, so a raw one would be written raw.""" - edit(project, "project.yaml", " path: spec", r' path: "sp\u2028ec"') + edit(project, "project.yaml", ' path: "spec"', + r' path: "sp\u2028ec"') register = (project / "sp\u2028ec" / "governance" / "review-authority" / "register.yaml") register.parent.mkdir(parents=True) diff --git a/tests/test_family.py b/tests/test_family.py index b25dbc29..d95d1c7c 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -1996,7 +1996,8 @@ def test_siblings_skips_a_clone_whose_project_id_is_not_the_rows(estate): sibling = estate["folder"] / "IRSS" manifest_path = sibling / "project.yaml" manifest_path.write_text( - manifest_path.read_text().replace("id: irss", "id: something-else", 1)) + manifest_path.read_text().replace('id: "irss"', 'id: "something-else"', + 1)) result = siblings(estate["holder"]) assert result.returncode == 1, result.stdout + result.stderr assert "WRONG PROJECT" in result.stdout diff --git a/tests/test_family_add_member_id.py b/tests/test_family_add_member_id.py index 88033bc1..5393473d 100644 --- a/tests/test_family_add_member_id.py +++ b/tests/test_family_add_member_id.py @@ -246,7 +246,11 @@ def renumbered(*args, **kwargs): git("clone", "-q", str(remotes / f"{SPARE}.git"), str(work), cwd=tmp_path) text = (work / "project.yaml").read_bytes() - changed = re.sub(rb"(?m)^id: irqs$", b"id: irrs", text) + # The template writes the id quoted (`id: "irqs"`, #259), and a + # hand-written manifest writes it bare: match either, and write the + # new id back in the quoting the old one had, so the member still + # loads and this still fails loudly if the line ever stops matching. + changed = re.sub(rb'(?m)^id: ("?)irqs\1$', rb"id: \1irrs\1", text) assert changed != text (work / "project.yaml").write_bytes(changed) git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", diff --git a/tests/test_manifest_validator.py b/tests/test_manifest_validator.py index 15ec8cd9..028c8d02 100644 --- a/tests/test_manifest_validator.py +++ b/tests/test_manifest_validator.py @@ -54,7 +54,8 @@ def test_a_topic_that_is_not_derived_from_the_id_is_a_finding(project): def test_a_missing_role_is_a_finding(project): edit(project, - ' - role: code\n repository: testorg/Atlas-code\n path: code\n', + ' - role: code\n repository: testorg/Atlas-code\n' + ' path: "code"\n', "") result = validate(project) assert result.returncode == 1 @@ -116,7 +117,7 @@ def test_an_election_with_no_elector_is_a_finding(project): def test_a_bad_id_is_a_finding(project): - edit(project, "id: atlas", "id: Atlas") + edit(project, 'id: "atlas"', 'id: "Atlas"') result = validate(project) assert result.returncode == 1 assert "manifest-id" in result.stderr @@ -234,8 +235,8 @@ def test_a_declared_descendant_may_be_the_assembly_root(project): def test_the_pin_file_makes_the_referent_real(project): edit(project, "neutral_product_pins: []", "neutral_product_pins:\n - openChart") - edit(project, "repository: testorg/Atlas-code\n path: code", - "repository: testorg/MedxChart\n path: code") + edit(project, 'repository: testorg/Atlas-code\n path: "code"', + 'repository: testorg/MedxChart\n path: "code"') edit(project, " form: project-leg\n role: code\n" " also_matches: []", " form: domain-descendant\n role: ~\n" @@ -270,8 +271,8 @@ def test_the_x_stem_spelling_of_a_referent_is_accepted_in_a_manifest(project): the CANONICAL `openFactory` spelling for the same referent set.""" edit(project, "neutral_product_pins: []", "neutral_product_pins:\n - openxFactory") - edit(project, "repository: testorg/Atlas-code\n path: code", - "repository: testorg/codexFactory\n path: code") + edit(project, 'repository: testorg/Atlas-code\n path: "code"', + 'repository: testorg/codexFactory\n path: "code"') edit(project, " form: project-leg\n role: code\n" " also_matches: []", " form: domain-descendant\n role: ~\n" @@ -573,8 +574,8 @@ def test_a_neutral_product_root_that_drops_the_overlap_is_a_finding(project): def test_a_neutral_product_is_still_not_a_spec_or_code_leg(project): """The edge of the relaxation, checked where a manifest can actually express it: the admission is `assembly` alone.""" - edit(project, "repository: testorg/Atlas-code\n path: code", - "repository: testorg/openDox\n path: code") + edit(project, 'repository: testorg/Atlas-code\n path: "code"', + 'repository: testorg/openDox\n path: "code"') result = validate(project) assert result.returncode == 1 assert "naming-not-a-leg" in result.stderr diff --git a/tests/test_pin_validator.py b/tests/test_pin_validator.py index 70254fc3..cefcc200 100644 --- a/tests/test_pin_validator.py +++ b/tests/test_pin_validator.py @@ -237,8 +237,8 @@ def test_a_missing_shape_pin_refuses(project): def test_a_pin_path_that_disagrees_with_the_manifest_is_a_finding(project): - edit_pin(project, "spec-pin.yaml", "submodule_path: spec", - "submodule_path: specification") + edit_pin(project, "spec-pin.yaml", 'submodule_path: "spec"', + 'submodule_path: "specification"') result = validate(project) assert result.returncode == 1 assert "pin-path-mismatch" in result.stderr diff --git a/tests/test_reference_default.py b/tests/test_reference_default.py index 764727eb..37c6941b 100644 --- a/tests/test_reference_default.py +++ b/tests/test_reference_default.py @@ -99,7 +99,7 @@ def test_the_scaffold_writes_the_reference_the_election_date_chooses( anything is created, and says that the date is what chose it.""" result, manifest = scaffold(tmp_path, "--elected-on", elected_on) assert f'reference: "{expected}"' in manifest - assert f"elected_on: {elected_on}" in manifest + assert f'elected_on: "{elected_on}"' in manifest assert f"reference {expected}" in result.stdout assert "(chosen by the election date)" in result.stdout diff --git a/tests/test_scaffold_e2e.py b/tests/test_scaffold_e2e.py index 88e0868b..45d7ce17 100644 --- a/tests/test_scaffold_e2e.py +++ b/tests/test_scaffold_e2e.py @@ -82,7 +82,7 @@ def test_a_descendant_form_project_scaffolds_and_records_the_overlap(tmp_path): # a leg whose name does NOT match the claim form records an empty overlap assert (" - role: spec\n" " repository: MedxSoft/MedxScribe-spec\n" - " path: spec\n" + ' path: "spec"\n' " naming:\n" " form: project-leg\n" " role: spec\n" @@ -132,7 +132,7 @@ def test_a_neutral_product_elects_the_shape_and_carries_three_repositories(tmp_p # matches the neutral form not at all. " - role: spec\n" " repository: testorg/openDox-spec\n" - " path: spec\n" + ' path: "spec"\n' " naming:\n" " form: project-leg\n" " role: spec\n" diff --git a/tests/test_scaffold_manifest_values.py b/tests/test_scaffold_manifest_values.py index a1b9a020..f36a9bf5 100644 --- a/tests/test_scaffold_manifest_values.py +++ b/tests/test_scaffold_manifest_values.py @@ -1,13 +1,25 @@ # SPDX-License-Identifier: Apache-2.0 """A scaffold refuses a value the new project's manifest cannot hold (#258, #261). -`scaffold-project.py` writes `--id` and `--tracking-branch` into +`scaffold-project.py` wrote `--id` and `--tracking-branch` into `project.yaml` bare, so `--id null` and `--id true` scaffolded a project whose first bootstrap failed `manifest-id`, and `--tracking-branch true`, `007` or `null` pushed every repository on the branch as typed while bootstrap read the manifest back as `True`, `7` or `main` and put each leg on a local branch the remotes do not have, with no upstream. It exited 0 and said nothing (#258). +THE TEMPLATE NOW QUOTES THEM (#259). `project.yaml` writes `id`, +`elected_on`, `visibility` and `tracking_branch` between double quotes, so +the reader reads each of those values back as typed, and the misread question, +which is asked of the TEMPLATE, refuses none of them: `--id null` and +`--tracking-branch 007` scaffold, and the manifest holds `"null"` and `"007"`. +The question is still asked. A template line that writes such a value bare is +refused again, with no edit to the rule: the tests below show it on a copy of +this standard whose `project.yaml` writes those four lines bare, as it did +before #259. `--tracking-branch=~` now reads back as `~`, which git refuses +as a branch name; that is #283's, whose fix (PR #355) refuses it before +anything is created as `scaffold-bad-tracking-branch`. + It also accepted what the project's own `validate-manifest.py` refuses -- a blank `--elected-by`, `--reference` or `--name`, `--elected-on 2026-W41-6` or `20261011` (which `date.fromisoformat` reads from Python 3.11), `--org @@ -23,8 +35,8 @@ template ships -- not of a copy of either rule. NO NETWORK AND NO GITHUB. Every run uses `--local-remote-dir`, which makes -bare repositories on disk, or `--dry-run`, which makes nothing; the one test -that proves `gh` is never asked puts a fake one on PATH. +bare repositories on disk, or `--dry-run`, which makes nothing; the two tests +that ask whether `gh` is reached put a fake one on PATH. """ from __future__ import annotations @@ -68,10 +80,11 @@ IDENTITY = ["--org", ORG, "--project", "Atlas", "--elected-by", "Test Human", "--elected-on", "2026-09-02"] -#: `(flag, value, what the project's reader makes of it)`. Each is a legal git -#: branch name or a legal id, the reader takes it for something else, and the -#: manifest writes it bare. -MISREAD_VALUES = [ +#: `(flag, value, what the project's reader makes of it bare)`. Each is a +#: legal git branch name or a legal id that the reader takes for something +#: else where a line writes it bare, as `project.yaml` did before #259. The +#: template quotes both fields now, so the manifest holds each one as typed. +ONCE_MISREAD_VALUES = [ pytest.param("--id", "null", "no value at all", id="id-null"), pytest.param("--id", "true", "the boolean true", id="id-true"), pytest.param("--id", "false", "the boolean false", id="id-false"), @@ -102,6 +115,28 @@ pytest.param("--tracking-branch", "0", "the number 0", id="branch-0"), ] +#: `(flag, value)`: a value the misread rule refused while the template wrote +#: it bare (`~` is no value at all to the reader) that reads back as typed now, +#: and is still no name git gives a branch. Refusing it is not this file's +#: rule: it is #283's, which asks `git check-ref-format --branch` (PR #355, +#: on main as 6c2a3712) and refuses it as `scaffold-bad-tracking-branch` +#: before anything exists. Without that rule the scaffold passed it and died +#: at `git init -b ~`, which against GitHub is after `gh repo create` has made +#: all three repositories; this row was an expected failure until #355 landed. +NOT_A_BRANCH = [ + pytest.param("--tracking-branch", "~", id="branch-tilde"), +] + +#: The placeholder each flag of `ONCE_MISREAD_VALUES` fills. +PLACEHOLDER_OF = {"--id": "PROJECT_ID", "--tracking-branch": "TRACKING_BRANCH"} + +#: The `project.yaml` lines #259 quoted that this file's rules name, as +#: `(key, placeholder)`. `written_bare` writes them bare again, as the +#: template did before #259. +QUOTED_BY_259 = (("id", "PROJECT_ID"), ("elected_on", "ELECTED_ON"), + ("visibility", "VISIBILITY"), + ("tracking_branch", "TRACKING_BRANCH")) + #: Values the reader takes for text, so the manifest holds what was typed. #: `yes` and `on` are YAML 1.1 booleans and this reader reads them as text; #: `0x1F` and `1e3` are numbers elsewhere and text here. They are not @@ -170,8 +205,9 @@ def adopter(): def scaffold(tmp_path: Path, *args: str, work: bool = False, - local: bool = True): - """One scaffold of `Atlas` into local bare remotes under `tmp_path`. + local: bool = True, tool: Path = SCAFFOLD): + """One scaffold of `Atlas` into local bare remotes under `tmp_path`, by + this standard's `scaffold-project.py` or, as `tool`, a copy's. Without `work` no `--work-dir` is given, so the tool would make its own in a temporary directory; `TMPDIR` (and Windows' `TEMP` and `TMP`) point at @@ -185,16 +221,17 @@ def scaffold(tmp_path: Path, *args: str, work: bool = False, flags += ["--local-remote-dir", str(tmp_path / REMOTES)] if work: flags += ["--work-dir", str(tmp_path / WORK)] - return run_script(SCAFFOLD, *flags, env={ + return run_script(tool, *flags, env={ "TMPDIR": str(scratch), "TMP": str(scratch), "TEMP": str(scratch)}) def assert_nothing_was_created(tmp_path: Path) -> None: """No bare remote, no work directory of the tool's own and none of ours.""" found = sorted(p.name for p in tmp_path.iterdir()) - assert found == [SCRATCH], f"a refused scaffold created something: {found}" + assert found == [SCRATCH], ( + f"a scaffold that makes nothing created something: {found}") assert list((tmp_path / SCRATCH).iterdir()) == [], ( - "a refused scaffold made a temporary work directory") + "a scaffold that makes nothing made a temporary work directory") def assert_refused(result, code: str) -> None: @@ -203,29 +240,103 @@ def assert_refused(result, code: str) -> None: assert result.stdout == "", "a plan was printed for a refused scaffold" -# --- #258: a value the project's reader turns into something else --------- +def manifest_in(tmp_path: Path, project: str = "Atlas") -> dict: + """The `project.yaml` a scaffold under `tmp_path` pushed, as git holds it + in the bare remote, read by the project's own reader.""" + bare = tmp_path / REMOTES / f"{project}.git" + return parse_yaml(git("show", "HEAD:project.yaml", cwd=bare).stdout) + -@pytest.mark.parametrize("flag, value, reading", MISREAD_VALUES) -def test_a_value_the_reader_misreads_is_refused_before_anything_is_made( - tmp_path, flag, value, reading): +def heads(bare: Path) -> list[str]: + """The branches a bare remote holds.""" + return git("for-each-ref", "--format=%(refname:short)", "refs/heads", + cwd=bare).stdout.split() + + +def written_bare(text: str) -> str: + """`project.yaml`'s text with the lines #259 quoted (`QUOTED_BY_259`) + written bare again, as the template wrote them before it.""" + for key, placeholder in QUOTED_BY_259: + quoted = f'\n{key}: "{{{{{placeholder}}}}}"\n' + assert text.count(quoted) == 1, f"{quoted!r} is not one line of it" + text = text.replace(quoted, f"\n{key}: {{{{{placeholder}}}}}\n") + return text + + +@pytest.fixture(scope="module") +def bare_standard(tmp_path_factory): + """This standard as its own checkout, with `project.yaml` writing the + four lines #259 quoted bare again. The misread rule is asked of the + template, so there it refuses what it refused before #259, and its tools + run from there to show it.""" + return a_copy_of_the_standard(tmp_path_factory.mktemp("bare"), + written_bare, "project.yaml") + + +def bare_template(standard: Path) -> Path: + """The assembly-root template of a copy of the standard.""" + return standard / "templates" / "assembly-root" + + +# --- #258: a value the project's reader turned into something else -------- + +@pytest.mark.parametrize("flag, value, reading", ONCE_MISREAD_VALUES) +def test_a_value_the_reader_once_misread_is_held_as_typed( + tmp_path, bare_standard, flag, value, reading): + """The misread rule refused each of these while `project.yaml` wrote it + bare, and it still refuses each where a template line does (asked here of + the copy of the standard that writes them bare). The template quotes them + now (#259), so this standard scaffolds each one, the manifest it pushes + holds the value as the text that was typed, and a branch is the one every + repository is created on.""" + placeholder = PLACEHOLDER_OF[flag] + (sentence,) = shape_materialize.manifest_misreads( + {placeholder: value}, {placeholder: flag}, + bare_template(bare_standard)) + assert f"{flag} is {value!r}" in sentence + assert f"reads it back as {reading} and not as that text" in sentence + assert shape_materialize.manifest_misreads( + {placeholder: value}, {placeholder: flag}) == [] # `=` form, so a value that begins with `~` or `-` reaches the tool as a # value and argparse does not take it for another option. result = scaffold(tmp_path, f"{flag}={value}") - assert_refused(result, MISREAD) - assert f"{flag} is {value!r}" in result.stderr - assert f"reads it back as {reading} and not as that text" in result.stderr - assert "Remediation: choose another value" in result.stderr + assert result.returncode == 0, result.stderr + result.stdout + manifest = manifest_in(tmp_path) + if flag == "--id": + assert manifest["id"] == value + assert manifest["topic"] == f"xf-project-{value}" + else: + assert manifest["tracking_branch"] == value + for name in ("Atlas", "Atlas-spec", "Atlas-code"): + assert heads(tmp_path / REMOTES / f"{name}.git") == [value], name + + +@pytest.mark.parametrize("flag, value", NOT_A_BRANCH) +def test_a_value_git_refuses_as_a_branch_is_refused_before_anything_is_made( + tmp_path, flag, value): + """No longer a misread, and still not a branch: it must be refused before + anything exists, by the rule that asks git (#283), and not die at the + first `git init -b` as `a git or gh command failed`.""" + result = scaffold(tmp_path, f"{flag}={value}") + assert_refused(result, "REFUSED scaffold-bad-tracking-branch: ") + assert "a git or gh command failed" not in result.stderr + assert flag in result.stderr assert_nothing_was_created(tmp_path) @pytest.mark.parametrize("flag, value", [("--id", "null"), ("--tracking-branch", "007")]) -def test_a_dry_run_refuses_a_misread_value_as_the_real_run_would( +def test_a_dry_run_accepts_a_value_once_misread_as_the_real_run_does( tmp_path, flag, value): - """`--dry-run` is a rehearsal: it must not print a plan for a scaffold the - real run refuses (the issue's own `--dry-run` accepted every one).""" + """`--dry-run` is a rehearsal, and it answers as the real run does. It + refused these as the real run did while the template wrote them bare; the + template quotes them now (#259), so it prints the plan the real run + carries out, and makes nothing.""" result = scaffold(tmp_path, "--dry-run", f"{flag}={value}") - assert_refused(result, MISREAD) + assert result.returncode == 0, result.stderr + result.stdout + assert "--dry-run: nothing was created." in result.stdout + if flag == "--id": + assert f"Atlas ({value})" in result.stdout assert_nothing_was_created(tmp_path) @@ -234,9 +345,9 @@ def test_a_dry_run_refuses_a_misread_value_as_the_real_run_would( ("--tracking-branch", "007"), ("--id", "2026"), ("--id", "null"), ]) def test_the_remediation_says_what_is_true_of_every_value_it_refuses( - tmp_path, flag, value): + tmp_path, bare_standard, flag, value): """`--tracking-branch 2026` and `1.5` are the values this refusal newly - takes from people: they scaffolded, bootstrapped and got their upstream, + took from people: they scaffolded, bootstrapped and got their upstream, because every reader of `tracking_branch` does `str()` and `str(2026)` is `2026` again. The remediation once said the first bootstrap would fail or put the legs on a branch the remotes lack, which is false of exactly @@ -244,9 +355,18 @@ def test_the_remediation_says_what_is_true_of_every_value_it_refuses( reader reads back something other than what was typed -- names the failures that do hold, says plainly that a number which survives `str()` is a number all the same, and gives the way forward: a value that is text - to the reader.""" - result = scaffold(tmp_path, "--dry-run", f"{flag}={value}") + to the reader. + + The template quotes `id:` and `tracking_branch:` now (#259), so this + standard refuses none of them and the remediation is not said; it is + asked of the copy whose template writes them bare, where it is.""" + accepted = scaffold(tmp_path, "--dry-run", f"{flag}={value}") + assert accepted.returncode == 0, accepted.stderr + accepted.stdout + assert "Remediation" not in accepted.stderr + result = scaffold(tmp_path, "--dry-run", f"{flag}={value}", + tool=bare_standard / "scaffold-project.py") assert_refused(result, MISREAD) + assert_nothing_was_created(tmp_path) said = result.stderr assert "what it reads back is not what was typed" in said assert ("an id that is not text fails the project's own " @@ -279,17 +399,26 @@ def test_the_remediations_claim_about_str_is_what_the_reader_does( ("True", "true", "the boolean true"), ("Null", "null", "no value at all"), ]) -def test_an_id_derived_from_the_project_name_is_asked_too( - tmp_path, project, derived, reading): +def test_an_id_derived_from_the_project_name_is_held_as_typed_too( + tmp_path, bare_standard, project, derived, reading): """With no `--id` the id is the project name lowercased, so `--project - Null` is `id: null`; the sentence says where the id came from, so it does - not point at a flag nobody typed.""" - result = scaffold(tmp_path, "--project", project) - assert_refused(result, MISREAD) + Null` is the id `null`. Where a template line writes `id:` bare it is + refused, and the sentence says where the id came from, so it does not + point at a flag nobody typed (asked of the copy that writes it bare). The + template quotes it now (#259), so this standard scaffolds the project and + the manifest holds the id as that text.""" + refused = scaffold(tmp_path, "--project", project, + tool=bare_standard / "scaffold-project.py") + assert_refused(refused, MISREAD) assert (f"--id (not given, so --project lowercased) is {derived!r}" - in result.stderr) - assert reading in result.stderr + in refused.stderr) + assert reading in refused.stderr assert_nothing_was_created(tmp_path) + result = scaffold(tmp_path, "--project", project) + assert result.returncode == 0, result.stderr + result.stdout + manifest = manifest_in(tmp_path, project) + assert manifest["id"] == derived + assert manifest["topic"] == f"xf-project-{derived}" def test_an_explicit_id_is_the_way_round_for_such_a_project_name(tmp_path): @@ -544,13 +673,10 @@ def test_the_names_a_refusal_shows_are_told_apart_by_their_tails(): # --- never GitHub ---------------------------------------------------------- -@WINDOWS_SKIP # the `gh` below is a `#!` script, which Windows does not run -@pytest.mark.parametrize("flag", ["--id=null", "--tracking-branch=true", - "--elected-by= ", "--org=test+org"]) -def test_a_refused_value_never_reaches_gh(tmp_path, flag): - """Local remotes are the test path; the real one calls `gh repo create`. - Each refusal comes before the first `gh` call, so no repository is made - on the forge for a project that was never going to scaffold.""" +def scaffold_against_a_fake_gh(tmp_path: Path, flag: str): + """A scaffold with no `--local-remote-dir`, so it goes to the forge, with + a fake `gh` first on PATH that logs each call and answers no. Returns the + run and the log's path, which exists only if `gh` was asked.""" bin_dir, log = tmp_path / "bin", tmp_path / "gh.log" bin_dir.mkdir() shim = bin_dir / "gh" @@ -565,12 +691,40 @@ def test_a_refused_value_never_reaches_gh(tmp_path, flag): SCAFFOLD, *IDENTITY, flag, "--work-dir", str(tmp_path / WORK), env={"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", "FAKE_GH_LOG": str(log)}) + return result, log + + +@WINDOWS_SKIP # the `gh` below is a `#!` script, which Windows does not run +@pytest.mark.parametrize("flag", ["--elected-by= ", "--org=test+org"]) +def test_a_refused_value_never_reaches_gh(tmp_path, flag): + """Local remotes are the test path; the real one calls `gh repo create`. + Each refusal comes before the first `gh` call, so no repository is made + on the forge for a project that was never going to scaffold.""" + result, log = scaffold_against_a_fake_gh(tmp_path, flag) assert result.returncode == 2, result.stderr + result.stdout assert "REFUSED" in result.stderr assert not log.exists(), f"gh was asked: {log.read_text()}" assert not (tmp_path / WORK).exists() +@WINDOWS_SKIP # the `gh` below is a `#!` script, which Windows does not run +@pytest.mark.parametrize("flag", ["--id=null", "--tracking-branch=true"]) +def test_a_value_once_misread_reaches_gh_as_an_accepted_one_does(tmp_path, + flag): + """These two were refused before the first `gh` call while the template + wrote them bare. It quotes them now (#259), so they pass every question + the scaffold asks, and the run goes on to the forge as an accepted value's + does: the fake `gh` is asked to create the first leg and says no. That is + also the fake answering where `test_a_refused_value_never_reaches_gh` + would have seen a call.""" + result, log = scaffold_against_a_fake_gh(tmp_path, flag) + assert result.returncode == 2, result.stderr + result.stdout + assert "manifest-misread" not in result.stderr + assert log.exists(), "gh was never asked" + assert f"repo create {ORG}/Atlas-spec " in log.read_text() + assert f"gh repo create {ORG}/Atlas-spec " in result.stderr + + # --- one definition, two readers ------------------------------------------ def test_both_tools_ask_the_one_definition(scaffolder, adopter): @@ -628,24 +782,29 @@ def test_every_name_a_refusal_uses_is_a_value_the_tools_hold(scaffolder, assert set(adopter.MANIFEST_FLAGS) == set(adopter.MANIFEST_PLAN_KEYS) -def test_quoting_the_template_would_make_the_refusal_go_away(tmp_path): - """#259 proposes quoting `id:` and `tracking_branch:` in the template. The - question is asked of the TEMPLATE, so the day it does, those values read - back and nothing here refuses them, with no edit to this rule.""" - copy = tmp_path / "assembly-root" - shutil.copytree(REPO / "templates" / "assembly-root", copy, - ignore=shutil.ignore_patterns("__pycache__", "*.pyc")) - manifest = copy / "project.yaml" - text = manifest.read_text(encoding="utf-8") - quoted = (text.replace("id: {{PROJECT_ID}}", 'id: "{{PROJECT_ID}}"') - .replace("tracking_branch: {{TRACKING_BRANCH}}", - 'tracking_branch: "{{TRACKING_BRANCH}}"')) - assert quoted != text - manifest.write_text(quoted, encoding="utf-8") +def test_the_template_quotes_them_so_the_misread_rule_refuses_nothing_there( + scaffolder, bare_standard): + """#259 quoted `id:` and `tracking_branch:` in the template, and + `elected_on:` and `visibility:` with them. The question is asked of the + TEMPLATE, so since it did, no value of any field the scaffold names is + read back as anything but itself, and the rule refuses none, with no edit + to it. Written bare again, the same lines are refused again.""" + text = (REPO / "templates" / "assembly-root" / "project.yaml").read_text( + encoding="utf-8") + for key, placeholder in QUOTED_BY_259: + assert f'\n{key}: "{{{{{placeholder}}}}}"\n' in text, key + once_misread = {param.values[1] for param in ONCE_MISREAD_VALUES} + # and `~`, a basic-format date, and line breaks `render` spells escaped + also = {"~", "20261011", "a\x0bb", "a\u2028b"} + for value in sorted(once_misread | also): + values = dict.fromkeys(scaffolder.MANIFEST_FLAGS, value) + assert shape_materialize.manifest_misreads( + values, scaffolder.MANIFEST_FLAGS) == [], repr(value) values = {"PROJECT_ID": "null", "TRACKING_BRANCH": "007"} what = {"PROJECT_ID": "--id", "TRACKING_BRANCH": "--tracking-branch"} - assert len(shape_materialize.manifest_misreads(values, what)) == 2 - assert shape_materialize.manifest_misreads(values, what, copy) == [] + assert shape_materialize.manifest_misreads(values, what) == [] + assert len(shape_materialize.manifest_misreads( + values, what, bare_template(bare_standard))) == 2 # and what the quoted line holds is the text, through the project's reader assert parse_yaml('tracking_branch: "007"\n') == {"tracking_branch": "007"} @@ -659,7 +818,9 @@ def test_a_placeholder_is_what_render_calls_one(tmp_path, monkeypatch): (`{{SPEC_TREE_SHA256}}`); a line holding one asked placeholder and one with a digit must then count as two and not be asked, as a line that is asked is rendered with a one-key table and refused as - `template-unsubstituted`.""" + `template-unsubstituted`. The template quotes `id:` (#259), which the + reader then reads back, so the copy writes it bare again to have a line + whose value is refused.""" monkeypatch.setattr(shape_materialize, "PLACEHOLDER_RE", re.compile(r"\{\{[A-Z0-9_]+\}\}")) assert shape_materialize._placeholders_of( @@ -670,7 +831,7 @@ def test_a_placeholder_is_what_render_calls_one(tmp_path, monkeypatch): ignore=shutil.ignore_patterns("__pycache__", "*.pyc")) manifest = copy / "project.yaml" manifest.write_text( - manifest.read_text(encoding="utf-8") + written_bare(manifest.read_text(encoding="utf-8")) + "extra: {{PROJECT_ID}}-{{SPEC_TREE_SHA256}}\n", encoding="utf-8") values = {"PROJECT_ID": "null", "SPEC_TREE_SHA256": "0" * 64} what = {"PROJECT_ID": "--id"} @@ -695,38 +856,53 @@ def test_a_line_a_terminal_would_show_as_something_else_is_quoted_by_python(): @pytest.mark.parametrize("char", ["\x0b", "\x1c", "\u2028"]) -def test_a_misread_sentence_holding_a_line_break_is_one_printable_line(char): - """The manifest is split at the break, so the reader refuses it; the - sentence says so without printing the break itself.""" +def test_a_misread_sentence_holding_a_line_break_is_one_printable_line( + bare_standard, char): + """Where a template line writes the value bare, the manifest is split at + the break, so the reader refuses it; the sentence says so without printing + the break itself. The template quotes `tracking_branch:` now (#259), and + `render` spells the break as an escape between the quotes, so the reader + reads it back and the rule refuses nothing; the sentence is asked of the + copy that writes the line bare.""" what = {"TRACKING_BRANCH": "--tracking-branch"} + assert shape_materialize.manifest_misreads( + {"TRACKING_BRANCH": f"a{char}b"}, what) == [] + bare = bare_template(bare_standard) (sentence,) = shape_materialize.manifest_misreads( - {"TRACKING_BRANCH": f"a{char}b"}, what) + {"TRACKING_BRANCH": f"a{char}b"}, what, bare) assert len(sentence.splitlines()) == 1 assert sentence.isprintable() assert "into project.yaml as 'tracking_branch: a\\" in sentence # a printable misread is quoted in backticks, as before (plain,) = shape_materialize.manifest_misreads( - {"TRACKING_BRANCH": "007"}, what) + {"TRACKING_BRANCH": "007"}, what, bare) assert "into project.yaml as `tracking_branch: 007`" in plain # --- the placeholders a misread rule would refuse --------------------------- -def test_the_misread_placeholders_are_the_ones_the_misread_rule_refuses(): +def test_the_misread_placeholders_are_the_ones_the_misread_rule_refuses( + bare_standard): """What a caller that leaves misreads to a rule of its own asks of the validator is whatever `manifest_misreads` would refuse, as an answer of - the project's reader and not a list of values.""" + the project's reader and not a list of values: none on this standard's + template, which quotes all three (#259), and the three where a template + writes them bare.""" what = {"PROJECT_ID": "--id", "TRACKING_BRANCH": "--tracking-branch", "ELECTED_BY": "--elected-by", "ELECTED_ON": "--elected-on", "VISIBILITY": "--visibility", "ORG": "--org"} values = {"PROJECT_ID": "007", "TRACKING_BRANCH": "2026", "ELECTED_BY": " ", "ELECTED_ON": "2026-09-02", "VISIBILITY": "true", "ORG": "acme"} - asked = shape_materialize.misread_placeholders(values, what) + assert shape_materialize.misread_placeholders(values, what) == () + assert shape_materialize.manifest_misreads(values, what) == [] + bare = bare_template(bare_standard) + asked = shape_materialize.misread_placeholders(values, what, bare) assert sorted(asked) == ["PROJECT_ID", "TRACKING_BRANCH", "VISIBILITY"] - assert len(shape_materialize.manifest_misreads(values, what)) == len(asked) + assert len(shape_materialize.manifest_misreads(values, what, bare)) == ( + len(asked)) assert shape_materialize.misread_placeholders( - {"PROJECT_ID": "atlas"}, what) == () + {"PROJECT_ID": "atlas"}, what, bare) == () # --- a validator that cannot be asked ------------------------------------- @@ -752,11 +928,12 @@ def test_the_misread_placeholders_are_the_ones_the_misread_rule_refuses(): ] -def a_copy_of_the_standard(base: Path, remake) -> Path: +def a_copy_of_the_standard(base: Path, remake, + rel: str = "scripts/validate-manifest.py") -> Path: """The standard, as its own git checkout (the tools name their revision), - with its template's `validate-manifest.py` made over by `remake`, which is - given the real text and returns the text to put there, or None to remove - the file.""" + with its template's `validate-manifest.py`, or the template file `rel`, + made over by `remake`, which is given the real text and returns the text + to put there, or None to remove the file.""" copy = base / "standard" shutil.copytree(REPO, copy, symlinks=True, ignore=shutil.ignore_patterns( ".git", "__pycache__", ".pytest_cache", "tests", "docs", "handoffs")) @@ -764,8 +941,7 @@ def a_copy_of_the_standard(base: Path, remake) -> Path: git("add", "-A", cwd=copy) git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "--no-gpg-sign", "-m", "a copy of the standard", cwd=copy) - path = copy / "templates" / "assembly-root" / "scripts" / ( - "validate-manifest.py") + path = copy / "templates" / "assembly-root" / rel remade = remake(path.read_text(encoding="utf-8")) if remade is None: path.unlink() diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index acb23366..1784628e 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -340,7 +340,7 @@ def test_an_internal_visibility_is_accepted(tmp_path): assert result.returncode == 0, result.stderr + result.stdout assert "visibility internal" in result.stdout manifest = (tmp_path / "work" / "Sample" / "project.yaml").read_text() - assert "visibility: internal" in manifest + assert 'visibility: "internal"' in manifest def test_a_name_outside_the_naming_policy_stops_before_the_scaffold(tmp_path): diff --git a/tests/test_setup_sh.py b/tests/test_setup_sh.py index 0a1cce63..8e3227ab 100644 --- a/tests/test_setup_sh.py +++ b/tests/test_setup_sh.py @@ -282,7 +282,7 @@ def test_an_internal_visibility_is_accepted(tmp_path): assert result.returncode == 0, result.stderr + result.stdout assert "visibility internal" in result.stdout manifest = (tmp_path / "work" / "Sample" / "project.yaml").read_text() - assert "visibility: internal" in manifest + assert 'visibility: "internal"' in manifest def test_a_name_outside_the_naming_policy_stops_before_the_scaffold(tmp_path): diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 154d8af3..3b60097e 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2759,8 +2759,8 @@ def test_a_sibling_at_the_right_origin_with_another_id_is_not_the_member( root, sibling = member_beside(holder, tmp_path) manifest = sibling / "project.yaml" manifest.write_text( - manifest.read_text(encoding="utf-8").replace("id: irrs", - "id: somethingelse"), + manifest.read_text(encoding="utf-8").replace('id: "irrs"', + 'id: "somethingelse"'), encoding="utf-8") row = rows_of(doctor(standard, root, "--json"))["members"] entry = row["detail"]["members"][0] @@ -5947,7 +5947,7 @@ def test_a_leg_the_manifest_points_outside_the_root_is_never_read(standard, manifest = project / "project.yaml" manifest.write_text( manifest.read_text(encoding="utf-8").replace( - "path: spec", "path: ../outside", 1), encoding="utf-8") + 'path: "spec"', 'path: "../outside"', 1), encoding="utf-8") row = rows_of(doctor(standard, project, "--json"))["placement"] legs = {entry["role"]: entry for entry in row["detail"]["legs"]} assert "outside" in legs["spec"]["state"], legs diff --git a/tests/test_template_text_placeholders.py b/tests/test_template_text_placeholders.py new file mode 100644 index 00000000..4a68103b --- /dev/null +++ b/tests/test_template_text_placeholders.py @@ -0,0 +1,452 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A text value YAML would read as another type reads back as typed (#259). + +The assembly root's `project.yaml`, its two leg pins and a family holder's +`family.yaml` wrote their text placeholders bare: `id: {{PROJECT_ID}}`, +`tracking_branch: {{TRACKING_BRANCH}}`, `path: {{SPEC_PATH}}`, +`submodule_path: {{SPEC_PATH}}`. Every reader of those files reads them with +`repo_shape`'s YAML subset, which takes a bare `007` as the number 7, `2026` +and `20261011` as numbers, `1.5` as a float, `true` as True and `null` as +None. The checks on the flags (`PROJECT_ID_RE`, `checked_value`, the leg-path +rule) admit every one of them, so on main: + +- `scaffold-project.py --id 007` wrote a project whose own gate failed + `manifest-id`, and `--spec-path 2026` one that failed `manifest-leg-path`; +- `--tracking-branch 007` pushed each leg on `007`, and the root's own + `bootstrap.py` then put the legs on a local `7` with no upstream (`true` + and `null` did the same on `True` and `main`); +- `adopt-project.py execute` wrote the same lines, and `family.py init` wrote + a holder its own `validate-family.py` refused (`family-id`, `family-org`). + +Brett Heap ruled on 2026-10-12, "quote #259". The templates write each of +those fields between double quotes, `render` spells the value there with +`double_quoted` (#284), and the reader takes it back exactly. A project made +before this keeps its bare lines, which read correctly for every name that is +plain text, so the readers keep their `str()`. + +NO NETWORK AND NO GITHUB: templates rendered in memory, and scaffolds, +adoptions and families into local bare remotes (`--local-remote-dir`), which +never call `gh`. +""" + +from __future__ import annotations + +import re +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import (ADOPT, FILE_PROTOCOL, REPO, SCAFFOLD, git, + make_source_repo, run_script) +from test_family import family_module + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import ( # noqa: E402 + YamlError, double_quoted, load_yaml, parse_yaml, +) +from shape_materialize import render # noqa: E402 + +FAMILY = REPO / "scripts" / "family.py" +BUMP = REPO / "scripts" / "bump-leg.py" +PROJECT = "Atlas" +ADOPTED = "Northwind" +HOLDER = "Contoso" +ORG = "testorg" +ADOPT_BRANCH = "adopt/three-repo-shape" + +PLACEHOLDER = re.compile(r"\{\{([A-Z0-9_]+)\}\}") + +#: Text the reader takes as something else when it stands bare after a key: +#: numbers, a float, the keywords it knows, and the dates no ISO form spells. +MISREAD = ("2026", "007", "1.5", "true", "null", "20261011", "~", "False") + +#: `(template, placeholder, where the loaded document holds it)` for every +#: text field a tool fills from a value it was given. A leg is found by its +#: role, never by its position in `legs:`. +TEXT_FIELDS = [ + ("templates/assembly-root/project.yaml", "PROJECT_ID", ("id",)), + ("templates/assembly-root/project.yaml", "ELECTED_ON", ("elected_on",)), + ("templates/assembly-root/project.yaml", "VISIBILITY", ("visibility",)), + ("templates/assembly-root/project.yaml", "TRACKING_BRANCH", + ("tracking_branch",)), + ("templates/assembly-root/project.yaml", "SPEC_PATH", + ("legs", "spec", "path")), + ("templates/assembly-root/project.yaml", "CODE_PATH", + ("legs", "code", "path")), + ("templates/assembly-root/contracts/spec-pin.yaml", "SPEC_PATH", + ("submodule_path",)), + ("templates/assembly-root/contracts/code-pin.yaml", "CODE_PATH", + ("submodule_path",)), + ("templates/family-root/family.yaml", "FAMILY_ID", ("id",)), + ("templates/family-root/family.yaml", "ORG", ("org",)), + ("templates/family-root/family.yaml", "CREATED_ON", ("created_on",)), + ("templates/family-root/family.yaml", "VISIBILITY", ("visibility",)), + ("templates/family-root/family.yaml", "TRACKING_BRANCH", + ("tracking_branch",)), +] + + +def held(document, where: tuple): + """The value at `where`: a key, or under `legs` the leg with that role.""" + node = document + for step in where: + if isinstance(node, list): + node = next(leg for leg in node if leg.get("role") == step) + else: + node = node[step] + return node + + +def rendered(template: str, placeholder: str, value: str) -> str: + """`template` with `value` in `placeholder` and a harmless stand-in in + every other placeholder: a line of its own for a block placeholder.""" + text = (REPO / template).read_text(encoding="utf-8") + values = {} + for key in set(PLACEHOLDER.findall(text)): + if key == placeholder: + values[key] = value + elif re.search(r"^\{\{" + key + r"\}\}", text, re.M): + values[key] = f" stand_in_{key.lower()}: 1" + else: + values[key] = f"v-{key.lower()}" + return render(text, values, template) + + +@pytest.mark.parametrize("value", MISREAD) +@pytest.mark.parametrize( + "template,placeholder,where", TEXT_FIELDS, + ids=[f"{t.split('/')[-1]}:{'.'.join(w)}" for t, _, w in TEXT_FIELDS]) +def test_a_text_field_reads_back_as_the_text_it_was_given(template, + placeholder, where, + value): + assert held(parse_yaml(rendered(template, placeholder, value)), + where) == value + + +# --- scaffold-project.py, end to end --------------------------------------- +# +# Each row puts a different value in each field, so that across the rows +# every field holds every value its flag admits: `--id` refuses `1.5` (a dot +# is not in `PROJECT_ID_RE`), and every other field takes all six. + +SCAFFOLDS = [ + # --id, --tracking-branch, --spec-path, --code-path + ("true", "2026", "007", "1.5"), + ("null", "007", "1.5", "true"), + ("20261011", "1.5", "true", "null"), + ("2026", "true", "null", "20261011"), + ("007", "null", "20261011", "2026"), + ("007", "20261011", "2026", "007"), +] + + +def heads(bare: Path) -> list[str]: + return git("for-each-ref", "--format=%(refname:short)", "refs/heads", + cwd=bare).stdout.split() + + +def clone(remote: Path, target: Path, *extra: str) -> Path: + proc = subprocess.run( + ["git", *FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", + *extra, str(remote), str(target)], + capture_output=True, text=True, check=False) + assert proc.returncode == 0, proc.stderr + return target + + +def assert_the_root_reads_what_it_was_given(manifest: dict, pins: dict, + project_id: str, branch: str, + spec: str, code: str) -> None: + assert manifest["id"] == project_id + assert manifest["tracking_branch"] == branch + assert held(manifest, ("legs", "spec", "path")) == spec + assert held(manifest, ("legs", "code", "path")) == code + assert pins == {"spec": spec, "code": code} + + +def pin_paths(root: Path, show=None) -> dict: + """Each leg pin's `submodule_path`, read from disk or through `show`.""" + out = {} + for role in ("spec", "code"): + rel = f"contracts/{role}-pin.yaml" + pin = parse_yaml(show(rel)) if show else load_yaml(root / rel) + out[role] = pin["submodule_path"] + return out + + +def assert_the_clone_works(root: Path, branch: str, spec: str, + code: str) -> None: + """The root's own bootstrap puts each leg on `branch` with its upstream, + and the root's own three validators pass.""" + booted = run_script(root / "scripts" / "bootstrap.py", cwd=root) + assert booted.returncode == 0, booted.stdout + booted.stderr + for path in (spec, code): + leg = root / path + assert git("symbolic-ref", "--short", "HEAD", cwd=leg).stdout.strip() \ + == branch, f"the leg at {path!r} is not on {branch!r}" + assert git("rev-parse", "--abbrev-ref", "@{upstream}", + cwd=leg).stdout.strip() == f"origin/{branch}" + for validator, args in (("validate-manifest.py", ()), + ("validate-pins.py", ()), + ("validate-repository-naming.py", + ("--project", "project.yaml"))): + checked = run_script(root / "scripts" / validator, *args, cwd=root) + assert checked.returncode == 0, \ + f"{validator}: {checked.stdout}{checked.stderr}" + + +@pytest.mark.parametrize("project_id,branch,spec,code", SCAFFOLDS, + ids=["-".join(row) for row in SCAFFOLDS]) +def test_a_scaffold_reads_back_every_value_it_was_given(tmp_path, project_id, + branch, spec, code): + remotes, work = tmp_path / "remotes", tmp_path / "work" + result = run_script(SCAFFOLD, "--org", ORG, "--project", PROJECT, + "--id", project_id, "--tracking-branch", branch, + "--spec-path", spec, "--code-path", code, + "--elected-by", "Test Human", + "--elected-on", "2026-09-02", + "--local-remote-dir", str(remotes), + "--work-dir", str(work)) + assert result.returncode == 0, result.stderr + result.stdout + written = work / PROJECT + assert_the_root_reads_what_it_was_given( + load_yaml(written / "project.yaml"), pin_paths(written), project_id, + branch, spec, code) + for name in (PROJECT, f"{PROJECT}-spec", f"{PROJECT}-code"): + assert heads(remotes / f"{name}.git") == [branch], name + + root = clone(remotes / f"{PROJECT}.git", tmp_path / "clone" / PROJECT) + assert_the_clone_works(root, branch, spec, code) + + # `bump-leg.py` reads the branch and the leg's path out of the same + # manifest. On the tracking branch it refuses, which it can only do if it + # read the branch the root is on; on a branch of its own it finds the + # leg's checkout at its path and reports the pin it already holds. + pinned = git("rev-parse", f"HEAD:{spec}", cwd=root).stdout.strip() + bump = ("--root", str(root), "--leg", "spec", "--to", pinned, + "--local-remote-dir", str(remotes)) + refused = run_script(BUMP, *bump) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert "bump-leg-on-tracking-branch" in refused.stderr + git("switch", "-q", "-c", "bump/spec", cwd=root) + again = run_script(BUMP, *bump) + assert again.returncode == 1, again.stdout + again.stderr + assert "nothing to do" in again.stdout + assert f"at {spec}/" in again.stdout + + +# --- adopt-project.py, end to end ------------------------------------------ +# +# `plan` writes a value the plan's own reader would misread between double +# quotes when it starts with a digit, so `2026`, `007`, `1.5` and `20261011` +# go through the flags. It writes `true` and `null` bare, and the plan's +# reader then takes them as a boolean and as no value: that is the PLAN's +# writer, #181's to fix. Those two rows quote the value in the plan by hand, +# as #181's writer will, and ask `check` and `execute` the rest. + +ADOPTIONS = [ + # --id, --tracking-branch, --spec-path, --code-path, by hand + ("007", "2026", "1.5", "20261011", False), + ("20261011", "007", "2026", "007", False), + ("2026", "1.5", "007", "2026", False), + ("2026", "20261011", "20261011", "1.5", False), + ("true", "true", "null", "true", True), + ("null", "null", "true", "null", True), +] + +pytest_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + +SOURCE_TREE = {"README.md": "# Thing\n", "specs/feature.md": "# Feature\n", + "src/main.py": "VALUE = 1\n"} + + +def quote_in_plan(plan: Path, edits: dict) -> None: + """Replace each `: ` line of the plan with the quoted value, + in bytes, so the plan stays LF on every platform.""" + data = plan.read_bytes() + for line, value in edits.items(): + key = line.split(b":", 1)[0] + assert data.count(line) == 1, line + data = data.replace(line, key + b': "' + value.encode() + b'"\n') + plan.write_bytes(data) + + +@pytest_filter_repo +@pytest.mark.parametrize("project_id,branch,spec,code,by_hand", ADOPTIONS, + ids=["-".join(row[:4]) for row in ADOPTIONS]) +def test_an_adoption_reads_back_every_value_it_was_given(tmp_path, project_id, + branch, spec, code, + by_hand): + source = make_source_repo(tmp_path / "Thing", tree=SOURCE_TREE, edits=()) + # Adopt refuses a tracking branch the source lacks, as + # `plan-tracking-branch-missing` (#479, main's 56713e21), so the source + # carries the branch this row adopts onto: flag rows and by-hand rows. + git("branch", branch, cwd=source) + plan = tmp_path / "adoption-plan.yaml" + flags = () if by_hand else ("--id", project_id, "--tracking-branch", + branch, "--spec-path", spec, + "--code-path", code) + planned = run_script(ADOPT, "plan", "--source", str(source), + "--project", ADOPTED, "--org", ORG, + "--elected-by", "Test Human", + "--elected-on", "2026-09-02", + "--out", str(plan), *flags) + assert planned.returncode == 0, planned.stderr + planned.stdout + if by_hand: + quote_in_plan(plan, {b"id: northwind\n": project_id, + b"tracking_branch: main\n": branch, + b" spec_path: spec\n": spec, + b" code_path: code\n": code}) + checked = run_script(ADOPT, "check", "--plan", str(plan)) + assert checked.returncode == 0, checked.stderr + checked.stdout + remotes = tmp_path / "remotes" + executed = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "work")) + assert executed.returncode == 0, executed.stderr + executed.stdout + + def show(rel: str) -> str: + return git("show", f"{ADOPT_BRANCH}:{rel}", cwd=source).stdout + + assert_the_root_reads_what_it_was_given( + parse_yaml(show("project.yaml")), pin_paths(source, show), project_id, + branch, spec, code) + for leg in ("spec", "code"): + assert heads(remotes / f"{ADOPTED}-{leg}.git") == [branch], leg + + root = clone(source, tmp_path / "clone" / ADOPTED, "-b", ADOPT_BRANCH) + assert_the_clone_works(root, branch, spec, code) + + +# --- family.py, end to end ------------------------------------------------- + +FAMILIES = [ + # --id, --org, --tracking-branch + ("007", "2026", "true"), + ("2026", "true", "null"), + ("true", "null", "007"), + ("null", "007", "1.5"), + ("20261011", "1.5", "20261011"), + ("007", "20261011", "2026"), +] + + +@pytest.mark.parametrize("family_id,org,branch", FAMILIES, + ids=["-".join(row) for row in FAMILIES]) +def test_a_family_holder_reads_back_every_value_it_was_given(tmp_path, + family_id, org, + branch): + remotes = tmp_path / "remotes" + created = run_script(FAMILY, "init", "--org", org, "--family", HOLDER, + "--id", family_id, "--tracking-branch", branch, + "--created-by", "Test Human", + "--created-on", "2026-09-04", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "work")) + assert created.returncode == 0, created.stderr + created.stdout + root = tmp_path / "work" / HOLDER + manifest = load_yaml(root / "family.yaml") + assert (manifest["id"], manifest["org"], manifest["tracking_branch"]) \ + == (family_id, org, branch) + assert heads(remotes / f"{HOLDER}.git") == [branch] + checked = run_script(root / "scripts" / "validate-family.py", cwd=root) + assert checked.returncode == 0, checked.stdout + checked.stderr + + +def test_a_member_whose_id_yaml_would_misread_is_pinned_under_that_id( + tmp_path): + """`family.py add` records the member's OWN `project.yaml` id in its row. + That id is read back exactly now, so the row has to be too: written bare, + `007` came back as 7 and the holder's validator reported + `member-id-mismatch` for a member it had just pinned. An ordinary id is + written as it always was, so a row whose id reads back as text does not + move.""" + remotes, work = tmp_path / "remotes", tmp_path / "work" + members = {"IRRS": "007", "IRSS": "true", "IRQS": None} + for project, project_id in members.items(): + flags = ("--id", project_id) if project_id else () + made = run_script(SCAFFOLD, "--org", ORG, "--project", project, + *flags, "--elected-by", "Test Human", + "--elected-on", "2026-09-04", + "--local-remote-dir", str(remotes), + "--work-dir", str(work)) + assert made.returncode == 0, made.stderr + made.stdout + created = run_script(FAMILY, "init", "--org", ORG, "--family", HOLDER, + "--created-by", "Test Human", + "--created-on", "2026-09-04", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "fam")) + assert created.returncode == 0, created.stderr + created.stdout + root = tmp_path / "fam" / HOLDER + # On a branch of its own, as a person adds members for a pull request: + # #487 (PR #507) makes `add` refuse the holder's tracking branch. + git("switch", "-q", "-c", "family/members", cwd=root) + for project in members: + added = run_script(FAMILY, "add", "--family-root", str(root), + "--member", f"{ORG}/{project}", + "--local-remote-dir", str(remotes)) + assert added.returncode == 0, added.stderr + added.stdout + rows = {row["project"]: row["id"] + for row in load_yaml(root / "family.yaml")["members"]} + assert rows == {"IRRS": "007", "IRSS": "true", "IRQS": "irqs"} + lines = (root / "family.yaml").read_bytes().splitlines() + assert b' id: "007"' in lines + assert b" id: irqs" in lines + checked = run_script(root / "scripts" / "validate-family.py", cwd=root) + assert checked.returncode == 0, checked.stdout + checked.stderr + + +#: Ids the holder's reader RAISES on, rather than reads as another type, when +#: they stand bare after `id:`. `PROJECT_ID_RE` keeps every one out of a +#: scaffold and out of `family.py add`, so no tool writes one; a hand-edited +#: row holds one between double quotes, and the next `bump`, `add` or +#: `remove` spells the whole block again. +UNREADABLE_BARE_IDS = ("[", "{", "&a", "*a", "!t", "'a", '"a') + +HAND_EDITED_HOLDER = """\ +name: Contoso +kind: family-manifest +members: + - project: IRRS + id: {spelled} + repository: testorg/IRRS + path: members/IRRS + pin: + revision_kind: commit + commit: "{commit}" + digest_algorithm: sha256 + digest_definition: sorted-ls-tree-r-v1 + tree_sha256: "{tree}" +""" + + +@pytest.mark.parametrize( + "member_id", UNREADABLE_BARE_IDS, + ids=["flow-sequence", "flow-mapping", "anchor", "alias", "tag", + "open-single-quote", "open-double-quote"]) +def test_a_member_id_the_reader_refuses_bare_is_rewritten_between_quotes( + tmp_path, member_id): + """`_member_id` asks the reader whether the bare id reads back as that + text, and an id the reader RAISES on is not read back as anything: it + gets the quotes, and the rewrite goes on. A call that let the error out + would stop `bump`, `add` and `remove` on a holder that holds such a row, + though the row itself is a quoted scalar the reader takes exactly.""" + with pytest.raises(YamlError): + parse_yaml(f"id: {member_id}\n") + spelled = double_quoted(member_id) + text = HAND_EDITED_HOLDER.format(spelled=spelled, commit="a" * 40, + tree="b" * 64) + (tmp_path / "family.yaml").write_bytes(text.encode("utf-8")) + family = family_module().Family.open(tmp_path) + assert [row["id"] for row in family.members] == [member_id] + + again = family.rewritten(family.members) + + assert parse_yaml(again)["members"][0]["id"] == member_id + assert f" id: {spelled}\n" in again + assert again == text