Harden the mutmut runner against env leaks and test-only scopes (#369) - #374
Harden the mutmut runner against env leaks and test-only scopes (#369)#374leynos wants to merge 4 commits into
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
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:
WalkthroughChangesThe mutation workflow excludes pytest modules from mutmut filters, removes workflow Mutation scope safeguards
Sequence Diagram(s)sequenceDiagram
participant ChangedFiles
participant _module_glob_for
participant main
participant GITHUB_STEP_SUMMARY
ChangedFiles->>_module_glob_for: provide changed Python paths
_module_glob_for->>main: return no mutable globs
main->>GITHUB_STEP_SUMMARY: append no-source summary
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 18 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (18 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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.
Replace the three-clause stem conditional in _is_test_file with a compiled regex and a named directory-name set, resolving the CodeScene Complex Conditional finding on PR #374.
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: c8a4acf36f
ℹ️ 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".
| if TEST_STEM_RE.match(path.stem): | ||
| return True | ||
| return any(part in TEST_DIR_NAMES for part in path.parts[:-1]) |
There was a problem hiding this comment.
When a caller intentionally includes a production module whose path matches this heuristic—for example, source_paths = ["src/"] with src/pkg/tests/helpers.py or src/pkg/request_test.py—the predicate drops that changed file even though mutmut is configured to generate mutants for it. A scheduled run will then omit the module or take the successful “no mutable source” path, silently losing mutation coverage. Test exclusions should be derived from the caller's mutmut configuration or known test-selection paths rather than inferred solely from directory and filename conventions.
Useful? React with 👍 / 👎.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workflow_scripts/mutation_run_mutmut.py (1)
119-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim private helper docstrings to a single-line summary.
_is_test_file,_module_glob_for,_workflow_input_env_names, and_write_no_source_summaryare all private (underscore-prefixed) yet carry multi-paragraph docstrings. Move the rationale into inline comments or fold it into the module-level notes instead.As per path instructions: "Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces."✏️ Example trim for `_is_test_file`
def _is_test_file(path: PurePosixPath) -> bool: - """Return True when ``path`` is a pytest test module, not source. - - mutmut only generates mutants for source files, so a change window of - test modules alone maps to no mutants and would abort ``mutmut run`` - with an empty filter. Files whose stem matches ``TEST_STEM_RE`` or - that sit under a ``test``/``tests`` directory are treated as - non-mutable and skipped. - """ + """Return True when ``path`` is a pytest test module, not source."""Apply the same trim to
_module_glob_for,_workflow_input_env_names, and_write_no_source_summary.Also applies to: 281-290, 332-341
🤖 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/mutation_run_mutmut.py` around lines 119 - 152, Trim the multi-line docstrings of private helpers _is_test_file, _module_glob_for, _workflow_input_env_names, and _write_no_source_summary to single-line summaries. Preserve any rationale needed for maintainers by moving it to inline comments or module-level notes, without changing the helpers’ behavior.
🤖 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_run_mutmut.py`:
- Around line 226-247: Update test_workflow_input_env_does_not_leak_into_mutmut
to remove the unused fake_uv parameter and add
pytest.mark.usefixtures("fake_uv") so the fixture's PATH setup still occurs
without triggering Ruff's unused-argument check.
---
Outside diff comments:
In `@workflow_scripts/mutation_run_mutmut.py`:
- Around line 119-152: Trim the multi-line docstrings of private helpers
_is_test_file, _module_glob_for, _workflow_input_env_names, and
_write_no_source_summary to single-line summaries. Preserve any rationale needed
for maintainers by moving it to inline comments or module-level notes, without
changing the helpers’ behavior.
🪄 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: e5219497-0e2d-4971-8121-fbf401fd534c
📒 Files selected for processing (3)
docs/mutation-mutmut-workflow.mdworkflow_scripts/mutation_run_mutmut.pyworkflow_scripts/tests/test_mutation_run_mutmut.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/polythene(auto-detected)
| @POSIX_SHIMS_ONLY | ||
| def test_workflow_input_env_does_not_leak_into_mutmut( | ||
| self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fake_uv: Path | ||
| ) -> None: | ||
| """``INPUT_*`` vars are stripped before mutmut runs the baseline. | ||
|
|
||
| Reproduces shared-actions#369: the caller sets | ||
| ``INPUT_MODULE_PREFIX_STRIP=""``; without sanitisation that value | ||
| reaches mutmut's in-process pytest baseline and breaks tests that | ||
| read the ambient environment. | ||
| """ | ||
| self._prepare(tmp_path, monkeypatch) | ||
| monkeypatch.setenv("INPUT_FILES", "src/mypkg/calc.py") | ||
| monkeypatch.setitem(local.env, "INPUT_FILES", "src/mypkg/calc.py") | ||
| monkeypatch.setenv("INPUT_MODULE_PREFIX_STRIP", "") | ||
| monkeypatch.setitem(local.env, "INPUT_MODULE_PREFIX_STRIP", "") | ||
| run_mutmut.app([]) | ||
| env_dump = (tmp_path / "uv-env.txt").read_text(encoding="utf-8") | ||
| leaked = [line for line in env_dump.splitlines() if line.startswith("INPUT_")] | ||
| assert not leaked, ( | ||
| f"no INPUT_* variable should reach the mutmut subprocess, found {leaked}" | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Drop the unused fake_uv parameter; use usefixtures instead.
fake_uv is only needed for its side effect of installing the shim on PATH; the test body never touches it, hence Ruff's ARG002. Renaming the parameter would break pytest's name-based fixture resolution, so switch to @pytest.mark.usefixtures("fake_uv").
🧹 Proposed fix
+ `@pytest.mark.usefixtures`("fake_uv")
`@POSIX_SHIMS_ONLY`
def test_workflow_input_env_does_not_leak_into_mutmut(
- self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fake_uv: Path
+ self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:📝 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.
| @POSIX_SHIMS_ONLY | |
| def test_workflow_input_env_does_not_leak_into_mutmut( | |
| self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fake_uv: Path | |
| ) -> None: | |
| """``INPUT_*`` vars are stripped before mutmut runs the baseline. | |
| Reproduces shared-actions#369: the caller sets | |
| ``INPUT_MODULE_PREFIX_STRIP=""``; without sanitisation that value | |
| reaches mutmut's in-process pytest baseline and breaks tests that | |
| read the ambient environment. | |
| """ | |
| self._prepare(tmp_path, monkeypatch) | |
| monkeypatch.setenv("INPUT_FILES", "src/mypkg/calc.py") | |
| monkeypatch.setitem(local.env, "INPUT_FILES", "src/mypkg/calc.py") | |
| monkeypatch.setenv("INPUT_MODULE_PREFIX_STRIP", "") | |
| monkeypatch.setitem(local.env, "INPUT_MODULE_PREFIX_STRIP", "") | |
| run_mutmut.app([]) | |
| env_dump = (tmp_path / "uv-env.txt").read_text(encoding="utf-8") | |
| leaked = [line for line in env_dump.splitlines() if line.startswith("INPUT_")] | |
| assert not leaked, ( | |
| f"no INPUT_* variable should reach the mutmut subprocess, found {leaked}" | |
| ) | |
| `@pytest.mark.usefixtures`("fake_uv") | |
| `@POSIX_SHIMS_ONLY` | |
| def test_workflow_input_env_does_not_leak_into_mutmut( | |
| self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch | |
| ) -> None: | |
| """``INPUT_*`` vars are stripped before mutmut runs the baseline. | |
| Reproduces shared-actions#369: the caller sets | |
| ``INPUT_MODULE_PREFIX_STRIP=""``; without sanitisation that value | |
| reaches mutmut's in-process pytest baseline and breaks tests that | |
| read the ambient environment. | |
| """ | |
| self._prepare(tmp_path, monkeypatch) | |
| monkeypatch.setenv("INPUT_FILES", "src/mypkg/calc.py") | |
| monkeypatch.setitem(local.env, "INPUT_FILES", "src/mypkg/calc.py") | |
| monkeypatch.setenv("INPUT_MODULE_PREFIX_STRIP", "") | |
| monkeypatch.setitem(local.env, "INPUT_MODULE_PREFIX_STRIP", "") | |
| run_mutmut.app([]) | |
| env_dump = (tmp_path / "uv-env.txt").read_text(encoding="utf-8") | |
| leaked = [line for line in env_dump.splitlines() if line.startswith("INPUT_")] | |
| assert not leaked, ( | |
| f"no INPUT_* variable should reach the mutmut subprocess, found {leaked}" | |
| ) |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 228-228: 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 226 - 247,
Update test_workflow_input_env_does_not_leak_into_mutmut to remove the unused
fake_uv parameter and add pytest.mark.usefixtures("fake_uv") so the fixture's
PATH setup still occurs without triggering Ruff's unused-argument check.
Source: Linters/SAST tools
|
@coderabbitai review |
✅ Action performedReview finished.
|
Strip the workflow's INPUT_* variables from the environment before invoking mutmut run: mutmut executes the caller's test suite in-process as its baseline, so INPUT_MODULE_PREFIX_STRIP="" set by the caller leaked into the baseline and broke the module-glob test on every scheduled run. The affected tests also clear ambient INPUT_* variables so they exercise the documented defaults rather than the surrounding CI environment. Skip pytest test modules (conftest, test_*, *_test, and files under test/ or tests/ directories) during module-glob translation, and record a "changed files contain no mutable source" note in the job summary when the scope short-circuits, so a test-only change window becomes a graceful no-op instead of aborting mutmut with an empty filter (leynos/agent-helper-scripts#74).
Note in docs/mutation-mutmut-workflow.md that test modules are excluded from module-glob translation and that a test-only change window short-circuits with a "no mutable source" summary note (leynos/agent-helper-scripts#74).
Replace the three-clause stem conditional in _is_test_file with a compiled regex and a named directory-name set, resolving the CodeScene Complex Conditional finding on PR #374.
c8a4acf to
1cb03f9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Record the coverage database regenerated by the post-validation hook so the tracked artifact matches the completed mutation-runner validation.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
2 participants
Summary
Two mutmut-side baseline-hygiene defects broke scheduled mutation runs:
INPUT_*variables onthe "Run mutation testing" step. mutmut runs the caller's test suite
in-process as its baseline, inheriting that environment, so
INPUT_MODULE_PREFIX_STRIP=""(set by this repo's ownmutation-testing-caller.yml, whose sources live at the repo root ratherthan under
src/) leaked into the baseline. The module-glob test read theambient value and observed
""instead of the documented"src/"default,failing every scheduled run since 2026-07-10.
test-file edits mapped a test path straight onto a mutmut module filter that
matches no mutants, so
mutmut runaborted withAssertionError: Filtered for specific mutants, but nothing matches.Changes
Both fixes live in
workflow_scripts/mutation_run_mutmut.py:INPUT_*beforemutmut run._run_mutation_testingnow runs thesubprocess inside a
local.env()block that deletes everyINPUT_*variable, so workflow inputs configure the script but never reach the mutated
project's in-process pytest baseline. The block restores the environment on
exit, leaving the later
mutmut resultscall unaffected._prepareclears theINPUT_*variables so tests exercise the script's documented defaults ratherthan whatever the surrounding CI environment happens to set.
_is_test_fileskipsconftest.py,test_*,*_test, and files under atest/testsdirectory. A scope of test files alone now yields no globs and takes the
existing short-circuit path.
changed files contain no mutable sourcetoGITHUB_STEP_SUMMARYinstead ofsilently returning, matching the
has_changes=falseskip semantics.Review walkthrough
In
workflow_scripts/mutation_run_mutmut.py:_is_test_file/_module_glob_for— test-file exclusion._workflow_input_env_names/_run_mutation_testing— env sanitisation._write_no_source_summary/main— graceful no-op summary.In
workflow_scripts/tests/test_mutation_run_mutmut.py:fake_uvnow dumps the subprocess environment; new tests cover the envleak, the test-only short-circuit, and test-file exclusion (red-green: all
failed before the fix).
Validation
make check-fmt,make lint,make typecheck,make test,make spellingrun locally on the branch.
Closes #369
Fixes leynos/agent-helper-scripts#74
References