Skip to content

Commit 319fe62

Browse files
committed
Add broker proxy tests to keep CI coverage above 90%
1 parent 6c36559 commit 319fe62

1 file changed

Lines changed: 70 additions & 2 deletions

File tree

tests/unit/test_broker_proxy.py

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,15 @@ def test_pid_belongs_to_broker_false_on_ps_failure(self, tmp_path: Path) -> None
818818
):
819819
assert proxy._pid_belongs_to_broker(123) is False
820820

821+
def test_pid_belongs_to_broker_false_for_unrelated_command(self, tmp_path: Path) -> None:
822+
cfg = _make_config(tmp_path)
823+
proxy = BrokerProxy(cfg)
824+
with patch(
825+
"mcpbridge_wrapper.broker.proxy.subprocess.check_output",
826+
return_value="python -m http.server 8000",
827+
):
828+
assert proxy._pid_belongs_to_broker(123) is False
829+
821830
def test_stop_stale_daemon_no_pid_file_noop(self, tmp_path: Path) -> None:
822831
"""No PID file means there is nothing to stop."""
823832
cfg = _make_config(tmp_path)
@@ -826,6 +835,21 @@ def test_stop_stale_daemon_no_pid_file_noop(self, tmp_path: Path) -> None:
826835
proxy._stop_stale_daemon()
827836
mock_kill.assert_not_called()
828837

838+
def test_stop_stale_daemon_invalid_pid_text_returns(self, tmp_path: Path) -> None:
839+
"""Corrupt PID file short-circuits without touching broker files."""
840+
cfg = _make_config(tmp_path)
841+
cfg.pid_file.write_text("not-a-pid")
842+
cfg.socket_path.write_text("sock")
843+
cfg.version_file.write_text("version")
844+
proxy = BrokerProxy(cfg)
845+
846+
with patch("mcpbridge_wrapper.broker.proxy.os.kill") as mock_kill:
847+
proxy._stop_stale_daemon()
848+
849+
mock_kill.assert_not_called()
850+
assert cfg.socket_path.exists()
851+
assert cfg.version_file.exists()
852+
829853
def test_stop_stale_daemon_skips_unrelated_pid(self, tmp_path: Path) -> None:
830854
"""Unrelated PID in stale pid file is never signaled."""
831855
cfg = _make_config(tmp_path)
@@ -862,6 +886,24 @@ def test_stop_stale_daemon_cleanup_when_process_missing(self, tmp_path: Path) ->
862886
assert not cfg.socket_path.exists()
863887
assert not cfg.version_file.exists()
864888

889+
def test_stop_stale_daemon_permission_error_cleans_files(self, tmp_path: Path) -> None:
890+
"""PermissionError on SIGTERM still triggers stale file cleanup."""
891+
cfg = _make_config(tmp_path)
892+
cfg.pid_file.write_text("432")
893+
cfg.socket_path.write_text("stale")
894+
cfg.version_file.write_text("old")
895+
proxy = BrokerProxy(cfg)
896+
897+
with patch.object(proxy, "_pid_belongs_to_broker", return_value=True), patch(
898+
"mcpbridge_wrapper.broker.proxy.os.kill",
899+
side_effect=PermissionError,
900+
):
901+
proxy._stop_stale_daemon()
902+
903+
assert not cfg.pid_file.exists()
904+
assert not cfg.socket_path.exists()
905+
assert not cfg.version_file.exists()
906+
865907
def test_stop_stale_daemon_waits_for_exit_then_cleans(self, tmp_path: Path) -> None:
866908
"""SIGTERM path waits for exit probe and removes broker files."""
867909
cfg = _make_config(tmp_path)
@@ -870,19 +912,45 @@ def test_stop_stale_daemon_waits_for_exit_then_cleans(self, tmp_path: Path) -> N
870912
cfg.version_file.write_text("old")
871913
proxy = BrokerProxy(cfg)
872914

915+
probes = {"count": 0}
916+
873917
def fake_kill(pid: int, sig: int) -> None:
874918
assert pid == 654
875919
if sig == signal.SIGTERM:
876920
return None
877-
raise ProcessLookupError
921+
probes["count"] += 1
922+
if probes["count"] > 1:
923+
raise ProcessLookupError
924+
return None
878925

879-
with patch("mcpbridge_wrapper.broker.proxy.os.kill", side_effect=fake_kill):
926+
with patch.object(proxy, "_pid_belongs_to_broker", return_value=True), patch(
927+
"mcpbridge_wrapper.broker.proxy.os.kill",
928+
side_effect=fake_kill,
929+
), patch("mcpbridge_wrapper.broker.proxy.time.sleep", return_value=None):
880930
proxy._stop_stale_daemon()
881931

932+
assert probes["count"] == 2
882933
assert not cfg.pid_file.exists()
883934
assert not cfg.socket_path.exists()
884935
assert not cfg.version_file.exists()
885936

937+
@pytest.mark.asyncio
938+
async def test_connect_with_timeout_returns_streams_on_success(self, tmp_path: Path) -> None:
939+
"""Successful unix connect returns the opened stream pair."""
940+
cfg = _make_config(tmp_path)
941+
proxy = BrokerProxy(cfg, connect_timeout=0.2)
942+
expected_reader = asyncio.StreamReader()
943+
expected_writer = MagicMock()
944+
945+
with patch(
946+
"mcpbridge_wrapper.broker.proxy.asyncio.open_unix_connection",
947+
AsyncMock(return_value=(expected_reader, expected_writer)),
948+
):
949+
reader, writer = await proxy._connect_with_timeout()
950+
951+
assert reader is expected_reader
952+
assert writer is expected_writer
953+
886954
@pytest.mark.asyncio
887955
async def test_version_mismatch_triggers_restart(self, tmp_path: Path) -> None:
888956
"""When version mismatches, old daemon is stopped and new one spawned."""

0 commit comments

Comments
 (0)