Skip to content

fix(automation): kill uncatchable run_cargo mutants - #58

Merged
martin-kolinek merged 1 commit into
mainfrom
martin-kolinek/fix-automation-run-cargo-mutants
Jul 20, 2026
Merged

fix(automation): kill uncatchable run_cargo mutants#58
martin-kolinek merged 1 commit into
mainfrom
martin-kolinek/fix-automation-run-cargo-mutants

Conversation

@martin-kolinek

Copy link
Copy Markdown
Collaborator

Problem

The scheduled-exhaustive CI leg (anvil-mutants-full) failed with 2 missed mutants in crates/automation/src/lib.rs (run_cargo):

  • crates/automation/src/lib.rs:76 — replace run_cargo -> Result<(), AppError> with Ok(())
  • crates/automation/src/lib.rs:83 — delete ! in run_cargo

Root cause

duct::cmd(...).run() already returns Err on a non-zero exit status (duct's default). That made the if !output.status.success() block effectively unreachable dead code — so flipping ! or stubbing the body with Ok(()) was unobservable, hence uncatchable. As a bonus bug, output.stdout/stderr were read in the bail message but are always empty because stdio is inherited (piped live).

Fix

  • Add .unchecked() so duct no longer short-circuits, making the status check the single observable success/failure decision.
  • Simplify the now-correct bail message (dropped the always-empty stdout/stderr).
  • Add test_run_cargo_success and test_run_cargo_reports_failure; the failure test kills both mutants.

Why the diff-scoped PR check never caught this

anvil-mutants-diff only mutates lines in the PR diff (--in-diff). run_cargo was added 2026-03-18, ~3 months before cargo-mutants CI landed (#33, 2026-06-16), and no PR has touched those lines since — so they never appeared in a mutation diff. The scheduled full sweep is the safety net that surfaced it.

Verification

  • File-scoped cargo mutants --file crates/automation/src/lib.rs: 4 caught, 1 unviable, 0 missed.
  • Full-workspace cargo mutants --workspace run in progress; 0 missed at ~50% at time of opening. (Change is isolated to one file, so it cannot affect other crates.)
  • cargo test -p automation, nightly fmt, and just spellcheck all clean.

`run_cargo` used `duct::cmd(...).run()`, which already returns an `Err`
on a non-zero exit status. That made the `if !output.status.success()`
block unreachable dead code (and its `output.stdout`/`stderr` reads were
always empty, since stdio is inherited). cargo-mutants` scheduled
exhaustive run flagged two uncatchable mutants there: "delete !" and
"replace body with Ok(())".

Add `.unchecked()` so duct no longer short-circuits on failure, making the
status check the single observable success/failure decision, and simplify
the now-correct bail message. Add unit tests covering both the success and
failure paths, which kill both mutants.

These lines predate the mutation-testing infra (added 3 months before
cargo-mutants CI) and were never in a PR diff since, so the diff-scoped
per-PR check never mutated them; the scheduled full sweep did.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5e5f60bf-ad77-4786-b8cf-79ee116be1db
Copilot AI review requested due to automatic review settings July 20, 2026 09:57
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (829341e) to head (def6d23).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #58   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files          50       50           
  Lines        2652     2652           
=======================================
  Hits         2652     2652           
Flag Coverage Δ
linux ?
linux-arm ?
scheduled ?
windows ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes automation::run_cargo so that mutation testing can reliably observe (and therefore kill) mutants related to the command’s success/failure decision, and adds focused tests to prevent regressions.

Changes:

  • Make run_cargo use duct::cmd(...).unchecked().run()? so non-zero exits don’t short-circuit into an error before the explicit status.success() check.
  • Simplify the failure path to avoid reporting stdout/stderr that aren’t captured when running with inherited output.
  • Add unit tests covering both a known-success invocation and a known-failure invocation to kill the previously missed mutants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@martin-kolinek
martin-kolinek merged commit 57e1b55 into main Jul 20, 2026
44 checks passed
@martin-kolinek
martin-kolinek deleted the martin-kolinek/fix-automation-run-cargo-mutants branch July 20, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants