Fix Type Annotations & Tests for AiohttpRequest (#132)

* Fix Type Annotations in `AiohttpRequest.do_request`

* try fixing tests on 3.9+

* try harder

* try skipping aiohttp tests on python3.8

Author: Bibo-Joshi

ptbcontrib/aiohttp_request/aiohttprequest.py

@@ -150,12 +150,10 @@ async def do_request(  # pylint: disable=too-many-arguments,too-many-positional-
         url: str,
         method: str,
         request_data: Optional[RequestData] = None,
-        read_timeout: Optional[Union[BaseRequest.DEFAULT_NONE, float]] = BaseRequest.DEFAULT_NONE,
-        write_timeout: Optional[Union[BaseRequest.DEFAULT_NONE, float]] = BaseRequest.DEFAULT_NONE,
-        connect_timeout: Optional[
-            Union[BaseRequest.DEFAULT_NONE, float]
-        ] = BaseRequest.DEFAULT_NONE,
-        pool_timeout: Optional[Union[BaseRequest.DEFAULT_NONE, float]] = BaseRequest.DEFAULT_NONE,
+        read_timeout: Optional[float] = BaseRequest.DEFAULT_NONE,
+        write_timeout: Optional[float] = BaseRequest.DEFAULT_NONE,
+        connect_timeout: Optional[float] = BaseRequest.DEFAULT_NONE,
+        pool_timeout: Optional[float] = BaseRequest.DEFAULT_NONE,
     ) -> tuple[int, bytes]:
         """See :meth:`BaseRequest.do_request`.
 
@@ -215,7 +213,8 @@ async def do_request(  # pylint: disable=too-many-arguments,too-many-positional-
                 timeout=timeout,
                 data=data,
             )
-        except TimeoutError as err:
+        # asyncio.TimeoutError is an alias of TimeoutError only starting with Python 3.11.
+        except asyncio.TimeoutError as err:
             if isinstance(err, aiohttp.ConnectionTimeoutError):
                 raise TimedOut(
                     message=(

requirements-dev.txt

@@ -1,6 +1,6 @@
 # cryptography is an optional dependency, but running the tests properly requires it
 cryptography!=3.4,!=3.4.1,!=3.4.2,!=3.4.3
-pygit2==1.9.1
+pygit2~=1.9
 
 pre-commit
 

run_tests.py

@@ -75,16 +75,28 @@ def run_tests(changed: bool, names: List[str]) -> int:
     exit_code = 0
     for name in names:
         try:
-            subprocess.check_call(  # nosec
+            result = subprocess.run(  # pylint: disable=subprocess-run-check  # nosec
                 [
                     sys.executable,
                     "-m",
                     "pip",
                     "install",
                     "-r",
                     str(ptbcontrib_path / name / "requirements.txt"),
-                ]
+                ],
+                capture_output=True,
+                text=True,
             )
+            if (
+                result.stderr
+                and "No matching distribution found for python-telegram-bot" in result.stderr
+            ):
+                print(
+                    f"Ignoring contribution {name}, as this PTB version is not "
+                    f"supported on Python {sys.version}. "
+                )
+                continue
+            result.check_returncode()
 
             result = subprocess.run(  # nosec
                 [sys.executable, "-m", "telegram"],

tests/test_aiohttp_request.py

@@ -25,15 +25,12 @@
 from collections.abc import Coroutine
 from http import HTTPStatus
 from typing import Any, Callable, Optional
+from unittest.mock import MagicMock
 
 import aiohttp
 import multidict
 import pytest
 import yarl
-from telegram import InputFile
-from telegram._utils.defaultvalue import DEFAULT_NONE
-from telegram._utils.strings import TextEncoding
-from telegram._utils.types import ODVInput
 from telegram.error import (
     BadRequest,
     ChatMigrated,
@@ -45,8 +42,7 @@
     TelegramError,
     TimedOut,
 )
-from telegram.request import RequestData
-from telegram.request._requestparameter import RequestParameter
+from telegram.request import BaseRequest, RequestData
 
 from ptbcontrib.aiohttp_request import AiohttpRequest
 
@@ -70,10 +66,10 @@ async def _request_wrapper(
         method: str,
         url: str,
         request_data: Optional[RequestData] = None,
-        read_timeout: ODVInput[float] = DEFAULT_NONE,
-        connect_timeout: ODVInput[float] = DEFAULT_NONE,
-        write_timeout: ODVInput[float] = DEFAULT_NONE,
-        pool_timeout: ODVInput[float] = DEFAULT_NONE,
+        read_timeout: Optional[float] = BaseRequest.DEFAULT_NONE,
+        connect_timeout: Optional[float] = BaseRequest.DEFAULT_NONE,
+        write_timeout: Optional[float] = BaseRequest.DEFAULT_NONE,
+        pool_timeout: Optional[float] = BaseRequest.DEFAULT_NONE,
     ) -> bytes:
         try:
             return await super()._request_wrapper(
@@ -203,10 +199,15 @@ async def test_illegal_json_response(
 
         await aiohttp_request.shutdown()
 
-        assert len(caplog.records) == 1
-        record = caplog.records[0]
-        assert record.name == "telegram.request.BaseRequest"
-        assert record.getMessage().endswith(f'invalid JSON data: "{server_response.decode()}"')
+        assert len(caplog.records) >= 1
+        found = False
+        for record in caplog.records:
+            if record.name == "telegram.request.BaseRequest":
+                assert record.getMessage().endswith(
+                    f'invalid JSON data: "{server_response.decode()}"'
+                )
+                found = True
+        assert found, "Expected log message not found"
 
     async def test_chat_migrated(self, monkeypatch, aiohttp_request: AiohttpRequest):
         server_response = b'{"ok": "False", "parameters": {"migrate_to_chat_id": 123}}'
@@ -222,17 +223,19 @@ async def test_chat_migrated(self, monkeypatch, aiohttp_request: AiohttpRequest)
 
         assert exc_info.value.new_chat_id == 123
 
-    async def test_retry_after(self, monkeypatch, aiohttp_request: AiohttpRequest):
+    async def test_retry_after(self, monkeypatch):
         server_response = b'{"ok": "False", "parameters": {"retry_after": 42}}'
+        aiohttp_request = AiohttpRequest()
 
         monkeypatch.setattr(
             aiohttp_request,
             "do_request",
             mocker_factory(response=server_response, return_code=HTTPStatus.BAD_REQUEST),
         )
 
-        with pytest.raises(RetryAfter, match="Retry in 42") as exc_info:
-            await aiohttp_request.post(None, None, None)
+        async with aiohttp_request:
+            with pytest.raises(RetryAfter, match="Retry in 42") as exc_info:
+                await aiohttp_request.post(None, None, None)
 
         assert exc_info.value.retry_after == 42
 
@@ -262,7 +265,7 @@ async def test_error_description(
         else:
             match = "Unknown HTTPError"
 
-        server_response = json.dumps(response_data).encode(TextEncoding.UTF_8)
+        server_response = json.dumps(response_data).encode("utf-8")
 
         monkeypatch.setattr(
             aiohttp_request,
@@ -364,7 +367,12 @@ async def make_assertion(*args, **kwargs):
         monkeypatch.setattr(aiohttp_request, "do_request", make_assertion)
 
         await aiohttp_request.post("url", None)
-        assert self.test_flag == (DEFAULT_NONE, DEFAULT_NONE, DEFAULT_NONE, DEFAULT_NONE)
+        assert self.test_flag == (
+            BaseRequest.DEFAULT_NONE,
+            BaseRequest.DEFAULT_NONE,
+            BaseRequest.DEFAULT_NONE,
+            BaseRequest.DEFAULT_NONE,
+        )
 
         await aiohttp_request.post(
             "url", None, read_timeout=1, connect_timeout=2, write_timeout=3, pool_timeout=4
@@ -527,12 +535,10 @@ async def make_assertion(self, **kwargs):
         assert code == HTTPStatus.OK
 
     async def test_do_request_params_with_data(self, monkeypatch, aiohttp_request):
-        mixed_rqs = RequestData(
-            [
-                RequestParameter("name", "value", [InputFile(obj="data", attach=True)]),
-                RequestParameter("second_name", "second_value", []),
-            ]
-        )
+        mixed_rqs = MagicMock()
+        mixed_rqs.contains_files = True
+        mixed_rqs.json_parameters = {"name": "value", "second_name": "second_value"}
+        mixed_rqs.multipart_data = {"attachment": ("data", b"content", "application/octet-stream")}
 
         async def make_assertion(self, **kwargs):
             method_assertion = kwargs.get("method") == "method"