google / google/artemis

MCP unit tests leak real spawn watchdogs for mocked processes

Open Beginner friendly
#43 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.