test(resume-arm): make the worktree refusal test exercise the refusal it is named for - #372
Conversation
… it is named for _validate_helper_path refuses two things, a helper path under a temp directory and one inside a linked worktree, and only the first was covered. test_guard_refuses_linked_worktree never called main() and never asserted a refusal; it called _is_in_linked_worktree and checked it returned a string. So removing ONLY the worktree refusal, leaving the detector intact so the test still had its function to call, left all nine tests green while the real CLI went on emitting four crontab lines pinning a path that vanishes with the worktree. One honest test absorbed a whole-feature mutant and hid a vacuous sibling behind it: killing the entire guard failed exactly one test, which reads as coverage. The test now goes through main() and asserts the message names the worktree. It neutralises _is_under_temp deliberately and is worthless without it: the refusals are checked in order and tmp_path is itself under a temp root, so a fixture built there trips the temp refusal first and never reaches the branch the test is named for. That masking is why the original settled for the detector. The detector assertion is kept under an honest name, with one more covering the main worktree, where a .git directory must keep not tripping the guard so arming still works. Mutant results. Worktree refusal only: 9 passed before, 1 failed and 10 passed after, failing on test_guard_refuses_linked_worktree. Whole guard disabled: 1 failed and 8 passed before, 2 failed and 9 passed after. scripts/resume_arm_time.py is unchanged; the guard was already correct.
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 9 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Reviewed by step-3.7-flash · Input: 56.2K · Output: 11.4K · Cached: 144.4K |
Closes
tsk-3m3irr, the follow-up carded when #369 was merged. Test-only:scripts/resume_arm_time.pyis untouched, because the guard was already correct.The defect
_validate_helper_path()refuses two things, a helper path under a temp directory and one inside a linked git worktree. Only the first was covered.test_guard_refuses_linked_worktreenever calledmain()and never asserted a refusal. It called the detector and checked it returned a string:The name promises
guard_refuses. The body proves a path-inspection function returns a string, so anyone reading the suite sees a test named for the behaviour and reasonably assumes the behaviour is covered.Why a whole-suite count could never have found this
Killing the entire guard fails exactly one test, and one failure reads as coverage. One honest test absorbed a whole-feature mutant and hid a vacuous sibling behind it. The mutant has to be aimed at the individual branch, leaving the detector intact so the existing test still has its function to call:
test_guard_refuses_linked_worktreeBoth "before" figures reproduce the card's recorded measurements exactly. Under the first mutant the real CLI goes on emitting four crontab lines pinning a path that vanishes when the worktree is removed, which is precisely what the guard exists to prevent.
The trap in the fix, which is the part worth reading
The obvious fix does not work, and it fails in a way that looks like success.
The two refusals are checked in order, temp first.
tmp_pathis itself under a temp root. So a fixture built there trips the temp refusal and never reaches the branch the test is named for. A test that only assertedSystemExitwould pass, would survive the whole-guard mutant, and would still not notice the worktree refusal being deleted. It would be the same vacuity in a new costume.So the test neutralises
_is_under_tempdeliberately and is worthless without it, which the docstring and the changelog both state outright. It then asserts the message names the worktree, names the fixture directory, and specifically that it is not the sibling refusal:Also added
The detector assertion is kept, under the honest name
test_is_in_linked_worktree_detects_git_file, plustest_is_in_linked_worktree_ignores_the_main_worktree. The second pins behaviour that must not change: in the main worktree.gitis a directory and the guard must keep not firing, or arming stops working on the live box. That is deliberate scope, not an oversight.Verification