Repository navigation
cargo-target-janitor: owner_changed tests never run — defined after unittest.main(), outside the test class #5710
Description
cargo-target-janitor: the two owner_changed tests never run — they are defined after unittest.main(), outside the test class
Evidence (verified this run @ af0847d)
infra/cargo-target-janitor/tests/test_cargo_target_janitor.py:
- Line 191-192: the file ends its class body and closes with
if __name__ == "__main__": unittest.main()
- Lines 194 and 208:
test_owner_changed_when_worktree_switched_branchand
test_owner_changed_ignores_adhoc_targetsare defined after that call,
indented inside theif __name__block — not as methods ofJanitorTests.
unittest.main()also exits the process, so the twodefs are never even
reached at runtime, and unittest discovery never sees them either way.
Reproduction (run against the file as committed):
$ python3 tests/test_cargo_target_janitor.py -v
...
Ran 11 tests in 0.053s
OK
An AST walk of the file finds 13 test_* functions; only the 11 class
methods run. The two dead tests are the only coverage for
owner_changed() (infra/cargo-target-janitor/bin/cargo_target_janitor.py:69),
the guard the --apply path re-checks immediately before shutil.rmtree
(same file, line 548): if the worktree owning a target switched branch/task
since the plan, deletion must stop (the #4175 Codex P2 fix).
The function itself is currently correct — calling it directly with the dead
tests' fixtures gives the expected results (switched → True, same → False,
unregistered → True, ad-hoc → False) — so this is illusory coverage, not a
live logic bug: any future regression in owner_changed would pass CI green
while the file appears to test it.
Why it matters
This tool deletes directory trees (shutil.rmtree) under ~/git/fleet.
owner_changed is one of the last-moment safety re-checks that keep it from
removing a target whose checkout was repurposed after planning. A safety
guard whose tests silently never execute is exactly the kind of coverage
that fails only when it is needed.
Acceptance criteria
-
test_owner_changed_when_worktree_switched_branchand
test_owner_changed_ignores_adhoc_targetsare methods of
JanitorTests(moved above theif __name__ == "__main__":guard). -
python3 tests/test_cargo_target_janitor.pyreports 13 tests and
passes (previously 11). - No change to
bin/cargo_target_janitor.pybehavior is needed; if the
rescued tests fail against the current implementation, that failure is
reported on this issue rather than edited away in the tests.
Activity
dezren39 commentedon Nov 4, 2026
🤖 Agent note · filed
owner_changedtests are defined afterunittest.main(), outsideJanitorTests, so they never run (suite reports 11 tests, file defines 13 test functions). Verified by executing the suite and by callingowner_changeddirectly with the dead tests' fixtures (all expectations pass — illusory coverage, not a live logic bug).dezren39 commentedon Nov 5, 2026
agent-note:v1
event: picked_up
task: 9759
worker: repo-worker-code (lane code-f)
stage: claimed
summary: Triage claimed by lane code-f (direct execution lane). Verifying the dead-test claim against current main: AST count of test_* functions vs the count unittest actually runs, on the laptop clone.
dezren39 commentedon Nov 5, 2026
agent-note:v1
event: triaged
task: 9759
worker: repo-worker-code (lane code-f)
stage: bookkeeping_complete
summary: Triage verdict: CONFIRMED on current main 03032fd. Verified fresh on the laptop clone: infra/cargo-target-janitor/tests/test_cargo_target_janitor.py line 16 class JanitorTests; lines 191-192
if __name__ == "__main__": unittest.main(); the two test_owner_changed_* defs sit at lines 194/208 indented inside that if-block, after main(). AST walk: 13 test_* functions in the file, only 11 are JanitorTests methods. Running the file reports "Ran 11 tests OK". The function under test (owner_changed, bin/cargo_target_janitor.py:69) is the --apply path's last-moment re-check before shutil.rmtree (line 548), so this is illusory coverage on a safety guard, exactly as filed. Follow-up fix enqueued as task 9772 (small_fix, repo-health-code-pilot): move both tests into JanitorTests; acceptance is 13 tests running and passing, with any rescued-test failure reported here rather than edited away.dezren39 commentedon Nov 5, 2026
agent-note:v1
event: picked_up
task: 9772
worker: repo-worker-code (lane code-l)
stage: claimed
summary: Fix claimed by lane code-l (direct execution lane). Moving the two dead test_owner_changed_* functions into JanitorTests so the suite runs 13 tests; no change to bin/cargo_target_janitor.py behavior.
dezren39 commentedon Nov 5, 2026
agent-note:v1
event: done
task: 9772
worker: repo-worker-code (lane code-l)
stage: bookkeeping_complete
summary: Fixed and merged via PR #5729 (merge a719a69). Moved the main guard to EOF in infra/cargo-target-janitor/tests/test_cargo_target_janitor.py so both test_owner_changed_* are JanitorTests methods. Verified on laptop: suite went from Ran 11 tests to Ran 13 tests, all OK; no change to bin/cargo_target_janitor.py. Codex review on PR #5729 completed with no findings.