Kill mutation-testing runner, graphql retry, and matrix survivors (#336, #340, #341, #342) - #355
Kill mutation-testing runner, graphql retry, and matrix survivors (#336, #340, #341, #342)#355leynos wants to merge 9 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
Validation
WalkthroughStrengthen mutation workflow contracts with targeted tests for ordering, boundaries, parsing, diagnostics, encoding, and Markdown output. Add GraphQL client coverage for retry limits, exponential backoff, exhausted retries, and persistent connection failures. ChangesMutation workflow contracts
GraphQL retry coverage
Supporting repository updates
Possibly related issues
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5142c7a80d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| falls out of a plain alphabetical sort; the sort-key variants are | ||
| equivalent and cannot be distinguished by dir-prefixed file paths. | ||
| """ | ||
| return (item[0] != ".", item[0]) # pragma: no mutate |
There was a problem hiding this comment.
Do not suppress this entire return expression: the line-wide pragma also excludes non-equivalent mutants such as changing != to ==, which would place every non-root target before root. The new root-first test would kill that regression, but mutmut now skips it and therefore overstates the matrix contract's mutation coverage; isolate only the genuinely equivalent constant/key variant on a separately suppressed line.
Useful? React with 👍 / 👎.
| """ | ||
| name, separator, status = line.partition(": ") | ||
| # pragma: result lines carry a single ": ", so rpartition matches partition. | ||
| name, separator, status = line.partition(": ") # pragma: no mutate |
There was a problem hiding this comment.
Do not suppress the whole parsing statement merely to ignore the partition/rpartition equivalent: the pragma also excludes non-equivalent mutations to the ": " delimiter. Such a regression would silently discard every mutmut result and publish an empty-results summary, while the added parsing tests would otherwise detect it; restructure the statement so only the equivalent method choice is suppressed.
Useful? React with 👍 / 👎.
| 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 |
There was a problem hiding this comment.
Do not apply a line-wide suppression to this open call: although the comment justifies excluding the encoding variant, the same pragma also hides non-equivalent mutations to the "a" file mode, which can make summary publication fail or stop appending. Move the deliberately ignored encoding value onto a separately suppressed expression so mutation runs continue checking the output-mode contract.
Useful? React with 👍 / 👎.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workflow_scripts/tests/test_graphql_client.py`:
- Line 15: Annotate the TEST_TOKEN constant with the explicit str type in the
test module, preserving its existing value and behavior. This maintains full
static type coverage as required by the project’s Pyright typing guidelines.
- Around line 86-99: Update the retry assertions in the test around fake_execute
and request_graphql to validate every recorded call, not only seen[0]. Keep the
expected four-call count, then assert that all entries in seen match the
unchanged TEST_TOKEN, query string, and variables for each retry attempt.
In `@workflow_scripts/tests/test_mutation_run_cargo.py`:
- Around line 104-125: Add failure messages to both assertions in
test_informative_codes_succeed and test_fault_codes_fail, including the tested
code and expected contract classification so mismatches clearly identify the
failing context.
In `@workflow_scripts/tests/test_mutation_run_mutmut.py`:
- Around line 291-299: Update test_failing_baseline_propagates_exit_code by
removing the unused fake_uv parameter and adding
pytest.mark.usefixtures("fake_uv") alongside the existing decorator, preserving
the fixture setup side effects without triggering Ruff ARG002.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b11869cd-15aa-4530-a5b6-262a29978d81
📒 Files selected for processing (8)
workflow_scripts/mutation_detect_changes.pyworkflow_scripts/mutation_run_mutmut.pyworkflow_scripts/mutation_summarize_cargo.pyworkflow_scripts/tests/test_graphql_client.pyworkflow_scripts/tests/test_mutation_detect_changes.pyworkflow_scripts/tests/test_mutation_run_cargo.pyworkflow_scripts/tests/test_mutation_run_mutmut.pyworkflow_scripts/tests/test_mutation_summarize_cargo.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/polythene(auto-detected)
| from workflow_scripts import graphql_client | ||
|
|
||
| # Test-only constant (not a real credential) | ||
| TEST_TOKEN = "test-token" # noqa: S105 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Annotate the test token constant.
Declare TEST_TOKEN: str to maintain the required full static type coverage.
Proposed fix
-TEST_TOKEN = "test-token" # noqa: S105
+TEST_TOKEN: str = "test-token" # noqa: S105As per coding guidelines, “Enable and maintain full static type coverage using Pyright type-checking. Use typing everywhere.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| TEST_TOKEN = "test-token" # noqa: S105 | |
| TEST_TOKEN: str = "test-token" # noqa: S105 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflow_scripts/tests/test_graphql_client.py` at line 15, Annotate the
TEST_TOKEN constant with the explicit str type in the test module, preserving
its existing value and behavior. This maintains full static type coverage as
required by the project’s Pyright typing guidelines.
Source: Coding guidelines
| 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[0] == (TEST_TOKEN, "query {}", variables), ( | ||
| "the token, query, and variables should reach each attempt unchanged" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert arguments for every retry attempt.
Replace the first-call-only assertion. It passes when later retries alter the token, query, or variables, leaving the stated retry contract untested. Snapshot each call and assert all four calls match the expected arguments.
Proposed fix
- seen: list[tuple[object, object, object]] = []
+ 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))
+ seen.append((token, query, variables.copy()))
@@
- assert seen[0] == (TEST_TOKEN, "query {}", variables), (
- "the token, query, and variables should reach each attempt unchanged"
+ expected_call = (TEST_TOKEN, "query {}", {"n": 1})
+ assert seen == [expected_call] * 4, (
+ "the token, query, and variables should reach every attempt unchanged"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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[0] == (TEST_TOKEN, "query {}", variables), ( | |
| "the token, query, and variables should reach each attempt unchanged" | |
| ) | |
| 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.copy())) | |
| 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" | |
| ) | |
| expected_call = (TEST_TOKEN, "query {}", {"n": 1}) | |
| assert seen == [expected_call] * 4, ( | |
| "the token, query, and variables should reach every attempt unchanged" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflow_scripts/tests/test_graphql_client.py` around lines 86 - 99, Update
the retry assertions in the test around fake_execute and request_graphql to
validate every recorded call, not only seen[0]. Keep the expected four-call
count, then assert that all entries in seen match the unchanged TEST_TOKEN,
query string, and variables for each retry attempt.
| 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 | ||
| assert meaning | ||
| assert actual == meaning, ( | ||
| f"code {code} should map to the exact meaning {meaning!r}" | ||
| ) | ||
|
|
||
| @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) | ||
| @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 | ||
| assert meaning | ||
| assert actual == meaning, ( |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add messages to both boolean assertions.
Include failure context when the success classification differs from the contract.
Proposed fix
- assert success
+ assert success, f"code {code} should be an informative success"
...
- assert not success
+ assert not success, f"code {code} should be classified as a fault"As per coding guidelines, “Use assert …, "message" over bare asserts”.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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 | |
| assert meaning | |
| assert actual == meaning, ( | |
| f"code {code} should map to the exact meaning {meaning!r}" | |
| ) | |
| @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) | |
| @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 | |
| assert meaning | |
| assert actual == meaning, ( | |
| 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 an 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, ( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflow_scripts/tests/test_mutation_run_cargo.py` around lines 104 - 125,
Add failure messages to both assertions in test_informative_codes_succeed and
test_fault_codes_fail, including the tested code and expected contract
classification so mismatches clearly identify the failing context.
Sources: Coding guidelines, Path instructions
| @POSIX_SHIMS_ONLY | ||
| def test_failing_baseline_propagates_exit_code( | ||
| 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: | ||
| """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.""" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 --type=py 'def fake_uv\b|`@pytest`\.fixture' workflow_scripts/testsRepository: leynos/shared-actions
Length of output: 2083
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '180,320p' workflow_scripts/tests/test_mutation_run_mutmut.pyRepository: leynos/shared-actions
Length of output: 6044
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("workflow_scripts/tests/test_mutation_run_mutmut.py")
for start, end in [(190, 220), (285, 305)]:
print(f"--- {start}-{end} ---")
for i, line in enumerate(path.read_text().splitlines(), start=1):
if start <= i <= end:
print(f"{i:4d}: {line}")
PYRepository: leynos/shared-actions
Length of output: 2467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\bfake_uv\b' workflow_scripts/tests/test_mutation_run_mutmut.pyRepository: leynos/shared-actions
Length of output: 470
Activate fake_uv via usefixtures. Remove the unused fake_uv parameter from test_failing_baseline_propagates_exit_code and add @pytest.mark.usefixtures("fake_uv") so the setup side effects stay in place without Ruff ARG002.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 296-296: Unused method argument: fake_uv
(ARG002)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflow_scripts/tests/test_mutation_run_mutmut.py` around lines 291 - 299,
Update test_failing_baseline_propagates_exit_code by removing the unused fake_uv
parameter and adding pytest.mark.usefixtures("fake_uv") alongside the existing
decorator, preserving the fixture setup side effects without triggering Ruff
ARG002.
Sources: Coding guidelines, Linters/SAST tools
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Address the mutation-testing survivors harvested in #342, covering the cargo-mutants runner (`mutation_run_cargo`) and the shard-report summariser (`mutation_summarize_cargo`). These scripts render the estate's survivor tables, so gaps here corrupt downstream triage. - Add shard-count boundary cases (1 and 2) so the ``--shard`` emission threshold is pinned, and assert exact exit-code meanings so the contract's meaning strings and unknown-code fallback are covered. - Cover the summariser's survivor extraction placeholder path, its default field values, the timeout and unviable counts, root-first ordering, diagnostic emissions for skipped/missing/invalid artefacts, the non-halting scan past a leading foreign directory, and the exact job-summary Markdown layout. - Annotate the locale-independent UTF-8 read as ``# pragma: no mutate``.
Address the mutation-testing survivors harvested in #341 for the mutmut runner (`mutation_run_mutmut`), which parses mutmut output into the survivor tables the estate relies on. - Pin the module-glob translation: a file outside the strip prefix keeps its full module path, and only the trailing separator is stripped. - Reject spaced names as noise, accumulate repeated status counts, and assert the exact survivor-table and summary Markdown layouts. - Pin the ``uv run --with`` argument prefix, and assert the run and results sub-invocations both pin the version and carry the results arguments, that a clean run reports its command/exit-code/counts diagnostics under their exact keys, and that a failing baseline logs its error diagnostic to stderr with mutmut's own exit code. - Annotate the partition/rpartition equivalence, the streaming-only RETCODE(FG=...) toggle, and the locale-independent UTF-8 writes as ``# pragma: no mutate``. Two rstrip-variant mutants on the prefix-strip line are genuine equivalents (PurePosixPath normalises trailing separators); they share a line with a killable variant, so they are left to #341 rather than masking the killable mutant with a pragma.
Address the retry and backoff boundary survivors harvested in #336. This retry logic guards the dependabot auto-merge estate, so a weak boundary here silently degrades transient-failure handling everywhere it is used. - Pin the strict ``attempt < max_retries`` retry boundary and the exponential ``base * 2**attempt`` backoff schedule (previously no-tests) via a monkeypatched sleep. - Pin the retry-or-fail decision: a remaining retry backs off on the current attempt, and an exhausted budget fails with the supplied message. - Pin the request loop's attempt budget (one attempt plus three retries) and that the token, query, and variables reach each attempt unchanged. The response-parsing, rate-limit, and status-code survivors, plus the unreachable defensive-tail mutants and the equivalent ``range`` upper bound, remain #336's remit and are deferred there.
Address the matrix-contract survivors harvested in #340 for the change-detection guard, whose matrix output decides which shard mutates what. - Pin the full-run extra-crate entry shape (empty files, shard 0), the scoped entries' target directories, the sorted matrix-JSON key order, and that the skip summary names the base ref and window. - Extract the root-first sort key into ``_root_first_key`` so its equivalent variants can be suppressed structurally with ``# pragma: no mutate`` (a trailing pragma cannot reach a multi-line ``sorted`` call), and annotate the locale-independent UTF-8 write. The changed-files git-argument, slug, and remaining survivors stay #340's remit and are deferred there to keep the PR within tolerance.
Match the Makefile formatter (unpinned ruff), which collapses the fake execute signature onto a single line.
Keep retry arguments, matrix ordering, result delimiters, and summary append behaviour observable to mutation testing. Add property coverage for the retry and ordering invariants, and improve failure diagnostics in the runner tests. Resolve formatter drift in Python documentation and make the `release-to-pypi-uv` test loader independent of ambient `GITHUB_ACTION_PATH` values under xdist.
Keep validation and stop hooks from leaving the `.coverage` database as an untracked worktree change.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Delete the coverage database introduced on `main` so validation cannot rewrite tracked test state. The existing ignore rule keeps future coverage runs from recreating a worktree change.
b5e5fb7 to
054912a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workflow_scripts/tests/test_mutation_properties.py`:
- Line 12: Update the mock import in test_mutation_properties.py to import mock
without the mock alias, satisfying Ruff PLR0402 while preserving existing mock
usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5a5ae523-bfec-4c2d-b01e-71db84b30c66
📒 Files selected for processing (24)
.coverage.github/actions/release-to-pypi-uv/tests/_helpers.py.gitignore.rules/python-00.md.rules/python-context-managers.md.rules/python-exception-design-raising-handling-and-logging.md.rules/python-generators.md.rules/python-return.md.rules/python-typing.mddocs/cmd-mox-users-guide.mddocs/execplans/2-4-5-extend-architecture-enforcement-to-orchestration-code.mddocs/execplans/support-cranelift-codegen.mddocs/local-validation-of-github-actions-with-act-and-pytest.mddocs/python-action-scripts.mddocs/scripting-standards.mdworkflow_scripts/mutation_detect_changes.pyworkflow_scripts/mutation_run_mutmut.pyworkflow_scripts/mutation_summarize_cargo.pyworkflow_scripts/tests/test_graphql_client.pyworkflow_scripts/tests/test_mutation_detect_changes.pyworkflow_scripts/tests/test_mutation_properties.pyworkflow_scripts/tests/test_mutation_run_cargo.pyworkflow_scripts/tests/test_mutation_run_mutmut.pyworkflow_scripts/tests/test_mutation_summarize_cargo.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/polythene(auto-detected)
|
|
||
| from __future__ import annotations | ||
|
|
||
| import unittest.mock as mock |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Import mock without an alias.
Replace the alias to satisfy Ruff PLR0402.
Apply the lint-compliant import
-import unittest.mock as mock
+from unittest import mock📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import unittest.mock as mock | |
| from unittest import mock |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 12-12: Use from unittest import mock in lieu of alias
Replace with from unittest import mock
(PLR0402)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workflow_scripts/tests/test_mutation_properties.py` at line 12, Update the
mock import in test_mutation_properties.py to import mock without the mock
alias, satisfying Ruff PLR0402 while preserving existing mock usage.
Sources: Coding guidelines, Linters/SAST tools
2 participants
Summary
Addresses #342, #341, #336, and #340 by killing the highest-value
mutation-testing survivors on the estate's mutation-testing helper
scripts. The two mutation runners come first: they produce every
downstream survivor table, so gaps there corrupt the whole estate's
triage. The
graphql_clientretry logic (which guards dependabotauto-merge estate-wide) and the change-detection matrix contract follow.
Each module was killed red-green with
mutmut apply/revert andconfirmed with scoped
mutmut run 'workflow_scripts.<module>*'re-runs. Equivalent and accepted-boundary mutants are annotated with
# pragma: no mutate(or suppressed structurally where a trailing pragmacannot reach the mutated node) with one-line justifications. Response
bulk beyond the named boundaries is deferred to the issues, noted below,
Before / after survivor counts (scoped
mutmut run)mutation_run_cargo(#342)mutation_summarize_cargo(#342)mutation_run_mutmut(#341)graphql_clientretry/backoff +request_graphql(#336)mutation_detect_changesmatrix contract (#340)"Residual" mutants are genuine equivalents that share a source line with
a killable mutant, so a pragma would mask the killable one; they are left
to their issue rather than suppressed.
Review walkthrough
mutation_run_cargo.py/tests:
shard-count boundary (1 and 2) and exact exit-code contract meanings
(including the unknown-code fallback).
mutation_summarize_cargo.py/tests:
survivor-placeholder extraction and defaults, timeout/unviable counts,
root-first ordering, skipped/missing/invalid diagnostics, the
non-halting scan past a leading foreign directory, and the exact
job-summary Markdown. UTF-8 read annotated.
mutation_run_mutmut.py/tests:
module-glob prefix handling, noise rejection, status accumulation,
exact survivor/summary Markdown, the
uv run --withargv contract forboth the run and results sub-invocations, the command/exit-code/counts
diagnostics, and the stderr failure path. The equivalent partition/rpartition method choice, streaming-only
RETCODE(FG=...), and UTF-8 encoding values are isolated on separatelysuppressed expressions, leaving the result delimiter and summary append
mode under mutation.
graphql_client:the strict
attempt < max_retriesboundary, the exponential backoffschedule (previously no-tests), the retry-or-fail decision, and the
request loop's attempt budget with argument forwarding.
mutation_detect_changes.py/tests:
full-run extra-crate shape, scoped target directories, sorted
matrix-JSON keys, and the skip-summary base-ref/window. The root-first sort key was extracted into
_root_first_key; only itsequivalent alphabetical-key variant is suppressed, leaving the
root-ranking predicate under mutation.
Deferred (per issue)
rstrip-variant mutants on_module_glob_for's prefix-stripline are equivalent (PurePosixPath normalises trailing separators) but
share a line with a killable
lstripvariant.unreachable defensive-tail mutants (23-26; a two-pragma annotation
overflows the line limit and restructuring defensive code is not
warranted), and the equivalent
rangeupper-bound mutant.changed_filesgit-argument,_slug_for, and remaining_write_skip_summarysurvivors.dependabot_automergebulk not touched. The Mutation testing: dependabot_automerge live-execution and merge-state survivors #337timeout mutant's killing test was assessed as not cheap within the
remaining budget and is deferred.
Red-green evidence (one transcript per module family)
graphql_client._should_retry(mutant 1,<→<=):mutation_run_cargo.interpret_exit_code(mutant 6, unknown-code defaultXXunexpected exit codeXX) andmutation_run_mutmut.count_statuses(mutant 2,
+= 1→= 1) were verified the same way (RED under theapplied mutant, GREEN after revert).
Validation
make check-fmt,make lint,make typecheck: pass.make test: 1041 passed / 14 skipped; 4 snapshots passed.and root-first/alphabetical ordering invariants.
release-to-pypi-uvtest loader to resolve co-located scriptsindependently of ambient
GITHUB_ACTION_PATH; its polluted-environmenttwo-worker xdist reproduction passes all 92 focused tests.
mutation_detect_changes;the critical root-order
!=to==mutant is killed, while 20 broaderissue-deferred survivors remain.
mutation_run_mutmutscope: 157 killed and the two documentedequivalent
_module_glob_forsurvivors remain.Act-gated workflow tests were left untouched.
References