Kill CLI and dispatch mutation survivors; annotate accepted diagnostics (#24, #25, #26, #29) - #32
Kill CLI and dispatch mutation survivors; annotate accepted diagnostics (#24, #25, #26, #29)#32leynos wants to merge 5 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
WalkthroughMove Ruff execution into the virtual environment. Mark non-behavioural code for mutation suppression. Expand CLI and PyPy dispatch-helper tests for argument forwarding, status handling, aliases, cached children, and metrics. ChangesTooling and mutation coverage
Possibly related issues
Possibly related PRs
Suggested labels: Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (17 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideAdds focused unit tests to kill previously surviving mutation testing cases in the CLI wrapper and object-dispatch helpers, and annotates accepted debug/info log lines and typing-only casts with File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Review findings actioned in d125051:
A full |
|
Heads-up: #34 replaced ci.yml's bespoke |
Cover gaps harvested from the 2026-07-10 mutmut run: - _build_builtin_child imported/builtin branches, exact argument forwarding, and metric keys (issue #24) - CLI argv slicing, install_patch logger wiring, msg_status fallback, legacy Run path, and the non-integer-status warning (issue #25) - dispatch argument propagation, the const special-attribute gate, the PyPy __class_getitem__ gate, and PatchError message content (issue #26)
Annotate diagnostic debug/info log lines in _patch.py and cli.py with "pragma: no mutate" (issue #29): their message content carries no behavioural contract, so string and argument mutants on those lines survive without indicating a test gap. Also annotate the equivalent mutants recorded in the triage worklist: typing-only typ.cast type strings (extracted onto dedicated lines so the suppression does not mask argument-forwarding mutants elsewhere in the same call), the _object_build_with_logger logger fallback, the version-string maxsplit variant, and the strict-mode default string.
Action the CodeScene code-duplication findings on the survivor-triage tests: extract a _make_modern_run factory shared by the modern Run stand-ins in test_cli.py, route all fake pylint module setup through _install_fake_pylint, and fold the two object-build member-forwarding tests into one parametrized test. Also add the integration-style dispatch test suggested by the Sourcery review: a genuine builtin routed through _dispatch_member_to_child takes the builtin branch and yields a child.
CI installed ruff as a bare global tool, so it always ran whatever release was newest on PyPI rather than the version pinned in uv.lock. Ruff 0.15.22 introduced the noqa-comments rule and broke this PR's diff on a version the repository had not adopted yet, repeating the drift the estate has already hit and fixed elsewhere. Route ruff through the synced dev virtualenv instead, matching how pytest is already resolved, so `make lint`/`check-fmt`/`fmt` use the pinned release and CI no longer fetches an unpinned one.
d9724d5 to
845a5b2
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/test_cli.py`:
- Line 42: Replace the broad suppression on the __init__ parameter exit in
tests/test_cli.py:42 with the narrow Ruff code A002 and add a brief
justification. Also update the suppression at tests/test_patch_helpers.py:1151
to use FBT001, preserving its existing justification.
In `@tests/test_patch_helpers.py`:
- Around line 993-996: The _RecordingImportedBuilder.imported_member method
should explicitly indicate that it overrides its base-class method. Import and
apply typing_extensions.override, and add typing_extensions to the project’s
development dependencies while preserving the existing method behavior.
🪄 Autofix
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: 2cf852b2-a65a-481b-8278-3309820113f8
📒 Files selected for processing (6)
.github/workflows/ci.ymlMakefilepkg/pylint_pypy_shim/_patch.pypkg/pylint_pypy_shim/cli.pytests/test_cli.pytests/test_patch_helpers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/shared-actions(auto-detected) → reviewed against open PR#355kill-mutation-survivorsinstead of the default branch
| """Build a modern ``Run`` stand-in recording ``(argv, exit)`` calls.""" | ||
|
|
||
| class FakeRun: | ||
| def __init__(self, argv: list[str], *, exit: bool) -> None: # ruff:ignore[builtin-argument-shadowing] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -eu
printf '%s\n' '--- Ruff configuration ---'
find . -maxdepth 3 -type f \( -name 'pyproject.toml' -o -name 'ruff.toml' -o -name '.ruff.toml' -o -name 'setup.cfg' \) -print
printf '%s\n' '--- Target lines ---'
sed -n '35,48p' tests/test_cli.py
sed -n '1144,1157p' tests/test_patch_helpers.py
printf '%s\n' '--- Rule definitions and suppression policy references ---'
rg -n -C 3 'builtin-argument-shadowing|boolean-type-hint-positional-argument|noqa|ruff:ignore|lint' pyproject.toml ruff.toml .ruff.toml setup.cfg .rules tests 2>/dev/null || trueRepository: leynos/pylint-pypy-shim
Length of output: 34409
🏁 Script executed:
set -eu
printf '%s\n' '--- Complete Ruff lint selection ---'
sed -n '30,96p' pyproject.toml
printf '%s\n' '--- Repository suppression style ---'
rg -n '# noqa:|# ruff:ignore' --glob '*.py' .
printf '%s\n' '--- Ruff availability and rule diagnostics ---'
if command -v ruff >/dev/null 2>&1; then
ruff --version
ruff check tests/test_cli.py tests/test_patch_helpers.py --output-format concise 2>&1 || true
else
printf '%s\n' 'ruff is not installed in the sandbox'
fiRepository: leynos/pylint-pypy-shim
Length of output: 4744
🏁 Script executed:
set -eu
printf '%s\n' '--- Ruff rule metadata ---'
ruff rule builtin-argument-shadowing --output-format json
ruff rule boolean-type-hint-positional-argument --output-format json
printf '%s\n' '--- Direct diagnostics with Ruff suppressions removed in-memory ---'
python3 - <<'PY'
from pathlib import Path
files = [Path("tests/test_cli.py"), Path("tests/test_patch_helpers.py")]
for path in files:
text = path.read_text()
text = text.replace("ruff:ignore[builtin-argument-shadowing]", "noqa: INVALID")
text = text.replace(
"ruff:ignore[boolean-type-hint-positional-argument]",
"noqa: INVALID",
)
Path("/tmp") # keep this verifier read-only with respect to the repository
out = Path("/tmp") / f"{path.name}.ruff-probe.py"
out.write_text(text)
print(out)
PY
ruff check /tmp/test_cli.py.ruff-probe.py /tmp/test_patch_helpers.py.ruff-probe.py --output-format concise 2>&1 || trueRepository: leynos/pylint-pypy-shim
Length of output: 6120
Use narrow # noqa suppressions with the correct Ruff codes.
Replace the directives with # noqa: A002 at tests/test_cli.py:42 and # noqa: FBT001 at tests/test_patch_helpers.py:1151. Add a brief justification to the A002 suppression and retain the existing justification for FBT001.
📍 Affects 2 files
tests/test_cli.py#L42-L42(this comment)tests/test_patch_helpers.py#L1151-L1151
🤖 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 `@tests/test_cli.py` at line 42, Replace the broad suppression on the __init__
parameter exit in tests/test_cli.py:42 with the narrow Ruff code A002 and add a
brief justification. Also update the suppression at
tests/test_patch_helpers.py:1151 to use FBT001, preserving its existing
justification.
Sources: Coding guidelines, Path instructions
| def imported_member(self, node: object, member: object, alias: str) -> bool: | ||
| """Record the exact arguments and return the canned verdict.""" | ||
| self.imported_member_calls.append((node, member, alias)) | ||
| return self._imported |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -eu
printf '%s\n' '--- project metadata ---'
find . -maxdepth 2 -type f \( -name 'pyproject.toml' -o -name 'setup.cfg' -o -name 'tox.ini' -o -name 'pytest.ini' \) -print
printf '%s\n' '--- relevant imports and class definitions ---'
rg -n -C 4 'imported_member|RecordingImportedBuilder|typing\.override|from typing import|python_requires|requires-python|target-version' tests/test_patch_helpers.py pyproject.toml setup.cfg tox.ini 2>/dev/null || true
printf '%s\n' '--- Python-version declarations ---'
rg -n -i 'python ?3|requires-python|target-version|pyright|typing-extensions' . -g '!*.lock' -g '!*.json' 2>/dev/null | head -200
printf '%s\n' '--- test file header ---'
sed -n '1,80p' tests/test_patch_helpers.py
printf '%s\n' '--- target section ---'
sed -n '960,1010p' tests/test_patch_helpers.pyRepository: leynos/pylint-pypy-shim
Length of output: 10664
🏁 Script executed:
set -eu
printf '%s\n' '--- dependency and typing configuration ---'
sed -n '1,90p' pyproject.toml
rg -n -i 'typing_extensions|override|python_version|pythonpath|typeCheckingMode' . -g '*.py' -g '*.pyi' -g '*.toml' -g '*.cfg' -g '*.ini' -g '*.md' 2>/dev/null | head -200
printf '%s\n' '--- FakeBuilder declaration and imported_member contract ---'
rg -n -C 8 'class FakeBuilder|def imported_member' tests/test_patch_support.py pkg tests 2>/dev/null | head -160
printf '%s\n' '--- supported-version documentation ---'
sed -n '210,235p' docs/developers-guide.mdRepository: leynos/pylint-pypy-shim
Length of output: 9075
🏁 Script executed (no clone):
set -eu
python3 - <<'PY'
import sys
import typing
print(f"runtime={sys.version_info.major}.{sys.version_info.minor}")
print(f"typing.override_available={hasattr(typing, 'override')}")
try:
import typing_extensions
except ImportError:
print("typing_extensions_available=False")
else:
print(f"typing_extensions_available=True")
print(f"typing_extensions.override_available={hasattr(typing_extensions, 'override')}")
PY
Length of output: 203
Add an override marker to _RecordingImportedBuilder.imported_member. Use typing_extensions.override because the project supports Python 3.11, where typing.override is unavailable. Add typing_extensions to the development dependencies.
🤖 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 `@tests/test_patch_helpers.py` around lines 993 - 996, The
_RecordingImportedBuilder.imported_member method should explicitly indicate that
it overrides its base-class method. Import and apply typing_extensions.override,
and add typing_extensions to the project’s development dependencies while
preserving the existing method behavior.
Source: Coding guidelines
Closes #24, #25, #26, #29.
Summary
First of two survivor-triage PRs following the 2026-07-10 mutmut 3.6.0
run on
main@03b8ed1(621 mutants; 381 killed; 217 survived; 23with no covering tests). This PR:
_build_builtin_child(previously entirely untested, Mutation testing: _build_builtin_child untested-path survivors #24), the CLIwrapper (Mutation testing: CLI exit-status and argument-handling survivors #25), and the dispatch argument-propagation paths (Mutation testing: dispatch argument-propagation survivors #26);
debug/infolog lines(Mutation testing: logging diagnostics survivors (accepted) #29) and the category-2 equivalent mutants recorded in the triage
worklist with
# pragma: no mutate, per the disposition in Mutation testing: logging diagnostics survivors (accepted) #29.The metrics-counter (#27) and
install_patchgate (#28) survivors aredeferred to the stacked follow-up PR #33 to keep this change within the
agreed size tolerance for new test code.
Before/after survivor counts
main@03b8ed1(baseline)mutmut run)the 131 accepted/equivalent survivors (107 category-4 diagnostics, 24
equivalents) along with 83 already-killed no-signal mutants that
share the same source lines.
follow-up PR Kill metrics-counter and install_patch gate mutation survivors (#27, #28) #33, which finishes at 407/407 killed with zero survivors.
"0"version-default stringsin
install_patch) proved killable once the skip-warning message isasserted exactly, so the follow-up PR kills them instead of
annotating the lines.
Review walkthrough
— new tests pin
sys.argv[1:]slicing, theinstall_patch(LOGGER)wiring, the
msg_statusfallback default, the legacyRunpath andits return status, and the exact non-integer-status warning message
(Mutation testing: CLI exit-status and argument-handling survivors #25).
—
_RecordingImportedBuilderpins the exact(node, member, alias)forwarding for
_build_builtin_childand_build_class_child; newtests cover the builtin/imported branches and their metric keys
(Mutation testing: _build_builtin_child untested-path survivors #24), the
PatchErrormessage content, the constspecial-attribute gate, the alias forwarding through dispatch, the
non-PyPy
__class_getitem__gate, and the resolved-member forwardingin
_object_build_with_logger(Mutation testing: dispatch argument-propagation survivors #26).and
pkg/pylint_pypy_shim/cli.py
—
# pragma: no mutateannotations. Multi-line log calls usestart/endpairs because mutmut's bare trailing pragma onlysuppresses nodes that begin on the annotated line. The typing-only
typ.castcalls were extracted onto dedicated annotated lines so thesuppression cannot mask the argument-forwarding mutants in the
surrounding calls — those are killed by tests instead. Line-level
suppression on the
maxsplitand strict-mode-default lines alsocovers previously killed mutants on those lines; the behaviour stays
pinned by the existing version and strict-mode tests.
Red-green evidence
climodule (hand-appliedx_main__mutmut_3,sys.argv[1:]→sys.argv[2:]):_patchmodule (hand-appliedx__build_builtin_child__mutmut_11,"dispatch.builtin"→"XXdispatch.builtinXX"):Validation
ruff format --check,ruff check,ty check pkg tests tools, andpytest -n auto(78 passed, 1 skipped) all pass on this branch.uv run mutmut runon this branch: 407 generated, 346 killed,61 survived (all deferred to Kill metrics-counter and install_patch gate mutation survivors (#27, #28) #33), 0 timeouts, 0
suspicious.
🤖 Generated with Claude Code