Skip to content

test(mcp): stop MCP dispatch tests from leaking real spawn watchdogs #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
basil-k-aji-dev wants to merge 1 commit into from
Closed

test(mcp): stop MCP dispatch tests from leaking real spawn watchdogs #93

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions tests/unit/mcp/test_mcp_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
mobile_manage_task,
mobile_run_task,
)
from mcp_server.tools import task_runner
from artemis.runtime import trace_store


Expand All @@ -43,6 +44,31 @@ def temp_trace_env(monkeypatch):
shutil.rmtree(temp_dir, ignore_errors=True)


@pytest.fixture(autouse=True)
def stub_spawn_watchdog(monkeypatch):
"""Record watchdog launches instead of starting real daemon threads.

`mobile_run_task` arms `_start_spawn_watchdog` after `Popen` returns.
These tests mock `Popen`, so the watchdog would monitor a PID that is a
MagicMock attribute and outlive both `temp_trace_env`'s directory and its
monkeypatches. Past its deadline it goes on to terminate a process tree,
cancel a reservation, write into the restored trace directory and dispatch
failure notifications -- for tasks that never existed.

Production watchdog behaviour is covered on its own in
`tests/unit/mcp/test_spawn_watchdog.py`, so nothing is lost here.

Yields the recorded `(trace_id, pid, queue_ticket, conversation_id)` calls.
"""
calls: list[tuple] = []
monkeypatch.setattr(
task_runner,
"_start_spawn_watchdog",
lambda *args: calls.append(args),
)
return calls


def test_tool_signatures():
# mobile_run_task signature check
sig_run = inspect.signature(mobile_run_task)
Expand Down Expand Up @@ -88,7 +114,9 @@ def test_mobile_run_task_invalid_model():
mobile_run_task(task_desc="test", conversation_id="conv-1", model="invalid_model")


def test_mobile_run_task_reserves_and_passes_global_queue_ticket(temp_trace_env):
def test_mobile_run_task_reserves_and_passes_global_queue_ticket(
temp_trace_env, stub_spawn_watchdog
):
process = MagicMock(pid=43210)
with (
patch(
Expand Down Expand Up @@ -119,9 +147,11 @@ def test_mobile_run_task_reserves_and_passes_global_queue_ticket(temp_trace_env)
status = trace_store.read_status(result["trace_id"])
assert status["queue_ticket"] == "queue-ticket-1"
assert status["device_serial"] is None
# The watchdog is armed for this dispatch, with this task's identity.
assert stub_spawn_watchdog == [(result["trace_id"], 43210, "queue-ticket-1", "conv-1")]


def test_mobile_run_task_with_device_serial(temp_trace_env):
def test_mobile_run_task_with_device_serial(temp_trace_env, stub_spawn_watchdog):
process = MagicMock(pid=54321)
with (
patch(
Expand Down Expand Up @@ -167,6 +197,9 @@ def test_mobile_run_task_with_device_serial(temp_trace_env):

status = trace_store.read_status(result["trace_id"])
assert status["device_serial"] == "pixel-11-pro-001"
# The watchdog is armed with the mocked runner's pid and this dispatch's
# ticket, not with whatever process happens to own that pid on the host.
assert stub_spawn_watchdog == [(result["trace_id"], 54321, "queue-ticket-dev", "conv-2")]


def test_mobile_run_task_dispatched_to_daemon(temp_trace_env, monkeypatch):
Expand Down