From 2aa0068f19f3b063e0b4f2c8bfbe8e91784ad5bf Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Tue, 11 Feb 2025 16:50:53 +0000 Subject: [PATCH 1/5] Cleanup questionable no cover comments --- aiohttp/_websocket/helpers.py | 2 +- aiohttp/_websocket/reader.py | 2 +- aiohttp/client_reqrep.py | 8 ++------ aiohttp/compression_utils.py | 2 +- aiohttp/helpers.py | 4 ++-- aiohttp/http_parser.py | 6 ++---- aiohttp/pytest_plugin.py | 8 ++++---- aiohttp/resolver.py | 2 +- aiohttp/tcp_helpers.py | 5 ++--- aiohttp/test_utils.py | 4 ++-- aiohttp/web.py | 2 +- aiohttp/web_routedef.py | 3 +-- aiohttp/web_runner.py | 10 +++++----- aiohttp/web_urldispatcher.py | 4 ++-- aiohttp/worker.py | 2 +- tests/test_client_functional.py | 4 ++-- tests/test_http_parser.py | 5 ++--- tests/test_run_app.py | 2 +- tests/test_web_app.py | 2 +- tests/test_web_urldispatcher.py | 6 +++--- tests/test_websocket_parser.py | 6 +++--- 21 files changed, 40 insertions(+), 49 deletions(-) diff --git a/aiohttp/_websocket/helpers.py b/aiohttp/_websocket/helpers.py index 0bb58df9228..06ced46cdab 100644 --- a/aiohttp/_websocket/helpers.py +++ b/aiohttp/_websocket/helpers.py @@ -52,7 +52,7 @@ def _websocket_mask_python(mask: bytes, data: bytearray) -> None: data[3::4] = data[3::4].translate(d) -if TYPE_CHECKING or NO_EXTENSIONS: # pragma: no cover +if TYPE_CHECKING or NO_EXTENSIONS: websocket_mask = _websocket_mask_python else: try: diff --git a/aiohttp/_websocket/reader.py b/aiohttp/_websocket/reader.py index 23f32265cfc..293754f781f 100644 --- a/aiohttp/_websocket/reader.py +++ b/aiohttp/_websocket/reader.py @@ -4,7 +4,7 @@ from ..helpers import NO_EXTENSIONS -if TYPE_CHECKING or NO_EXTENSIONS: # pragma: no cover +if TYPE_CHECKING or NO_EXTENSIONS: from .reader_py import ( WebSocketDataQueue as WebSocketDataQueuePython, WebSocketReader as WebSocketReaderPython, diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index d3d9ced4044..900a02f7086 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -953,9 +953,7 @@ def links(self) -> "MultiDictProxy[MultiDictProxy[Union[str, URL]]]": for val in re.split(r",(?=\s*<)", links_str): match = re.match(r"\s*<(.*)>(.*)", val) - if match is None: # pragma: no cover - # the check exists to suppress mypy error - continue + assert match is not None url, params_str = match.groups() params = params_str.split(";")[1:] @@ -963,9 +961,7 @@ def links(self) -> "MultiDictProxy[MultiDictProxy[Union[str, URL]]]": for param in params: match = re.match(r"^\s*(\S*)\s*=\s*(['\"]?)(.*?)(\2)\s*$", param, re.M) - if match is None: # pragma: no cover - # the check exists to suppress mypy error - continue + assert match is not None key, _, value, _ = match.groups() link.add(key, value) diff --git a/aiohttp/compression_utils.py b/aiohttp/compression_utils.py index ebe8857f487..7efba7600a3 100644 --- a/aiohttp/compression_utils.py +++ b/aiohttp/compression_utils.py @@ -10,7 +10,7 @@ import brotli HAS_BROTLI = True -except ImportError: # pragma: no cover +except ImportError: HAS_BROTLI = False MAX_SYNC_CHUNK_SIZE = 1024 diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 7f6506939f4..188967721bf 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -214,7 +214,7 @@ def netrc_from_env() -> Optional[netrc.netrc]: else: try: home_dir = Path.home() - except RuntimeError as e: # pragma: no cover + except RuntimeError as e: # if pathlib can't resolve home, it may raise a RuntimeError client_logger.debug( "Could not resolve home directory when " @@ -776,7 +776,7 @@ def set_exception( self, exc: Union[Type[BaseException], BaseException], exc_cause: BaseException = ..., - ) -> None: ... # pragma: no cover + ) -> None: ... def set_exception( diff --git a/aiohttp/http_parser.py b/aiohttp/http_parser.py index bb78aea2fcd..ba648eb6bd8 100644 --- a/aiohttp/http_parser.py +++ b/aiohttp/http_parser.py @@ -932,7 +932,7 @@ def __init__(self, out: StreamReader, encoding: Optional[str]) -> None: self.decompressor: Union[BrotliDecompressor, ZLibDecompressor] if encoding == "br": - if not HAS_BROTLI: # pragma: no cover + if not HAS_BROTLI: raise ContentEncodingError( "Can not decode content-encoding: brotli (br). " "Please install `Brotli`" @@ -1003,7 +1003,7 @@ def end_http_chunk_receiving(self) -> None: RawRequestMessagePy = RawRequestMessage RawResponseMessagePy = RawResponseMessage -try: +with suppress(ImportError): if not NO_EXTENSIONS: from ._http_parser import ( # type: ignore[import-not-found,no-redef] HttpRequestParser, @@ -1016,5 +1016,3 @@ def end_http_chunk_receiving(self) -> None: HttpResponseParserC = HttpResponseParser RawRequestMessageC = RawRequestMessage RawResponseMessageC = RawResponseMessage -except ImportError: # pragma: no cover - pass diff --git a/aiohttp/pytest_plugin.py b/aiohttp/pytest_plugin.py index 1803257cc36..f3deadaf0db 100644 --- a/aiohttp/pytest_plugin.py +++ b/aiohttp/pytest_plugin.py @@ -33,7 +33,7 @@ try: import uvloop -except ImportError: # pragma: no cover +except ImportError: uvloop = None # type: ignore[assignment] _Request = TypeVar("_Request", bound=BaseRequest) @@ -243,7 +243,7 @@ def pytest_generate_tests(metafunc): # type: ignore[no-untyped-def] avail_factories: Dict[str, Type[asyncio.AbstractEventLoopPolicy]] avail_factories = {"pyloop": asyncio.DefaultEventLoopPolicy} - if uvloop is not None: # pragma: no cover + if uvloop is not None: avail_factories["uvloop"] = uvloop.EventLoopPolicy if loops == "all": @@ -253,7 +253,7 @@ def pytest_generate_tests(metafunc): # type: ignore[no-untyped-def] for name in loops.split(","): required = not name.endswith("?") name = name.strip(" ?") - if name not in avail_factories: # pragma: no cover + if name not in avail_factories: if required: raise ValueError( "Unknown loop '%s', available loops: %s" @@ -274,7 +274,7 @@ def loop(loop_factory, fast, loop_debug): # type: ignore[no-untyped-def] asyncio.set_event_loop_policy(policy) with loop_context(fast=fast) as _loop: if loop_debug: - _loop.set_debug(True) # pragma: no cover + _loop.set_debug(True) asyncio.set_event_loop(_loop) yield _loop diff --git a/aiohttp/resolver.py b/aiohttp/resolver.py index 1025df06398..9528cfe48da 100644 --- a/aiohttp/resolver.py +++ b/aiohttp/resolver.py @@ -11,7 +11,7 @@ import aiodns aiodns_default = hasattr(aiodns.DNSResolver, "getaddrinfo") -except ImportError: # pragma: no cover +except ImportError: aiodns = None # type: ignore[assignment] aiodns_default = False diff --git a/aiohttp/tcp_helpers.py b/aiohttp/tcp_helpers.py index 88b24422374..c345850660a 100644 --- a/aiohttp/tcp_helpers.py +++ b/aiohttp/tcp_helpers.py @@ -3,7 +3,6 @@ import asyncio import socket from contextlib import suppress -from typing import Optional # noqa __all__ = ("tcp_keepalive", "tcp_nodelay") @@ -17,8 +16,8 @@ def tcp_keepalive(transport: asyncio.Transport) -> None: else: - def tcp_keepalive(transport: asyncio.Transport) -> None: # pragma: no cover - pass + def tcp_keepalive(transport: asyncio.Transport) -> None: + """Noop when keepalive not supported.""" def tcp_nodelay(transport: asyncio.Transport, value: bool) -> None: diff --git a/aiohttp/test_utils.py b/aiohttp/test_utils.py index eb40ab2f4d7..7d29125ab41 100644 --- a/aiohttp/test_utils.py +++ b/aiohttp/test_utils.py @@ -156,9 +156,9 @@ async def start_server(self, **kwargs: Any) -> None: self.scheme = "https" if self._ssl else "http" self._root = URL(f"{self.scheme}://{absolute_host}:{self.port}") - @abstractmethod # pragma: no cover + @abstractmethod async def _make_runner(self, **kwargs: Any) -> BaseRunner[_Request]: - pass + """Return a new runner for the server.""" def make_url(self, path: StrOrURL) -> URL: assert self._root is not None diff --git a/aiohttp/web.py b/aiohttp/web.py index 3de39fc6e9d..13dd078e7f6 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -503,7 +503,7 @@ def run_app( try: asyncio.set_event_loop(loop) loop.run_until_complete(main_task) - except (GracefulExit, KeyboardInterrupt): # pragma: no cover + except (GracefulExit, KeyboardInterrupt): pass finally: try: diff --git a/aiohttp/web_routedef.py b/aiohttp/web_routedef.py index d9d42e49143..d909e48920b 100644 --- a/aiohttp/web_routedef.py +++ b/aiohttp/web_routedef.py @@ -1,6 +1,5 @@ import abc import dataclasses -import os # noqa from typing import ( TYPE_CHECKING, Any, @@ -48,7 +47,7 @@ class AbstractRouteDef(abc.ABC): @abc.abstractmethod def register(self, router: UrlDispatcher) -> List[AbstractRoute]: - pass # pragma: no cover + """Register itself into the given router.""" _HandlerType = Union[Type[AbstractView], Handler] diff --git a/aiohttp/web_runner.py b/aiohttp/web_runner.py index 55dbdcc61c3..fd0850b6186 100644 --- a/aiohttp/web_runner.py +++ b/aiohttp/web_runner.py @@ -67,7 +67,7 @@ def __init__( @property @abstractmethod def name(self) -> str: - pass # pragma: no cover + """Return the name of the site (e.g. a URL).""" @abstractmethod async def start(self) -> None: @@ -276,7 +276,7 @@ async def setup(self) -> None: try: loop.add_signal_handler(signal.SIGINT, _raise_graceful_exit) loop.add_signal_handler(signal.SIGTERM, _raise_graceful_exit) - except NotImplementedError: # pragma: no cover + except NotImplementedError: # add_signal_handler is not implemented on Windows pass @@ -309,17 +309,17 @@ async def cleanup(self) -> None: try: loop.remove_signal_handler(signal.SIGINT) loop.remove_signal_handler(signal.SIGTERM) - except NotImplementedError: # pragma: no cover + except NotImplementedError: # remove_signal_handler is not implemented on Windows pass @abstractmethod async def _make_server(self) -> Server[_Request]: - pass # pragma: no cover + """Return a new server for the runner to serve requests.""" @abstractmethod async def _cleanup_server(self) -> None: - pass # pragma: no cover + """Run any cleanup steps after the server is shutdown.""" def _reg_site(self, site: BaseSite) -> None: if site in self._sites: diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index e16f973984c..c9a86a9f7b5 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -275,7 +275,7 @@ def current_app(self) -> "Application": @current_app.setter def current_app(self, app: "Application") -> None: - if DEBUG: # pragma: no cover + if DEBUG: if app not in self._apps: raise RuntimeError( "Expected one of the following apps {!r}, got {!r}".format( @@ -368,7 +368,7 @@ async def resolve(self, request: Request) -> _Resolve: @abc.abstractmethod def _match(self, path: str) -> Optional[Dict[str, str]]: - pass # pragma: no cover + """Return dict of path values if path matches this resource, otherwise None.""" def __len__(self) -> int: return len(self._routes) diff --git a/aiohttp/worker.py b/aiohttp/worker.py index 4674388e8ba..360ab8ac00f 100644 --- a/aiohttp/worker.py +++ b/aiohttp/worker.py @@ -38,7 +38,7 @@ class GunicornWebWorker(base.Worker): # type: ignore[misc,no-any-unimported] DEFAULT_AIOHTTP_LOG_FORMAT = AccessLogger.LOG_FORMAT DEFAULT_GUNICORN_LOG_FORMAT = GunicornAccessLogFormat.default - def __init__(self, *args: Any, **kw: Any) -> None: # pragma: no cover + def __init__(self, *args: Any, **kw: Any) -> None: super().__init__(*args, **kw) self._task: Optional[asyncio.Task[None]] = None diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 414ee1dd51b..af63cda9577 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -2895,7 +2895,7 @@ async def handler(request: web.Request) -> web.Response: with pytest.raises(aiohttp.ClientResponseError): async with aiohttp.request("GET", url, raise_for_status=True): - assert False, "never executed" # pragma: no cover + assert False async def test_session_raise_for_status_coro(aiohttp_client: AiohttpClient) -> None: @@ -3414,7 +3414,7 @@ async def handler(request: web.Request) -> NoReturn: async def test_aiohttp_request_ctx_manager_not_found() -> None: with pytest.raises(aiohttp.ClientConnectionError): async with aiohttp.request("GET", "http://wrong-dns-name.com"): - assert False, "never executed" # pragma: no cover + assert False async def test_raising_client_connector_dns_error_on_dns_failure() -> None: diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index b67f225b55e..90d3f687247 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -2,6 +2,7 @@ import asyncio import re +from contextlib import suppress from typing import Any, Dict, Iterable, List, Type from unittest import mock from urllib.parse import quote @@ -36,13 +37,11 @@ REQUEST_PARSERS = [HttpRequestParserPy] RESPONSE_PARSERS = [HttpResponseParserPy] -try: +with suppress(ImportError): from aiohttp.http_parser import HttpRequestParserC, HttpResponseParserC REQUEST_PARSERS.append(HttpRequestParserC) RESPONSE_PARSERS.append(HttpResponseParserC) -except ImportError: # pragma: no cover - pass @pytest.fixture diff --git a/tests/test_run_app.py b/tests/test_run_app.py index 7649fa9a276..9673145b4e4 100644 --- a/tests/test_run_app.py +++ b/tests/test_run_app.py @@ -1050,7 +1050,7 @@ def __init__(self, *args: Any, **kwargs: Any): async def test() -> None: await asyncio.sleep(0.5) async with ClientSession() as sess: - for _ in range(5): # pragma: no cover + for _ in range(5): # Retry for flaky tests # pragma: no cover try: with pytest.raises(asyncio.TimeoutError): async with sess.get( diff --git a/tests/test_web_app.py b/tests/test_web_app.py index 12202778926..62d69efb528 100644 --- a/tests/test_web_app.py +++ b/tests/test_web_app.py @@ -191,7 +191,7 @@ def test_app_run_middlewares() -> None: assert root._run_middlewares is False async def middleware(request: web.Request, handler: Handler) -> web.StreamResponse: - return await handler(request) # pragma: no cover + assert False root = web.Application(middlewares=[middleware]) sub = web.Application() diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index d21ecaa101a..f06022e3ec4 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -380,7 +380,7 @@ async def test_handler_metadata_persistence() -> None: async def async_handler(request: web.Request) -> web.Response: """Doc""" - return web.Response() # pragma: no cover + assert False app.router.add_get("/async", async_handler) @@ -696,7 +696,7 @@ def test_reuse_last_added_resource(path: str) -> None: app = web.Application() async def handler(request: web.Request) -> web.Response: - return web.Response() # pragma: no cover + assert False app.router.add_get(path, handler, name="a") app.router.add_post(path, handler, name="a") @@ -708,7 +708,7 @@ def test_resource_raw_match() -> None: app = web.Application() async def handler(request: web.Request) -> web.Response: - return web.Response() # pragma: no cover + assert False route = app.router.add_get("/a", handler, name="a") assert route.resource is not None diff --git a/tests/test_websocket_parser.py b/tests/test_websocket_parser.py index a4f93e8904b..8feec27332e 100644 --- a/tests/test_websocket_parser.py +++ b/tests/test_websocket_parser.py @@ -50,7 +50,7 @@ def build_frame( if message.endswith(WS_DEFLATE_TRAILING): message = message[:-4] msg_length = len(message) - if use_mask: # pragma: no cover + if use_mask: mask_bit = 0x80 else: mask_bit = 0 @@ -65,12 +65,12 @@ def build_frame( if msg_length < 126: header = PACK_LEN1(header_first_byte, msg_length | mask_bit) - elif msg_length < (1 << 16): # pragma: no cover + elif msg_length < (1 << 16): header = PACK_LEN2(header_first_byte, 126 | mask_bit, msg_length) else: header = PACK_LEN3(header_first_byte, 127 | mask_bit, msg_length) - if use_mask: # pragma: no cover + if use_mask: maski = random.randrange(0, 0xFFFFFFFF) mask = maski.to_bytes(4, "big") message = bytearray(message) From 11ea751b6567b3f14f0495979b4fd61bf755ca9f Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Tue, 11 Feb 2025 17:06:26 +0000 Subject: [PATCH 2/5] Update test_websocket_parser.py --- tests/test_websocket_parser.py | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/tests/test_websocket_parser.py b/tests/test_websocket_parser.py index 8feec27332e..4716804cd85 100644 --- a/tests/test_websocket_parser.py +++ b/tests/test_websocket_parser.py @@ -37,7 +37,6 @@ class PatchableWebSocketReader(WebSocketReader): def build_frame( message: bytes, opcode: int, - use_mask: bool = False, noheader: bool = False, is_fin: bool = True, compress: bool = False, @@ -50,10 +49,6 @@ def build_frame( if message.endswith(WS_DEFLATE_TRAILING): message = message[:-4] msg_length = len(message) - if use_mask: - mask_bit = 0x80 - else: - mask_bit = 0 if is_fin: header_first_byte = 0x80 | opcode @@ -64,26 +59,15 @@ def build_frame( header_first_byte |= 0x40 if msg_length < 126: - header = PACK_LEN1(header_first_byte, msg_length | mask_bit) - elif msg_length < (1 << 16): - header = PACK_LEN2(header_first_byte, 126 | mask_bit, msg_length) + header = PACK_LEN1(header_first_byte, msg_length) else: - header = PACK_LEN3(header_first_byte, 127 | mask_bit, msg_length) - - if use_mask: - maski = random.randrange(0, 0xFFFFFFFF) - mask = maski.to_bytes(4, "big") - message = bytearray(message) - websocket_mask(mask, message) - if noheader: - return message - else: - return header + mask + message + assert msg_length < (1 << 16) + header = PACK_LEN2(header_first_byte, 126, msg_length) + + if noheader: + return message else: - if noheader: - return message - else: - return header + message + return header + message def build_close_frame( From c31fcc17bf3285a81d30ee8a7bc7264906b30435 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Tue, 11 Feb 2025 17:11:35 +0000 Subject: [PATCH 3/5] Update test_websocket_parser.py --- tests/test_websocket_parser.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/test_websocket_parser.py b/tests/test_websocket_parser.py index 4716804cd85..7e187b897c4 100644 --- a/tests/test_websocket_parser.py +++ b/tests/test_websocket_parser.py @@ -1,6 +1,5 @@ import asyncio import pickle -import random import struct import zlib from typing import Union @@ -9,13 +8,7 @@ import pytest from aiohttp._websocket import helpers as _websocket_helpers -from aiohttp._websocket.helpers import ( - PACK_CLOSE_CODE, - PACK_LEN1, - PACK_LEN2, - PACK_LEN3, - websocket_mask, -) +from aiohttp._websocket.helpers import PACK_CLOSE_CODE, PACK_LEN1, PACK_LEN2 from aiohttp._websocket.models import WS_DEFLATE_TRAILING from aiohttp._websocket.reader import WebSocketDataQueue from aiohttp.base_protocol import BaseProtocol From a48ecdb1915151a6e9020e5a776fdef3d618c6cc Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Tue, 11 Feb 2025 17:15:16 +0000 Subject: [PATCH 4/5] Update aiohttp/client_reqrep.py --- aiohttp/client_reqrep.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 900a02f7086..d58c35a042a 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -953,7 +953,8 @@ def links(self) -> "MultiDictProxy[MultiDictProxy[Union[str, URL]]]": for val in re.split(r",(?=\s*<)", links_str): match = re.match(r"\s*<(.*)>(.*)", val) - assert match is not None + if match is None: # Malformed link + continue url, params_str = match.groups() params = params_str.split(";")[1:] From b2c1c06829ae484d0191458377215aedcab1c616 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Tue, 11 Feb 2025 17:16:27 +0000 Subject: [PATCH 5/5] Update aiohttp/client_reqrep.py --- aiohttp/client_reqrep.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index d58c35a042a..f42379d45c8 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -962,7 +962,8 @@ def links(self) -> "MultiDictProxy[MultiDictProxy[Union[str, URL]]]": for param in params: match = re.match(r"^\s*(\S*)\s*=\s*(['\"]?)(.*?)(\2)\s*$", param, re.M) - assert match is not None + if match is None: # Malformed param + continue key, _, value, _ = match.groups() link.add(key, value)