From e1c9c8b7861f039fbb79aeebabb34ffaa088531a Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Fri, 7 Mar 2025 07:29:24 -0500 Subject: [PATCH] Finish out effort of adding and enabling blockbuster in tests (#5735) * Finish out effort of adding and enabling blockbuster * Skip getting addon file size until securetar fixed * Fix test for devcontainer and blocking I/O * Fix docker fixture and load_config to post_init --- supervisor/backups/backup.py | 6 ++- supervisor/bootstrap.py | 2 +- supervisor/docker/manager.py | 41 +++++++++++++++---- supervisor/host/apparmor.py | 14 +++++-- supervisor/mounts/manager.py | 7 +++- supervisor/resolution/checks/core_security.py | 15 ++++--- tests/backups/test_manager.py | 38 ++++++++++------- tests/conftest.py | 21 ++++------ tests/dbus/test_interface.py | 7 +++- tests/resolution/check/test_check.py | 8 +++- tests/store/test_custom_repository.py | 18 +++++--- tests/store/test_store_manager.py | 8 +++- tests/test_ingress.py | 8 ++-- tests/utils/test_apparmor.py | 4 +- tests/utils/test_dbus.py | 7 +++- tests/utils/test_sentry.py | 5 ++- 16 files changed, 139 insertions(+), 70 deletions(-) diff --git a/supervisor/backups/backup.py b/supervisor/backups/backup.py index c5dd67054..f290189c5 100644 --- a/supervisor/backups/backup.py +++ b/supervisor/backups/backup.py @@ -601,7 +601,9 @@ class Backup(JobGroup): ATTR_SLUG: addon.slug, ATTR_NAME: addon.name, ATTR_VERSION: addon.version, - ATTR_SIZE: addon_file.size, + # Bug - addon_file.size used to give us this information + # It always returns 0 in current securetar. Skipping until fixed + ATTR_SIZE: 0, } ) @@ -640,7 +642,7 @@ class Backup(JobGroup): ) # If exists inside backup - if not addon_file.path.exists(): + if not await self.sys_run_in_executor(addon_file.path.exists): raise BackupError(f"Can't find backup {addon_slug}", _LOGGER.error) # Perform a restore diff --git a/supervisor/bootstrap.py b/supervisor/bootstrap.py index 5275f8fd5..ec04d177c 100644 --- a/supervisor/bootstrap.py +++ b/supervisor/bootstrap.py @@ -55,7 +55,7 @@ async def initialize_coresys() -> CoreSys: coresys = await CoreSys().load_config() # Initialize core objects - coresys.docker = await DockerAPI(coresys).load_config() + coresys.docker = await DockerAPI(coresys).post_init() coresys.resolution = await ResolutionManager(coresys).load_config() await coresys.resolution.load_modules() coresys.jobs = await JobManager(coresys).load_config() diff --git a/supervisor/docker/manager.py b/supervisor/docker/manager.py index 1e1862abf..982d01d34 100644 --- a/supervisor/docker/manager.py +++ b/supervisor/docker/manager.py @@ -1,6 +1,8 @@ """Manager for Supervisor Docker.""" +import asyncio from contextlib import suppress +from functools import partial from ipaddress import IPv4Address import logging import os @@ -105,19 +107,42 @@ class DockerAPI: def __init__(self, coresys: CoreSys): """Initialize Docker base wrapper.""" - self.docker: DockerClient = DockerClient( - base_url=f"unix:/{str(SOCKET_DOCKER)}", version="auto", timeout=900 - ) - self.network: DockerNetwork = DockerNetwork(self.docker) - self._info: DockerInfo = DockerInfo.new(self.docker.info()) + self._docker: DockerClient | None = None + self._network: DockerNetwork | None = None + self._info: DockerInfo | None = None self.config: DockerConfig = DockerConfig() self._monitor: DockerMonitor = DockerMonitor(coresys) - async def load_config(self) -> Self: - """Load config in executor.""" + async def post_init(self) -> Self: + """Post init actions that must be done in event loop.""" + self._docker = await asyncio.get_running_loop().run_in_executor( + None, + partial( + DockerClient, + base_url=f"unix:/{str(SOCKET_DOCKER)}", + version="auto", + timeout=900, + ), + ) + self._network = DockerNetwork(self._docker) + self._info = DockerInfo.new(self.docker.info()) await self.config.read_data() return self + @property + def docker(self) -> DockerClient: + """Get docker API client.""" + if not self._docker: + raise RuntimeError("Docker API Client not initialized!") + return self._docker + + @property + def network(self) -> DockerNetwork: + """Get Docker network.""" + if not self._network: + raise RuntimeError("Docker Network not initialized!") + return self._network + @property def images(self) -> ImageCollection: """Return API images.""" @@ -136,6 +161,8 @@ class DockerAPI: @property def info(self) -> DockerInfo: """Return local docker info.""" + if not self._info: + raise RuntimeError("Docker Info not initialized!") return self._info @property diff --git a/supervisor/host/apparmor.py b/supervisor/host/apparmor.py index ae5843101..25dde37e6 100644 --- a/supervisor/host/apparmor.py +++ b/supervisor/host/apparmor.py @@ -54,10 +54,16 @@ class AppArmorControl(CoreSysAttributes): async def load(self) -> None: """Load available profiles.""" - for content in self.sys_config.path_apparmor.iterdir(): - if not content.is_file(): - continue - self._profiles.add(content.name) + + def find_profiles() -> set[str]: + profiles: set[str] = set() + for content in self.sys_config.path_apparmor.iterdir(): + if not content.is_file(): + continue + profiles.add(content.name) + return profiles + + self._profiles = await self.sys_run_in_executor(find_profiles) _LOGGER.info("Loading AppArmor Profiles: %s", self._profiles) diff --git a/supervisor/mounts/manager.py b/supervisor/mounts/manager.py index 8289d0389..0926cde6b 100644 --- a/supervisor/mounts/manager.py +++ b/supervisor/mounts/manager.py @@ -292,9 +292,12 @@ class MountManager(FileConfiguration, CoreSysAttributes): where.as_posix(), ) path = self.sys_config.path_emergency / mount.name - if not path.exists(): - path.mkdir(mode=0o444) + def emergency_mkdir(): + if not path.exists(): + path.mkdir(mode=0o444) + + await self.sys_run_in_executor(emergency_mkdir) path = self.sys_config.local_to_extern_path(path) self._bound_mounts[mount.name] = bound_mount = BoundMount( diff --git a/supervisor/resolution/checks/core_security.py b/supervisor/resolution/checks/core_security.py index 5114617eb..0c9a3aded 100644 --- a/supervisor/resolution/checks/core_security.py +++ b/supervisor/resolution/checks/core_security.py @@ -30,9 +30,7 @@ class CheckCoreSecurity(CheckBase): # Security issue < 2021.1.5 & Custom components try: if self.sys_homeassistant.version < AwesomeVersion("2021.1.5"): - if Path( - self.sys_config.path_homeassistant, "custom_components" - ).exists(): + if await self.sys_run_in_executor(self._custom_components_exists): self.sys_resolution.create_issue( IssueType.SECURITY, ContextType.CORE, @@ -49,9 +47,14 @@ class CheckCoreSecurity(CheckBase): return False except AwesomeVersionException: return True - if not Path(self.sys_config.path_homeassistant, "custom_components").exists(): - return False - return True + return await self.sys_run_in_executor(self._custom_components_exists) + + def _custom_components_exists(self) -> bool: + """Return true if custom components folder exists. + + Must be run in executor. + """ + return Path(self.sys_config.path_homeassistant, "custom_components").exists() @property def issue(self) -> IssueType: diff --git a/tests/backups/test_manager.py b/tests/backups/test_manager.py index 38dee17ff..b1e8b4e8d 100644 --- a/tests/backups/test_manager.py +++ b/tests/backups/test_manager.py @@ -1679,15 +1679,17 @@ async def test_skip_homeassistant_database( coresys.homeassistant.backups_exclude_database = exclude_db_setting test_file = coresys.config.path_homeassistant / "configuration.yaml" - (test_db := coresys.config.path_homeassistant / "home-assistant_v2.db").touch() - ( - test_db_wal := coresys.config.path_homeassistant / "home-assistant_v2.db-wal" - ).touch() - ( - test_db_shm := coresys.config.path_homeassistant / "home-assistant_v2.db-shm" - ).touch() + test_db = coresys.config.path_homeassistant / "home-assistant_v2.db" + test_db_wal = coresys.config.path_homeassistant / "home-assistant_v2.db-wal" + test_db_shm = coresys.config.path_homeassistant / "home-assistant_v2.db-shm" - write_json_file(test_file, {"default_config": {}}) + def setup_1(): + test_db.touch() + test_db_wal.touch() + test_db_shm.touch() + write_json_file(test_file, {"default_config": {}}) + + await coresys.run_in_executor(setup_1) kwargs = {} if exclude_db_setting else {"homeassistant_exclude_database": True} if partial_backup: @@ -1697,9 +1699,12 @@ async def test_skip_homeassistant_database( else: backup: Backup = await coresys.backups.do_backup_full(**kwargs) - test_file.unlink() - write_json_file(test_db, {"hello": "world"}) - write_json_file(test_db_wal, {"hello": "world"}) + def setup_2(): + test_file.unlink() + write_json_file(test_db, {"hello": "world"}) + write_json_file(test_db_wal, {"hello": "world"}) + + await coresys.run_in_executor(setup_2) with ( patch.object(HomeAssistantCore, "update"), @@ -1707,10 +1712,13 @@ async def test_skip_homeassistant_database( ): await coresys.backups.do_restore_partial(backup, homeassistant=True) - assert read_json_file(test_file) == {"default_config": {}} - assert read_json_file(test_db) == {"hello": "world"} - assert read_json_file(test_db_wal) == {"hello": "world"} - assert not test_db_shm.exists() + def test_assertions(): + assert read_json_file(test_file) == {"default_config": {}} + assert read_json_file(test_db) == {"hello": "world"} + assert read_json_file(test_db_wal) == {"hello": "world"} + assert not test_db_shm.exists() + + await coresys.run_in_executor(test_assertions) @pytest.mark.usefixtures("tmp_supervisor_data", "path_extern") diff --git a/tests/conftest.py b/tests/conftest.py index 8eca71c06..8e87e4baa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -64,21 +64,16 @@ from .dbus_service_mocks.network_manager import NetworkManager as NetworkManager # pylint: disable=redefined-outer-name, protected-access -# This commented out code is left in intentionally -# Intent is to enable this for all tests at all times as an autouse fixture -# Findings from PR were growing too big so disabling temporarily to create a checkpoint -# @pytest.fixture(autouse=True) -def blockbuster(request: pytest.FixtureRequest) -> BlockBuster: +@pytest.fixture(autouse=True) +def blockbuster() -> BlockBuster: """Raise for blocking I/O in event loop.""" - # Excluded modules doesn't seem to stop test code from raising for blocking I/O - # Defaulting to only scanning supervisor core code seems like the best we can do easily - # Added a parameter so we could potentially go module by module in test and eliminate blocking I/O - # Then we could tell it to scan everything by default. That will require more follow-up work + # Only scanning supervisor code for now as that's our primary interest + # This will still raise for tests that call utilities in supervisor code that block + # But it will ignore calls to libraries and such that do blocking I/O directly from tests + # Removing that would be nice but a todo for the future # pylint: disable-next=contextmanager-generator-missing-cleanup - with blockbuster_ctx( - scanned_modules=getattr(request, "param", ["supervisor"]) - ) as bb: + with blockbuster_ctx(scanned_modules=["supervisor"]) as bb: yield bb @@ -118,7 +113,7 @@ async def docker() -> DockerAPI: ), patch("supervisor.docker.manager.DockerAPI.unload"), ): - docker_obj = DockerAPI(MagicMock()) + docker_obj = await DockerAPI(MagicMock()).post_init() docker_obj.config._data = {"registries": {}} with patch("supervisor.docker.monitor.DockerMonitor.load"): await docker_obj.load() diff --git a/tests/dbus/test_interface.py b/tests/dbus/test_interface.py index a51520017..63228da6f 100644 --- a/tests/dbus/test_interface.py +++ b/tests/dbus/test_interface.py @@ -1,5 +1,6 @@ """Test dbus interface.""" +import asyncio from unittest.mock import patch from dbus_fast.aio.message_bus import MessageBus @@ -145,9 +146,11 @@ async def test_proxy_missing_properties_interface(dbus_session_bus: MessageBus): proxy.object_path = DBUS_OBJECT_BASE proxy.properties_interface = "test.no.properties.interface" - async def mock_introspect(*args, **kwargs): + def mock_introspect(*args, **kwargs): """Return introspection without properties.""" - return load_fixture("test_no_properties_interface.xml") + return asyncio.get_running_loop().run_in_executor( + None, load_fixture, "test_no_properties_interface.xml" + ) with ( patch.object(MessageBus, "introspect", new=mock_introspect), diff --git a/tests/resolution/check/test_check.py b/tests/resolution/check/test_check.py index 47fb9253d..4c8e5d7c3 100644 --- a/tests/resolution/check/test_check.py +++ b/tests/resolution/check/test_check.py @@ -113,6 +113,10 @@ async def test_get_checks(coresys: CoreSys): async def test_dynamic_check_loader(coresys: CoreSys): """Test dynamic check loader, this ensures that all checks have defined a setup function.""" - coresys.resolution.check.load_modules() - for check in await coresys.run_in_executor(get_valid_modules, "checks"): + + def load_modules(): + coresys.resolution.check.load_modules() + return get_valid_modules("checks") + + for check in await coresys.run_in_executor(load_modules): assert check in coresys.resolution.check._checks diff --git a/tests/store/test_custom_repository.py b/tests/store/test_custom_repository.py index d8f92c67b..c4247966a 100644 --- a/tests/store/test_custom_repository.py +++ b/tests/store/test_custom_repository.py @@ -58,7 +58,9 @@ async def test_add_invalid_repository(coresys: CoreSys, store_manager: StoreMana current + ["http://example.com"], add_with_errors=True ) - assert not store_manager.get_from_url("http://example.com").validate() + assert not await coresys.run_in_executor( + store_manager.get_from_url("http://example.com").validate + ) assert "http://example.com" in coresys.store.repository_urls assert coresys.resolution.suggestions[-1].type == SuggestionType.EXECUTE_REMOVE @@ -176,11 +178,15 @@ async def test_preinstall_valid_repository( """Test add core repository valid.""" with patch("supervisor.store.repository.Repository.load", return_value=None): await store_manager.update_repositories(BUILTIN_REPOSITORIES) - assert store_manager.get("core").validate() - assert store_manager.get("local").validate() - assert store_manager.get("a0d7b954").validate() - assert store_manager.get("5c53de3b").validate() - assert store_manager.get("d5369777").validate() + + def validate(): + assert store_manager.get("core").validate() + assert store_manager.get("local").validate() + assert store_manager.get("a0d7b954").validate() + assert store_manager.get("5c53de3b").validate() + assert store_manager.get("d5369777").validate() + + await coresys.run_in_executor(validate) @pytest.mark.parametrize("use_update", [True, False]) diff --git a/tests/store/test_store_manager.py b/tests/store/test_store_manager.py index b479b3172..43a6609ec 100644 --- a/tests/store/test_store_manager.py +++ b/tests/store/test_store_manager.py @@ -146,7 +146,9 @@ async def test_update_unavailable_addon( ): """Test updating addon when new version not available for system.""" addon_config = dict( - load_yaml_fixture("addons/local/ssh/config.yaml"), + await coresys.run_in_executor( + load_yaml_fixture, "addons/local/ssh/config.yaml" + ), version=AwesomeVersion("10.0.0"), **config, ) @@ -201,7 +203,9 @@ async def test_install_unavailable_addon( ): """Test updating addon when new version not available for system.""" addon_config = dict( - load_yaml_fixture("addons/local/ssh/config.yaml"), + await coresys.run_in_executor( + load_yaml_fixture, "addons/local/ssh/config.yaml" + ), version=AwesomeVersion("10.0.0"), **config, ) diff --git a/tests/test_ingress.py b/tests/test_ingress.py index 4503e4500..b5b1598df 100644 --- a/tests/test_ingress.py +++ b/tests/test_ingress.py @@ -80,9 +80,11 @@ async def test_ingress_save_data(coresys: CoreSys, tmp_supervisor_data: Path): ) await ingress.save_data() - assert config_file.exists() - data = read_json_file(config_file) - assert data == { + def get_config(): + assert config_file.exists() + return read_json_file(config_file) + + assert await coresys.run_in_executor(get_config) == { "session": {session: ANY}, "session_data": { session: {"user": {"id": "123", "displayname": "Test", "username": "test"}} diff --git a/tests/utils/test_apparmor.py b/tests/utils/test_apparmor.py index fd8048221..ceb281f06 100644 --- a/tests/utils/test_apparmor.py +++ b/tests/utils/test_apparmor.py @@ -55,7 +55,7 @@ async def test_apparmor_multiple_profiles(caplog: pytest.LogCaptureFixture): ) -async def test_apparmor_profile_adjust(tmp_path: Path): +def test_apparmor_profile_adjust(tmp_path: Path): """Test apparmor profile adjust.""" profile_out = tmp_path / "apparmor_out.txt" adjust_profile("test", get_fixture_path("apparmor_valid.txt"), profile_out) @@ -63,7 +63,7 @@ async def test_apparmor_profile_adjust(tmp_path: Path): assert profile_out.read_text(encoding="utf-8") == TEST_PROFILE -async def test_apparmor_profile_adjust_mediate(tmp_path: Path): +def test_apparmor_profile_adjust_mediate(tmp_path: Path): """Test apparmor profile adjust when name matches a flag.""" profile_out = tmp_path / "apparmor_out.txt" adjust_profile("test", get_fixture_path("apparmor_valid_mediate.txt"), profile_out) diff --git a/tests/utils/test_dbus.py b/tests/utils/test_dbus.py index e7c761d48..3fb2445d0 100644 --- a/tests/utils/test_dbus.py +++ b/tests/utils/test_dbus.py @@ -1,5 +1,6 @@ """Test dbus utility.""" +import asyncio from unittest.mock import AsyncMock, Mock, patch from dbus_fast import ErrorType @@ -48,9 +49,11 @@ async def fixture_test_service(dbus_session_bus: MessageBus) -> TestInterface: async def test_missing_properties_interface(dbus_session_bus: MessageBus): """Test introspection missing properties interface.""" - async def mock_introspect(*args, **kwargs): + def mock_introspect(*args, **kwargs): """Return introspection without properties.""" - return load_fixture("test_no_properties_interface.xml") + return asyncio.get_running_loop().run_in_executor( + None, load_fixture, "test_no_properties_interface.xml" + ) with patch.object(MessageBus, "introspect", new=mock_introspect): service = await DBus.connect( diff --git a/tests/utils/test_sentry.py b/tests/utils/test_sentry.py index ff7982af9..d8fd5598c 100644 --- a/tests/utils/test_sentry.py +++ b/tests/utils/test_sentry.py @@ -2,10 +2,13 @@ from unittest.mock import patch +import pytest + from supervisor.bootstrap import initialize_coresys -async def test_sentry_disabled_by_default(supervisor_name): +@pytest.mark.usefixtures("supervisor_name", "docker") +async def test_sentry_disabled_by_default(): """Test diagnostics off by default.""" with ( patch("supervisor.bootstrap.initialize_system"),