Handle expected warnings & raise unexpected warnings (#1315)

* chore: replace fixture loop with event_loop

* chore: mark expected warnings

* chore: raise unexpected warnings

* chore: rm unused record

* fix: rm parenthesized context manager

* chore: warnings shall not pass

* chore: replace fixture loop with event_loop

* chore: mark expected warnings

* chore: raise unexpected warnings

* chore: rm unused record

* fix: rm parenthesized context manager

* chore: warnings shall not pass

* Revert "chore: raise unexpected warnings"

This reverts commit 4c91df243d.

* chore: warnings shall not pass v2

* fix: graceful aiohttp session close

* chore: minor typo

* chore: mark expected warnings

* fix: temporary mute ResourceWarning

#1320

* fix: close pool with redis

* chore: code reformat and lint

* chore: simplify tests with fixture

* chore: make aresponses clear

* chore: divide asserts with blank line

* chore: rm duplicated assertions

* chore: rm unnecessary extra

* chore: bump test dependencies

* chore: bump test dependencies (fix)

.github/PULL_REQUEST_TEMPLATE.md

@@ -30,6 +30,5 @@ Please describe the tests that you ran to verify your changes. Provide instructi
 - [ ] My code follows the style guidelines of this project
 - [ ] I have performed a self-review of my own code
 - [ ] I have made corresponding changes to the documentation
-- [ ] My changes generate no new warnings
 - [ ] I have added tests that prove my fix is effective or that my feature works
 - [ ] New and existing unit tests pass locally with my changes

aiogram/client/session/aiohttp.py

@@ -139,6 +139,10 @@ class AiohttpSession(BaseSession):
         if self._session is not None and not self._session.closed:
             await self._session.close()
 
+            # Wait 250 ms for the underlying SSL connections to close
+            # https://docs.aiohttp.org/en/stable/client_advanced.html#graceful-shutdown
+            await asyncio.sleep(0.25)
+
     def build_form_data(self, bot: Bot, method: TelegramMethod[TelegramType]) -> FormData:
         form = FormData(quote_fields=False)
         files: Dict[str, InputFile] = {}

aiogram/fsm/storage/redis.py

@@ -138,7 +138,7 @@ class RedisStorage(BaseStorage):
         return RedisEventIsolation(redis=self.redis, key_builder=self.key_builder, **kwargs)
 
     async def close(self) -> None:
-        await self.redis.close()
+        await self.redis.close(close_connection_pool=True)
 
     async def set_state(
         self,

pyproject.toml

@@ -70,16 +70,16 @@ cli = [
 ]
 test = [
     "pytest~=7.4.0",
-    "pytest-html~=3.2.0",
+    "pytest-html~=4.0.0",
     "pytest-asyncio~=0.21.0",
     "pytest-lazy-fixture~=0.6.3",
-    "pytest-mock~=3.10.0",
+    "pytest-mock~=3.11.0",
     "pytest-mypy~=0.10.0",
-    "pytest-cov~=4.0.0",
+    "pytest-cov~=4.1.0",
     "pytest-aiohttp~=1.0.4",
     "aresponses~=2.1.6",
-    "pytz~=2022.7.1",
+    "pytz~=2023.3",
-    "pycryptodomex~=3.18",
+    "pycryptodomex~=3.19",
 ]
 docs = [
     "Sphinx~=7.1.1",
@@ -240,6 +240,10 @@ asyncio_mode = "auto"
 testpaths = [
     "tests",
 ]
+filterwarnings = [
+    "error",
+    "ignore::pytest.PytestUnraisableExceptionWarning",
+]
 
 [tool.coverage.run]
 branch = false

tests/test_api/test_client/test_bot.py

@@ -15,6 +15,26 @@ from aiogram.types import File, PhotoSize
 from tests.mocked_bot import MockedBot
 
 
+@pytest.fixture()
+async def bot():
+    """Override mocked bot fixture with real bot."""
+    async with Bot("42:TEST").context() as bot:
+        yield bot
+
+
+@pytest.fixture()
+def mocked_bot():
+    """Mocked bot fixture."""
+    return MockedBot()
+
+
+@pytest.fixture()
+async def session():
+    """Override session fixture."""
+    async with AiohttpSession() as session:
+        yield session
+
+
 class TestBot:
     def test_init(self):
         bot = Bot("42:TEST")
@@ -30,9 +50,7 @@ class TestBot:
         assert bot == Bot("42:TEST")
         assert bot != "42:TEST"
 
-    async def test_emit(self):
+    async def test_emit(self, bot: Bot):
-        bot = Bot("42:TEST")
-
         method = GetMe()
 
         with patch(
@@ -42,8 +60,7 @@ class TestBot:
             await bot(method)
             mocked_make_request.assert_awaited_with(bot, method, timeout=None)
 
-    async def test_close(self):
+    async def test_close(self, session: AiohttpSession):
-        session = AiohttpSession()
         bot = Bot("42:TEST", session=session)
         await session.create_session()
 
@@ -56,18 +73,23 @@ class TestBot:
     @pytest.mark.parametrize("close", [True, False])
     async def test_context_manager(self, close: bool):
         with patch(
-            "aiogram.client.session.aiohttp.AiohttpSession.close", new_callable=AsyncMock
+            target="aiogram.client.session.aiohttp.AiohttpSession.close",
+            new_callable=AsyncMock,
         ) as mocked_close:
-            async with Bot("42:TEST", session=AiohttpSession()).context(auto_close=close) as bot:
+            session = AiohttpSession()
+            async with Bot("42:TEST", session=session).context(auto_close=close) as bot:
                 assert isinstance(bot, Bot)
+
             if close:
                 mocked_close.assert_awaited()
             else:
                 mocked_close.assert_not_awaited()
+                await session.close()
 
     async def test_download_file(self, aresponses: ResponsesMockServer):
         aresponses.add(
-            aresponses.ANY, aresponses.ANY, "get", aresponses.Response(status=200, body=b"\f" * 10)
+            method_pattern="get",
+            response=aresponses.Response(status=200, body=b"\f" * 10),
         )
 
         # https://github.com/Tinche/aiofiles#writing-tests-for-aiofiles
@@ -77,30 +99,34 @@ class TestBot:
 
         mock_file = MagicMock()
 
-        bot = Bot("42:TEST")
+        async with Bot("42:TEST").context() as bot:
             with patch("aiofiles.threadpool.sync_open", return_value=mock_file):
                 await bot.download_file("TEST", "file.png")
                 mock_file.write.assert_called_once_with(b"\f" * 10)
 
-    async def test_download_file_default_destination(self, aresponses: ResponsesMockServer):
+    async def test_download_file_default_destination(
-        bot = Bot("42:TEST")
+        self,
-
+        bot: Bot,
+        aresponses: ResponsesMockServer,
+    ):
         aresponses.add(
-            aresponses.ANY, aresponses.ANY, "get", aresponses.Response(status=200, body=b"\f" * 10)
+            method_pattern="get",
+            response=aresponses.Response(status=200, body=b"\f" * 10),
         )
-
         result = await bot.download_file("TEST")
 
         assert isinstance(result, io.BytesIO)
         assert result.read() == b"\f" * 10
 
-    async def test_download_file_custom_destination(self, aresponses: ResponsesMockServer):
+    async def test_download_file_custom_destination(
-        bot = Bot("42:TEST")
+        self,
-
+        bot: Bot,
+        aresponses: ResponsesMockServer,
+    ):
         aresponses.add(
-            aresponses.ANY, aresponses.ANY, "get", aresponses.Response(status=200, body=b"\f" * 10)
+            method_pattern="get",
+            response=aresponses.Response(status=200, body=b"\f" * 10),
         )
-
         custom = io.BytesIO()
 
         result = await bot.download_file("TEST", custom)
@@ -109,19 +135,19 @@ class TestBot:
         assert result is custom
         assert result.read() == b"\f" * 10
 
-    async def test_download(self, bot: MockedBot, aresponses: ResponsesMockServer):
+    async def test_download(self, mocked_bot: MockedBot):
-        bot.add_result_for(
+        mocked_bot.add_result_for(
             GetFile, ok=True, result=File(file_id="file id", file_unique_id="file id")
         )
-        bot.add_result_for(
+        mocked_bot.add_result_for(
             GetFile, ok=True, result=File(file_id="file id", file_unique_id="file id")
         )
 
-        assert await bot.download(File(file_id="file id", file_unique_id="file id"))
+        assert await mocked_bot.download(File(file_id="file id", file_unique_id="file id"))
-        assert await bot.download("file id")
+        assert await mocked_bot.download("file id")
 
         with pytest.raises(TypeError):
-            await bot.download(
+            await mocked_bot.download(
                 [PhotoSize(file_id="file id", file_unique_id="file id", width=123, height=123)]
             )
 

tests/test_api/test_client/test_session/test_aiohttp_session.py

@@ -24,17 +24,15 @@ class BareInputFile(InputFile):
 class TestAiohttpSession:
     async def test_create_session(self):
         session = AiohttpSession()
-
         assert session._session is None
         aiohttp_session = await session.create_session()
         assert session._session is not None
         assert isinstance(aiohttp_session, aiohttp.ClientSession)
+        await session.close()
 
     async def test_create_proxy_session(self):
-        session = AiohttpSession(
+        auth = aiohttp.BasicAuth("login", "password", "encoding")
-            proxy=("socks5://proxy.url/", aiohttp.BasicAuth("login", "password", "encoding"))
+        async with AiohttpSession(proxy=("socks5://proxy.url/", auth)) as session:
-        )
-
             assert session._connector_type == aiohttp_socks.ProxyConnector
 
             assert isinstance(session._connector_init, dict)
@@ -44,8 +42,7 @@ class TestAiohttpSession:
             assert isinstance(aiohttp_session.connector, aiohttp_socks.ProxyConnector)
 
     async def test_create_proxy_session_proxy_url(self):
-        session = AiohttpSession(proxy="socks4://proxy.url/")
+        async with AiohttpSession(proxy="socks4://proxy.url/") as session:
-
             assert isinstance(session.proxy, str)
 
             assert isinstance(session._connector_init, dict)
@@ -55,27 +52,22 @@ class TestAiohttpSession:
             assert isinstance(aiohttp_session.connector, aiohttp_socks.ProxyConnector)
 
     async def test_create_proxy_session_chained_proxies(self):
-        session = AiohttpSession(
+        proxy_chain = [
-            proxy=[
             "socks4://proxy.url/",
             "socks5://proxy.url/",
             "http://user:password@127.0.0.1:3128",
         ]
-        )
+        async with AiohttpSession(proxy=proxy_chain) as session:
-
             assert isinstance(session.proxy, list)
 
             assert isinstance(session._connector_init, dict)
-        assert isinstance(session._connector_init["proxy_infos"], list)
-        assert isinstance(session._connector_init["proxy_infos"][0], aiohttp_socks.ProxyInfo)
 
-        assert (
+            proxy_infos = session._connector_init["proxy_infos"]
-            session._connector_init["proxy_infos"][0].proxy_type is aiohttp_socks.ProxyType.SOCKS4
+            assert isinstance(proxy_infos, list)
-        )
+            assert isinstance(proxy_infos[0], aiohttp_socks.ProxyInfo)
-        assert (
+            assert proxy_infos[0].proxy_type is aiohttp_socks.ProxyType.SOCKS4
-            session._connector_init["proxy_infos"][1].proxy_type is aiohttp_socks.ProxyType.SOCKS5
+            assert proxy_infos[1].proxy_type is aiohttp_socks.ProxyType.SOCKS5
-        )
+            assert proxy_infos[2].proxy_type is aiohttp_socks.ProxyType.HTTP
-        assert session._connector_init["proxy_infos"][2].proxy_type is aiohttp_socks.ProxyType.HTTP
 
             aiohttp_session = await session.create_session()
             assert isinstance(aiohttp_session.connector, aiohttp_socks.ChainProxyConnector)
@@ -93,6 +85,7 @@ class TestAiohttpSession:
         assert session._should_reset_connector
         await session.create_session()
         assert session._should_reset_connector is False
+
         await session.close()
 
     async def test_close_session(self):
@@ -170,7 +163,7 @@ class TestAiohttpSession:
             ),
         )
 
-        session = AiohttpSession()
+        async with AiohttpSession() as session:
 
             class TestMethod(TelegramMethod[int]):
                 __returning__ = int
@@ -184,11 +177,10 @@ class TestAiohttpSession:
 
     @pytest.mark.parametrize("error", [ClientError("mocked"), asyncio.TimeoutError()])
     async def test_make_request_network_error(self, error):
-        bot = Bot("42:TEST")
-
         async def side_effect(*args, **kwargs):
             raise error
 
+        async with Bot("42:TEST").context() as bot:
             with patch(
                 "aiohttp.client.ClientSession._request",
                 new_callable=AsyncMock,
@@ -202,7 +194,7 @@ class TestAiohttpSession:
             aresponses.ANY, aresponses.ANY, "get", aresponses.Response(status=200, body=b"\f" * 10)
         )
 
-        session = AiohttpSession()
+        async with AiohttpSession() as session:
             stream = session.stream_content(
                 "https://www.python.org/static/img/python-logo.png",
                 timeout=5,
@@ -229,7 +221,7 @@ class TestAiohttpSession:
                 body=b"File not found",
             ),
         )
-        session = AiohttpSession()
+        async with AiohttpSession() as session:
             stream = session.stream_content(
                 "https://www.python.org/static/img/python-logo.png",
                 timeout=5,
@@ -242,14 +234,15 @@ class TestAiohttpSession:
                     ...
 
     async def test_context_manager(self):
-        session = AiohttpSession()
+        async with AiohttpSession() as session:
             assert isinstance(session, AsyncContextManager)
 
             with patch(
                 "aiogram.client.session.aiohttp.AiohttpSession.create_session",
                 new_callable=AsyncMock,
             ) as mocked_create_session, patch(
-            "aiogram.client.session.aiohttp.AiohttpSession.close", new_callable=AsyncMock
+                "aiogram.client.session.aiohttp.AiohttpSession.close",
+                new_callable=AsyncMock,
             ) as mocked_close:
                 async with session as ctx:
                     assert session == ctx

tests/test_api/test_types/test_input_file.py

@@ -68,9 +68,12 @@ class TestInputFile:
 
     async def test_url_input_file(self, aresponses: ResponsesMockServer):
         aresponses.add(
-            aresponses.ANY, aresponses.ANY, "get", aresponses.Response(status=200, body=b"\f" * 10)
+            aresponses.ANY,
+            aresponses.ANY,
+            "get",
+            aresponses.Response(status=200, body=b"\f" * 10),
         )
-        bot = Bot(token="42:TEST")
+        async with Bot(token="42:TEST").context() as bot:
             file = URLInputFile("https://test.org/", chunk_size=1)
 
             size = 0

tests/test_dispatcher/test_dispatcher.py

@@ -187,6 +187,7 @@ class TestDispatcher:
     async def test_process_update_empty(self, bot: MockedBot):
         dispatcher = Dispatcher()
 
+        with pytest.warns(RuntimeWarning, match="Detected unknown update type") as record:
             result = await dispatcher._process_update(bot=bot, update=Update(update_id=42))
             assert not result
 
@@ -197,6 +198,7 @@ class TestDispatcher:
         async def update_handler(update: Update):
             pass
 
+        with pytest.warns(RuntimeWarning, match="Detected unknown update type"):
             assert await dispatcher._process_update(bot=bot, update=Update(update_id=42))
 
     @pytest.mark.parametrize(
@@ -479,9 +481,11 @@ class TestDispatcher:
 
     async def test_listen_unknown_update(self):
         dp = Dispatcher()
-
+        pattern = "Detected unknown update type"
-        with pytest.raises(SkipHandler):
+        with pytest.raises(SkipHandler), pytest.warns(RuntimeWarning, match=pattern) as record:
             await dp._listen_update(Update(update_id=42))
+            if not record:
+                pytest.fail("Expected 'Detected unknown update type' warning.")
 
     async def test_listen_unhandled_update(self):
         dp = Dispatcher()
@@ -608,7 +612,9 @@ class TestDispatcher:
         async def update_handler(update: Update):
             raise Exception("Kaboom!")
 
+        with pytest.warns(RuntimeWarning, match="Detected unknown update type"):
             assert await dispatcher._process_update(bot=bot, update=Update(update_id=42))
+
         log_records = [rec.message for rec in caplog.records]
         assert len(log_records) == 1
         assert "Cause exception while process update" in log_records[0]
@@ -834,6 +840,8 @@ class TestDispatcher:
         dispatcher = Dispatcher()
         dispatcher.message.register(invalid_message_handler)
 
+        pattern = r"Detected slow response into webhook"
+        with pytest.warns(RuntimeWarning, match=pattern) as record:
             response = await dispatcher.feed_webhook_update(bot, RAW_UPDATE, _timeout=0.1)
             assert response is None
             await asyncio.sleep(0.5)
@@ -841,6 +849,9 @@ class TestDispatcher:
             log_records = [rec.message for rec in caplog.records]
             assert "Cause exception while process update" in log_records[0]
 
+            if not record:
+                pytest.fail("Expected 'Detected slow response into webhook' warning.")
+
     def test_specify_updates_calculation(self):
         def simple_msg_handler() -> None:
             ...

tests/test_filters/test_exception.py

@@ -72,4 +72,5 @@ class TestDispatchException:
         async def handler0(event):
             return "Handled"
 
+        with pytest.warns(RuntimeWarning, match="Detected unknown update type"):
             assert await dp.feed_update(bot, Update(update_id=0)) == "Handled"

tests/test_fsm/storage/test_isolation.py

@@ -18,10 +18,11 @@ def create_storage_key(bot: MockedBot):
     ],
 )
 class TestIsolations:
+    @pytest.mark.filterwarnings("ignore::ResourceWarning")
     async def test_lock(
         self,
         isolation: BaseEventIsolation,
         storage_key: StorageKey,
     ):
         async with isolation.lock(key=storage_key):
-            assert True, "You are kidding me?"
+            assert True, "Are you kidding me?"

tests/test_utils/test_chat_action.py

@@ -13,9 +13,9 @@ from tests.mocked_bot import MockedBot
 
 
 class TestChatActionSender:
-    async def test_wait(self, bot: Bot, loop: asyncio.BaseEventLoop):
+    async def test_wait(self, bot: Bot, event_loop: asyncio.BaseEventLoop):
         sender = ChatActionSender.typing(bot=bot, chat_id=42)
-        loop.call_soon(sender._close_event.set)
+        event_loop.call_soon(sender._close_event.set)
         start = time.monotonic()
         await sender._wait(1)
         assert time.monotonic() - start < 1

tests/test_utils/test_i18n.py

@@ -175,7 +175,7 @@ class TestConstI18nMiddleware:
 
 
 class TestFSMI18nMiddleware:
-    async def test_middleware(self, i18n: I18n, bot: MockedBot, extra):
+    async def test_middleware(self, i18n: I18n, bot: MockedBot):
         middleware = FSMI18nMiddleware(i18n=i18n)
         storage = MemoryStorage()
         state = FSMContext(storage=storage, key=StorageKey(user_id=42, chat_id=42, bot_id=bot.id))
@@ -185,12 +185,14 @@ class TestFSMI18nMiddleware:
         }
         result = await middleware(next_call, Update(update_id=42), data)
         assert result == "test"
+
         await middleware.set_locale(state, "uk")
         assert i18n.current_locale == "uk"
+
         result = await middleware(next_call, Update(update_id=42), data)
         assert result == "тест"
 
-    async def test_without_state(self, i18n: I18n, bot: MockedBot, extra):
+    async def test_without_state(self, i18n: I18n, bot: MockedBot):
         middleware = FSMI18nMiddleware(i18n=i18n)
         data = {
             "event_from_user": User(id=42, is_bot=False, language_code="it", first_name="Test"),
@@ -198,7 +200,3 @@ class TestFSMI18nMiddleware:
         result = await middleware(next_call, Update(update_id=42), data)
         assert i18n.current_locale == "en"
         assert result == "test"
-
-        assert i18n.current_locale == "en"
-        result = await middleware(next_call, Update(update_id=42), data)
-        assert i18n.current_locale == "en"