From 59ebb26e60ea1e776ed11d00aef1d20bdb535e98 Mon Sep 17 00:00:00 2001 From: Shadow_Lu Date: Tue, 28 Jul 2026 15:51:55 +0530 Subject: [PATCH 01/25] Fix CaseInsensitiveDict.copy() sharing state with the original (#7783) * Fix CaseInsensitiveDict.copy() sharing state with the original * Address review: don't re-normalise in __copy__, keep _keys in sync in __ior__ UserDict.__ior__ writes self.data directly, bypassing __setitem__, so _keys never learned about the new keys. --- scrapy/utils/datatypes.py | 16 ++++++++++++++++ tests/test_utils_datatypes.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/scrapy/utils/datatypes.py b/scrapy/utils/datatypes.py index c020ff4b9..e761a2474 100644 --- a/scrapy/utils/datatypes.py +++ b/scrapy/utils/datatypes.py @@ -132,6 +132,22 @@ class CaseInsensitiveDict(collections.UserDict[str | bytes, Any]): def __repr__(self) -> str: return f"<{self.__class__.__name__}: {super().__repr__()}>" + # UserDict.copy() shallow-copies the instance, which would share self._keys + # between the copy and the original. + def __copy__(self) -> Self: + new = self.__class__() + new.data = self.data.copy() + new._keys = self._keys.copy() + return new + + copy = __copy__ + + # UserDict.__ior__ updates self.data directly, which would leave self._keys + # out of date. + def __ior__(self, other: Any) -> Self: # type: ignore[override,misc] + self.update(other) + return self + def _normkey(self, key: str | bytes) -> str | bytes: return key diff --git a/tests/test_utils_datatypes.py b/tests/test_utils_datatypes.py index fe1f60c7d..f43d20e69 100644 --- a/tests/test_utils_datatypes.py +++ b/tests/test_utils_datatypes.py @@ -204,6 +204,15 @@ class TestCaseInsensitiveDictBase(ABC): assert h1.get("header1") == h3.get("header1") assert h1.get("header1") == h3.get("HEADER1") + def test_copy_is_independent(self): + h1 = self.dict_class({"header1": "value1", "header2": "value2"}) + for h2 in (copy.copy(h1), h1.copy()): + del h2["header1"] + h2["header3"] = "value3" + assert "header1" in h1 + assert "header3" not in h1 + assert dict(h1) == {"header1": "value1", "header2": "value2"} + class TestCaseInsensitiveDict(TestCaseInsensitiveDictBase): dict_class = CaseInsensitiveDict # type: ignore[assignment] @@ -220,6 +229,28 @@ class TestCaseInsensitiveDict(TestCaseInsensitiveDictBase): assert isinstance(iterkeys, Iterator) assert list(iterkeys) == ["AsDf", "FoO"] + def test_copy_keeps_values(self): + class MyDict(self.dict_class): + def _normvalue(self, value): + return value + 1 + + d = MyDict({"key": 1}) + for copied in (copy.copy(d), d.copy()): + assert copied["key"] == 2 + + def test_ior(self): + d = self.dict_class({"header1": "value1"}) + d |= {"HEADER1": "value2", "header2": "value3"} + assert len(d) == 2 + assert d["HeAdEr1"] == "value2" + assert d["HeAdEr2"] == "value3" + + def test_ior_mapping(self): + d = self.dict_class({"header1": "value1"}) + d |= self.dict_class({"HEADER1": "value2"}) + assert len(d) == 1 + assert d["HeAdEr1"] == "value2" + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") class TestCaselessDict(TestCaseInsensitiveDictBase): From 5a65bdcc18e51a85fbc9eca0a25e4be7aa4ce4e3 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 28 Jul 2026 12:24:04 +0200 Subject: [PATCH 02/25] Improve spiders coverage (#7768) * Skip the deprecated scrapy.mail in coverage data * Improve CrawlSpider coverage * Improve XMLFeedSpider coverage * Improve SitemapSpider coverage * Solve mypy issues * Align new spider tests with the shared test helper structure --- scrapy/mail.py | 2 + scrapy/spiders/feed.py | 11 +-- tests/spiders.py | 46 +++++++++++++ tests/test_crawl.py | 16 +++++ tests/test_spider.py | 130 ++++++++++++++++++++++++++++++++++- tests/test_spider_crawl.py | 43 ++++++++++++ tests/test_spider_sitemap.py | 62 +++++++++++++++++ tests/utils/crawl.py | 27 ++++++++ 8 files changed, 326 insertions(+), 11 deletions(-) create mode 100644 tests/utils/crawl.py diff --git a/scrapy/mail.py b/scrapy/mail.py index 97123e63c..0691312a3 100644 --- a/scrapy/mail.py +++ b/scrapy/mail.py @@ -2,6 +2,8 @@ Mail sending helpers """ +# pragma: no file cover + from __future__ import annotations import logging diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index 925f31ede..1e7ac9c34 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -9,7 +9,7 @@ from __future__ import annotations from typing import TYPE_CHECKING, Any -from scrapy.exceptions import NotConfigured, NotSupported +from scrapy.exceptions import NotSupported from scrapy.http import Response, TextResponse from scrapy.selector import Selector from scrapy.spiders import Spider @@ -76,11 +76,6 @@ class XMLFeedSpider(Spider): yield from self.process_results(response, ret) def _parse(self, response: Response, **kwargs: Any) -> Any: - if not hasattr(self, "parse_node"): - raise NotConfigured( - "You must define parse_node method in order to scrape this XML feed" - ) - response = self.adapt_response(response) nodes: Iterable[Selector] if self.iterator == "iternodes": @@ -158,9 +153,5 @@ class CSVFeedSpider(Spider): yield from self.process_results(response, ret) def _parse(self, response: Response, **kwargs: Any) -> Any: - if not hasattr(self, "parse_row"): - raise NotConfigured( - "You must define parse_row method in order to scrape this CSV feed" - ) response = self.adapt_response(response) return self.parse_rows(response) diff --git a/tests/spiders.py b/tests/spiders.py index da14fdbe3..7c7d3007c 100644 --- a/tests/spiders.py +++ b/tests/spiders.py @@ -38,6 +38,35 @@ class MockServerSpider(Spider): self.is_secure = is_secure +class RawResponseSpider(MockServerSpider): + """Base class for spiders that fetch a response built by the test itself. + + Subclasses return the body from :meth:`raw_body` and request + :attr:`raw_url`, which the mock server answers with that body verbatim + under :attr:`content_type`. This lets tests reach parsing code that only + a specific kind of response triggers while still going through a regular + crawl, instead of calling internal parsing methods directly. + """ + + name = "raw_response" + content_type = "text/plain" + + def raw_body(self) -> str: + raise NotImplementedError + + @property + def raw_url(self) -> str: + assert self.mockserver + raw = ( + "HTTP/1.1 200 OK\r\n" + f"Content-Type: {self.content_type}\r\n" + "Connection: close\r\n" + "\r\n" + f"{self.raw_body()}" + ) + return self.mockserver.url("/raw?" + urlencode({"raw": raw})) + + class MetaSpider(MockServerSpider): name = "meta" @@ -496,6 +525,23 @@ class CrawlSpiderWithErrback(CrawlSpiderWithParseMethod): self.logger.info("[errback] status %i", failure.value.response.status) +class CrawlSpiderWithoutErrback(CrawlSpiderWithParseMethod): + name = "crawl_spider_without_errback" + + async def start(self): + test_body = b""" + + Page title + +

Item 200

+

Item 404

+ + + """ + url = self.mockserver.url("/alpayload") + yield Request(url, method="POST", body=test_body) + + class CrawlSpiderWithProcessRequestCallbackKeywordArguments(CrawlSpiderWithParseMethod): name = "crawl_spider_with_process_request_cb_kwargs" rules = ( diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 9c479068a..d284805be 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -41,6 +41,7 @@ from tests.spiders import ( CrawlSpiderWithAsyncCallback, CrawlSpiderWithAsyncGeneratorCallback, CrawlSpiderWithErrback, + CrawlSpiderWithoutErrback, CrawlSpiderWithParseMethod, CrawlSpiderWithProcessRequestCallbackKeywordArguments, DelaySpider, @@ -505,6 +506,21 @@ class TestCrawlSpider: assert "[errback] status 500" in caplog.text assert "[errback] status 501" in caplog.text + @coroutine_test + async def test_crawlspider_without_errback( + self, caplog: pytest.LogCaptureFixture, mockserver: MockServer + ) -> None: + crawler = get_crawler(CrawlSpiderWithoutErrback) + with caplog.at_level(logging.INFO): + await crawler.crawl_async(mockserver=mockserver) + + # The failing request (404) is followed by a rule without an errback, + # so the failure is dropped silently and the crawl finishes normally. + assert "[parse] status 200 (foo: None)" in caplog.text + assert "[errback]" not in caplog.text + assert crawler.stats + assert crawler.stats.get_value("downloader/response_status_count/404") == 1 + @coroutine_test async def test_crawlspider_process_request_cb_kwargs( self, caplog: pytest.LogCaptureFixture, mockserver: MockServer diff --git a/tests/test_spider.py b/tests/test_spider.py index 03d17199f..38cb8da18 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -1,11 +1,26 @@ from __future__ import annotations +from typing import TYPE_CHECKING + import pytest -from scrapy.http import Response, TextResponse, XmlResponse +from scrapy.http import Request, Response, TextResponse, XmlResponse from scrapy.spiders import CSVFeedSpider, Spider, XMLFeedSpider from tests import get_testdata +from tests.spiders import RawResponseSpider from tests.utils.bases.spider import TestSpiderBase +from tests.utils.crawl import crawl_items +from tests.utils.decorators import coroutine_test + +if TYPE_CHECKING: + from tests.mockserver.http import MockServer + + +class RawFeedSpider(RawResponseSpider): + content_type = "text/xml" + + async def start(self): + yield Request(self.raw_url) class TestSpider(TestSpiderBase): @@ -60,6 +75,89 @@ class TestXMLFeedSpider(TestSpiderBase): }, ], iterator + @coroutine_test + async def test_parse_node_uses_parse_item(self, mockserver: MockServer): + # parse_node falls back to parse_item for backward compatibility. + class _Spider(RawFeedSpider, self.spider_class): # type: ignore[name-defined,misc] + itertag = "item" + + def raw_body(self): + return "1" + + def parse_item(self, response, selector): + return {"id": selector.xpath("id/text()").get()} + + items, _ = await crawl_items(_Spider, mockserver) + assert items == [{"id": "1"}] + + @coroutine_test + async def test_parse_node_not_defined(self, mockserver: MockServer): + class _Spider(RawFeedSpider, self.spider_class): # type: ignore[name-defined,misc] + itertag = "item" + + def raw_body(self): + return "1" + + items, crawler = await crawl_items(_Spider, mockserver) + assert items == [] + assert crawler.stats + assert crawler.stats.get_value("spider_exceptions/NotImplementedError") == 1 + + @coroutine_test + async def test_html_iterator(self, mockserver: MockServer): + class _Spider(RawFeedSpider, self.spider_class): # type: ignore[name-defined,misc] + iterator = "html" + itertag = "item" + content_type = "text/html" + + def raw_body(self): + return ( + "1" + "2" + ) + + def parse_node(self, response, selector): + return {"id": selector.xpath("id/text()").get()} + + items, _ = await crawl_items(_Spider, mockserver) + assert items == [{"id": "1"}, {"id": "2"}] + + @coroutine_test + async def test_unsupported_iterator(self, mockserver: MockServer): + class _Spider(RawFeedSpider, self.spider_class): # type: ignore[name-defined,misc] + iterator = "unsupported" + + def raw_body(self): + return "" + + def parse_node(self, response, selector): + return {} + + items, crawler = await crawl_items(_Spider, mockserver) + assert items == [] + assert crawler.stats + assert crawler.stats.get_value("spider_exceptions/NotSupported") == 1 + + @pytest.mark.parametrize("feed_iterator", ["xml", "html"]) + @coroutine_test + async def test_non_text_response(self, feed_iterator: str, mockserver: MockServer): + # The xml and html iterators require a text response. + class _Spider(RawFeedSpider, self.spider_class): # type: ignore[name-defined,misc] + content_type = "application/octet-stream" + iterator = feed_iterator + + def raw_body(self): + # A binary (non-text) body, so the response is a plain Response. + return "\x00\x01\x02\x03" + + def parse_node(self, response, selector): + return {} + + items, crawler = await crawl_items(_Spider, mockserver) + assert items == [] + assert crawler.stats + assert crawler.stats.get_value("spider_exceptions/ValueError") == 1 + class TestCSVFeedSpider(TestSpiderBase): spider_class = CSVFeedSpider @@ -81,6 +179,36 @@ class TestCSVFeedSpider(TestSpiderBase): assert rows[0] == {"id": "1", "name": "alpha", "value": "foobar"} assert len(rows) == 4 + @coroutine_test + async def test_parse(self, mockserver: MockServer): + class _Spider(RawFeedSpider, self.spider_class): # type: ignore[name-defined,misc] + content_type = "text/csv" + delimiter = "," + quotechar = "'" + + def raw_body(self): + return get_testdata("feeds", "feed-sample6.csv").decode() + + def parse_row(self, response, row): + return row + + items, _ = await crawl_items(_Spider, mockserver) + assert items[0] == {"id": "1", "name": "alpha", "value": "foobar"} + assert len(items) == 4 + + @coroutine_test + async def test_parse_row_not_defined(self, mockserver: MockServer): + class _Spider(RawFeedSpider, self.spider_class): # type: ignore[name-defined,misc] + content_type = "text/csv" + + def raw_body(self): + return "id\n1\n" + + items, crawler = await crawl_items(_Spider, mockserver) + assert items == [] + assert crawler.stats + assert crawler.stats.get_value("spider_exceptions/NotImplementedError") == 1 + class TestNoParseMethodSpider: spider_class = Spider diff --git a/tests/test_spider_crawl.py b/tests/test_spider_crawl.py index f34f9add9..9d5f0548b 100644 --- a/tests/test_spider_crawl.py +++ b/tests/test_spider_crawl.py @@ -12,6 +12,7 @@ from scrapy.linkextractors import LinkExtractor from scrapy.spiders import CrawlSpider, Rule, Spider from scrapy.utils.test import get_crawler from tests.utils.bases.spider import TestSpiderBase +from tests.utils.decorators import coroutine_test class TestCrawlSpider(TestSpiderBase): @@ -293,6 +294,48 @@ class TestCrawlSpider(TestSpiderBase): TextResponse(spider.start_urls, body=b""), None, None ) + @coroutine_test + async def test_parse_with_rules_without_callback(self): + response = HtmlResponse( + "http://example.org/somepage/index.html", body=self.test_body + ) + + class _CrawlSpider(CrawlSpider): + name = "test" + allowed_domains = ["example.org"] + rules = (Rule(),) + + spider = _CrawlSpider.from_crawler(get_crawler(_CrawlSpider)) + results = [ + r async for r in spider.parse_with_rules(response, None, {}, follow=True) + ] + assert [r.url for r in results] == [ + "http://example.org/somepage/item/12.html", + "http://example.org/about.html", + "http://example.org/nofollow.html", + ] + + @coroutine_test + async def test_parse_with_rules_without_following(self): + response = HtmlResponse( + "http://example.org/somepage/index.html", body=self.test_body + ) + item = {"name": "item"} + + class _CrawlSpider(CrawlSpider): + name = "test" + allowed_domains = ["example.org"] + rules = (Rule(),) + + spider = _CrawlSpider.from_crawler(get_crawler(_CrawlSpider)) + results = [ + r + async for r in spider.parse_with_rules( + response, lambda response: [item], {}, follow=False + ) + ] + assert results == [item] + class TestDeprecation: def test_crawl_spider(self): diff --git a/tests/test_spider_sitemap.py b/tests/test_spider_sitemap.py index fd62e0016..2f1ccab81 100644 --- a/tests/test_spider_sitemap.py +++ b/tests/test_spider_sitemap.py @@ -6,6 +6,7 @@ from datetime import datetime from io import BytesIO from logging import WARNING from pathlib import Path +from typing import TYPE_CHECKING import pytest @@ -13,9 +14,30 @@ from scrapy.http import HtmlResponse, Request, Response, TextResponse, XmlRespon from scrapy.spiders import SitemapSpider from scrapy.utils.test import get_crawler from tests import tests_datadir +from tests.spiders import RawResponseSpider from tests.utils.bases.spider import TestSpiderBase +from tests.utils.crawl import crawl_items from tests.utils.decorators import coroutine_test +if TYPE_CHECKING: + from tests.mockserver.http import MockServer + + +class RawSitemapSpider(RawResponseSpider): + """Feeds :meth:`raw_body` to :class:`~scrapy.spiders.SitemapSpider` as a + sitemap, so that it is fetched and followed through a regular crawl. + + Subclasses build the document in :meth:`raw_body`, typically using + :attr:`mockserver` to point ```` entries at real endpoints. + """ + + content_type = "application/xml" + + async def start(self): + self.sitemap_urls = [self.raw_url] + async for request in super().start(): + yield request + class TestSitemapSpider(TestSpiderBase): spider_class = SitemapSpider @@ -253,6 +275,46 @@ Sitemap: /sitemap-relative-url.xml urls = [req.url for req in spider._parse_sitemap(r)] assert urls == result + @coroutine_test + async def test_sitemap_rules_with_callable(self, mockserver: MockServer): + # A sitemap_rules entry may hold a callable instead of a method name. + def parse_item(response): + yield {"url": response.url} + + class _Spider(RawSitemapSpider, self.spider_class): # type: ignore[name-defined,misc] + sitemap_rules = [("", parse_item)] + + def raw_body(self): + loc = self.mockserver.url("/text") + return ( + '' + '' + f"{loc}" + "" + ) + + items, _ = await crawl_items(_Spider, mockserver) + assert items == [{"url": mockserver.url("/text")}] + + @coroutine_test + async def test_sitemap_empty_loc(self, mockserver: MockServer): + class _Spider(RawSitemapSpider, self.spider_class): # type: ignore[name-defined,misc] + def parse(self, response): + yield {"url": response.url} + + def raw_body(self): + loc = self.mockserver.url("/text") + return ( + '' + '' + "" + f"{loc}" + "" + ) + + items, _ = await crawl_items(_Spider, mockserver) + assert items == [{"url": mockserver.url("/text")}] + def test_parse_sitemap_empty_body(self, caplog: pytest.LogCaptureFixture) -> None: r = XmlResponse(url="http://www.example.com/sitemap.xml", body=b"") spider = self.spider_class("example.com") diff --git a/tests/utils/crawl.py b/tests/utils/crawl.py new file mode 100644 index 000000000..4631d909d --- /dev/null +++ b/tests/utils/crawl.py @@ -0,0 +1,27 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from scrapy import signals +from scrapy.utils.test import get_crawler + +if TYPE_CHECKING: + from scrapy.crawler import Crawler + from scrapy.spiders import Spider + from tests.mockserver.http import MockServer + + +async def crawl_items( + spider_cls: type[Spider], mockserver: MockServer, **kwargs: Any +) -> tuple[list[Any], Crawler]: + """Run *spider_cls* against *mockserver* and return the scraped items along + with the crawler, which gives tests access to the resulting stats.""" + items: list[Any] = [] + + def collect(item: Any) -> None: + items.append(item) + + crawler = get_crawler(spider_cls) + crawler.signals.connect(collect, signals.item_scraped) + await crawler.crawl_async(mockserver=mockserver, **kwargs) + return items, crawler From ad816d2b3a00c04c86ac8b3fd85e19a7e1b355f8 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 28 Jul 2026 14:04:54 +0200 Subject: [PATCH 03/25] Improve test coverage for scrapy.cmdline (#7795) --- scrapy/cmdline.py | 6 +- tests/test_commands.py | 244 ++++++++++++++++++++++++++++++---- tests/utils/bases/commands.py | 6 + 3 files changed, 226 insertions(+), 30 deletions(-) diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index 6c306afdb..e6d5ff96a 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -225,13 +225,11 @@ def _run_command(cmd: ScrapyCommand, args: list[str], opts: argparse.Namespace) def _run_command_profiled( cmd: ScrapyCommand, args: list[str], opts: argparse.Namespace ) -> None: - if opts.profile: - sys.stderr.write(f"scrapy: writing cProfile stats to {opts.profile!r}\n") + sys.stderr.write(f"scrapy: writing cProfile stats to {opts.profile!r}\n") loc = locals() p = cProfile.Profile() p.runctx("cmd.run(args, opts)", globals(), loc) - if opts.profile: - p.dump_stats(opts.profile) + p.dump_stats(opts.profile) if __name__ == "__main__": diff --git a/tests/test_commands.py b/tests/test_commands.py index 51f98db1b..3e687e811 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -3,14 +3,12 @@ from __future__ import annotations import argparse import json import sys -from io import StringIO from typing import TYPE_CHECKING -from unittest import mock import pytest import scrapy -from scrapy.cmdline import _pop_command_name, _print_unknown_command_msg +from scrapy.cmdline import _pop_command_name, execute from scrapy.commands import ScrapyCommand, ScrapyHelpFormatter, view from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.settings import Settings @@ -153,12 +151,6 @@ class MySpider(scrapy.Spider): self._append_settings(proj_mod_path, "LOG_LEVEL = 'DEBUG'\n") - @staticmethod - def _append_settings(proj_mod_path: Path, text: str) -> None: - """Add text to the end of the project settings.py.""" - with (proj_mod_path / "settings.py").open("a", encoding="utf-8") as f: - f.write(text) - @staticmethod def _replace_custom_settings( proj_mod_path: Path, spider_name: str, text: str @@ -347,23 +339,223 @@ class TestMiscCommands(TestProjectBase): subdir.mkdir(exist_ok=True) assert call("list", cwd=subdir) == 0 - def test_command_not_found(self) -> None: - na_msg = """ -The list command is not available from this location. -These commands are only available from within a project: check, crawl, edit, list, parse. -""" - not_found_msg = """ -Unknown command: abc -""" - params = [ - ("list", False, na_msg), - ("abc", False, not_found_msg), - ("abc", True, not_found_msg), - ] - for cmdname, inproject, message in params: - with mock.patch("sys.stdout", new=StringIO()) as out: - _print_unknown_command_msg(Settings(), cmdname, inproject) - assert out.getvalue().strip() == message.strip() + +class TestCommandListing(TestProjectBase): + """Tests for the command list that ``scrapy`` prints when called without a + command name.""" + + def test_outside_project(self) -> None: + returncode, out, err = proc() + assert returncode == 0, err + assert f"Scrapy {scrapy.__version__} - no active project" in out + assert "Available commands:" in out + assert "Create new project" in out + assert "More commands available when run from project directory" in out + assert 'Use "scrapy -h" to see more info about a command' in out + + def test_inside_project(self, proj_path: Path) -> None: + returncode, out, err = proc(cwd=proj_path) + assert returncode == 0, err + assert ( + f"Scrapy {scrapy.__version__} - active project: {self.project_name}" in out + ) + assert "List available spiders" in out + assert "More commands available when run from project directory" not in out + + +class TestUnknownCommand(TestProjectBase): + def test_outside_project(self) -> None: + returncode, out, err = proc("abc") + assert returncode == 2, err + assert f"Scrapy {scrapy.__version__} - no active project" in out + assert "Unknown command: abc" in out + assert 'Use "scrapy" to see available commands' in out + + def test_inside_project(self, proj_path: Path) -> None: + returncode, out, err = proc("abc", cwd=proj_path) + assert returncode == 2, err + assert ( + f"Scrapy {scrapy.__version__} - active project: {self.project_name}" in out + ) + assert "Unknown command: abc" in out + + def test_project_only_command_outside_project(self) -> None: + returncode, out, err = proc("list") + assert returncode == 2, err + assert "The list command is not available from this location." in out + assert ( + "These commands are only available from within a project: " + "check, crawl, edit, list, parse." in out + ) + + +class TestCommandsModule(TestProjectBase): + """Tests for commands defined in the module of the COMMANDS_MODULE setting.""" + + @pytest.fixture + def proj_path_with_commands(self, proj_path: Path) -> Path: + commands_path = proj_path / self.project_name / "commands" + commands_path.mkdir() + (commands_path / "__init__.py").touch() + (commands_path / "mycmd.py").write_text( + """ +from scrapy.commands import ScrapyCommand + + +class Command(ScrapyCommand): + requires_crawler_process = False + + def short_desc(self): + return "My custom command" + + def run(self, args, opts): + print("My custom command ran") +""", + encoding="utf-8", + ) + (commands_path / "helpcmd.py").write_text( + """ +from scrapy.commands import ScrapyCommand +from scrapy.exceptions import UsageError + + +class Command(ScrapyCommand): + requires_crawler_process = False + + def short_desc(self): + return "My command that asks for its help message" + + def run(self, args, opts): + raise UsageError +""", + encoding="utf-8", + ) + (commands_path / "silentcmd.py").write_text( + """ +from scrapy.commands import ScrapyCommand +from scrapy.exceptions import UsageError + + +class Command(ScrapyCommand): + requires_crawler_process = False + + def short_desc(self): + return "My command that fails silently" + + def run(self, args, opts): + raise UsageError(print_help=False) +""", + encoding="utf-8", + ) + self._append_settings( + proj_path / self.project_name, + f'\nCOMMANDS_MODULE = "{self.project_name}.commands"\n', + ) + return proj_path + + def test_listed(self, proj_path_with_commands: Path) -> None: + returncode, out, err = proc(cwd=proj_path_with_commands) + assert returncode == 0, err + assert "My custom command" in out + + def test_run(self, proj_path_with_commands: Path) -> None: + returncode, out, err = proc("mycmd", cwd=proj_path_with_commands) + assert returncode == 0, err + assert "My custom command ran" in out + + def test_usage_error(self, proj_path_with_commands: Path) -> None: + """A message-less UsageError makes the help message be printed.""" + returncode, out, err = proc("helpcmd", cwd=proj_path_with_commands) + assert returncode == 2, err + assert "scrapy helpcmd" in out + + def test_usage_error_without_help(self, proj_path_with_commands: Path) -> None: + """A message-less UsageError with print_help disabled prints nothing.""" + returncode, out, err = proc("silentcmd", cwd=proj_path_with_commands) + assert returncode == 2, err + assert not out + + +class TestEntryPointCommands: + """Tests for commands defined in the scrapy.commands entry point group.""" + + @staticmethod + def _write_dist(path: Path, entry_point: str) -> None: + """Write into *path* a package with a command and a function, and the + metadata of an installed distribution that declares *entry_point* in + the scrapy.commands entry point group. + + Since ``python -m scrapy.cmdline`` puts the current working directory + in the import path, running it with *path* as the working directory + makes Scrapy find that entry point. + """ + package_path = path / "mycmds" + package_path.mkdir() + (package_path / "__init__.py").touch() + (package_path / "mycmd.py").write_text( + """ +from scrapy.commands import ScrapyCommand + + +class Command(ScrapyCommand): + requires_crawler_process = False + + def short_desc(self): + return "My entry point command" + + def run(self, args, opts): + print("My entry point command ran") + + +def not_a_command(): + pass +""", + encoding="utf-8", + ) + dist_info_path = path / "mycmds-1.0.dist-info" + dist_info_path.mkdir() + (dist_info_path / "METADATA").write_text( + "Metadata-Version: 2.1\nName: mycmds\nVersion: 1.0\n", encoding="utf-8" + ) + (dist_info_path / "entry_points.txt").write_text( + f"[scrapy.commands]\n{entry_point}\n", encoding="utf-8" + ) + + def test_listed(self, tmp_path: Path) -> None: + self._write_dist(tmp_path, "mycmd = mycmds.mycmd:Command") + returncode, out, err = proc(cwd=tmp_path) + assert returncode == 0, err + assert "My entry point command" in out + + def test_run(self, tmp_path: Path) -> None: + self._write_dist(tmp_path, "mycmd = mycmds.mycmd:Command") + returncode, out, err = proc("mycmd", cwd=tmp_path) + assert returncode == 0, err + assert "My entry point command ran" in out + + def test_not_a_class(self, tmp_path: Path) -> None: + self._write_dist(tmp_path, "mycmd = mycmds.mycmd:not_a_command") + returncode, _, err = proc("version", cwd=tmp_path) + assert returncode == 1 + assert "ValueError: Invalid entry point mycmd" in err + + +class TestExecute: + """Tests for calls to scrapy.cmdline.execute() from Python code, which the + command line does not cover.""" + + def test_argv(self, capsys: pytest.CaptureFixture[str]) -> None: + with pytest.raises(SystemExit) as exc_info: + execute(["scrapy", "version"]) + assert exc_info.value.code == 0 + assert scrapy.__version__ in capsys.readouterr().out + + def test_settings(self, capsys: pytest.CaptureFixture[str]) -> None: + settings = Settings() + with pytest.raises(SystemExit) as exc_info: + execute(["scrapy", "settings", "--get", "BOT_NAME"], settings=settings) + assert exc_info.value.code == 0 + assert capsys.readouterr().out.strip() == "scrapybot" class TestBenchCommand: diff --git a/tests/utils/bases/commands.py b/tests/utils/bases/commands.py index 594544c83..55ef686fa 100644 --- a/tests/utils/bases/commands.py +++ b/tests/utils/bases/commands.py @@ -32,3 +32,9 @@ class TestProjectBase: proj_path = tmp_path / self.project_name copytree(_proj_path_cached, proj_path) return proj_path + + @staticmethod + def _append_settings(proj_mod_path: Path, text: str) -> None: + """Add text to the end of the project settings.py.""" + with (proj_mod_path / "settings.py").open("a", encoding="utf-8") as f: + f.write(text) From e7d8b34e73ef2598b27ea0bca31a292e008fff76 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 28 Jul 2026 17:24:34 +0500 Subject: [PATCH 04/25] Next refactoring pass of test_utils_*. (#7797) --- pyproject.toml | 7 - scrapy/core/downloader/handlers/ftp.py | 3 +- scrapy/http/headers.py | 26 +- scrapy/http/request/__init__.py | 12 +- scrapy/http/response/__init__.py | 23 +- scrapy/http/response/text.py | 12 +- scrapy/utils/datatypes.py | 40 +- scrapy/utils/decorators.py | 17 +- tests/test_utils_asyncgen.py | 24 +- tests/test_utils_asyncio.py | 9 +- tests/test_utils_curl.py | 2 + tests/test_utils_datatypes.py | 60 +-- tests/test_utils_decorators.py | 20 +- tests/test_utils_defer.py | 24 +- tests/test_utils_deprecate.py | 46 +- tests/test_utils_display.py | 16 +- tests/test_utils_gz.py | 2 + tests/test_utils_httpobj.py | 2 + tests/test_utils_misc/__init__.py | 261 +++++------ ...t_return_with_argument_inside_generator.py | 421 +++++++++--------- tests/test_utils_project.py | 10 +- tests/test_utils_python.py | 21 +- tests/test_utils_reactor.py | 2 + tests/test_utils_request.py | 26 +- tests/test_utils_response.py | 6 +- tests/test_utils_serialize.py | 2 + tests/test_utils_signal.py | 3 - tests/test_utils_sitemap.py | 2 + tests/test_utils_template.py | 9 +- tests/test_utils_trackref.py | 6 +- tests/test_utils_url.py | 4 +- tests_typing/test_http_request.mypy-testing | 2 +- tests_typing/test_http_response.mypy-testing | 2 +- 33 files changed, 619 insertions(+), 503 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9b1e64121..576a42e5c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -178,13 +178,6 @@ module = [ "tests.test_squeues", "tests.test_squeues_request", "tests.test_stats", - "tests.test_utils_datatypes", - "tests.test_utils_decorators", - "tests.test_utils_defer", - "tests.test_utils_deprecate", - "tests.test_utils_misc.test_return_with_argument_inside_generator", - "tests.test_utils_python", - "tests.test_utils_request", "tests.utils.bases.http_request", "tests.utils.bases.http_response", "tests.utils.bases.spider", diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 07ff4a74e..29b3e3c0f 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -126,5 +126,4 @@ class FTPDownloadHandler(BaseDownloadHandler): headers = {"local filename": protocol.filename or b"", "size": protocol.size} body = protocol.filename or protocol.body.read() respcls = responsetypes.from_args(url=request.url, body=body) - # hints for Headers-related types may need to be fixed to not use AnyStr - return respcls(url=request.url, status=200, body=body, headers=headers) # type: ignore[arg-type] + return respcls(url=request.url, status=200, body=body, headers=headers) diff --git a/scrapy/http/headers.py b/scrapy/http/headers.py index 34d4ec6f2..b55ef6191 100644 --- a/scrapy/http/headers.py +++ b/scrapy/http/headers.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Mapping -from typing import TYPE_CHECKING, Any, AnyStr, TypeAlias, cast +from typing import TYPE_CHECKING, Any, TypeAlias, cast from w3lib.http import headers_dict_to_raw @@ -25,14 +25,20 @@ class Headers(CaselessDict): def __init__( self, - seq: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None = None, + seq: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None = None, encoding: str = "utf-8", ): self.encoding: str = encoding super().__init__(seq) def update( # type: ignore[override] - self, seq: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] + self, + seq: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]], ) -> None: seq = seq.items() if isinstance(seq, Mapping) else seq iseq: dict[bytes, list[bytes]] = {} @@ -40,7 +46,7 @@ class Headers(CaselessDict): iseq.setdefault(self.normkey(k), []).extend(self.normvalue(v)) super().update(iseq) - def normkey(self, key: AnyStr) -> bytes: # type: ignore[override] + def normkey(self, key: str | bytes) -> bytes: """Normalize key to bytes""" return self._tobytes(key.title()) @@ -67,19 +73,19 @@ class Headers(CaselessDict): return str(x).encode(self.encoding) raise TypeError(f"Unsupported value type: {type(x)}") - def __getitem__(self, key: AnyStr) -> bytes | None: + def __getitem__(self, key: str | bytes) -> bytes | None: try: return cast("list[bytes]", super().__getitem__(key))[-1] except IndexError: return None - def get(self, key: AnyStr, def_val: Any = None) -> bytes | None: + def get(self, key: str | bytes, def_val: Any = None) -> bytes | None: try: return cast("list[bytes]", super().get(key, def_val))[-1] except IndexError: return None - def getlist(self, key: AnyStr, def_val: Any = None) -> list[bytes]: + def getlist(self, key: str | bytes, def_val: Any = None) -> list[bytes]: try: return cast("list[bytes]", super().__getitem__(key)) except KeyError: @@ -87,15 +93,15 @@ class Headers(CaselessDict): return self.normvalue(def_val) return [] - def setlist(self, key: AnyStr, list_: Iterable[_RawValue]) -> None: + def setlist(self, key: str | bytes, list_: Iterable[_RawValue]) -> None: self[key] = list_ def setlistdefault( - self, key: AnyStr, default_list: Iterable[_RawValue] = () + self, key: str | bytes, default_list: Iterable[_RawValue] = () ) -> Any: return self.setdefault(key, default_list) - def appendlist(self, key: AnyStr, value: Iterable[_RawValue]) -> None: + def appendlist(self, key: str | bytes, value: Iterable[_RawValue]) -> None: lst = self.getlist(key) lst.extend(self.normvalue(value)) self[key] = lst diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 73c2e7dd4..57517a65b 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -11,7 +11,6 @@ import inspect from typing import ( TYPE_CHECKING, Any, - AnyStr, Concatenate, NoReturn, TypeAlias, @@ -125,7 +124,10 @@ class Request(object_ref): url: str, callback: CallbackT | None = None, method: str = "GET", - headers: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None = None, + headers: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None = None, body: bytes | str | None = None, cookies: CookiesT | None = None, meta: dict[str, Any] | None = None, @@ -310,7 +312,11 @@ class Request(object_ref): @headers.setter def headers( - self, value: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None + self, + value: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None, ) -> None: if isinstance(value, Headers): self._headers = value diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index 09b1c8b32..f1db11488 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -7,7 +7,7 @@ See documentation in docs/topics/request-response.rst from __future__ import annotations -from typing import TYPE_CHECKING, Any, AnyStr, TypeVar, overload +from typing import TYPE_CHECKING, Any, TypeVar, overload from urllib.parse import urljoin from scrapy.exceptions import NotSupported @@ -72,7 +72,10 @@ class Response(object_ref): self, url: str, status: int = 200, - headers: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None = None, + headers: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None = None, body: bytes = b"", flags: list[str] | None = None, request: Request | None = None, @@ -145,7 +148,11 @@ class Response(object_ref): @headers.setter def headers( - self, value: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None + self, + value: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None, ) -> None: if isinstance(value, Headers): self._headers = value @@ -222,7 +229,10 @@ class Response(object_ref): url: str | Link, callback: CallbackT | None = None, method: str = "GET", - headers: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None = None, + headers: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None = None, body: bytes | str | None = None, cookies: CookiesT | None = None, meta: dict[str, Any] | None = None, @@ -272,7 +282,10 @@ class Response(object_ref): urls: Iterable[str | Link], callback: CallbackT | None = None, method: str = "GET", - headers: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None = None, + headers: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None = None, body: bytes | str | None = None, cookies: CookiesT | None = None, meta: dict[str, Any] | None = None, diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index 6876e35e8..d01e23e47 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -9,7 +9,7 @@ from __future__ import annotations import json from contextlib import suppress -from typing import TYPE_CHECKING, Any, AnyStr, cast +from typing import TYPE_CHECKING, Any, cast from urllib.parse import urljoin import parsel @@ -170,7 +170,10 @@ class TextResponse(Response): url: str | Link | parsel.Selector, callback: CallbackT | None = None, method: str = "GET", - headers: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None = None, + headers: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None = None, body: bytes | str | None = None, cookies: CookiesT | None = None, meta: dict[str, Any] | None = None, @@ -223,7 +226,10 @@ class TextResponse(Response): urls: Iterable[str | Link] | parsel.SelectorList[Any] | None = None, callback: CallbackT | None = None, method: str = "GET", - headers: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None = None, + headers: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None = None, body: bytes | str | None = None, cookies: CookiesT | None = None, meta: dict[str, Any] | None = None, diff --git a/scrapy/utils/datatypes.py b/scrapy/utils/datatypes.py index e761a2474..9a945c61c 100644 --- a/scrapy/utils/datatypes.py +++ b/scrapy/utils/datatypes.py @@ -11,12 +11,12 @@ import warnings import weakref from collections import OrderedDict from collections.abc import Mapping -from typing import TYPE_CHECKING, Any, AnyStr, TypeVar, cast +from typing import TYPE_CHECKING, Any, TypeVar, cast from scrapy.exceptions import ScrapyDeprecationWarning if TYPE_CHECKING: - from collections.abc import Iterable, Sequence + from collections.abc import Container, Iterable # typing.Self requires Python 3.11 from typing_extensions import Self @@ -44,22 +44,25 @@ class CaselessDict(dict): # type: ignore[type-arg] def __init__( self, - seq: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]] | None = None, + seq: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]] + | None = None, ): super().__init__() if seq: self.update(seq) - def __getitem__(self, key: AnyStr) -> Any: + def __getitem__(self, key: str | bytes) -> Any: return dict.__getitem__(self, self.normkey(key)) - def __setitem__(self, key: AnyStr, value: Any) -> None: + def __setitem__(self, key: str | bytes, value: Any) -> None: dict.__setitem__(self, self.normkey(key), self.normvalue(value)) - def __delitem__(self, key: AnyStr) -> None: + def __delitem__(self, key: str | bytes) -> None: dict.__delitem__(self, self.normkey(key)) - def __contains__(self, key: AnyStr) -> bool: # type: ignore[override] + def __contains__(self, key: str | bytes) -> bool: # type: ignore[override] return dict.__contains__(self, self.normkey(key)) has_key = __contains__ @@ -69,7 +72,7 @@ class CaselessDict(dict): # type: ignore[type-arg] copy = __copy__ - def normkey(self, key: AnyStr) -> AnyStr: + def normkey(self, key: str | bytes) -> str | bytes: """Method to normalize dictionary key access""" return key.lower() @@ -77,23 +80,28 @@ class CaselessDict(dict): # type: ignore[type-arg] """Method to normalize values prior to be set""" return value - def get(self, key: AnyStr, def_val: Any = None) -> Any: + def get(self, key: str | bytes, def_val: Any = None) -> Any: return dict.get(self, self.normkey(key), self.normvalue(def_val)) - def setdefault(self, key: AnyStr, def_val: Any = None) -> Any: + def setdefault(self, key: str | bytes, def_val: Any = None) -> Any: return dict.setdefault(self, self.normkey(key), self.normvalue(def_val)) # doesn't fully implement MutableMapping.update() - def update(self, seq: Mapping[AnyStr, Any] | Iterable[tuple[AnyStr, Any]]) -> None: # type: ignore[override] + def update( # type: ignore[override] + self, + seq: Mapping[str, Any] + | Mapping[bytes, Any] + | Iterable[tuple[str | bytes, Any]], + ) -> None: seq = seq.items() if isinstance(seq, Mapping) else seq iseq = ((self.normkey(k), self.normvalue(v)) for k, v in seq) super().update(iseq) @classmethod - def fromkeys(cls, keys: Iterable[AnyStr], value: Any = None) -> Self: # type: ignore[override] - return cls((k, value) for k in keys) # type: ignore[misc] + def fromkeys(cls, keys: Iterable[str | bytes], value: Any = None) -> Self: # type: ignore[override] + return cls((k, value) for k in keys) - def pop(self, key: AnyStr, *args: Any) -> Any: + def pop(self, key: str | bytes, *args: Any) -> Any: return dict.pop(self, self.normkey(key), *args) @@ -205,8 +213,8 @@ class LocalWeakReferencedCache(weakref.WeakKeyDictionary[_KT, _VT | None]): class SequenceExclude: """Object to test if an item is NOT within some sequence.""" - def __init__(self, seq: Sequence[Any]): - self.seq: Sequence[Any] = seq + def __init__(self, seq: Container[Any]): + self.seq: Container[Any] = seq def __contains__(self, item: Any) -> bool: return item not in self.seq diff --git a/scrapy/utils/decorators.py b/scrapy/utils/decorators.py index a5bb6fa24..4960dc27a 100644 --- a/scrapy/utils/decorators.py +++ b/scrapy/utils/decorators.py @@ -19,9 +19,19 @@ _T = TypeVar("_T") _P = ParamSpec("_P") +@overload +def deprecated(use_instead: Callable[_P, _T]) -> Callable[_P, _T]: ... + + +@overload def deprecated( - use_instead: Any = None, -) -> Callable[[Callable[_P, _T]], Callable[_P, _T]]: + use_instead: str | None = None, +) -> Callable[[Callable[_P, _T]], Callable[_P, _T]]: ... + + +def deprecated( + use_instead: Callable[_P, _T] | str | None = None, +) -> Callable[_P, _T] | Callable[[Callable[_P, _T]], Callable[_P, _T]]: """This is a decorator which can be used to mark functions as deprecated. It will result in a warning being emitted when the function is used.""" @@ -38,8 +48,9 @@ def deprecated( return wrapped if callable(use_instead): - deco = deco(use_instead) + func = use_instead use_instead = None + return deco(func) return deco diff --git a/tests/test_utils_asyncgen.py b/tests/test_utils_asyncgen.py index fc4e1c487..1d36a66fc 100644 --- a/tests/test_utils_asyncgen.py +++ b/tests/test_utils_asyncgen.py @@ -1,16 +1,18 @@ +from __future__ import annotations + from scrapy.utils.asyncgen import as_async_generator, collect_asyncgen from tests.utils.decorators import coroutine_test -class TestAsyncgenUtils: - @coroutine_test - async def test_as_async_generator(self): - ag = as_async_generator(range(42)) - results = [i async for i in ag] - assert results == list(range(42)) +@coroutine_test +async def test_as_async_generator(): + ag = as_async_generator(range(42)) + results = [i async for i in ag] + assert results == list(range(42)) - @coroutine_test - async def test_collect_asyncgen(self): - ag = as_async_generator(range(42)) - results = await collect_asyncgen(ag) - assert results == list(range(42)) + +@coroutine_test +async def test_collect_asyncgen(): + ag = as_async_generator(range(42)) + results = await collect_asyncgen(ag) + assert results == list(range(42)) diff --git a/tests/test_utils_asyncio.py b/tests/test_utils_asyncio.py index 7528dc51a..9b7eb22fa 100644 --- a/tests/test_utils_asyncio.py +++ b/tests/test_utils_asyncio.py @@ -20,11 +20,10 @@ if TYPE_CHECKING: from collections.abc import AsyncGenerator -class TestAsyncio: - @coroutine_test - async def test_is_asyncio_available(self, reactor_pytest: str) -> None: - # the result should depend only on the pytest --reactor argument - assert is_asyncio_available() == (reactor_pytest != "default") +@coroutine_test +async def test_is_asyncio_available(reactor_pytest: str) -> None: + # the result should depend only on the pytest --reactor argument + assert is_asyncio_available() == (reactor_pytest != "default") @pytest.mark.only_asyncio diff --git a/tests/test_utils_curl.py b/tests/test_utils_curl.py index fce9fc984..6b30744bb 100644 --- a/tests/test_utils_curl.py +++ b/tests/test_utils_curl.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import warnings from typing import Any diff --git a/tests/test_utils_datatypes.py b/tests/test_utils_datatypes.py index f43d20e69..af203ca61 100644 --- a/tests/test_utils_datatypes.py +++ b/tests/test_utils_datatypes.py @@ -1,7 +1,9 @@ +from __future__ import annotations + import copy from abc import ABC, abstractmethod from collections.abc import Iterator, Mapping, MutableMapping -from typing import Any +from typing import Any, Generic, TypeVar import pytest @@ -16,11 +18,13 @@ from scrapy.utils.datatypes import ( ) from scrapy.utils.python import garbage_collect +_DictT = TypeVar("_DictT", bound="CaselessDict | CaseInsensitiveDict") -class TestCaseInsensitiveDictBase(ABC): + +class TestCaseInsensitiveDictBase(ABC, Generic[_DictT]): @property @abstractmethod - def dict_class(self) -> type[MutableMapping[str, Any]]: + def dict_class(self) -> type[_DictT]: raise NotImplementedError def test_init_dict(self): @@ -36,17 +40,17 @@ class TestCaseInsensitiveDictBase(ABC): assert d["black"] == 3 def test_init_mapping(self): - class MyMapping(Mapping): - def __init__(self, **kwargs): + class MyMapping(Mapping[str, int]): + def __init__(self, **kwargs: int) -> None: self._d = kwargs - def __getitem__(self, key): + def __getitem__(self, key: str) -> int: return self._d[key] - def __iter__(self): + def __iter__(self) -> Iterator[str]: return iter(self._d) - def __len__(self): + def __len__(self) -> int: return len(self._d) seq = MyMapping(red=1, black=3) @@ -55,23 +59,23 @@ class TestCaseInsensitiveDictBase(ABC): assert d["black"] == 3 def test_init_mutable_mapping(self): - class MyMutableMapping(MutableMapping): - def __init__(self, **kwargs): + class MyMutableMapping(MutableMapping[str, int]): + def __init__(self, **kwargs: int) -> None: self._d = kwargs - def __getitem__(self, key): + def __getitem__(self, key: str) -> int: return self._d[key] - def __setitem__(self, key, value): + def __setitem__(self, key: str, value: int) -> None: self._d[key] = value - def __delitem__(self, key): + def __delitem__(self, key: str) -> None: del self._d[key] - def __iter__(self): + def __iter__(self) -> Iterator[str]: return iter(self._d) - def __len__(self): + def __len__(self) -> int: return len(self._d) seq = MyMutableMapping(red=1, black=3) @@ -149,7 +153,7 @@ class TestCaseInsensitiveDictBase(ABC): d.pop("A") def test_normkey(self): - class MyDict(self.dict_class): + class MyDict(self.dict_class): # type: ignore[misc,name-defined] def _normkey(self, key): return key.title() @@ -160,7 +164,7 @@ class TestCaseInsensitiveDictBase(ABC): assert list(d.keys()) == ["Key-One"] def test_normvalue(self): - class MyDict(self.dict_class): + class MyDict(self.dict_class): # type: ignore[misc,name-defined] def _normvalue(self, value): if value is not None: return value + 1 @@ -214,8 +218,8 @@ class TestCaseInsensitiveDictBase(ABC): assert dict(h1) == {"header1": "value1", "header2": "value2"} -class TestCaseInsensitiveDict(TestCaseInsensitiveDictBase): - dict_class = CaseInsensitiveDict # type: ignore[assignment] +class TestCaseInsensitiveDict(TestCaseInsensitiveDictBase[CaseInsensitiveDict]): + dict_class = CaseInsensitiveDict def test_repr(self): d1 = self.dict_class({"foo": "bar"}) @@ -230,7 +234,7 @@ class TestCaseInsensitiveDict(TestCaseInsensitiveDictBase): assert list(iterkeys) == ["AsDf", "FoO"] def test_copy_keeps_values(self): - class MyDict(self.dict_class): + class MyDict(self.dict_class): # type: ignore[misc,name-defined] def _normvalue(self, value): return value + 1 @@ -253,7 +257,7 @@ class TestCaseInsensitiveDict(TestCaseInsensitiveDictBase): @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") -class TestCaselessDict(TestCaseInsensitiveDictBase): +class TestCaselessDict(TestCaseInsensitiveDictBase[CaselessDict]): dict_class = CaselessDict def test_deprecation_message(self): @@ -319,7 +323,7 @@ class TestSequenceExclude: class TestLocalCache: def test_cache_with_limit(self): - cache = LocalCache(limit=2) + cache: LocalCache[str, int] = LocalCache(limit=2) cache["a"] = 1 cache["b"] = 2 cache["c"] = 3 @@ -332,7 +336,7 @@ class TestLocalCache: def test_cache_without_limit(self): maximum = 10**4 - cache = LocalCache() + cache: LocalCache[str, int] = LocalCache() for x in range(maximum): cache[str(x)] = x assert len(cache) == maximum @@ -341,7 +345,7 @@ class TestLocalCache: assert cache[str(x)] == x def test_cache_with_zero_limit(self): - cache = LocalCache(limit=0) + cache: LocalCache[str, int] = LocalCache(limit=0) cache["a"] = 1 cache["b"] = 2 cache["c"] = 3 @@ -353,7 +357,9 @@ class TestLocalCache: class TestLocalWeakReferencedCache: def test_cache_with_limit(self): - cache = LocalWeakReferencedCache(limit=2) + cache: LocalWeakReferencedCache[Request, int] = LocalWeakReferencedCache( + limit=2 + ) r1 = Request("https://example.org") r2 = Request("https://example.com") r3 = Request("https://example.net") @@ -375,7 +381,7 @@ class TestLocalWeakReferencedCache: assert len(cache) == 1 def test_cache_non_weak_referenceable_objects(self): - cache = LocalWeakReferencedCache() + cache: LocalWeakReferencedCache[Any, int] = LocalWeakReferencedCache() k1 = None k2 = 1 k3 = [1, 2, 3] @@ -389,7 +395,7 @@ class TestLocalWeakReferencedCache: def test_cache_without_limit(self): maximum = 10**4 - cache = LocalWeakReferencedCache() + cache: LocalWeakReferencedCache[Request, int] = LocalWeakReferencedCache() refs = [] for x in range(maximum): refs.append(Request(f"https://example.org/{x}")) diff --git a/tests/test_utils_decorators.py b/tests/test_utils_decorators.py index 9743e1a50..807294a57 100644 --- a/tests/test_utils_decorators.py +++ b/tests/test_utils_decorators.py @@ -1,6 +1,7 @@ from __future__ import annotations import warnings +from typing import TYPE_CHECKING import pytest from twisted.internet.defer import Deferred @@ -10,11 +11,14 @@ from scrapy.utils.decorators import _warn_spider_arg, deprecated, inthread from scrapy.utils.defer import maybe_deferred_to_future from tests.utils.decorators import coroutine_test +if TYPE_CHECKING: + from collections.abc import AsyncGenerator + class TestDeprecated: def test_warns_and_still_calls(self): @deprecated() - def add(a, b): + def add(a: int, b: int) -> int: return a + b with pytest.warns( @@ -26,7 +30,7 @@ class TestDeprecated: def test_use_instead_in_message(self): @deprecated(use_instead="other_function") - def old(): + def old() -> None: return None with pytest.warns( @@ -37,7 +41,7 @@ class TestDeprecated: def test_applied_without_parentheses(self): @deprecated - def square(x): + def square(x: int) -> int: return x * x with pytest.warns( @@ -65,7 +69,7 @@ class TestInthread: class TestWarnSpiderArg: def test_sync_warns_with_spider_arg(self): @_warn_spider_arg - def parse(response, spider=None): + def parse(response: str, spider: str | None = None) -> str: return response with pytest.warns( @@ -75,7 +79,7 @@ class TestWarnSpiderArg: def test_sync_no_warning_without_spider_arg(self): @_warn_spider_arg - def parse(response, spider=None): + def parse(response: str, spider: str | None = None) -> str: return response with warnings.catch_warnings(): @@ -85,7 +89,7 @@ class TestWarnSpiderArg: @coroutine_test async def test_async_warns_with_spider_arg(self): @_warn_spider_arg - async def parse(response, spider=None): + async def parse(response: str, spider: str | None = None) -> str: return response with pytest.warns( @@ -96,7 +100,9 @@ class TestWarnSpiderArg: @coroutine_test async def test_asyncgen_warns_with_spider_arg(self): @_warn_spider_arg - async def parse(response, spider=None): + async def parse( + response: str, spider: str | None = None + ) -> AsyncGenerator[str]: yield response with pytest.warns( diff --git a/tests/test_utils_defer.py b/tests/test_utils_defer.py index 978c24f5a..175a4fe03 100644 --- a/tests/test_utils_defer.py +++ b/tests/test_utils_defer.py @@ -24,6 +24,8 @@ from tests.utils.decorators import coroutine_test, inline_callbacks_test if TYPE_CHECKING: from collections.abc import AsyncGenerator, Awaitable, Callable, Generator + from twisted.python.failure import Failure + @pytest.mark.requires_reactor # mustbe_deferred() requires a reactor @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") @@ -70,7 +72,7 @@ class TestIterErrback: def itergood() -> Generator[int, None, None]: yield from range(10) - errors = [] + errors: list[Failure] = [] out = list(iter_errback(itergood(), errors.append)) assert out == list(range(10)) assert not errors @@ -82,7 +84,7 @@ class TestIterErrback: 1 / 0 yield x - errors = [] + errors: list[Failure] = [] out = list(iter_errback(iterbad(), errors.append)) assert out == [0, 1, 2, 3, 4] assert len(errors) == 1 @@ -96,7 +98,7 @@ class TestAiterErrback: for x in range(10): yield x - errors = [] + errors: list[Failure] = [] out = await collect_asyncgen(aiter_errback(itergood(), errors.append)) assert out == list(range(10)) assert not errors @@ -109,7 +111,7 @@ class TestAiterErrback: 1 / 0 yield x - errors = [] + errors: list[Failure] = [] out = await collect_asyncgen(aiter_errback(iterbad(), errors.append)) assert out == [0, 1, 2, 3, 4] assert len(errors) == 1 @@ -202,7 +204,7 @@ class TestParallelAsync: for length in [20, 50, 100]: parallel_count = [0] max_parallel_count = [0] - results = [] + results: list[int] = [] ait = self.get_async_iterable(length) dl = parallel_async( ait, @@ -222,7 +224,7 @@ class TestParallelAsync: for length in [20, 50, 100]: parallel_count = [0] max_parallel_count = [0] - results = [] + results: list[int] = [] ait = self.get_async_iterable_with_delays(length) dl = parallel_async( ait, @@ -240,7 +242,7 @@ class TestParallelAsync: class TestDeferredFromCoro: def test_deferred(self): - d = Deferred() + d: Deferred[None] = Deferred() result = deferred_from_coro(d) assert isinstance(result, Deferred) assert result is d @@ -274,7 +276,7 @@ class TestDeferredFromCoro: @pytest.mark.only_asyncio @inline_callbacks_test def test_future(self): - future = Future() + future: Future[int] = Future() result = deferred_from_coro(future) assert isinstance(result, Deferred) future.set_result(42) @@ -324,7 +326,7 @@ class TestDeferredFFromCoroF: class TestDeferredToFuture: @coroutine_test async def test_deferred(self): - d = Deferred() + d: Deferred[int] = Deferred() result = deferred_to_future(d) assert isinstance(result, Future) d.callback(42) @@ -359,7 +361,7 @@ class TestDeferredToFuture: class TestMaybeDeferredToFutureAsyncio: @coroutine_test async def test_deferred(self): - d = Deferred() + d: Deferred[int] = Deferred() result = maybe_deferred_to_future(d) assert isinstance(result, Future) d.callback(42) @@ -394,7 +396,7 @@ class TestMaybeDeferredToFutureAsyncio: class TestMaybeDeferredToFutureNotAsyncio: @coroutine_test async def test_deferred(self): - d = Deferred() + d: Deferred[int] = Deferred() result = maybe_deferred_to_future(d) assert isinstance(result, Deferred) assert result is d diff --git a/tests/test_utils_deprecate.py b/tests/test_utils_deprecate.py index 0706fec99..4c8585916 100644 --- a/tests/test_utils_deprecate.py +++ b/tests/test_utils_deprecate.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import inspect import warnings from unittest import mock @@ -38,7 +40,7 @@ class TestWarnWhenSubclassed: ) with pytest.warns(MyWarning, match=msg) as w: - class UserClass(Deprecated): + class UserClass(Deprecated): # type: ignore[misc, valid-type] pass assert w[0].lineno == inspect.getsourcelines(UserClass)[1] @@ -57,7 +59,7 @@ class TestWarnWhenSubclassed: match=r"UserClass inherits from deprecated class bar\.OldClass, please inherit from foo\.NewClass", ): - class UserClass(Deprecated): + class UserClass(Deprecated): # type: ignore[misc, valid-type] pass with pytest.warns( @@ -76,7 +78,7 @@ class TestWarnWhenSubclassed: match="UserClass inherits from deprecated class", ): - class UserClass(Deprecated): + class UserClass(Deprecated): # type: ignore[misc, valid-type] pass with warnings.catch_warnings(): @@ -95,16 +97,16 @@ class TestWarnWhenSubclassed: match="UserClass inherits from deprecated class", ): - class UserClass(Deprecated): + class UserClass(Deprecated): # type: ignore[misc, valid-type] pass with warnings.catch_warnings(): warnings.simplefilter("error", MyWarning) - class FooClass(Deprecated): + class FooClass(Deprecated): # type: ignore[misc, valid-type] pass - class BarClass(Deprecated): + class BarClass(Deprecated): # type: ignore[misc, valid-type] pass def test_warning_on_instance(self): @@ -112,22 +114,20 @@ class TestWarnWhenSubclassed: "Deprecated", NewName, warn_category=MyWarning ) - with pytest.warns(MyWarning) as w: - _, lineno = Deprecated(), inspect.getlineno(inspect.currentframe()) - - w = [x for x in w if x.category is MyWarning] + with pytest.warns( + MyWarning, + match=r"tests\.test_utils_deprecate\.Deprecated is deprecated, " + r"instantiate tests\.test_utils_deprecate\.NewName instead\.", + ) as w: + _, lineno = Deprecated(), inspect.getlineno(inspect.currentframe()) # type: ignore[arg-type] assert len(w) == 1 - assert ( - str(w[0].message) == "tests.test_utils_deprecate.Deprecated is deprecated, " - "instantiate tests.test_utils_deprecate.NewName instead." - ) assert w[0].lineno == lineno # ignore subclassing warnings with warnings.catch_warnings(): warnings.simplefilter("ignore", MyWarning) - class UserClass(Deprecated): + class UserClass(Deprecated): # type: ignore[misc, valid-type] pass with warnings.catch_warnings(): @@ -141,7 +141,7 @@ class TestWarnWhenSubclassed: match=r"UserClass2 inherits from deprecated class tests\.test_utils_deprecate\.Deprecated, please inherit from tests\.test_utils_deprecate\.NewName", ): - class UserClass2(Deprecated): + class UserClass2(Deprecated): # type: ignore[misc, valid-type] pass def test_issubclass(self): @@ -155,10 +155,10 @@ class TestWarnWhenSubclassed: class UpdatedUserClass1a(NewName): pass - class OutdatedUserClass1(DeprecatedName): + class OutdatedUserClass1(DeprecatedName): # type: ignore[misc, valid-type] pass - class OutdatedUserClass1a(DeprecatedName): + class OutdatedUserClass1a(DeprecatedName): # type: ignore[misc, valid-type] pass class UnrelatedClass: @@ -174,7 +174,7 @@ class TestWarnWhenSubclassed: assert not issubclass(OutdatedUserClass1a, OutdatedUserClass1) with pytest.raises(TypeError): - issubclass(object(), DeprecatedName) + issubclass(object(), DeprecatedName) # type: ignore[arg-type] def test_isinstance(self): with warnings.catch_warnings(): @@ -187,10 +187,10 @@ class TestWarnWhenSubclassed: class UpdatedUserClass2a(NewName): pass - class OutdatedUserClass2(DeprecatedName): + class OutdatedUserClass2(DeprecatedName): # type: ignore[misc, valid-type] pass - class OutdatedUserClass2a(DeprecatedName): + class OutdatedUserClass2a(DeprecatedName): # type: ignore[misc, valid-type] pass class UnrelatedClass: @@ -211,7 +211,7 @@ class TestWarnWhenSubclassed: warnings.simplefilter("ignore", ScrapyDeprecationWarning) Deprecated = create_deprecated_class("Deprecated", NewName, {"foo": "bar"}) - assert Deprecated.foo == "bar" + assert Deprecated.foo == "bar" # type: ignore[attr-defined] def test_deprecate_a_class_with_custom_metaclass(self): Meta1 = type("Meta1", (type,), {}) @@ -242,7 +242,7 @@ class TestWarnWhenSubclassed: match=r"UserClass inherits from deprecated class tests\.test_utils_deprecate\.AlsoDeprecated, please inherit from foo\.Bar", ): - class UserClass(AlsoDeprecated): + class UserClass(AlsoDeprecated): # type: ignore[misc, valid-type] pass def test_inspect_stack(self): diff --git a/tests/test_utils_display.py b/tests/test_utils_display.py index 9f9e24957..a87816c20 100644 --- a/tests/test_utils_display.py +++ b/tests/test_utils_display.py @@ -31,13 +31,13 @@ plain_string = "{'a': 1}" @mock.patch("sys.platform", "linux") @mock.patch("sys.stdout.isatty") -def test_pformat(isatty): +def test_pformat(isatty: mock.Mock) -> None: isatty.return_value = True assert pformat(value) in colorized_strings @mock.patch("sys.stdout.isatty") -def test_pformat_dont_colorize(isatty): +def test_pformat_dont_colorize(isatty: mock.Mock) -> None: isatty.return_value = True assert pformat(value, colorize=False) == plain_string @@ -49,7 +49,7 @@ def test_pformat_not_tty(): @mock.patch("sys.platform", "win32") @mock.patch("platform.version") @mock.patch("sys.stdout.isatty") -def test_pformat_old_windows(isatty, version): +def test_pformat_old_windows(isatty: mock.Mock, version: mock.Mock) -> None: isatty.return_value = True version.return_value = "10.0.14392" assert pformat(value) in colorized_strings @@ -59,7 +59,9 @@ def test_pformat_old_windows(isatty, version): @mock.patch("scrapy.utils.display._enable_windows_terminal_processing") @mock.patch("platform.version") @mock.patch("sys.stdout.isatty") -def test_pformat_windows_no_terminal_processing(isatty, version, terminal_processing): +def test_pformat_windows_no_terminal_processing( + isatty: mock.Mock, version: mock.Mock, terminal_processing: mock.Mock +) -> None: isatty.return_value = True version.return_value = "10.0.14393" terminal_processing.return_value = False @@ -70,7 +72,9 @@ def test_pformat_windows_no_terminal_processing(isatty, version, terminal_proces @mock.patch("scrapy.utils.display._enable_windows_terminal_processing") @mock.patch("platform.version") @mock.patch("sys.stdout.isatty") -def test_pformat_windows(isatty, version, terminal_processing): +def test_pformat_windows( + isatty: mock.Mock, version: mock.Mock, terminal_processing: mock.Mock +) -> None: isatty.return_value = True version.return_value = "10.0.14393" terminal_processing.return_value = True @@ -79,7 +83,7 @@ def test_pformat_windows(isatty, version, terminal_processing): @mock.patch("sys.platform", "linux") @mock.patch("sys.stdout.isatty") -def test_pformat_no_pygments(isatty): +def test_pformat_no_pygments(isatty: mock.Mock) -> None: isatty.return_value = True real_import = builtins.__import__ diff --git a/tests/test_utils_gz.py b/tests/test_utils_gz.py index 06fdf9cba..75f8be6f6 100644 --- a/tests/test_utils_gz.py +++ b/tests/test_utils_gz.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from gzip import BadGzipFile from pathlib import Path diff --git a/tests/test_utils_httpobj.py b/tests/test_utils_httpobj.py index 0eb330461..610c463ec 100644 --- a/tests/test_utils_httpobj.py +++ b/tests/test_utils_httpobj.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from urllib.parse import urlparse from scrapy.http import Request diff --git a/tests/test_utils_misc/__init__.py b/tests/test_utils_misc/__init__.py index ab6965ed5..0775ec608 100644 --- a/tests/test_utils_misc/__init__.py +++ b/tests/test_utils_misc/__init__.py @@ -20,150 +20,157 @@ from scrapy.utils.misc import ( ) -class TestUtilsMisc: - def test_load_object_class(self): - obj = load_object(Field) - assert obj is Field - obj = load_object("scrapy.item.Field") - assert obj is Field +def test_load_object_class() -> None: + obj = load_object(Field) + assert obj is Field + obj = load_object("scrapy.item.Field") + assert obj is Field - def test_load_object_function(self): - obj = load_object(load_object) - assert obj is load_object - obj = load_object("scrapy.utils.misc.load_object") - assert obj is load_object - def test_load_object_exceptions(self): - with pytest.raises(ImportError): - load_object("nomodule999.mod.function") - with pytest.raises(NameError): - load_object("scrapy.utils.misc.load_object999") - with pytest.raises(TypeError): - load_object({}) # type: ignore[arg-type] +def test_load_object_function() -> None: + obj = load_object(load_object) + assert obj is load_object + obj = load_object("scrapy.utils.misc.load_object") + assert obj is load_object - def test_walk_modules(self): - mods = walk_modules_iter("tests.test_utils_misc.test_walk_modules") + +def test_load_object_exceptions() -> None: + with pytest.raises(ImportError): + load_object("nomodule999.mod.function") + with pytest.raises(NameError): + load_object("scrapy.utils.misc.load_object999") + with pytest.raises(TypeError): + load_object({}) # type: ignore[arg-type] + + +def test_walk_modules() -> None: + mods = walk_modules_iter("tests.test_utils_misc.test_walk_modules") + expected = [ + "tests.test_utils_misc.test_walk_modules", + "tests.test_utils_misc.test_walk_modules.mod", + "tests.test_utils_misc.test_walk_modules.mod.mod0", + "tests.test_utils_misc.test_walk_modules.mod1", + ] + assert {m.__name__ for m in mods} == set(expected) + + mods = walk_modules_iter("tests.test_utils_misc.test_walk_modules.mod") + expected = [ + "tests.test_utils_misc.test_walk_modules.mod", + "tests.test_utils_misc.test_walk_modules.mod.mod0", + ] + assert {m.__name__ for m in mods} == set(expected) + + mods = walk_modules_iter("tests.test_utils_misc.test_walk_modules.mod1") + expected = [ + "tests.test_utils_misc.test_walk_modules.mod1", + ] + assert {m.__name__ for m in mods} == set(expected) + + with pytest.raises(ImportError): + for _ in walk_modules_iter("nomodule999"): + pass + with ( + pytest.raises(ImportError), + pytest.warns( + ScrapyDeprecationWarning, + match="The scrapy.utils.misc.walk_modules function is deprecated and will be " + "removed in a future version of Scrapy. " + "Use scrapy.utils.misc.walk_modules_iter instead.", + ), + ): + walk_modules("nomodule999") + + +def test_walk_modules_egg() -> None: + egg = str(Path(__file__).parent / "test.egg") + sys.path.append(egg) + try: + mods = walk_modules_iter("testegg") expected = [ - "tests.test_utils_misc.test_walk_modules", - "tests.test_utils_misc.test_walk_modules.mod", - "tests.test_utils_misc.test_walk_modules.mod.mod0", - "tests.test_utils_misc.test_walk_modules.mod1", + "testegg.spiders", + "testegg.spiders.a", + "testegg.spiders.b", + "testegg", ] assert {m.__name__ for m in mods} == set(expected) + finally: + sys.path.remove(egg) - mods = walk_modules_iter("tests.test_utils_misc.test_walk_modules.mod") - expected = [ - "tests.test_utils_misc.test_walk_modules.mod", - "tests.test_utils_misc.test_walk_modules.mod.mod0", - ] - assert {m.__name__ for m in mods} == set(expected) - mods = walk_modules_iter("tests.test_utils_misc.test_walk_modules.mod1") - expected = [ - "tests.test_utils_misc.test_walk_modules.mod1", - ] - assert {m.__name__ for m in mods} == set(expected) +def test_arg_to_iter() -> None: + class TestItem(Item): + name = Field() - with pytest.raises(ImportError): - for _ in walk_modules_iter("nomodule999"): - pass - with ( - pytest.raises(ImportError), - pytest.warns( - ScrapyDeprecationWarning, - match="The scrapy.utils.misc.walk_modules function is deprecated and will be " - "removed in a future version of Scrapy. " - "Use scrapy.utils.misc.walk_modules_iter instead.", - ), - ): - walk_modules("nomodule999") + assert hasattr(arg_to_iter(None), "__iter__") + assert hasattr(arg_to_iter(100), "__iter__") + assert hasattr(arg_to_iter("lala"), "__iter__") + assert hasattr(arg_to_iter([1, 2, 3]), "__iter__") + assert hasattr(arg_to_iter(c for c in "abcd"), "__iter__") - def test_walk_modules_egg(self): - egg = str(Path(__file__).parent / "test.egg") - sys.path.append(egg) - try: - mods = walk_modules_iter("testegg") - expected = [ - "testegg.spiders", - "testegg.spiders.a", - "testegg.spiders.b", - "testegg", - ] - assert {m.__name__ for m in mods} == set(expected) - finally: - sys.path.remove(egg) + assert not list(arg_to_iter(None)) + assert list(arg_to_iter("lala")) == ["lala"] + assert list(arg_to_iter(100)) == [100] + assert list(arg_to_iter(c for c in "abc")) == ["a", "b", "c"] + assert list(arg_to_iter([1, 2, 3])) == [1, 2, 3] + assert list(arg_to_iter({"a": 1})) == [{"a": 1}] + assert list(arg_to_iter(TestItem(name="john"))) == [TestItem(name="john")] - def test_arg_to_iter(self): - class TestItem(Item): - name = Field() - assert hasattr(arg_to_iter(None), "__iter__") - assert hasattr(arg_to_iter(100), "__iter__") - assert hasattr(arg_to_iter("lala"), "__iter__") - assert hasattr(arg_to_iter([1, 2, 3]), "__iter__") - assert hasattr(arg_to_iter(c for c in "abcd"), "__iter__") +def test_build_from_crawler() -> None: + crawler = mock.MagicMock(spec_set=["settings"]) + args = (True, 100.0) + kwargs = {"key": "val"} - assert not list(arg_to_iter(None)) - assert list(arg_to_iter("lala")) == ["lala"] - assert list(arg_to_iter(100)) == [100] - assert list(arg_to_iter(c for c in "abc")) == ["a", "b", "c"] - assert list(arg_to_iter([1, 2, 3])) == [1, 2, 3] - assert list(arg_to_iter({"a": 1})) == [{"a": 1}] - assert list(arg_to_iter(TestItem(name="john"))) == [TestItem(name="john")] + def _test_with_crawler(mock: mock.MagicMock, crawler: mock.MagicMock) -> None: + build_from_crawler(mock, crawler, *args, **kwargs) + if hasattr(mock, "from_crawler"): + mock.from_crawler.assert_called_once_with(crawler, *args, **kwargs) + assert mock.call_count == 0 + else: + mock.assert_called_once_with(*args, **kwargs) - def test_build_from_crawler(self): - crawler = mock.MagicMock(spec_set=["settings"]) - args = (True, 100.0) - kwargs = {"key": "val"} + # Check usage of correct constructor using 2 mocks: + # 1. with no alternative constructors + # 2. with from_crawler() constructor + spec_sets = ( + ["__qualname__"], + ["__qualname__", "from_crawler"], + ) + for specs in spec_sets: + m = mock.MagicMock(spec_set=specs) + _test_with_crawler(m, crawler) + m.reset_mock() - def _test_with_crawler(mock: mock.MagicMock, crawler: mock.MagicMock) -> None: - build_from_crawler(mock, crawler, *args, **kwargs) - if hasattr(mock, "from_crawler"): - mock.from_crawler.assert_called_once_with(crawler, *args, **kwargs) - assert mock.call_count == 0 - else: - mock.assert_called_once_with(*args, **kwargs) + # Check adoption of crawler + m = mock.MagicMock(spec_set=["__qualname__", "from_crawler"]) + m.from_crawler.return_value = None + with pytest.raises(TypeError): + build_from_crawler(m, crawler, *args, **kwargs) - # Check usage of correct constructor using 2 mocks: - # 1. with no alternative constructors - # 2. with from_crawler() constructor - spec_sets = ( - ["__qualname__"], - ["__qualname__", "from_crawler"], - ) - for specs in spec_sets: - m = mock.MagicMock(spec_set=specs) - _test_with_crawler(m, crawler) - m.reset_mock() - # Check adoption of crawler - m = mock.MagicMock(spec_set=["__qualname__", "from_crawler"]) - m.from_crawler.return_value = None - with pytest.raises(TypeError): - build_from_crawler(m, crawler, *args, **kwargs) +def test_set_environ() -> None: + assert os.environ.get("some_test_environ") is None + with set_environ(some_test_environ="test_value"): + assert os.environ.get("some_test_environ") == "test_value" + assert os.environ.get("some_test_environ") is None - def test_set_environ(self): - assert os.environ.get("some_test_environ") is None - with set_environ(some_test_environ="test_value"): - assert os.environ.get("some_test_environ") == "test_value" - assert os.environ.get("some_test_environ") is None + os.environ["some_test_environ"] = "test" + assert os.environ.get("some_test_environ") == "test" + with set_environ(some_test_environ="test_value"): + assert os.environ.get("some_test_environ") == "test_value" + assert os.environ.get("some_test_environ") == "test" - os.environ["some_test_environ"] = "test" - assert os.environ.get("some_test_environ") == "test" - with set_environ(some_test_environ="test_value"): - assert os.environ.get("some_test_environ") == "test_value" - assert os.environ.get("some_test_environ") == "test" - def test_rel_has_nofollow(self): - assert rel_has_nofollow("ugc nofollow") is True - assert rel_has_nofollow("ugc,nofollow") is True - assert rel_has_nofollow("ugc") is False - assert rel_has_nofollow("nofollow") is True - assert rel_has_nofollow("nofollowfoo") is False - assert rel_has_nofollow("foonofollow") is False - assert rel_has_nofollow("ugc, , nofollow") is True - # rel attribute values are ASCII case-insensitive per the HTML spec - assert rel_has_nofollow("NoFollow") is True - assert rel_has_nofollow("NOFOLLOW") is True - assert rel_has_nofollow("UGC NoFollow") is True - assert rel_has_nofollow("ugc,NoFollow") is True +def test_rel_has_nofollow() -> None: + assert rel_has_nofollow("ugc nofollow") is True + assert rel_has_nofollow("ugc,nofollow") is True + assert rel_has_nofollow("ugc") is False + assert rel_has_nofollow("nofollow") is True + assert rel_has_nofollow("nofollowfoo") is False + assert rel_has_nofollow("foonofollow") is False + assert rel_has_nofollow("ugc, , nofollow") is True + # rel attribute values are ASCII case-insensitive per the HTML spec + assert rel_has_nofollow("NoFollow") is True + assert rel_has_nofollow("NOFOLLOW") is True + assert rel_has_nofollow("UGC NoFollow") is True + assert rel_has_nofollow("ugc,NoFollow") is True diff --git a/tests/test_utils_misc/test_return_with_argument_inside_generator.py b/tests/test_utils_misc/test_return_with_argument_inside_generator.py index 1acc3aac2..7343a135a 100644 --- a/tests/test_utils_misc/test_return_with_argument_inside_generator.py +++ b/tests/test_utils_misc/test_return_with_argument_inside_generator.py @@ -1,5 +1,8 @@ +from __future__ import annotations + import warnings from functools import partial +from typing import TYPE_CHECKING, Any from unittest import mock import pytest @@ -9,6 +12,11 @@ from scrapy.utils.misc import ( warn_on_generator_with_return_value, ) +if TYPE_CHECKING: + from collections.abc import Generator + + from scrapy import Spider + def _indentation_error(*args, **kwargs): raise IndentationError @@ -35,244 +43,239 @@ https://example.org yield url -def generator_that_returns_stuff(): +def generator_that_returns_stuff() -> Generator[int, None, int]: yield 1 yield 2 return 3 -class TestUtilsMisc: - @pytest.fixture - def mock_spider(self): - class MockSettings: - def __init__(self, settings_dict=None): - self.settings_dict = settings_dict or { - "WARN_ON_GENERATOR_RETURN_VALUE": True - } +@pytest.fixture +def mock_spider() -> Spider: + class MockSettings: + def __init__(self, settings_dict: dict[str, Any] | None = None): + self.settings_dict = settings_dict or { + "WARN_ON_GENERATOR_RETURN_VALUE": True + } - def getbool(self, name, default=False): - return self.settings_dict.get(name, default) + def getbool(self, name, default=False): + return self.settings_dict.get(name, default) - class MockSpider: - def __init__(self): - self.settings = MockSettings() + class MockSpider: + def __init__(self) -> None: + self.settings = MockSettings() - return MockSpider() + return MockSpider() # type: ignore[return-value] - def test_generators_return_something(self, mock_spider): - def f1(): - yield 1 - return 2 - def g1(): - yield 1 - return "asdf" +def test_generators_return_something(mock_spider): + def f1(): + yield 1 + return 2 - def h1(): - yield 1 + def g1(): + yield 1 + return "asdf" - def helper(): - return 0 + def h1(): + yield 1 - yield helper() - return 2 + def helper() -> int: + return 0 - def i1(): - """ - docstring - """ - url = """ -https://example.org + yield helper() + return 2 + + def i1(): """ - yield url - return 1 - - assert is_generator_with_return_value(top_level_return_something) - assert is_generator_with_return_value(f1) - assert is_generator_with_return_value(g1) - assert is_generator_with_return_value(h1) - assert is_generator_with_return_value(i1) - - with pytest.warns( - UserWarning, - match='The "MockSpider.top_level_return_something" method is a generator', - ): - warn_on_generator_with_return_value(mock_spider, top_level_return_something) - with pytest.warns( - UserWarning, match='The "MockSpider.f1" method is a generator' - ): - warn_on_generator_with_return_value(mock_spider, f1) - with pytest.warns( - UserWarning, match='The "MockSpider.g1" method is a generator' - ): - warn_on_generator_with_return_value(mock_spider, g1) - with pytest.warns( - UserWarning, match='The "MockSpider.h1" method is a generator' - ): - warn_on_generator_with_return_value(mock_spider, h1) - with pytest.warns( - UserWarning, match='The "MockSpider.i1" method is a generator' - ): - warn_on_generator_with_return_value(mock_spider, i1) - - def test_generators_return_none(self, mock_spider): - def f2(): - yield 1 - - def g2(): - yield 1 - - def h2(): - yield 1 - - def i2(): - yield 1 - yield from generator_that_returns_stuff() - - def j2(): - yield 1 - - def helper(): - return 0 - - yield helper() - - def k2(): - """ - docstring - """ - url = """ -https://example.org + docstring """ - yield url - - def l2(): - return - - assert not is_generator_with_return_value(top_level_return_none) - assert not is_generator_with_return_value(f2) - assert not is_generator_with_return_value(g2) - assert not is_generator_with_return_value(h2) - assert not is_generator_with_return_value(i2) - assert not is_generator_with_return_value(j2) # not recursive - assert not is_generator_with_return_value(k2) # not recursive - assert not is_generator_with_return_value(l2) - - with warnings.catch_warnings(): - warnings.simplefilter("error", UserWarning) - warn_on_generator_with_return_value(mock_spider, top_level_return_none) - warn_on_generator_with_return_value(mock_spider, f2) - warn_on_generator_with_return_value(mock_spider, g2) - warn_on_generator_with_return_value(mock_spider, h2) - warn_on_generator_with_return_value(mock_spider, i2) - warn_on_generator_with_return_value(mock_spider, j2) - warn_on_generator_with_return_value(mock_spider, k2) - warn_on_generator_with_return_value(mock_spider, l2) - - def test_generators_return_none_with_decorator(self, mock_spider): - def decorator(func): - def inner_func(): - func() - - return inner_func - - @decorator - def f3(): - yield 1 - - @decorator - def g3(): - yield 1 - - @decorator - def h3(): - yield 1 - - @decorator - def i3(): - yield 1 - yield from generator_that_returns_stuff() - - @decorator - def j3(): - yield 1 - - def helper(): - return 0 - - yield helper() - - @decorator - def k3(): - """ - docstring - """ - url = """ + url = """ https://example.org + """ + yield url + return 1 + + assert is_generator_with_return_value(top_level_return_something) + assert is_generator_with_return_value(f1) + assert is_generator_with_return_value(g1) + assert is_generator_with_return_value(h1) + assert is_generator_with_return_value(i1) + + with pytest.warns( + UserWarning, + match='The "MockSpider.top_level_return_something" method is a generator', + ): + warn_on_generator_with_return_value(mock_spider, top_level_return_something) + with pytest.warns(UserWarning, match='The "MockSpider.f1" method is a generator'): + warn_on_generator_with_return_value(mock_spider, f1) + with pytest.warns(UserWarning, match='The "MockSpider.g1" method is a generator'): + warn_on_generator_with_return_value(mock_spider, g1) + with pytest.warns(UserWarning, match='The "MockSpider.h1" method is a generator'): + warn_on_generator_with_return_value(mock_spider, h1) + with pytest.warns(UserWarning, match='The "MockSpider.i1" method is a generator'): + warn_on_generator_with_return_value(mock_spider, i1) + + +def test_generators_return_none(mock_spider): + def f2(): + yield 1 + + def g2(): + yield 1 + + def h2(): + yield 1 + + def i2(): + yield 1 + yield from generator_that_returns_stuff() + + def j2(): + yield 1 + + def helper() -> int: + return 0 + + yield helper() + + def k2(): """ - yield url + docstring + """ + url = """ +https://example.org + """ + yield url - @decorator - def l3(): - return + def l2(): + return - assert not is_generator_with_return_value(top_level_return_none) - assert not is_generator_with_return_value(f3) - assert not is_generator_with_return_value(g3) - assert not is_generator_with_return_value(h3) - assert not is_generator_with_return_value(i3) - assert not is_generator_with_return_value(j3) # not recursive - assert not is_generator_with_return_value(k3) # not recursive - assert not is_generator_with_return_value(l3) + assert not is_generator_with_return_value(top_level_return_none) + assert not is_generator_with_return_value(f2) + assert not is_generator_with_return_value(g2) + assert not is_generator_with_return_value(h2) + assert not is_generator_with_return_value(i2) + assert not is_generator_with_return_value(j2) # not recursive + assert not is_generator_with_return_value(k2) # not recursive + assert not is_generator_with_return_value(l2) - with warnings.catch_warnings(): - warnings.simplefilter("error", UserWarning) - warn_on_generator_with_return_value(mock_spider, top_level_return_none) - warn_on_generator_with_return_value(mock_spider, f3) - warn_on_generator_with_return_value(mock_spider, g3) - warn_on_generator_with_return_value(mock_spider, h3) - warn_on_generator_with_return_value(mock_spider, i3) - warn_on_generator_with_return_value(mock_spider, j3) - warn_on_generator_with_return_value(mock_spider, k3) - warn_on_generator_with_return_value(mock_spider, l3) + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + warn_on_generator_with_return_value(mock_spider, top_level_return_none) + warn_on_generator_with_return_value(mock_spider, f2) + warn_on_generator_with_return_value(mock_spider, g2) + warn_on_generator_with_return_value(mock_spider, h2) + warn_on_generator_with_return_value(mock_spider, i2) + warn_on_generator_with_return_value(mock_spider, j2) + warn_on_generator_with_return_value(mock_spider, k2) + warn_on_generator_with_return_value(mock_spider, l2) - @mock.patch( - "scrapy.utils.misc.is_generator_with_return_value", new=_indentation_error - ) - def test_indentation_error(self, mock_spider): - with pytest.warns(UserWarning, match="Unable to determine"): - warn_on_generator_with_return_value(mock_spider, top_level_return_none) - def test_partial(self): - def cb(arg1, arg2): - yield {} +def test_generators_return_none_with_decorator(mock_spider): + def decorator(func): + def inner_func(): + func() - partial_cb = partial(cb, arg1=42) - assert not is_generator_with_return_value(partial_cb) + return inner_func - def test_warn_on_generator_with_return_value_settings_disabled(self): - class MockSettings: - def __init__(self, settings_dict=None): - self.settings_dict = settings_dict or {} + @decorator + def f3(): + yield 1 - def getbool(self, name, default=False): - return self.settings_dict.get(name, default) + @decorator + def g3(): + yield 1 - class MockSpider: - def __init__(self): - self.settings = MockSettings({"WARN_ON_GENERATOR_RETURN_VALUE": False}) + @decorator + def h3(): + yield 1 - spider = MockSpider() + @decorator + def i3(): + yield 1 + yield from generator_that_returns_stuff() - def gen_with_return(): - yield 1 - return "value" + @decorator + def j3(): + yield 1 - with warnings.catch_warnings(): - warnings.simplefilter("error", UserWarning) - warn_on_generator_with_return_value(spider, gen_with_return) + def helper() -> int: + return 0 - spider.settings.settings_dict["WARN_ON_GENERATOR_RETURN_VALUE"] = True + yield helper() - with pytest.warns(UserWarning, match="is a generator"): - warn_on_generator_with_return_value(spider, gen_with_return) + @decorator + def k3(): + """ + docstring + """ + url = """ +https://example.org + """ + yield url + + @decorator + def l3(): + return + + assert not is_generator_with_return_value(top_level_return_none) + assert not is_generator_with_return_value(f3) + assert not is_generator_with_return_value(g3) + assert not is_generator_with_return_value(h3) + assert not is_generator_with_return_value(i3) + assert not is_generator_with_return_value(j3) # not recursive + assert not is_generator_with_return_value(k3) # not recursive + assert not is_generator_with_return_value(l3) + + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + warn_on_generator_with_return_value(mock_spider, top_level_return_none) + warn_on_generator_with_return_value(mock_spider, f3) + warn_on_generator_with_return_value(mock_spider, g3) + warn_on_generator_with_return_value(mock_spider, h3) + warn_on_generator_with_return_value(mock_spider, i3) + warn_on_generator_with_return_value(mock_spider, j3) + warn_on_generator_with_return_value(mock_spider, k3) + warn_on_generator_with_return_value(mock_spider, l3) + + +@mock.patch("scrapy.utils.misc.is_generator_with_return_value", new=_indentation_error) +def test_indentation_error(mock_spider): + with pytest.warns(UserWarning, match="Unable to determine"): + warn_on_generator_with_return_value(mock_spider, top_level_return_none) + + +def test_partial() -> None: + def cb(arg1, arg2): + yield {} + + partial_cb = partial(cb, arg1=42) + assert not is_generator_with_return_value(partial_cb) + + +def test_warn_on_generator_with_return_value_settings_disabled() -> None: + class MockSettings: + def __init__(self, settings_dict: dict[str, Any] | None = None): + self.settings_dict = settings_dict or {} + + def getbool(self, name, default=False): + return self.settings_dict.get(name, default) + + class MockSpider: + def __init__(self) -> None: + self.settings = MockSettings({"WARN_ON_GENERATOR_RETURN_VALUE": False}) + + spider = MockSpider() + + def gen_with_return(): + yield 1 + return "value" + + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + warn_on_generator_with_return_value(spider, gen_with_return) # type: ignore[arg-type] + + spider.settings.settings_dict["WARN_ON_GENERATOR_RETURN_VALUE"] = True + + with pytest.warns(UserWarning, match="is a generator"): + warn_on_generator_with_return_value(spider, gen_with_return) # type: ignore[arg-type] diff --git a/tests/test_utils_project.py b/tests/test_utils_project.py index 5333a55cb..7eade6463 100644 --- a/tests/test_utils_project.py +++ b/tests/test_utils_project.py @@ -1,14 +1,20 @@ +from __future__ import annotations + import os from pathlib import Path +from typing import TYPE_CHECKING import pytest from scrapy.utils.misc import set_environ from scrapy.utils.project import data_path, get_project_settings +if TYPE_CHECKING: + from collections.abc import Generator + @pytest.fixture -def proj_path(tmp_path): +def proj_path(tmp_path: Path) -> Generator[Path]: prev_dir = Path.cwd() project_dir = tmp_path @@ -21,7 +27,7 @@ def proj_path(tmp_path): os.chdir(prev_dir) -def test_data_path_outside_project(): +def test_data_path_outside_project() -> None: assert str(Path(".scrapy", "somepath")) == data_path("somepath") abspath = str(Path(os.path.sep, "absolute", "path")) assert abspath == data_path(abspath) diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index 2e8047e2d..c3b5dfc99 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -22,8 +22,7 @@ from scrapy.utils.python import ( from tests.utils.decorators import coroutine_test if TYPE_CHECKING: - from collections.abc import Iterable, Mapping - + from collections.abc import AsyncIterator, Iterable, Mapping _KT = TypeVar("_KT") _VT = TypeVar("_VT") @@ -31,22 +30,22 @@ _VT = TypeVar("_VT") class TestMutableAsyncChain: @staticmethod - async def g1(): + async def g1() -> AsyncIterator[int]: for i in range(3): yield i @staticmethod - async def g2(): + async def g2() -> AsyncIterator[int]: return yield @staticmethod - async def g3(): + async def g3() -> AsyncIterator[int]: for i in range(7, 10): yield i @staticmethod - async def g4(): + async def g4() -> AsyncIterator[int]: for i in range(3, 5): yield i 1 / 0 @@ -85,7 +84,7 @@ class TestToUnicode: def test_converting_a_strange_object_should_raise_type_error(self): with pytest.raises(TypeError): - to_unicode(423) + to_unicode(423) # type: ignore[arg-type] def test_errors_argument(self): assert to_unicode(b"a\xedb", "utf-8", errors="replace") == "a\ufffdb" @@ -103,7 +102,7 @@ class TestToBytes: def test_converting_a_strange_object_should_raise_type_error(self): with pytest.raises(TypeError): - to_bytes(pytest) + to_bytes(pytest) # type: ignore[arg-type] def test_errors_argument(self): assert to_bytes("a\ufffdb", "latin-1", errors="replace") == b"a?b" @@ -112,10 +111,10 @@ class TestToBytes: def test_memoizemethod_noargs(): class A: @memoizemethod_noargs - def cached(self): + def cached(self) -> object: return object() - def noncached(self): + def noncached(self) -> object: return object() a = A() @@ -150,7 +149,7 @@ def test_get_func_args(): pass class A: - def __init__(self, a, b, c): + def __init__(self, a: int, b: int, c: int): pass def method(self, a, b, c): diff --git a/tests/test_utils_reactor.py b/tests/test_utils_reactor.py index 7d39a478e..44cb5c306 100644 --- a/tests/test_utils_reactor.py +++ b/tests/test_utils_reactor.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import asyncio import pytest diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index 1642a932b..eb95be06c 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -2,7 +2,7 @@ from __future__ import annotations import json from hashlib import sha1 -from typing import Any +from typing import TYPE_CHECKING, Any, Protocol from weakref import WeakKeyDictionary import pytest @@ -17,6 +17,9 @@ from scrapy.utils.request import ( ) from scrapy.utils.test import get_crawler +if TYPE_CHECKING: + from collections.abc import Iterable + @pytest.mark.parametrize( ("r", "expected"), @@ -56,8 +59,18 @@ def test_request_httprepr_for_non_http_request(r: Request) -> None: request_httprepr(r) +class _FingerprintFunction(Protocol): + def __call__( + self, + request: Request, + *, + include_headers: Iterable[bytes | str] | None = None, + keep_fragments: bool = False, + ) -> bytes: ... + + class TestFingerprint: - function: staticmethod[[Request], bytes] = staticmethod(fingerprint) + function: _FingerprintFunction = staticmethod(fingerprint) cache: ( WeakKeyDictionary[ Request, dict[tuple[tuple[bytes, ...] | None, bool, bool], bytes] @@ -261,6 +274,7 @@ class TestRequestFingerprinter: def test_fingerprint(self): crawler = get_crawler() request = Request("https://example.com") + assert crawler.request_fingerprinter assert crawler.request_fingerprinter.fingerprint(request) == fingerprint( request ) @@ -277,6 +291,7 @@ class TestCustomRequestFingerprinter: } crawler = get_crawler(settings_dict=settings) + assert crawler.request_fingerprinter r1 = Request("http://www.example.com", headers={"X-ID": "1"}) fp1 = crawler.request_fingerprinter.fingerprint(r1) r2 = Request("http://www.example.com", headers={"X-ID": "2"}) @@ -285,9 +300,9 @@ class TestCustomRequestFingerprinter: def test_dont_canonicalize(self): class RequestFingerprinter: - cache = WeakKeyDictionary() + cache: WeakKeyDictionary[Request, bytes] = WeakKeyDictionary() - def fingerprint(self, request): + def fingerprint(self, request: Request) -> bytes: if request not in self.cache: fp = sha1() fp.update(to_bytes(request.url)) @@ -299,6 +314,7 @@ class TestCustomRequestFingerprinter: } crawler = get_crawler(settings_dict=settings) + assert crawler.request_fingerprinter r1 = Request("http://www.example.com?a=1&a=2") fp1 = crawler.request_fingerprinter.fingerprint(r1) r2 = Request("http://www.example.com?a=2&a=1") @@ -317,6 +333,7 @@ class TestCustomRequestFingerprinter: } crawler = get_crawler(settings_dict=settings) + assert crawler.request_fingerprinter r1 = Request("http://www.example.com") fp1 = crawler.request_fingerprinter.fingerprint(r1) r2 = Request("http://www.example.com", meta={"fingerprint": "a"}) @@ -348,6 +365,7 @@ class TestCustomRequestFingerprinter: } crawler = get_crawler(settings_dict=settings) + assert crawler.request_fingerprinter request = Request("http://www.example.com") fingerprint = crawler.request_fingerprinter.fingerprint(request) assert fingerprint == settings["FINGERPRINT"] diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index 146cdb802..608b2bbd9 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from pathlib import Path from time import process_time from urllib.parse import urlparse @@ -15,7 +17,7 @@ from scrapy.utils.response import ( ) -def _read_browser_output(burl: str): +def _read_browser_output(burl: str) -> bytes: path = urlparse(burl).path if not path or not Path(path).exists(): path = burl.replace("file://", "") @@ -224,7 +226,7 @@ def test_open_in_browser_redos_head(): (b"real", b"real"), ], ) -def test_remove_html_comments(input_body, output_body): +def test_remove_html_comments(input_body: bytes, output_body: bytes) -> None: assert _remove_html_comments(input_body) == output_body diff --git a/tests/test_utils_serialize.py b/tests/test_utils_serialize.py index 2e6a790f8..2702c2cce 100644 --- a/tests/test_utils_serialize.py +++ b/tests/test_utils_serialize.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import dataclasses import datetime import json diff --git a/tests/test_utils_signal.py b/tests/test_utils_signal.py index dbb9caf2a..3109bc149 100644 --- a/tests/test_utils_signal.py +++ b/tests/test_utils_signal.py @@ -21,9 +21,6 @@ from tests.utils.decorators import coroutine_test if TYPE_CHECKING: from collections.abc import Callable -if TYPE_CHECKING: - from collections.abc import Callable - class TestSendCatchLog: # whether the function being tested returns exceptions or failures diff --git a/tests/test_utils_sitemap.py b/tests/test_utils_sitemap.py index ac57e1739..9f2dce6d6 100644 --- a/tests/test_utils_sitemap.py +++ b/tests/test_utils_sitemap.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import pytest from scrapy.exceptions import ScrapyDeprecationWarning diff --git a/tests/test_utils_template.py b/tests/test_utils_template.py index 4515ce36e..076edf3db 100644 --- a/tests/test_utils_template.py +++ b/tests/test_utils_template.py @@ -1,7 +1,14 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + from scrapy.utils.template import render_templatefile +if TYPE_CHECKING: + from pathlib import Path -def test_simple_render(tmp_path): + +def test_simple_render(tmp_path: Path) -> None: context = {"project_name": "proj", "name": "spi", "classname": "TheSpider"} template = "from ${project_name}.spiders.${name} import ${classname}" rendered = "from proj.spiders.spi import TheSpider" diff --git a/tests/test_utils_trackref.py b/tests/test_utils_trackref.py index 2334c76e9..5458aa603 100644 --- a/tests/test_utils_trackref.py +++ b/tests/test_utils_trackref.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import sys from io import StringIO from unittest import mock @@ -46,13 +48,13 @@ Bar 1 oldest: 0s ago @mock.patch("sys.stdout", new_callable=StringIO) -def test_print_live_refs_empty(stdout): +def test_print_live_refs_empty(stdout: StringIO) -> None: trackref.print_live_refs() assert stdout.getvalue() == "Live References\n\n\n" @mock.patch("sys.stdout", new_callable=StringIO) -def test_print_live_refs_with_objects(stdout): +def test_print_live_refs_with_objects(stdout: StringIO) -> None: o1 = Foo() # noqa: F841 trackref.print_live_refs() assert ( diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index 5b98131a1..d9d162c23 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import pytest from scrapy.linkextractors import IGNORED_EXTENSIONS @@ -191,7 +193,7 @@ def test_guess_scheme(url: str, expected: str): ), ], ) -def test_guess_scheme_skipped(url: str, expected: str, reason: str): +def test_guess_scheme_skipped(url: str, expected: str, reason: str) -> None: pytest.skip(reason) diff --git a/tests_typing/test_http_request.mypy-testing b/tests_typing/test_http_request.mypy-testing index a431091d5..4ff562dff 100644 --- a/tests_typing/test_http_request.mypy-testing +++ b/tests_typing/test_http_request.mypy-testing @@ -16,7 +16,7 @@ class MyRequest2(Request): @pytest.mark.mypy_testing def mypy_test_headers() -> None: - Request("data:,", headers=1) # E: Argument "headers" to "Request" has incompatible type "int"; expected "Mapping[str, Any] | Iterable[tuple[str, Any]] | None" + Request("data:,", headers=1) # E: Argument "headers" to "Request" has incompatible type "int"; expected "Mapping[str, Any] | Mapping[bytes, Any] | Iterable[tuple[str | bytes, Any]] | None" Request("data:,", headers=None) Request("data:,", headers={}) Request("data:,", headers=[]) diff --git a/tests_typing/test_http_response.mypy-testing b/tests_typing/test_http_response.mypy-testing index d497c2470..1c157328c 100644 --- a/tests_typing/test_http_response.mypy-testing +++ b/tests_typing/test_http_response.mypy-testing @@ -7,7 +7,7 @@ from scrapy.http import HtmlResponse, Response, TextResponse @pytest.mark.mypy_testing def mypy_test_headers() -> None: - Response("data:,", headers=1) # E: Argument "headers" to "Response" has incompatible type "int"; expected "Mapping[str, Any] | Iterable[tuple[str, Any]] | None" + Response("data:,", headers=1) # E: Argument "headers" to "Response" has incompatible type "int"; expected "Mapping[str, Any] | Mapping[bytes, Any] | Iterable[tuple[str | bytes, Any]] | None" Response("data:,", headers=None) Response("data:,", headers={}) Response("data:,", headers=[]) From bc5b5fb1f6d8fef2f57ef74a728dd609b5c00cf4 Mon Sep 17 00:00:00 2001 From: Youssef Mohamed <114195599+MegumiinUwU@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:47:15 +0300 Subject: [PATCH 05/25] Add Request.to_curl() (#7743) (#7802) --- docs/topics/request-response.rst | 2 ++ scrapy/http/request/__init__.py | 14 ++++++++++++++ tests/test_utils_request.py | 11 +++++++++++ tests/utils/bases/http_request.py | 9 +++++++++ 4 files changed, 36 insertions(+) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index b83a04032..8e565907f 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -260,6 +260,8 @@ Request objects .. automethod:: from_curl + .. automethod:: to_curl + .. automethod:: to_dict diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 57517a65b..68847283a 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -387,6 +387,20 @@ class Request(object_ref): request_kwargs.update(kwargs) return cls(**request_kwargs) + def to_curl(self) -> str: + """Return a string with a `cURL `_ command equivalent + to this request. + + Inverse of :meth:`from_curl`. See also + :func:`scrapy.utils.request.request_to_curl`. + + .. versionadded:: VERSION + """ + # Imported here to avoid a circular import. + from scrapy.utils.request import request_to_curl # noqa: PLC0415 + + return request_to_curl(self) + def to_dict(self, *, spider: scrapy.Spider | None = None) -> dict[str, Any]: """Return a dictionary containing the Request's data. diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index eb95be06c..935447bc4 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -475,3 +475,14 @@ class TestRequestToCurl: " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=1'" ) self._test_request(request_object, expected_curl_command) + + def test_request_to_curl_method(self) -> None: + request_object = Request( + "https://www.httpbin.org/post", + method="POST", + body=json.dumps({"foo": "bar"}), + ) + expected_curl_command = ( + 'curl -X POST https://www.httpbin.org/post --data-raw \'{"foo": "bar"}\'' + ) + assert request_object.to_curl() == expected_curl_command diff --git a/tests/utils/bases/http_request.py b/tests/utils/bases/http_request.py index c255b5e4c..3a1e588ef 100644 --- a/tests/utils/bases/http_request.py +++ b/tests/utils/bases/http_request.py @@ -6,6 +6,7 @@ import pytest from scrapy.http import Headers, Request from scrapy.http.request import NO_CALLBACK +from scrapy.utils.request import request_to_curl class TestRequestBase(ABC): @@ -488,3 +489,11 @@ class TestRequestBase(ABC): 'curl -X PATCH "http://example.org" --foo -z', ignore_unknown_options=False, ) + + def test_to_curl(self): + # Note: more curated tests regarding curl conversion are in + # `test_utils_request.py` + r = self.request_class( + "http://www.example.com/", method="POST", body=b"foo=bar" + ) + assert r.to_curl() == request_to_curl(r) From 8b5147ae2ec62bca66e3b957a8d0f1c3654ecf2e Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 29 Jul 2026 11:32:18 +0200 Subject: [PATCH 06/25] Improve test coverage for scrapy.pipelines (#7798) * Improve test coverage for scrapy.pipelines * Restore old Pillow support --- tests/test_pipeline_files.py | 220 ++++++++++++++++++++++++++++++--- tests/test_pipeline_images.py | 52 +++++++- tests/test_pipeline_media.py | 60 +++++++++ tests/utils/media_pipelines.py | 6 + 4 files changed, 321 insertions(+), 17 deletions(-) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4f6fa21a0..97daa514e 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -33,7 +33,7 @@ from scrapy.pipelines.files import ( GCSFilesStore, S3FilesStore, ) -from scrapy.pipelines.media import MediaPipeline, _MediaRequestFiltered +from scrapy.pipelines.media import _MediaRequestFiltered from scrapy.settings import Settings from scrapy.utils.asyncio import call_later from scrapy.utils.defer import maybe_deferred_to_future @@ -43,11 +43,7 @@ from tests.mockserver.ftp import MockFTPServer from tests.utils.decorators import coroutine_test, inline_callbacks_test from .utils.cloud import mock_google_cloud_storage -from .utils.media_pipelines import mocked_download_func - -# required by persist_file() and stat_file(), but as some stores don't use the argument -# we can pass this singleton to keep type hints correct -DUMMY_SPIDER_INFO = MediaPipeline.SpiderInfo(DefaultSpider()) +from .utils.media_pipelines import DUMMY_SPIDER_INFO, mocked_download_func def get_ftp_content_and_delete( @@ -94,16 +90,19 @@ class DeferredFSFilesStore(FSFilesStore): class TestFilesPipeline: def setup_method(self): self.tempdir = mkdtemp() - settings_dict = {"FILES_STORE": self.tempdir} - crawler = get_crawler(DefaultSpider, settings_dict=settings_dict) - crawler.spider = crawler._create_spider() - crawler.engine = MagicMock(download_async=mocked_download_func) - self.pipeline = FilesPipeline.from_crawler(crawler) - self.pipeline.open_spider() + self.pipeline = self._create_pipeline(FilesPipeline) def teardown_method(self): rmtree(self.tempdir) + def _create_pipeline(self, pipeline_cls: type[FilesPipeline]) -> FilesPipeline: + crawler = get_crawler(DefaultSpider, {"FILES_STORE": self.tempdir}) + crawler.spider = crawler._create_spider() + crawler.engine = MagicMock(download_async=mocked_download_func) + pipeline = pipeline_cls.from_crawler(crawler) + pipeline.open_spider() + return pipeline + def test_file_path_query_parameters(self): file_path = self.pipeline.file_path @@ -254,6 +253,107 @@ class TestFilesPipeline: assert result["files"][0]["checksum"] != "abc" assert result["files"][0]["status"] == "cached" + @coroutine_test + async def test_file_stat_without_last_modified(self) -> None: + """A stat result without a last modification time forces a download.""" + item_url = "http://example.com/file4.pdf" + item = _create_item_with_files(item_url) + with ( + mock.patch.object(FilesPipeline, "inc_stats", return_value=True), + mock.patch.object( + FSFilesStore, "stat_file", return_value={"checksum": "abc"} + ), + mock.patch.object( + FilesPipeline, + "get_media_requests", + return_value=[_prepare_request_object(item_url)], + ), + ): + result = await self.pipeline.process_item(item) + assert result["files"][0]["checksum"] != "abc" + assert result["files"][0]["status"] == "downloaded" + + @coroutine_test + async def test_file_empty_content(self, caplog: pytest.LogCaptureFixture) -> None: + item_url = "http://example.com/empty.pdf" + item = _create_item_with_files(item_url) + request = Request( + item_url, meta={"response": Response(item_url, status=200, body=b"")} + ) + with ( + caplog.at_level(logging.WARNING), + mock.patch.object( + FilesPipeline, "get_media_requests", return_value=[request] + ), + ): + result = await self.pipeline.process_item(item) + assert result["files"] == [] + assert "File (empty-content): Empty file from" in caplog.text + + @coroutine_test + async def test_file_downloaded_file_exception( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """A FileException from file_downloaded() is logged as a warning and + kept as is.""" + + class FailingFilesPipeline(FilesPipeline): + def file_downloaded(self, response, request, info, *, item=None): + raise FileException("boom") + + item_url = "http://example.com/file5.pdf" + item = _create_item_with_files(item_url) + pipeline = self._create_pipeline(FailingFilesPipeline) + with ( + caplog.at_level(logging.WARNING), + mock.patch.object( + FilesPipeline, + "get_media_requests", + return_value=[_prepare_request_object(item_url)], + ), + ): + result = await pipeline.process_item(item) + assert result["files"] == [] + records = [ + r for r in caplog.records if "Error processing file" in r.getMessage() + ] + assert len(records) == 1 + assert records[0].levelname == "WARNING" + assert "boom" in records[0].getMessage() + + @coroutine_test + async def test_file_downloaded_unknown_error( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """Any other exception from file_downloaded() is logged as an error and + reported as a FileException.""" + + class FailingFilesPipeline(FilesPipeline): + def file_downloaded(self, response, request, info, *, item=None): + raise RuntimeError("boom") + + item_url = "http://example.com/file6.pdf" + item = _create_item_with_files(item_url) + pipeline = self._create_pipeline(FailingFilesPipeline) + with ( + caplog.at_level(logging.WARNING), + mock.patch.object( + FilesPipeline, + "get_media_requests", + return_value=[_prepare_request_object(item_url)], + ), + ): + result = await pipeline.process_item(item) + assert result["files"] == [] + records = [ + r for r in caplog.records if "Error processing file" in r.getMessage() + ] + assert len(records) == 1 + assert records[0].levelname == "ERROR" + exc_info = records[0].exc_info + assert exc_info is not None + assert exc_info[0] is RuntimeError + @coroutine_test async def test_async_store(self) -> None: """Test that async persist_file() works and is awaited.""" @@ -648,9 +748,24 @@ class TestFilesPipelineCustomSettings: request = Request("http://example.com/image01.jpg") assert pipeline.file_path(request) == Path("subdir/image01.jpg") - def test_files_store_constructor_with_pathlike_object(self, tmp_path): - fs_store = FSFilesStore(tmp_path) - assert fs_store.basedir == str(tmp_path) + +class TestFSFilesStore: + def test_constructor_with_pathlike_object(self, tmp_path: Path) -> None: + assert FSFilesStore(tmp_path).basedir == str(tmp_path) + + def test_constructor_with_uri(self, tmp_path: Path) -> None: + assert FSFilesStore(f"file://{tmp_path}").basedir == str(tmp_path) + + def test_stat_file(self, tmp_path: Path) -> None: + store = FSFilesStore(tmp_path) + store.persist_file("full/filename", BytesIO(b"data"), DUMMY_SPIDER_INFO) + stat = store.stat_file("full/filename", DUMMY_SPIDER_INFO) + assert stat["checksum"] == "8d777f385d3dfec8815d20f7496026dc" + assert stat["last_modified"] == pytest.approx(time.time(), abs=60) + + def test_stat_missing_file(self, tmp_path: Path) -> None: + store = FSFilesStore(tmp_path) + assert store.stat_file("full/filename", DUMMY_SPIDER_INFO) == {} @pytest.mark.requires_botocore @@ -695,6 +810,59 @@ class TestS3FilesStore: # The call to read does not happen with Stubber assert buffer.method_calls == [mock.call.seek(0)] + @inline_callbacks_test + def test_persist_without_headers(self): + """Without custom headers only the default ones are sent.""" + bucket = "mybucket" + key = "export.csv" + buffer = mock.MagicMock() + + store = S3FilesStore(f"s3://{bucket}/{key}") + from botocore.stub import Stubber # noqa: PLC0415 + + with Stubber(store.s3_client) as stub: + stub.add_response( + "put_object", + expected_params={ + "ACL": S3FilesStore.POLICY, + "Body": buffer, + "Bucket": bucket, + "CacheControl": S3FilesStore.HEADERS["Cache-Control"], + "Key": key, + "Metadata": {}, + }, + service_response={}, + ) + + yield store.persist_file("", buffer, info=DUMMY_SPIDER_INFO) + + stub.assert_no_pending_responses() + + def test_missing_botocore(self): + with ( + mock.patch( + "scrapy.pipelines.files.is_botocore_available", return_value=False + ), + pytest.raises(NotConfigured, match="missing botocore library"), + ): + S3FilesStore("s3://mybucket/key") + + def test_wrong_uri_scheme(self): + with pytest.raises( + ValueError, + match=re.escape( + "Incorrect URI scheme in ftp://mybucket/key, expected 's3'" + ), + ): + S3FilesStore("ftp://mybucket/key") + + def test_unsupported_header(self): + store = S3FilesStore("s3://mybucket/key") + with pytest.raises( + TypeError, match='Header "X-Custom" is not supported by botocore' + ): + store._headers_to_botocore_kwargs({"X-Custom": "value"}) + @inline_callbacks_test def test_stat(self): bucket = "mybucket" @@ -901,6 +1069,28 @@ class TestFTPFileStore: ) assert data == content + @inline_callbacks_test + def test_persist_active_mode(self, monkeypatch: pytest.MonkeyPatch): + data = b"active mode" + path = "full/filename" + monkeypatch.setattr(FTPFilesStore, "FTP_USERNAME", "anonymous") + monkeypatch.setattr(FTPFilesStore, "FTP_PASSWORD", "guest") + monkeypatch.setattr(FTPFilesStore, "USE_ACTIVE_MODE", True) + with MockFTPServer() as ftp_server: + store = FTPFilesStore(ftp_server.url("/")) + yield store.persist_file(path, BytesIO(data), info=DUMMY_SPIDER_INFO) + stat = yield store.stat_file(path, info=DUMMY_SPIDER_INFO) + assert stat["checksum"] == "ff1575649a39a27c13faa0d37c84bab3" + + def test_wrong_uri_scheme(self): + with pytest.raises( + ValueError, + match=re.escape( + "Incorrect URI scheme in http://example.com/, expected 'ftp'" + ), + ): + FTPFilesStore("http://example.com/") + class ItemWithFiles(Item): file_urls = Field() diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 1b73dd157..19e61579f 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -3,20 +3,26 @@ from __future__ import annotations import dataclasses import io import random +import sys from abc import ABC, abstractmethod +from pathlib import Path from shutil import rmtree from tempfile import mkdtemp +from types import SimpleNamespace from typing import Any import attr import pytest from itemadapter import ItemAdapter +from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response from scrapy.item import Field, Item -from scrapy.pipelines.files import GCSFilesStore, S3FilesStore +from scrapy.pipelines.files import GCSFilesStore, S3FilesStore, _md5sum from scrapy.pipelines.images import ImageException, ImagesPipeline from scrapy.utils.test import get_crawler +from tests.utils.decorators import coroutine_test +from tests.utils.media_pipelines import DUMMY_SPIDER_INFO try: from PIL import Image @@ -40,6 +46,11 @@ class TestImagesPipeline: def teardown_method(self): rmtree(self.tempdir) + def test_missing_pillow(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setitem(sys.modules, "PIL", None) + with pytest.raises(NotConfigured, match="requires installing Pillow"): + ImagesPipeline(self.tempdir, crawler=get_crawler()) + def test_file_path(self): file_path = self.pipeline.file_path assert ( @@ -197,6 +208,25 @@ class TestImagesPipeline: assert path == "full/3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg" assert new_im.getpixel((0, 0)) == (255, 0, 0) + @coroutine_test + async def test_image_downloaded(self) -> None: + """The image and its thumbnails are stored, and the checksum of the + full-size image is returned.""" + self.pipeline.thumbs = {"small": (20, 20)} + _, buf = _create_image("JPEG", "RGB", (50, 50), (0, 0, 0)) + url = "https://dev.mydeco.com/mydeco.gif" + response = Response(url=url, body=buf.getvalue()) + + checksum = await self.pipeline.image_downloaded( + response, Request(url=url), DUMMY_SPIDER_INFO + ) + + buf.seek(0) + assert checksum == _md5sum(buf) + name = "3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg" + assert Path(self.tempdir, "full", name).read_bytes() == buf.getvalue() + assert Path(self.tempdir, "thumbs", "small", name).exists() + def test_convert_image(self): SIZE = (100, 100) # straight forward case: RGB and JPEG @@ -230,6 +260,24 @@ class TestImagesPipeline: assert converted.mode == "RGB" assert converted.getcolors() == [(10000, (205, 230, 255))] + def test_convert_image_legacy_resampling_filter( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Pillow older than 9.1.0 has Image.ANTIALIAS instead of + Image.Resampling.LANCZOS.""" + # Image.LANCZOS is the only spelling that exists in every supported + # Pillow version, but Pillow defines it dynamically, hence the ignore. + monkeypatch.setattr( + self.pipeline, + "_Image", + SimpleNamespace(ANTIALIAS=Image.LANCZOS), # type: ignore[attr-defined] + ) + im, buf = _create_image("JPEG", "RGB", (100, 100), (0, 127, 255)) + + thumbnail, _ = self.pipeline.convert_image(im, size=(10, 25), response_body=buf) + + assert thumbnail.size == (10, 10) + @pytest.mark.parametrize( "bad_type", [ @@ -581,7 +629,7 @@ class TestImagesPipelineCustomSettings: GCSFilesStore.POLICY = old_policy -def _create_image(format_, *a, **kw): +def _create_image(format_: str, *a: Any, **kw: Any) -> tuple[Image.Image, io.BytesIO]: buf = io.BytesIO() Image.new(*a, **kw).save(buf, format_) buf.seek(0) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index 23c19e4be..ba1c18006 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -319,6 +319,42 @@ class TestMediaPipeline(TestBaseMediaPipeline): assert self.fingerprint(req1) == self.fingerprint(req2) assert new_item["results"] == [(True, {})] + @coroutine_test + async def test_failures_are_cached_across_multiple_items(self): + self.pipe.LOG_FAILED_RESULTS = False + exc = Exception("foo") + req1 = Request("http://url1", meta={"response": exc}) + new_item = await self.pipe.process_item({"requests": req1}) + assert new_item["results"][0][1].value is exc + + # rsp2 is ignored, the cached failure must be reused because request + # fingerprints are the same + req2 = Request( + req1.url, meta={"response": Response("http://donot.download.me")} + ) + new_item = await self.pipe.process_item({"requests": req2}) + assert new_item["results"][0][0] is False + assert new_item["results"][0][1].value is exc + assert self.pipe._mockcalled.count("media_to_download") == 1 + + @coroutine_test + async def test_cached_failure_calls_errback(self): + """The errback of a request is called for a cached failure as well.""" + self.pipe.LOG_FAILED_RESULTS = False + exc = Exception("foo") + await self.pipe.process_item( + {"requests": Request("http://url1", meta={"response": exc})} + ) + + def errback(failure): + self.pipe._mockcalled.append("request_errback") + return {"recovered": failure.value} + + req = Request("http://url1", errback=errback) + new_item = await self.pipe.process_item({"requests": req}) + assert new_item["results"] == [(True, {"recovered": exc})] + assert self.pipe._mockcalled.count("request_errback") == 1 + @coroutine_test async def test_results_are_cached_for_requests_of_single_item(self): rsp1 = Response("http://url1") @@ -472,6 +508,30 @@ class TestBuildFromCrawler: assert pipe._from_crawler_called +class MediaFailedNonePipeline(MockedMediaPipeline): + def media_failed(self, failure, request, info): + self._mockcalled.append("media_failed") + + +class TestMediaFailedNone(TestBaseMediaPipeline): + """Test what happens when media_failed() neither raises an exception nor + returns a failure.""" + + pipeline_class = MediaFailedNonePipeline + + @coroutine_test + async def test_result_none(self): + req = Request("http://url1", meta={"response": Exception("foo")}) + new_item = await self.pipe.process_item({"requests": req}) + assert new_item["results"] == [(True, None)] + assert self.pipe._mockcalled == [ + "get_media_requests", + "media_to_download", + "media_failed", + "item_completed", + ] + + class MediaFailedFailurePipeline(MockedMediaPipeline): def media_failed(self, failure, request, info): self._mockcalled.append("media_failed") diff --git a/tests/utils/media_pipelines.py b/tests/utils/media_pipelines.py index 283c95940..7e013b797 100644 --- a/tests/utils/media_pipelines.py +++ b/tests/utils/media_pipelines.py @@ -3,6 +3,12 @@ from __future__ import annotations from typing import Any from scrapy.http.request import NO_CALLBACK, Request +from scrapy.pipelines.media import MediaPipeline +from scrapy.utils.spider import DefaultSpider + +# required by persist_file() and stat_file(), but as some stores don't use the argument +# we can pass this singleton to keep type hints correct +DUMMY_SPIDER_INFO = MediaPipeline.SpiderInfo(DefaultSpider()) async def mocked_download_func(request: Request) -> Any: From 0cbb20e8e83996a4d8a3339254c2d7e9b763ce7f Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 29 Jul 2026 11:39:27 +0200 Subject: [PATCH 07/25] Treat broken cache records as cache misses (#7805) --- docs/topics/downloader-middleware.rst | 4 + scrapy/downloadermiddlewares/httpcache.py | 21 +++++- tests/test_downloadermiddleware_httpcache.py | 77 +++++++++++++++++--- 3 files changed, 90 insertions(+), 12 deletions(-) diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 934eb19ac..fcfe7fd29 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -564,6 +564,10 @@ defines the methods described below. Return response if present in cache, or ``None`` otherwise. + If this method raises an exception, e.g. because the cache entry is + corrupted, the middleware logs a warning and handles the request as a + cache miss. + :param spider: the spider which generated the request :type spider: :class:`~scrapy.Spider` object diff --git a/scrapy/downloadermiddlewares/httpcache.py b/scrapy/downloadermiddlewares/httpcache.py index c6c811809..e7ca0ac0e 100644 --- a/scrapy/downloadermiddlewares/httpcache.py +++ b/scrapy/downloadermiddlewares/httpcache.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging from email.utils import formatdate from typing import TYPE_CHECKING @@ -28,6 +29,9 @@ if TYPE_CHECKING: from scrapy.statscollectors import StatsCollector +logger = logging.getLogger(__name__) + + class HttpCacheMiddleware: DOWNLOAD_EXCEPTIONS = ( ConnectionDone, @@ -77,9 +81,20 @@ class HttpCacheMiddleware: return None # Look for cached response and check if expired - cachedresponse: Response | None = self.storage.retrieve_response( - self.crawler.spider, request - ) + cachedresponse: Response | None + try: + cachedresponse = self.storage.retrieve_response( + self.crawler.spider, request + ) + except Exception: + self.stats.inc_value("httpcache/retrieve_error") + logger.warning( + f"Could not read the cache entry for {request}, treating it as a " + f"cache miss.", + exc_info=True, + extra={"spider": self.crawler.spider}, + ) + cachedresponse = None if cachedresponse is None: self.stats.inc_value("httpcache/miss") if self.ignore_missing: diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py index 9d5d6874e..ce56ee11d 100644 --- a/tests/test_downloadermiddleware_httpcache.py +++ b/tests/test_downloadermiddleware_httpcache.py @@ -1,10 +1,12 @@ from __future__ import annotations import email.utils +import logging import shutil import tempfile import time from contextlib import contextmanager +from pathlib import Path from typing import TYPE_CHECKING, Any from unittest import mock @@ -93,6 +95,12 @@ class TestBase: class StorageTestMixin: """Mixin containing storage-specific test methods.""" + def _corrupt_cache_entry( + self, storage: Any, spider: Spider, request: Request + ) -> None: + """Make the cache entry of *request* unreadable for *storage*.""" + raise NotImplementedError + def test_storage(self): with self._storage(HTTPCACHE_EXPIRATION_SECS=1) as (storage, crawler): request2 = self.request.copy() @@ -115,6 +123,38 @@ class StorageTestMixin: with mock.patch("scrapy.extensions.httpcache.time", return_value=future): assert storage.retrieve_response(crawler.spider, self.request) + def test_corrupted_cache_entry_is_a_miss(self, caplog): + with self._middleware() as mw: + spider = mw.crawler.spider + mw.storage.store_response(spider, self.request, self.response) + self._corrupt_cache_entry(mw.storage, spider, self.request) + + caplog.clear() + with caplog.at_level(logging.WARNING): + assert mw.process_request(self.request) is None + + assert "treating it as a cache miss" in caplog.text + assert mw.crawler.stats.get_value("httpcache/retrieve_error") == 1 + assert mw.crawler.stats.get_value("httpcache/miss") == 1 + + # Storing the response again replaces the corrupted cache entry. + mw.storage.store_response(spider, self.request, self.response) + self.assertEqualResponse( + self.response, mw.storage.retrieve_response(spider, self.request) + ) + + def test_corrupted_cache_entry_ignore_missing(self): + with self._middleware(HTTPCACHE_IGNORE_MISSING=True) as mw: + spider = mw.crawler.spider + mw.storage.store_response(spider, self.request, self.response) + self._corrupt_cache_entry(mw.storage, spider, self.request) + + with pytest.raises(IgnoreRequest): + mw.process_request(self.request) + + assert mw.crawler.stats.get_value("httpcache/retrieve_error") == 1 + assert mw.crawler.stats.get_value("httpcache/ignore") == 1 + def test_storage_no_content_type_header(self): """Test that the response body is used to get the right response class even if there is no Content-Type header""" @@ -556,29 +596,43 @@ class RFC2616PolicyTestMixin(PolicyTestMixin): # Concrete test classes that combine storage and policy mixins -class TestFilesystemStorageWithDummyPolicy( - TestBase, StorageTestMixin, DummyPolicyTestMixin -): +class FilesystemStorageTestMixin(StorageTestMixin): storage_class = "scrapy.extensions.httpcache.FilesystemCacheStorage" + + def _corrupt_cache_entry(self, storage, spider, request) -> None: + rpath = Path(storage._get_request_path(spider, request)) + (rpath / "response_body").unlink() + + +class DbmStorageTestMixin(StorageTestMixin): + storage_class = "scrapy.extensions.httpcache.DbmCacheStorage" + + def _corrupt_cache_entry(self, storage, spider, request) -> None: + key = storage._fingerprinter.fingerprint(request).hex() + storage.db[f"{key}_data"] = b"not a pickle" + + +class TestFilesystemStorageWithDummyPolicy( + TestBase, FilesystemStorageTestMixin, DummyPolicyTestMixin +): policy_class = "scrapy.extensions.httpcache.DummyPolicy" class TestFilesystemStorageWithRFC2616Policy( - TestBase, StorageTestMixin, RFC2616PolicyTestMixin + TestBase, FilesystemStorageTestMixin, RFC2616PolicyTestMixin ): - storage_class = "scrapy.extensions.httpcache.FilesystemCacheStorage" policy_class = "scrapy.extensions.httpcache.RFC2616Policy" -class TestDbmStorageWithDummyPolicy(TestBase, StorageTestMixin, DummyPolicyTestMixin): - storage_class = "scrapy.extensions.httpcache.DbmCacheStorage" +class TestDbmStorageWithDummyPolicy( + TestBase, DbmStorageTestMixin, DummyPolicyTestMixin +): policy_class = "scrapy.extensions.httpcache.DummyPolicy" class TestDbmStorageWithRFC2616Policy( - TestBase, StorageTestMixin, RFC2616PolicyTestMixin + TestBase, DbmStorageTestMixin, RFC2616PolicyTestMixin ): - storage_class = "scrapy.extensions.httpcache.DbmCacheStorage" policy_class = "scrapy.extensions.httpcache.RFC2616Policy" @@ -599,3 +653,8 @@ class TestFilesystemStorageGzipWithDummyPolicy(TestFilesystemStorageWithDummyPol def _get_settings(self, **new_settings) -> dict[str, Any]: new_settings.setdefault("HTTPCACHE_GZIP", True) return super()._get_settings(**new_settings) + + def _corrupt_cache_entry(self, storage, spider, request) -> None: + # A spider killed while writing a gzip file leaves it truncated. + body_path = Path(storage._get_request_path(spider, request), "response_body") + body_path.write_bytes(body_path.read_bytes()[:-5]) From 01447f996500367fb999560d08a40cd2ff68815a Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:00:24 -0300 Subject: [PATCH 08/25] fix(commands/parse): Restore request callback before invoking spider (#7803) * fix(commands/parse): restore request callback before invoking spider * Add other test --- scrapy/commands/parse.py | 2 ++ tests/test_command_parse.py | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index 2ac65bf3f..93194ded7 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -346,6 +346,8 @@ class Command(BaseRunSpiderCommand): self.first_response = response cb = self._get_callback(spider=spider, opts=opts, response=response) + assert response.request + response.request.callback = cb # parse items and requests depth: int = response.meta["_depth"] diff --git a/tests/test_command_parse.py b/tests/test_command_parse.py index 9b7131c7a..772cc82e2 100644 --- a/tests/test_command_parse.py +++ b/tests/test_command_parse.py @@ -126,6 +126,32 @@ class MySpider(scrapy.Spider): else: self.logger.debug('It Does Not Work :(') +class RetryRequestSpider(BaseSpider): + name = 'retry_request' + + def parse(self, response): + if response.meta.get('retried'): + yield {{'retried': True}} + return + response.meta['retried'] = True + yield response.request.replace(dont_filter=True) + +class CustomCallbackRetryRequestSpider(BaseSpider): + name = 'retry_request_custom_callback' + + def parse(self, response): + yield response.request.replace( + callback=self.parse_retry, + dont_filter=True, + ) + + def parse_retry(self, response): + if response.meta.get('retried'): + yield {{'retried_with_custom_callback': True}} + return + response.meta['retried'] = True + yield response.request.replace(dont_filter=True) + class MyGoodCrawlSpider(CrawlSpider): name = 'goodcrawl{self.spider_name}' @@ -381,6 +407,36 @@ ITEM_PIPELINES = {{'{self.project_name}.pipelines.MyPipeline': 1}} ) assert "[{}, {'foo': 'bar'}]" in out + def test_retry_response_request( + self, proj_path: Path, mockserver: MockServer + ) -> None: + _, out, stderr = proc( + "parse", + "--spider", + "retry_request", + "-d", + "2", + mockserver.url("/html"), + cwd=proj_path, + ) + assert "RecursionError" not in stderr + assert "{'retried': True}" in out + + def test_retry_response_request_with_custom_callback( + self, proj_path: Path, mockserver: MockServer + ) -> None: + _, out, stderr = proc( + "parse", + "--spider", + "retry_request_custom_callback", + "-d", + "3", + mockserver.url("/html"), + cwd=proj_path, + ) + assert "RecursionError" not in stderr + assert "{'retried_with_custom_callback': True}" in out + def test_wrong_callback_passed( self, proj_path: Path, mockserver: MockServer ) -> None: From b2d4eedea8873700b2597a44dabafe7c9d169275 Mon Sep 17 00:00:00 2001 From: Fandu <113630375+mrfandu1@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:15:03 +0545 Subject: [PATCH 09/25] Fix immediate delivery of full feed export batches (#7730) (#7733) * Store full feed batches before spider closes (#7730) Start closing and storing each batch as soon as it reaches the configured item count. Track unfinished close tasks so spider shutdown still waits for all deliveries before emitting the exporter-closed signal. Add an end-to-end regression test that verifies the first batch is stored while the crawl is still running. * Remove the issue reference --------- Co-authored-by: Andrey Rakhmatullin --- scrapy/extensions/feedexport.py | 39 +++++++++++++++++++++++-------- tests/test_feedexport_batch.py | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 678a29e2e..c2997921d 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -13,7 +13,7 @@ import re import sys import warnings from abc import ABC, abstractmethod -from collections.abc import Callable, Coroutine +from collections.abc import Callable from datetime import datetime, timezone from pathlib import Path, PureWindowsPath from tempfile import NamedTemporaryFile @@ -475,7 +475,7 @@ class FeedExporter: self.feeds = {} self.slots: list[FeedSlot] = [] self.filters: dict[str, ItemFilter] = {} - self._pending_close_coros: list[Coroutine[Any, Any, None]] = [] + self._pending_close_tasks: list[asyncio.Task[None] | Deferred[None]] = [] if not self.settings["FEEDS"] and not self.settings["FEED_URI"]: raise NotConfigured @@ -539,23 +539,44 @@ class FeedExporter: ) async def close_spider(self, spider: Spider) -> None: - self._pending_close_coros.extend( - self._close_slot(slot, spider) for slot in self.slots - ) + for slot in self.slots: + self._schedule_slot_close(slot, spider) - if self._pending_close_coros: + if self._pending_close_tasks: if is_asyncio_available(): await asyncio.wait( - [asyncio.create_task(coro) for coro in self._pending_close_coros] + cast("list[asyncio.Task[None]]", list(self._pending_close_tasks)) ) else: await DeferredList( - deferred_from_coro(coro) for coro in self._pending_close_coros + cast("list[Deferred[None]]", list(self._pending_close_tasks)) ) # Send FEED_EXPORTER_CLOSED signal await self.crawler.signals.send_catch_log_async(signals.feed_exporter_closed) + def _schedule_slot_close( + self, slot: FeedSlot, spider: Spider + ) -> asyncio.Task[None] | Deferred[None]: + """Start closing the slot without waiting for it to finish, keeping + track of the pending work so that it can be awaited in + :meth:`close_spider` if it hasn't finished by then.""" + aw: asyncio.Task[None] | Deferred[None] + coro = self._close_slot(slot, spider) + if is_asyncio_available(): + aw = asyncio.create_task(coro) + self._pending_close_tasks.append(aw) + aw.add_done_callback(self._pending_close_tasks.remove) + else: + aw = deferred_from_coro(coro) + self._pending_close_tasks.append(aw) + aw.addBoth(self._untrack_pending_close_task, aw) + return aw + + def _untrack_pending_close_task(self, result: Any, aw: Deferred[None]) -> Any: + self._pending_close_tasks.remove(aw) + return result + @staticmethod def _get_file(slot_: FeedSlot) -> IO[bytes]: assert slot_.file @@ -652,7 +673,7 @@ class FeedExporter: uri_params = self._get_uri_params( spider, self.feeds[slot.uri_template]["uri_params"], slot ) - self._pending_close_coros.append(self._close_slot(slot, spider)) + self._schedule_slot_close(slot, spider) slots.append( self._start_new_batch( batch_id=slot.batch_id + 1, diff --git a/tests/test_feedexport_batch.py b/tests/test_feedexport_batch.py index 80ff6229b..4b0962c43 100644 --- a/tests/test_feedexport_batch.py +++ b/tests/test_feedexport_batch.py @@ -210,6 +210,47 @@ class TestBatchDeliveries(TestFeedExportBase): header = MyItem.fields.keys() await self.assertExported(items, header, rows, settings=settings) + @coroutine_test + async def test_batch_delivered_when_full(self): + """Full batches must be finalized and delivered as soon as they are + full, instead of when the spider closes.""" + dir_path = self._random_temp_filename() + batch1_path = Path(dir_path, "1.json") + mockserver_url = self.mockserver.url("/") + batch1_contents: list[bytes | None] = [] + + class TestSpider(scrapy.Spider): + name = "testspider" + start_urls = [mockserver_url] + + def parse(self, response): + yield {"foo": "bar1"} + yield {"foo": "bar2"} + yield scrapy.Request( + mockserver_url, callback=self.parse2, dont_filter=True + ) + + def parse2(self, response): + # the first batch was full after the second item, so it must + # have been delivered by now + batch1_contents.append( + batch1_path.read_bytes() if batch1_path.exists() else None + ) + yield {"foo": "bar3"} + + settings = { + "FEEDS": { + build_url(dir_path / "%(batch_id)d.json"): {"format": "json"}, + }, + "FEED_EXPORT_BATCH_ITEM_COUNT": 2, + } + crawler = get_crawler(TestSpider, settings) + await crawler.crawl_async() + + assert batch1_contents, "the second request was not processed" + assert batch1_contents[0] is not None, "batch 1 was not stored during the crawl" + assert json.loads(batch1_contents[0]) == [{"foo": "bar1"}, {"foo": "bar2"}] + def test_wrong_path(self): """If path is without %(batch_time)s and %(batch_id) an exception must be raised""" settings = { From 5b4828a012fcd136a8f46915e19be07a5029e57e Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Thu, 30 Jul 2026 03:02:54 -0300 Subject: [PATCH 10/25] docs(practices): Remove scrapoxy mention (#7817) --- docs/topics/practices.rst | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index 23738c98c..dfa1e21f6 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -533,8 +533,7 @@ Here are some tips to keep in mind when dealing with these kinds of sites: * if possible, use `Common Crawl`_ to fetch pages, instead of hitting the sites directly * use a pool of rotating IPs. For example, the free `Tor project`_ or paid - services like `ProxyMesh`_. An open source alternative is `scrapoxy`_, a - super proxy that you can attach your own proxies to. + services like `ProxyMesh`_. * for HTTPS websites, if blocking appears related to TLS behavior, consider adjusting the :setting:`DOWNLOAD_TLS_MIN_VERSION` and :setting:`DOWNLOAD_TLS_MAX_VERSION` settings, since some websites may respond @@ -559,5 +558,4 @@ projects that detects common mistakes and anti-patterns. .. _ProxyMesh: https://proxymesh.com/ .. _Common Crawl: https://commoncrawl.org/ .. _testspiders: https://github.com/scrapinghub/testspiders -.. _scrapoxy: https://scrapoxy.io/ .. _Zyte API: https://docs.zyte.com/zyte-api/get-started.html From 98696efa809ddde93f78cabd555db832f270553f Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 30 Jul 2026 12:46:45 +0200 Subject: [PATCH 11/25] Export item fields in declaration order (#7824) --- docs/topics/exporters.rst | 10 ++++++++++ scrapy/exporters.py | 17 ++++++++++++++++- tests/test_exporters.py | 12 ++++++++++++ tests/test_feedexport.py | 12 ++++++------ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/docs/topics/exporters.rst b/docs/topics/exporters.rst index ecd154122..c43b7e20f 100644 --- a/docs/topics/exporters.rst +++ b/docs/topics/exporters.rst @@ -211,6 +211,16 @@ BaseItemExporter - ``None`` (all fields [2]_, default) + Fields are exported in declaration order, i.e. the order in which + they are defined in the :ref:`item class `. For + :class:`dict` items, which have no declared fields, the key order of + each item is used instead. + + .. versionchanged:: VERSION + Fields of non-\ :class:`dict` items used to be exported in the + order in which they had been populated, except in + :class:`CsvItemExporter`, which has always used declaration order. + - A list of fields: .. code-block:: python diff --git a/scrapy/exporters.py b/scrapy/exporters.py index 5a11df833..ea600d1a8 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -74,6 +74,17 @@ class BaseItemExporter(ABC): def finish_exporting(self) -> None: # noqa: B027 pass + @staticmethod + def _get_populated_field_names(adapter: ItemAdapter) -> Iterable[str]: + """Return the populated field names of *adapter*, in declaration order. + + Populated fields that are not declared, which some item types allow, + come last, in item order. + """ + populated = set(adapter.keys()) + declared = (name for name in adapter.field_names() if name in populated) + return dict.fromkeys([*declared, *adapter.keys()]) + def _get_serialized_fields( self, item: Any, default_value: Any = None, include_empty: bool | None = None ) -> Iterable[tuple[str, Any]]: @@ -86,7 +97,11 @@ class BaseItemExporter(ABC): include_empty = self.export_empty_fields if self.fields_to_export is None: - field_iter = item.field_names() if include_empty else item.keys() + field_iter = ( + item.field_names() + if include_empty + else self._get_populated_field_names(item) + ) elif isinstance(self.fields_to_export, Mapping): if include_empty: field_iter = self.fields_to_export.items() diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 3b997767e..b857728ba 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -118,6 +118,18 @@ class TestBaseItemExporter(ABC): ie = self._get_exporter(fields_to_export={"name": "名稱"}) assert list(ie._get_serialized_fields(self.i)) == [("名稱", "John\xa3")] + def test_field_order(self): + item = self.item_class(age="22", name="John\xa3") + ie = self._get_exporter() + assert [name for name, _ in ie._get_serialized_fields(item)] == ["name", "age"] + + def test_field_order_dict_item(self): + ie = self._get_exporter() + assert [name for name, _ in ie._get_serialized_fields({"age": "22"})] == ["age"] + assert [ + name for name, _ in ie._get_serialized_fields({"age": "22", "name": "John"}) + ] == ["age", "name"] + def test_field_custom_serializer(self): i = self.custom_field_item_class(name="John\xa3", age="22") a = ItemAdapter(i) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 92c414bef..1cca287af 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -675,14 +675,14 @@ class TestFeedExport(TestFeedExportBase): formats = { "csv": b"foo,egg,baz\r\nbar1,spam1,\r\n", - "json": b'[\n{"hello": "world2", "foo": "bar2"}\n]', + "json": b'[\n{"foo": "bar2", "hello": "world2"}\n]', "jsonlines": ( - b'{"foo": "bar1", "egg": "spam1"}\n{"hello": "world2", "foo": "bar2"}\n' + b'{"foo": "bar1", "egg": "spam1"}\n{"foo": "bar2", "hello": "world2"}\n' ), "xml": ( b'\n\n' - b"bar1spam1\n" - b"world2bar2\nworld3" + b"bar1spam1\n" + b"bar2world2\nworld3" b"spam3\n" ), } @@ -740,8 +740,8 @@ class TestFeedExport(TestFeedExportBase): "json": b'[\n{"foo": "bar1", "egg": "spam1"}\n]', "xml": ( b'\n\n' - b"bar1spam1\n" - b"world2bar2\n" + b"bar1spam1\n" + b"bar2world2\n" ), "jsonlines": b'{"foo": "bar1", "egg": "spam1"}\n', } From 433603e6cab4ccea6aee18843deaa03f61e216b8 Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 30 Jul 2026 15:51:45 +0200 Subject: [PATCH 12/25] Add AWS_MAX_POOL_CONNECTIONS (#7794) --- docs/topics/feed-exports.rst | 5 +++-- docs/topics/media-pipeline.rst | 3 +++ docs/topics/settings.rst | 20 +++++++++++++++++ scrapy/extensions/feedexport.py | 11 +++++++++ scrapy/pipelines/files.py | 13 ++++++++++- scrapy/settings/default_settings.py | 2 ++ scrapy/utils/boto.py | 15 +++++++++++++ tests/test_feedexport_storages.py | 35 +++++++++++++++++++++++++++++ tests/test_pipeline_files.py | 27 ++++++++++++++++++++++ 9 files changed, 128 insertions(+), 3 deletions(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 66768c97b..2f686fd0f 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -218,12 +218,13 @@ passed through the following settings: .. _temporary security credentials: https://docs.aws.amazon.com/IAM/latest/UserGuide/security-creds.html -You can also define a custom ACL, custom endpoint, and region name for exported -feeds using these settings: +You can also define a custom ACL, custom endpoint, region name and connection +pool size for exported feeds using these settings: - :setting:`FEED_STORAGE_S3_ACL` - :setting:`AWS_ENDPOINT_URL` - :setting:`AWS_REGION_NAME` +- :setting:`AWS_MAX_POOL_CONNECTIONS` The default value for the ``overwrite`` key in the :setting:`FEEDS` for this storage backend is: ``True``. diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 4ceb4732a..b16066d0c 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -268,6 +268,9 @@ For self-hosting you also might feel the need not to use SSL and not to verify S AWS_USE_SSL = False # or True (None by default) AWS_VERIFY = False # or True (None by default) +To reuse connections for as many files as you check or upload in parallel, set +:setting:`AWS_MAX_POOL_CONNECTIONS` accordingly. + .. _canned ACLs: https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html#canned-acl .. _Minio: https://github.com/minio/minio .. _Zenko CloudServer: https://www.zenko.io/cloudserver/ diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 8068824e3..1b6851d04 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -458,6 +458,26 @@ Default: ``None`` Endpoint URL used for S3-like storage, for example Minio or s3.scality. +.. setting:: AWS_MAX_POOL_CONNECTIONS + +AWS_MAX_POOL_CONNECTIONS +------------------------ + +.. versionadded:: VERSION + +Default: ``None`` + +Maximum number of connections that AWS clients, such as those of the +:ref:`S3 feed storage backend ` and of the +:ref:`S3 media pipeline storage backend `, keep in their +connection pool. + +If ``None``, the value of :setting:`REACTOR_THREADPOOL_MAXSIZE` is used. + +Values lower than the number of parallel AWS calls do not limit those calls, but +their connections are closed instead of reused, which hurts performance, and +``Connection pool is full, discarding connection`` warnings are logged. + .. setting:: AWS_REGION_NAME AWS_REGION_NAME diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index c2997921d..448279546 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -28,6 +28,7 @@ from scrapy import Spider, signals from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.extensions.postprocessing import PostProcessingManager from scrapy.utils.asyncio import is_asyncio_available, run_in_thread +from scrapy.utils.boto import _get_max_pool_connections from scrapy.utils.conf import feed_complete_default_values_from_settings from scrapy.utils.defer import deferred_from_coro, ensure_awaitable from scrapy.utils.ftp import ftp_store_file @@ -213,11 +214,14 @@ class S3FeedStorage(BlockingFeedStorage): feed_options: dict[str, Any] | None = None, session_token: str | None = None, region_name: str | None = None, + max_pool_connections: int | None = None, ): try: import boto3.session # noqa: PLC0415 except ImportError: raise NotConfigured("missing boto3 library") from None + from botocore.config import Config # noqa: PLC0415 + u = urlparse(uri) assert u.hostname self.bucketname: str = u.hostname @@ -228,6 +232,7 @@ class S3FeedStorage(BlockingFeedStorage): self.acl: str | None = acl self.endpoint_url: str | None = endpoint_url self.region_name: str | None = region_name + self.max_pool_connections: int | None = max_pool_connections boto3_session = boto3.session.Session() self.s3_client = boto3_session.client( @@ -237,6 +242,11 @@ class S3FeedStorage(BlockingFeedStorage): aws_session_token=self.session_token, endpoint_url=self.endpoint_url, region_name=self.region_name, + config=( + Config(max_pool_connections=self.max_pool_connections) + if self.max_pool_connections is not None + else None + ), ) if feed_options and feed_options.get("overwrite", True) is False: @@ -262,6 +272,7 @@ class S3FeedStorage(BlockingFeedStorage): acl=crawler.settings["FEED_STORAGE_S3_ACL"] or None, endpoint_url=crawler.settings["AWS_ENDPOINT_URL"] or None, region_name=crawler.settings["AWS_REGION_NAME"] or None, + max_pool_connections=_get_max_pool_connections(crawler.settings), feed_options=feed_options, ) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 8e8082332..55a3676e5 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -37,7 +37,7 @@ from scrapy.pipelines.media import ( _MediaRequestFiltered, ) from scrapy.utils.asyncio import run_in_thread -from scrapy.utils.boto import is_botocore_available +from scrapy.utils.boto import _get_max_pool_connections, is_botocore_available from scrapy.utils.datatypes import CaseInsensitiveDict from scrapy.utils.defer import deferred_from_coro, ensure_awaitable from scrapy.utils.ftp import ftp_store_file @@ -164,6 +164,9 @@ class S3FilesStore: AWS_REGION_NAME = None AWS_USE_SSL = None AWS_VERIFY = None + # Overridden from settings.AWS_MAX_POOL_CONNECTIONS in + # FilesPipeline.from_crawler(); None means the botocore default + AWS_MAX_POOL_CONNECTIONS: int | None = None POLICY = "private" # Overridden from settings.FILES_STORE_S3_ACL in FilesPipeline.from_crawler() HEADERS: ClassVar[dict[str, str]] = { @@ -174,7 +177,13 @@ class S3FilesStore: if not is_botocore_available(): raise NotConfigured("missing botocore library") import botocore.session # noqa: PLC0415 + from botocore.config import Config # noqa: PLC0415 + config = ( + Config(max_pool_connections=self.AWS_MAX_POOL_CONNECTIONS) + if self.AWS_MAX_POOL_CONNECTIONS is not None + else None + ) session = botocore.session.get_session() self.s3_client = session.create_client( "s3", @@ -185,6 +194,7 @@ class S3FilesStore: region_name=self.AWS_REGION_NAME, use_ssl=self.AWS_USE_SSL, verify=self.AWS_VERIFY, + config=config, ) if not uri.startswith("s3://"): raise ValueError(f"Incorrect URI scheme in {uri}, expected 's3'") @@ -522,6 +532,7 @@ class FilesPipeline(MediaPipeline): s3store.AWS_REGION_NAME = settings["AWS_REGION_NAME"] s3store.AWS_USE_SSL = settings["AWS_USE_SSL"] s3store.AWS_VERIFY = settings["AWS_VERIFY"] + s3store.AWS_MAX_POOL_CONNECTIONS = _get_max_pool_connections(settings) s3store.POLICY = settings["FILES_STORE_S3_ACL"] gcs_store: type[GCSFilesStore] = cast( diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 993f2436d..a44b36c8a 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -28,6 +28,7 @@ __all__ = [ "AUTOTHROTTLE_TARGET_CONCURRENCY", "AWS_ACCESS_KEY_ID", "AWS_ENDPOINT_URL", + "AWS_MAX_POOL_CONNECTIONS", "AWS_REGION_NAME", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN", @@ -229,6 +230,7 @@ AUTOTHROTTLE_TARGET_CONCURRENCY = 1.0 AWS_ACCESS_KEY_ID = None AWS_SECRET_ACCESS_KEY = None AWS_ENDPOINT_URL = None +AWS_MAX_POOL_CONNECTIONS = None AWS_REGION_NAME = None AWS_SESSION_TOKEN = None AWS_USE_SSL = None diff --git a/scrapy/utils/boto.py b/scrapy/utils/boto.py index 2a77ee2ac..76ee0e7ec 100644 --- a/scrapy/utils/boto.py +++ b/scrapy/utils/boto.py @@ -1,7 +1,22 @@ """Boto/botocore helpers""" +from __future__ import annotations + from importlib.util import find_spec +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from scrapy.settings import BaseSettings def is_botocore_available() -> bool: return find_spec("botocore") is not None + + +def _get_max_pool_connections(settings: BaseSettings) -> int: + """Return the maximum number of connections that AWS clients may keep in + their connection pool. + """ + return settings.getint("AWS_MAX_POOL_CONNECTIONS") or settings.getint( + "REACTOR_THREADPOOL_MAXSIZE" + ) diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index 66488540f..b1e3787fd 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -381,6 +381,41 @@ class TestS3FeedStorage: assert storage.region_name == region_name assert storage.s3_client._client_config.region_name == region_name + def test_init_without_max_pool_connections(self) -> None: + storage = S3FeedStorage("s3://mybucket/export.csv", "access_key", "secret_key") + assert storage.max_pool_connections is None + config: Any = storage.s3_client.meta.config + assert config.max_pool_connections == 10 + + def test_init_with_max_pool_connections(self) -> None: + storage = S3FeedStorage( + "s3://mybucket/export.csv", + "access_key", + "secret_key", + max_pool_connections=30, + ) + assert storage.max_pool_connections == 30 + config: Any = storage.s3_client.meta.config + assert config.max_pool_connections == 30 + + @pytest.mark.parametrize( + ("settings", "expected"), + [ + ({}, 10), + ({"REACTOR_THREADPOOL_MAXSIZE": 20}, 20), + ({"AWS_MAX_POOL_CONNECTIONS": 30}, 30), + ({"AWS_MAX_POOL_CONNECTIONS": 30, "REACTOR_THREADPOOL_MAXSIZE": 20}, 30), + ], + ) + def test_from_crawler_max_pool_connections( + self, settings: dict[str, Any], expected: int + ) -> None: + crawler = get_crawler(settings_dict=settings) + storage = S3FeedStorage.from_crawler(crawler, "s3://mybucket/export.csv") + assert storage.max_pool_connections == expected + config: Any = storage.s3_client.meta.config + assert config.max_pool_connections == expected + @coroutine_test async def test_store_without_acl(self): storage = S3FeedStorage( diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 97daa514e..4e7fb118b 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -895,6 +895,33 @@ class TestS3FilesStore: stub.assert_no_pending_responses() + def test_default_max_pool_connections(self) -> None: + store = S3FilesStore("s3://mybucket/prefix/") + config: Any = store.s3_client.meta.config + assert config.max_pool_connections == 10 + + @pytest.mark.parametrize( + ("settings", "expected"), + [ + ({}, 10), + ({"REACTOR_THREADPOOL_MAXSIZE": 20}, 20), + ({"AWS_MAX_POOL_CONNECTIONS": 30}, 30), + ({"AWS_MAX_POOL_CONNECTIONS": 30, "REACTOR_THREADPOOL_MAXSIZE": 20}, 30), + ], + ) + def test_max_pool_connections( + self, monkeypatch: pytest.MonkeyPatch, settings: dict[str, Any], expected: int + ) -> None: + # restores the value that FilesPipeline.from_crawler() sets on the class + monkeypatch.setattr(S3FilesStore, "AWS_MAX_POOL_CONNECTIONS", None) + crawler = get_crawler( + settings_dict={"FILES_STORE": "s3://mybucket/prefix/", **settings} + ) + store = FilesPipeline.from_crawler(crawler).store + assert isinstance(store, S3FilesStore) + config: Any = store.s3_client.meta.config + assert config.max_pool_connections == expected + class TestGCSFilesStore: @staticmethod From f02a99fe71dd0ff2fde1ef7a533cd61904fa1c9f Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 30 Jul 2026 17:15:15 +0200 Subject: [PATCH 13/25] Add doc sections for callbacks and errbacks (#7821) --- docs/faq.rst | 2 +- docs/intro/tutorial.rst | 2 +- docs/topics/coroutines.rst | 4 +- docs/topics/jobs.rst | 10 +- docs/topics/request-response.rst | 480 ++++++++++++++++++++----------- docs/topics/spiders.rst | 67 ++--- scrapy/http/request/__init__.py | 8 +- scrapy/spiders/__init__.py | 16 ++ 8 files changed, 372 insertions(+), 217 deletions(-) diff --git a/docs/faq.rst b/docs/faq.rst index 0446a6868..1a574e5da 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -97,7 +97,7 @@ handler documentation. How can I scrape an item with attributes in different pages? ------------------------------------------------------------ -See :ref:`topics-request-response-ref-request-callback-arguments`. +See :ref:`callback-data`. How can I simulate a user login in my spider? --------------------------------------------- diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index c4e04364b..eaf492c95 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -769,7 +769,7 @@ crawlers on top of it. Also, a common pattern is to build an item with data from more than one page, using a :ref:`trick to pass additional data to the callbacks -`. +`. Using spider arguments diff --git a/docs/topics/coroutines.rst b/docs/topics/coroutines.rst index 9dcd9d69c..b7ddb0a57 100644 --- a/docs/topics/coroutines.rst +++ b/docs/topics/coroutines.rst @@ -21,7 +21,9 @@ hence use coroutine syntax (e.g. ``await``, ``async for``, ``async with``): .. versionadded:: 2.13 -- :class:`~scrapy.Request` callbacks. +- :class:`~scrapy.Request` :ref:`callbacks `, which may + also be defined as :term:`asynchronous generators `. - The :meth:`process_item` method of :ref:`item pipelines `. diff --git a/docs/topics/jobs.rst b/docs/topics/jobs.rst index c9916110d..dcff10772 100644 --- a/docs/topics/jobs.rst +++ b/docs/topics/jobs.rst @@ -96,9 +96,13 @@ Request serialization --------------------- For persistence to work, :class:`~scrapy.Request` objects must be -serializable with :mod:`pickle`, except for the ``callback`` and ``errback`` -values passed to their ``__init__`` method, which must be methods of the -running :class:`~scrapy.Spider` class. +serializable with :mod:`pickle`, except for the :ref:`callback +` and :ref:`errback +` values passed to their ``__init__`` +method, which must be methods of the running :class:`~scrapy.Spider` class. + +Requests that cannot be serialized are kept in memory only: they are still +sent, but they are lost when the crawl is paused. If you wish to log the requests that couldn't be serialized, you can set the :setting:`SCHEDULER_DEBUG` setting to ``True`` in the project's settings page. diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 8e565907f..75158440b 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -205,10 +205,11 @@ Request objects Request metadata can also be accessed through the :attr:`~scrapy.http.Response.meta` attribute of a response. - To pass data from one spider callback to another, consider using - :attr:`cb_kwargs` instead. However, request metadata may be the right - choice in certain scenarios, such as to maintain some debugging data - across all follow-up requests (e.g. the source URL). + To pass your own data from one spider callback to another, use + :attr:`cb_kwargs` instead, see :ref:`callback-data`. However, request + metadata may be the right choice in certain scenarios, such as to + maintain some debugging data across all follow-up requests (e.g. the + source URL). A common use of request metadata is to define request-specific parameters for Scrapy components (extensions, middlewares, etc.). For @@ -248,7 +249,7 @@ Request objects .. method:: Request.copy() Return a new Request which is a copy of this Request. See also: - :ref:`topics-request-response-ref-request-callback-arguments`. + :ref:`callback-data`. .. method:: Request.replace([url, method, headers, body, cookies, meta, flags, encoding, priority, dont_filter, callback, errback, cb_kwargs, cls]) @@ -256,7 +257,7 @@ Request objects given new values by whichever keyword arguments are specified. The :attr:`~scrapy.Request.cb_kwargs` and :attr:`~scrapy.Request.meta` attributes are shallow copied by default (unless new values are given as arguments). See also - :ref:`topics-request-response-ref-request-callback-arguments`. + :ref:`callback-data`. .. automethod:: from_curl @@ -347,160 +348,6 @@ Other functions related to requests .. autofunction:: scrapy.utils.httpobj.urlparse_cached -.. _topics-request-response-ref-request-callback-arguments: - -Passing additional data to callback functions ---------------------------------------------- - -The callback of a request is a function that will be called when the response -of that request is downloaded. The callback function will be called with the -downloaded :class:`Response` object as its first argument. - -Example: - -.. code-block:: python - - def parse_page1(self, response): - return scrapy.Request( - "http://www.example.com/some_page.html", callback=self.parse_page2 - ) - - - def parse_page2(self, response): - # this would log http://www.example.com/some_page.html - self.logger.info("Visited %s", response.url) - -In some cases you may be interested in passing arguments to those callback -functions so you can receive the arguments later, in the second callback. -The following example shows how to achieve this by using the -:attr:`.Request.cb_kwargs` attribute: - -.. code-block:: python - - def parse(self, response): - request = scrapy.Request( - "http://www.example.com/index.html", - callback=self.parse_page2, - cb_kwargs=dict(main_url=response.url), - ) - request.cb_kwargs["foo"] = "bar" # add more arguments for the callback - yield request - - - def parse_page2(self, response, main_url, foo): - yield dict( - main_url=main_url, - other_url=response.url, - foo=foo, - ) - -.. caution:: :attr:`.Request.cb_kwargs` was introduced in version ``1.7``. - Prior to that, using :attr:`.Request.meta` was recommended for passing - information around callbacks. After ``1.7``, :attr:`.Request.cb_kwargs` - became the preferred way for handling user information, leaving :attr:`.Request.meta` - for communication with components like middlewares and extensions. - -.. _topics-request-response-ref-errbacks: - -Using errbacks to catch exceptions in request processing --------------------------------------------------------- - -The errback of a request is a function that will be called when an exception -is raise while processing it. - -It receives a :exc:`~twisted.python.failure.Failure` as first parameter and can -be used to track connection establishment timeouts, DNS errors etc. - -Here's an example spider logging all errors and catching some specific -errors if needed: - -.. code-block:: python - - import scrapy - - from scrapy.spidermiddlewares.httperror import HttpError - from twisted.internet.error import DNSLookupError - from twisted.internet.error import TimeoutError, TCPTimedOutError - - - class ErrbackSpider(scrapy.Spider): - name = "errback_example" - start_urls = [ - "http://www.httpbin.org/", # HTTP 200 expected - "http://www.httpbin.org/status/404", # Not found error - "http://www.httpbin.org/status/500", # server issue - "http://www.httpbin.org:12345/", # non-responding host, timeout expected - "https://example.invalid/", # DNS error expected - ] - - async def start(self): - for u in self.start_urls: - yield scrapy.Request( - u, - callback=self.parse_httpbin, - errback=self.errback_httpbin, - dont_filter=True, - ) - - def parse_httpbin(self, response): - self.logger.info(f"Got successful response from {response.url}") - # do something useful here... - - def errback_httpbin(self, failure): - # log all failures - self.logger.error(repr(failure)) - - # in case you want to do something special for some errors, - # you may need the failure's type: - - if failure.check(HttpError): - # these exceptions come from HttpError spider middleware - # you can get the non-200 response - response = failure.value.response - self.logger.error("HttpError on %s", response.url) - - elif failure.check(DNSLookupError): - # this is the original request - request = failure.request - self.logger.error("DNSLookupError on %s", request.url) - - elif failure.check(TimeoutError, TCPTimedOutError): - request = failure.request - self.logger.error("TimeoutError on %s", request.url) - - -.. _errback-cb_kwargs: - -Accessing additional data in errback functions ----------------------------------------------- - -In case of a failure to process the request, you may be interested in -accessing arguments to the callback functions so you can process further -based on the arguments in the errback. The following example shows how to -achieve this by using ``Failure.request.cb_kwargs``: - -.. code-block:: python - - def parse(self, response): - request = scrapy.Request( - "http://www.example.com/index.html", - callback=self.parse_page2, - errback=self.errback_page2, - cb_kwargs=dict(main_url=response.url), - ) - yield request - - - def parse_page2(self, response, main_url): - pass - - - def errback_page2(self, failure): - yield dict( - main_url=failure.request.cb_kwargs["main_url"], - ) - - .. _request-fingerprints: Request fingerprints @@ -702,6 +549,319 @@ The following built-in Scrapy components have such restrictions: 45-character-long keys must be supported. +.. _callbacks: + +Callbacks +========= + +A callback is a function that Scrapy calls with the :class:`Response` of a +:class:`~scrapy.Request` once that request has been downloaded, so that you can +extract data from that response and generate additional requests to continue +the crawl: + +.. code-block:: python + + from scrapy import Request, Spider + + + class BookSpider(Spider): + name = "books" + + async def start(self): + yield Request("https://books.toscrape.com/", callback=self.parse_home) + + def parse_home(self, response): + for url in response.css("h3 a::attr(href)").getall(): + yield Request(response.urljoin(url), callback=self.parse_book) + + def parse_book(self, response): + yield {"title": response.css("h1::text").get()} + +Requests may also define an :ref:`errback `, which Scrapy calls +instead of the callback when an exception is raised while processing the +request or its response, e.g. a connection error or, by default, a non-2xx +response. + + +.. _callback-assignment: + +Assigning a callback to a request +--------------------------------- + +To assign a callback to a request, use the ``callback`` parameter of +:class:`~scrapy.Request`, which sets the :attr:`.Request.callback` attribute: + +.. code-block:: python + + from scrapy import Request + + + def parse_home(response): ... + + + request = Request("https://books.toscrape.com/", callback=parse_home) + +Requests with no callback, i.e. with :attr:`~scrapy.Request.callback` set to +``None``, are handled by the :meth:`~scrapy.Spider.parse` method of the spider: + +.. code-block:: python + + request = Request("https://books.toscrape.com/") # Handled by parse() + +If a request is never meant to reach a spider callback, e.g. because a +:ref:`component ` sends it and handles its response itself, +assign the special :func:`~scrapy.http.request.NO_CALLBACK` value to it +instead, so that :ref:`downloader middlewares ` +can tell such requests apart. + +While :attr:`~scrapy.Request.callback` only accepts callables, some spider +classes let you also define a callback by name: both :attr:`CrawlSpider.rules +` and :attr:`SitemapSpider.sitemap_rules +` accept the name of a spider +method as a string. + + +.. _writing-callbacks: + +Writing a callback +------------------ + +Any callable can be a callback, as long as it takes the response as its first +positional parameter, and any :ref:`additional callback data ` +as keyword parameters. Spider methods are the most common choice, but plain +functions, lambda expressions and other callable objects work as well. + +.. note:: If you enable :ref:`job persistence ` through the + :setting:`JOBDIR` setting, callbacks must be methods of the running spider. + Requests with any other callback cannot be serialized, so they are kept in + memory only and lost when you pause the crawl. See + :ref:`request-serialization`. + +A callback can be: + +- A regular function: + + .. code-block:: python + + def parse(self, response): + return {"url": response.url} + +- A generator function: + + .. code-block:: python + + def parse(self, response): + yield {"url": response.url} + +- A coroutine function, i.e. defined with ``async def``: + + .. code-block:: python + + async def parse(self, response): + return {"url": response.url} + +- An asynchronous generator function: + + .. code-block:: python + + async def parse(self, response): + yield {"url": response.url} + +The last two allow using ``await``, ``async for`` and ``async with`` in your +callback. See :ref:`topics-coroutines`. + + +.. _callback-output: + +Callback output +--------------- + +A callback may return or yield any of the following: + +- ``None``, which does nothing. + + Callbacks that produce no output at all, e.g. callbacks that only log + information about the response, are perfectly valid. ``None`` values within + an iterable of callback output are ignored as well. + +- A :class:`~scrapy.Request` object, which Scrapy schedules, downloads and + eventually sends to its own callback. + +- An :ref:`item object `, which Scrapy sends to the + :ref:`item pipelines `. + + Any object that is neither ``None`` nor a :class:`~scrapy.Request` object + is treated as an item. + +- An iterable of any of the values above, e.g. a list or, more commonly, a + generator. + + :term:`Asynchronous iterables `, e.g. an + :term:`asynchronous generator`, are also supported. + +.. note:: When a callback *returns* an object, Scrapy iterates that object if + it supports iteration, except for :class:`dict`, :class:`~scrapy.Item`, + :class:`str` and :class:`bytes` objects, which are always handled as single + items. + +.. note:: In a generator callback, a ``return`` statement with a value does not + produce any output, since such a value is not part of what the generator + yields. Scrapy logs a warning when it detects such a callback, see + :setting:`WARN_ON_GENERATOR_RETURN_VALUE`. + +Before Scrapy acts on the output of a callback, that output goes through the +:meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_spider_output` method +of your :ref:`spider middlewares `, which may modify +it or drop part of it. + +If a callback raises an exception, the :attr:`~scrapy.Request.errback` of the +request is *not* called. The exception goes through the +:meth:`~scrapy.spidermiddlewares.SpiderMiddleware.process_spider_exception` +method of your spider middlewares instead and, unless one of them handles it, +Scrapy logs it and sends the :signal:`spider_error` signal. + + +.. _callback-data: +.. _topics-request-response-ref-request-callback-arguments: + +Passing additional data to callback functions +--------------------------------------------- + +In some cases you may be interested in passing data to a callback in addition +to the response, e.g. data extracted from the response that triggered the +request. The following example shows how to achieve this by using the +:attr:`.Request.cb_kwargs` attribute: + +.. code-block:: python + + from scrapy import Request + + + def parse(self, response): + request = Request( + "http://www.example.com/index.html", + callback=self.parse_page2, + cb_kwargs=dict(main_url=response.url), + ) + request.cb_kwargs["foo"] = "bar" # add more arguments for the callback + yield request + + + def parse_page2(self, response, main_url, foo): + yield dict( + main_url=main_url, + other_url=response.url, + foo=foo, + ) + +:attr:`.Request.cb_kwargs` is the recommended way to pass your own data to a +callback. Use :attr:`.Request.meta` only for data aimed at :ref:`components +`, such as middlewares and extensions. + +.. _errbacks: +.. _topics-request-response-ref-errbacks: + +Errbacks +======== + +The errback of a request is a function that will be called when an exception +is raise while processing it. + +It receives a :exc:`~twisted.python.failure.Failure` as first parameter and can +be used to track connection establishment timeouts, DNS errors etc. + +Here's an example spider logging all errors and catching some specific +errors if needed: + +.. code-block:: python + + from scrapy import Request, Spider + from scrapy.spidermiddlewares.httperror import HttpError + from twisted.internet.error import DNSLookupError + from twisted.internet.error import TimeoutError, TCPTimedOutError + + + class ErrbackSpider(Spider): + name = "errback_example" + start_urls = [ + "http://www.httpbin.org/", # HTTP 200 expected + "http://www.httpbin.org/status/404", # Not found error + "http://www.httpbin.org/status/500", # server issue + "http://www.httpbin.org:12345/", # non-responding host, timeout expected + "https://example.invalid/", # DNS error expected + ] + + async def start(self): + for u in self.start_urls: + yield Request( + u, + callback=self.parse_httpbin, + errback=self.errback_httpbin, + dont_filter=True, + ) + + def parse_httpbin(self, response): + self.logger.info(f"Got successful response from {response.url}") + # do something useful here... + + def errback_httpbin(self, failure): + # log all failures + self.logger.error(repr(failure)) + + # in case you want to do something special for some errors, + # you may need the failure's type: + + if failure.check(HttpError): + # these exceptions come from HttpError spider middleware + # you can get the non-200 response + response = failure.value.response + self.logger.error("HttpError on %s", response.url) + + elif failure.check(DNSLookupError): + # this is the original request + request = failure.request + self.logger.error("DNSLookupError on %s", request.url) + + elif failure.check(TimeoutError, TCPTimedOutError): + request = failure.request + self.logger.error("TimeoutError on %s", request.url) + + +.. _errback-cb_kwargs: + +Accessing additional data in errback functions +---------------------------------------------- + +In case of a failure to process the request, you may be interested in +accessing arguments to the callback functions so you can process further +based on the arguments in the errback. The following example shows how to +achieve this by using ``Failure.request.cb_kwargs``: + +.. code-block:: python + + from scrapy import Request + + + def parse(self, response): + request = Request( + "http://www.example.com/index.html", + callback=self.parse_page2, + errback=self.errback_page2, + cb_kwargs=dict(main_url=response.url), + ) + yield request + + + def parse_page2(self, response, main_url): + pass + + + def errback_page2(self, failure): + yield dict( + main_url=failure.request.cb_kwargs["main_url"], + ) + + .. _topics-request-meta: Request.meta special keys diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 69ff08fa1..8fbf0c52d 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -4,43 +4,31 @@ Spiders ======= -Spiders are classes which define how a certain site (or a group of sites) will be -scraped, including how to perform the crawl (i.e. follow links) and how to -extract structured data from their pages (i.e. scraping items). In other words, -Spiders are the place where you define the custom behaviour for crawling and -parsing pages for a particular site (or, in some cases, a group of sites). +Spiders are classes that define how a site, or a group of sites, is scraped: +which requests to send, and how to parse their responses to extract data and to +send additional requests. -For spiders, the scraping cycle goes through something like this: +A crawl goes as follows: -1. You start by generating the initial requests to crawl the first URLs, and - specify a callback function to be called with the response downloaded from - those requests. +1. Scrapy iterates the :meth:`~scrapy.Spider.start` method of the spider to + get the initial requests. By default, that method yields a + :class:`~scrapy.Request` object for each URL in + :attr:`~scrapy.Spider.start_urls`, with :meth:`~scrapy.Spider.parse` as + :ref:`callback `. - The first requests to perform are obtained by iterating the - :meth:`~scrapy.Spider.start` method, which by default yields a - :class:`~scrapy.Request` object for each URL in the - :attr:`~scrapy.Spider.start_urls` spider attribute, with the - :attr:`~scrapy.Spider.parse` method set as :attr:`~scrapy.Request.callback` - function to handle each :class:`~scrapy.http.Response`. +2. Scrapy downloads each request and calls its callback with the resulting + :class:`~scrapy.http.Response`. -2. In the callback function, you parse the response (web page) and return - :ref:`item objects `, - :class:`~scrapy.Request` objects, or an iterable of these objects. - Those Requests will also contain a callback (maybe - the same) and will then be downloaded by Scrapy and then their - response handled by the specified callback. +3. Callbacks parse the response, typically using :ref:`topics-selectors`, and + return or yield :ref:`item objects ` with the extracted data + and :class:`~scrapy.Request` objects to continue the crawl, which go back + to step 2. See :ref:`callback-output`. -3. In callback functions, you parse the page contents, typically using - :ref:`topics-selectors` (but you can also use BeautifulSoup, lxml or whatever - mechanism you prefer) and generate items with the parsed data. +4. Items go through :ref:`item pipelines `, and are + usually stored through :ref:`topics-feed-exports`. -4. Finally, the items returned from the spider will be typically persisted to a - database (in some :ref:`Item Pipeline `) or written to - a file using :ref:`topics-feed-exports`. - -Even though this cycle applies (more or less) to any kind of spider, there are -different kinds of default spiders bundled into Scrapy for different purposes. -We will talk about those types here. +Scrapy includes different spider classes for different purposes, described +below. .. _topics-spiders-ref: @@ -191,22 +179,7 @@ scrapy.Spider .. automethod:: start - .. method:: parse(response) - - This is the default callback used by Scrapy to process downloaded - responses, when their requests don't specify a callback. - - The ``parse`` method is in charge of processing the response and returning - scraped data and/or more URLs to follow. Other Requests callbacks have - the same requirements as the :class:`~scrapy.Spider` class. - - This method, as well as any other Request callback, must return a - :class:`~scrapy.Request` object, an :ref:`item object `, an - iterable of :class:`~scrapy.Request` objects and/or :ref:`item objects - `, or ``None``. - - :param response: the response to parse - :type response: :class:`~scrapy.http.Response` + .. automethod:: parse .. method:: closed(reason) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 68847283a..7d67bb6d7 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -169,7 +169,8 @@ class Request(object_ref): #: #: The callable must expect the response as its first parameter, and #: support any additional keyword arguments set through - #: :attr:`cb_kwargs`. + #: :attr:`cb_kwargs`. See :ref:`writing-callbacks` and + #: :ref:`callback-output`. #: #: In addition to an arbitrary callable, the following values are also #: supported: @@ -190,8 +191,7 @@ class Request(object_ref): #: raises exceptions for non-2xx responses by default, sending them #: to the :attr:`errback` instead. #: - #: .. seealso:: - #: :ref:`topics-request-response-ref-request-callback-arguments` + #: .. seealso:: :ref:`callbacks` self.callback: CallbackT | None = callback #: :class:`~collections.abc.Callable` to handle exceptions raised @@ -200,7 +200,7 @@ class Request(object_ref): #: The callable must expect a :exc:`~twisted.python.failure.Failure` as #: its first parameter. #: - #: .. seealso:: :ref:`topics-request-response-ref-errbacks` + #: .. seealso:: :ref:`errbacks` self.errback: Callable[[Failure], Any] | None = errback self._cookies: CookiesT | None = cookies or None diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 02dfa2ac6..6244e3264 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -143,6 +143,22 @@ class Spider(object_ref): else: def parse(self, response: Response, **kwargs: Any) -> Any: + """Process *response*, i.e. extract data from it and generate new + requests. + + This is the default :ref:`callback `: Scrapy uses + it for the response to any request that does not define a + :attr:`~scrapy.Request.callback`, such as the requests that + :meth:`start` yields by default. + + Any :attr:`~scrapy.Request.cb_kwargs` of the request are passed as + keyword parameters. + + Spiders must define this method, unless every request that they + send defines a callback. + + See :ref:`callback-output` about the supported return values. + """ raise NotImplementedError( f"{self.__class__.__name__}.parse callback is not defined" ) From 3fc7148c5ec7537c000af27b705e33d797429a3a Mon Sep 17 00:00:00 2001 From: Janit Rajkarnikar <108281535+aniJani@users.noreply.github.com> Date: Thu, 30 Jul 2026 10:18:45 -0500 Subject: [PATCH 14/25] Don't evaluate annotations when inspecting signatures (#7818) Since Python 3.14 (PEP 649) annotations are evaluated lazily, so inspect.signature() raises NameError for callables whose annotations reference names imported only under TYPE_CHECKING. This broke middleware registration (via argument_is_required()) and custom stats collectors (via _warn_spider_arg) for user code with such annotations. Use annotation_format=Format.FORWARDREF on 3.14+: parameter names, kinds and defaults are unchanged, and unresolvable annotations become ForwardRef proxies instead of raising. Resolves #7796. --- scrapy/utils/decorators.py | 3 ++- scrapy/utils/python.py | 21 ++++++++++++++++++++- tests/test_utils_decorators.py | 30 ++++++++++++++++++++++++++++-- tests/test_utils_python.py | 21 ++++++++++++++++++++- 4 files changed, 70 insertions(+), 5 deletions(-) diff --git a/scrapy/utils/decorators.py b/scrapy/utils/decorators.py index 4960dc27a..2924c81f9 100644 --- a/scrapy/utils/decorators.py +++ b/scrapy/utils/decorators.py @@ -10,6 +10,7 @@ from twisted.internet.defer import Deferred, maybeDeferred from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.asyncio import run_in_thread from scrapy.utils.defer import deferred_from_coro +from scrapy.utils.python import _signature if TYPE_CHECKING: from collections.abc import AsyncGenerator, Callable, Coroutine @@ -109,7 +110,7 @@ def _warn_spider_arg( ): """Decorator to warn if a ``spider`` argument is passed to a function.""" - sig = inspect.signature(func) + sig = _signature(func) def check_args(*args: _P.args, **kwargs: _P.kwargs) -> None: bound = sig.bind(*args, **kwargs) diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 8a7517c1d..40fc05257 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -178,11 +178,30 @@ def binary_is_text(data: bytes) -> bool: return all(c not in _BINARYCHARS for c in data) +# PEP 649 (Python 3.14+) made annotation evaluation lazy, so inspect.signature() +# can raise NameError for names imported only under TYPE_CHECKING. We only need +# parameter names, kinds and defaults, so leave such annotations as ForwardRefs. +if sys.version_info >= (3, 14): + from annotationlib import Format + + def _signature(func: Callable[..., Any]) -> inspect.Signature: + return inspect.signature(func, annotation_format=Format.FORWARDREF) + +else: + + def _signature(func: Callable[..., Any]) -> inspect.Signature: + return inspect.signature(func) + + def get_func_args_dict( func: Callable[..., Any], stripself: bool = False ) -> Mapping[str, inspect.Parameter]: """Return the argument dict of a callable object. + Annotations are not evaluated, so on Python 3.14 and later the ``annotation`` + attribute of the returned parameters may be a ``ForwardRef`` instead of the + resolved type. + .. versionadded:: 2.14 """ if not callable(func): @@ -190,7 +209,7 @@ def get_func_args_dict( args: Mapping[str, inspect.Parameter] try: - sig = inspect.signature(func) + sig = _signature(func) except ValueError: return {} diff --git a/tests/test_utils_decorators.py b/tests/test_utils_decorators.py index 807294a57..4c29d2917 100644 --- a/tests/test_utils_decorators.py +++ b/tests/test_utils_decorators.py @@ -1,7 +1,8 @@ from __future__ import annotations +import sys import warnings -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any import pytest from twisted.internet.defer import Deferred @@ -12,7 +13,7 @@ from scrapy.utils.defer import maybe_deferred_to_future from tests.utils.decorators import coroutine_test if TYPE_CHECKING: - from collections.abc import AsyncGenerator + from collections.abc import AsyncGenerator, Callable class TestDeprecated: @@ -77,6 +78,31 @@ class TestWarnSpiderArg: ): assert parse("response", spider="spider") == "response" + @pytest.mark.skipif( + sys.version_info < (3, 14), + reason="annotations are only lazily evaluated since Python 3.14 (PEP 649)", + ) + def test_sync_warns_with_unresolvable_annotations(self): + # dont_inherit=True, or the module's future import stringizes the annotations + namespace: dict[str, Any] = {} + exec( # pylint: disable=exec-used + compile( + "def parse(response: OnlyAtTypeCheckingTime," + " spider: OnlyAtTypeCheckingTime | None = None): return response", + "", + "exec", + dont_inherit=True, + ), + namespace, + ) + parse_func: Callable[..., str] = namespace["parse"] + parse = _warn_spider_arg(parse_func) + + with pytest.warns( + ScrapyDeprecationWarning, match=r"Passing a 'spider' argument" + ): + assert parse("response", spider="spider") == "response" + def test_sync_no_warning_without_spider_arg(self): @_warn_spider_arg def parse(response: str, spider: str | None = None) -> str: diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index c3b5dfc99..099b5ccc2 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -4,7 +4,7 @@ import functools import operator import platform import sys -from typing import TYPE_CHECKING, TypeVar +from typing import TYPE_CHECKING, Any, TypeVar import pytest @@ -190,6 +190,25 @@ def test_get_func_args(): ] +@pytest.mark.skipif( + sys.version_info < (3, 14), + reason="annotations are only lazily evaluated since Python 3.14 (PEP 649)", +) +def test_get_func_args_unresolvable_annotations(): + # dont_inherit=True, or the module's future import stringizes the annotations + namespace: dict[str, Any] = {} + exec( # pylint: disable=exec-used + compile( + "def f(a: OnlyAtTypeCheckingTime, b: int = 1) -> OnlyAtTypeCheckingTime: pass", + "", + "exec", + dont_inherit=True, + ), + namespace, + ) + assert get_func_args(namespace["f"]) == ["a", "b"] + + @pytest.mark.parametrize( ("value", "expected"), [ From 7436afc95f521482b1b1473c88f6b5ab1d430bea Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 30 Jul 2026 17:27:27 +0200 Subject: [PATCH 15/25] Improve test coverage for scrapy.extensions (#7809) --- scrapy/extensions/closespider.py | 2 +- scrapy/extensions/feedexport.py | 4 +- scrapy/extensions/memusage.py | 2 +- scrapy/extensions/periodic_log.py | 3 +- tests/test_downloadermiddleware_httpcache.py | 71 ++++++++++++++++++++ tests/test_extension_memusage.py | 66 +++++++++++++++++- tests/test_extension_periodic_log.py | 36 ++++++++++ tests/test_extension_telnet.py | 55 ++++++++++++++- tests/test_feedexport.py | 26 +++++++ tests/test_feedexport_postprocess.py | 10 +++ tests/test_feedexport_storages.py | 15 +++++ 11 files changed, 278 insertions(+), 12 deletions(-) diff --git a/scrapy/extensions/closespider.py b/scrapy/extensions/closespider.py index a4362b182..9cb792e30 100644 --- a/scrapy/extensions/closespider.py +++ b/scrapy/extensions/closespider.py @@ -119,7 +119,7 @@ class CloseSpider: self.task = None if self.task_no_item: - if self.task_no_item.running: + if self.task_no_item.running: # pragma: no branch self.task_no_item.stop() self.task_no_item = None diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 448279546..118462bc9 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -465,7 +465,7 @@ class FeedSlot: ) def finish_exporting(self) -> None: - if self._exporting: + if self._exporting: # pragma: no branch assert self.exporter self.exporter.finish_exporting() self._exporting = False @@ -553,7 +553,7 @@ class FeedExporter: for slot in self.slots: self._schedule_slot_close(slot, spider) - if self._pending_close_tasks: + if self._pending_close_tasks: # pragma: no branch if is_asyncio_available(): await asyncio.wait( cast("list[asyncio.Task[None]]", list(self._pending_close_tasks)) diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index 1444c8941..e0e289ce8 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -94,7 +94,7 @@ class MemoryUsage: def engine_stopped(self) -> None: for tsk in self.tasks: - if tsk.running: + if tsk.running: # pragma: no branch tsk.stop() def update(self) -> None: diff --git a/scrapy/extensions/periodic_log.py b/scrapy/extensions/periodic_log.py index cd35c8165..cbcc8b70e 100644 --- a/scrapy/extensions/periodic_log.py +++ b/scrapy/extensions/periodic_log.py @@ -38,7 +38,6 @@ class PeriodicLog: ): self.stats: StatsCollector = stats self.interval: float = interval - self.multiplier: float = 60.0 / self.interval self.task: AsyncioLoopingCall | LoopingCall | None = None self.encoder: JSONEncoder = ScrapyJSONEncoder(sort_keys=True, indent=4) self.ext_stats_enabled: bool = bool(ext_stats) @@ -165,5 +164,5 @@ class PeriodicLog: def spider_closed(self, spider: Spider, reason: str) -> None: self.log() - if self.task and self.task.running: + if self.task and self.task.running: # pragma: no branch self.task.stop() diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py index ce56ee11d..6e8486eb8 100644 --- a/tests/test_downloadermiddleware_httpcache.py +++ b/tests/test_downloadermiddleware_httpcache.py @@ -14,6 +14,7 @@ import pytest from scrapy.downloadermiddlewares.httpcache import HttpCacheMiddleware from scrapy.exceptions import IgnoreRequest +from scrapy.extensions.httpcache import DummyPolicy from scrapy.http import HtmlResponse, Request, Response from scrapy.spiders import Spider from scrapy.utils.test import get_crawler @@ -24,6 +25,14 @@ if TYPE_CHECKING: from scrapy.crawler import Crawler +class AlwaysStalePolicy(DummyPolicy): + """:class:`~scrapy.extensions.httpcache.DummyPolicy` that always + revalidates cached responses.""" + + def is_cached_response_fresh(self, cachedresponse, request): + return False + + class TestBase: """Base class with common setup and helper methods.""" @@ -282,6 +291,21 @@ class DummyPolicyTestMixin(PolicyTestMixin): self.assertEqualResponse(self.response, response) assert "cached" in response.flags + def test_revalidation_keeps_cached_response(self): + # The dummy policy considers every cached response valid, so a policy + # that subclasses it to force revalidation always gets the cached + # response back, whatever the new response is. + with self._middleware(HTTPCACHE_POLICY=AlwaysStalePolicy) as mw: + assert mw.process_request(self.request) is None + mw.process_response(self.request, self.response) + + assert mw.process_request(self.request) is None + fresh_response = self.response.replace(body=b"new body") + response = mw.process_response(self.request, fresh_response) + self.assertEqualResponse(self.response, response) + assert "cached" in response.flags + assert mw.stats.get_value("httpcache/revalidate") == 1 + class RFC2616PolicyTestMixin(PolicyTestMixin): """Mixin containing RFC2616 policy specific test methods.""" @@ -553,6 +577,53 @@ class RFC2616PolicyTestMixin(PolicyTestMixin): else: assert "cached" in res5.flags + def test_middleware_ignore_schemes(self): + # file responses are not cached by default + req = Request("file:///tmp/t.txt") + res = Response(req.url, headers={"Expires": self.tomorrow}) + with self._middleware() as mw: + assert mw.process_request(req) is None + mw.process_response(req, res) + + assert mw.storage.retrieve_response(mw.crawler.spider, req) is None + assert mw.process_request(req) is None + + def test_max_stale_with_value(self): + # A response that expired one day ago. + headers = {"Date": self.yesterday, "Expires": self.yesterday} + with self._middleware() as mw: + req0 = Request("http://example.com") + res0 = Response(req0.url, headers=headers) + self._process_requestresponse(mw, req0, res0) + + # max-stale greater than the staleness of the cached response + req1 = req0.replace(headers={"Cache-Control": "max-stale=172800"}) + res1 = mw.process_request(req1) + assert isinstance(res1, Response) + assert "cached" in res1.flags + + # max-stale lower than the staleness of the cached response + req2 = req0.replace(headers={"Cache-Control": "max-stale=60"}) + assert mw.process_request(req2) is None + + # a non-integer max-stale value is ignored + req3 = req0.replace(headers={"Cache-Control": "max-stale=soon"}) + assert mw.process_request(req3) is None + + def test_response_dated_in_the_future(self): + # A Date header ahead of the local clock must not make the cached + # response look aged. + headers = {"Date": self.tomorrow, "Cache-Control": "max-age=10"} + with self._middleware() as mw: + req0 = Request("http://example.com") + res0 = Response(req0.url, headers=headers) + res1 = self._process_requestresponse(mw, req0, res0) + assert "cached" not in res1.flags + + res2 = self._process_requestresponse(mw, req0, None) + self.assertEqualResponse(res1, res2) + assert "cached" in res2.flags + def test_process_exception(self): with self._middleware() as mw: res0 = Response(self.request.url, headers={"Expires": self.yesterday}) diff --git a/tests/test_extension_memusage.py b/tests/test_extension_memusage.py index a474725d8..76e8ca5d6 100644 --- a/tests/test_extension_memusage.py +++ b/tests/test_extension_memusage.py @@ -2,6 +2,7 @@ from __future__ import annotations import logging import sys +from typing import TYPE_CHECKING import pytest @@ -13,8 +14,12 @@ from scrapy.extensions.memusage import MemoryUsage from scrapy.spiders import Spider from scrapy.utils.test import get_crawler from tests.utils import OneShotLoop +from tests.utils.cmdline import proc from tests.utils.decorators import coroutine_test +if TYPE_CHECKING: + from tests.mockserver.http import MockServer + # MemoryUsage relies on the stdlib 'resource' module (not available on Windows) pytestmark = pytest.mark.skipif( sys.platform.startswith("win"), @@ -25,6 +30,14 @@ pytestmark = pytest.mark.skipif( MB = 1024 * 1024 +class TwoShotLoop(OneShotLoop): + """Like :class:`OneShotLoop`, but runs the check twice.""" + + def start(self, interval: float, now: bool = True) -> None: + super().start(interval, now=now) + self.func() + + class _LoopSpider(Spider): name = "loop-data-spider" @@ -50,6 +63,49 @@ def test_memusage_disabled() -> None: MemoryUsage.from_crawler(get_crawler(settings_dict=settings)) +def test_memusage_limit_stops_crawler_without_spider(mockserver: MockServer) -> None: + # The Scrapy shell starts the engine without opening a spider, so the + # whole crawler is stopped instead of a spider being closed. + _, out, err = proc( + "shell", + mockserver.url("/text"), + "-c", + "response.status", + "--set", + "MEMUSAGE_LIMIT_MB=1", + ) + assert "Memory usage exceeded 1MiB" in err + assert "200" in out + + +@coroutine_test +async def test_memusage_below_thresholds_logs_peak( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + settings = { + "MEMUSAGE_LIMIT_MB": 100, + "MEMUSAGE_WARNING_MB": 50, + "MEMUSAGE_CHECK_INTERVAL_SECONDS": 0.01, + "TELNETCONSOLE_ENABLED": False, + "LOG_LEVEL": "INFO", + } + + monkeypatch.setattr(memusage_mod, "create_looping_call", OneShotLoop) + monkeypatch.setattr(MemoryUsage, "get_virtual_size", lambda _: 25 * MB) + + crawler = get_crawler(spidercls=_LoopSpider, settings_dict=settings) + + with caplog.at_level(logging.INFO, logger="scrapy.extensions.memusage"): + await crawler.crawl_async(url="data:,", loops=1) + + assert crawler.stats + assert crawler.stats.get_value("memusage/limit_reached") is None + assert crawler.stats.get_value("memusage/warning_reached") is None + assert crawler.stats.get_value("memusage/max") == 25 * MB + assert crawler.stats.get_value("finish_reason") == "finished" + assert any("Peak memory usage is 25MiB" in r.getMessage() for r in caplog.records) + + @coroutine_test async def test_memusage_limit_closes_spider_with_reason_and_error_log( caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch @@ -92,8 +148,9 @@ async def test_memusage_warning_logs_but_allows_normal_finish( "LOG_LEVEL": "INFO", } - # Avoid background LoopingCall that can log after the test finishes. - monkeypatch.setattr(memusage_mod, "create_looping_call", OneShotLoop) + # Avoid background LoopingCall that can log after the test finishes; check + # twice, since the warning is only meant to be reported once. + monkeypatch.setattr(memusage_mod, "create_looping_call", TwoShotLoop) monkeypatch.setattr(MemoryUsage, "get_virtual_size", lambda self: 75 * MB) crawler = get_crawler(spidercls=_LoopSpider, settings_dict=settings) @@ -112,4 +169,7 @@ async def test_memusage_warning_logs_but_allows_normal_finish( assert crawler.stats assert crawler.stats.get_value("memusage/warning_reached") == 1 assert crawler.stats.get_value("finish_reason") == "finished" - assert any("memory usage reached" in r.getMessage().lower() for r in caplog.records) + warnings_logged = [ + r for r in caplog.records if "memory usage reached" in r.getMessage().lower() + ] + assert len(warnings_logged) == 1 diff --git a/tests/test_extension_periodic_log.py b/tests/test_extension_periodic_log.py index ffe7a0dc7..723fb6e17 100644 --- a/tests/test_extension_periodic_log.py +++ b/tests/test_extension_periodic_log.py @@ -1,8 +1,13 @@ from __future__ import annotations import datetime +import json +import logging from typing import TYPE_CHECKING, Any +import pytest + +from scrapy.exceptions import NotConfigured from scrapy.extensions.periodic_log import PeriodicLog from scrapy.utils.test import get_crawler @@ -86,6 +91,14 @@ class TestPeriodicLog: assert extension({"PERIODIC_LOG_DELTA": True, "LOGSTATS_INTERVAL": 60}) assert extension({"PERIODIC_LOG_DELTA": "True", "LOGSTATS_INTERVAL": 60}) + def test_no_interval(self): + with pytest.raises(NotConfigured): + extension({"PERIODIC_LOG_STATS": True, "LOGSTATS_INTERVAL": 0}) + + def test_nothing_enabled(self): + with pytest.raises(NotConfigured): + extension({"LOGSTATS_INTERVAL": 60}) + @coroutine_test async def test_log_delta(self): def emulate( @@ -212,3 +225,26 @@ class TestPeriodicLog: {"PERIODIC_LOG_STATS": {"include": ["downloader/"], "exclude": ["bytes"]}}, lambda k, v: "downloader/" in k and "bytes" not in k, ) + + @coroutine_test + async def test_log_timing(self, caplog: pytest.LogCaptureFixture) -> None: + settings = { + "EXTENSIONS": {"scrapy.extensions.periodic_log.PeriodicLog": 0}, + "PERIODIC_LOG_TIMING_ENABLED": True, + "LOGSTATS_INTERVAL": 30, + } + crawler = get_crawler(MetaSpider, settings) + with caplog.at_level(logging.INFO, logger="scrapy.extensions.periodic_log"): + await crawler.crawl_async() + + records = [ + r for r in caplog.records if r.name == "scrapy.extensions.periodic_log" + ] + assert records, "PeriodicLog logged nothing" + # Only the timing section is enabled, and it is logged on spider close. + data = json.loads(records[-1].getMessage()) + assert list(data) == ["time"] + assert data["time"]["log_interval"] == 30 + assert data["time"]["log_interval_real"] >= 0 + assert data["time"]["elapsed"] >= 0 + assert data["time"]["start_time"] <= data["time"]["utcnow"] diff --git a/tests/test_extension_telnet.py b/tests/test_extension_telnet.py index 20c801558..fca0e3153 100644 --- a/tests/test_extension_telnet.py +++ b/tests/test_extension_telnet.py @@ -7,7 +7,8 @@ import pytest from twisted.conch.telnet import ITelnetProtocol from twisted.cred import credentials -from scrapy.extensions.telnet import TelnetConsole +from scrapy import Spider +from scrapy.extensions.telnet import TelnetConsole, update_telnet_vars from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.test import get_crawler from tests.utils.decorators import coroutine_test @@ -16,16 +17,20 @@ if TYPE_CHECKING: from collections.abc import Generator from scrapy.crawler import Crawler + from scrapy.http import Response pytestmark = pytest.mark.requires_reactor # TelnetConsole requires a reactor -def _get_crawler(settings_dict: dict[str, Any] | None = None) -> Crawler: +def _get_crawler( + spidercls: type[Spider] | None = None, + settings_dict: dict[str, Any] | None = None, +) -> Crawler: settings = { "TELNETCONSOLE_ENABLED": True, **(settings_dict or {}), } - return get_crawler(settings_dict=settings) + return get_crawler(spidercls, settings_dict=settings) @contextmanager @@ -84,3 +89,47 @@ def test_invalid_reversed_portrange() -> None: console = TelnetConsole(_get_crawler(settings_dict=settings)) with pytest.raises(ValueError, match=r"invalid portrange: \[2, 1\]"): console.start_listening() + + +@coroutine_test +async def test_telnet_vars() -> None: + """Log into the console of a running crawl, which is when the telnet + variables are built.""" + received: list[dict[str, Any]] = [] + + def on_update_telnet_vars(telnet_vars: dict[str, Any]) -> None: + received.append(telnet_vars) + + class TelnetSpider(Spider): + name = "telnet" + start_urls = ["data:,"] + + async def parse(self, response: Response) -> None: + assert self.crawler.extensions + console = next( + ext + for ext in self.crawler.extensions.middlewares + if isinstance(ext, TelnetConsole) + ) + creds = credentials.UsernamePassword( + console.username.encode("utf8"), console.password.encode("utf8") + ) + portal = console.protocol().protocolArgs[0] + await maybe_deferred_to_future(portal.login(creds, None, ITelnetProtocol)) + + crawler = _get_crawler(TelnetSpider) + crawler.signals.connect(on_update_telnet_vars, signal=update_telnet_vars) + await crawler.crawl_async() + + assert len(received) == 1 + telnet_vars = received[0] + assert telnet_vars["crawler"] is crawler + assert telnet_vars["engine"] is crawler.engine + assert telnet_vars["spider"] is crawler.spider + assert telnet_vars["extensions"] is crawler.extensions + assert telnet_vars["stats"] is crawler.stats + assert telnet_vars["settings"] is crawler.settings + assert callable(telnet_vars["est"]) + assert callable(telnet_vars["p"]) + assert callable(telnet_vars["prefs"]) + assert "telnetconsole.html" in telnet_vars["help"] diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 1cca287af..40a763efd 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -24,6 +24,7 @@ from scrapy.extensions.feedexport import ( FeedExporter, FeedSlot, FileFeedStorage, + ItemFilter, apply_uri_params, ) from scrapy.utils.python import to_unicode @@ -1289,6 +1290,13 @@ class TestFeedExporterSignals: assert self.feed_exporter_closed_received +class TestItemFilter: + def test_no_feed_options(self): + item_filter = ItemFilter(None) + assert item_filter.item_classes == () + assert item_filter.accepts(MyItem({"foo": "bar"})) + + class TestFeedExportInit: def test_unsupported_storage(self): settings = { @@ -1300,6 +1308,24 @@ class TestFeedExportInit: with pytest.raises(NotConfigured): FeedExporter.from_crawler(crawler) + def test_disabled_storage(self, caplog: pytest.LogCaptureFixture): + class DisabledFeedStorage: + def __init__(self, uri, *, feed_options=None): + raise NotConfigured("not today") + + settings = { + "FEED_STORAGES": {"disabled": DisabledFeedStorage}, + "FEEDS": { + "disabled://uri": {}, + }, + } + crawler = get_crawler(settings_dict=settings) + with caplog.at_level(logging.ERROR), pytest.raises(NotConfigured): + FeedExporter.from_crawler(crawler) + assert ( + "Disabled feed storage scheme: disabled. Reason: not today" in caplog.text + ) + def test_unsupported_format(self): settings = { "FEEDS": { diff --git a/tests/test_feedexport_postprocess.py b/tests/test_feedexport_postprocess.py index f120ce36f..36d8586ce 100644 --- a/tests/test_feedexport_postprocess.py +++ b/tests/test_feedexport_postprocess.py @@ -12,6 +12,7 @@ from typing import TYPE_CHECKING, Any import pytest +from scrapy.extensions.postprocessing import PostProcessingManager from scrapy.utils.test import get_crawler from tests.utils.bases.feedexport import TestFeedExportBase from tests.utils.decorators import coroutine_test @@ -87,6 +88,15 @@ class TestFeedPostProcessedExports(TestFeedExportBase): data_stream.seek(0) return data_stream.read() + def test_tell_reports_target_file_position(self): + """Exporters that wrap the file they get, e.g. through + :class:`io.TextIOWrapper`, need it to report a position.""" + file = BytesIO() + manager = PostProcessingManager([self.MyPlugin1], file, {}) + assert manager.tell() == 0 + manager.write(b"foo") + assert manager.tell() == file.tell() == 3 + @coroutine_test async def test_gzip_plugin(self): filename = self._named_tempfile("gzip_file") diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index b1e3787fd..4d28872b7 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging import os import string +import sys import tempfile from io import BytesIO from pathlib import Path @@ -14,6 +15,7 @@ import pytest from w3lib.url import path_to_file_uri import scrapy +from scrapy.exceptions import NotConfigured from scrapy.extensions.feedexport import ( BlockingFeedStorage, FileFeedStorage, @@ -166,6 +168,12 @@ class TestFTPFeedStorage: st = FTPFeedStorage(f"ftp://foo:{pw_quoted}@example.com/some_path", {}) assert st.password == string.punctuation + def test_uri_without_hostname(self): + with pytest.raises( + ValueError, match="Got a storage URI without a hostname: ftp:///some_path" + ): + FTPFeedStorage("ftp:///some_path") + class MyBlockingFeedStorage(BlockingFeedStorage): def _store_in_thread(self, file: IO[bytes]) -> None: @@ -205,6 +213,13 @@ class TestBlockingFeedStorage: b.open(spider=spider) +def test_s3_without_boto3(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setitem(sys.modules, "boto3", None) + monkeypatch.setitem(sys.modules, "boto3.session", None) + with pytest.raises(NotConfigured, match="missing boto3 library"): + S3FeedStorage("s3://mybucket/export.csv", "access_key", "secret_key") + + @pytest.mark.requires_boto3 class TestS3FeedStorage: def test_parse_credentials(self): From 259be5d2dd02ab1be876bf4b6be1849cbab88c6f Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 08:20:21 +0200 Subject: [PATCH 16/25] CI: Run coverage in as few jobs as necessary (#7834) * CI: Run coverage in as few jobs as necessary * Complete test coverage --- .github/workflows/tests-macos.yml | 9 +++++++-- .github/workflows/tests-ubuntu.yml | 14 +++++++++++++- .github/workflows/tests-windows.yml | 4 +++- tests/test_utils_console.py | 19 ++++++++++++++++++- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/.github/workflows/tests-macos.yml b/.github/workflows/tests-macos.yml index 0409b3ef2..566b34e50 100644 --- a/.github/workflows/tests-macos.yml +++ b/.github/workflows/tests-macos.yml @@ -14,14 +14,18 @@ jobs: tests: runs-on: macos-latest env: - PYTEST_ADDOPTS: -n auto + PYTEST_ADDOPTS: ${{ matrix.coverage && '-n auto' || '-n auto --no-cov' }} strategy: fail-fast: false matrix: - python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] + python-version: ["3.10", "3.11", "3.12", "3.13"] env: - TOXENV: py include: + - python-version: '3.14' + env: + TOXENV: py + coverage: true - python-version: '3.14' env: TOXENV: no-reactor @@ -41,6 +45,7 @@ jobs: tox - name: Upload coverage report + if: ${{ matrix.coverage }} uses: codecov/codecov-action@v5 - name: Upload test results diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index 15f25d9b8..ad2bcfce7 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -14,7 +14,7 @@ jobs: tests: runs-on: ubuntu-latest env: - PYTEST_ADDOPTS: -n auto + PYTEST_ADDOPTS: ${{ matrix.coverage && '-n auto' || '-n auto --no-cov' }} strategy: fail-fast: false matrix: @@ -34,12 +34,15 @@ jobs: - python-version: "3.14" env: TOXENV: py + coverage: true - python-version: "3.14" env: TOXENV: default-reactor + coverage: true - python-version: "3.14" env: TOXENV: no-reactor + coverage: true # pinned due to https://github.com/pypy/pypy/issues/5388 - python-version: pypy3.11-7.3.20 env: @@ -49,12 +52,15 @@ jobs: - python-version: "3.10.19" env: TOXENV: min + coverage: true - python-version: "3.10.19" env: TOXENV: min-default-reactor + coverage: true - python-version: "3.10.19" env: TOXENV: min-no-reactor + coverage: true # pinned due to https://github.com/pypy/pypy/issues/5388 - python-version: pypy3.11-7.3.20 env: @@ -62,16 +68,20 @@ jobs: - python-version: "3.10.19" env: TOXENV: min-extra-deps + coverage: true - python-version: "3.10.19" env: TOXENV: min-botocore + coverage: true - python-version: "3.14" env: TOXENV: extra-deps + coverage: true - python-version: "3.14" env: TOXENV: no-reactor-extra-deps + coverage: true # pinned due to https://github.com/pypy/pypy/issues/5388 - python-version: pypy3.11-7.3.20 env: @@ -79,6 +89,7 @@ jobs: - python-version: "3.14" env: TOXENV: botocore + coverage: true steps: - uses: actions/checkout@v6 @@ -104,6 +115,7 @@ jobs: tox - name: Upload coverage report + if: ${{ matrix.coverage }} uses: codecov/codecov-action@v5 - name: Upload test results diff --git a/.github/workflows/tests-windows.yml b/.github/workflows/tests-windows.yml index f413782bc..c33f96b12 100644 --- a/.github/workflows/tests-windows.yml +++ b/.github/workflows/tests-windows.yml @@ -14,7 +14,7 @@ jobs: tests: runs-on: windows-latest env: - PYTEST_ADDOPTS: -n auto + PYTEST_ADDOPTS: ${{ matrix.coverage && '-n auto' || '-n auto --no-cov' }} strategy: fail-fast: false matrix: @@ -34,6 +34,7 @@ jobs: - python-version: "3.14" env: TOXENV: py + coverage: true - python-version: "3.14" env: TOXENV: default-reactor @@ -68,6 +69,7 @@ jobs: tox - name: Upload coverage report + if: ${{ matrix.coverage }} uses: codecov/codecov-action@v5 - name: Upload test results diff --git a/tests/test_utils_console.py b/tests/test_utils_console.py index ad9aa3dff..ab0c72d8a 100644 --- a/tests/test_utils_console.py +++ b/tests/test_utils_console.py @@ -4,7 +4,7 @@ from importlib.util import find_spec import pytest -from scrapy.utils.console import get_shell_embed_func +from scrapy.utils.console import get_shell_embed_func, start_python_console def test_get_shell_embed_func(): @@ -59,3 +59,20 @@ def test_get_shell_embed_func_default(): else: expected = "_embed_standard_shell" assert shell.__name__ == expected + + +def test_start_python_console_exit(monkeypatch: pytest.MonkeyPatch) -> None: + def embed(namespace: dict[str, object], banner: str) -> None: + raise SystemExit + + monkeypatch.setattr( + "scrapy.utils.console.get_shell_embed_func", lambda shells: embed + ) + start_python_console() + + +def test_start_python_console_no_shell(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + "scrapy.utils.console.get_shell_embed_func", lambda shells: None + ) + start_python_console() From 434fd1154ad26e04ea2e438ab5d3b8dd70e73885 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 08:38:33 +0200 Subject: [PATCH 17/25] Improve pre-crawler setting docs (#7835) --- docs/topics/commands.rst | 2 ++ docs/topics/settings.rst | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index ee2c3a3cd..e8a843e80 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -665,6 +665,8 @@ Example: COMMANDS_MODULE = "mybot.commands" +.. note:: This is a :ref:`pre-crawler setting `. + .. _Deploying your project: https://scrapyd.readthedocs.io/en/latest/deploy.html Register commands via setup.py entry points diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 1b6851d04..81055afc2 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -305,10 +305,21 @@ These settings cannot be :ref:`set from a spider `. These settings are: -- :setting:`TWISTED_REACTOR_ENABLED` +- :setting:`ADDONS` +- :setting:`COMMANDS_MODULE` +- :setting:`FORCE_CRAWLER_PROCESS` - :setting:`SPIDER_LOADER_CLASS` and settings used by the corresponding spider loader class, e.g. :setting:`SPIDER_MODULES` and :setting:`SPIDER_LOADER_WARN_ONLY` for the default spider loader class. +- :setting:`TWISTED_REACTOR_ENABLED` + +:setting:`ADDONS` is a special case: it can be set from a spider, but the +``update_pre_crawler_settings()`` method of :ref:`add-ons ` +enabled that way is not called. + +:setting:`TWISTED_REACTOR` also acts as a pre-crawler setting when running a +:ref:`command that needs a CrawlerProcess `, +since its project-level value determines the crawler process class. .. _reactor-settings: @@ -409,6 +420,9 @@ Default: ``{}`` A dict containing paths to the add-ons enabled in your project and their priorities. For more information, see :ref:`topics-addons`. +.. note:: This is a :ref:`pre-crawler setting `, with a + caveat described in that section. + .. setting:: ASYNCIO_EVENT_LOOP ASYNCIO_EVENT_LOOP @@ -1402,6 +1416,8 @@ When :setting:`TWISTED_REACTOR_ENABLED` is set to ``False``, Set this to ``True`` if you want to set :setting:`TWISTED_REACTOR` to a non-default value in :ref:`per-spider settings `. +.. note:: This is a :ref:`pre-crawler setting `. + .. setting:: FTP_PASSIVE_MODE FTP_PASSIVE_MODE From 37661508dbaec28bd68e2d05306e20b7ee0f5652 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 08:59:52 +0200 Subject: [PATCH 18/25] Add RobotParser.crawl_delay() and a robots_parsed signal (#7830) * Add RobotParser.crawl_delay() and a robots_parsed signal * Improve test coverage --- docs/topics/signals.rst | 21 +++++++++++++++ scrapy/downloadermiddlewares/robotstxt.py | 12 +++++++-- scrapy/robotstxt.py | 21 +++++++++++++++ scrapy/signals.py | 1 + tests/test_downloadermiddleware_robotstxt.py | 17 ++++++++++++ tests/test_robotstxt_interface.py | 27 ++++++++++++++++++++ 6 files changed, 97 insertions(+), 2 deletions(-) diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 03996bee6..f7f9f5cca 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -504,6 +504,27 @@ headers_received :param spider: the spider associated with the response :type spider: :class:`~scrapy.Spider` object +robots_parsed +~~~~~~~~~~~~~ + +.. signal:: robots_parsed +.. function:: robots_parsed(robotparser, request) + + .. versionadded:: VERSION + + Sent by + :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware` after it + downloads and parses a :file:`robots.txt` file, for the host that *request* + targets. + + This signal supports :ref:`asynchronous handlers `. + + :param robotparser: the parser holding the parsed :file:`robots.txt` contents + :type robotparser: :class:`~scrapy.robotstxt.RobotParser` object + + :param request: the request that triggered the :file:`robots.txt` download + :type request: :class:`~scrapy.Request` object + Response signals ---------------- diff --git a/scrapy/downloadermiddlewares/robotstxt.py b/scrapy/downloadermiddlewares/robotstxt.py index 7d0c17884..81a3a887f 100644 --- a/scrapy/downloadermiddlewares/robotstxt.py +++ b/scrapy/downloadermiddlewares/robotstxt.py @@ -11,6 +11,7 @@ from typing import TYPE_CHECKING from twisted.internet.defer import Deferred +from scrapy import signals from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request, Response from scrapy.http.request import NO_CALLBACK @@ -98,7 +99,7 @@ class RobotsTxtMiddleware: assert self.crawler.stats try: resp = await self.crawler.engine.download_async(robotsreq) - self._parse_robots(resp, netloc) + await self._parse_robots(resp, netloc, request) except Exception as e: if not isinstance(e, IgnoreRequest): logger.error( @@ -115,13 +116,20 @@ class RobotsTxtMiddleware: return await maybe_deferred_to_future(parser) return parser - def _parse_robots(self, response: Response, netloc: str) -> None: + async def _parse_robots( + self, response: Response, netloc: str, request: Request + ) -> None: assert self.crawler.stats self.crawler.stats.inc_value("robotstxt/response_count") self.crawler.stats.inc_value( f"robotstxt/response_status_count/{response.status}" ) rp = self._parserimpl.from_crawler(self.crawler, response.body) + await self.crawler.signals.send_catch_log_async( + signal=signals.robots_parsed, + robotparser=rp, + request=request, + ) rp_dfd = self._parsers[netloc] assert isinstance(rp_dfd, Deferred) self._parsers[netloc] = rp diff --git a/scrapy/robotstxt.py b/scrapy/robotstxt.py index 0c64ea5a5..b54011784 100644 --- a/scrapy/robotstxt.py +++ b/scrapy/robotstxt.py @@ -67,6 +67,15 @@ class RobotParser(metaclass=ABCMeta): :type user_agent: str or bytes """ + def crawl_delay(self, user_agent: str | bytes) -> float | None: + """Return the ``Crawl-delay`` directive for ``user_agent`` as a number + of seconds, or ``None`` if it is not set or the backend does not support + it. + + .. versionadded:: VERSION + """ + return None + class PythonRobotParser(RobotParser): def __init__(self, robotstxt_body: bytes, spider: Spider | None): @@ -85,6 +94,10 @@ class PythonRobotParser(RobotParser): url = to_unicode(url) return self.rp.can_fetch(user_agent, url) + def crawl_delay(self, user_agent: str | bytes) -> float | None: + delay = self.rp.crawl_delay(to_unicode(user_agent)) + return None if delay is None else float(delay) + class RerpRobotParser(RobotParser): def __init__(self, robotstxt_body: bytes, spider: Spider | None): @@ -105,6 +118,10 @@ class RerpRobotParser(RobotParser): url = to_unicode(url) return cast("bool", self.rp.is_allowed(user_agent, url)) + def crawl_delay(self, user_agent: str | bytes) -> float | None: + delay = self.rp.get_crawl_delay(to_unicode(user_agent)) + return None if delay is None else float(delay) + class ProtegoRobotParser(RobotParser): def __init__(self, robotstxt_body: bytes, spider: Spider | None): @@ -121,3 +138,7 @@ class ProtegoRobotParser(RobotParser): user_agent = to_unicode(user_agent) url = to_unicode(url) return self.rp.can_fetch(url, user_agent) + + def crawl_delay(self, user_agent: str | bytes) -> float | None: + delay = self.rp.crawl_delay(to_unicode(user_agent)) + return None if delay is None else float(delay) diff --git a/scrapy/signals.py b/scrapy/signals.py index 972f4fd60..3afeb6eab 100644 --- a/scrapy/signals.py +++ b/scrapy/signals.py @@ -21,6 +21,7 @@ response_received = object() response_downloaded = object() headers_received = object() bytes_received = object() +robots_parsed = object() item_scraped = object() item_dropped = object() item_error = object() diff --git a/tests/test_downloadermiddleware_robotstxt.py b/tests/test_downloadermiddleware_robotstxt.py index f82041a62..1f2575f8f 100644 --- a/tests/test_downloadermiddleware_robotstxt.py +++ b/tests/test_downloadermiddleware_robotstxt.py @@ -8,6 +8,7 @@ import pytest from twisted.internet.defer import Deferred, DeferredList from twisted.python import failure +from scrapy import signals from scrapy.downloadermiddlewares.robotstxt import RobotsTxtMiddleware from scrapy.exceptions import CannotResolveHostError, IgnoreRequest, NotConfigured from scrapy.http import Request, Response, TextResponse @@ -27,6 +28,7 @@ class TestRobotsTxtMiddleware: self.crawler: mock.MagicMock = mock.MagicMock() self.crawler.settings = Settings() self.crawler.engine.download_async = mock.AsyncMock() + self.crawler.signals.send_catch_log_async = mock.AsyncMock(return_value=[]) def teardown_method(self): del self.crawler @@ -74,6 +76,21 @@ Disallow: /some/randome/page.html Request("http://site.local/wiki/Käyttäjä:"), middleware ) + @coroutine_test + async def test_robotstxt_emits_robots_parsed_signal(self): + crawler = self._get_successful_crawler() + middleware = RobotsTxtMiddleware(crawler) + request = Request("http://site.local/allowed") + await self.assertNotIgnored(request, middleware) + calls = [ + kwargs + for _, kwargs in crawler.signals.send_catch_log_async.call_args_list + if kwargs.get("signal") is signals.robots_parsed + ] + assert len(calls) == 1 + assert calls[0]["request"] is request + assert calls[0]["robotparser"] is not None + @coroutine_test async def test_robotstxt_multiple_reqs(self) -> None: middleware = RobotsTxtMiddleware(self._get_successful_crawler()) diff --git a/tests/test_robotstxt_interface.py b/tests/test_robotstxt_interface.py index da94a4e95..ea67877f8 100644 --- a/tests/test_robotstxt_interface.py +++ b/tests/test_robotstxt_interface.py @@ -4,6 +4,7 @@ from scrapy.robotstxt import ( ProtegoRobotParser, PythonRobotParser, RerpRobotParser, + RobotParser, decode_robotstxt, ) from scrapy.utils._deps_compat import STDLIB_IMPROVED_ROBOTFILEPARSER @@ -78,6 +79,16 @@ class BaseRobotParserTest: assert rp.allowed("https://site.local/index.html", "*") assert rp.allowed("https://site.local/disallowed", "*") + def test_crawl_delay(self): + robotstxt_body = b"User-agent: *\nDisallow: /private\nCrawl-delay: 10\n" + rp = self.parser_cls.from_crawler(crawler=None, robotstxt_body=robotstxt_body) + assert rp.crawl_delay("*") == 10.0 + + def test_crawl_delay_unset(self): + robotstxt_body = b"User-agent: *\nDisallow: /private\n" + rp = self.parser_cls.from_crawler(crawler=None, robotstxt_body=robotstxt_body) + assert rp.crawl_delay("*") is None + def test_unicode_url_and_useragent(self): robotstxt_robotstxt_body = """ User-Agent: * @@ -102,6 +113,22 @@ class BaseRobotParserTest: assert not rp.allowed("https://site.local/some/randome/page.html", "UnicödeBöt") +class TestRobotParser: + def test_crawl_delay_unsupported(self): + class AllowAllRobotParser(RobotParser): + @classmethod + def from_crawler(cls, crawler, robotstxt_body): + return cls() + + def allowed(self, url, user_agent): + return True + + rp = AllowAllRobotParser.from_crawler( + crawler=None, robotstxt_body=b"User-agent: *\nCrawl-delay: 10\n" + ) + assert rp.crawl_delay("*") is None + + class TestDecodeRobotsTxt: def test_native_string_conversion(self): robotstxt_body = b"User-agent: *\nDisallow: /\n" From 746bc7548d358ba96bfa74e9ce6ceb06f11c550d Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 09:17:34 +0200 Subject: [PATCH 19/25] Clarify crawl vs runspider in help and docs (#7832) --- docs/topics/commands.rst | 11 +++++++---- scrapy/commands/crawl.py | 2 +- scrapy/commands/runspider.py | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index e8a843e80..343193627 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -114,8 +114,8 @@ some usage help and the available commands:: scrapy [options] [args] Available commands: - crawl Run a spider fetch Fetch a URL using the Scrapy downloader + runspider Run a spider from a Python file, no project required [...] The first line will print the currently active project if you're inside a @@ -263,7 +263,9 @@ crawl * Syntax: ``scrapy crawl `` * Requires project: *yes* -Start crawling using a spider. +Start crawling using the spider with the given :attr:`~scrapy.Spider.name`, +which must be one of those that :command:`list` reports. To run a spider from a +file instead, use :command:`runspider`. Supported options: @@ -571,8 +573,9 @@ runspider * Syntax: ``scrapy runspider `` * Requires project: *no* -Run a spider self-contained in a Python file, without having to create a -project. +Run the spider defined in the given Python file, without requiring a project. + +Supported options: the same as :command:`crawl`. Example usage:: diff --git a/scrapy/commands/crawl.py b/scrapy/commands/crawl.py index 866ba9f6b..4e086e057 100644 --- a/scrapy/commands/crawl.py +++ b/scrapy/commands/crawl.py @@ -16,7 +16,7 @@ class Command(BaseRunSpiderCommand): return "[options] " def short_desc(self) -> str: - return "Run a spider" + return "Run a spider of the current project, by name" def run(self, args: list[str], opts: argparse.Namespace) -> None: if len(args) < 1: diff --git a/scrapy/commands/runspider.py b/scrapy/commands/runspider.py index 0b9036457..9cdb393ab 100644 --- a/scrapy/commands/runspider.py +++ b/scrapy/commands/runspider.py @@ -38,7 +38,7 @@ class Command(BaseRunSpiderCommand): return "[options] " def short_desc(self) -> str: - return "Run a self-contained spider (without creating a project)" + return "Run a spider from a Python file, no project required" def long_desc(self) -> str: return "Run the spider defined in the given file" From ea7c0af2f96400be88513172ea55579997e1b892 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 09:18:06 +0200 Subject: [PATCH 20/25] Use a single badge for all tests (#7836) --- README.rst | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/README.rst b/README.rst index 6235cb20c..651294add 100644 --- a/README.rst +++ b/README.rst @@ -5,7 +5,7 @@ :alt: Scrapy :width: 480px -|version| |python_version| |ubuntu| |macos| |windows| |coverage| |conda| |deepwiki| +|version| |python_version| |tests| |coverage| |conda| |deepwiki| .. |version| image:: https://img.shields.io/pypi/v/Scrapy.svg :target: https://pypi.org/pypi/Scrapy @@ -15,17 +15,9 @@ :target: https://pypi.org/pypi/Scrapy :alt: Supported Python Versions -.. |ubuntu| image:: https://github.com/scrapy/scrapy/workflows/Ubuntu/badge.svg - :target: https://github.com/scrapy/scrapy/actions?query=workflow%3AUbuntu - :alt: Ubuntu - -.. |macos| image:: https://github.com/scrapy/scrapy/workflows/macOS/badge.svg - :target: https://github.com/scrapy/scrapy/actions?query=workflow%3AmacOS - :alt: macOS - -.. |windows| image:: https://github.com/scrapy/scrapy/workflows/Windows/badge.svg - :target: https://github.com/scrapy/scrapy/actions?query=workflow%3AWindows - :alt: Windows +.. |tests| image:: https://img.shields.io/github/check-runs/scrapy/scrapy/master?label=tests + :target: https://github.com/scrapy/scrapy/actions?query=branch%3Amaster + :alt: Tests .. |coverage| image:: https://img.shields.io/codecov/c/github/scrapy/scrapy/master.svg :target: https://codecov.io/github/scrapy/scrapy?branch=master From 3180116cd08942997548f62f90a66e01e64a690b Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 09:19:49 +0200 Subject: [PATCH 21/25] Improve test coverage for scrapy.commands (#7829) --- scrapy/commands/genspider.py | 5 +- scrapy/commands/parse.py | 2 +- tests/test_command_crawl.py | 12 +++ tests/test_command_fetch.py | 32 ++++++++ tests/test_command_genspider.py | 21 ++++- tests/test_command_parse.py | 125 ++++++++++++++++++++++++++++ tests/test_command_runspider.py | 6 ++ tests/test_command_shell.py | 28 +++++++ tests/test_commands.py | 139 ++++++++++++++++++++++++++++---- tests/utils/cmdline.py | 13 +++ 10 files changed, 363 insertions(+), 20 deletions(-) diff --git a/scrapy/commands/genspider.py b/scrapy/commands/genspider.py index 4277232c3..52f9cd4b0 100644 --- a/scrapy/commands/genspider.py +++ b/scrapy/commands/genspider.py @@ -32,10 +32,7 @@ def sanitize_module_name(module_name: str) -> str: def extract_domain(url: str) -> str: """Extract domain name from URL string""" - o = urlparse(url) - if o.scheme == "" and o.netloc == "": - o = urlparse("//" + url.lstrip("/")) - return o.netloc + return urlparse(url).netloc def verify_url_scheme(url: str) -> str: diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index 93194ded7..51caed57f 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -41,7 +41,7 @@ class Command(BaseRunSpiderCommand): spider: Spider | None = None items: ClassVar[dict[int, list[Any]]] = {} requests: ClassVar[dict[int, list[Request]]] = {} - spidercls: type[Spider] | None + spidercls: type[Spider] | None = None first_response = None diff --git a/tests/test_command_crawl.py b/tests/test_command_crawl.py index 70c26e6d0..5306e3bf8 100644 --- a/tests/test_command_crawl.py +++ b/tests/test_command_crawl.py @@ -23,6 +23,18 @@ class TestCrawlCommand(TestProjectBase): _, _, stderr = self.crawl(code, proj_path, args=args) return stderr + def test_no_spider(self, proj_path: Path) -> None: + returncode, out, _ = proc("crawl", cwd=proj_path) + assert returncode == 2 + assert "Usage" in out + + def test_multiple_spiders(self, proj_path: Path) -> None: + returncode, _, err = proc("crawl", "myspider", "myspider2", cwd=proj_path) + assert returncode == 2 + assert ( + "running 'scrapy crawl' with more than one spider is not supported" in err + ) + def test_no_output(self, proj_path: Path) -> None: spider_code = """ import scrapy diff --git a/tests/test_command_fetch.py b/tests/test_command_fetch.py index d98dac968..c6a3afc91 100644 --- a/tests/test_command_fetch.py +++ b/tests/test_command_fetch.py @@ -2,13 +2,24 @@ from __future__ import annotations from typing import TYPE_CHECKING +import pytest + +from tests.utils.bases.commands import TestProjectBase from tests.utils.cmdline import proc if TYPE_CHECKING: + from pathlib import Path + from tests.mockserver.http import MockServer class TestFetchCommand: + @pytest.mark.parametrize("args", [(), ("not-a-url",), ("a:b", "c:d")]) + def test_bad_arguments(self, args: tuple[str, ...]) -> None: + returncode, out, _ = proc("fetch", *args) + assert returncode == 2 + assert "Usage" in out + def test_output(self, mockserver: MockServer) -> None: _, out, _ = proc("fetch", mockserver.url("/text")) assert out.strip() == "Works" @@ -36,3 +47,24 @@ class TestFetchCommand: "fetch", "-s", "TWISTED_REACTOR_ENABLED=False", mockserver.url("/text") ) assert out.strip() == "Works" + + +class TestFetchCommandWithSpider(TestProjectBase): + @pytest.fixture(autouse=True) + def create_files(self, proj_path: Path) -> None: + (proj_path / self.project_name / "spiders" / "myspider.py").write_text( + """ +import scrapy + +class MySpider(scrapy.Spider): + name = "myspider" + custom_settings = {"USER_AGENT": "myspider-user-agent"} +""", + encoding="utf-8", + ) + + def test_spider(self, proj_path: Path, mockserver: MockServer) -> None: + _, out, err = proc( + "fetch", "--spider", "myspider", mockserver.url("/echo"), cwd=proj_path + ) + assert "myspider-user-agent" in out, err diff --git a/tests/test_command_genspider.py b/tests/test_command_genspider.py index 8bb6a2332..ddf25af4c 100644 --- a/tests/test_command_genspider.py +++ b/tests/test_command_genspider.py @@ -64,6 +64,24 @@ class TestGenspiderCommand(TestProjectBase): assert call("genspider", "--dump=basic", cwd=proj_path) == 0 assert call("genspider", "-d", "basic", cwd=proj_path) == 0 + @pytest.mark.parametrize( + "args", + [("--dump=nonexistent",), ("-t", "nonexistent", "test_name", "test.com")], + ) + def test_unknown_template(self, args: tuple[str, ...], proj_path: Path) -> None: + returncode, out, err = proc("genspider", *args, cwd=proj_path) + assert returncode == 0, err + assert "Unable to find template: nonexistent" in out + assert not (proj_path / self.project_name / "spiders" / "test_name.py").exists() + + def test_name_not_starting_with_a_letter(self, proj_path: Path) -> None: + """The module name, unlike the spider name, is prefixed with a letter.""" + _, out, err = proc("genspider", "1st_spider", "test.com", cwd=proj_path) + assert "Created spider '1st_spider'" in out, err + spider = proj_path / self.project_name / "spiders" / "a1st_spider.py" + assert spider.exists() + assert find_in_file(spider, r'name\s*=\s*"1st_spider"') is not None + @pytest.mark.skipif( sys.platform == "win32", reason="requires a POSIX shell editor script" ) @@ -87,7 +105,8 @@ class TestGenspiderCommand(TestProjectBase): ) def test_same_name_as_project(self, proj_path: Path) -> None: - assert call("genspider", self.project_name, cwd=proj_path) == 2 + _, out, err = proc("genspider", self.project_name, "test.com", cwd=proj_path) + assert "Cannot create a spider with the same name as your project" in out, err assert not ( proj_path / self.project_name / "spiders" / f"{self.project_name}.py" ).exists() diff --git a/tests/test_command_parse.py b/tests/test_command_parse.py index 772cc82e2..e434055b7 100644 --- a/tests/test_command_parse.py +++ b/tests/test_command_parse.py @@ -3,6 +3,7 @@ from __future__ import annotations import argparse import re from typing import TYPE_CHECKING +from urllib.parse import urlparse import pytest @@ -552,6 +553,130 @@ ITEM_PIPELINES = {{'{self.project_name}.pipelines.MyPipeline': 1}} content = '[\n{},\n{"foo": "bar"}\n]' assert file_path.read_text(encoding="utf-8") == content + @pytest.mark.parametrize("args", [(), ("not-a-url",), ("a:b", "c:d")]) + def test_bad_arguments(self, args: tuple[str, ...], proj_path: Path) -> None: + returncode, out, _ = proc("parse", *args, cwd=proj_path) + assert returncode == 2 + assert "Usage" in out + + @pytest.mark.parametrize( + ("option", "message"), + [ + ("--meta", "Invalid -m/--meta value"), + ("-m", "Invalid -m/--meta value"), + ("--cbkwargs", "Invalid --cbkwargs value"), + ], + ) + def test_invalid_json( + self, option: str, message: str, proj_path: Path, mockserver: MockServer + ) -> None: + returncode, _, err = proc( + "parse", + "--spider", + self.spider_name, + option, + "{invalid", + mockserver.url("/html"), + cwd=proj_path, + ) + assert returncode == 2 + assert message in err + + def test_unknown_spider(self, proj_path: Path, mockserver: MockServer) -> None: + returncode, _, err = proc( + "parse", + "--spider", + "nonexistent", + mockserver.url("/html"), + cwd=proj_path, + ) + assert returncode == 0, err + assert "Unable to find spider: nonexistent" in err + + def test_spider_found_by_url(self, proj_path: Path, mockserver: MockServer) -> None: + """Without --spider, the spider is chosen based on the URL.""" + url = mockserver.url("/html") + # The spider name doubles as a domain of the spider, and it is matched + # against the netloc of the URL, hence the port. + (proj_path / self.project_name / "spiders" / "urlspider.py").write_text( + f""" +import scrapy + +class UrlSpider(scrapy.Spider): + name = "{urlparse(url).netloc}" + + def parse(self, response): + return [{{"found_by_url": True}}] +""", + encoding="utf-8", + ) + returncode, out, err = proc("parse", url, cwd=proj_path) + assert returncode == 0, err + assert "Unable to find spider for" not in err + assert "{'found_by_url': True}" in out + + def test_legacy_item_processor( + self, proj_path: Path, mockserver: MockServer + ) -> None: + """--pipelines supports an ITEM_PROCESSOR without process_item_async().""" + (proj_path / self.project_name / "legacy.py").write_text( + """ +import logging + +from twisted.internet.defer import succeed + + +class LegacyItemProcessor: + @classmethod + def from_crawler(cls, crawler): + return cls() + + def open_spider(self, spider): + return succeed(None) + + def close_spider(self, spider): + return succeed(None) + + def process_item(self, item, spider): + logging.info("Legacy item processor!") + return succeed(item) +""", + encoding="utf-8", + ) + _, _, stderr = proc( + "parse", + "--spider", + self.spider_name, + "--pipelines", + "-c", + "parse", + "-s", + f"ITEM_PROCESSOR={self.project_name}.legacy.LegacyItemProcessor", + mockserver.url("/html"), + cwd=proj_path, + ) + assert "INFO: Legacy item processor!" in stderr + + @pytest.mark.parametrize("verbose", [True, False]) + def test_no_items_no_links( + self, verbose: bool, proj_path: Path, mockserver: MockServer + ) -> None: + args = ["--verbose"] if verbose else [] + _, out, err = proc( + "parse", + "--spider", + self.spider_name, + "-c", + "parse", + "--noitems", + "--nolinks", + *args, + mockserver.url("/html"), + cwd=proj_path, + ) + assert "# Scraped Items" not in out, err + assert "# Requests" not in out + def test_parse_add_options(self): command = parse.Command() command.settings = Settings() diff --git a/tests/test_command_runspider.py b/tests/test_command_runspider.py index 2b410b5c6..11036eaeb 100644 --- a/tests/test_command_runspider.py +++ b/tests/test_command_runspider.py @@ -136,6 +136,12 @@ class MySpider(scrapy.Spider): log = self.get_log(tmp_path, "from scrapy.spiders import Spider\n") assert "No spider found in file" in log + @pytest.mark.parametrize("args", [(), ("a.py", "b.py")]) + def test_runspider_bad_arguments(self, args: tuple[str, ...]) -> None: + returncode, out, _ = proc("runspider", *args) + assert returncode == 2 + assert "Usage" in out + def test_runspider_file_not_found(self) -> None: _, _, log = proc("runspider", "some_non_existent_file") assert "File not found: some_non_existent_file" in log diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index f24200f53..29667a1ae 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -18,6 +18,7 @@ from scrapy.shell import Shell, inspect_response from scrapy.utils.reactor import _asyncio_reactor_path from scrapy.utils.test import get_crawler from tests import NON_EXISTING_RESOLVABLE, tests_datadir +from tests.utils.bases.commands import TestProjectBase from tests.utils.cmdline import proc from tests.utils.decorators import coroutine_test @@ -162,6 +163,33 @@ class TestShellCommand: assert ret == 0, out +class TestShellCommandWithSpider(TestProjectBase): + @pytest.fixture(autouse=True) + def create_files(self, proj_path: Path) -> None: + (proj_path / self.project_name / "spiders" / "myspider.py").write_text( + """ +import scrapy + +class MySpider(scrapy.Spider): + name = "myspider" +""", + encoding="utf-8", + ) + + def test_spider(self, proj_path: Path, mockserver: MockServer) -> None: + ret, out, err = proc( + "shell", + "--spider", + "myspider", + mockserver.url("/text"), + "-c", + "spider.name", + cwd=proj_path, + ) + assert ret == 0, err + assert out.strip() == "myspider" + + class TestInteractiveShell: def test_fetch(self, mockserver: MockServer) -> None: args = ( diff --git a/tests/test_commands.py b/tests/test_commands.py index 3e687e811..f20ecc153 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -3,21 +3,27 @@ from __future__ import annotations import argparse import json import sys +from pathlib import Path from typing import TYPE_CHECKING import pytest import scrapy from scrapy.cmdline import _pop_command_name, execute -from scrapy.commands import ScrapyCommand, ScrapyHelpFormatter, view +from scrapy.commands import ScrapyCommand, ScrapyHelpFormatter from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.settings import Settings from scrapy.utils.reactor import _asyncio_reactor_path from tests.utils.bases.commands import TestProjectBase -from tests.utils.cmdline import call, proc, write_recording_editor +from tests.utils.cmdline import ( + call, + proc, + write_recording_browser, + write_recording_editor, +) if TYPE_CHECKING: - from pathlib import Path + from tests.mockserver.http import MockServer class EmptyCommand(ScrapyCommand): @@ -107,6 +113,93 @@ class TestCommandSettings: ) +class TestGlobalOptions: + """Tests for the options that every command supports.""" + + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + name = "myspider" + + async def start(self): + self.logger.debug("It works!") + return + yield +""" + + @pytest.fixture + def spider_path(self, tmp_path: Path) -> Path: + path = tmp_path / "myspider.py" + path.write_text(self.spider_code, encoding="utf-8") + return path + + def test_invalid_set(self, spider_path: Path) -> None: + returncode, _, err = proc("runspider", str(spider_path), "-s", "FOO") + assert returncode == 2 + assert "Invalid -s value, use -s NAME=VALUE" in err + + def test_invalid_spider_argument(self, spider_path: Path) -> None: + returncode, _, err = proc("runspider", str(spider_path), "-a", "FOO") + assert returncode == 2 + assert "Invalid -a value, use -a NAME=VALUE" in err + + def test_logfile(self, tmp_path: Path, spider_path: Path) -> None: + logfile = tmp_path / "scrapy.log" + returncode, _, err = proc( + "runspider", str(spider_path), "--logfile", str(logfile) + ) + assert returncode == 0, err + assert "It works!" in logfile.read_text(encoding="utf-8") + assert "It works!" not in err + + def test_loglevel(self, spider_path: Path) -> None: + returncode, _, err = proc("runspider", str(spider_path), "--loglevel", "INFO") + assert returncode == 0, err + assert "It works!" not in err + assert "Spider closed (finished)" in err + + def test_nolog(self, spider_path: Path) -> None: + returncode, _, err = proc("runspider", str(spider_path), "--nolog") + assert returncode == 0, err + assert not err + + def test_pidfile(self, tmp_path: Path, spider_path: Path) -> None: + pidfile = tmp_path / "scrapy.pid" + returncode, _, err = proc( + "runspider", str(spider_path), "--pidfile", str(pidfile) + ) + assert returncode == 0, err + assert pidfile.read_text(encoding="utf-8").strip().isdigit() + + def test_pdb(self, spider_path: Path) -> None: + returncode, _, err = proc("runspider", str(spider_path), "--pdb") + assert returncode == 0, err + assert "It works!" in err + + +class TestSettingsCommand: + @pytest.mark.parametrize( + ("option", "setting", "expected"), + [ + ("--get", "BOT_NAME", "scrapybot"), + ("--getbool", "COOKIES_ENABLED", "True"), + ("--getint", "CONCURRENT_REQUESTS", "16"), + ("--getfloat", "DOWNLOAD_DELAY", "0.0"), + ("--getlist", "SPIDER_MODULES", "[]"), + ], + ) + def test_get(self, option: str, setting: str, expected: str) -> None: + returncode, out, err = proc("settings", option, setting) + assert returncode == 0, err + assert out.startswith(expected) + + def test_no_option(self) -> None: + returncode, out, err = proc("settings") + assert returncode == 0, err + assert not out + + class TestCommandCrawlerProcess(TestProjectBase): """Test that the command uses the expected kind of *CrawlerProcess and produces expected errors when needed.""" @@ -577,18 +670,31 @@ class TestBenchCommand: class TestViewCommand: - def test_methods(self) -> None: - command = view.Command() - command.settings = Settings() - parser = argparse.ArgumentParser( - prog="scrapy", - prefix_chars="-", - formatter_class=ScrapyHelpFormatter, - conflict_handler="resolve", + @pytest.mark.skipif( + sys.platform == "win32", reason="requires a POSIX shell browser script" + ) + def test_view( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, mockserver: MockServer + ) -> None: + opened = tmp_path / "opened.txt" + browser = tmp_path / "fake-browser.sh" + write_recording_browser(browser, opened) + monkeypatch.setenv("BROWSER", str(browser)) + + returncode, _, err = proc("view", mockserver.url("/html"), cwd=tmp_path) + + assert returncode == 0, err + url = opened.read_text(encoding="utf-8") + assert url.startswith("file://") + body = Path(url.removeprefix("file://")).read_text(encoding="utf-8") + assert "

Works

" in body + + def test_non_text_response(self, mockserver: MockServer) -> None: + returncode, _, err = proc( + "view", mockserver.url("/static/files/images/scrapy.png") ) - command.add_options(parser) - assert command.short_desc() == "Open URL in browser, as seen by Scrapy" - assert "URL using the Scrapy downloader and show its" in command.long_desc() + assert returncode == 0, err + assert "Cannot view a non-text response." in err class TestEditCommand(TestProjectBase): @@ -615,6 +721,11 @@ class TestEditCommand(TestProjectBase): assert returncode == 1 assert "Spider not found: nonexistent" in err + def test_edit_no_spider(self, proj_path: Path) -> None: + returncode, out, _ = proc("edit", cwd=proj_path) + assert returncode == 2 + assert "Usage" in out + class TestHelpMessage(TestProjectBase): @pytest.mark.parametrize( diff --git a/tests/utils/cmdline.py b/tests/utils/cmdline.py index 62dff3d4c..095cb17a7 100644 --- a/tests/utils/cmdline.py +++ b/tests/utils/cmdline.py @@ -46,3 +46,16 @@ def write_recording_editor(editor: Path) -> None: open (its last argument) into the file given as its first argument.""" editor.write_text('#!/bin/sh\nprintf "%s" "$2" > "$1"\n', encoding="utf-8") editor.chmod(0o755) + + +def write_recording_browser(browser: Path, recorded: Path) -> None: + """Create an executable browser script that writes the URL it is asked to + open into *recorded*. + + ``webbrowser`` only passes the URL to the command from the ``BROWSER`` + environment variable, hence the hardcoded output path. + """ + browser.write_text( + f'#!/bin/sh\nprintf "%s" "$1" > "{recorded}"\n', encoding="utf-8" + ) + browser.chmod(0o755) From aa5ded25398f9803b98d601439a4f0bf63f469f7 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 10:27:14 +0200 Subject: [PATCH 22/25] Set up CodSpeed (#7831) * Set up CodSpeed * CodSpeed: update permissions --- .github/workflows/codspeed.yml | 45 +++++++++++++++++ conftest.py | 3 ++ tests/benchmarks/__init__.py | 28 +++++++++++ tests/benchmarks/conftest.py | 27 ++++++++++ tests/benchmarks/test_benchmark_crawl.py | 64 ++++++++++++++++++++++++ tox.ini | 18 +++++++ 6 files changed, 185 insertions(+) create mode 100644 .github/workflows/codspeed.yml create mode 100644 tests/benchmarks/__init__.py create mode 100644 tests/benchmarks/conftest.py create mode 100644 tests/benchmarks/test_benchmark_crawl.py diff --git a/.github/workflows/codspeed.yml b/.github/workflows/codspeed.yml new file mode 100644 index 000000000..6930c4c1c --- /dev/null +++ b/.github/workflows/codspeed.yml @@ -0,0 +1,45 @@ +--- +name: codspeed + +on: + push: + branches: + - master + pull_request: + paths: + - scrapy/** + - tests/benchmarks/** + - .github/workflows/codspeed.yml + - tox.ini + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +permissions: {} + +jobs: + benchmark: + runs-on: ubuntu-latest + permissions: + contents: read + id-token: write # OIDC authentication with CodSpeed + + steps: + - uses: actions/checkout@v6 + + - name: Set up Python 3.14 + uses: actions/setup-python@v6 + with: + python-version: '3.14' + - name: Install dependencies + run: | + pip install --upgrade pip + pip install --upgrade tox + tox -n -e benchmark + - name: Run benchmarks + uses: CodSpeedHQ/action@v4 + with: + mode: simulation + run: tox -e benchmark diff --git a/conftest.py b/conftest.py index 27c398792..5a535c168 100644 --- a/conftest.py +++ b/conftest.py @@ -54,6 +54,9 @@ if not H2_ENABLED: if find_spec("httpx2") is None and find_spec("httpx") is None: collect_ignore.append("scrapy/core/downloader/handlers/_httpx.py") +if find_spec("pytest_codspeed") is None: + collect_ignore.append("tests/benchmarks") + def pytest_addoption(parser, pluginmanager): if pluginmanager.hasplugin("twisted"): diff --git a/tests/benchmarks/__init__.py b/tests/benchmarks/__init__.py new file mode 100644 index 000000000..7b5ca0cb9 --- /dev/null +++ b/tests/benchmarks/__init__.py @@ -0,0 +1,28 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from scrapy.utils.test import get_crawler + +if TYPE_CHECKING: + from scrapy import Spider + from scrapy.crawler import Crawler + + +def crawl(spidercls: type[Spider], settings: dict[str, Any], **kwargs: Any) -> Crawler: + """Run a crawl to completion and return its crawler. + + Unlike the rest of the test suite, benchmarks run without ``pytest-twisted`` + and drive the reactor themselves, since the code being measured must be + callable synchronously by ``pytest-codspeed``. + """ + from twisted.internet import reactor + + crawler = get_crawler(spidercls, settings) + result: list[Any] = [] + crawler.crawl(**kwargs).addBoth(result.append) + while not result: + reactor.iterate(0.001) + if isinstance(result[0], BaseException): + raise result[0] + return crawler diff --git a/tests/benchmarks/conftest.py b/tests/benchmarks/conftest.py new file mode 100644 index 000000000..55356083d --- /dev/null +++ b/tests/benchmarks/conftest.py @@ -0,0 +1,27 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +from scrapy.utils.reactor import install_reactor + +if TYPE_CHECKING: + from collections.abc import Generator + + +@pytest.fixture(scope="session", autouse=True) +def running_reactor() -> Generator[None]: + install_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor") + + from twisted.internet import reactor + + # Marks the reactor as running without blocking, so that crawls can be + # driven with reactor.iterate(), see tests.benchmarks.crawl(). + reactor.startRunning(installSignalHandlers=False) + + yield + + reactor.stop() + # Lets the shutdown event triggers run, e.g. to join the thread pool. + reactor.iterate(0) diff --git a/tests/benchmarks/test_benchmark_crawl.py b/tests/benchmarks/test_benchmark_crawl.py new file mode 100644 index 000000000..4ad0e672b --- /dev/null +++ b/tests/benchmarks/test_benchmark_crawl.py @@ -0,0 +1,64 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any +from urllib.parse import urlencode + +import pytest + +from scrapy import Field, Item, Request, Spider +from scrapy.linkextractors import LinkExtractor +from tests.benchmarks import crawl + +if TYPE_CHECKING: + from collections.abc import AsyncIterator + + from pytest_codspeed import BenchmarkFixture # type: ignore[import-not-found] + + from scrapy.http import Response + from tests.mockserver.http import MockServer + +pytest.importorskip("pytest_codspeed", reason="Benchmarks require pytest-codspeed") + +PAGES = 100 +LINKS_PER_PAGE = 5 + + +class _Page(Item): + url = Field() + anchors = Field() + + +class _FollowSpider(Spider): + name = "benchmark" + url: str + link_extractor = LinkExtractor() + + async def start(self) -> AsyncIterator[Any]: + yield Request(self.url, dont_filter=True) + + def parse(self, response: Response) -> Any: + yield _Page( + url=response.url, + anchors=response.css("a::text").getall(), + ) + for link in self.link_extractor.extract_links(response): # type: ignore[arg-type] + yield Request(link.url) + + +class _Pipeline: + def process_item(self, item: Any) -> Any: + return item + + +def test_benchmark_crawl(benchmark: BenchmarkFixture, mockserver: MockServer) -> None: + """Crawl of a set of interlinked pages served over HTTP.""" + query = urlencode({"total": PAGES, "show": LINKS_PER_PAGE, "order": "desc"}) + url = mockserver.url(f"/follow?{query}") + settings = {"ITEM_PIPELINES": {_Pipeline: 100}, "LOG_ENABLED": False} + + def run() -> None: + crawler = crawl(_FollowSpider, settings, url=url) + assert crawler.stats + assert crawler.stats.get_value("item_scraped_count") == PAGES + 1 + + benchmark(run) diff --git a/tox.ini b/tox.ini index e10a7cc0c..18e1579c9 100644 --- a/tox.ini +++ b/tox.ini @@ -30,6 +30,7 @@ envlist = botocore pypy3 pypy3-extra-deps + benchmark minversion = 1.7.0 [test-requirements] @@ -317,3 +318,20 @@ setenv = {[min]setenv} commands = pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=min-botocore.junit.xml -o junit_family=legacy} -m requires_botocore + + +# CPU benchmarks, tracked on CodSpeed. +# +# pytest-twisted is left out on purpose: benchmarked code must be callable +# synchronously, so tests/benchmarks drives the reactor itself. + +[testenv:benchmark] +basepython = python3.14 +deps = + pytest >= 8.4.1 + pytest-codspeed +passenv = + *codspeed* + *ci* +commands = + pytest {posargs:tests/benchmarks} --codspeed --codspeed-mode=simulation From 6cefaa5434da050bc23ffaaf2c1b38be1041bd31 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 12:43:40 +0200 Subject: [PATCH 23/25] Add a stats reference (#7814) --- .pre-commit-config.yaml | 2 +- docs/requirements.in | 2 +- docs/requirements.txt | 2 +- docs/topics/extensions.rst | 21 +- docs/topics/settings.rst | 3 +- docs/topics/stats.rst | 641 ++++++++++++++++++++++++++++++++++ scrapy/core/scheduler.py | 8 +- scrapy/extensions/logcount.py | 2 +- tox.ini | 2 +- 9 files changed, 659 insertions(+), 24 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 311df7052..c27348c7a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -27,6 +27,6 @@ repos: hooks: - id: sphinx-lint - repo: https://github.com/scrapy/sphinx-scrapy - rev: 0.8.8 + rev: 0.8.9 hooks: - id: sphinx-scrapy diff --git a/docs/requirements.in b/docs/requirements.in index a1f3a7468..257365380 100644 --- a/docs/requirements.in +++ b/docs/requirements.in @@ -5,4 +5,4 @@ sphinx sphinx-notfound-page sphinx-rtd-theme sphinx-rtd-dark-mode -sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.8 +sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.9 diff --git a/docs/requirements.txt b/docs/requirements.txt index a5cbad302..87634cea9 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -153,7 +153,7 @@ sphinx-rtd-theme==3.1.0 # via # -r docs/requirements.in # sphinx-rtd-dark-mode -sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@c0b2ac815afc3cb8857d575cecb5d55c05e6b737 +sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@912ed0507405e16ac60a47dd08195a1cd0ced984 # via -r docs/requirements.in sphinx-sitemap==2.9.0 # via sphinx-scrapy diff --git a/docs/topics/extensions.rst b/docs/topics/extensions.rst index 5598ab983..78b38cc3f 100644 --- a/docs/topics/extensions.rst +++ b/docs/topics/extensions.rst @@ -136,18 +136,10 @@ Core Stats extension Enable the collection of core statistics, provided the stats collection is enabled (see :ref:`topics-stats`). -The following stats are collected: - -* ``start_time``: start date/time of the crawl (:class:`~datetime.datetime`). -* ``finish_time``: end date/time of the crawl (:class:`~datetime.datetime`). -* ``elapsed_time_seconds``: total crawl duration in seconds (:class:`float`). -* ``finish_reason``: the closing reason string (e.g. ``"finished"``, - ``"closespider_timeout"``). -* ``item_scraped_count``: total number of items that passed all pipelines. -* ``item_dropped_count``: total number of items dropped by a pipeline. -* ``item_dropped_reasons_count/``: per-exception drop count - (e.g. ``item_dropped_reasons_count/DropItem``). -* ``response_received_count``: total number of HTTP responses received. +The following stats are collected: :stat:`elapsed_time_seconds`, +:stat:`finish_reason`, :stat:`finish_time`, :stat:`item_dropped_count`, +:stat:`item_dropped_reasons_count/{exception}`, :stat:`item_scraped_count`, +:stat:`response_received_count`, :stat:`start_time`. Log Count extension ~~~~~~~~~~~~~~~~~~~ @@ -190,7 +182,7 @@ Monitors the memory used by the Scrapy process that runs the spider and: 1. sends a :signal:`memusage_warning_reached` signal when it exceeds :setting:`MEMUSAGE_WARNING_MB` -2. closes the spider with the `"memusage_exceeded"` reason when it exceeds +2. closes the spider with the ``"memusage_exceeded"`` reason when it exceeds :setting:`MEMUSAGE_LIMIT_MB` This extension is enabled by the :setting:`MEMUSAGE_ENABLED` setting and @@ -214,7 +206,8 @@ An extension for debugging memory usage. It collects information about: * objects left alive that shouldn't. For more info, see :ref:`topics-leaks-trackrefs` To enable this extension, turn on the :setting:`MEMDEBUG_ENABLED` setting. The -info will be stored in the stats. +info will be stored in the :stat:`memdebug/gc_garbage_count` and +:stat:`memdebug/live_refs/{cls}` stats. .. _topics-extensions-ref-spiderstate: diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 81055afc2..b07dff180 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1871,7 +1871,8 @@ Default: ``False`` Setting to ``True`` will log debug information about the requests scheduler. This currently logs (only once) if the requests cannot be serialized to disk. -Stats counter (``scheduler/unserializable``) tracks the number of times this happens. +The :stat:`scheduler/unserializable` stat tracks the number of times this +happens. Example entry in logs:: diff --git a/docs/topics/stats.rst b/docs/topics/stats.rst index 0cf4a72cc..c702cefe7 100644 --- a/docs/topics/stats.rst +++ b/docs/topics/stats.rst @@ -21,6 +21,8 @@ using the Stats Collector from. Another feature of the Stats Collector is that it's very efficient (when enabled) and extremely efficient (almost unnoticeable) when disabled. +See :ref:`topics-stats-reference` below for the stats that Scrapy sets. + .. _topics-stats-usecases: Common Stats Collector uses @@ -101,3 +103,642 @@ DummyStatsCollector ------------------- .. autoclass:: DummyStatsCollector + +.. _topics-stats-reference: + +Built-in stats reference +======================== + +Scrapy sets the following :ref:`stats `. Components other than +those built into Scrapy may set additional stats; see their documentation. + +Stat keys that contain a ``{placeholder}`` below stand for a family of stats, +one per actual value of the placeholder. + +.. note:: Most stats are set by a specific :ref:`component + `, and are only present if that component is enabled and + its code path is reached. A stat that is missing from + :meth:`~scrapy.statscollectors.StatsCollector.get_stats` output is + equivalent to a counter of 0. + +.. stat:: downloader/exception_count + +``downloader/exception_count`` + Number of exceptions raised while downloading requests. + + Set by :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + +.. stat:: downloader/exception_type_count/{exception_type} + +``downloader/exception_type_count/{exception_type}`` + Number of exceptions raised while downloading requests, per exception type, + where ``{exception_type}`` is the import path of the exception class, e.g. + ``twisted.internet.error.DNSLookupError``. + + Set by :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + +.. stat:: downloader/request_bytes + +``downloader/request_bytes`` + Total size, in bytes, of the requests sent, counting the request line, the + headers and the body. As with :stat:`downloader/request_count`, requests + served from the cache are also counted. + + It is an approximation, reconstructed from each :class:`~scrapy.Request` + object instead of measured on the wire, so it does not account for the + actual bytes that the :ref:`download handler + ` sends, e.g. transport-level overhead. + + Set by :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + +.. stat:: downloader/request_count + +``downloader/request_count`` + Number of requests sent. + + Requests that :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware` + serves from the cache are also counted, even though they are never sent, + because it handles requests after + :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + + Set by :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + +.. stat:: downloader/request_method_count/{method} + +``downloader/request_method_count/{method}`` + Number of requests sent, per HTTP method, e.g. ``GET`` or ``POST``. As with + :stat:`downloader/request_count`, requests served from the cache are also + counted. + + Set by :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + +.. stat:: downloader/response_bytes + +``downloader/response_bytes`` + Total size, in bytes, of the responses received, counting the status line, + the headers and the body. It covers the same responses as + :stat:`downloader/response_count`. + + The body is counted as received, i.e. still compressed for responses that + used ``Content-Encoding``, because + :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats` handles + responses before + :class:`~scrapy.downloadermiddlewares.httpcompression.HttpCompressionMiddleware` + decompresses them. See :stat:`httpcompression/response_bytes` for + decompressed sizes. + + Set by :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + +.. stat:: downloader/response_count + +``downloader/response_count`` + Number of responses received. + + It counts responses that :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware` + serves from the cache, even though they do not come from the network, and + responses that a downloader middleware consumes before they reach your + spider, e.g. redirect responses that :class:`~scrapy.downloadermiddlewares.redirect.RedirectMiddleware` + turns into new requests. Compare with :stat:`response_received_count`. + + Set by :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + +.. stat:: downloader/response_status_count/{status_code} + +``downloader/response_status_count/{status_code}`` + Number of responses received, per HTTP status code, e.g. ``200`` or + ``404``. It covers the same responses as :stat:`downloader/response_count`. + + Set by :class:`~scrapy.downloadermiddlewares.stats.DownloaderStats`. + +.. stat:: dupefilter/filtered + +``dupefilter/filtered`` + Number of requests dropped as duplicates. + + Set by :class:`~scrapy.dupefilters.RFPDupeFilter`. + +.. stat:: elapsed_time_seconds + +``elapsed_time_seconds`` + Time, as a :class:`float`, in seconds, between the :signal:`spider_opened` + and the :signal:`spider_closed` signals. + + Set by :class:`~scrapy.extensions.corestats.CoreStats`. + +.. stat:: feedexport/failed_count/{storage} + +``feedexport/failed_count/{storage}`` + Number of :ref:`feeds ` that could not be stored, per + :ref:`storage backend `, where ``{storage}`` + is the class name of the storage backend, e.g. ``FileFeedStorage``. + +.. stat:: feedexport/success_count/{storage} + +``feedexport/success_count/{storage}`` + Number of :ref:`feeds ` stored successfully, per + :ref:`storage backend `, where ``{storage}`` + is the class name of the storage backend, e.g. ``FileFeedStorage``. + +.. stat:: file_count + +``file_count`` + Number of files handled by the :ref:`media pipelines + `. + +.. stat:: file_status_count/{status} + +``file_status_count/{status}`` + Number of files handled by the :ref:`media pipelines + `, per status, where ``{status}`` is one of: + + - ``downloaded``: the file was downloaded. + + - ``cached``: the file came from the + :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware` + cache. + + - ``uptodate``: the file was already in the storage backend and had not + :ref:`expired `, so it was not downloaded again. + +.. stat:: finish_reason + +``finish_reason`` + String indicating why the crawl finished. It matches the *reason* argument + of the :signal:`spider_closed` signal. + + Scrapy uses the following reasons: + + - ``cancelled``: the spider was closed without a more specific reason, + e.g. because :exc:`~scrapy.exceptions.CloseSpider` was raised without + one. + + - ``closespider_errorcount``: see :setting:`CLOSESPIDER_ERRORCOUNT`. + + - ``closespider_itemcount``: see :setting:`CLOSESPIDER_ITEMCOUNT`. + + - ``closespider_pagecount``: see :setting:`CLOSESPIDER_PAGECOUNT`. + + - ``closespider_pagecount_no_item``: see + :setting:`CLOSESPIDER_PAGECOUNT_NO_ITEM`. + + - ``closespider_timeout``: see :setting:`CLOSESPIDER_TIMEOUT`. + + - ``closespider_timeout_no_item``: see + :setting:`CLOSESPIDER_TIMEOUT_NO_ITEM`. + + - ``finished``: the spider became idle with no pending requests, i.e. it + finished normally. + + - ``memusage_exceeded``: see :setting:`MEMUSAGE_LIMIT_MB`. + + - ``shutdown``: the crawl was interrupted, e.g. by a system signal such + as ``SIGINT`` (:kbd:`Ctrl-C`). + + Third-party components and your own code may use any other reason, e.g. by + raising :exc:`~scrapy.exceptions.CloseSpider` with it. + + Set by :class:`~scrapy.extensions.corestats.CoreStats`. + +.. stat:: finish_time + +``finish_time`` + Timezone-aware :class:`~datetime.datetime` object, in UTC, indicating when + the :signal:`spider_closed` signal was sent. + + Set by :class:`~scrapy.extensions.corestats.CoreStats`. + +.. stat:: httpcache/errorrecovery + +``httpcache/errorrecovery`` + Number of times that a stale cached response was used because downloading a + fresh response raised an exception. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/firsthand + +``httpcache/firsthand`` + Number of responses that were downloaded without a matching cache entry to + validate against, i.e. responses for requests counted in + :stat:`httpcache/miss`. + + It is lower than :stat:`httpcache/miss` when some of those requests yield + no response, either because they are dropped (see + :stat:`httpcache/ignore`) or because their download fails. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/hit + +``httpcache/hit`` + Number of requests served from the cache. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/ignore + +``httpcache/ignore`` + Number of requests dropped because they were not in the cache and + :setting:`HTTPCACHE_IGNORE_MISSING` is ``True``. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/invalidate + +``httpcache/invalidate`` + Number of times that a cached response failed validation and was replaced + with a freshly downloaded response. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/miss + +``httpcache/miss`` + Number of requests for which no cache entry could be read, either because + there was none or because reading it failed, in which case the request is + also counted in :stat:`httpcache/retrieve_error`. Those requests are + downloaded (see :stat:`httpcache/firsthand`), or dropped if + :setting:`HTTPCACHE_IGNORE_MISSING` is ``True`` (see + :stat:`httpcache/ignore`). + + Requests with a stale cache entry are not counted here; see + :stat:`httpcache/revalidate` and :stat:`httpcache/invalidate`. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/retrieve_error + +``httpcache/retrieve_error`` + Number of cache entries that could not be read, and hence were treated as + cache misses. Those requests are also counted in :stat:`httpcache/miss`. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/revalidate + +``httpcache/revalidate`` + Number of times that a cached response was successfully validated against + the target server, and hence used instead of the fresh response. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/store + +``httpcache/store`` + Number of responses stored in the cache. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcache/uncacheable + +``httpcache/uncacheable`` + Number of responses not stored in the cache because the + :setting:`HTTPCACHE_POLICY` did not allow it. + + Every response considered for caching is counted either here or in + :stat:`httpcache/store`, so ``httpcache/store + httpcache/uncacheable`` + equals ``httpcache/firsthand + httpcache/invalidate``. + + Set by :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware`. + +.. stat:: httpcompression/response_bytes + +``httpcompression/response_bytes`` + Total size, in bytes, of decompressed response bodies, counting only the + body and only responses that were actually decompressed. Compare with + :stat:`downloader/response_bytes`. + + Set by + :class:`~scrapy.downloadermiddlewares.httpcompression.HttpCompressionMiddleware`. + +.. stat:: httpcompression/response_count + +``httpcompression/response_count`` + Number of decompressed responses. + + Set by + :class:`~scrapy.downloadermiddlewares.httpcompression.HttpCompressionMiddleware`. + +.. stat:: httperror/response_ignored_count + +``httperror/response_ignored_count`` + Number of responses dropped because of their HTTP status code. + + Set by :class:`~scrapy.spidermiddlewares.httperror.HttpErrorMiddleware`. + +.. stat:: httperror/response_ignored_status_count/{status_code} + +``httperror/response_ignored_status_count/{status_code}`` + Number of responses dropped because of their HTTP status code, per HTTP + status code, e.g. ``404``. + + Set by :class:`~scrapy.spidermiddlewares.httperror.HttpErrorMiddleware`. + +.. stat:: item_dropped_count + +``item_dropped_count`` + Number of items dropped by an :ref:`item pipeline + `, i.e. number of times that the + :signal:`item_dropped` signal was sent. + + Set by :class:`~scrapy.extensions.corestats.CoreStats`. + +.. stat:: item_dropped_reasons_count/{exception} + +``item_dropped_reasons_count/{exception}`` + Number of items dropped, per exception, where ``{exception}`` is the class + name of the exception that caused the item to be dropped. + + Only :exc:`~scrapy.exceptions.DropItem` and its subclasses drop items, and + each one is counted under its own class name, e.g. + ``item_dropped_reasons_count/DropItem`` for + :exc:`~scrapy.exceptions.DropItem` itself and + ``item_dropped_reasons_count/MyDropItem`` for a ``MyDropItem`` subclass of + it. Any other exception raised by an :ref:`item pipeline + ` triggers the :signal:`item_error` signal instead of + :signal:`item_dropped`, and is not counted here or in + :stat:`item_dropped_count`. + + Set by :class:`~scrapy.extensions.corestats.CoreStats`. + +.. stat:: item_scraped_count + +``item_scraped_count`` + Number of items that passed all :ref:`item pipelines + `, i.e. number of times that the + :signal:`item_scraped` signal was sent. + + Set by :class:`~scrapy.extensions.corestats.CoreStats`. + +.. stat:: items_per_minute + +``items_per_minute`` + Average number of items scraped per minute during the crawl. + + It is ``None`` if the crawl took less than a minute. + + Set by :class:`~scrapy.extensions.logstats.LogStats`. + +.. stat:: log_count/{level} + +``log_count/{level}`` + Number of log messages, per logging level name, e.g. ``INFO`` or + ``WARNING``. + + Only messages that the :setting:`LOG_LEVEL` setting allows are counted. + + Set by :class:`~scrapy.extensions.logcount.LogCount`. + +.. stat:: memdebug/gc_garbage_count + +``memdebug/gc_garbage_count`` + Number of objects in :data:`gc.garbage` when the spider is closed. + + Set by :class:`~scrapy.extensions.memdebug.MemoryDebugger`, which requires + :setting:`MEMDEBUG_ENABLED` to be ``True``. + +.. stat:: memdebug/live_refs/{cls} + +``memdebug/live_refs/{cls}`` + Number of live objects of class ``{cls}`` when the spider is closed, as + reported by :ref:`trackref `, e.g. + ``memdebug/live_refs/HtmlResponse``. + + Only set for classes with at least 1 live object. + + Set by :class:`~scrapy.extensions.memdebug.MemoryDebugger`, which requires + :setting:`MEMDEBUG_ENABLED` to be ``True``. + +.. stat:: memusage/limit_reached + +``memusage/limit_reached`` + ``1`` if memory usage exceeded :setting:`MEMUSAGE_LIMIT_MB`, which also + stops the crawl. + + Set by :class:`~scrapy.extensions.memusage.MemoryUsage`. + +.. stat:: memusage/max + +``memusage/max`` + Maximum peak memory usage, in bytes, observed during the crawl. + + Set by :class:`~scrapy.extensions.memusage.MemoryUsage`. + +.. stat:: memusage/startup + +``memusage/startup`` + Peak memory usage, in bytes, when the engine started. + + Set by :class:`~scrapy.extensions.memusage.MemoryUsage`. + +.. stat:: memusage/warning_reached + +``memusage/warning_reached`` + ``1`` if memory usage exceeded :setting:`MEMUSAGE_WARNING_MB`. + + Set by :class:`~scrapy.extensions.memusage.MemoryUsage`. + +.. stat:: offsite/domains + +``offsite/domains`` + Number of distinct domains for which at least 1 request was dropped for + being offsite. + + Set by :class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware`. + +.. stat:: offsite/filtered + +``offsite/filtered`` + Number of requests dropped for being offsite. + + Set by :class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware`. + +.. stat:: request_depth_count/{depth} + +``request_depth_count/{depth}`` + Number of requests scheduled at depth ``{depth}``, e.g. + ``request_depth_count/2``. + + Set by :class:`~scrapy.spidermiddlewares.depth.DepthMiddleware`, which + requires :setting:`DEPTH_STATS_VERBOSE` to be ``True`` for this stat. + +.. stat:: request_depth_max + +``request_depth_max`` + Maximum depth reached. + + Set by :class:`~scrapy.spidermiddlewares.depth.DepthMiddleware`. + +.. stat:: response_received_count + +``response_received_count`` + Number of responses received, i.e. number of times that the + :signal:`response_received` signal was sent. + + Unlike :stat:`downloader/response_count`, it does not count responses that + a downloader middleware consumes before they reach the engine, e.g. + redirect responses that :class:`~scrapy.downloadermiddlewares.redirect.RedirectMiddleware` + turns into new requests. Both count responses that :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware` + serves from the cache. + + Set by :class:`~scrapy.extensions.corestats.CoreStats`. + +.. stat:: responses_per_minute + +``responses_per_minute`` + Average number of responses received per minute during the crawl. + + It is ``None`` if the crawl took less than a minute. + + Set by :class:`~scrapy.extensions.logstats.LogStats`. + +.. stat:: retry/count + +``retry/count`` + Number of requests retried. + + Set by :func:`~scrapy.downloadermiddlewares.retry.get_retry_request`, which + :class:`~scrapy.downloadermiddlewares.retry.RetryMiddleware` uses. + +.. stat:: retry/max_reached + +``retry/max_reached`` + Number of requests that were not retried because they had already been + retried :setting:`RETRY_TIMES` times. + + Set by :func:`~scrapy.downloadermiddlewares.retry.get_retry_request`, which + :class:`~scrapy.downloadermiddlewares.retry.RetryMiddleware` uses. + +.. stat:: retry/reason_count/{reason} + +``retry/reason_count/{reason}`` + Number of requests retried, per reason, e.g. + ``retry/reason_count/twisted.internet.error.TimeoutError`` or + ``retry/reason_count/504 Gateway Time-out``. + + Set by :func:`~scrapy.downloadermiddlewares.retry.get_retry_request`, which + :class:`~scrapy.downloadermiddlewares.retry.RetryMiddleware` uses. + +.. note:: Code calling + :func:`~scrapy.downloadermiddlewares.retry.get_retry_request` may pass a + custom *stats_base_key*, in which case ``retry`` is replaced with that key + in the 3 stats above. + +.. stat:: robotstxt/exception_count/{exception_type} + +``robotstxt/exception_count/{exception_type}`` + Number of exceptions raised while downloading ``robots.txt`` files, per + exception type, where ``{exception_type}`` is the string representation of + the exception class, e.g. ````. + + Set by + :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware`. + +.. stat:: robotstxt/forbidden + +``robotstxt/forbidden`` + Number of requests dropped for being disallowed by ``robots.txt``. + + Set by + :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware`. + +.. stat:: robotstxt/request_count + +``robotstxt/request_count`` + Number of ``robots.txt`` files requested, i.e. 1 per network location for + which at least 1 request was sent. + + Set by + :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware`. + +.. stat:: robotstxt/response_count + +``robotstxt/response_count`` + Number of ``robots.txt`` responses received. + + Set by + :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware`. + +.. stat:: robotstxt/response_status_count/{status_code} + +``robotstxt/response_status_count/{status_code}`` + Number of ``robots.txt`` responses received, per HTTP status code, e.g. + ``404``. + + Set by + :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware`. + +.. stat:: scheduler/dequeued + +``scheduler/dequeued`` + Number of requests read from the :ref:`scheduler `. + +.. stat:: scheduler/dequeued/disk + +``scheduler/dequeued/disk`` + Number of requests read from the disk queue of the :ref:`scheduler + `. + +.. stat:: scheduler/dequeued/memory + +``scheduler/dequeued/memory`` + Number of requests read from the memory queue of the :ref:`scheduler + `. + +.. stat:: scheduler/enqueued + +``scheduler/enqueued`` + Number of requests stored into the :ref:`scheduler `. + +.. stat:: scheduler/enqueued/disk + +``scheduler/enqueued/disk`` + Number of requests stored into the disk queue of the :ref:`scheduler + `. + +.. stat:: scheduler/enqueued/memory + +``scheduler/enqueued/memory`` + Number of requests stored into the memory queue of the :ref:`scheduler + `. + +.. stat:: scheduler/unserializable + +``scheduler/unserializable`` + Number of requests that could not be stored into the disk queue of the + :ref:`scheduler ` because they could not be + :ref:`serialized `, and hence were stored into the + memory queue instead. + +.. stat:: spider_exceptions/count + +``spider_exceptions/count`` + Number of unhandled exceptions raised by spider callbacks. + + Set by the :ref:`scraper `. + +.. stat:: spider_exceptions/{exception} + +``spider_exceptions/{exception}`` + Number of unhandled exceptions raised by spider callbacks, per exception, + where ``{exception}`` is the class name of the exception, e.g. + ``spider_exceptions/ValueError``. + + Set by the :ref:`scraper `. + +.. stat:: start_time + +``start_time`` + Timezone-aware :class:`~datetime.datetime` object, in UTC, indicating when + the :signal:`spider_opened` signal was sent. + + Set by :class:`~scrapy.extensions.corestats.CoreStats`. + +.. stat:: urllength/request_ignored_count + +``urllength/request_ignored_count`` + Number of requests dropped for having a URL longer than + :setting:`URLLENGTH_LIMIT`. + + Set by :class:`~scrapy.spidermiddlewares.urllength.UrlLengthMiddleware`. diff --git a/scrapy/core/scheduler.py b/scrapy/core/scheduler.py index 42c517222..a511b0c7b 100644 --- a/scrapy/core/scheduler.py +++ b/scrapy/core/scheduler.py @@ -366,8 +366,8 @@ class Scheduler(BaseScheduler): Unless the received request is filtered out by the Dupefilter, attempt to push it into the disk queue, falling back to pushing it into the memory queue. - Increment the appropriate stats, such as: ``scheduler/enqueued``, - ``scheduler/enqueued/disk``, ``scheduler/enqueued/memory``. + Increment the appropriate stats, such as: :stat:`scheduler/enqueued`, + :stat:`scheduler/enqueued/disk`, :stat:`scheduler/enqueued/memory`. Return ``True`` if the request was stored successfully, ``False`` otherwise. """ @@ -390,8 +390,8 @@ class Scheduler(BaseScheduler): falling back to the disk queue if the memory queue is empty. Return ``None`` if there are no more enqueued requests. - Increment the appropriate stats, such as: ``scheduler/dequeued``, - ``scheduler/dequeued/disk``, ``scheduler/dequeued/memory``. + Increment the appropriate stats, such as: :stat:`scheduler/dequeued`, + :stat:`scheduler/dequeued/disk`, :stat:`scheduler/dequeued/memory`. """ request: Request | None = self.mqs.pop() assert self.stats is not None diff --git a/scrapy/extensions/logcount.py b/scrapy/extensions/logcount.py index e6d51a7d8..fcce64438 100644 --- a/scrapy/extensions/logcount.py +++ b/scrapy/extensions/logcount.py @@ -20,7 +20,7 @@ class LogCount: """Install a log handler that counts log messages by level. The handler installed is :class:`scrapy.utils.log.LogCounterHandler`. - The counts are stored in stats as ``log_count/``. + The counts are stored in the :stat:`log_count/{level}` stat. .. versionadded:: 2.14 """ diff --git a/tox.ini b/tox.ini index 18e1579c9..b59221340 100644 --- a/tox.ini +++ b/tox.ini @@ -5,7 +5,7 @@ [tox] requires = - sphinx-scrapy[tox] @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.8 + sphinx-scrapy[tox] @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.9 envlist = pre-commit pylint From 6f87d3f86334855484c8e1b0888976c70596b0e9 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 13:46:33 +0200 Subject: [PATCH 24/25] Rename the current benchmark for the future (#7839) --- .../{test_benchmark_crawl.py => test_crawl.py} | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) rename tests/benchmarks/{test_benchmark_crawl.py => test_crawl.py} (82%) diff --git a/tests/benchmarks/test_benchmark_crawl.py b/tests/benchmarks/test_crawl.py similarity index 82% rename from tests/benchmarks/test_benchmark_crawl.py rename to tests/benchmarks/test_crawl.py index 4ad0e672b..0fdfe742b 100644 --- a/tests/benchmarks/test_benchmark_crawl.py +++ b/tests/benchmarks/test_crawl.py @@ -50,8 +50,13 @@ class _Pipeline: return item -def test_benchmark_crawl(benchmark: BenchmarkFixture, mockserver: MockServer) -> None: - """Crawl of a set of interlinked pages served over HTTP.""" +def test_overhead_http(benchmark: BenchmarkFixture, mockserver: MockServer) -> None: + """Per-request overhead of a crawl over HTTP. + + The pages are small on purpose, so that the cost of parsing them stays + negligible next to the cost of moving requests and responses through the + engine, the middlewares and the download handler. + """ query = urlencode({"total": PAGES, "show": LINKS_PER_PAGE, "order": "desc"}) url = mockserver.url(f"/follow?{query}") settings = {"ITEM_PIPELINES": {_Pipeline: 100}, "LOG_ENABLED": False} From 1f03fbc17e1bf7ecdd7b7df06d7e3cf8c4f654f7 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 31 Jul 2026 15:59:38 +0200 Subject: [PATCH 25/25] Add scrapy.utils.asyncio.sleep() (#7843) --- docs/topics/asyncio.rst | 1 + scrapy/utils/asyncio.py | 20 +++++++++++++++++++- scrapy/utils/defer.py | 11 ++--------- tests/test_crawler_subprocess.py | 5 +++-- tests/test_engine_loop.py | 11 +++++------ tests/test_utils_asyncio.py | 11 +++++++++++ tests/utils/__init__.py | 10 ---------- 7 files changed, 41 insertions(+), 28 deletions(-) diff --git a/docs/topics/asyncio.rst b/docs/topics/asyncio.rst index 63c217e93..afccb491d 100644 --- a/docs/topics/asyncio.rst +++ b/docs/topics/asyncio.rst @@ -267,6 +267,7 @@ Here are some examples of APIs and patterns that need a replacement: Scrapy provides unified helpers for some of these examples: +.. autofunction:: scrapy.utils.asyncio.sleep .. autofunction:: scrapy.utils.asyncio.call_later .. autofunction:: scrapy.utils.asyncio.create_looping_call .. autoclass:: scrapy.utils.asyncio.AsyncioLoopingCall diff --git a/scrapy/utils/asyncio.py b/scrapy/utils/asyncio.py index 44604c0fe..7c7697f56 100644 --- a/scrapy/utils/asyncio.py +++ b/scrapy/utils/asyncio.py @@ -9,7 +9,7 @@ from collections.abc import AsyncIterator, Callable, Coroutine, Iterable from typing import TYPE_CHECKING, Any, Concatenate, ParamSpec, TypeVar from twisted.internet.defer import Deferred -from twisted.internet.task import LoopingCall +from twisted.internet.task import LoopingCall, deferLater from twisted.internet.threads import deferToThread from scrapy.utils.asyncgen import as_async_generator @@ -293,6 +293,24 @@ class CallLaterResult: self._delayed_call = None +async def sleep(seconds: float) -> None: + """Sleep for *seconds*. + + .. versionadded:: VERSION + + This uses either :func:`asyncio.sleep` or + :func:`~twisted.internet.task.deferLater`, depending on whether asyncio + support is available. + """ + if is_asyncio_available(): + await asyncio.sleep(seconds) + return + + from twisted.internet import reactor + + await deferLater(reactor, seconds) + + async def run_in_thread( func: Callable[_P, _T], *args: _P.args, **kwargs: _P.kwargs ) -> _T: diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index d0259b634..7c6235f29 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -27,7 +27,7 @@ from twisted.internet.task import Cooperator from twisted.python import failure from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.utils.asyncio import is_asyncio_available +from scrapy.utils.asyncio import is_asyncio_available, sleep from scrapy.utils.python import global_object_name if TYPE_CHECKING: @@ -90,14 +90,7 @@ async def _defer_sleep_async() -> None: """Delay by _DEFER_DELAY so reactor has a chance to go through readers and writers before attending pending delayed calls, so do not set delay to zero. """ - if is_asyncio_available(): - await asyncio.sleep(_DEFER_DELAY) - else: - from twisted.internet import reactor - - d: Deferred[None] = Deferred() - reactor.callLater(_DEFER_DELAY, d.callback, None) - await d + await sleep(_DEFER_DELAY) def defer_result(result: Any) -> Deferred[Any]: # pragma: no cover diff --git a/tests/test_crawler_subprocess.py b/tests/test_crawler_subprocess.py index fe2f83161..733b6797d 100644 --- a/tests/test_crawler_subprocess.py +++ b/tests/test_crawler_subprocess.py @@ -14,7 +14,8 @@ from packaging.version import parse as parse_version from pexpect.popen_spawn import PopenSpawn from w3lib import __version__ as w3lib_version -from tests.utils import async_sleep, get_script_run_env +from scrapy.utils.asyncio import sleep +from tests.utils import get_script_run_env from tests.utils.decorators import coroutine_test if TYPE_CHECKING: @@ -244,7 +245,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): p.kill(sig) p.expect_exact("shutting down gracefully") # sending the second signal too fast often causes problems - await async_sleep(0.01) + await sleep(0.01) p.kill(sig) p.expect_exact("forcing unclean shutdown") p.wait() # type: ignore[no-untyped-call] diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index c15c396d3..14ec3d184 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -6,10 +6,9 @@ from typing import TYPE_CHECKING, Any from scrapy import Request, Spider, signals from scrapy.core.scheduler import BaseScheduler -from scrapy.utils.asyncio import call_later +from scrapy.utils.asyncio import call_later, sleep from scrapy.utils.test import get_crawler from tests.mockserver.http import MockServer -from tests.utils import async_sleep from tests.utils.decorators import coroutine_test if TYPE_CHECKING: @@ -65,23 +64,23 @@ class TestMain: async def start(self): yield Request("data:,a") - await async_sleep(seconds) + await sleep(seconds) self.crawler.engine._slot.scheduler.pause() self.crawler.engine._slot.scheduler.enqueue_request(Request("data:,b")) # During this time, the scheduler reports having requests but # returns None. - await async_sleep(seconds) + await sleep(seconds) self.crawler.engine._slot.scheduler.unpause() # The scheduler request is processed. - await async_sleep(seconds) + await sleep(seconds) yield Request("data:,c") - await async_sleep(seconds) + await sleep(seconds) self.crawler.engine._slot.scheduler.pause() self.crawler.engine._slot.scheduler.enqueue_request(Request("data:,d")) diff --git a/tests/test_utils_asyncio.py b/tests/test_utils_asyncio.py index 9b7eb22fa..4bd54acd4 100644 --- a/tests/test_utils_asyncio.py +++ b/tests/test_utils_asyncio.py @@ -12,7 +12,9 @@ from scrapy.utils.asyncgen import as_async_generator from scrapy.utils.asyncio import ( AsyncioLoopingCall, _parallel_asyncio, + call_later, is_asyncio_available, + sleep, ) from tests.utils.decorators import coroutine_test @@ -26,6 +28,15 @@ async def test_is_asyncio_available(reactor_pytest: str) -> None: assert is_asyncio_available() == (reactor_pytest != "default") +@coroutine_test +async def test_sleep() -> None: + events: list[str] = [] + call_later(0.05, events.append, "call_later") + await sleep(0.1) + events.append("sleep") + assert events == ["call_later", "sleep"] + + @pytest.mark.only_asyncio class TestParallelAsyncio: """Test for scrapy.utils.asyncio.parallel_asyncio(), based on tests.test_utils_defer.TestParallelAsync.""" diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py index b0632a7ea..b27c5ade7 100644 --- a/tests/utils/__init__.py +++ b/tests/utils/__init__.py @@ -1,6 +1,5 @@ from __future__ import annotations -import asyncio import os from pathlib import Path from typing import TYPE_CHECKING @@ -8,8 +7,6 @@ from typing import TYPE_CHECKING from twisted.internet.defer import Deferred from scrapy.settings import Settings, default_settings -from scrapy.utils.asyncio import is_asyncio_available -from scrapy.utils.defer import maybe_deferred_to_future if TYPE_CHECKING: from collections.abc import Callable @@ -23,13 +20,6 @@ def twisted_sleep(seconds: float): return d -async def async_sleep(seconds: float) -> None: - if is_asyncio_available(): - await asyncio.sleep(seconds) - else: - await maybe_deferred_to_future(twisted_sleep(seconds)) - - def get_script_run_env() -> dict[str, str]: """Return a OS environment dict suitable to run scripts shipped with tests."""