diff --git a/contracts/path-classification.yaml b/contracts/path-classification.yaml index 389160c4..1be1c818 100644 --- a/contracts/path-classification.yaml +++ b/contracts/path-classification.yaml @@ -441,14 +441,15 @@ rules: # the file blocks nothing the shape writes and no validator, gate or # doctor row reads it. As for the rules above, what `execute` does is said # of a repository WHEN IT IS ADOPTED: `shape-doctor.py --placement-plan` - # prints this question too, by name, for a leg already split, and reads - # the leg's copy as a question, because the verdict's leg stays null. + # prints this question too, for a leg already split, and reads the leg's + # copy as a question, because the verdict's leg stays null. # # `kind:` DECIDES, NOT THE NAME, as it does for the advisory. `plan` asks # this question only of a file that declares `kind: - # single-repository-record` at the commit it plans. One of this name that - # declares anything else, or does not parse, is no record, and is asked - # the general question below as any other top-level data file is. + # single-repository-record` at the commit it plans, and the doctor only + # of a leg's copy that declares it in the leg's index (#691). One of this + # name that declares anything else, or does not parse, is no record, and + # is asked the general question below as any other top-level data file is. - id: ambiguous-single-repository-record paths: - "single-repository.yaml" diff --git a/shape-doctor.py b/shape-doctor.py index 3dc65fc9..8d7ec170 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -6458,6 +6458,13 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, the record answers into this leg, which are returned apart, and `unslashed_rows` names the rows for this leg that name a directory without its `/`. + + AND A LEG'S `single-repository.yaml` IS ASKED THE RECORD'S QUESTION ONLY + WHERE IT IS THE RECORD (#691). The walk asks it by name; `plan`'s own + `ask_about_a_record_only` reads the file's `kind:` and gives any other + the general question. It is called here over the leg's INDEX, which is + what this row lists, so a file edited or deleted on disk and a link are + read as `plan` reads them. """ entry = {"role": role, "path": rel} mount = root / rel @@ -6494,7 +6501,11 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, kept = [(path, size) for path, size in files if path not in gitlinks and not any(regex.match(path) for regex in patterns)] - walked = adopt.walk(policy, kept) + # `plan`'s own `ask_about_a_record_only` (#528, #691), over the leg's + # INDEX -- `""` is git's `:`, the index `leg_tracked_files` + # lists -- so a link is read as its blob and never followed. + walked = adopt.ask_about_a_record_only( + policy, adopt.walk(policy, kept), adopt.Source(mount, None, "", "")) submodules, wrong = leg_submodules(policy, role, rel, held, record) answered = gitmodules_the_record_answers(adopt, submodules, unwritable) misplaced, review, settled = walked_findings(walked, role, rel, answered) diff --git a/tests/test_adopt_single_repository_record.py b/tests/test_adopt_single_repository_record.py index 1acd24b0..06f6d476 100644 --- a/tests/test_adopt_single_repository_record.py +++ b/tests/test_adopt_single_repository_record.py @@ -18,12 +18,15 @@ unless a person writes `root`. The verdict keeps `leg` None, so the doctor, which walks a leg with the same `walk`, still reads a leg's copy as a question. And `kind:` decides, not the name: a file of that name that is no -record at the commit `plan` reads is asked the general question. +record at the commit `plan` reads is asked the general question, and so is +a leg's copy that is no record in the leg's index, which the doctor reads +(#691). """ from __future__ import annotations import importlib.util +import os import shutil import sys from argparse import Namespace @@ -41,6 +44,9 @@ needs_filter_repo = pytest.mark.skipif( shutil.which("git-filter-repo") is None, reason="git filter-repo is not installed: `pip install git-filter-repo`") +needs_disk_symlinks = pytest.mark.skipif( + sys.platform == "win32", + reason="a tracked symlink is a plain file in a Windows checkout") PROJECT = "Northwind" RULE = "ambiguous-single-repository-record" @@ -356,6 +362,119 @@ def test_the_doctor_reads_a_legs_copy_as_a_question_and_not_misplaced( assert row["review_required"] is True +# --- the doctor asks the record's question of a record only (#691) ----------- + +@pytest.fixture(scope="module") +def doctor(): + return load_without_bytecode(REPO / "shape-doctor.py", + "shape_doctor_single_repository_record_leg") + + +def leg_audited(adopter, doctor, tmp_path, text: str, edit=None) -> tuple: + """`audit_leg`'s `(paths in the wrong leg, questions)` for a code leg + that commits `RECORD_FILE` with `text`, a source file and a `widgets/` + the walk folds into one entry beside it, and then has `edit(leg)` done + to its disk. The folded entry is what a reclassification of EVERY entry + would turn into a second question.""" + leg = make_source_repo(tmp_path / "code", edits=(), tree={ + "src/main.c": "int main(void) { return 0; }\n", + "widgets/a.c": "int a;\n", "widgets/b.c": "int b;\n", + RECORD_FILE: text}) + if edit is not None: + edit(leg) + _, misplaced, review, _, _ = doctor.audit_leg( + adopter, PathPolicy.load(POLICY), [], "code", "code", tmp_path, {}, + {}) + return misplaced, review + + +def size_of_the_file(tmp_path) -> int: + """The bytes the leg's `RECORD_FILE` holds on this disk, which is what + the row counts. `write_text` made them, so on Windows they are not + `len(text.encode())`.""" + return len((tmp_path / "code" / RECORD_FILE).read_bytes()) + + +@pytest.mark.parametrize("body", list(NOT_A_RECORD.values()), + ids=list(NOT_A_RECORD)) +def test_the_doctor_asks_a_leg_file_that_is_no_record_the_general_question( + adopter, doctor, tmp_path, body): + """`plan` asks the general question of a file of that name that is no + record, and so does the placement row: it does not tell the person the + file records that the repository stays single. Nothing else is asked, + and the entry keeps its one file and its bytes.""" + misplaced, review = leg_audited(adopter, doctor, tmp_path, body) + row, = review + general = PathPolicy.load(POLICY).classify_file("release.yaml") + assert misplaced == [] + assert (row["path"], row["rule"], row["classified_as"]) == ( + f"code/{RECORD_FILE}", GENERAL, None) + assert row["review_required"] is True + assert row["question"] == general.question + assert row["reason"] == general.reason + assert (row["files"], row["bytes"]) == (1, size_of_the_file(tmp_path)) + + +@pytest.mark.parametrize("body", [RECORD, "kind: single-repository-record\n"], + ids=["a full record", "the kind alone"]) +def test_the_doctor_keeps_the_records_question_for_a_record( + adopter, doctor, tmp_path, body): + """`kind:` decides, so a leg's copy that declares the record's kind, with + a field left out or none, is asked the record's question, whose text + names its default, as before.""" + _, review = leg_audited(adopter, doctor, tmp_path, body) + row, = review + assert (row["rule"], row["classified_as"]) == (RULE, None) + assert row["question"] == record_question() + assert "(drop, the default" in row["question"] + assert (row["files"], row["bytes"]) == (1, size_of_the_file(tmp_path)) + + +def deleted(leg) -> None: + (leg / RECORD_FILE).unlink() + + +def rewritten(text: str): + """An `edit` that rewrites the leg's `RECORD_FILE` on disk, unstaged.""" + return lambda leg: (leg / RECORD_FILE).write_bytes(text.encode()) + + +@pytest.mark.parametrize("committed, edit, rule", [ + (RECORD, deleted, RULE), + (RECORD, rewritten(NOT_A_RECORD["another kind"]), RULE), + (NOT_A_RECORD["another kind"], rewritten(RECORD), GENERAL), +], ids=["deleted-on-disk", "edited-to-no-record", "edited-to-a-record"]) +def test_the_doctor_reads_the_legs_index_and_not_its_disk( + adopter, doctor, tmp_path, committed, edit, rule): + """THE ROW LISTS THE LEG'S INDEX, so the file is read there, as `plan` + reads its commit: one that is tracked and gone from the disk, or edited + there and not staged, is asked as what it is in the index.""" + _, review = leg_audited(adopter, doctor, tmp_path, committed, edit) + row, = review + assert (row["rule"], row["classified_as"]) == (rule, None) + + +@needs_disk_symlinks +def test_a_tracked_link_to_a_record_is_asked_the_general_question( + adopter, doctor, tmp_path): + """A tracked link is read as its blob, the target's name, which is no + record, and never followed: not to a record beside the leg, which the + leg's own repository does not hold, as `plan` reads it.""" + (tmp_path / "outside.yaml").write_bytes(RECORD.encode()) + + def link(leg) -> None: + (leg / RECORD_FILE).unlink() + try: + os.symlink("../outside.yaml", leg / RECORD_FILE) + except (NotImplementedError, OSError) as exc: + pytest.skip(f"this platform cannot create a symlink here: {exc}") + git("add", "--", RECORD_FILE, cwd=leg) + + _, review = leg_audited(adopter, doctor, tmp_path, RECORD, link) + row, = review + assert (row["rule"], row["classified_as"]) == (GENERAL, None) + + # --- what each answer does ---------------------------------------------------- def adopted(source, tmp_path, leg: str | None): diff --git a/tests/test_doctor_recorded_answers.py b/tests/test_doctor_recorded_answers.py index 2693379b..1c5329a0 100644 --- a/tests/test_doctor_recorded_answers.py +++ b/tests/test_doctor_recorded_answers.py @@ -66,6 +66,13 @@ RULES_ANSWER = "editor rules the code leg's tooling reads" #: A golden run, as two files the policy cannot call (`ambiguous-examples`). GOLDEN = ("examples/golden/case.json", "examples/golden/case.expected.json") +#: A top-level file named as the staying-single record is, and what `kind:` +#: makes of it (#691): the record the policy asks its own question of, or a +#: file that is not one, which is asked the general question. +RECORD_FILE = "single-repository.yaml" +RECORD_KINDS = [ + ("kind: single-repository-record\n", "ambiguous-single-repository-record"), + ("kind: release-config\n", "ambiguous-top-level-yaml")] MOVED_CLAUSE = ("1 submodule(s) moved out of the leg project.yaml's " "`submodules:` records for them, which no longer holds them " "(#525): spec/vendor/rules (recorded in the code leg)") @@ -213,6 +220,26 @@ def test_an_answer_no_leg_holds_is_silent(standard, project): COMPLIANT, "ok", before["reason"]), row +@pytest.mark.parametrize("body, rule", RECORD_KINDS, + ids=["a-record", "not-a-record"]) +def test_an_answered_single_repository_yaml_names_the_rule_that_asked_it( + standard, project, body, rule): + """THE RULE IS THE QUESTION'S, AND `kind:` DECIDES WHICH QUESTION (#691). + A leg's file of this name answered in the code leg is listed under + `answered_paths` with the rule that asked it of that file: the record's + where it is the record, and the general one where it declares anything + else, as `adopt-project.py plan` asks it.""" + stage(project / "code", RECORD_FILE, body) + record_answers(project, (RECORD_FILE, "code", "kept in the code leg")) + result, _, row = placement(standard, project) + assert result.returncode == 0, result.stdout + result.stderr + assert row["status"] == "ok", row + [answered] = row["detail"]["answered_paths"] + assert (answered["path"], answered["rule"]) == ( + f"code/{RECORD_FILE}", rule), answered + assert f"code/{RECORD_FILE} (rule {rule})" in row["reason"], row["reason"] + + # --- L1-08: a submodule moved, and one removed ------------------------------ def moved_rules(project: Path, *, gitmodules: bool = False) -> None: