Replace fixed-duration sleeps after bus events with gather

tests/addons/test_addon.py

@@ -6,7 +6,7 @@
 from http import HTTPStatus
 from pathlib import Path, PurePath
 from typing import Any
-from unittest.mock import MagicMock, PropertyMock, call, patch
+from unittest.mock import AsyncMock, MagicMock, PropertyMock, call, patch
 
 import aiodocker
 from aiodocker.containers import DockerContainer
@@ -47,13 +47,14 @@
 
 from .test_manager import BOOT_FAIL_ISSUE, BOOT_FAIL_SUGGESTIONS
 
-from tests.common import get_fixture_path, is_in_list
+from tests.common import fire_bus_event, get_fixture_path, is_in_list
 from tests.const import TEST_ADDON_SLUG
 
 
-def _fire_test_event(coresys: CoreSys, name: str, state: ContainerState):
-    """Fire a test event."""
-    coresys.bus.fire_event(
+async def _fire_test_event(coresys: CoreSys, name: str, state: ContainerState) -> None:
+    """Fire a test event and await the listener tasks the bus spawned."""
+    await fire_bus_event(
+        coresys,
         BusEvent.DOCKER_CONTAINER_STATE_CHANGE,
         DockerContainerStateEvent(
             name=name,
@@ -125,25 +126,30 @@ async def test_app_state_listener(coresys: CoreSys, install_app_ssh: App) -> Non
     assert install_app_ssh.state == AppState.UNKNOWN
 
     with patch.object(App, "watchdog_container"):
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING
+        )
         assert install_app_ssh.state == AppState.STARTED
 
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED
+        )
         assert install_app_ssh.state == AppState.STOPPED
 
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.HEALTHY)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.HEALTHY
+        )
         assert install_app_ssh.state == AppState.STARTED
 
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.FAILED)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.FAILED
+        )
         assert install_app_ssh.state == AppState.ERROR
 
         # Test other apps are ignored
-        _fire_test_event(coresys, "addon_local_non_installed", ContainerState.RUNNING)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, "addon_local_non_installed", ContainerState.RUNNING
+        )
         assert install_app_ssh.state == AppState.ERROR
 
 
@@ -155,15 +161,20 @@ async def test_app_watchdog(coresys: CoreSys, install_app_ssh: App) -> None:
     install_app_ssh.watchdog = True
     install_app_ssh._manual_stop = False  # pylint: disable=protected-access
 
+    # Watchdog does ``await (await self.start())`` because App.start returns
+    # an asyncio.Task. The mock must mirror that shape.
+    done_task = asyncio.get_running_loop().create_future()
+    done_task.set_result(None)
     with (
-        patch.object(App, "restart") as restart,
-        patch.object(App, "start") as start,
+        patch.object(App, "restart", AsyncMock(return_value=done_task)) as restart,
+        patch.object(App, "start", AsyncMock(return_value=done_task)) as start,
         patch.object(DockerApp, "current_state") as current_state,
     ):
         # Restart if it becomes unhealthy
         current_state.return_value = ContainerState.UNHEALTHY
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.UNHEALTHY)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.UNHEALTHY
+        )
         restart.assert_called_once()
         start.assert_not_called()
 
@@ -172,8 +183,9 @@ async def test_app_watchdog(coresys: CoreSys, install_app_ssh: App) -> None:
         # Rebuild if it failed
         current_state.return_value = ContainerState.FAILED
         with patch.object(DockerApp, "stop") as stop:
-            _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.FAILED)
-            await asyncio.sleep(0)
+            await _fire_test_event(
+                coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.FAILED
+            )
             stop.assert_called_once_with(remove_container=True)
             restart.assert_not_called()
             start.assert_called_once()
@@ -182,15 +194,17 @@ async def test_app_watchdog(coresys: CoreSys, install_app_ssh: App) -> None:
 
         # Do not process event if container state has changed since fired
         current_state.return_value = ContainerState.HEALTHY
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.FAILED)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.FAILED
+        )
         restart.assert_not_called()
         start.assert_not_called()
 
         # Other apps ignored
         current_state.return_value = ContainerState.UNHEALTHY
-        _fire_test_event(coresys, "addon_local_non_installed", ContainerState.UNHEALTHY)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, "addon_local_non_installed", ContainerState.UNHEALTHY
+        )
         restart.assert_not_called()
         start.assert_not_called()
 
@@ -216,8 +230,9 @@ async def test_watchdog_port_conflict_does_not_retry(
         patch("supervisor.addons.addon.async_capture_exception") as capture_exception,
     ):
         caplog.clear()
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.FAILED)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.FAILED
+        )
 
         start.assert_called_once()
         capture_exception.assert_not_called()
@@ -231,8 +246,12 @@ async def test_watchdog_on_stop(coresys: CoreSys, install_app_ssh: App) -> None:
 
     install_app_ssh.watchdog = True
 
+    # Watchdog does ``await (await self.restart())`` because App.restart
+    # returns an asyncio.Task; the mock must mirror that shape.
+    done_task = asyncio.get_running_loop().create_future()
+    done_task.set_result(None)
     with (
-        patch.object(App, "restart") as restart,
+        patch.object(App, "restart", AsyncMock(return_value=done_task)) as restart,
         patch.object(
             DockerApp,
             "current_state",
@@ -241,18 +260,22 @@ async def test_watchdog_on_stop(coresys: CoreSys, install_app_ssh: App) -> None:
         patch.object(DockerApp, "stop"),
     ):
         # Do not restart when app stopped by user
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING
+        )
         await install_app_ssh.stop()
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED
+        )
         restart.assert_not_called()
 
         # Do restart app if it stops and user didn't do it
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
-        await asyncio.sleep(0)
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING
+        )
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED
+        )
         restart.assert_called_once()
 
 
@@ -279,8 +302,7 @@ async def test_listener_attached_on_install(coresys: CoreSys):
     # Normally this would be defaulted to False on start of the app but test skips that
     coresys.apps.get_local_only(TEST_ADDON_SLUG).watchdog = False
 
-    _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
-    await asyncio.sleep(0)
+    await _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
     assert coresys.apps.get(TEST_ADDON_SLUG).state == AppState.STARTED
 
 
@@ -297,8 +319,12 @@ async def test_watchdog_during_attach(
     store = coresys.apps.store[TEST_ADDON_SLUG]
     await coresys.apps.data.install(store)
 
+    # Watchdog does ``await (await self.restart())`` because App.restart
+    # returns an asyncio.Task; the mock must mirror that shape.
+    done_task = asyncio.get_running_loop().create_future()
+    done_task.set_result(None)
     with (
-        patch.object(App, "restart") as restart,
+        patch.object(App, "restart", AsyncMock(return_value=done_task)) as restart,
         patch.object(HwHelper, "last_boot", return_value=utcnow()),
         patch.object(DockerApp, "attach"),
         patch.object(
@@ -315,8 +341,9 @@ async def test_watchdog_during_attach(
         app.watchdog = True
 
         await app.load()
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED)
-        await asyncio.sleep(0)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED
+        )
 
         assert restart.call_count == restart_count
 
@@ -386,7 +413,7 @@ async def test_start(coresys: CoreSys, install_app_ssh: App) -> None:
     start_task = await install_app_ssh.start()
     assert start_task
 
-    _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
+    await _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
     await start_task
     assert install_app_ssh.state == AppState.STARTED
 
@@ -409,14 +436,12 @@ async def test_start_wait_healthcheck(
     start_task = await install_app_ssh.start()
     assert start_task
 
-    _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
-    await asyncio.sleep(0.01)
+    await _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
 
     assert not start_task.done()
     assert install_app_ssh.state == AppState.STARTUP
 
-    _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", state)
-    await asyncio.sleep(0.01)
+    await _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", state)
 
     assert start_task.done()
     assert install_app_ssh.state == AppState.STARTED
@@ -455,7 +480,7 @@ async def test_restart(coresys: CoreSys, install_app_ssh: App) -> None:
     start_task = await install_app_ssh.restart()
     assert start_task
 
-    _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
+    await _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
     await start_task
     assert install_app_ssh.state == AppState.STARTED
 
@@ -654,7 +679,9 @@ async def test_backup_cold_mode_with_watchdog(
     async def mock_stop(*args, **kwargs):
         container.show.return_value["State"]["Status"] = "stopped"
         container.show.return_value["State"]["Running"] = False
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED
+        )
 
     # Patching out the normal end of backup process leaves the container in a stopped state
     # Watchdog should still not try to restart it though, it should remain this way
@@ -738,7 +765,9 @@ async def test_restore_while_running_with_watchdog(
     async def mock_stop(*args, **kwargs):
         container.show.return_value["State"]["Status"] = "stopped"
         container.show.return_value["State"]["Running"] = False
-        _fire_test_event(coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED)
+        await _fire_test_event(
+            coresys, f"addon_{TEST_ADDON_SLUG}", ContainerState.STOPPED
+        )
 
     # We restore a stopped backup so restore will not restart it
     # Watchdog will see it stop and should not attempt reanimation either

tests/addons/test_manager.py

@@ -39,7 +39,7 @@
 from supervisor.utils import check_exception_chain
 from supervisor.utils.common import write_json_file
 
-from tests.common import load_json_fixture
+from tests.common import fire_bus_event, load_json_fixture
 from tests.const import TEST_ADDON_SLUG
 
 BOOT_FAIL_ISSUE = Issue(
@@ -384,7 +384,8 @@ async def test_start_wait_resolved_on_uninstall_in_startup(
     start_task = await install_app_ssh.start()
     assert start_task
 
-    coresys.bus.fire_event(
+    await fire_bus_event(
+        coresys,
         BusEvent.DOCKER_CONTAINER_STATE_CHANGE,
         DockerContainerStateEvent(
             name=f"addon_{TEST_ADDON_SLUG}",
@@ -393,7 +394,6 @@ async def test_start_wait_resolved_on_uninstall_in_startup(
             time=1,
         ),
     )
-    await asyncio.sleep(0.01)
 
     assert not start_task.done()
     assert install_app_ssh.state == AppState.STARTUP
@@ -525,6 +525,11 @@ async def mock_update(*args, **kwargs):
         patch.object(App, "restart") as restart,
     ):
         await coresys.apps.update("local_ssh")
+        # mock_update yielded once (sleep(0)), giving the watchdog task
+        # spawned by mock_stop time to run to completion within update's
+        # own awaits — so by the time we get here it's already done.
+        # A trailing sleep(0) defends against scheduling jitter without
+        # racing the assertion.
         await asyncio.sleep(0)
         start.assert_called_once()
         restart.assert_not_called()

tests/api/test_addons.py

@@ -123,7 +123,7 @@ async def test_api_app_start_healthcheck(
 
     async def container_events():
         nonlocal state_changes
-        await asyncio.sleep(0.01)
+        await asyncio.sleep(0)
 
         await install_app_ssh.container_state_changed(
             _create_test_event(f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)
@@ -162,7 +162,7 @@ async def test_api_app_restart_healthcheck(
 
     async def container_events():
         nonlocal state_changes
-        await asyncio.sleep(0.01)
+        await asyncio.sleep(0)
 
         await install_app_ssh.container_state_changed(
             _create_test_event(f"addon_{TEST_ADDON_SLUG}", ContainerState.RUNNING)

tests/api/test_store.py

@@ -225,7 +225,7 @@ async def test_api_store_update_healthcheck(
 
     async def container_events():
         nonlocal state_changes
-        await asyncio.sleep(0.01)
+        await asyncio.sleep(0)
 
         await install_app_ssh.container_state_changed(
             DockerContainerStateEvent(

tests/backups/test_manager.py

@@ -966,7 +966,7 @@ async def test_backup_with_healthcheck(
 
     async def container_events():
         nonlocal state_changes
-        await asyncio.sleep(0.01)
+        await asyncio.sleep(0)
 
         await install_app_ssh.container_state_changed(
             DockerContainerStateEvent(

tests/common.py

@@ -12,13 +12,26 @@
 
 from dbus_fast.aio.message_bus import MessageBus
 
+from supervisor.const import BusEvent
+from supervisor.coresys import CoreSys
 from supervisor.jobs.decorator import Job
 from supervisor.resolution.validate import get_valid_modules
 from supervisor.utils.yaml import read_yaml_file
 
 from .dbus_service_mocks.base import DBusServiceMock
 
 
+async def fire_bus_event(coresys: CoreSys, event: BusEvent, data: Any) -> None:
+    """Fire a bus event and await its listener tasks.
+
+    ``Bus.fire_event`` is sync and returns the listener tasks it spawned.
+    Tests that drive a system under test by firing a bus event need to
+    wait for those listener tasks to finish before asserting; this helper
+    bundles the gather so call sites stay short.
+    """
+    await asyncio.gather(*coresys.bus.fire_event(event, data))
+
+
 def get_fixture_path(filename: str) -> Path:
     """Get path for fixture."""
     return Path(Path(__file__).parent.joinpath("fixtures"), filename)