From 8d5b7fa3ee59171ebb151c7b2c27cccdbd90e542 Mon Sep 17 00:00:00 2001 From: Nick Farrell Date: Tue, 30 Apr 2024 11:31:48 +1000 Subject: [PATCH 1/4] Use different dev dependencies for different fedora versions It is not sufficient to use one set of dev dependencies. In particular, pytest-asyncio has introduced a breaking change in f39. --- .github/workflows/test.yml | 2 +- pyproject.toml | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 16c7992e..208f3e2b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: cache: "pip" - name: Install requirements - run: pip install -e '.[cassandra,dev,f${{ matrix.fedora-version }}]' + run: pip install -e '.[cassandra,f${{ matrix.fedora-version }}-dev,f${{ matrix.fedora-version }}]' - name: Execute lints and tests run: make tests diff --git a/pyproject.toml b/pyproject.toml index e0339243..3f137525 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,7 +90,7 @@ f39 = [ "wcmatch == 8.4.1", "zstandard == 0.21.0", ] -dev = [ +f38-dev = [ # Needed by pre-commit to lint and test the project "pre-commit>=3.7.0", "anyio==3.5.0", @@ -116,6 +116,28 @@ dev = [ "respx==0.20.1", ] +f39-dev = [ + # Needed by pre-commit to lint and test the project + "pre-commit==3.7.0", + "anyio==3.7.0", + "pylint==3.0.4", + "pytest-asyncio==0.23.5", + "pytest-cov==4.0.0", + "pytest-mock==3.11.1", + "pytest-order", + "pytest-timeout==2.1.0", + "pytest-watch==4.2.0", + "pytest==7.3.2", + "mypy==1.8.0", + # Types for things that don't seem to have them + "types-botocore>=1.0.2", + "types-PyYAML>=6.0.12.2", + "types-requests>=2.28.11.5", + "types-tabulate>=0.9.0.0", + "types-ujson>=5.9.0.0", + "types-urllib3>=1.26.25.4", +] + [project.urls] "Homepage" = "https://github.com/Aiven-Open/astacus" "Bug Tracker" = "https://github.com/Aiven-Open/astacus/issues" From a1b315908cd8fe6af691bf185354b99fe206aa2e Mon Sep 17 00:00:00 2001 From: Nick Farrell Date: Tue, 30 Apr 2024 11:38:30 +1000 Subject: [PATCH 2/4] Fix regression causing tests to not be run in GHA --- .github/workflows/test.yml | 2 +- pyproject.toml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 208f3e2b..1cadb172 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -35,7 +35,7 @@ jobs: run: pip install -e '.[cassandra,f${{ matrix.fedora-version }}-dev,f${{ matrix.fedora-version }}]' - name: Execute lints and tests - run: make tests + run: make test - id: upload-codecov # Third-party action pinned to v2.1.0 diff --git a/pyproject.toml b/pyproject.toml index 3f137525..a047a8a5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -136,6 +136,9 @@ f39-dev = [ "types-tabulate>=0.9.0.0", "types-ujson>=5.9.0.0", "types-urllib3>=1.26.25.4", + "coverage==7.2.7", + "freezegun==1.2.2", + "respx==0.20.2", ] [project.urls] From ce330ea14c1bd2e6a66599571d8561ad0c531ab3 Mon Sep 17 00:00:00 2001 From: Nick Farrell Date: Tue, 30 Apr 2024 11:58:39 +1000 Subject: [PATCH 3/4] improve linting Use a later version of `flake8`, and resolve a deprecation warning. --- .pre-commit-config.yaml | 2 +- astacus/coordinator/plugins/clickhouse/dependencies.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1f7ebcd0..89618528 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,7 +21,7 @@ repos: - id: black - repo: https://github.com/pycqa/flake8 - rev: 6.0.0 + rev: 7.0.0 hooks: - id: flake8 files: \.py$ diff --git a/astacus/coordinator/plugins/clickhouse/dependencies.py b/astacus/coordinator/plugins/clickhouse/dependencies.py index 209c02bd..09624ac9 100644 --- a/astacus/coordinator/plugins/clickhouse/dependencies.py +++ b/astacus/coordinator/plugins/clickhouse/dependencies.py @@ -3,8 +3,8 @@ See LICENSE for details """ from astacus.coordinator.plugins.clickhouse.manifest import AccessEntity, Table -from collections.abc import Sequence -from typing import Callable, Hashable, TypeVar +from collections.abc import Hashable, Sequence +from typing import Callable, TypeVar # noinspection PyCompatibility import graphlib From 1648340b3992f6fe6a8695531fb2feb6e9ea9779 Mon Sep 17 00:00:00 2001 From: Nick Farrell Date: Tue, 30 Apr 2024 12:17:57 +1000 Subject: [PATCH 4/4] 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,