From 7558a4d7506ca4a7ae34f6411152751a81c607e0 Mon Sep 17 00:00:00 2001 From: "John R. D'Orazio" Date: Sat, 8 Aug 2026 03:25:04 +0200 Subject: [PATCH 1/2] Convert validation guards from asserts; add the missing test module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #11. All three guards in generate_seed.py were `assert`s, which Python strips under -O. Running the generator with that flag skipped validation entirely: it exited 0, printed its usual success line, and wrote a seed carrying unresolvable cross-references. The three checks move into a `validate(entries, known_types)` function that raises ValueError, following the shape coecdr already uses for its own generator. main() catches it and exits via sys.exit with a one-line message, so a data problem reads as a diagnostic rather than a traceback. Every problem is now collected and reported together. Repairing seed data took one run per error before; a run that finds a duplicate id, an unknown ctype and a malformed esi: reference now reports all three at once. Adds scripts/test_generate_seed.py, matching crpdr's and coecdr's test modules — this repository had none, which is why the -O behaviour went unnoticed. Thirteen tests: validate accepts the committed seed and null types, rejects duplicate ids, unknown ctype: values and every malformed esi: form including the bare prefix, and reports multiple problems together. The SurvivesOptimizedMode class is the regression guard proper: it runs validate in a subprocess under -O and -OO and requires it to still raise, so reintroducing `assert` here fails the suite. Also pins slugify, including the Italian arcidiocesi case fixed in #2, which had no test. Verified: exit 1 under -O, -OO and no flags with the seed left byte-identical on disk; a valid run still regenerates data/circumscriptions.json unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 3 +- scripts/generate_seed.py | 55 +++++++++---- scripts/test_generate_seed.py | 142 ++++++++++++++++++++++++++++++++++ 3 files changed, 186 insertions(+), 14 deletions(-) create mode 100644 scripts/test_generate_seed.py diff --git a/README.md b/README.md index 1fd3be4..fe8d0c3 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,8 @@ Examples: `circ:us-boston`, `circ:it-roma`, `circ:al-shkodre-pult`, `circ:us-por - [`data/circumscriptions.json`](data/circumscriptions.json) — the seed registry: 2,935 Latin-rite circumscriptions across 203 countries, generated from the Liturgical Calendar API's world dioceses index, each with its draft canonical ID, the API's `diocese_id` as a cross-reference key, name, nation, and (where available) civil province. - [`data/circumscription_types.json`](data/circumscription_types.json) — the companion types registry: draft canonical IDs (`ctype:diocese`, `ctype:archeparchy`, `ctype:territorial-abbacy`, …) for the 16 canonical ranks and juridic forms a circumscription can hold, each with its Latin name, the church it belongs to, whether it is territorial, the title of the one who governs it, and the governing canon. Each circumscription's `type` field is a cross-reference into this file. - [`docs/schema-proposal.md`](docs/schema-proposal.md) — the proposed schema and the open questions for the committee. -- [`scripts/generate_seed.py`](scripts/generate_seed.py) — regenerates the seed from the API's `world_dioceses.json`. +- [`scripts/generate_seed.py`](scripts/generate_seed.py) — regenerates the seed from the API's `world_dioceses.json`, validating every identifier and cross-reference before writing. +- [`scripts/test_generate_seed.py`](scripts/test_generate_seed.py) — unit tests: `python3 -m unittest discover -s scripts -v`. ## Companion registries diff --git a/scripts/generate_seed.py b/scripts/generate_seed.py index d0e1b5b..a273c6e 100644 --- a/scripts/generate_seed.py +++ b/scripts/generate_seed.py @@ -53,6 +53,44 @@ def slugify(name): return s +# A `church_sui_iuris` value is a cross-reference into the CESIDR, a separate +# repository, so it can only be checked for shape here — the bare prefix `esi:` +# resolves to nothing and must not pass. +CHURCH_SUI_IURIS = re.compile(r"esi:[a-z0-9]+(-[a-z0-9]+)*") + + +def validate(entries, known_types): + """Raise ValueError describing every problem found in `entries`. + + Deliberately not `assert`: Python strips assertions under -O, which would + let the generator write unresolvable cross-references and exit 0. Every + problem is collected so that repairing seed data does not take one run per + error. + """ + problems = [] + + ids = [e["id"] for e in entries] + dupes = sorted({i for i in ids if ids.count(i) > 1}) + if dupes: + problems.append(f"duplicate ids: {dupes[:10]}") + + # Every `type` is a cross-reference into data/circumscription_types.json; + # a typo there would silently produce an unresolvable reference. A null + # `type` is expected — the seed is untyped pending an authoritative pass. + unknown = sorted({e["type"] for e in entries + if e["type"] and e["type"] not in known_types}) + if unknown: + problems.append(f"unknown circumscription types: {unknown}") + + bad = sorted({str(e["church_sui_iuris"]) for e in entries + if not CHURCH_SUI_IURIS.fullmatch(str(e["church_sui_iuris"]))}) + if bad: + problems.append(f"malformed church_sui_iuris references: {bad}") + + if problems: + raise ValueError("; ".join(problems)) + + def main(): if len(sys.argv) < 2: sys.exit(__doc__) @@ -88,21 +126,12 @@ def main(): if over.get("note"): entry["note"] = over["note"] entries.append(entry) - ids = [e["id"] for e in entries] - dupes = {i for i in ids if ids.count(i) > 1} - assert not dupes, f"duplicate ids: {sorted(dupes)[:10]}" - # Every `type` is a cross-reference into data/circumscription_types.json; - # a typo there would silently produce an unresolvable reference. types_path = repo_root / "data" / "circumscription_types.json" known = {t["id"] for t in json.load(open(types_path, encoding="utf-8"))["entries"]} - unknown = {e["type"] for e in entries if e["type"] and e["type"] not in known} - assert not unknown, f"unknown circumscription types: {sorted(unknown)}" - # `church_sui_iuris` is a cross-reference into the CESIDR, a separate - # repository, so it can only be checked for shape here. - bad = {e["church_sui_iuris"] for e in entries - if not re.fullmatch(r"esi:[a-z0-9]+(-[a-z0-9]+)*", - str(e["church_sui_iuris"]))} - assert not bad, f"malformed church_sui_iuris references: {sorted(bad)}" + try: + validate(entries, known) + except ValueError as exc: + sys.exit(f"generate_seed.py: {exc}") out = { "$comment": "CECDR seed registry: draft canonical IDs for Catholic " "ecclesiastical circumscriptions, generated from the " diff --git a/scripts/test_generate_seed.py b/scripts/test_generate_seed.py new file mode 100644 index 0000000..a787729 --- /dev/null +++ b/scripts/test_generate_seed.py @@ -0,0 +1,142 @@ +"""Unit tests for generate_seed.py. Run from the repo root: + + python3 -m unittest discover -s scripts -v +""" + +import copy +import json +import subprocess +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import generate_seed as gs + +SCRIPTS = Path(__file__).resolve().parent +REPO = SCRIPTS.parent +SEED = REPO / "data" / "circumscriptions.json" +TYPES = REPO / "data" / "circumscription_types.json" + + +class Validate(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.entries = json.loads(SEED.read_text(encoding="utf-8"))["entries"] + cls.known = {t["id"] for t in + json.loads(TYPES.read_text(encoding="utf-8"))["entries"]} + + def bad_copy(self): + return copy.deepcopy(self.entries[:50]) + + def test_accepts_the_committed_seed(self): + gs.validate(self.entries, self.known) # must not raise + + def test_accepts_null_type(self): + # The seed is untyped pending an authoritative pass; null must pass. + entries = self.bad_copy() + self.assertIsNone(entries[0]["type"]) + gs.validate(entries, self.known) + + def test_rejects_duplicate_id(self): + entries = self.bad_copy() + entries[1]["id"] = entries[0]["id"] + with self.assertRaises(ValueError) as ctx: + gs.validate(entries, self.known) + self.assertIn("duplicate ids", str(ctx.exception)) + + def test_rejects_unknown_circumscription_type(self): + entries = self.bad_copy() + entries[0]["type"] = "ctype:bogus" + with self.assertRaises(ValueError) as ctx: + gs.validate(entries, self.known) + self.assertIn("ctype:bogus", str(ctx.exception)) + + def test_rejects_malformed_church_sui_iuris(self): + # "esi:" is the case that a startswith() check let through: it carries + # no slug and resolves to nothing. + for value in ("esi:", "latin", "esi:Latin", "esi:-latin", "esi:latin-", ""): + with self.subTest(value=value): + entries = self.bad_copy() + entries[0]["church_sui_iuris"] = value + with self.assertRaises(ValueError) as ctx: + gs.validate(entries, self.known) + self.assertIn("malformed church_sui_iuris", str(ctx.exception)) + + def test_accepts_other_well_formed_esi_references(self): + # CESIDR mints 24; the seed only uses esi:latin so far. + for value in ("esi:ukrainian", "esi:syro-malabar", "esi:italo-albanian"): + with self.subTest(value=value): + entries = self.bad_copy() + entries[0]["church_sui_iuris"] = value + gs.validate(entries, self.known) + + def test_reports_every_problem_at_once(self): + entries = self.bad_copy() + entries[1]["id"] = entries[0]["id"] + entries[2]["type"] = "ctype:bogus" + entries[3]["church_sui_iuris"] = "esi:" + with self.assertRaises(ValueError) as ctx: + gs.validate(entries, self.known) + message = str(ctx.exception) + self.assertIn("duplicate ids", message) + self.assertIn("ctype:bogus", message) + self.assertIn("malformed church_sui_iuris", message) + + +class SurvivesOptimizedMode(unittest.TestCase): + """The regression this module exists for. + + Python strips `assert` under -O. When these guards were assertions, running + the generator with -O skipped validation entirely: it exited 0 and wrote + unresolvable cross-references. Any future reintroduction of `assert` here + must fail this test. + """ + + def run_validate(self, flags, snippet): + code = ("import sys; sys.path.insert(0, %r); import generate_seed as gs; %s" + % (str(SCRIPTS), snippet)) + return subprocess.run([sys.executable, *flags, "-c", code], + capture_output=True, text=True) + + def test_guards_still_raise_under_dash_O(self): + entry = {"id": "circ:xx-a", "type": None, "church_sui_iuris": "esi:"} + snippet = "gs.validate([%r], set())" % entry + for flags in ([], ["-O"], ["-OO"]): + with self.subTest(flags=flags or ["(none)"]): + result = self.run_validate(flags, snippet) + self.assertNotEqual(result.returncode, 0) + self.assertIn("ValueError", result.stderr) + + def test_valid_input_passes_under_dash_O(self): + entry = {"id": "circ:xx-a", "type": None, "church_sui_iuris": "esi:latin"} + snippet = "gs.validate([%r], set()); print('ok')" % entry + result = self.run_validate(["-O"], snippet) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "ok") + + +class Slugify(unittest.TestCase): + """Pins the strip rule, including the Italian case fixed in #2.""" + + def test_strips_italian_archdiocesan_form(self): + # "arcidiocesi", not "archdiocesi" — the bug that left 58 slugs wrong. + self.assertEqual(gs.slugify("Arcidiocesi di Acerenza"), "acerenza") + + def test_strips_remaining_styled_forms(self): + self.assertEqual(gs.slugify("Diocesi di Lanusei"), "lanusei") + self.assertEqual(gs.slugify("Archdiocese of Boston"), "boston") + self.assertEqual(gs.slugify("Diocese of Brooklyn"), "brooklyn") + + def test_leaves_bare_see_names_alone(self): + self.assertEqual(gs.slugify("Lezhë"), "lezhe") + self.assertEqual(gs.slugify("Shkodrë-Pult"), "shkodre-pult") + + def test_does_not_strip_type_words_mid_name(self): + # Only the leading styled form is generic; "Diocese" elsewhere is not. + self.assertEqual(gs.slugify("Archdiocese for the Military Services"), + "archdiocese-for-the-military-services") + + +if __name__ == "__main__": + unittest.main() From fb86dac39c47c2a28a9dffc39e71ed57f969e4a3 Mon Sep 17 00:00:00 2001 From: "John R. D'Orazio" Date: Sat, 8 Aug 2026 04:06:59 +0200 Subject: [PATCH 2/2] Test main()'s no-write behaviour, not just validate() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite drove validate() only, so it pinned detection but not the thing that actually went wrong in #11: the generator writing a corrupted seed and exiting 0. Mutating main() to detect the problem and write anyway left the previous suite fully green. Adds MainRefusesToWrite, which runs main() as a subprocess against a synthetic source in a temporary repo and asserts the output file is never created. Covers duplicate ids (two same-named sees with no province, which the generator cannot qualify) and unknown ctype: values (injected through MANUAL), each under no flags, -O and -OO, plus a sound-input case that must still write, and a case proving an existing seed is left byte-intact when a later run fails. church_sui_iuris has no main()-level counterpart: the value is a literal in main() that no source input or MANUAL override can reach, so it is unreachable end-to-end by construction. Noted in the test file and left covered at the validate() level. Also extends the optimized-mode test from one guard to all three, so reintroducing `assert` for any single check fails the suite rather than only for the church_sui_iuris one. 17 tests. Verified by mutation: reverting the guards to asserts fails 23 subtests, and making main() write anyway fails 7 — the latter passing cleanly before this commit. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/test_generate_seed.py | 100 +++++++++++++++++++++++++++++++--- 1 file changed, 92 insertions(+), 8 deletions(-) diff --git a/scripts/test_generate_seed.py b/scripts/test_generate_seed.py index a787729..d271db0 100644 --- a/scripts/test_generate_seed.py +++ b/scripts/test_generate_seed.py @@ -5,8 +5,10 @@ import copy import json +import shutil import subprocess import sys +import tempfile import unittest from pathlib import Path @@ -99,14 +101,20 @@ def run_validate(self, flags, snippet): return subprocess.run([sys.executable, *flags, "-c", code], capture_output=True, text=True) - def test_guards_still_raise_under_dash_O(self): - entry = {"id": "circ:xx-a", "type": None, "church_sui_iuris": "esi:"} - snippet = "gs.validate([%r], set())" % entry - for flags in ([], ["-O"], ["-OO"]): - with self.subTest(flags=flags or ["(none)"]): - result = self.run_validate(flags, snippet) - self.assertNotEqual(result.returncode, 0) - self.assertIn("ValueError", result.stderr) + def test_every_guard_still_raises_under_dash_O(self): + ok = {"id": "circ:xx-a", "type": None, "church_sui_iuris": "esi:latin"} + cases = { + "duplicate id": ([ok, dict(ok)], "set()"), + "unknown type": ([dict(ok, type="ctype:bogus")], "set()"), + "malformed esi": ([dict(ok, church_sui_iuris="esi:")], "set()"), + } + for name, (entries, known) in cases.items(): + snippet = "gs.validate(%r, %s)" % (entries, known) + for flags in ([], ["-O"], ["-OO"]): + with self.subTest(guard=name, flags=flags or ["(none)"]): + result = self.run_validate(flags, snippet) + self.assertNotEqual(result.returncode, 0) + self.assertIn("ValueError", result.stderr) def test_valid_input_passes_under_dash_O(self): entry = {"id": "circ:xx-a", "type": None, "church_sui_iuris": "esi:latin"} @@ -116,6 +124,82 @@ def test_valid_input_passes_under_dash_O(self): self.assertEqual(result.stdout.strip(), "ok") +class MainRefusesToWrite(unittest.TestCase): + """main() must abort *before* writing, not merely detect the problem. + + The #11 regression was end-to-end: exit 0 plus a corrupted seed on disk. + validate() raising is necessary but not sufficient, so these drive main() + against a synthetic source and assert the output file is never created. + """ + + def build(self, dioceses): + tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, tmp, ignore_errors=True) + repo = tmp / "repo" + (repo / "data").mkdir(parents=True) + shutil.copy(TYPES, repo / "data" / "circumscription_types.json") + src = tmp / "world_dioceses.json" + src.write_text(json.dumps({"catholic_dioceses_latin_rite": [ + {"country_iso": "xx", "country_name_english": "Testland", + "dioceses": dioceses}]}), encoding="utf-8") + return src, repo, repo / "data" / "circumscriptions.json" + + def run_main(self, flags, src, repo, inject=""): + code = ("import sys; sys.path.insert(0, %r); import generate_seed as gs; %s" + "sys.argv = ['generate_seed.py', %r, %r]; gs.main()" + % (str(SCRIPTS), inject, str(src), str(repo))) + return subprocess.run([sys.executable, *flags, "-c", code], + capture_output=True, text=True) + + ALPHA_BETA = [{"diocese_name": "Alpha", "diocese_id": "alpha_xx"}, + {"diocese_name": "Beta", "diocese_id": "beta_xx"}] + # Two sees of the same name with no province: the generator cannot qualify + # them, so both collapse onto circ:xx-alpha. + HOMONYMS = [{"diocese_name": "Alpha", "diocese_id": "alpha1_xx"}, + {"diocese_name": "Alpha", "diocese_id": "alpha2_xx"}] + + def test_writes_when_input_is_sound(self): + for flags in ([], ["-O"], ["-OO"]): + with self.subTest(flags=flags or ["(none)"]): + src, repo, out = self.build(self.ALPHA_BETA) + result = self.run_main(flags, src, repo) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(out.exists()) + self.assertEqual( + json.loads(out.read_text(encoding="utf-8"))["entry_count"], 2) + + def test_duplicate_ids_abort_before_writing(self): + for flags in ([], ["-O"], ["-OO"]): + with self.subTest(flags=flags or ["(none)"]): + src, repo, out = self.build(self.HOMONYMS) + result = self.run_main(flags, src, repo) + self.assertNotEqual(result.returncode, 0) + self.assertIn("duplicate ids", result.stderr) + self.assertFalse(out.exists(), "seed was written despite bad input") + + def test_unknown_type_aborts_before_writing(self): + inject = "gs.MANUAL['alpha_xx'] = {'type': 'ctype:bogus'}; " + for flags in ([], ["-O"], ["-OO"]): + with self.subTest(flags=flags or ["(none)"]): + src, repo, out = self.build(self.ALPHA_BETA) + result = self.run_main(flags, src, repo, inject) + self.assertNotEqual(result.returncode, 0) + self.assertIn("ctype:bogus", result.stderr) + self.assertFalse(out.exists(), "seed was written despite bad input") + + def test_an_existing_seed_is_left_untouched_on_failure(self): + src, repo, out = self.build(self.HOMONYMS) + out.write_text("SENTINEL", encoding="utf-8") + result = self.run_main(["-O"], src, repo) + self.assertNotEqual(result.returncode, 0) + self.assertEqual(out.read_text(encoding="utf-8"), "SENTINEL") + + # `church_sui_iuris` has no main()-level counterpart: the value is a literal + # in main() and no source input or MANUAL override can reach it, so it is + # unreachable end-to-end by construction. It is covered at the validate() + # level in SurvivesOptimizedMode instead. + + class Slugify(unittest.TestCase): """Pins the strip rule, including the Italian case fixed in #2."""