test: browser tree-kill unit tests no longer SIGKILL real processes under PID 999

Two mocked tests drive _legacy_kill_process_tree down its shared-group
branch with a MagicMock proc whose pid is 999. That branch snapshots
psutil.Process(proc.pid).children(recursive=True) and SIGKILLs each
descendant, and psutil was never patched: the tests killed the real
descendants of whatever the machine runs as PID 999. On macOS runners
PIDs are allocated system-wide, and when 999 was an ancestor of the
test worker the file died mid-run (CRASHED, killed by SIGKILL), which
is how it surfaced on a PR whose diff never touches the browser tool.

Both tests now run against a fake process table, and the shared-group
test asserts the snapshotted descendant is killed individually (the
contract it was written for).

Live repro: inside an unprivileged user+PID namespace with PID 999 an
ancestor of the test runner, main's file reports CRASHED (killed by
SIGKILL); with this change it passes 9/9.
This commit is contained in:
Teknium
2026-10-06 03:13:51 -07:00
parent e046826bf3
commit b82d35571c

View File

@@ -11,6 +11,26 @@ import pytest
from tools.browser_tool_lifecycle import _kill_process_tree, _legacy_kill_process_tree
@pytest.fixture
def fake_process_table(monkeypatch):
"""psutil for a MagicMock proc: the shared-group branch snapshots
psutil.Process(proc.pid).children() and SIGKILLs each one. Against the real
table, pid=999 is whatever the machine runs there, and when that is an
ancestor of the test worker the test SIGKILLs its own interpreter (seen as
a CRASHED file on macOS runners, whose PIDs are allocated system-wide)."""
import psutil
child = MagicMock()
queried = []
def process(pid):
queried.append(pid)
return MagicMock(children=MagicMock(return_value=[child]))
monkeypatch.setattr(psutil, "Process", process)
return queried, child
@pytest.mark.platforms("posix")
def test_posix_kills_process_group_term_then_kill(monkeypatch):
proc = MagicMock(pid=999)
@@ -22,12 +42,13 @@ def test_posix_kills_process_group_term_then_kill(monkeypatch):
@pytest.mark.platforms("posix")
def test_posix_missing_process_returns_silently(monkeypatch):
def test_posix_missing_process_returns_silently(monkeypatch, fake_process_table):
def missing(pid):
raise ProcessLookupError()
monkeypatch.setattr(os, "getpgid", missing)
_legacy_kill_process_tree(MagicMock(pid=999))
assert fake_process_table[0] == [999]
@pytest.mark.platforms("posix")
@@ -68,7 +89,7 @@ def test_windows_taskkill_targets_tree_and_is_best_effort(error):
@pytest.mark.platforms("posix")
def test_posix_shared_group_is_never_killpgd(monkeypatch):
def test_posix_shared_group_is_never_killpgd(monkeypatch, fake_process_table):
"""A child spawned without start_new_session shares OUR process group, so
os.getpgid returns the caller's own pgid — killpg would signal the whole
Hermes/test-runner tree. Only a group leader may be killpg'd; the direct
@@ -83,6 +104,9 @@ def test_posix_shared_group_is_never_killpgd(monkeypatch):
assert killpg_calls == []
proc.kill.assert_called_once()
queried, descendant = fake_process_table
assert queried == [999]
descendant.kill.assert_called_once()
@pytest.mark.platforms("posix")