From 1648340b3992f6fe6a8695531fb2feb6e9ea9779 Mon Sep 17 00:00:00 2001 From: Nick Farrell Date: Tue, 30 Apr 2024 12:17:57 +1000 Subject: [PATCH] descope fixtures from module to function Due to changes in pytest-asyncio 0.23, the event loop used by fixtures/tests with different scopes is not the same. For example, a fixture declared with "module" scope will use a different event loop to tests, which run with the "function" scope's event loop. This causes many tests to fail. Despite being less efficient, the prudent way forward for now is to modify the fixtures to be recreated for each test. Astacus' tests are very fast, so this does not slow down CI runs very much at all. --- tests/integration/conftest.py | 14 ++------------ .../coordinator/plugins/clickhouse/conftest.py | 10 +++++----- .../coordinator/plugins/clickhouse/test_plugin.py | 4 ++-- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 4d7298c2..7067fce2 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -4,7 +4,7 @@ """ from astacus.common.utils import build_netloc from astacus.coordinator.plugins.zookeeper import KazooZooKeeperClient -from collections.abc import AsyncIterator, Iterator, Mapping, Sequence +from collections.abc import AsyncIterator, Mapping, Sequence from pathlib import Path from types import MappingProxyType @@ -21,16 +21,6 @@ logger = logging.getLogger(__name__) -@pytest.fixture(scope="module", name="event_loop") -def fixture_event_loop() -> Iterator[asyncio.AbstractEventLoop]: - # This is the same as the original `event_loop` fixture from `pytest_asyncio` - # but with a module scope, re-declaring this fixture is their suggested way - # of locally increasing the scope of this fixture. - loop = asyncio.get_event_loop_policy().new_event_loop() - yield loop - loop.close() - - async def get_command_path(name: str) -> Path | None: process = await asyncio.create_subprocess_exec( "which", name, stderr=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE @@ -147,7 +137,7 @@ def fixture_ports() -> Ports: return Ports() -@pytest.fixture(scope="module", name="zookeeper") +@pytest.fixture(name="zookeeper") async def fixture_zookeeper(ports: Ports) -> AsyncIterator[Service]: async with create_zookeeper(ports) as zookeeper: yield zookeeper diff --git a/tests/integration/coordinator/plugins/clickhouse/conftest.py b/tests/integration/coordinator/plugins/clickhouse/conftest.py index 80ec5112..d70ca999 100644 --- a/tests/integration/coordinator/plugins/clickhouse/conftest.py +++ b/tests/integration/coordinator/plugins/clickhouse/conftest.py @@ -79,7 +79,7 @@ class ClickHouseServiceCluster(ServiceCluster): ClickHouseCommand = Sequence[str | Path] -@pytest.fixture(scope="module", name="clickhouse_command") +@pytest.fixture(scope="function", name="clickhouse_command") async def fixture_clickhouse_command(request: FixtureRequest) -> ClickHouseCommand: clickhouse_path = request.config.getoption(CLICKHOUSE_PATH_OPTION) if clickhouse_path is None: @@ -91,7 +91,7 @@ async def fixture_clickhouse_command(request: FixtureRequest) -> ClickHouseComma return get_clickhouse_command(clickhouse_path) -@pytest.fixture(scope="module", name="clickhouse_restore_command") +@pytest.fixture(scope="function", name="clickhouse_restore_command") def fixture_clickhouse_restore_command(request: FixtureRequest, clickhouse_command: ClickHouseCommand) -> ClickHouseCommand: clickhouse_restore_path = request.config.getoption(CLICKHOUSE_RESTORE_PATH_OPTION) if clickhouse_restore_path is None: @@ -103,7 +103,7 @@ def get_clickhouse_command(clickhouse_path: Path) -> ClickHouseCommand: return [clickhouse_path] if clickhouse_path.name.endswith("-server") else [clickhouse_path, "server"] -@pytest.fixture(scope="module", name="clickhouse") +@pytest.fixture(scope="function", name="clickhouse") async def fixture_clickhouse(ports: Ports, clickhouse_command: ClickHouseCommand) -> AsyncIterator[Service]: async with create_clickhouse_service(ports, clickhouse_command) as service: yield service @@ -192,13 +192,13 @@ def bucket(self, *, bucket_name: str) -> Iterator[MinioBucket]: s3_client.delete_bucket(Bucket=bucket_name) # type: ignore[attr-defined] -@pytest.fixture(scope="module", name="minio") +@pytest.fixture(scope="function", name="minio") async def fixture_minio(ports: Ports) -> AsyncIterator[MinioService]: async with create_minio_service(ports) as service: yield service -@pytest.fixture(scope="module", name="minio_bucket") +@pytest.fixture(scope="function", name="minio_bucket") async def fixture_minio_bucket(minio: MinioService) -> AsyncIterator[MinioBucket]: with minio.bucket(bucket_name="clickhouse-bucket") as bucket: yield bucket diff --git a/tests/integration/coordinator/plugins/clickhouse/test_plugin.py b/tests/integration/coordinator/plugins/clickhouse/test_plugin.py index 53cc05f9..e3876b8b 100644 --- a/tests/integration/coordinator/plugins/clickhouse/test_plugin.py +++ b/tests/integration/coordinator/plugins/clickhouse/test_plugin.py @@ -93,7 +93,7 @@ async def restorable_cluster_manager( yield storage_path -@pytest.fixture(scope="module", name="restorable_cluster") +@pytest.fixture(scope="function", name="restorable_cluster") async def fixture_restorable_cluster( ports: Ports, clickhouse_command: ClickHouseCommand, @@ -148,7 +148,7 @@ async def restored_cluster_manager( yield clients -@pytest.fixture(scope="module", name="restored_cluster", params=[*get_restore_steps_names(), None]) +@pytest.fixture(scope="function", name="restored_cluster", params=[*get_restore_steps_names(), None]) async def fixture_restored_cluster( ports: Ports, request: SubRequest,