From 16353dbdd1de71e6b72539d653d10de68aa6086f Mon Sep 17 00:00:00 2001 From: Jamie Holding Date: Mon, 28 Sep 2026 19:06:16 +0100 Subject: [PATCH] test: a committed mutant list, run nightly My own mutation checks scored 14/14 on this package while an independent 54-mutant sweep found 27 survivors, including a checkpoint that recorded the newest row instead of the page boundary -- the defect 4.0.0 exists to fix -- and a "drift gate" that still passed with 30 of 36 columns dropped. A mutant list written by the author of the tests contains the mutations those tests already catch. That is grading my own homework, and the score was the reassuring kind. So the list is committed and reviewable: what is being checked, and more usefully what is not. Each entry carries why it matters, most of them naming a defect that actually reached a customer. Add one whenever a defect reaches main -- the mutation is the proof the new test would have caught it. Nightly and non-gating: it re-runs the suite once per mutant, which is minutes, and a survivor is information rather than a reason to block a merge. A mutant whose `find` no longer matches counts as a failure too, because a stale mutant has been silently testing nothing. Its first real run found two gaps, and this commit closes both. A budget exhausted on a RESUMED run deleted the archive. The `written == 0` deletion guard existed on the generic failure path and the budget path had the same test with nothing behind it -- and the budget path is the more dangerous of the two, because exit 75 is the ORDINARY outcome of a long back fill: the file vanishes, the next run appends only the tail and records complete. The line was added hours earlier while closing an "an empty file is a lie" finding, which is the same mistake made twice in a day: fix the site in front of me, do not look for its twin. Nothing published ever had the unguarded version. And `str(EntityType.SHOW)` is "EntityType.SHOW", so a customer filtering a CSV on SHOW matches nothing and is told nothing. The JavaScript SDK pins this; Python never did, because every stub in the file hands the writer a plain string and so never exercises the unwrap. One mutant also had to be split: a single `find` string matched both deletion sites, so it tested whichever came first while appearing to cover two. 19/19 killed, 336 tests. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/mutation.yml | 44 ++++++++++ tests/mutation/mutants.json | 156 +++++++++++++++++++++++++++++++++ tests/mutation/run.py | 93 ++++++++++++++++++++ tests/unit/test_backfill.py | 85 +++++++++++++++++- 4 files changed, 377 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/mutation.yml create mode 100644 tests/mutation/mutants.json create mode 100644 tests/mutation/run.py diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml new file mode 100644 index 0000000..425a595 --- /dev/null +++ b/.github/workflows/mutation.yml @@ -0,0 +1,44 @@ +name: Mutation + +# NIGHTLY AND NON-GATING, deliberately. It re-runs the whole suite once per +# mutant, which is minutes rather than the seconds a commit gate can spend, and a +# survivor is information rather than a reason to block a merge. +# +# The mutants are committed (tests/mutation/mutants.json) rather than generated. +# A list written by the author of the tests contains the mutations those tests +# already catch: on 2026-09-28 an author-written set scored 14/14 on this package +# while an independent 54-mutant sweep found 27 survivors, one of them the defect +# that release existed to fix. A list a reviewer can read is the part that makes +# the score mean anything. +on: + schedule: + - cron: '41 4 * * *' + workflow_dispatch: + # On demand from a PR too, because the moment a mutant is worth adding is the + # moment a defect is being fixed. + pull_request: + paths: + - 'tests/mutation/**' + +jobs: + mutate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + - uses: actions/setup-python@v6 + with: + python-version: '3.12' + - run: pip install -e '.[dev]' + # The suite must be green BEFORE anything is mutated, or every mutant + # "survives" against an already-red suite and the run means nothing. + - name: Baseline + run: pytest -q tests/unit + - name: Mutate + id: mutate + continue-on-error: true + run: python tests/mutation/run.py | tee "$GITHUB_STEP_SUMMARY" + - name: Say so in the log when something survived + if: steps.mutate.outcome == 'failure' + run: | + echo "::warning::A mutant survived or went stale — see the job summary." + echo "Not a failure: a survivor is a gap to close, not a merge to block." diff --git a/tests/mutation/mutants.json b/tests/mutation/mutants.json new file mode 100644 index 0000000..f122b4f --- /dev/null +++ b/tests/mutation/mutants.json @@ -0,0 +1,156 @@ +{ + "_comment": [ + "THE MUTANTS, COMMITTED, because the alternative is grading my own homework.", + "", + "A mutation run is only as good as its mutant list, and a list written by the", + "same person as the tests contains the mutations those tests already catch. On", + "2026-09-28 that produced a 14/14 score on this package while an independent", + "reviewer's 54-mutant sweep found 27 survivors -- including a checkpoint that", + "recorded the newest row instead of the page boundary, which is the defect the", + "release existed to fix, and a drift gate that still passed with 30 of 36", + "columns dropped.", + "", + "Committing the list is most of the value: a reviewer can read what is being", + "checked and, more importantly, what is not. Add a mutant whenever a defect", + "reaches main -- the mutation is the proof the new test would have caught it.", + "", + "Each entry replaces `find` with `replace` in `file`, once, then runs the whole", + "suite. A SURVIVOR is a change to production code that breaks nothing, which", + "means either the tests are blind to it or the code does not matter." + ], + "mutants": [ + { + "name": "checkpoint the newest row instead of the page boundary", + "file": "themeparks/backfill.py", + "find": "progress.resume_from = _next_page_start(page.next_url)", + "replace": "progress.resume_from = page.end", + "why": "The defect 4.0.0 exists to fix. An entity that stopped reporting has no rows for the tail days of its page, so a row-derived checkpoint re-downloads days already written." + }, + { + "name": "resume ignores the recorded boundary", + "file": "themeparks/backfill.py", + "find": "resume_at = (state.get(\"resumeFrom\") or state.get(\"lastDay\")) if resuming else None", + "replace": "resume_at = state.get(\"lastDay\") if resuming else None", + "why": "The fallback is for state files with no boundary. Preferring it always reintroduces the duplicate." + }, + { + "name": "a resumed run losing its BUDGET deletes the accumulated file", + "file": "themeparks/backfill.py", + "find": " if progress.written == 0 and not resumed_run:\n # A budget spent before the first page left a 0-byte file that reads\n # as \"this park has no history\".\n out_path.unlink(missing_ok=True)", + "replace": " if progress.written == 0:\n out_path.unlink(missing_ok=True)", + "why": "Exit 75 is the ORDINARY outcome of a long back fill, so this is the more dangerous of the two sites: the file vanishes, the next run appends only the tail and records complete." + }, + { + "name": "a resumed run failing any OTHER way deletes the accumulated file", + "file": "themeparks/backfill.py", + "find": " if progress.written == 0 and not resumed_run:\n out_path.unlink(missing_ok=True)\n raise", + "replace": " if progress.written == 0:\n out_path.unlink(missing_ok=True)\n raise", + "why": "`written` counts rows THIS process wrote. On a resumed run the file holds everything the previous runs fetched." + }, + { + "name": "the column fingerprint is ignored", + "file": "themeparks/backfill.py", + "find": "if state.get(\"columns\") != _columns_fingerprint(fmt):", + "replace": "if False:", + "why": "3.3.0's 19-column file resuming under a 41-column build appends rows no reader can parse." + }, + { + "name": "a state file from the other SDK is accepted", + "file": "themeparks/backfill.py", + "find": "if state.get(\"sdk\") != SDK_NAME:", + "replace": "if False:", + "why": "The two SDKs' keys differ only on the interrupted path, so the safe paths interoperate silently and a cross-SDK resume duplicates." + }, + { + "name": "the format leaves the state filename", + "file": "themeparks/backfill.py", + "find": "return out_dir / f\"{park_id}.{fmt}{STATE_SUFFIX}\"", + "replace": "return out_dir / f\"{park_id}{STATE_SUFFIX}\"", + "why": "One state file for two formats doubles every row on an ndjson -> csv -> ndjson round trip." + }, + { + "name": "a unique substring returns the display label as the park name", + "file": "themeparks/backfill.py", + "find": " if len(park_hits) == 1:\n return park_hits", + "replace": " if len(park_hits) == 1:\n return [(pid, f\"{pname} (x)\") for pid, pname in park_hits]", + "why": "Shipped in 3.3.0: parkName read 'Magic Kingdom Park (Walt Disney World\u00ae Resort)' for ~94,000 rows." + }, + { + "name": "normalize becomes a bare casefold", + "file": "themeparks/backfill.py", + "find": " folded = unicodedata.normalize(\"NFKD\", value.casefold())\n return \"\".join(c for c in folded if c.isalnum() and not unicodedata.combining(c))", + "replace": " return value.casefold()", + "why": "Five live park names carry \u00ae, a curly apostrophe or an accent, and the documented example is one of them." + }, + { + "name": "an exact destination no longer beats an exact park", + "file": "themeparks/backfill.py", + "find": " if len(exact_dest) == 1:\n return parks_in(next(iter(exact_dest)))", + "replace": " if False:\n return parks_in(next(iter(exact_dest)))", + "why": "Seven real names are both a destination and one of its several parks. Cedar Point would download one of two." + }, + { + "name": "the CSV writer stops quoting a carriage return", + "file": "themeparks/backfill.py", + "find": " if any(ch in text for ch in ('\"', \",\", \"\\r\", \"\\n\")):", + "replace": " if any(ch in text for ch in ('\"', \",\", \"\\n\")):", + "why": "Python's csv module did exactly this on 3.9 and 3.10, and one row parsed as two." + }, + { + "name": "the CSV loses its BOM", + "file": "themeparks/backfill.py", + "find": "handle.write(\"\\ufeff\" + _csv_line(dict(zip(CSV_COLUMNS, CSV_COLUMNS))))", + "replace": "handle.write(_csv_line(dict(zip(CSV_COLUMNS, CSV_COLUMNS))))", + "why": "Excel on Windows then reads the local code page and mangles every \u00ae and accent." + }, + { + "name": "a negative number is prefixed as a formula", + "file": "themeparks/backfill.py", + "find": " if not text.startswith(_FORMULA_LEADERS) or _NUMERIC.match(text):", + "replace": " if not text.startswith(_FORMULA_LEADERS):", + "why": "Every negative value in the file becomes text and arithmetic breaks in the tool the prefixing protects." + }, + { + "name": "the row renames the run", + "file": "themeparks/backfill.py", + "find": " payload.update(identity)", + "replace": " pass", + "why": "Models keep undeclared fields, so a row carrying its own entityId would relabel every line of a paid export." + }, + { + "name": "a collection of models is flattened as a nested block", + "file": "themeparks/backfill.py", + "find": " if get_origin(annotation) in _COLLECTION_ORIGINS:\n return None\n", + "replace": "", + "why": "Phantom columns in the header, then AttributeError on the first row, the moment the API adds an array of objects." + }, + { + "name": "a network failure exits 1 instead of 75", + "file": "themeparks/backfill.py", + "find": " return EX_TEMPFAIL\n except ThemeParksError as exc:", + "replace": " return 1\n except ThemeParksError as exc:", + "why": "A resumable failure should make a scheduler retry, not alert." + }, + { + "name": "the page hook fires before its rows", + "file": "themeparks/_ergonomic/history.py", + "find": " yield from _daily_entity_rows(envelope)\n # AFTER the rows, never before: a caller checkpointing on this has to\n # be able to trust that everything the page held is already written.\n if on_page is not None:\n on_page(_page_of(envelope))", + "replace": " if on_page is not None:\n on_page(_page_of(envelope))\n yield from _daily_entity_rows(envelope)", + "why": "A consumer that died mid-page would record a checkpoint past rows it never wrote. Losing rows is worse than duplicating them." + }, + { + "name": "the entity label comes from somewhere other than the response", + "file": "themeparks/_ergonomic/history.py", + "find": " inner = getattr(kind, \"value\", kind)", + "replace": " inner = kind", + "why": "`str(EntityType.SHOW)` is 'EntityType.SHOW', so a customer filtering on 'SHOW' matches nothing and is told nothing." + }, + { + "name": "models stop keeping undeclared fields", + "file": "themeparks/_models_base.py", + "find": " model_config = ConfigDict(extra=\"allow\")", + "replace": " model_config = ConfigDict(extra=\"ignore\")", + "why": "A stale vendored spec then deletes real data at parse time, which is how unknownMinutes, inParkHours and extremeWaits were lost." + } + ] +} diff --git a/tests/mutation/run.py b/tests/mutation/run.py new file mode 100644 index 0000000..e3f3126 --- /dev/null +++ b/tests/mutation/run.py @@ -0,0 +1,93 @@ +"""Apply each committed mutant, run the suite, and report the survivors. + +A survivor is a change to production code that breaks no test: either the tests +are blind to it, or the code does not matter. Both are worth knowing and neither +is worth blocking a commit over, so this is a nightly job rather than a gate -- +it re-runs the whole suite once per mutant, which is minutes, not seconds. + +WHY THE LIST IS COMMITTED rather than generated: a list written by the author of +the tests contains the mutations those tests already catch. On 2026-09-28 an +author-written set scored 14/14 on this package while an independent 54-mutant +sweep found 27 survivors, one of which was the defect that release existed to +fix. A reviewable list is the part that makes the score mean anything. + +Usage: + python tests/mutation/run.py # every mutant + python tests/mutation/run.py --list # names only, runs nothing +""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +MUTANTS = Path(__file__).with_name("mutants.json") + + +def run_suite() -> bool: + """True when the suite passes. Quiet: only the verdict matters here.""" + proc = subprocess.run( + [sys.executable, "-m", "pytest", "-q", "-x", "--no-header", "tests/unit"], + cwd=ROOT, + capture_output=True, + text=True, + check=False, + ) + return proc.returncode == 0 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--list", action="store_true", help="print the mutants and exit") + args = parser.parse_args() + + mutants = json.loads(MUTANTS.read_text(encoding="utf-8"))["mutants"] + if args.list: + for m in mutants: + print(f"{m['name']}\n {m['file']}: {m['why']}") + return 0 + + # A mutant whose `find` no longer matches is NOT a pass. The code moved and + # nobody updated the mutant, so it has been silently testing nothing -- which + # is the same failure mode as a test that cannot fail. + stale: list[str] = [] + survived: list[dict[str, str]] = [] + killed = 0 + + for mutant in mutants: + path = ROOT / mutant["file"] + original = path.read_text(encoding="utf-8") + if mutant["find"] not in original: + stale.append(mutant["name"]) + print(f"STALE {mutant['name']}", flush=True) + continue + path.write_text(original.replace(mutant["find"], mutant["replace"], 1), encoding="utf-8") + try: + passed = run_suite() + finally: + # Restored whatever happened, including a KeyboardInterrupt: leaving a + # mutated working tree behind is worse than any result. + path.write_text(original, encoding="utf-8") + if passed: + survived.append(mutant) + print(f"SURVIVED {mutant['name']}", flush=True) + else: + killed += 1 + print(f"killed {mutant['name']}", flush=True) + + total = len(mutants) + print(f"\n{killed}/{total} killed, {len(survived)} survived, {len(stale)} stale") + for m in survived: + print(f"\nSURVIVED: {m['name']}\n {m['file']}\n {m['why']}") + for name in stale: + print(f"\nSTALE: {name}\n its `find` no longer matches; the mutant is testing nothing") + + return 1 if (survived or stale) else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/unit/test_backfill.py b/tests/unit/test_backfill.py index 61ead2e..8dcc998 100644 --- a/tests/unit/test_backfill.py +++ b/tests/unit/test_backfill.py @@ -23,6 +23,7 @@ import inspect import json from datetime import date +from enum import Enum from pathlib import Path from typing import Union @@ -31,7 +32,13 @@ from themeparks import APIError, BudgetExhaustedError, NetworkError, RateLimitError, backfill from themeparks._client import PACKAGE_VERSION -from themeparks._ergonomic.history import EntityRef, HistoryApi, HistoryPage, HistorySpan +from themeparks._ergonomic.history import ( + EntityRef, + HistoryApi, + HistoryPage, + HistorySpan, + _ref, +) from themeparks._generated.models import ( HistoryDailyRow, HistoryDailyStats, @@ -1262,3 +1269,79 @@ def test_the_contract_is_not_trivially_satisfiable(self) -> None: assert len(contract["columns"]) == 41 assert len(contract["fingerprint"]) == 16 assert contract["columns"][:5] == backfill.IDENTITY_COLUMNS + + +class TestTheBudgetPathAlsoKeepsAnEarlierRunsRows: + """Exit 75 on a RESUMED run must not delete what earlier runs downloaded. + + Found by the committed mutant list, not by review: the guard existed on the + generic failure path and the budget path had the same `written == 0` test + with no test behind it. It is the more dangerous of the two, because exit 75 + is the ORDINARY outcome of a long back fill -- a scheduler hits it, the file + vanishes, the next run appends only the tail and records `complete: true`. + """ + + def _resumed(self, tmp_path: Path) -> None: + (tmp_path / "p.ndjson").write_text('{"row": 1}\n{"row": 2}\n', encoding="utf-8") + _state_file(tmp_path, lastDay="2025-06-30", resumeFrom="2025-07-01") + + def test_a_spent_budget_on_a_resumed_run_keeps_the_file(self, tmp_path: Path) -> None: + self._resumed(tmp_path) + hist = _History(archive_from="2025-01-01", through="2026-09-28", floor=None) + + def days_with_entities(start=None, end=None, *, max_wait=120.0, on_page=None): + raise BudgetExhaustedError("429", status=429, body={}, url="u", retry_after=2700.0) + yield # pragma: no cover - keeps this a generator + + hist.days_with_entities = days_with_entities # type: ignore[assignment] + code = backfill.backfill_park(_Client(hist), _Park("p", "P"), tmp_path, "ndjson") + + assert code == backfill.EX_TEMPFAIL + assert (tmp_path / "p.ndjson").exists(), "a retryable failure destroyed the archive" + assert (tmp_path / "p.ndjson").read_text(encoding="utf-8").count("\n") == 2 + + def test_a_spent_budget_on_a_first_run_leaves_no_empty_file(self, tmp_path: Path) -> None: + # The other half, so the guard cannot become "never delete": a 0-byte file + # reads as "this park has no history". + hist = _History(archive_from="2025-01-01", through="2026-09-28", floor=None) + + def days_with_entities(start=None, end=None, *, max_wait=120.0, on_page=None): + raise BudgetExhaustedError("429", status=429, body={}, url="u", retry_after=2700.0) + yield # pragma: no cover + + hist.days_with_entities = days_with_entities # type: ignore[assignment] + assert backfill.backfill_park(_Client(hist), _Park("p", "P"), tmp_path, "ndjson") == ( + backfill.EX_TEMPFAIL + ) + assert not (tmp_path / "p.ndjson").exists() + + +class TestTheEntityTypeIsTheApisString: + def test_an_enum_entity_type_is_unwrapped_to_its_value(self) -> None: + """`str(EntityType.SHOW)` is 'EntityType.SHOW', not 'SHOW'. + + A customer filtering a CSV on 'SHOW' matches nothing and is told nothing. + The JavaScript SDK pins this; Python had no equivalent, because every + stub in this file hands the writer a plain string and so never exercises + the unwrap. Found by the committed mutant list. + """ + + class FakeEntityType(str, Enum): + SHOW = "SHOW" + + class FakeEntity: + id = "ent-1" + name = "Fantasmic!" + entityType = FakeEntityType.SHOW # noqa: N815 - the API's own spelling + + ref = _ref(FakeEntity()) + assert ref.entity_type == "SHOW" + assert "EntityType" not in ref.entity_type + + def test_a_plain_string_entity_type_is_unchanged(self) -> None: + class FakeEntity: + id = "ent-1" + name = "Space Mountain" + entityType = "ATTRACTION" # noqa: N815 - the API's own spelling + + assert _ref(FakeEntity()).entity_type == "ATTRACTION"