diff --git a/.coverage b/.coverage deleted file mode 100644 index 84c8fe98..00000000 Binary files a/.coverage and /dev/null differ diff --git a/.github/actions/release-to-pypi-uv/tests/_helpers.py b/.github/actions/release-to-pypi-uv/tests/_helpers.py index 758bc728..6bb39c89 100644 --- a/.github/actions/release-to-pypi-uv/tests/_helpers.py +++ b/.github/actions/release-to-pypi-uv/tests/_helpers.py @@ -3,7 +3,6 @@ from __future__ import annotations import importlib.util -import os import sys import typing as typ from pathlib import Path @@ -11,21 +10,8 @@ if typ.TYPE_CHECKING: # pragma: no cover - imported for annotations only from types import ModuleType -if _ACTION_PATH := os.environ.get("GITHUB_ACTION_PATH"): - _action_root = Path(_ACTION_PATH).resolve() - scripts_candidate = _action_root / "scripts" - if scripts_candidate.is_dir(): - SCRIPTS_DIR = scripts_candidate - try: - REPO_ROOT = _action_root.parents[2] - except IndexError: - REPO_ROOT = scripts_candidate.parents[3] - else: - SCRIPTS_DIR = Path(__file__).resolve().parents[1] / "scripts" - REPO_ROOT = SCRIPTS_DIR.parents[3] -else: - SCRIPTS_DIR = Path(__file__).resolve().parents[1] / "scripts" - REPO_ROOT = SCRIPTS_DIR.parents[3] +SCRIPTS_DIR = Path(__file__).resolve().parents[1] / "scripts" +REPO_ROOT = SCRIPTS_DIR.parents[3] def load_script_module(name: str) -> ModuleType: diff --git a/.gitignore b/.gitignore index 4bb8ceaa..6e7146d6 100644 --- a/.gitignore +++ b/.gitignore @@ -145,6 +145,7 @@ __pycache__/ venv/ .env.* .pytest_cache/ +.coverage .mypy_cache/ *.egg-info/ diff --git a/.rules/python-00.md b/.rules/python-00.md index 11d89b61..9837b169 100644 --- a/.rules/python-00.md +++ b/.rules/python-00.md @@ -127,6 +127,7 @@ def login_user(username: str, password: str) -> bool: def test_login_success(): assert login_user("alice", "correct-password") is True + def test_login_failure(): assert not login_user("alice", "wrong-password") ``` diff --git a/.rules/python-context-managers.md b/.rules/python-context-managers.md index 49fb600f..dd524b44 100644 --- a/.rules/python-context-managers.md +++ b/.rules/python-context-managers.md @@ -23,6 +23,7 @@ Use this for straightforward procedural setup/teardown: ```python from contextlib import contextmanager + @contextmanager def managed_file(path: str, mode: str): f = open(path, mode) @@ -31,6 +32,7 @@ def managed_file(path: str, mode: str): finally: f.close() + # Usage: with managed_file("/tmp/data.txt", "w") as f: f.write("hello") @@ -53,6 +55,7 @@ class Resource: def __exit__(self, exc_type, exc_val, exc_tb): self.conn.close() + # Usage: with Resource() as conn: conn.send("ping") diff --git a/.rules/python-exception-design-raising-handling-and-logging.md b/.rules/python-exception-design-raising-handling-and-logging.md index e39da2e0..12219863 100644 --- a/.rules/python-exception-design-raising-handling-and-logging.md +++ b/.rules/python-exception-design-raising-handling-and-logging.md @@ -14,6 +14,7 @@ class enables callers to catch all domain failures without vendor leakage. class PaymentsError(Exception): """All payment-layer errors.""" + class CardDeclinedError(PaymentsError): # ✅ ends with Error (N818) def __init__(self, code: str, *, retry_after: int | None = None): super().__init__(f"Card declined ({code})") @@ -114,13 +115,14 @@ clarifies intent. ```python import logging + logger = logging.getLogger(__name__) # ❌ LOG issues -logging.warning(f"failed for {user_id}") # f-string (LOG004/LOG014) -logging.warning("failed for %s" % user_id) # %-formatting (LOG007) -logging.warn("deprecated") # warn() (LOG009) -logging.error("bad root logger") # root logger usage (LOG015) +logging.warning(f"failed for {user_id}") # f-string (LOG004/LOG014) +logging.warning("failed for %s" % user_id) # %-formatting (LOG007) +logging.warn("deprecated") # warn() (LOG009) +logging.error("bad root logger") # root logger usage (LOG015) # ✅ Correct logger.warning("Failed for user_id=%s", user_id) # lazy interpolation @@ -203,7 +205,7 @@ def charge(amount_pennies: int, card_token: str) -> str: try: return gateway.charge(amount_pennies, card_token) except gateway.Timeout as exc: - raise PaymentsError("Gateway timeout") from exc # ✅ TRY201 + raise PaymentsError("Gateway timeout") from exc # ✅ TRY201 except gateway.CardDeclined as exc: raise CardDeclinedError(exc.code, retry_after=60) from exc ``` @@ -227,6 +229,7 @@ def must_have_key(d: dict, key: str) -> None: msg = f"Missing required key: {key!r}" raise KeyError(msg) + logger.info("Dispatching order_id=%s to shop_id=%s", order_id, shop_id) # structured ``` diff --git a/.rules/python-generators.md b/.rules/python-generators.md index 2abbc161..527e3384 100644 --- a/.rules/python-generators.md +++ b/.rules/python-generators.md @@ -34,6 +34,7 @@ def iter_user_names(users): if user.active and user.name: yield user.name.upper() + def get_names(users): return list(iter_user_names(users)) ``` @@ -50,11 +51,10 @@ def get_names(users): ```python from itertools import islice + def top_active_emails(users): emails = ( - user.email.lower() - for user in users - if user.active and user.email is not None + user.email.lower() for user in users if user.active and user.email is not None ) return list(islice(emails, 10)) ``` diff --git a/.rules/python-return.md b/.rules/python-return.md index e6953931..7879439b 100644 --- a/.rules/python-return.md +++ b/.rules/python-return.md @@ -11,6 +11,7 @@ Follow these rules: def func(): return None + # GOOD: def func(): return @@ -30,6 +31,7 @@ def func(x): return x # implicitly returns None (bad) + # GOOD: def func(x): if x > 0: @@ -50,6 +52,7 @@ def func(x): return x # no return (bad) + # GOOD: def func(x): if x > 0: @@ -69,6 +72,7 @@ def func(): result = compute() return result + # GOOD: def func(): return compute() diff --git a/.rules/python-typing.md b/.rules/python-typing.md index e9b1d34b..83e29903 100644 --- a/.rules/python-typing.md +++ b/.rules/python-typing.md @@ -13,14 +13,17 @@ with integers or strings is required (e.g. for database or JSON serialization). ```python import enum + class Status(enum.Enum): PENDING = enum.auto() COMPLETE = enum.auto() + class ErrorCode(enum.IntEnum): OK = 0 NOT_FOUND = 404 + class Role(enum.StrEnum): ADMIN = enum.auto() GUEST = enum.auto() @@ -65,6 +68,7 @@ returns the same instance. ```python import typing + class Builder: def add(self, value: int) -> typing.Self: self.values.append(value) @@ -81,9 +85,10 @@ enables static analysis tools to detect typos and signature mismatches. ```python import typing + class Base: - def run(self) -> None: - ... + def run(self) -> None: ... + class Child(Base): @typing.override @@ -101,6 +106,7 @@ checkers. ```python import typing + def is_str_list(val: list[object]) -> typing.TypeIs[list[str]]: return all(isinstance(x, str) for x in val) ``` @@ -116,6 +122,7 @@ type is provided. ```python T = typing.TypeVar("T", default=int) + class Box[T]: def __init__(self, value: T = T()): self.value = value diff --git a/docs/cmd-mox-users-guide.md b/docs/cmd-mox-users-guide.md index 3c6356ce..529a9dc0 100644 --- a/docs/cmd-mox-users-guide.md +++ b/docs/cmd-mox-users-guide.md @@ -80,9 +80,9 @@ behaviour. Combine methods to describe how a command should be invoked: ```python -cmd_mox.mock("git") \ - .with_args("clone", "https://example.com/repo.git") \ - .returns(exit_code=0) +cmd_mox.mock("git").with_args("clone", "https://example.com/repo.git").returns( + exit_code=0 +) ``` Arguments can be matched more flexibly using comparators: @@ -90,8 +90,9 @@ Arguments can be matched more flexibly using comparators: ```python from cmd_mox import Regex, Contains -cmd_mox.mock("curl") \ - .with_matching_args(Regex(r"--header=User-Agent:.*"), Contains("example")) +cmd_mox.mock("curl").with_matching_args( + Regex(r"--header=User-Agent:.*"), Contains("example") +) ``` The design document lists the available comparators: diff --git a/docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md b/docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md index 8ec7a05e..058d780b 100644 --- a/docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md +++ b/docs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.md @@ -730,6 +730,7 @@ The implementation introduces one new error type and a guard function: ```python class ArchitectureBoundaryError(Exception): """Raised when orchestration code violates hexagonal architecture boundaries.""" + pass ``` @@ -757,6 +758,7 @@ from episodic.orchestration._checkpoint_payload import ( _planner_result_from_payload, ) + @given( # Strategy TBD based on actual DTO types st.just(...) @@ -765,6 +767,7 @@ def test_checkpoint_payload_round_trip(payload: dict) -> None: """Assert checkpoint payloads round-trip without data loss.""" # Implementation TBD + def test_checkpoint_payload_boundary_purity(payload: dict) -> None: """Assert checkpoint payloads contain no adapter types.""" # Implementation TBD diff --git a/docs/execplans/support-cranelift-codegen.md b/docs/execplans/support-cranelift-codegen.md index 16411e0e..fe7049f8 100644 --- a/docs/execplans/support-cranelift-codegen.md +++ b/docs/execplans/support-cranelift-codegen.md @@ -484,7 +484,7 @@ Inspect each log file. All must pass. If any fail, fix the issue and re-run. cargo_config_dir = tmp_path / ".cargo" cargo_config_dir.mkdir() (cargo_config_dir / "config.toml").write_text( - '[unstable]\ncodegen-backend = true\n\n' + "[unstable]\ncodegen-backend = true\n\n" '[profile.dev]\ncodegen-backend = "cranelift"\n\n' '[profile.test]\ncodegen-backend = "cranelift"\n', ) diff --git a/docs/local-validation-of-github-actions-with-act-and-pytest.md b/docs/local-validation-of-github-actions-with-act-and-pytest.md index 8b5559fb..ba271e08 100644 --- a/docs/local-validation-of-github-actions-with-act-and-pytest.md +++ b/docs/local-validation-of-github-actions-with-act-and-pytest.md @@ -211,6 +211,7 @@ loop: ```python from cmd_mox import CmdMox + def test_record(tmp_path: Path) -> None: artifact_dir = tmp_path / "act-artifacts" with CmdMox() as mox: diff --git a/docs/python-action-scripts.md b/docs/python-action-scripts.md index b5a2bbe0..97e7aab6 100644 --- a/docs/python-action-scripts.md +++ b/docs/python-action-scripts.md @@ -35,8 +35,7 @@ app.config = (*tuple(getattr(app, "config", ())), _env_config) @app.default -def main(*, bin_name: str, version: str, formats: list[str] | None = None) -> None: - ... +def main(*, bin_name: str, version: str, formats: list[str] | None = None) -> None: ... if __name__ == "__main__": diff --git a/docs/scripting-standards.md b/docs/scripting-standards.md index ed094422..535c023c 100644 --- a/docs/scripting-standards.md +++ b/docs/scripting-standards.md @@ -100,16 +100,16 @@ def main( # Required parameters bin_name: Annotated[str, Parameter(required=True)], version: Annotated[str, Parameter(required=True)], - # Optional scalars package_name: Optional[str] = None, target: Optional[str] = None, outdir: Optional[Path] = None, dry_run: bool = False, - # Lists (whitespace/newline separated by default) formats: list[str] | None = None, - man_paths: Annotated[list[Path] | None, Parameter(env_var="INPUT_MAN_PATHS")] = None, + man_paths: Annotated[ + list[Path] | None, Parameter(env_var="INPUT_MAN_PATHS") + ] = None, deb_depends: list[str] | None = None, rpm_depends: list[str] | None = None, ): @@ -257,7 +257,9 @@ f.write_text("1.2.3\n", encoding="utf-8") version = f.read_text(encoding="utf-8").strip() # Atomic write pattern (tmp → replace) -with tempfile.NamedTemporaryFile("w", delete=False, dir=f.parent, encoding="utf-8") as tmp: +with tempfile.NamedTemporaryFile( + "w", delete=False, dir=f.parent, encoding="utf-8" +) as tmp: tmp.write("new-contents\n") tmp_path = Path(tmp.name) @@ -302,6 +304,7 @@ from plumbum.cmd import git app = App(config=cyclopts.config.Env("INPUT_", command=False)) + @app.default def main( *, @@ -326,6 +329,7 @@ def main( "dist": str(dist), }) + if __name__ == "__main__": app() ``` diff --git a/workflow_scripts/mutation_detect_changes.py b/workflow_scripts/mutation_detect_changes.py index 93b797de..455674a9 100644 --- a/workflow_scripts/mutation_detect_changes.py +++ b/workflow_scripts/mutation_detect_changes.py @@ -274,11 +274,22 @@ def full_run_matrix(config: DetectionConfig) -> list[MatrixEntry]: return entries +def _root_first_key(item: tuple[str, list[str]]) -> tuple[bool, str]: + """Sort key placing the root (``.``) target first, then alphabetical. + + ``.`` sorts before every directory name, so the root-first order also + falls out of a plain alphabetical sort. + """ + root_rank = item[0] != "." + alphabetical_key = item[0] # pragma: no mutate - plain sorting is equivalent + return (root_rank, alphabetical_key) + + def scoped_run_matrix( buckets: dict[str, list[str]], config: DetectionConfig ) -> list[MatrixEntry]: """Build the single-shard matrix for a scoped (scheduled) run.""" - ordered = sorted(buckets.items(), key=lambda item: (item[0] != ".", item[0])) + ordered = sorted(buckets.items(), key=_root_first_key) # pragma: no mutate del config # scoped runs never shard; kept for signature symmetry return [ MatrixEntry( @@ -311,7 +322,8 @@ def _write_skip_summary(config: DetectionConfig) -> None: summary_path = os.environ.get("GITHUB_STEP_SUMMARY") if not summary_path: return - with Path(summary_path).open("a", encoding="utf-8") as handle: + encoding = "utf-8" # pragma: no mutate - locale-independent UTF-8 alias + with Path(summary_path).open("a", encoding=encoding) as handle: handle.write( SKIP_SUMMARY_TEMPLATE.format( base_ref=config.base_ref, window_hours=config.window_hours diff --git a/workflow_scripts/mutation_run_mutmut.py b/workflow_scripts/mutation_run_mutmut.py index 2be1a158..700f6036 100644 --- a/workflow_scripts/mutation_run_mutmut.py +++ b/workflow_scripts/mutation_run_mutmut.py @@ -142,7 +142,9 @@ def _parse_result_line(line: str) -> MutantResult | None: space-free mutant name containing the ``__mutmut_`` marker; anything else (warnings, blank lines, progress output) is noise. """ - name, separator, status = line.partition(": ") + # Result lines carry one delimiter, so partition and rpartition are equivalent. + split_result = line.partition # pragma: no mutate + name, separator, status = split_result(": ") if not separator: return None is_mutant_name = " " not in name and "__mutmut_" in name @@ -254,7 +256,9 @@ def _run_mutation_testing( *globs, ] emit("mutation_mutmut_command", ["uv", *run_arguments]) - code = local["uv"][run_arguments] & RETCODE(FG=True) + # pragma: RETCODE(FG=...) only toggles console streaming; the captured + # exit code is identical, so no in-process test can observe the change. + code = local["uv"][run_arguments] & RETCODE(FG=True) # pragma: no mutate emit("mutation_mutmut_exit_code", code) if code != 0: emit( @@ -270,9 +274,11 @@ def _publish_results(mutmut_version: str, results_file: str, summary_path: str) results_text = local["uv"][ *_mutmut_command(mutmut_version), "results", "--all", "true" ]() - Path(results_file).write_text(results_text, encoding="utf-8") + # pragma below: encoding is a locale-independent UTF-8 codec alias. + Path(results_file).write_text(results_text, encoding="utf-8") # pragma: no mutate results = parse_results(results_text) - with Path(summary_path).open("a", encoding="utf-8") as handle: + # pragma below: encoding is a locale-independent UTF-8 codec alias. + with Path(summary_path).open("a", encoding="utf-8") as handle: # pragma: no mutate handle.write(render_summary(results)) emit("mutation_mutmut_counts", count_statuses(results)) diff --git a/workflow_scripts/mutation_summarize_cargo.py b/workflow_scripts/mutation_summarize_cargo.py index 2cdf1c33..13b0ebe8 100644 --- a/workflow_scripts/mutation_summarize_cargo.py +++ b/workflow_scripts/mutation_summarize_cargo.py @@ -184,7 +184,9 @@ def collect_reports(report_root: Path) -> list[TargetReport]: emit("mutation_summary_missing_outcomes", artefact_dir.name) continue try: - payload = json.loads(outcomes_path.read_text(encoding="utf-8")) + # pragma below: encoding is a locale-independent UTF-8 codec alias. + raw = outcomes_path.read_text(encoding="utf-8") # pragma: no mutate + payload = json.loads(raw) except json.JSONDecodeError as error: emit("mutation_summary_invalid_outcomes", f"{artefact_dir.name}: {error}") continue diff --git a/workflow_scripts/tests/test_graphql_client.py b/workflow_scripts/tests/test_graphql_client.py new file mode 100644 index 00000000..12ba201e --- /dev/null +++ b/workflow_scripts/tests/test_graphql_client.py @@ -0,0 +1,99 @@ +"""Unit tests for the GitHub GraphQL client retry and backoff logic. + +These tests pin the retry and backoff boundaries harvested in #336; the +response-parsing, rate-limit, and status-code survivors remain that +issue's remit. +""" + +from __future__ import annotations + +import pytest + +from workflow_scripts import graphql_client + +# Test-only constant (not a real credential) +TEST_TOKEN: str = "test-token" # noqa: S105 + + +class TestShouldRetry: + """Boundary of the retry-budget predicate.""" + + def test_retries_remain_below_the_limit(self) -> None: + """An attempt below the limit still has retries left.""" + assert graphql_client._should_retry(2, 3) is True, ( + "an attempt below max_retries should still retry" + ) + + def test_final_attempt_does_not_retry(self) -> None: + """The attempt equal to the limit is the last (strict boundary).""" + assert graphql_client._should_retry(3, 3) is False, ( + "the attempt equal to max_retries must not retry" + ) + + +class TestBackoffSleep: + """Exponential backoff schedule.""" + + @pytest.mark.parametrize(("attempt", "expected"), [(0, 1.0), (3, 8.0)]) + def test_exponential_schedule( + self, monkeypatch: pytest.MonkeyPatch, attempt: int, expected: float + ) -> None: + """Backoff sleeps ``base * 2**attempt`` seconds for each attempt.""" + recorded: list[float] = [] + monkeypatch.setattr(graphql_client.time, "sleep", recorded.append) + graphql_client._backoff_sleep(attempt) + assert recorded == [expected], ( + f"attempt {attempt} should back off {expected} seconds" + ) + + +class TestAttemptRetryOrFail: + """The retry-or-fail decision that guards estate-wide automerge.""" + + def test_backs_off_on_the_current_attempt_when_retries_remain( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A remaining retry backs off on the current attempt without failing.""" + recorded: list[int] = [] + monkeypatch.setattr(graphql_client, "_backoff_sleep", recorded.append) + graphql_client._attempt_retry_or_fail(0, 3, "boom") + assert recorded == [0], ( + "a remaining retry should back off on the current attempt, not fail" + ) + + def test_fails_with_message_when_budget_exhausted( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + """An exhausted budget fails with the supplied error message.""" + monkeypatch.setattr(graphql_client, "_backoff_sleep", lambda *_: None) + with pytest.raises(SystemExit) as excinfo: + graphql_client._attempt_retry_or_fail(3, 3, "budget-exhausted-message") + assert excinfo.value.code == 1, "an exhausted budget should exit with code 1" + assert "budget-exhausted-message" in capsys.readouterr().err, ( + "the failure should carry the supplied error message" + ) + + +class TestRequestGraphqlRetries: + """The retry loop's attempt budget.""" + + def test_connection_errors_retry_then_fail( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A persistent connection error retries three times, then fails.""" + seen: list[tuple[str, str, dict[str, object]]] = [] + + def fake_execute(token: str, query: str, variables: dict[str, object]) -> None: + seen.append((token, query, variables)) + + monkeypatch.setattr(graphql_client, "_execute_graphql_attempt", fake_execute) + monkeypatch.setattr(graphql_client, "_backoff_sleep", lambda *_: None) + variables: dict[str, object] = {"n": 1} + with pytest.raises(SystemExit): + graphql_client.request_graphql(TEST_TOKEN, "query {}", variables) + assert len(seen) == 4, ( + "one initial attempt plus three retries should run before failing" + ) + assert seen == [(TEST_TOKEN, "query {}", variables)] * 4, ( + "the token, query, and variables should reach each attempt unchanged" + ) diff --git a/workflow_scripts/tests/test_mutation_detect_changes.py b/workflow_scripts/tests/test_mutation_detect_changes.py index b5bd7d8d..1fe385da 100644 --- a/workflow_scripts/tests/test_mutation_detect_changes.py +++ b/workflow_scripts/tests/test_mutation_detect_changes.py @@ -1,4 +1,9 @@ -"""Unit tests for the mutation change-detection helper script.""" +"""Unit tests for the mutation change-detection helper script. + +The extra-crate shape, target-directory, sorted-matrix-key, and +skip-summary base-ref/window assertions below kill the +``mutation_detect_changes`` matrix-contract survivors tracked in #340. +""" from __future__ import annotations @@ -133,6 +138,8 @@ def test_full_run_shards_root_only(self) -> None: assert len(extra) == 1 assert extra[0].shard_count == 1 assert extra[0].slug == "testkit" + assert extra[0].files == "", "extra crates run unscoped (empty files)" + assert extra[0].shard == 0, "single-shard extras use shard index 0" def test_scoped_run_strips_extra_dir_prefix(self) -> None: """Scoped entries carry files relative to their target directory.""" @@ -143,6 +150,9 @@ def test_scoped_run_strips_extra_dir_prefix(self) -> None: } entries = detect.scoped_run_matrix(buckets, config) assert [e.slug for e in entries] == ["root", "testkit"] + assert [e.dir for e in entries] == [".", "testkit"], ( + "each entry should carry its own target directory" + ) assert entries[0].files == "src/a.rs src/b.rs" assert entries[1].files == "src/lib.rs" assert all(e.shard == 0 and e.shard_count == 1 for e in entries) @@ -154,6 +164,10 @@ def test_matrix_json_shape(self) -> None: assert list(payload) == ["include"] assert payload["include"][0]["dir"] == "." assert payload["include"][0]["slug"] == "root" + first = payload["include"][0] + assert list(first) == sorted(first), ( + "matrix entry keys should be serialized in sorted order" + ) class TestMainEntry: @@ -208,6 +222,8 @@ def test_schedule_without_changes_skips( assert json.loads(outputs["matrix"]) == {"include": []} summary = (tmp_path / "github_summary").read_text(encoding="utf-8") assert "Mutation testing skipped" in summary + assert "`HEAD`" in summary, "the skip message should name the base ref" + assert "25 hours" in summary, "the skip message should name the window" def test_schedule_with_changes_scopes( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, git_repo: Path diff --git a/workflow_scripts/tests/test_mutation_properties.py b/workflow_scripts/tests/test_mutation_properties.py index a375708a..16e85dab 100644 --- a/workflow_scripts/tests/test_mutation_properties.py +++ b/workflow_scripts/tests/test_mutation_properties.py @@ -9,9 +9,12 @@ from __future__ import annotations +import unittest.mock as mock + from hypothesis import given from hypothesis import strategies as st +from workflow_scripts import graphql_client from workflow_scripts import mutation_detect_changes as detect from workflow_scripts import mutation_run_mutmut as run_mutmut from workflow_scripts import mutation_summarize_cargo as summarize @@ -21,6 +24,50 @@ COUNTED = st.sampled_from(["CaughtMutant", "MissedMutant", "Timeout", "Unviable"]) +@given( + attempt=st.integers(min_value=0, max_value=20), + max_retries=st.integers(min_value=0, max_value=20), +) +def test_retry_budget_is_strict( + attempt: int, + max_retries: int, +) -> None: + """Exactly the attempts below the retry limit have budget remaining.""" + assert graphql_client._should_retry(attempt, max_retries) is (attempt < max_retries) + + +@given( + attempt=st.integers(min_value=0, max_value=20), + base_seconds=st.integers(min_value=1, max_value=10), +) +def test_backoff_is_base_times_power_of_two( + attempt: int, + base_seconds: int, +) -> None: + """Backoff grows exponentially from the supplied base duration.""" + recorded: list[float] = [] + with mock.patch.object(graphql_client.time, "sleep", recorded.append): + graphql_client._backoff_sleep(attempt, base_seconds) + assert recorded == [base_seconds * (2**attempt)] + + +@given( + extras=st.lists(SEGMENT, unique=True, max_size=8), + include_root=st.booleans(), +) +def test_scoped_matrix_orders_root_first_then_alphabetically( + extras: list[str], + *, + include_root: bool, +) -> None: + """Root leads every scoped matrix and remaining targets are alphabetical.""" + targets = [*extras, *(["."] if include_root else [])] + buckets = {target: [f"{target}/src/lib.rs"] for target in reversed(targets)} + entries = detect.scoped_run_matrix(buckets, detect.DetectionConfig()) + expected = sorted(targets, key=lambda target: (target != ".", target)) + assert [entry.dir for entry in entries] == expected + + @given(tokens=st.lists(SEGMENT, max_size=8)) def test_split_csv_round_trips_clean_tokens(tokens: list[str]) -> None: """Joining tokens with padded commas and splitting returns them.""" diff --git a/workflow_scripts/tests/test_mutation_run_cargo.py b/workflow_scripts/tests/test_mutation_run_cargo.py index 0af87cfd..10ddd92e 100644 --- a/workflow_scripts/tests/test_mutation_run_cargo.py +++ b/workflow_scripts/tests/test_mutation_run_cargo.py @@ -1,4 +1,8 @@ -"""Unit tests for the cargo-mutants run wrapper script.""" +"""Unit tests for the cargo-mutants run wrapper script. + +The shard-boundary and exit-code-meaning tests below kill the +``mutation_run_cargo`` survivors tracked in #342. +""" from __future__ import annotations @@ -40,6 +44,29 @@ def test_files_become_repeated_file_arguments(self) -> None: ) assert arguments[-4:] == ["--file", "src/a.rs", "--file", "src/b.rs"] + def test_shard_boundary_at_two_emits_shard(self) -> None: + """A shard-count of exactly two still emits ``--shard`` (boundary). + + Kills the shard-count boundary survivors tracked in #342. + """ + arguments = run_cargo.build_arguments( + run_cargo.MutantsInvocation(shard=0, shard_count=2) + ) + assert "--shard" in arguments, "shard-count 2 should still emit --shard" + assert arguments[arguments.index("--shard") + 1] == "0/2", ( + "the shard argument should read index/count" + ) + + def test_single_shard_omits_shard(self) -> None: + """A shard-count of one leaves ``--shard`` out (boundary). + + Kills the shard-count boundary survivors tracked in #342. + """ + arguments = run_cargo.build_arguments( + run_cargo.MutantsInvocation(shard=0, shard_count=1) + ) + assert "--shard" not in arguments, "shard-count 1 should not emit --shard" + def test_shard_and_dir_and_excludes(self) -> None: """Sharding, target dir, and exclude globs are all emitted.""" arguments = run_cargo.build_arguments( @@ -70,19 +97,34 @@ def test_extra_args_are_shell_lexed(self) -> None: class TestInterpretExitCode: """Classification of cargo-mutants exit codes.""" - @pytest.mark.parametrize("code", [0, 2, 3]) - def test_informative_codes_succeed(self, code: int) -> None: - """Missed mutants and timeouts are informative outcomes.""" - success, meaning = run_cargo.interpret_exit_code(code) - assert success - assert meaning - - @pytest.mark.parametrize("code", [1, 4, 70, 99]) - def test_fault_codes_fail(self, code: int) -> None: - """Usage errors, failing baselines, and unknowns are faults.""" - success, meaning = run_cargo.interpret_exit_code(code) - assert not success - assert meaning + @pytest.mark.parametrize( + ("code", "meaning"), + [(0, "all mutants caught"), (2, "missed mutants"), (3, "test timeouts")], + ) + def test_informative_codes_succeed(self, code: int, meaning: str) -> None: + """Informative codes succeed and report their exact contract meaning.""" + success, actual = run_cargo.interpret_exit_code(code) + assert success, f"code {code} should be classified as informative success" + assert actual == meaning, ( + f"code {code} should map to the exact meaning {meaning!r}" + ) + + @pytest.mark.parametrize( + ("code", "meaning"), + [ + (1, "usage error"), + (4, "baseline tests failing"), + (70, "internal error"), + (99, "unexpected exit code"), + ], + ) + def test_fault_codes_fail(self, code: int, meaning: str) -> None: + """Faults report their exact meaning; unknown codes fall back verbatim.""" + success, actual = run_cargo.interpret_exit_code(code) + assert not success, f"code {code} should be classified as a fault" + assert actual == meaning, ( + f"code {code} should map to the exact meaning {meaning!r}" + ) @pytest.fixture diff --git a/workflow_scripts/tests/test_mutation_run_mutmut.py b/workflow_scripts/tests/test_mutation_run_mutmut.py index 08b0a1ff..829ec227 100644 --- a/workflow_scripts/tests/test_mutation_run_mutmut.py +++ b/workflow_scripts/tests/test_mutation_run_mutmut.py @@ -1,4 +1,9 @@ -"""Unit tests for the mutmut run-and-summarize helper script.""" +"""Unit tests for the mutmut run-and-summarize helper script. + +The module-glob prefix, noise-rejection, status-accumulation, exact +survivor/summary Markdown, and ``uv run --with`` argv tests below kill +the ``mutation_run_mutmut`` survivors tracked in #341. +""" from __future__ import annotations @@ -57,6 +62,18 @@ def test_non_python_and_bare_prefix_are_ignored(self) -> None: "non-Python paths and bare prefixes should produce no globs" ) + def test_file_outside_prefix_keeps_full_module_path(self) -> None: + """A file outside the strip prefix is translated without stripping.""" + assert run_mutmut.files_to_module_globs("other/mod.py", "src/") == [ + "other.mod.*" + ], "files outside the prefix should keep their full module path" + + def test_prefix_strip_removes_only_trailing_separator(self) -> None: + """Only the trailing separator is stripped; leading ones are kept.""" + assert run_mutmut.files_to_module_globs("src/mod.py", "/src/") == [ + "src.mod.*" + ], "a leading slash in the prefix must not strip the file's own prefix" + class TestParseResults: """Parsing of ``mutmut results --all true`` output.""" @@ -74,6 +91,12 @@ def test_result_lines_parse_and_noise_is_ignored(self) -> None: "parsed results should retain the mutant name" ) + def test_names_with_spaces_are_treated_as_noise(self) -> None: + """A ``name: status`` line whose name has a space is not a result.""" + assert run_mutmut.parse_results("foo bar__mutmut_2: survived") == [], ( + "a spaced name should be rejected as noise, not parsed as a mutant" + ) + def test_counts_group_by_status(self) -> None: """Status counts aggregate across mutants.""" results = run_mutmut.parse_results(RESULTS_TEXT) @@ -84,6 +107,16 @@ def test_counts_group_by_status(self) -> None: "timeout": 1, }, "status counts should aggregate across mutants" + def test_counts_accumulate_repeated_status(self) -> None: + """A status seen twice is counted twice, not reset to one.""" + results = [ + run_mutmut.MutantResult(name="a__mutmut_1", status="killed"), + run_mutmut.MutantResult(name="b__mutmut_1", status="killed"), + ] + assert run_mutmut.count_statuses(results) == {"killed": 2}, ( + "repeated statuses should accumulate rather than reset" + ) + class TestRenderSummary: """Markdown rendering of mutmut results.""" @@ -105,11 +138,56 @@ def test_survivor_table_lists_survived_and_untested(self) -> None: "timed-out mutants should not appear in the survivor table" ) - def test_empty_results_render_message(self) -> None: - """No mutants yields an explanatory message.""" - assert "No mutants were tested." in run_mutmut.render_summary([]), ( - "empty results should render an explanatory message" + def test_exact_markdown_layout(self) -> None: + """The rendered Markdown matches the contracted layout byte-for-byte.""" + results = [ + run_mutmut.MutantResult(name="pkg.mod.x_f__mutmut_1", status="killed"), + run_mutmut.MutantResult(name="pkg.mod.x_f__mutmut_2", status="killed"), + run_mutmut.MutantResult(name="pkg.mod.x_g__mutmut_1", status="survived"), + run_mutmut.MutantResult(name="pkg.mod.x_h__mutmut_1", status="no tests"), + ] + expected = ( + "## Mutation testing results (mutmut)\n" + "\n" + "- **killed:** 2\n" + "- **no tests:** 1\n" + "- **survived:** 1\n" + "\n" + "### Surviving mutants\n" + "\n" + "Inspect a survivor with `uv run mutmut show `.\n" + "\n" + "| Mutant | Status |\n" + "| ------ | ------ |\n" + "| `pkg.mod.x_g__mutmut_1` | survived |\n" + "| `pkg.mod.x_h__mutmut_1` | no tests |\n" ) + assert run_mutmut.render_summary(results) == expected, ( + "the summary Markdown should match the contracted layout exactly" + ) + + def test_empty_results_render_message(self) -> None: + """No mutants yields the exact explanatory message.""" + assert ( + run_mutmut.render_summary([]) + == "## Mutation testing results (mutmut)\n\nNo mutants were tested.\n" + ), "empty results should render the exact explanatory message" + + +class TestMutmutCommand: + """Construction of the ``uv run --with`` argument prefix. + + Kills the ``uv run --with`` argv-contract survivors tracked in #341. + """ + + def test_pins_version_and_subcommands(self) -> None: + """The prefix injects the pinned mutmut and the run subcommand.""" + assert run_mutmut._mutmut_command("3.6.0") == [ + "run", + "--with", + "mutmut==3.6.0", + "mutmut", + ], "the uv prefix should pin the requested mutmut version verbatim" @pytest.fixture @@ -157,7 +235,11 @@ def _prepare( @POSIX_SHIMS_ONLY def test_scoped_run_passes_module_globs( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fake_uv: Path + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + fake_uv: Path, + capsys: pytest.CaptureFixture[str], ) -> None: """Changed files reach mutmut as module globs; summary is written.""" summary_file, results_file = self._prepare(tmp_path, monkeypatch) @@ -167,18 +249,54 @@ def test_scoped_run_passes_module_globs( assert "mutmut run mypkg.calc.*" in recorded.replace(" ", " "), ( "changed files should reach mutmut run as module globs" ) + # Both the run and results sub-invocations must pin the version, so a + # dropped version anywhere leaves a bare ``mutmut==None`` behind. + assert "mutmut==None" not in recorded, ( + "every mutmut sub-invocation should pin the requested version" + ) + assert recorded.count("mutmut==3.6.0") >= 2, ( + "both the run and results invocations should pin the version" + ) + for token in ("results", "--all", "true"): + assert token in recorded.split(), ( + f"the results invocation should pass the {token!r} argument" + ) assert "survived" in results_file.read_text(encoding="utf-8"), ( "the results file should capture the mutmut results output" ) assert "Surviving mutants" in summary_file.read_text(encoding="utf-8"), ( "the job summary should include the survivors section" ) + # Structured diagnostics carry the contracted keys and values. + out = capsys.readouterr().out + command_line = next( + line for line in out.splitlines() if "mutation_mutmut_command=" in line + ) + assert command_line.startswith('mutation_mutmut_command=["uv"'), ( + "the command diagnostic should name uv under its exact key" + ) + assert "mutmut==3.6.0" in command_line, ( + "the command diagnostic should record the pinned version" + ) + assert "mutation_mutmut_exit_code=0" in out, ( + "a clean run should report exit code 0 under its exact key" + ) + counts_line = next( + line for line in out.splitlines() if "mutation_mutmut_counts=" in line + ) + assert "survived" in counts_line, ( + "the counts diagnostic should carry the parsed status tallies" + ) @POSIX_SHIMS_ONLY + @pytest.mark.usefixtures("fake_uv") def test_failing_baseline_propagates_exit_code( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fake_uv: Path + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], ) -> None: - """A non-zero mutmut run fails the step with mutmut's own code.""" + """A non-zero mutmut run fails the step and logs to stderr.""" self._prepare(tmp_path, monkeypatch) monkeypatch.setenv("FAKE_MUTMUT_RUN_EXIT", "3") monkeypatch.setitem(local.env, "FAKE_MUTMUT_RUN_EXIT", "3") @@ -187,6 +305,13 @@ def test_failing_baseline_propagates_exit_code( assert excinfo.value.code == 3, ( "a failing mutmut run should propagate its exit code" ) + captured = capsys.readouterr() + assert "mutation_mutmut_error=mutmut run failed with exit code 3" in ( + captured.err + ), "the failure diagnostic and message should be written to stderr" + assert "mutation_mutmut_error=" not in captured.out, ( + "the failure diagnostic must not leak onto stdout" + ) def test_scope_without_python_files_short_circuits( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fake_uv: Path diff --git a/workflow_scripts/tests/test_mutation_summarize_cargo.py b/workflow_scripts/tests/test_mutation_summarize_cargo.py index eb373aea..babb8ce1 100644 --- a/workflow_scripts/tests/test_mutation_summarize_cargo.py +++ b/workflow_scripts/tests/test_mutation_summarize_cargo.py @@ -1,4 +1,9 @@ -"""Unit tests for the cargo-mutants summary merge script.""" +"""Unit tests for the cargo-mutants summary merge script. + +The non-dict-outcome, survivor-placeholder, timeout/unviable-count, +root-first-ordering, diagnostics, and exact-Markdown tests below kill the +``mutation_summarize_cargo`` survivors tracked in #342. +""" from __future__ import annotations @@ -68,6 +73,42 @@ def test_empty_payload_is_harmless(self) -> None: assert sum(counts.values()) == 0, "an empty payload should yield zero counts" assert survivors == [], "an empty payload should yield no survivors" + def test_non_dict_outcome_is_skipped_not_fatal(self) -> None: + """A non-dict outcome is skipped; later valid outcomes still count.""" + payload = {"outcomes": ["garbage", _mutant_outcome("CaughtMutant")]} + counts, _ = summarize.parse_outcomes(payload) + assert counts["CaughtMutant"] == 1, ( + "parsing should continue past a non-dict outcome, not stop" + ) + + +class TestSurvivorFrom: + """Extraction of a surviving mutant from one scenario object. + + Kills the ``_survivor_from`` placeholder-defaulting survivors tracked + in #342. + """ + + _PLACEHOLDER = summarize.SurvivingMutant(file="?", line=0, name="?") + + def test_non_dict_mutant_yields_placeholder(self) -> None: + """A scenario whose ``Mutant`` is not a dict yields the placeholder.""" + assert summarize._survivor_from({"Mutant": "nope"}) == self._PLACEHOLDER, ( + "a non-dict Mutant should fall back to the ?/0/? placeholder" + ) + + def test_absent_mutant_key_yields_placeholder(self) -> None: + """A scenario without a ``Mutant`` key yields the placeholder.""" + assert summarize._survivor_from({}) == self._PLACEHOLDER, ( + "a missing Mutant key should fall back to the ?/0/? placeholder" + ) + + def test_empty_mutant_defaults_each_field(self) -> None: + """An empty ``Mutant`` dict defaults file, line, and name.""" + assert summarize._survivor_from({"Mutant": {}}) == self._PLACEHOLDER, ( + "an empty Mutant should default file='?', line=0, and name='?'" + ) + class TestCollectReports: """Merging of shard artefact directories.""" @@ -103,6 +144,63 @@ def test_shards_merge_per_target(self, tmp_path: Path) -> None: "survivors should pool across a target's shards" ) + def test_timeout_and_unviable_counts_are_carried(self, tmp_path: Path) -> None: + """Timeout and unviable outcomes reach the merged report counts.""" + _write_report( + tmp_path, + "mutation-report-root-0", + [ + _mutant_outcome("Timeout"), + _mutant_outcome("Unviable"), + _mutant_outcome("Unviable"), + ], + ) + report = summarize.collect_reports(tmp_path)[0] + assert report.timeout == 1, "the timeout count should reach the report" + assert report.unviable == 2, "the unviable count should reach the report" + + def test_root_target_sorts_first(self, tmp_path: Path) -> None: + """The root target leads even when another slug sorts before it.""" + _write_report( + tmp_path, "mutation-report-aaa-0", [_mutant_outcome("CaughtMutant")] + ) + _write_report( + tmp_path, "mutation-report-root-0", [_mutant_outcome("CaughtMutant")] + ) + reports = summarize.collect_reports(tmp_path) + assert [report.slug for report in reports] == ["root", "aaa"], ( + "root should sort first, ahead of the alphabetically-earlier slug" + ) + + def test_diagnostics_and_foreign_dir_does_not_halt( + self, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Skipped, missing, and invalid dirs emit diagnostics without halting.""" + (tmp_path / "0-foreign").mkdir() + (tmp_path / "mutation-report-a-0").mkdir() # missing outcomes.json + invalid = tmp_path / "mutation-report-b-0" + invalid.mkdir() + (invalid / "outcomes.json").write_text("not json", encoding="utf-8") + _write_report( + tmp_path, "mutation-report-root-0", [_mutant_outcome("CaughtMutant")] + ) + reports = summarize.collect_reports(tmp_path) + out = capsys.readouterr().out + assert "mutation_summary_skipped_dir=0-foreign" in out, ( + "an unmatched directory should emit the skipped-dir diagnostic" + ) + assert "mutation_summary_missing_outcomes=mutation-report-a-0" in out, ( + "a directory without outcomes.json should emit the missing diagnostic" + ) + assert "mutation_summary_invalid_outcomes=mutation-report-b-0:" in out, ( + "invalid JSON should emit the invalid-outcomes diagnostic with the name" + ) + # The foreign dir sorts first; scanning must continue to the root report. + assert [report.slug for report in reports] == ["root"], ( + "a leading foreign directory must not halt the scan" + ) + assert reports[0].caught == 1, "the trailing valid report should be collected" + def test_malformed_and_foreign_dirs_are_skipped(self, tmp_path: Path) -> None: """Unrelated directories and invalid JSON do not break the merge.""" (tmp_path / "unrelated").mkdir() @@ -142,12 +240,45 @@ def test_survivor_table_and_counts(self, tmp_path: Path) -> None: "the survivor table should escape pipe characters" ) - def test_no_reports_message(self) -> None: - """An empty report set renders an explanatory message.""" - assert "No reports were produced" in summarize.render_summary([]), ( - "an empty report set should render an explanatory message" + def test_exact_markdown_layout(self) -> None: + """The rendered Markdown matches the contracted layout byte-for-byte.""" + report = summarize.TargetReport( + slug="root", + caught=2, + missed=1, + timeout=3, + unviable=0, + survivors=( + summarize.SurvivingMutant( + file="src/a.rs", line=7, name="replace + with -" + ), + ), + ) + expected = ( + "## Mutation testing results (root)\n" + "\n" + "- **Caught:** 2\n" + "- **Missed (survived):** 1\n" + "- **Timeout:** 3\n" + "- **Unviable:** 0\n" + "\n" + "### Surviving mutants\n" + "\n" + "| File | Line | Mutation |\n" + "| ---- | ---- | -------- |\n" + "| src/a.rs | 7 | replace + with - |\n" + ) + assert summarize.render_summary([report]) == expected, ( + "the summary Markdown should match the contracted layout exactly" ) + def test_no_reports_message(self) -> None: + """An empty report set renders the exact explanatory message.""" + assert ( + summarize.render_summary([]) + == "## Mutation testing results\n\nNo reports were produced.\n" + ), "an empty report set should render the exact explanatory message" + class TestMainEntry: """End-to-end behaviour of the CLI entry point."""