MCP unit tests leak real spawn watchdogs for mocked processes
- Dominant language
- Python
- Stars
- 5.9k
- Forks
- 516
- Avg merge
- 22m
- Merged PRs (30d)
- 5
Description
## Problem
`tests/unit/mcp/test_mcp_tools.py` mocks `subprocess.Popen` but leaves `_start_spawn_watchdog` live. Four unit tests start real daemon threads for fake runner PIDs. Those threads outlive the test's temporary trace directory and monkeypatches. After the 60-second deadline, they can invoke real process-tree termination, cancel reservations, write into the restored trace directory, and dispatch desktop failure notifications for simulated tasks.
This was observed during an extended Windows deterministic-suite run: a desktop "Artemis Task Failed" notification referred to fake PID `54321` and test conversation `conv-2`. The notification's trace ID matched the combined test worktree's notification log. No private screenshot is attached.
## Root cause
`mobile_run_task()` calls `_start_spawn_watchdog()` after the mocked `Popen` returns. `temp_trace_env` removes its directory and restores `trace_store.TRACES_DIR` before the real monitoring thread completes. Mocking only the child process does not isolate the background work that manages it.
## Safe reproduction
A temporary pytest guard intercepted `threading.Thread.start` for names starting with `spawn-watchdog-`, recorded attempts without actually starting them, and asserted at teardown that no such attempts occurred. Other threads were left unchanged. Running the MCP tool module with that guard on `4f9eb3d` produced:
```text
ERROR test_mobile_run_task_reserves_and_passes_global_queue_ticket
ERROR test_mobile_run_task_with_device_serial
ERROR test_mobile_run_task_forwards_pro_tuning_to_background_runner
ERROR test_mobile_run_task_omits_pro_tuning_flags_when_unset
AssertionError: Test leaked 1 real spawn watchdog thread(s)
25 passed, 4 errors in 1.55s
```
The guard prevents process termination and notifications while reproducing the leak.
## Fix and after log
An autouse fixture local to the MCP test module now replaces the watchdog launcher for mocked MCP dispatch tests. Queue/serial dispatch tests assert the exact watchdog arguments. Production watchdog behavior remains covered separately by `test_spawn_watchdog.py`, including successful startup, terminal tasks, deadlines, cleanup, and notifications with their external effects mocked.
```text
Same guarded MCP tool module: 25 passed in 1.23s
Dedicated production watchdog tests: 4 passed in 0.49s
```
The fixture is scoped to each test in the MCP tool test module (not to the entire suite). This is an additional test-isolation fix in #42; no production behavior changes are needed.
Contributor guide
Research direction
Start with tests/unit/mcp/test_mcp_tools.py and inspect the mocked MCP dispatch tests plus the watchdog launcher. Use the described Thread.start guard and run that module to verify no real spawn-watchdog threads are started; then run test_spawn_watchdog.py to confirm production watchdog coverage still passes with external effects mocked.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 85/100