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