From 60e2b0ae8c89c026ca4ce7066248eb691cddf0ab Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Sat, 8 Aug 2026 11:32:39 +0200 Subject: [PATCH] Improve test coverage --- scrapy/core/http2/agent.py | 28 ++----------------- .../test_downloader_handler_twisted_http2.py | 19 +++++++++++++ 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/scrapy/core/http2/agent.py b/scrapy/core/http2/agent.py index 7ebe28814..1ff5ac097 100644 --- a/scrapy/core/http2/agent.py +++ b/scrapy/core/http2/agent.py @@ -6,12 +6,7 @@ from typing import TYPE_CHECKING from twisted.internet import defer from twisted.internet.defer import Deferred from twisted.python.failure import Failure -from twisted.web.client import ( - URI, - BrowserLikePolicyForHTTPS, - ResponseFailed, - _StandardEndpointFactory, -) +from twisted.web.client import URI, BrowserLikePolicyForHTTPS, _StandardEndpointFactory from twisted.web.error import SchemeNotSupported from scrapy.core.downloader.contextfactory import _AcceptableProtocolsContextFactory @@ -82,9 +77,6 @@ class H2ConnectionPool: self._pending_requests[key] = pending_requests conn_lost_deferred: Deferred[list[BaseException]] = Deferred() - conn_lost_deferred.addCallback( - self._fail_pending_requests, key, pending_requests - ) factory = H2ClientFactory( uri, @@ -139,26 +131,10 @@ class H2ConnectionPool: def _remove_connection( self, errors: list[BaseException], key: ConnectionKeyT, conn: H2ClientProtocol - ) -> list[BaseException]: + ) -> None: # a newer connection may have taken over the key already if self._connections.get(key) is conn: del self._connections[key] - return errors - - def _fail_pending_requests( - self, - errors: list[BaseException], - key: ConnectionKeyT, - pending_requests: deque[Deferred[H2ClientProtocol]], - ) -> list[BaseException]: - """Call the errback of the requests that were waiting for this - connection, unless a newer connection is expected to serve them.""" - if self._pending_requests.get(key) is pending_requests: - del self._pending_requests[key] - while pending_requests: - d = pending_requests.popleft() - d.errback(ResponseFailed(errors)) - return errors def close_connections(self) -> None: """Close all the HTTP/2 connections and remove them from pool.""" diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 449f2d635..7aece4f75 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -7,12 +7,14 @@ import sys from typing import TYPE_CHECKING, Any import pytest +from twisted.internet.defer import DeferredList from twisted.web.http import H2_ENABLED from scrapy import Spider from scrapy.crawler import Crawler from scrapy.exceptions import DownloadFailedError, NotConfigured from scrapy.http import Request +from scrapy.utils.defer import deferred_from_coro, maybe_deferred_to_future from tests.utils.bases.download_handlers_http import ( TestHttpProxyBase, TestHttpsBase, @@ -110,6 +112,23 @@ class TestHttp2(H2DownloadHandlerMixin, TestHttpsBase): response2 = await download_handler.download_request(request2) assert response2.headers["Content-Length"] == b"79" + @coroutine_test + async def test_parallel_requests_same_domain(self, mockserver: MockServer) -> None: + url = mockserver.url("/connection-id", is_secure=self.is_secure) + async with self.get_dh() as download_handler: + results = await maybe_deferred_to_future( + DeferredList( + [ + deferred_from_coro(download_handler.download_request(request)) + for request in (Request(url), Request(url)) + ], + fireOnOneErrback=True, + ) + ) + # the second request waited for the connection that the first one was + # opening instead of opening a second one + assert len({response.text for _, response in results}) == 1 + @pytest.mark.xfail(reason="https://github.com/python-hyper/h2/issues/1247") @coroutine_test async def test_connect_request(self, mockserver: MockServer) -> None: