Add mutation-testing reusable workflows for cargo-mutants and mutmut#319
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Adds two reusable, informational mutation-testing workflows: one for Rust repositories using Also adds the supporting workflow scripts for detecting changes, running and summarising Includes a reusable WalkthroughAdd reusable mutation-testing workflows for Rust and Python, plus the helper scripts, tests, fixtures, and docs that wire change detection, execution, summary reporting, and workflow-source resolution end to end. ChangesMutation testing workflows
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 18 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (18 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Excess Number of Function Argumentsworkflow_scripts/mutation_run_cargo.py: build_arguments What lead to degradation?build_arguments has 7 arguments, max arguments = 4 Why does this problem occur?Functions with many arguments indicate either a) low cohesion where the function has too many responsibilities, or b) a missing abstraction that encapsulates those arguments. How to fix it?Start by investigating the responsibilities of the function. Make sure it doesn't do too many things, in which case it should be split into smaller and more cohesive functions. Consider the refactoring INTRODUCE PARAMETER OBJECT to encapsulate arguments that refer to the same logical concept. |
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Complex Conditionalworkflow_scripts/mutation_run_mutmut.py: parse_results What lead to degradation?parse_results has 1 complex conditionals with 2 branches, threshold = 2 Why does this problem occur?A complex conditional is an expression inside a branch such as an if-statmeent which consists of multiple, logical operations. Example: if (x.started() && y.running()).Complex conditionals make the code even harder to read, and contribute to the Complex Method code smell. Encapsulate them. How to fix it?Apply the DECOMPOSE CONDITIONAL refactoring so that the complex conditional is encapsulated in a separate function with a good name that captures the business rule. Optionally, for simple expressions, introduce a new variable which holds the result of the complex conditional. Helpful refactoring examplesTo get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes. SAMPLE# complex_conditional.js
function messageReceived(message, timeReceived) {
- // Ignore all messages which aren't from known customers:
- if (!message.sender &&
- customers.getId(message.name) == null) {
+ // Refactoring #1: encapsulate the business rule in a
+ // function. A clear name replaces the need for the comment:
+ if (!knownCustomer(message)) {
log('spam received -- ignoring');
return;
}
- // Provide an auto-reply when outside business hours:
- if ((timeReceived.getHours() > 17) ||
- (timeReceived.getHours() < 8)) {
+ // Refactoring #2: encapsulate the business rule.
+ // Again, note how a clear function name replaces the
+ // need for a code comment:
+ if (outsideBusinessHours(timeReceived)) {
return autoReplyTo(message);
}
pingAgentFor(message);
+}
+
+function outsideBusinessHours(timeReceived) {
+ // Refactoring #3: replace magic numbers with
+ // symbols that communicate with the code reader:
+ const closingHour = 17;
+ const openingHour = 8;
+
+ const hours = timeReceived.getHours();
+
+ // Refactoring #4: simple conditional rules can
+ // be further clarified by introducing a variable:
+ const afterClosing = hours > closingHour;
+ const beforeOpening = hours < openingHour;
+
+ // Yeah -- look how clear the business rule is now!
+ return afterClosing || beforeOpening;
} |
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd mutation_run_mutmut.py
echo "---"
ast-grep run --pattern 'def parse_results($$$) {
$$$
}' --lang python workflow_scripts/mutation_run_mutmut.py
echo "---full file---"
cat -n workflow_scripts/mutation_run_mutmut.pyLength of output: 2194 🏁 Script executed: #!/bin/bash
fd mutation_run_mutmut.py
echo "---"
cat -n workflow_scripts/mutation_run_mutmut.pyLength of output: 10841 File: Extract the multi-clause guard into a named predicate so the business rule ("this line represents a mutant result") is legible without unpacking boolean algebra. def parse_results(text: str) -> list[MutantResult]:
"""Parse ``mutmut results --all true`` output into mutant results.
Parameters
----------
text : str
Raw command output; result lines have the form
`` name: status``.
Returns
-------
list[MutantResult]
Parsed results, in output order. Lines that do not look like
mutant results (warnings, blank lines) are ignored.
"""
results: list[MutantResult] = []
for raw_line in text.splitlines():
line = raw_line.strip()
name, separator, status = line.partition(": ")
- if not separator or " " in name or "__mutmut_" not in name:
- continue
+ if not _is_mutant_result_line(name, separator):
+ continue
results.append(MutantResult(name=name, status=status.strip()))
return results
+
+
+def _is_mutant_result_line(name: str, separator: str) -> bool:
+ """Decide whether a parsed ``name``/``separator`` pair is a mutant result.
+
+ Parameters
+ ----------
+ name : str
+ Text preceding the ``: `` separator, expected to be a mutant name.
+ separator : str
+ The partition separator; empty when ``": "`` was absent from the line.
+
+ Returns
+ -------
+ bool
+ ``True`` when the line looks like a ``name: status`` mutant result
+ rather than a warning or blank line.
+ """
+ has_separator = bool(separator)
+ is_single_token = " " not in name
+ is_mutant_name = "__mutmut_" in name
+ return has_separator and is_single_token and is_mutant_namePrompt for an AI coding agent: |
Two advisory rules failed the quality gate on PR #319: - `build_arguments` in `mutation_run_cargo.py` took seven keyword parameters. Introduce a frozen `MutantsInvocation` dataclass as a parameter object and update the CLI entry point and tests. - `parse_results` in `mutation_run_mutmut.py` used a three-clause compound conditional. Extract `_parse_result_line`, which classifies one line with simple, named checks and returns `None` for noise. Behaviour is unchanged; all 47 mutation-script tests pass, and check-fmt, typecheck, lint, and the full suite remain green.
Plan two reusable workflows — `mutation-cargo.yml` (cargo-mutants) and `mutation-mutmut.yml` (mutmut) — generalizing the workflow proven in leynos/wireframe (ADR-007 there): daily cron with a change-detection guard, file-scoped mutation runs, informative exit-code policy, and job-summary survivor reporting, never gating pull requests. Seed the plan with the wireframe findings (`--output` nesting, exit-code taxonomy, `--in-place`, cron drift, companion-crate false survivors, env-block injection hardening) and the house conventions: helper logic as tested Python scripts under `workflow_scripts/`, act + pytest black-box integration tests with stubbed mutation tools, tool version pins living beside the report parsers, and caller docs mirroring the dependabot-automerge page. Stage A researches mutmut 3.x semantics before the Python workflow is designed; Stage G validates both workflows against pilot repositories and records the confirmed tool versions.
Capture measurements from the first full wireframe dispatch run: 1,821 mutants, 84s + 120s baseline, 361s auto timeout, ~28s per mutant — roughly 14–15 single-job hours, beyond GitHub's 360-minute job ceiling. Design consequences: - Add configurable sharding to `mutation-cargo.yml`: `shard-count` (default 6) fans full dispatch runs out to a matrix of `cargo mutants --shard k/N` legs (~2.5–3 hours each at wireframe scale), with a merge-and-summarize job combining shard artefacts. Scoped daily runs stay single-shard, since each shard re-pays the baseline cost for a handful of mutants. - Add an `exclude-globs` input: the run's survivors mixed genuine assertion gaps with scaffolding noise (example codecs, test helpers), so callers need a supported way to exclude such paths. Also record that `--in-place` cache reuse works as intended and that `RUSTFLAGS: -D warnings` produced no spurious unviable mutants.
Implement Stage B of the mutation-testing ExecPlan: - `workflow_scripts/mutation_detect_changes.py`: change-detection guard shared by both planned workflows. Buckets files changed within the window (commit timestamps, tip-existence filtered) into targets and emits a matrix: scoped scheduled runs stay single-shard; full dispatch runs fan the root target across `shard-count` shards. - `workflow_scripts/mutation_run_cargo.py`: builds the cargo-mutants invocation (`--in-place`, scoped `--file` args, `--shard k/N`, `--exclude` globs, shell-lexed extra args) and applies the informational exit-code contract: 0/2/3 succeed, 1/4/70 and unknowns fail with the tool's code. - `workflow_scripts/mutation_summarize_cargo.py`: merges shard `outcomes.json` artefacts per target and renders counts plus a surviving-mutants table to the job summary. Field names validated against cargo-mutants 27.x source; the parser and the workflow's version pin (default 27.1.0) travel together. - `.github/workflows/mutation-cargo.yml`: detect → sharded mutants matrix → summarize, reusing the dependabot-automerge OIDC source-resolution and `ACT` bypass pattern per job, with SHA-pinned third-party actions and least-privilege permissions. 36 unit tests cover bucketing, window edges, deleted-file filtering, dispatch bypass, matrix shapes, argument construction, exit-code mapping (via a fake `cargo` on PATH), shard-report merging, and summary rendering.
Implement Stage C of the mutation-testing ExecPlan: - `workflow_scripts/mutation_run_mutmut.py`: runs mutmut in the caller's uv-managed project (injected via `uv run --with mutmut==<pin>`), translating changed files to module-glob mutant names because mutmut 3.x rejects file paths as run arguments. Parses `mutmut results --all true` into status counts and a survivors table appended to the job summary, and writes the raw listing for the artefact. A non-zero `mutmut run` (failing baseline) fails the step; survivors do not, matching mutmut's own exit-code behaviour. - `.github/workflows/mutation-mutmut.yml`: single job — mutmut has no shard equivalent, so no matrix or cross-job merge — with an inline detect step reusing `mutation_detect_changes.py` under a `*.py` pathspec, the OIDC source-resolution and `ACT` bypass pattern, and artefact upload of the results listing plus `mutmut-stats.json`. Callers must be uv-managed projects with `[tool.mutmut]` configured; pip-only projects are out of scope for the first release. 11 unit tests cover module-glob translation (src layout, `__init__.py` collapse, non-Python inputs), results parsing, summary rendering, and the CLI paths (scoped run, failing baseline, empty scope) via a fake `uv` on PATH.
Implement Stage D of the mutation-testing ExecPlan: wrapper workflows (`test-mutation-cargo.yml`, `test-mutation-mutmut.yml`) call the reusable workflows locally with never-matching path prefixes, so scheduled runs deterministically take the change-detection skip path. The act-driven tests assert the real detection script reports `has_changes=False` and that no mutation tool is invoked. The mutation-run path is deliberately not act-tested: the harness cannot inject stub binaries onto a `workflow_call` job's PATH inside the container, so that path is covered by the helper-script unit tests and by pilot-repository validation (Stage G). Recorded in the ExecPlan Decision Log, revising the earlier stubbed-tools intent.
Implement Stage E of the mutation-testing ExecPlan: caller-facing documentation for `mutation-cargo.yml` and `mutation-mutmut.yml` mirroring the dependabot-automerge page — behaviour (guard, sharding, exit-code contract), required permissions, pinned-SHA usage examples, inputs tables, triage caveats (false survivors in companion crates, equivalent mutants, "no tests" semantics), and local validation. Add both workflows to the README reusable-workflows index.
Record Stage F evidence (all deterministic gates green via scrutineer; CodeRabbit review with zero findings; act tests environment-blocked locally) and write the retrospective. Stage G pilot validation remains, gated on merge so callers can pin a SHA.
The fake `cargo`/`uv` helpers are POSIX shell scripts, so on Windows
runners PATH lookup fell through to the real tools: real `cargo` failed
with exit 101 (`no such command: mutants`), and the mutmut
failing-baseline test passed only by coincidence because real mutmut
exits 1 on Windows ("please use the WSL").
Mark every shim-dependent test with
`skipif(sys.platform == "win32", reason="fake ... helper emits POSIX
sh")`, matching the generate-coverage precedent — including the
coincidentally passing one. The reusable workflows only run on
`ubuntu-latest`, so Windows coverage of the run wrappers carries no
signal. Platform-independent tests (argument construction, exit-code
classification, parsing, rendering, validation errors) still run
everywhere.
Record the finding in the ExecPlan's Surprises & Discoveries.
Two advisory rules failed the quality gate on PR #319: - `build_arguments` in `mutation_run_cargo.py` took seven keyword parameters. Introduce a frozen `MutantsInvocation` dataclass as a parameter object and update the CLI entry point and tests. - `parse_results` in `mutation_run_mutmut.py` used a three-clause compound conditional. Extract `_parse_result_line`, which classifies one line with simple, named checks and returns `None` for noise. Behaviour is unchanged; all 47 mutation-script tests pass, and check-fmt, typecheck, lint, and the full suite remain green.
CodeScene's advisory gate flagged file-level Overall Code Complexity in `mutation_run_mutmut.py` after the previous round fixed the individual findings. Flatten the branchy functions by extraction, without behaviour change: - `_module_glob_for` performs the single-file glob translation; `files_to_module_globs` becomes a de-duplicating comprehension. - `_render_survivor_table` isolates the survivors table; `render_summary` composes sections. - `_run_mutation_testing` and `_publish_results` split the CLI entry point's run and reporting phases. All 47 mutation-script tests pass; check-fmt, typecheck, lint, and the full suite remain green.
05951dd to
95a2b6a
Compare
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). User-Facing Documentation |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd users-guide.md
echo "---"
fd -g '*users-guide*'
echo "---readme docs section---"
sed -n '1,50p' README.md | grep -n -i "guide"
echo "---check build_arguments---"
rg -n "def build_arguments" -A 20 workflow_scripts/mutation_run_cargo.py
echo "---check dataclass---"
rg -n "CargoMutantsRunConfig" workflow_scripts/mutation_run_cargo.py workflow_scripts/tests/test_mutation_run_cargo.py
echo "---check parse_results---"
rg -n "_is_mutant_result_line|def parse_results" -A 15 workflow_scripts/mutation_run_mutmut.py
echo "---list docs dir---"
fd . docs -t fLength of output: 3548 🏁 Script executed: #!/bin/bash
sed -n '140,200p' workflow_scripts/mutation_run_mutmut.py
echo "---readme table---"
sed -n '20,45p' README.md
echo "---check MutantsInvocation fields---"
rg -n "class MutantsInvocation" -A 15 workflow_scripts/mutation_run_cargo.py
echo "---search existing github issues for users-guide---"
gh issue list --repo leynos/shared-actions --search "users-guide" --state all
gh issue list --repo leynos/shared-actions --search "user guide" --state allLength of output: 4962
Code-health findings: both resolved.
Documentation warning: not resolved.
Here is a prompt for an AI coding agent to close this gap. If you would prefer to defer this to a follow-up PR instead, let me know and I will open a GitHub issue with this same scope. ✏️ Learnings added
|
|
@coderabbitai please create a new GitHub issue for the following: Documentation warning: not resolved.
Here is a prompt for an AI coding agent to close this gap. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh issue create \
--repo leynos/shared-actions \
--assignee leynos \
--title "Add top-level docs/users-guide.md covering mutation-testing workflows" \
--body "$(cat <<'EOF'
## Summary
`docs/users-guide.md` does not exist in the repository. Only `docs/cmd-mox-users-guide.md` and `docs/syspath-hack-users-guide.md` are present, and these cover unrelated actions. The two new scheduled mutation-testing reusable workflows added in PR `#319` (`.github/workflows/mutation-cargo.yml` for cargo-mutants, and `.github/workflows/mutation-mutmut.yml` for mutmut) are signposted only in the README's "Reusable workflows" table and their own workflow-specific guides (`docs/mutation-cargo-workflow.md`, `docs/mutation-mutmut-workflow.md`), not in a top-level user guide.
## Rationale
Callers adopting reusable workflows benefit from a single top-level entry point that introduces available workflows and links onward to detailed usage docs, rather than having to discover them solely from the README table.
## Required changes
- Add a top-level `docs/users-guide.md` (or confirm/extend an existing equivalent top-level guide, if one is intended to serve this purpose — search first before creating a new file).
- Introduce the two mutation-testing workflows: `.github/workflows/mutation-cargo.yml` (cargo-mutants, Rust) and `.github/workflows/mutation-mutmut.yml` (mutmut, uv-managed Python).
- State their purpose plainly: informational, non-blocking, scheduled mutation testing that reports surviving mutants via job summaries and artefacts, with a daily change-detection guard that skips quiet days.
- Link onward to `docs/mutation-cargo-workflow.md` and `docs/mutation-mutmut-workflow.md` for full usage examples, inputs, and required permissions (`contents: read`, `id-token: write`).
- Match the structure and tone of existing reusable-workflow documentation (see `docs/dependabot-automerge-workflow.md` for the sibling pattern already listed in the README table).
- Cross-link the new guide from `README.md` where user-facing guides are indexed.
- Check for translated copies of a user guide (locale-suffixed filenames or a translations directory) and keep them in sync; if none exist, note that explicitly.
- Run markdownlint and any documentation link-check target to confirm the new file passes validation.
## Affected areas
- `docs/users-guide.md` (new)
- `README.md`
- `docs/mutation-cargo-workflow.md`, `docs/mutation-mutmut-workflow.md` (cross-links only)
## Acceptance criteria
- [ ] `docs/users-guide.md` exists, introduces both mutation-testing workflows, and links to their dedicated guides.
- [ ] README links to the new guide correctly.
- [ ] Any translated guides are updated or their absence is confirmed.
- [ ] Markdownlint and documentation validation pass.
## Backlinks
- Pull request: https://github.com/leynos/shared-actions/pull/319
- Originating comment: https://github.com/leynos/shared-actions/pull/319#issuecomment (see review thread requesting this issue)
- Requested by: `@leynos`
EOF
)"Length of output: 5020 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh issue create \
--repo leynos/shared-actions \
--assignee leynos \
--title "Add top-level docs/users-guide.md covering mutation-testing workflows" \
--body "$(cat <<'EOF'
## Summary
`docs/users-guide.md` does not exist in the repository. Only `docs/cmd-mox-users-guide.md` and `docs/syspath-hack-users-guide.md` are present, and these cover unrelated actions. The two new scheduled mutation-testing reusable workflows added in PR `#319` (`.github/workflows/mutation-cargo.yml` for cargo-mutants, and `.github/workflows/mutation-mutmut.yml` for mutmut) are signposted only in the README's "Reusable workflows" table and their own workflow-specific guides (`docs/mutation-cargo-workflow.md`, `docs/mutation-mutmut-workflow.md`), not in a top-level user guide.
## Rationale
Callers adopting reusable workflows benefit from a single top-level entry point that introduces available workflows and links onward to detailed usage docs, rather than having to discover them solely from the README table.
## Required changes
- Add a top-level `docs/users-guide.md` (or confirm/extend an existing equivalent top-level guide, if one is intended to serve this purpose — search first before creating a new file).
- Introduce the two mutation-testing workflows: `.github/workflows/mutation-cargo.yml` (cargo-mutants, Rust) and `.github/workflows/mutation-mutmut.yml` (mutmut, uv-managed Python).
- State their purpose plainly: informational, non-blocking, scheduled mutation testing that reports surviving mutants via job summaries and artefacts, with a daily change-detection guard that skips quiet days.
- Link onward to `docs/mutation-cargo-workflow.md` and `docs/mutation-mutmut-workflow.md` for full usage examples, inputs, and required permissions (`contents: read`, `id-token: write`).
- Match the structure and tone of existing reusable-workflow documentation (see `docs/dependabot-automerge-workflow.md` for the sibling pattern already listed in the README table).
- Cross-link the new guide from `README.md` where user-facing guides are indexed.
- Check for translated copies of a user guide (locale-suffixed filenames or a translations directory) and keep them in sync; if none exist, note that explicitly.
- Run markdownlint and any documentation link-check target to confirm the new file passes validation.
## Affected areas
- `docs/users-guide.md` (new)
- `README.md`
- `docs/mutation-cargo-workflow.md`, `docs/mutation-mutmut-workflow.md` (cross-links only)
## Acceptance criteria
- [ ] `docs/users-guide.md` exists, introduces both mutation-testing workflows, and links to their dedicated guides.
- [ ] README links to the new guide correctly.
- [ ] Any translated guides are updated or their absence is confirmed.
- [ ] Markdownlint and documentation validation pass.
## Backlinks
- Pull request: https://github.com/leynos/shared-actions/pull/319
- Requested by: `@leynos`
EOF
)"Length of output: 3119
(=^・ω・^=) |
Summary
This branch implements scheduled, informational mutation-testing reusable workflows for the leynos estate:
mutation-cargo.ymlfor Rust repositories usingcargo-mutants, andmutation-mutmut.ymlfor uv-managed Python repositories usingmutmut. Both generalize the workflow proven in leynos/wireframe (its ADR-007): a daily-cron-friendly change-detection guard makes quiet days a cheap no-op, scoped runs mutate only recently changed files, full dispatch runs fan out across shards (cargo only), and surviving mutants land in the job summary and artefacts. Neither workflow gates pull requests.Execplan: docs/execplans/add-mutation-testing-workflows.md — Stages A–F implemented in this branch; Stage G (pilot caller validation on wireframe and cmd-mox) is deliberately deferred until merge, because callers must pin a shared-actions SHA.
Key design points, each recorded in the execplan's Decision Log:
workflow_scripts/(per the scripting standards), not in workflow shell; 47 unit tests cover detection bucketing, window edges, shard matrices, argument construction, exit-code mapping, report merging, results parsing, and summary rendering.shard-count, default 6) with a merge-and-summarize job — sized from wireframe's measured first run (1,821 mutants ≈ 14–15 single-job hours, beyond GitHub's 360-minute ceiling). mutmut has no shard equivalent, so its workflow is a single job.source_pathsreplacespaths_to_mutate, file paths are rejected as run arguments (changed files translate to module globs such aspkg.mod.*), and results are parsed frommutmut results --all true.ACTbypass pattern, keeping helper scripts in lockstep with the caller's pinned SHA.Review walkthrough
Validation
make check-fmt: pass (207 files already formatted)make typecheck: pass (ty check, both search-path groups)make lint: pass (ruff + action-validator over all action manifests)make test: pass — 989 passed, 14 skippedmake markdownlint: pass (69 files, 0 errors)make nixie: pass (all Mermaid diagrams)coderabbit review --agent: completed, 0 findingsactis not installed on the implementation machine); they are opt-in viaACT_WORKFLOW_TESTS=1 make testand should be confirmed on a machine with act before or during Stage G.Notes
workflow_calljob's PATH — it is covered by the unit tests and by Stage G pilot runs (see the Decision Log entry revising the original stubbed-tools intent).mutation-mutmut.ymlrequires callers to be uv-managed projects with[tool.mutmut]configured; pip-only projects are out of scope for the first release.mutation-cargo.ymlcaller, add a mutmut caller to cmd-mox, verify real reports parse, and adjust version pins if the pilots surface drift.