diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 9fd5a40df..1d64c2f39 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -633,43 +633,52 @@ class TestCrawlSpider: yield crawler.crawl(seed=url, mockserver=self.mockserver) assert crawler.spider.meta["responses"][0].certificate is None - @inlineCallbacks - def test_response_ssl_certificate(self): - crawler = get_crawler(SingleRequestSpider) - url = self.mockserver.url("/echo?body=test", is_secure=True) - yield crawler.crawl(seed=url, mockserver=self.mockserver) - cert = crawler.spider.meta["responses"][0].certificate - assert isinstance(cert, Certificate) - assert cert.getSubject().commonName == b"localhost" - assert cert.getIssuer().commonName == b"localhost" - - @pytest.mark.xfail( - reason="Responses with no body return early and contain no certificate" + @pytest.mark.parametrize( + "url", + [ + "/echo?body=test", + pytest.param( + "/status?n=200", + marks=pytest.mark.xfail( + reason="With HTTP11DownloadHandler, responses with no body are returned early and contain no certificate", + strict=True, + ), + ), + ], ) - @inlineCallbacks - def test_response_ssl_certificate_empty_response(self): + @deferred_f_from_coro_f + async def test_response_ssl_certificate( + self, mockserver: MockServer, url: str + ) -> None: crawler = get_crawler(SingleRequestSpider) - url = self.mockserver.url("/status?n=200", is_secure=True) - yield crawler.crawl(seed=url, mockserver=self.mockserver) + url = mockserver.url(url, is_secure=True) + await crawler.crawl_async(seed=url, mockserver=mockserver) + assert isinstance(crawler.spider, SingleRequestSpider) cert = crawler.spider.meta["responses"][0].certificate assert isinstance(cert, Certificate) assert cert.getSubject().commonName == b"localhost" assert cert.getIssuer().commonName == b"localhost" - @inlineCallbacks - def test_dns_server_ip_address_none(self): + @pytest.mark.parametrize( + "url", + [ + "/echo?body=test", + pytest.param( + "/status?n=200", + marks=pytest.mark.xfail( + reason="With HTTP11DownloadHandler, responses with no body are returned early and contain no ip_address", + strict=True, + ), + ), + ], + ) + @deferred_f_from_coro_f + async def test_response_ip_address(self, mockserver: MockServer, url: str) -> None: crawler = get_crawler(SingleRequestSpider) - url = self.mockserver.url("/status?n=200") - yield crawler.crawl(seed=url, mockserver=self.mockserver) - ip_address = crawler.spider.meta["responses"][0].ip_address - assert ip_address is None - - @inlineCallbacks - def test_dns_server_ip_address(self): - crawler = get_crawler(SingleRequestSpider) - url = self.mockserver.url("/echo?body=test") + url = mockserver.url(url) expected_netloc, _ = urlparse(url).netloc.split(":") - yield crawler.crawl(seed=url, mockserver=self.mockserver) + await crawler.crawl_async(seed=url, mockserver=mockserver) + assert isinstance(crawler.spider, SingleRequestSpider) ip_address = crawler.spider.meta["responses"][0].ip_address assert isinstance(ip_address, IPv4Address) assert str(ip_address) == gethostbyname(expected_netloc) diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index 79b2a6fc5..eb2735c4d 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -60,7 +60,16 @@ class TestHttps11CustomCiphers(HTTP11DownloadHandlerMixin, TestHttpsCustomCipher class TestHttp11WithCrawler(TestHttpWithCrawlerBase): @property def settings_dict(self) -> dict[str, Any] | None: - return None # default handler settings + return { + "DOWNLOAD_HANDLERS": { + "http": "scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler", + "https": "scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler", + } + } + + +class TestHttps11WithCrawler(TestHttp11WithCrawler): + is_secure = True class TestHttp11Proxy(HTTP11DownloadHandlerMixin, TestHttpProxyBase): diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 3fbc8d416..5ae35a461 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -4,14 +4,12 @@ from __future__ import annotations import json from typing import TYPE_CHECKING, Any -from unittest import mock import pytest from testfixtures import LogCapture -from twisted.internet import defer from twisted.web.http import H2_ENABLED -from scrapy.exceptions import DownloadCancelledError, UnsupportedURLSchemeError +from scrapy.exceptions import UnsupportedURLSchemeError from scrapy.http import Request from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from tests.test_downloader_handlers_http_base import ( @@ -57,32 +55,6 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): response = await download_handler.download_request(request) assert response.protocol == "h2" - @deferred_f_from_coro_f - async def test_download_with_maxsize_very_large_file( - self, mockserver: MockServer - ) -> None: - from twisted.internet import reactor - - with mock.patch("scrapy.core.http2.stream.logger") as logger: - request = Request( - mockserver.url("/largechunkedfile", is_secure=self.is_secure) - ) - - def check(logger: mock.Mock) -> None: - logger.error.assert_called_once_with(mock.ANY) - - async with self.get_dh({"DOWNLOAD_MAXSIZE": 1_500}) as download_handler: - with pytest.raises(DownloadCancelledError): - await download_handler.download_request(request) - - # As the error message is logged in the dataReceived callback, we - # have to give a bit of time to the reactor to process the queue - # after closing the connection. - d: defer.Deferred[mock.Mock] = defer.Deferred() - d.addCallback(check) - reactor.callLater(0.1, d.callback, logger) - await maybe_deferred_to_future(d) - def test_download_cause_data_loss(self) -> None: # type: ignore[override] pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) @@ -206,7 +178,8 @@ class TestHttp2WithCrawler(TestHttpWithCrawlerBase): def settings_dict(self) -> dict[str, Any] | None: return { "DOWNLOAD_HANDLERS": { - "https": "scrapy.core.downloader.handlers.http2.H2DownloadHandler" + "http": None, + "https": "scrapy.core.downloader.handlers.http2.H2DownloadHandler", } } diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index 5e3b49548..7ad787346 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -8,12 +8,13 @@ import sys from abc import ABC, abstractmethod from contextlib import asynccontextmanager from http import HTTPStatus +from ipaddress import IPv4Address +from socket import gethostbyname from typing import TYPE_CHECKING, Any -from unittest import mock +from urllib.parse import urlparse import pytest -from testfixtures import LogCapture -from twisted.internet import defer +from twisted.internet.ssl import Certificate from scrapy.exceptions import ( CannotResolveHostError, @@ -25,7 +26,6 @@ from scrapy.exceptions import ( UnsupportedURLSchemeError, ) from scrapy.http import Headers, HtmlResponse, Request, Response, TextResponse -from scrapy.utils.asyncio import call_later from scrapy.utils.defer import ( deferred_f_from_coro_f, deferred_from_coro, @@ -135,6 +135,48 @@ class TestHttpBase(ABC): assert header_name in body["headers"] assert body["headers"][header_name] == [header_value] + @deferred_f_from_coro_f + async def test_request_header_none(self, mockserver: MockServer) -> None: + """Adding a header with None as the value should not send that header.""" + request_headers = { + "Cookie": None, + "X-Custom-Header": None, + } + request = Request( + mockserver.url("/echo", is_secure=self.is_secure), + headers=request_headers, + ) + async with self.get_dh() as download_handler: + response = await download_handler.download_request(request) + assert response.status == HTTPStatus.OK + body = json.loads(response.body.decode("utf-8")) + assert "headers" in body + for header_name in request_headers: + assert header_name not in body["headers"] + + @pytest.mark.parametrize( + "request_headers", + [ + {"X-Custom-Header": ["foo", "bar"]}, + [("X-Custom-Header", "foo"), ("X-Custom-Header", "bar")], + ], + ) + @deferred_f_from_coro_f + async def test_request_header_duplicate( + self, mockserver: MockServer, request_headers: Any + ) -> None: + """All values for a header should be sent.""" + request = Request( + mockserver.url("/echo", is_secure=self.is_secure), + headers=request_headers, + ) + async with self.get_dh() as download_handler: + response = await download_handler.download_request(request) + assert response.status == HTTPStatus.OK + body = json.loads(response.body.decode("utf-8")) + assert "headers" in body + assert body["headers"]["X-Custom-Header"] == ["foo", "bar"] + @deferred_f_from_coro_f async def test_server_receives_correct_request_body( self, mockserver: MockServer @@ -166,7 +208,7 @@ class TestHttpBase(ABC): "Content-Encoding": "gzip", "Content-MD5": "Q2hlY2sgSW50ZWdyaXR5IQ==", "Content-Type": "text/html; charset=utf-8", - "Date": "Date: Tue, 15 Nov 1994 08:12:31 GMT", + "Date": "Tue, 15 Nov 1994 08:12:31 GMT", "Pragma": "no-cache", "Retry-After": "120", "Set-Cookie": "CookieName=CookieValue; Max-Age=3600; Version=1", @@ -450,28 +492,14 @@ class TestHttp11Base(TestHttpBase): @deferred_f_from_coro_f async def test_download_with_maxsize_very_large_file( - self, mockserver: MockServer + self, mockserver: MockServer, caplog: pytest.LogCaptureFixture ) -> None: - # TODO: the logger check is specific to scrapy.core.downloader.handlers.http11 - with mock.patch("scrapy.core.downloader.handlers.http11.logger") as logger: - request = Request( - mockserver.url("/largechunkedfile", is_secure=self.is_secure) - ) + request = Request(mockserver.url("/largechunkedfile", is_secure=self.is_secure)) + async with self.get_dh({"DOWNLOAD_MAXSIZE": 1_500}) as download_handler: + with pytest.raises(DownloadCancelledError): + await download_handler.download_request(request) - def check(logger: mock.Mock) -> None: - logger.warning.assert_called_once_with(mock.ANY, mock.ANY) - - async with self.get_dh({"DOWNLOAD_MAXSIZE": 1_500}) as download_handler: - with pytest.raises(DownloadCancelledError): - await download_handler.download_request(request) - - # As the error message is logged in the dataReceived callback, we - # have to give a bit of time to the reactor to process the queue - # after closing the connection. - d: defer.Deferred[mock.Mock] = defer.Deferred() - d.addCallback(check) - call_later(0.1, d.callback, logger) - await maybe_deferred_to_future(d) + assert "larger than download max size" in caplog.text @deferred_f_from_coro_f async def test_download_with_maxsize_per_req(self, mockserver: MockServer) -> None: @@ -618,17 +646,17 @@ class TestHttps11Base(TestHttp11Base): pytest.skip("Unable to test on HTTPS") @deferred_f_from_coro_f - async def test_tls_logging(self, mockserver: MockServer) -> None: + async def test_tls_logging( + self, mockserver: MockServer, caplog: pytest.LogCaptureFixture + ) -> None: request = Request(mockserver.url("/text", is_secure=self.is_secure)) async with self.get_dh( {"DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING": True} ) as download_handler: - with LogCapture() as log_capture: + with caplog.at_level("DEBUG"): response = await download_handler.download_request(request) assert response.body == b"Works" - log_capture.check_present( - ("scrapy.core.downloader.tls", "DEBUG", self.tls_log_message) - ) + assert self.tls_log_message in caplog.text class TestSimpleHttpsBase(ABC): @@ -744,6 +772,33 @@ class TestHttpWithCrawlerBase(ABC): reason = crawler.spider.meta["close_reason"] # type: ignore[attr-defined] assert reason == "finished" + @deferred_f_from_coro_f + async def test_response_ssl_certificate(self, mockserver: MockServer) -> None: + if not self.is_secure: + pytest.skip("Only applies to HTTPS") + # copy of TestCrawl.test_response_ssl_certificate() + # the current test implementation can only work for Twisted-based download handlers + crawler = get_crawler(SingleRequestSpider, self.settings_dict) + url = mockserver.url("/echo?body=test", is_secure=self.is_secure) + await crawler.crawl_async(seed=url, mockserver=mockserver) + assert isinstance(crawler.spider, SingleRequestSpider) + cert = crawler.spider.meta["responses"][0].certificate + assert isinstance(cert, Certificate) + assert cert.getSubject().commonName == b"localhost" + assert cert.getIssuer().commonName == b"localhost" + + @deferred_f_from_coro_f + async def test_response_ip_address(self, mockserver: MockServer) -> None: + # copy of TestCrawl.test_response_ip_address() + crawler = get_crawler(SingleRequestSpider, self.settings_dict) + url = mockserver.url("/echo?body=test", is_secure=self.is_secure) + expected_netloc, _ = urlparse(url).netloc.split(":") + await crawler.crawl_async(seed=url, mockserver=mockserver) + assert isinstance(crawler.spider, SingleRequestSpider) + ip_address = crawler.spider.meta["responses"][0].ip_address + assert isinstance(ip_address, IPv4Address) + assert str(ip_address) == gethostbyname(expected_netloc) + class TestHttpProxyBase(ABC): is_secure = False diff --git a/tests/test_engine_stop_download_bytes.py b/tests/test_engine_stop_download_bytes.py index 1d7df70eb..b46ebe2d9 100644 --- a/tests/test_engine_stop_download_bytes.py +++ b/tests/test_engine_stop_download_bytes.py @@ -2,8 +2,6 @@ from __future__ import annotations from typing import TYPE_CHECKING -from testfixtures import LogCapture - from scrapy.exceptions import StopDownload from scrapy.utils.defer import deferred_f_from_coro_f from tests.test_engine import ( @@ -16,6 +14,8 @@ from tests.test_engine import ( ) if TYPE_CHECKING: + import pytest + from tests.mockserver.http import MockServer @@ -27,7 +27,9 @@ class BytesReceivedCrawlerRun(CrawlerRun): class TestBytesReceivedEngine(TestEngineBase): @deferred_f_from_coro_f - async def test_crawler(self, mockserver: MockServer) -> None: + async def test_crawler( + self, mockserver: MockServer, caplog: pytest.LogCaptureFixture + ) -> None: for spider in ( MySpider, DictItemsSpider, @@ -35,32 +37,13 @@ class TestBytesReceivedEngine(TestEngineBase): DataClassItemsSpider, ): run = BytesReceivedCrawlerRun(spider) - with LogCapture() as log: + with caplog.at_level("DEBUG"): await run.run(mockserver) - log.check_present( - ( - "scrapy.core.downloader.handlers.http11", - "DEBUG", - f"Download stopped for " - "from signal handler BytesReceivedCrawlerRun.bytes_received", - ) - ) - log.check_present( - ( - "scrapy.core.downloader.handlers.http11", - "DEBUG", - f"Download stopped for " - "from signal handler BytesReceivedCrawlerRun.bytes_received", - ) - ) - log.check_present( - ( - "scrapy.core.downloader.handlers.http11", - "DEBUG", - f"Download stopped for " - "from signal handler BytesReceivedCrawlerRun.bytes_received", - ) - ) + for url in ("/redirected", "/static/", "/numbers"): + assert ( + f"Download stopped for " + "from signal handler BytesReceivedCrawlerRun.bytes_received" + ) in caplog.text self._assert_visited_urls(run) self._assert_scheduled_requests(run, count=9) self._assert_downloaded_responses(run, count=9) diff --git a/tests/test_engine_stop_download_headers.py b/tests/test_engine_stop_download_headers.py index c01413d4e..76cfbe8ed 100644 --- a/tests/test_engine_stop_download_headers.py +++ b/tests/test_engine_stop_download_headers.py @@ -2,8 +2,6 @@ from __future__ import annotations from typing import TYPE_CHECKING -from testfixtures import LogCapture - from scrapy.exceptions import StopDownload from scrapy.utils.defer import deferred_f_from_coro_f from tests.test_engine import ( @@ -16,6 +14,8 @@ from tests.test_engine import ( ) if TYPE_CHECKING: + import pytest + from tests.mockserver.http import MockServer @@ -27,7 +27,9 @@ class HeadersReceivedCrawlerRun(CrawlerRun): class TestHeadersReceivedEngine(TestEngineBase): @deferred_f_from_coro_f - async def test_crawler(self, mockserver: MockServer) -> None: + async def test_crawler( + self, mockserver: MockServer, caplog: pytest.LogCaptureFixture + ) -> None: for spider in ( MySpider, DictItemsSpider, @@ -35,32 +37,13 @@ class TestHeadersReceivedEngine(TestEngineBase): DataClassItemsSpider, ): run = HeadersReceivedCrawlerRun(spider) - with LogCapture() as log: + with caplog.at_level("DEBUG"): await run.run(mockserver) - log.check_present( - ( - "scrapy.core.downloader.handlers.http11", - "DEBUG", - f"Download stopped for from" - " signal handler HeadersReceivedCrawlerRun.headers_received", - ) - ) - log.check_present( - ( - "scrapy.core.downloader.handlers.http11", - "DEBUG", - f"Download stopped for from signal" - " handler HeadersReceivedCrawlerRun.headers_received", - ) - ) - log.check_present( - ( - "scrapy.core.downloader.handlers.http11", - "DEBUG", - f"Download stopped for from" - " signal handler HeadersReceivedCrawlerRun.headers_received", - ) - ) + for url in ("/redirected", "/static/", "/numbers"): + assert ( + f"Download stopped for " + "from signal handler HeadersReceivedCrawlerRun.headers_received" + ) in caplog.text self._assert_visited_urls(run) self._assert_downloaded_responses(run, count=6) self._assert_signals_caught(run) diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index f315388cd..acf8dd412 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -476,7 +476,8 @@ class TestMiddlewareManagerSpider: ): await mwman.close_spider_async() - def test_deprecated_spider_arg_with_crawler(self, crawler: Crawler) -> None: + @deferred_f_from_coro_f + async def test_deprecated_spider_arg_with_crawler(self, crawler: Crawler) -> None: """Crawler is provided and has a spider, works. The instance passed to a deprecated method is ignored, even if mismatched.""" mwman = ItemPipelineManager(crawler=crawler) @@ -485,14 +486,15 @@ class TestMiddlewareManagerSpider: ScrapyDeprecationWarning, match=r"ItemPipelineManager.open_spider\(\) is deprecated, use open_spider_async\(\) instead", ): - mwman.open_spider(DefaultSpider()) + await maybe_deferred_to_future(mwman.open_spider(DefaultSpider())) with pytest.warns( ScrapyDeprecationWarning, match=r"ItemPipelineManager.close_spider\(\) is deprecated, use close_spider_async\(\) instead", ): - mwman.close_spider(DefaultSpider()) + await maybe_deferred_to_future(mwman.close_spider(DefaultSpider())) - def test_deprecated_spider_arg_without_crawler(self) -> None: + @deferred_f_from_coro_f + async def test_deprecated_spider_arg_without_crawler(self) -> None: """The first instance passed to a deprecated method is used. Mismatched ones raise an error.""" with pytest.warns( ScrapyDeprecationWarning, @@ -504,7 +506,7 @@ class TestMiddlewareManagerSpider: ScrapyDeprecationWarning, match=r"ItemPipelineManager.open_spider\(\) is deprecated, use open_spider_async\(\) instead", ): - mwman.open_spider(spider) + await maybe_deferred_to_future(mwman.open_spider(spider)) with ( pytest.warns( ScrapyDeprecationWarning, @@ -514,12 +516,12 @@ class TestMiddlewareManagerSpider: RuntimeError, match="Different instances of Spider were passed" ), ): - mwman.close_spider(DefaultSpider()) + await maybe_deferred_to_future(mwman.close_spider(DefaultSpider())) with pytest.warns( ScrapyDeprecationWarning, match=r"ItemPipelineManager.close_spider\(\) is deprecated, use close_spider_async\(\) instead", ): - mwman.close_spider(spider) + await maybe_deferred_to_future(mwman.close_spider(spider)) @deferred_f_from_coro_f async def test_no_spider_arg_without_crawler(self) -> None: diff --git a/tests/test_utils_defer.py b/tests/test_utils_defer.py index df7f1a20e..be623c3fb 100644 --- a/tests/test_utils_defer.py +++ b/tests/test_utils_defer.py @@ -321,9 +321,11 @@ class TestDeferredFFromCoroF: yield self._assert_result(c_f) + @pytest.mark.only_asyncio @inlineCallbacks def test_coroutine_asyncio(self): async def c_f() -> int: + await asyncio.sleep(0.01) return 42 yield self._assert_result(c_f)