From 073fa26aa07c9c5ab485c92b0abbce19ac8bf2be Mon Sep 17 00:00:00 2001 From: Matthew Wolter Date: Sat, 11 Apr 2026 19:48:02 -0700 Subject: [PATCH] G3: add verification tests for F01, F02, F03, N11 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven new tests in tests/unit/test_connection_manager.py covering all four G3 findings: - test_g3_tcp_connect_returns_plain_string (F01): CONNECTED event payload contains a plain string, not an asyncio.Future. - test_g3_reconnect_loop_does_not_compound (F03): after max_attempts failures, exactly that many connect() calls are made — no fan-out. - test_g3_disconnect_cancels_reconnect_loop (F03): disconnect() mid-loop cancels the single task cleanly. - test_g3_reconnect_callback_called_after_reconnect (F02): callback is invoked after a successful reconnect. - test_g3_reconnect_callback_failure_does_not_crash_loop (F02): callback exception is logged, reconnect still succeeds. - test_g3_connect_none_is_soft_failure (N11): connect() returning None does not set _is_connected or emit CONNECTED. - test_g3_no_reconnect_callback_is_noop (N11/F02): no callback provided — reconnect works, backwards-compatible. Refs: Forensics report findings F01, F02, F03, N11 --- tests/unit/test_connection_manager.py | 294 ++++++++++++++++++++++++++ 1 file changed, 294 insertions(+) create mode 100644 tests/unit/test_connection_manager.py diff --git a/tests/unit/test_connection_manager.py b/tests/unit/test_connection_manager.py new file mode 100644 index 0000000..0713f8a --- /dev/null +++ b/tests/unit/test_connection_manager.py @@ -0,0 +1,294 @@ +"""Tests for G3 reconnect-path fixes (F01, F02, F03, N11).""" + +import asyncio + +import pytest + +from meshcore.connection_manager import ConnectionManager +from meshcore.events import Event, EventDispatcher, EventType + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +class FakeConnection: + """Minimal stub that satisfies ConnectionProtocol.""" + + def __init__(self, connect_results=None): + """ + Args: + connect_results: iterator of return values for successive + connect() calls. ``None`` means soft failure; a string + means success; raising is also supported via sentinel. + """ + self._connect_results = list(connect_results or ["ok"]) + self._call_index = 0 + self.reader = None + + async def connect(self): + if self._call_index < len(self._connect_results): + result = self._connect_results[self._call_index] + self._call_index += 1 + else: + result = self._connect_results[-1] + if isinstance(result, Exception): + raise result + return result + + async def disconnect(self): + pass + + async def send(self, data): + pass + + def set_reader(self, reader): + self.reader = reader + + +class RaisingConnection(FakeConnection): + """Connection that raises on every connect() attempt.""" + + def __init__(self, exc=None): + super().__init__() + self._exc = exc or ConnectionError("boom") + + async def connect(self): + raise self._exc + + +class _EventCollector: + """Subscribes to all events and records them.""" + + def __init__(self, dispatcher: EventDispatcher): + self.events: list[Event] = [] + dispatcher.subscribe(None, self._on_event) + + async def _on_event(self, event: Event): + self.events.append(event) + + +# --------------------------------------------------------------------------- +# F01 — TCP connect() should return a plain value, not an asyncio.Future +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_g3_tcp_connect_returns_plain_string(): + """F01: After the fix, TCPConnection.connect() returns self.host (a + plain string), not an asyncio.Future. We test indirectly via + ConnectionManager — the CONNECTED event payload should contain a plain + string, not a Future object.""" + conn = FakeConnection(connect_results=["10.0.0.1"]) + dispatcher = EventDispatcher() + await dispatcher.start() + try: + collector = _EventCollector(dispatcher) + mgr = ConnectionManager(conn, dispatcher) + + result = await mgr.connect() + + assert result == "10.0.0.1" + # Give the dispatcher a moment to deliver the event + await asyncio.sleep(0.05) + connected_events = [e for e in collector.events if e.type == EventType.CONNECTED] + assert len(connected_events) == 1 + payload = connected_events[0].payload + assert payload["connection_info"] == "10.0.0.1" + # The payload value must NOT be an asyncio.Future + assert not isinstance(payload["connection_info"], asyncio.Future) + finally: + await dispatcher.stop() + + +# --------------------------------------------------------------------------- +# F03 — Reconnect attempts must not compound (no tail-recursive create_task) +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_g3_reconnect_loop_does_not_compound(): + """F03: _attempt_reconnect must use a single iterative loop. After + max_reconnect_attempts failures, exactly that many connect() calls + should have been made — no exponential fan-out from orphaned tasks.""" + # All attempts fail (return None) + conn = FakeConnection(connect_results=[None, None, None, None]) + dispatcher = EventDispatcher() + await dispatcher.start() + try: + collector = _EventCollector(dispatcher) + mgr = ConnectionManager( + conn, dispatcher, auto_reconnect=True, max_reconnect_attempts=3, + ) + mgr._is_connected = True # simulate a live connection + + await mgr.handle_disconnect("test_disconnect") + # Wait for the reconnect loop to exhaust all attempts + # (3 attempts × 1s sleep each, but we can just await the task) + if mgr._reconnect_task: + await mgr._reconnect_task + + # Exactly 3 connect() calls should have been made + assert conn._call_index == 3 + + # A DISCONNECTED event with max_attempts_exceeded should have fired + await asyncio.sleep(0.05) + disconnected = [e for e in collector.events if e.type == EventType.DISCONNECTED] + assert len(disconnected) == 1 + assert disconnected[0].payload.get("max_attempts_exceeded") is True + finally: + await dispatcher.stop() + + +@pytest.mark.asyncio +async def test_g3_disconnect_cancels_reconnect_loop(): + """F03: disconnect() during an active reconnect loop must cancel the + single task cleanly — no orphaned tasks left running.""" + # Simulate a connection that always fails (returns None), giving us + # time to call disconnect() mid-loop. + conn = FakeConnection(connect_results=[None, None, None, None, None]) + dispatcher = EventDispatcher() + await dispatcher.start() + try: + mgr = ConnectionManager( + conn, dispatcher, auto_reconnect=True, max_reconnect_attempts=5, + ) + mgr._is_connected = True + + await mgr.handle_disconnect("test_disconnect") + + # Let the first attempt start (wait just past the 1s sleep) + await asyncio.sleep(1.2) + assert conn._call_index >= 1 # at least one attempt made + + # Now disconnect — should cancel the loop + await mgr.disconnect() + + assert mgr._reconnect_task is None + calls_at_cancel = conn._call_index + + # Wait a bit and confirm no more attempts happened + await asyncio.sleep(2) + assert conn._call_index == calls_at_cancel + finally: + await dispatcher.stop() + + +# --------------------------------------------------------------------------- +# F02 — reconnect_callback (send_appstart) is called after reconnect +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_g3_reconnect_callback_called_after_reconnect(): + """F02: When ConnectionManager reconnects successfully, the + reconnect_callback (e.g. send_appstart) must be invoked.""" + callback_called = [] + + async def fake_appstart(): + callback_called.append(True) + + # First connect() fails (None), second succeeds + conn = FakeConnection(connect_results=[None, "10.0.0.1"]) + dispatcher = EventDispatcher() + await dispatcher.start() + try: + mgr = ConnectionManager( + conn, dispatcher, + auto_reconnect=True, + max_reconnect_attempts=3, + reconnect_callback=fake_appstart, + ) + mgr._is_connected = True + + await mgr.handle_disconnect("test_disconnect") + if mgr._reconnect_task: + await mgr._reconnect_task + + assert len(callback_called) == 1 + finally: + await dispatcher.stop() + + +@pytest.mark.asyncio +async def test_g3_reconnect_callback_failure_does_not_crash_loop(): + """F02: If the reconnect_callback raises, the reconnect still counts + as successful (transport is up) — the callback failure is logged but + does not crash the loop or leave the manager in a broken state.""" + async def failing_callback(): + raise RuntimeError("appstart failed") + + # connect() succeeds on first attempt + conn = FakeConnection(connect_results=["10.0.0.1"]) + dispatcher = EventDispatcher() + await dispatcher.start() + try: + collector = _EventCollector(dispatcher) + mgr = ConnectionManager( + conn, dispatcher, + auto_reconnect=True, + max_reconnect_attempts=3, + reconnect_callback=failing_callback, + ) + mgr._is_connected = True + + await mgr.handle_disconnect("test_disconnect") + if mgr._reconnect_task: + await mgr._reconnect_task + + # Despite callback failure, CONNECTED event should have fired + await asyncio.sleep(0.05) + connected = [e for e in collector.events if e.type == EventType.CONNECTED] + assert len(connected) == 1 + assert mgr._is_connected is True + finally: + await dispatcher.stop() + + +# --------------------------------------------------------------------------- +# N11 — connect() returning None is a soft failure (BLE scan miss) +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_g3_connect_none_is_soft_failure(): + """N11: When connect() returns None (e.g. BLE scan found no device), + ConnectionManager.connect() should NOT set _is_connected and should + NOT emit a CONNECTED event.""" + conn = FakeConnection(connect_results=[None]) + dispatcher = EventDispatcher() + await dispatcher.start() + try: + collector = _EventCollector(dispatcher) + mgr = ConnectionManager(conn, dispatcher) + + result = await mgr.connect() + + assert result is None + assert mgr._is_connected is False + await asyncio.sleep(0.05) + connected = [e for e in collector.events if e.type == EventType.CONNECTED] + assert len(connected) == 0 + finally: + await dispatcher.stop() + + +@pytest.mark.asyncio +async def test_g3_no_reconnect_callback_is_noop(): + """N11/F02: When no reconnect_callback is provided (backwards compat + for direct ConnectionManager users), reconnect should still work.""" + conn = FakeConnection(connect_results=["10.0.0.1"]) + dispatcher = EventDispatcher() + await dispatcher.start() + try: + mgr = ConnectionManager( + conn, dispatcher, + auto_reconnect=True, + max_reconnect_attempts=3, + # No reconnect_callback — default None + ) + mgr._is_connected = True + + await mgr.handle_disconnect("test_disconnect") + if mgr._reconnect_task: + await mgr._reconnect_task + + assert mgr._is_connected is True + finally: + await dispatcher.stop()