From 346032f38de0e4d0e1f7c435ccfe7c503b4c7f0d Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 7 Aug 2026 21:06:55 +0200 Subject: [PATCH] Improve coverage --- .../test_downloader_handler_twisted_http11.py | 46 +++++++++++++++++++ tests/utils/bases/download_handlers_http.py | 19 ++++++-- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index 7f6072523..b1cf7fc6e 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -7,12 +7,20 @@ from contextlib import asynccontextmanager from typing import TYPE_CHECKING, Any import pytest +from twisted.internet.defer import DeferredList + +try: + import resource +except ImportError: + resource = None # type: ignore[assignment] from scrapy import Spider from scrapy.core.downloader.handlers._base_http import _auto_connection_limit from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler from scrapy.crawler import Crawler from scrapy.exceptions import NotConfigured +from scrapy.http import Request +from scrapy.utils.defer import deferred_from_coro, maybe_deferred_to_future from scrapy.utils.misc import build_from_crawler from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler @@ -36,6 +44,7 @@ if TYPE_CHECKING: from collections.abc import AsyncGenerator from scrapy.core.downloader.handlers import DownloadHandlerProtocol + from tests.mockserver.http import MockServer pytestmark = pytest.mark.requires_reactor # HTTP11DownloadHandler requires a reactor @@ -102,6 +111,43 @@ async def test_connection_limit_below_concurrent_requests( assert "CONCURRENT_CONNECTIONS_PER_HANDLER (4)" in caplog.text +@pytest.mark.skipif(resource is None, reason="No file descriptor limit") +@coroutine_test +async def test_connection_limit_auto_without_file_descriptor_limit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + infinity = resource.RLIM_INFINITY + monkeypatch.setattr(resource, "getrlimit", lambda _: (infinity, infinity)) + async with _get_dh({}) as dh: + assert dh._pool._limit == 0 + + +@coroutine_test +async def test_connection_limit_with_several_connections_per_host( + mockserver: MockServer, +) -> None: + url = mockserver.url("/connection-id") + slow_url = mockserver.url("/connection-id?delay=0.5") + other_url = url.replace("127.0.0.1", "localhost") + async with _get_dh({"CONCURRENT_CONNECTIONS_PER_HANDLER": 2}) as dh: + results = await maybe_deferred_to_future( + DeferredList( + [ + deferred_from_coro(dh.download_request(Request(slow_url))) + for _ in range(2) + ], + fireOnOneErrback=True, + ) + ) + ids = {response.text for _, response in results} + assert len(ids) == 2 + # only one of the two connections to the host makes room for the + # connection to the other host, so the other one stays reusable + await dh.download_request(Request(other_url)) + reused = await dh.download_request(Request(url)) + assert reused.text in ids + + @coroutine_test async def test_keepalive_timeout() -> None: async with _get_dh({"CONNECTION_KEEPALIVE_TIMEOUT": 5}) as dh: diff --git a/tests/utils/bases/download_handlers_http.py b/tests/utils/bases/download_handlers_http.py index bf24da4ed..826f4c1cb 100644 --- a/tests/utils/bases/download_handlers_http.py +++ b/tests/utils/bases/download_handlers_http.py @@ -134,23 +134,32 @@ class TestHttpBase(ABC): settings_dict = {"CONCURRENT_CONNECTIONS_PER_HANDLER": 1} assert len(await self._connection_ids(urls, settings_dict)) == 4 + settings_dict = {"CONCURRENT_CONNECTIONS_PER_HANDLER": 0} + assert len(await self._connection_ids(urls, settings_dict)) == 2 + @coroutine_test async def test_connection_limit_spares_connections_in_use( self, mockserver: MockServer ) -> None: - url = mockserver.url("/connection-id?delay=0.5", is_secure=self.is_secure) - urls = [url, self._other_host(url)] + url = mockserver.url("/connection-id", is_secure=self.is_secure) + slow_url = mockserver.url("/connection-id?delay=0.5", is_secure=self.is_secure) + other_url = self._other_host(url) async with self.get_dh({"CONCURRENT_CONNECTIONS_PER_HANDLER": 1}) as dh: + first = await dh.download_request(Request(url)) results = await maybe_deferred_to_future( DeferredList( [ - deferred_from_coro(dh.download_request(Request(url))) - for url in urls + deferred_from_coro(dh.download_request(Request(slow_url))), + deferred_from_coro(dh.download_request(Request(other_url))), ], fireOnOneErrback=True, ) ) - assert len({response.text for _, response in results}) == 2 + ids = {response.text for _, response in results} + # the connection serving the slow request was kept even though the + # request to the other host had to exceed the limit to get one + assert first.text in ids + assert len(ids) == 2 @coroutine_test async def test_unsupported_scheme(self) -> None: