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()