From 1df3adaba1889ac95f12b8cce702ff37fc387e14 Mon Sep 17 00:00:00 2001 From: Rishat-F <66554797+Rishat-F@users.noreply.github.com> Date: Mon, 17 Jun 2024 00:55:59 +0300 Subject: [PATCH] Fail redis and mongo tests if incorrect URI provided + some storages tests refactoring (#1510) * Smaller timeout for MongoStorage connection By default serverSelectionTimeoutMS=30000. This is too much * Correct ConnectionError for RedisStorage in tests * Remove unused import in conftest.py * Refactor skipping redis and mongo tests * Fail redis and mongo tests if incorrect URI If incorrect URIs provided to "--redis" and/or "--mongo" options tests should fail with ERRORs instead of skipping. Otherwise the next scenario is possible: 1) developer breaks RedisStorage and/or MongoStorage code 2) tests are run with incorrect redis and/or mongo URIs provided by "--redis" and "--mongo" options. For example, wrong port specified. 3) tests pass because skipping doesn't fail tests run 4) developer or reviewer doesn't notice that redis and/or mongo tests were skipped 5) broken code gets in codebase * Remove unused fixtures passing in storages tests * Define create_storage_key fixture in conftest.py * Linters formatting * Changes description * Revert "Smaller timeout for MongoStorage connection" This reverts commit d88b7ec61264385a8f8ee42d5b20267156ebb9c5. * Smaller timeout for MongoStorage connection in tests The default 30s timeout is too long * Add test for MongoStorage for 100% coverage * Linters formatting * Move skipping redis/mongo tests in earlier fixtures * Replace vars with constants in conftest.py * Linters formatting --- CHANGES/1510.bugfix.rst | 11 +++++ aiogram/filters/command.py | 2 +- examples/multibot.py | 2 +- tests/conftest.py | 61 ++++++++++++++---------- tests/test_fsm/storage/test_isolation.py | 6 --- tests/test_fsm/storage/test_mongo.py | 14 +++--- tests/test_fsm/storage/test_storages.py | 14 ++---- 7 files changed, 60 insertions(+), 50 deletions(-) create mode 100644 CHANGES/1510.bugfix.rst diff --git a/CHANGES/1510.bugfix.rst b/CHANGES/1510.bugfix.rst new file mode 100644 index 00000000..2b712404 --- /dev/null +++ b/CHANGES/1510.bugfix.rst @@ -0,0 +1,11 @@ +Fail redis and mongo tests if incorrect URI provided + some storages tests refactoring + +If incorrect URIs provided to "--redis" and/or "--mongo" options tests should fail with errors instead of skipping. +Otherwise the next scenario is possible: + 1) developer breaks RedisStorage and/or MongoStorage code + 2) tests are run with incorrect redis and/or mongo URIsprovided by "--redis" and "--mongo" options (for example, wrong port specified) + 3) tests pass because skipping doesn't fail tests run + 4) developer or reviewer doesn't notice that redis and/or mongo tests were skipped + 5) broken code gets in codebase + +Also some refactorings done (related with storages and storages tests). diff --git a/aiogram/filters/command.py b/aiogram/filters/command.py index b4cc5541..f52ac263 100644 --- a/aiogram/filters/command.py +++ b/aiogram/filters/command.py @@ -3,6 +3,7 @@ from __future__ import annotations import re from dataclasses import dataclass, field, replace from typing import ( + TYPE_CHECKING, Any, Dict, Iterable, @@ -10,7 +11,6 @@ from typing import ( Optional, Pattern, Sequence, - TYPE_CHECKING, Union, cast, ) diff --git a/examples/multibot.py b/examples/multibot.py index 82fac4a1..d5dbe5fc 100644 --- a/examples/multibot.py +++ b/examples/multibot.py @@ -4,6 +4,7 @@ from os import getenv from typing import Any, Dict, Union from aiohttp import web +from finite_state_machine import form_router from aiogram import Bot, Dispatcher, F, Router from aiogram.client.session.aiohttp import AiohttpSession @@ -18,7 +19,6 @@ from aiogram.webhook.aiohttp_server import ( TokenBasedRequestHandler, setup_application, ) -from finite_state_machine import form_router main_router = Router() diff --git a/tests/conftest.py b/tests/conftest.py index f13fec10..497dd50f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,19 +5,27 @@ from _pytest.config import UsageError from pymongo.errors import InvalidURI, PyMongoError from pymongo.uri_parser import parse_uri as parse_mongo_url from redis.asyncio.connection import parse_url as parse_redis_url +from redis.exceptions import ConnectionError from aiogram import Dispatcher +from aiogram.fsm.storage.base import StorageKey from aiogram.fsm.storage.memory import ( DisabledEventIsolation, MemoryStorage, SimpleEventIsolation, ) from aiogram.fsm.storage.mongo import MongoStorage -from aiogram.fsm.storage.redis import RedisEventIsolation, RedisStorage +from aiogram.fsm.storage.redis import RedisStorage from tests.mocked_bot import MockedBot DATA_DIR = Path(__file__).parent / "data" +CHAT_ID = -42 +USER_ID = 42 + +SKIP_MESSAGE_PATTERN = 'Need "--{db}" option with {db} URI to run' +INVALID_URI_PATTERN = "Invalid {db} URI {uri!r}: {err}" + def pytest_addoption(parser): parser.addoption("--redis", default=None, help="run tests which require redis connection") @@ -29,37 +37,27 @@ def pytest_configure(config): config.addinivalue_line("markers", "mongo: marked tests require mongo connection to run") -def pytest_collection_modifyitems(config, items): - for db, parse_uri in [("redis", parse_redis_url), ("mongo", parse_mongo_url)]: - uri = config.getoption(f"--{db}") - if uri is None: - skip = pytest.mark.skip(reason=f"need --{db} option with {db} URI to run") - for item in items: - if db in item.keywords: - item.add_marker(skip) - else: - try: - parse_uri(uri) - except (ValueError, InvalidURI) as e: - raise UsageError(f"Invalid {db} URI {uri!r}: {e}") - - @pytest.fixture() def redis_server(request): redis_uri = request.config.getoption("--redis") - return redis_uri + if redis_uri is None: + pytest.skip(SKIP_MESSAGE_PATTERN.format(db="redis")) + else: + return redis_uri @pytest.fixture() @pytest.mark.redis async def redis_storage(redis_server): - if not redis_server: - pytest.skip("Redis is not available here") + try: + parse_redis_url(redis_server) + except ValueError as e: + raise UsageError(INVALID_URI_PATTERN.format(db="redis", uri=redis_server, err=e)) storage = RedisStorage.from_url(redis_server) try: await storage.redis.info() except ConnectionError as e: - pytest.skip(str(e)) + pytest.fail(str(e)) try: yield storage finally: @@ -71,19 +69,27 @@ async def redis_storage(redis_server): @pytest.fixture() def mongo_server(request): mongo_uri = request.config.getoption("--mongo") - return mongo_uri + if mongo_uri is None: + pytest.skip(SKIP_MESSAGE_PATTERN.format(db="mongo")) + else: + return mongo_uri @pytest.fixture() @pytest.mark.mongo async def mongo_storage(mongo_server): - if not mongo_server: - pytest.skip("MongoDB is not available here") - storage = MongoStorage.from_url(mongo_server) + try: + parse_mongo_url(mongo_server) + except InvalidURI as e: + raise UsageError(INVALID_URI_PATTERN.format(db="mongo", uri=mongo_server, err=e)) + storage = MongoStorage.from_url( + url=mongo_server, + connection_kwargs={"serverSelectionTimeoutMS": 2000}, + ) try: await storage._client.server_info() except PyMongoError as e: - pytest.skip(str(e)) + pytest.fail(str(e)) else: yield storage await storage._client.drop_database(storage._database) @@ -130,6 +136,11 @@ def bot(): return MockedBot() +@pytest.fixture(name="storage_key") +def create_storage_key(bot: MockedBot): + return StorageKey(chat_id=CHAT_ID, user_id=USER_ID, bot_id=bot.id) + + @pytest.fixture() async def dispatcher(): dp = Dispatcher() diff --git a/tests/test_fsm/storage/test_isolation.py b/tests/test_fsm/storage/test_isolation.py index c95a1cba..867362c1 100644 --- a/tests/test_fsm/storage/test_isolation.py +++ b/tests/test_fsm/storage/test_isolation.py @@ -5,12 +5,6 @@ import pytest from aiogram.fsm.storage.base import BaseEventIsolation, StorageKey from aiogram.fsm.storage.redis import RedisEventIsolation, RedisStorage -from tests.mocked_bot import MockedBot - - -@pytest.fixture(name="storage_key") -def create_storage_key(bot: MockedBot): - return StorageKey(chat_id=-42, user_id=42, bot_id=bot.id) @pytest.mark.parametrize( diff --git a/tests/test_fsm/storage/test_mongo.py b/tests/test_fsm/storage/test_mongo.py index 48b95719..95a476d6 100644 --- a/tests/test_fsm/storage/test_mongo.py +++ b/tests/test_fsm/storage/test_mongo.py @@ -1,17 +1,19 @@ import pytest +from pymongo.errors import PyMongoError from aiogram.fsm.state import State from aiogram.fsm.storage.mongo import MongoStorage, StorageKey -from tests.mocked_bot import MockedBot +from tests.conftest import CHAT_ID, USER_ID PREFIX = "fsm" -CHAT_ID = -42 -USER_ID = 42 -@pytest.fixture(name="storage_key") -def create_storage_key(bot: MockedBot): - return StorageKey(chat_id=CHAT_ID, user_id=USER_ID, bot_id=bot.id) +async def test_get_storage_passing_only_url(mongo_server): + storage = MongoStorage.from_url(url=mongo_server) + try: + await storage._client.server_info() + except PyMongoError as e: + pytest.fail(str(e)) async def test_update_not_existing_data_with_empty_dictionary( diff --git a/tests/test_fsm/storage/test_storages.py b/tests/test_fsm/storage/test_storages.py index 1c1f87a2..690bc791 100644 --- a/tests/test_fsm/storage/test_storages.py +++ b/tests/test_fsm/storage/test_storages.py @@ -1,12 +1,6 @@ import pytest from aiogram.fsm.storage.base import BaseStorage, StorageKey -from tests.mocked_bot import MockedBot - - -@pytest.fixture(name="storage_key") -def create_storage_key(bot: MockedBot): - return StorageKey(chat_id=-42, user_id=42, bot_id=bot.id) @pytest.mark.parametrize( @@ -18,7 +12,7 @@ def create_storage_key(bot: MockedBot): ], ) class TestStorages: - async def test_set_state(self, bot: MockedBot, storage: BaseStorage, storage_key: StorageKey): + async def test_set_state(self, storage: BaseStorage, storage_key: StorageKey): assert await storage.get_state(key=storage_key) is None await storage.set_state(key=storage_key, state="state") @@ -26,7 +20,7 @@ class TestStorages: await storage.set_state(key=storage_key, state=None) assert await storage.get_state(key=storage_key) is None - async def test_set_data(self, bot: MockedBot, storage: BaseStorage, storage_key: StorageKey): + async def test_set_data(self, storage: BaseStorage, storage_key: StorageKey): assert await storage.get_data(key=storage_key) == {} await storage.set_data(key=storage_key, data={"foo": "bar"}) @@ -34,9 +28,7 @@ class TestStorages: await storage.set_data(key=storage_key, data={}) assert await storage.get_data(key=storage_key) == {} - async def test_update_data( - self, bot: MockedBot, storage: BaseStorage, storage_key: StorageKey - ): + async def test_update_data(self, storage: BaseStorage, storage_key: StorageKey): assert await storage.get_data(key=storage_key) == {} assert await storage.update_data(key=storage_key, data={"foo": "bar"}) == {"foo": "bar"} assert await storage.update_data(key=storage_key, data={}) == {"foo": "bar"}