From 069cffab35ef448f2ac9d62687df7a57da0f15d8 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Tue, 15 Sep 2026 19:46:05 +1200 Subject: [PATCH] tools(parity): make a one-sided property declarable `_required_v3_exemptions` built its kind strings with `name[:-1]`, so `unpaired_properties` stemmed to `add-unpaired-propertie`. The vocabulary `_validate_dispositions` accepts is `add-unpaired-property`. Those two never meet. A disposition written as `add-unpaired-propertie` is rejected by validation, and one written as `add-unpaired-property` never matches the requirement, so a one-sided property can be neither waived nor cleared and `--refresh-derived` refuses for as long as it exists. Files, functions and constants all survive the naive strip; only the `-ies` plural does not, which is why nothing caught it. No existing test declares a one-sided property, and none had appeared in the tree until Lift Log added two. Replaces the strip with explicit singulars and adds a test asserting the invariant directly: every kind the ratchet can REQUIRE must be a kind a disposition may DECLARE. Reverting the fix fails it on the exact string. Nothing else referenced the misspelling, and the removal side already read correctly because "pairs" strips to "pair". The workflow runs an explicit module list rather than discovery, so the new test would not have run in CI. Adding it to that list and raising the collected-count floor from 113 to 115: the three existing modules produce exactly 113, so the floor tracks the real total rather than sitting loosely below it, and leaving it alone would have let these two tests disappear later without anything going red. --- .github/workflows/parity-governance.yml | 5 +- Tools/parity_ratchet.py | 17 ++++-- Tools/tests/test_parity_disposition_kinds.py | 64 ++++++++++++++++++++ 3 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 Tools/tests/test_parity_disposition_kinds.py diff --git a/.github/workflows/parity-governance.yml b/.github/workflows/parity-governance.yml index c266946f24..2f7e82e65c 100644 --- a/.github/workflows/parity-governance.yml +++ b/.github/workflows/parity-governance.yml @@ -73,11 +73,12 @@ jobs: tests.test_parity_ledger \ tests.test_parity_governance_acceptance \ tests.test_rr_legacy_preservation_contract \ + tests.test_parity_disposition_kinds \ 2>&1 | tee "$RUNNER_TEMP/out.txt" ran=$(grep -oE '^Ran [0-9]+ test' "$RUNNER_TEMP/out.txt" | grep -oE '[0-9]+') echo "collected ${ran:-0} tests" - if [ "${ran:-0}" -lt 113 ]; then - echo "::error::expected at least 113 parity-governance tests, collected ${ran:-0} — discovery is broken, not the suite" + if [ "${ran:-0}" -lt 115 ]; then + echo "::error::expected at least 115 parity-governance tests, collected ${ran:-0} — discovery is broken, not the suite" exit 1 fi working-directory: Tools diff --git a/Tools/parity_ratchet.py b/Tools/parity_ratchet.py index 5129c1a5ef..7ba321bc00 100644 --- a/Tools/parity_ratchet.py +++ b/Tools/parity_ratchet.py @@ -346,14 +346,19 @@ def _required_v3_exemptions( current_findings: set[str], ) -> set[tuple[str, str]]: required: set[tuple[str, str]] = set() - for name in ( - "unpaired_files", - "unpaired_functions", - "unpaired_properties", - "unpaired_constants", + # Explicit singulars rather than name[:-1]: "unpaired_properties" stems to + # "unpaired-propertie", which _validate_dispositions does not accept, so a one-sided + # property could be REQUIRED to carry a disposition that could never be written. Every + # other set survives the naive strip, which is why this went unnoticed: no test had a + # one-sided property until Lift Log added two. + for name, singular in ( + ("unpaired_files", "unpaired-file"), + ("unpaired_functions", "unpaired-function"), + ("unpaired_properties", "unpaired-property"), + ("unpaired_constants", "unpaired-constant"), ): for identity in set(current_sets[name]) - set(base_sets[name]): - required.add((f"add-{name[:-1].replace('_', '-')}", identity)) + required.add((f"add-{singular}", identity)) for name in ("function_pairs", "property_pairs", "constant_pairs"): for identity in set(base_sets[name]) - set(current_sets[name]): required.add((f"remove-{name[:-1].replace('_', '-')}", identity)) diff --git a/Tools/tests/test_parity_disposition_kinds.py b/Tools/tests/test_parity_disposition_kinds.py new file mode 100644 index 0000000000..a0dea4aff1 --- /dev/null +++ b/Tools/tests/test_parity_disposition_kinds.py @@ -0,0 +1,64 @@ +"""Every kind the ratchet can REQUIRE must be a kind a disposition may declare. + +`_required_v3_exemptions` builds its kind strings from the set names, and +`_validate_dispositions` accepts a fixed vocabulary. If those drift apart, the +requirement becomes unsatisfiable: the required kind is rejected by validation, +and the accepted kind never matches the requirement, so the debt can neither be +waived nor cleared and `--refresh-derived` refuses forever. + +That happened with `unpaired_properties`, which a naive `name[:-1]` stems to +`unpaired-propertie`. Every other set survives the strip, which is why no test +caught it until a one-sided property first appeared. +""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +import parity_ledger +import parity_ratchet + + +class DispositionKindVocabularyTests(unittest.TestCase): + EMPTY = { + "unpaired_files": [], "unpaired_functions": [], "unpaired_properties": [], + "unpaired_constants": [], "function_pairs": [], "property_pairs": [], + "constant_pairs": [], + } + + def _required_kinds(self) -> set[str]: + current = dict(self.EMPTY) + identity = "swift" + chr(0) + "X.swift::a/1#1" + for name in ("unpaired_files", "unpaired_functions", + "unpaired_properties", "unpaired_constants"): + current[name] = [identity] + required = parity_ratchet._required_v3_exemptions( + self.EMPTY, current, set(), set() + ) + return {kind for kind, _ in required} + + def test_every_required_add_kind_is_declarable(self) -> None: + identity = "swift" + chr(0) + "X.swift::a/1#1" + for kind in sorted(self._required_kinds()): + doc = { + "schema_version": 1, + "dispositions": [{ + "type": "platform_specific", + "kind": kind, + "identity": identity, + "identity_sha256": parity_ledger._canonical_sha256(identity), + "platform": "swift", + "rationale": "Declared one-sided on purpose for this vocabulary test.", + }], + } + with self.subTest(kind=kind): + parity_ratchet._validate_dispositions(doc, "test") + + def test_property_kind_is_singular(self) -> None: + self.assertIn("add-unpaired-property", self._required_kinds()) + self.assertNotIn("add-unpaired-propertie", self._required_kinds()) + + +if __name__ == "__main__": + unittest.main()