From a5614544a2ae3e2efe9d96ff32e476b57d9bec8f Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 9 Aug 2026 19:51:20 +0200 Subject: [PATCH] Type tests involving items and exports (#7867) --- pyproject.toml | 12 --- scrapy/extensions/feedexport.py | 2 +- scrapy/pipelines/images.py | 4 +- tests/mockserver/ftp.py | 8 +- tests/test_exporters.py | 74 +++++++------ tests/test_feedexport.py | 36 ++++--- tests/test_feedexport_postprocess.py | 18 +++- tests/test_feedexport_storages.py | 50 ++++----- tests/test_feedexport_uri_params.py | 33 ++++-- tests/test_item.py | 44 ++++---- tests/test_loader.py | 6 +- tests/test_pipeline_crawl.py | 10 +- tests/test_pipeline_files.py | 37 ++++--- tests/test_pipeline_images.py | 52 +++++---- tests/test_pipeline_media.py | 156 +++++++++++++++++++-------- tests/test_pipelines.py | 12 ++- 16 files changed, 343 insertions(+), 211 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4edfb7ec8..5ec1d05a4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -123,23 +123,11 @@ module = [ "tests.test_downloaderslotssettings", "tests.test_dupefilters", "tests.test_engine_loop", - "tests.test_exporters", "tests.test_extension_statsmailer", "tests.test_extension_throttle", - "tests.test_feedexport", - "tests.test_feedexport_postprocess", - "tests.test_feedexport_storages", - "tests.test_feedexport_uri_params", - "tests.test_item", "tests.test_linkextractors", - "tests.test_loader", "tests.test_logformatter", "tests.test_mail", - "tests.test_pipeline_crawl", - "tests.test_pipeline_files", - "tests.test_pipeline_images", - "tests.test_pipeline_media", - "tests.test_pipelines", "tests.test_pqueues", "tests.test_scheduler_base", "tests.test_settings", diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index fc2b2f43f..e4294ac90 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -149,7 +149,7 @@ class BlockingFeedStorage(ABC): return NamedTemporaryFile(prefix="feed-", dir=path) - def store(self, file: IO[bytes]) -> Deferred[None] | None: + def store(self, file: IO[bytes]) -> Deferred[None]: return deferred_from_coro(run_in_thread(self._store_in_thread, file)) @abstractmethod diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 79b6c4f27..5e7a4b409 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -29,7 +29,7 @@ from scrapy.utils.defer import ensure_awaitable from scrapy.utils.python import to_bytes if TYPE_CHECKING: - from collections.abc import Iterable + from collections.abc import Iterator from os import PathLike from PIL import Image @@ -180,7 +180,7 @@ class ImagesPipeline(FilesPipeline): info: MediaPipeline.SpiderInfo, *, item: Any = None, - ) -> Iterable[tuple[str, Image.Image, BytesIO]]: + ) -> Iterator[tuple[str, Image.Image, BytesIO]]: path = self.file_path(request, response=response, info=info, item=item) orig_image = self._Image.open(BytesIO(response.body)) transposed_image = self._ImageOps.exif_transpose(orig_image) diff --git a/tests/mockserver/ftp.py b/tests/mockserver/ftp.py index 1edd64dda..72760b5ac 100644 --- a/tests/mockserver/ftp.py +++ b/tests/mockserver/ftp.py @@ -27,11 +27,12 @@ class MockFTPServer: (anonymous) and a temporary root path that you can read from the :attr:`path` attribute.""" + proc: Popen[str] + port: int + path: Path + def __init__(self) -> None: - self.proc: Popen[str] | None = None self.host: str = "127.0.0.1" - self.port: int | None = None - self.path: Path | None = None def __enter__(self) -> Self: self.path = Path(mkdtemp()) @@ -63,7 +64,6 @@ class MockFTPServer: traceback: TracebackType | None, ) -> None: rmtree(str(self.path)) - assert self.proc is not None self.proc.kill() self.proc.communicate() diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 99afb8beb..956697a04 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -4,6 +4,7 @@ import marshal import pickle import re from abc import ABC, abstractmethod +from collections.abc import Mapping from datetime import datetime from io import BytesIO from typing import Any @@ -63,18 +64,18 @@ class TestBaseItemExporter(ABC): self.ie = self._get_exporter() @abstractmethod - def _get_exporter(self, **kwargs) -> BaseItemExporter: + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: raise NotImplementedError - def _check_output(self): # noqa: B027 + def _check_output(self) -> None: # noqa: B027 pass - def _assert_expected_item(self, exported_dict): + def _assert_expected_item(self, exported_dict: dict[str, Any]) -> None: for k, v in exported_dict.items(): exported_dict[k] = to_unicode(v) assert self.i == self.item_class(**exported_dict) - def _get_nonstring_types_item(self): + def _get_nonstring_types_item(self) -> dict[str, Any]: return { "boolean": False, "number": 22, @@ -82,7 +83,7 @@ class TestBaseItemExporter(ABC): "float": 3.14, } - def assertItemExportWorks(self, item): + def assertItemExportWorks(self, item: Any) -> None: self.ie.start_exporting() self.ie.export_item(item) self.ie.finish_exporting() @@ -92,7 +93,7 @@ class TestBaseItemExporter(ABC): del self.ie self._check_output() - def test_export_item(self): + def test_export_item(self) -> None: self.assertItemExportWorks(self.i) def test_export_dict_item(self): @@ -142,7 +143,7 @@ class TestBaseItemExporter(ABC): class TestPythonItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PythonItemExporter(**kwargs) def test_invalid_option(self): @@ -173,6 +174,7 @@ class TestPythonItemExporter(TestBaseItemExporter): "age": [{"age": [{"age": "22", "name": "Joseph"}], "name": "Maria"}], "name": "Jesus", } + assert exported is not None assert isinstance(exported["age"][0], dict) assert isinstance(exported["age"][0]["age"][0], dict) @@ -186,6 +188,7 @@ class TestPythonItemExporter(TestBaseItemExporter): "age": [{"age": [{"age": "22", "name": "Joseph"}], "name": "Maria"}], "name": "Jesus", } + assert exported is not None assert isinstance(exported["age"][0], dict) assert isinstance(exported["age"][0]["age"][0], dict) @@ -202,10 +205,10 @@ class TestPythonItemExporterDataclass(TestPythonItemExporter): class TestPprintItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PprintItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self._assert_expected_item(eval(self.output.getvalue())) @@ -215,10 +218,10 @@ class TestPprintItemExporterDataclass(TestPprintItemExporter): class TestPickleItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PickleItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self._assert_expected_item(pickle.loads(self.output.getvalue())) def test_export_multiple_items(self): @@ -252,10 +255,10 @@ class TestPickleItemExporterDataclass(TestPickleItemExporter): class TestMarshalItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return MarshalItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self.output.seek(0) self._assert_expected_item(marshal.load(self.output)) @@ -279,7 +282,7 @@ class TestMarshalItemExporterDataclass(TestMarshalItemExporter): class TestCsvItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: # We need a fresh instance for each exporter, because # CsvItemExporter.stream.__del__() closes the underlying file # (CsvItemExporter.finish_exporting() calls detach() but not all tests @@ -287,8 +290,10 @@ class TestCsvItemExporter(TestBaseItemExporter): self.output = BytesIO() return CsvItemExporter(self.output, **kwargs) - def assertCsvEqual(self, first, second, msg=None): - def split_csv(csv): + def assertCsvEqual( + self, first: bytes | str, second: bytes | str, msg: str | None = None + ) -> None: + def split_csv(csv: bytes | str) -> list[list[str]]: return [ sorted(re.split(r"(,|\s+)", line)) for line in to_unicode(csv).splitlines(True) @@ -296,13 +301,15 @@ class TestCsvItemExporter(TestBaseItemExporter): assert split_csv(first) == split_csv(second), msg - def _check_output(self): + def _check_output(self) -> None: self.output.seek(0) self.assertCsvEqual( to_unicode(self.output.read()), "age,name\r\n22,John\xa3\r\n" ) - def assertExportResult(self, item, expected, **kwargs): + def assertExportResult( + self, item: Any, expected: bytes | str = b"", **kwargs: Any + ) -> None: fp = BytesIO() ie = CsvItemExporter(fp, **kwargs) ie.start_exporting() @@ -383,7 +390,6 @@ class TestCsvItemExporter(TestBaseItemExporter): with pytest.raises(UnicodeEncodeError): self.assertExportResult( item={"text": "W\u0275\u200brd"}, - expected=None, encoding="windows-1251", ) @@ -417,7 +423,7 @@ class TestCsvItemExporterDataclass(TestCsvItemExporter): class TestXmlItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: # We need a fresh instance for each exporter, because # XmlItemExporter.stream.__del__() closes the underlying file # (XmlItemExporter.finish_exporting() calls detach() but not all tests @@ -425,20 +431,22 @@ class TestXmlItemExporter(TestBaseItemExporter): self.output = BytesIO() return XmlItemExporter(self.output, **kwargs) - def assertXmlEquivalent(self, first, second, msg=None): - def xmltuple(elem): + def assertXmlEquivalent( + self, first: bytes, second: bytes, msg: str | None = None + ) -> None: + def xmltuple(elem: Any) -> list[Any]: children = list(elem.iterchildren()) if children: return [(child.tag, sorted(xmltuple(child))) for child in children] return [(elem.tag, [(elem.text, ())])] - def xmlsplit(xmlcontent): + def xmlsplit(xmlcontent: bytes) -> list[Any]: doc = lxml.etree.fromstring(xmlcontent) return xmltuple(doc) assert xmlsplit(first) == xmlsplit(second), msg - def assertExportResult(self, item, expected_value): + def assertExportResult(self, item: Any, expected_value: bytes) -> None: fp = BytesIO() ie = XmlItemExporter(fp) ie.start_exporting() @@ -447,7 +455,7 @@ class TestXmlItemExporter(TestBaseItemExporter): del ie # See the first “del self.ie” in this file for context. self.assertXmlEquivalent(fp.getvalue(), expected_value) - def _check_output(self): + def _check_output(self) -> None: expected_value = ( b'\n' b"22John\xc2\xa3" @@ -538,10 +546,10 @@ class TestJsonLinesItemExporter(TestBaseItemExporter): "age": {"name": "Maria", "age": {"name": "Joseph", "age": "22"}}, } - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return JsonLinesItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: exported = json.loads(to_unicode(self.output.getvalue().strip())) assert exported == ItemAdapter(self.i).asdict() @@ -582,14 +590,14 @@ class TestJsonLinesItemExporterDataclass(TestJsonLinesItemExporter): class TestJsonItemExporter(TestJsonLinesItemExporter): _expected_nested = [TestJsonLinesItemExporter._expected_nested] - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return JsonItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: exported = json.loads(to_unicode(self.output.getvalue().strip())) assert exported == [ItemAdapter(self.i).asdict()] - def assertTwoItemsExported(self, item): + def assertTwoItemsExported(self, item: Any) -> None: self.ie.start_exporting() self.ie.export_item(item) self.ie.export_item(item) @@ -658,7 +666,7 @@ class TestJsonItemExporter(TestJsonLinesItemExporter): class TestJsonItemExporterToBytes(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: kwargs["encoding"] = "latin" return JsonItemExporter(self.output, **kwargs) @@ -690,7 +698,9 @@ class TestCustomExporterItem: def test_exporter_custom_serializer(self): class CustomItemExporter(BaseItemExporter): - def serialize_field(self, field, name, value): + def serialize_field( + self, field: Mapping[str, Any] | Field, name: str, value: Any + ) -> Any: if name == "age": return str(int(value) + 1) return super().serialize_field(field, name, value) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 40a763efd..d0f6a297c 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -100,6 +100,8 @@ class InstrumentedFeedSlot(FeedSlot): """Instrumented FeedSlot subclass for keeping track of calls to start_exporting and finish_exporting.""" + update_listener: Callable[[str], None] + def start_exporting(self): self.update_listener("start") super().start_exporting() @@ -109,7 +111,7 @@ class InstrumentedFeedSlot(FeedSlot): super().finish_exporting() @classmethod - def subscribe__listener(cls, listener): + def subscribe__listener(cls, listener: IsExportingListener) -> None: cls.update_listener = listener.update @@ -119,7 +121,7 @@ class IsExportingListener: finish_exporting and when a call to finish_exporting has been made before a call to start_exporting.""" - def __init__(self): + def __init__(self) -> None: self.start_without_finish = False self.finish_without_start = False @@ -307,6 +309,7 @@ class TestFeedExport(TestFeedExportBase): } crawler = get_crawler(ItemSpider, settings) yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/success_count/FileFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/success_count/FileFeedStorage") == 1 @@ -330,6 +333,7 @@ class TestFeedExport(TestFeedExportBase): side_effect=store, ): yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/failed_count/FileFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/failed_count/FileFeedStorage") == 1 @@ -347,6 +351,7 @@ class TestFeedExport(TestFeedExportBase): } crawler = get_crawler(ItemSpider, settings) yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/success_count/FileFeedStorage" in crawler.stats.get_stats() assert "feedexport/success_count/StdoutFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/success_count/FileFeedStorage") == 1 @@ -487,7 +492,7 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_start_finish_exporting_no_items(self): - items = [] + items: list[Any] = [] settings = { "FEEDS": { self._random_temp_filename(): {"format": "json"}, @@ -526,7 +531,7 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_start_finish_exporting_no_items_exception(self): - items = [] + items: list[Any] = [] settings = { "FEEDS": { self._random_temp_filename(): {"format": "json"}, @@ -611,7 +616,7 @@ class TestFeedExport(TestFeedExportBase): items = [{"foo": "bar"}] header = ["foo"] rows = [{"foo": "bar"}] - settings = {"FEED_EXPORT_FIELDS": []} + settings: dict[str, Any] = {"FEED_EXPORT_FIELDS": []} await self.assertExportedCsv(items, header, rows) await self.assertExportedJsonLines(items, rows, settings) @@ -727,14 +732,14 @@ class TestFeedExport(TestFeedExportBase): def accepts(self, item): return isinstance(item, MyItem) - class CustomFilter2(scrapy.extensions.feedexport.ItemFilter): + class CustomFilter2(ItemFilter): def accepts(self, item): return "foo" in item.fields - class CustomFilter3(scrapy.extensions.feedexport.ItemFilter): + class CustomFilter3(ItemFilter): def accepts(self, item): return ( - isinstance(item, tuple(self.item_classes)) and item["foo"] == "bar1" + isinstance(item, tuple(self.item_classes)) and item["foo"] == "bar1" # type: ignore[index] ) formats = { @@ -834,7 +839,7 @@ class TestFeedExport(TestFeedExportBase): } for fmt, expected in formats.items(): - settings = { + settings: dict[str, Any] = { "FEEDS": { self._random_temp_filename(): {"format": fmt}, }, @@ -911,7 +916,7 @@ class TestFeedExport(TestFeedExportBase): {"key": "value"}, ] - test_cases = [ + test_cases: list[dict[str, Any]] = [ # JSON { "format": "json", @@ -1132,7 +1137,7 @@ class TestFeedExport(TestFeedExportBase): expected_with_title_csv = b"foo,bar\r\nFOO,BAR\r\n" expected_without_title_csv = b"FOO,BAR\r\n" - test_cases = [ + test_cases: list[dict[str, Any]] = [ # with title { "options": { @@ -1166,6 +1171,9 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_storage_file_no_postprocessing(self): class Storage: + open_file: IO[bytes] + store_file: IO[bytes] + def __init__(self, uri, *, feed_options=None): pass @@ -1187,6 +1195,10 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_storage_file_postprocessing(self): class Storage: + open_file: IO[bytes] + store_file: IO[bytes] + file_was_closed: bool + def __init__(self, uri, *, feed_options=None): pass @@ -1299,7 +1311,7 @@ class TestItemFilter: class TestFeedExportInit: def test_unsupported_storage(self): - settings = { + settings: dict[str, Any] = { "FEEDS": { "unsupported://uri": {}, }, diff --git a/tests/test_feedexport_postprocess.py b/tests/test_feedexport_postprocess.py index 36d8586ce..69696e65e 100644 --- a/tests/test_feedexport_postprocess.py +++ b/tests/test_feedexport_postprocess.py @@ -74,7 +74,13 @@ class TestFeedPostProcessedExports(TestFeedExportBase): return content - def get_gzip_compressed(self, data, compresslevel=9, mtime=0, filename=""): + def get_gzip_compressed( + self, + data: bytes, + compresslevel: int = 9, + mtime: int = 0, + filename: str = "", + ) -> bytes: data_stream = BytesIO() gzipf = gzip.GzipFile( fileobj=data_stream, @@ -539,11 +545,13 @@ class TestFeedPostProcessedExports(TestFeedExportBase): data = await self.exported_data(self.items, settings) - for filename, result in data.items(): + for filename, data_bytes in data.items(): + expected: Any + result: Any if "pickle" in filename: - expected, result = self.items[0], pickle.loads(result) + expected, result = self.items[0], pickle.loads(data_bytes) elif "marshal" in filename: - expected, result = self.items[0], marshal.loads(result) + expected, result = self.items[0], marshal.loads(data_bytes) else: - expected = filename_to_expected[filename] + expected, result = filename_to_expected[filename], data_bytes assert result == expected diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index 6f9e33449..d7fb5c9c2 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -95,27 +95,34 @@ class TestFileFeedStorage: assert storage.path == path +def get_test_spider(settings: dict[str, Any] | None = None) -> scrapy.Spider: + class TestSpider(scrapy.Spider): + name = "test_spider" + + crawler = get_crawler(settings_dict=settings) + return TestSpider.from_crawler(crawler) + + class TestFTPFeedStorage: - def get_test_spider(self, settings=None): - class TestSpider(scrapy.Spider): - name = "test_spider" - - crawler = get_crawler(settings_dict=settings) - return TestSpider.from_crawler(crawler) - - async def _store(self, uri, content, feed_options=None, settings=None): + async def _store( + self, + uri: str, + content: bytes, + feed_options: dict[str, Any] | None = None, + settings: dict[str, Any] | None = None, + ) -> None: crawler = get_crawler(settings_dict=settings or {}) storage = FTPFeedStorage.from_crawler( crawler, uri, feed_options=feed_options, ) - spider = self.get_test_spider() + spider = get_test_spider() file = storage.open(spider) file.write(content) await maybe_deferred_to_future(storage.store(file)) - def _assert_stored(self, path: Path, content): + def _assert_stored(self, path: Path, content: bytes) -> None: assert path.exists() try: assert path.read_bytes() == content @@ -165,7 +172,7 @@ class TestFTPFeedStorage: def test_uri_auth_quote(self): # RFC3986: 3.2.1. User Information pw_quoted = quote(string.punctuation, safe="") - st = FTPFeedStorage(f"ftp://foo:{pw_quoted}@example.com/some_path", {}) + st = FTPFeedStorage(f"ftp://foo:{pw_quoted}@example.com/some_path") assert st.password == string.punctuation def test_uri_without_hostname(self): @@ -181,24 +188,17 @@ class MyBlockingFeedStorage(BlockingFeedStorage): class TestBlockingFeedStorage: - def get_test_spider(self, settings=None): - class TestSpider(scrapy.Spider): - name = "test_spider" - - crawler = get_crawler(settings_dict=settings) - return TestSpider.from_crawler(crawler) - def test_default_temp_dir(self): b = MyBlockingFeedStorage() - storage_file = b.open(self.get_test_spider()) + storage_file = b.open(get_test_spider()) storage_dir = Path(storage_file.name).parent assert str(storage_dir) == tempfile.gettempdir() def test_temp_file(self, tmp_path): b = MyBlockingFeedStorage() - spider = self.get_test_spider({"FEED_TEMPDIR": str(tmp_path)}) + spider = get_test_spider({"FEED_TEMPDIR": str(tmp_path)}) storage_file = b.open(spider) storage_dir = Path(storage_file.name).parent assert storage_dir == tmp_path @@ -207,7 +207,7 @@ class TestBlockingFeedStorage: b = MyBlockingFeedStorage() invalid_path = tmp_path / "invalid_path" - spider = self.get_test_spider({"FEED_TEMPDIR": str(invalid_path)}) + spider = get_test_spider({"FEED_TEMPDIR": str(invalid_path)}) with pytest.raises(OSError, match="Not a Directory:"): b.open(spider=spider) @@ -311,7 +311,7 @@ class TestS3FeedStorage: assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" assert storage.region_name == region_name - assert storage.s3_client._client_config.region_name == region_name + assert storage.s3_client._client_config.region_name == region_name # type: ignore[attr-defined] def test_from_crawler_without_acl(self): settings = { @@ -353,7 +353,7 @@ class TestS3FeedStorage: ) assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" - assert storage.s3_client._client_config.region_name == "us-east-1" + assert storage.s3_client._client_config.region_name == "us-east-1" # type: ignore[attr-defined] def test_from_crawler_with_acl(self): settings = { @@ -394,7 +394,7 @@ class TestS3FeedStorage: assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" assert storage.region_name == region_name - assert storage.s3_client._client_config.region_name == region_name + assert storage.s3_client._client_config.region_name == region_name # type: ignore[attr-defined] def test_init_without_max_pool_connections(self) -> None: storage = S3FeedStorage("s3://mybucket/export.csv", "access_key", "secret_key") @@ -497,7 +497,7 @@ class TestGCSFeedStorage: def test_parse_empty_acl(self): pytest.importorskip("google.cloud.storage") - settings = {"GCS_PROJECT_ID": "123", "FEED_STORAGE_GCS_ACL": ""} + settings: dict[str, Any] = {"GCS_PROJECT_ID": "123", "FEED_STORAGE_GCS_ACL": ""} crawler = get_crawler(settings_dict=settings) storage = GCSFeedStorage.from_crawler(crawler, "gs://mybucket/export.csv") assert storage.acl is None diff --git a/tests/test_feedexport_uri_params.py b/tests/test_feedexport_uri_params.py index 150d8449f..21c291732 100644 --- a/tests/test_feedexport_uri_params.py +++ b/tests/test_feedexport_uri_params.py @@ -2,6 +2,7 @@ from __future__ import annotations import warnings from abc import ABC, abstractmethod +from typing import TYPE_CHECKING, Any import pytest @@ -10,16 +11,27 @@ from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.extensions.feedexport import FeedExporter from scrapy.utils.test import get_crawler +if TYPE_CHECKING: + from collections.abc import Callable + + from scrapy.crawler import Crawler + class TestURIParams(ABC): spider_name = "uri_params_spider" deprecated_options = False @abstractmethod - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: raise NotImplementedError - def _crawler_feed_exporter(self, settings): + def _crawler_feed_exporter( + self, settings: dict[str, Any] + ) -> tuple[Crawler, FeedExporter]: if self.deprecated_options: with pytest.warns( ScrapyDeprecationWarning, @@ -29,6 +41,7 @@ class TestURIParams(ABC): else: crawler = get_crawler(settings_dict=settings) feed_exporter = crawler.get_extension(FeedExporter) + assert feed_exporter is not None return crawler, feed_exporter def test_default(self): @@ -116,8 +129,12 @@ class TestURIParams(ABC): class TestURIParamsSetting(TestURIParams): deprecated_options = True - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): - extra_settings = {} + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: + extra_settings: dict[str, Any] = {} if uri_params: extra_settings["FEED_URI_PARAMS"] = uri_params return { @@ -129,8 +146,12 @@ class TestURIParamsSetting(TestURIParams): class TestURIParamsFeedOption(TestURIParams): deprecated_options = False - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): - options = { + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: + options: dict[str, Any] = { "format": "jl", } if uri_params: diff --git a/tests/test_item.py b/tests/test_item.py index 7b4c2e918..45732f157 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -1,4 +1,5 @@ from abc import ABCMeta +from typing import Any from unittest import mock import pytest @@ -7,9 +8,6 @@ from scrapy.item import Field, Item, ItemMeta class TestItem: - def assertSortedEqual(self, first, second, msg=None): - assert sorted(first) == sorted(second), msg - def test_simple(self): class TestItem(Item): name = Field() @@ -98,16 +96,16 @@ class TestItem: i = TestItem() with pytest.raises(AttributeError): - i.name = "john" + i.name = "john" # type: ignore[assignment] def test_custom_methods(self): class TestItem(Item): name = Field() - def get_name(self): + def get_name(self) -> Any: return self["name"] - def change_name(self, name): + def change_name(self, name: str) -> None: self["name"] = name i = TestItem() @@ -121,40 +119,40 @@ class TestItem: def test_metaclass(self): class TestItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] i = TestItem() i["name"] = "John" - assert list(i.keys()) == ["name"] - assert list(i.values()) == ["John"] + assert list(i.keys()) == ["name"] # type: ignore[operator] + assert list(i.values()) == ["John"] # type: ignore[operator] i["keys"] = "Keys" i["values"] = "Values" - self.assertSortedEqual(list(i.keys()), ["keys", "values", "name"]) - self.assertSortedEqual(list(i.values()), ["Keys", "Values", "John"]) + assert sorted(i.keys()) == ["keys", "name", "values"] # type: ignore[operator] + assert sorted(i.values()) == ["John", "Keys", "Values"] # type: ignore[operator] def test_metaclass_with_fields_attribute(self): class TestItem(Item): fields = {"new": Field(default="X")} item = TestItem(new="New") - self.assertSortedEqual(list(item.keys()), ["new"]) - self.assertSortedEqual(list(item.values()), ["New"]) + assert list(item.keys()) == ["new"] + assert list(item.values()) == ["New"] def test_fields_order(self): class TestItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] assert list(TestItem.fields) == ["name", "keys", "values"] def test_fields_order_inheritance(self): class ParentItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] class TestItem(ParentItem): extra = Field() @@ -169,16 +167,16 @@ class TestItem: def test_metaclass_inheritance(self): class ParentItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] class TestItem(ParentItem): keys = Field() i = TestItem() i["keys"] = 3 - assert list(i.keys()) == ["keys"] - assert list(i.values()) == [3] + assert list(i.keys()) == ["keys"] # type: ignore[operator] + assert list(i.values()) == [3] # type: ignore[operator] def test_metaclass_multiple_inheritance_simple(self): class A(Item): @@ -314,7 +312,7 @@ class TestItemMeta: def f(self): # For rationale of this see: # https://github.com/python/cpython/blob/ee1a81b77444c6715cbe610e951c655b6adab88b/Lib/test/test_super.py#L222 - return __class__ + return __class__ # type: ignore[name-defined] MyItem() diff --git a/tests/test_loader.py b/tests/test_loader.py index c094d25d8..969bf5b98 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -79,7 +79,7 @@ class TestBasicItemLoader: class InitializationTestMixin: - item_class: type | None = None + item_class: type def test_keep_single_value(self): """Loaded item should contain values from the initial item""" @@ -311,7 +311,7 @@ class TestSelectortemLoader: def test_init_method_with_base_response(self): """Selector should be None after initialization""" response = Response("https://scrapy.org") - l = ProcessorItemLoader(response=response) + l = ProcessorItemLoader(response=response) # type: ignore[arg-type] assert l.selector is None def test_init_method_with_response(self): @@ -461,6 +461,7 @@ class TestSubselectorLoader: l = NestedItemLoader(response=self.response) nl = l.nested_xpath("//header") + assert nl.selector is not None nl.add_xpath("name", "div/text()") nl.add_css("name_div", "#id") nl.add_value("name_value", nl.selector.xpath('div[@id = "id"]/text()').getall()) @@ -476,6 +477,7 @@ class TestSubselectorLoader: def test_nested_css(self): l = NestedItemLoader(response=self.response) nl = l.nested_css("header") + assert nl.selector is not None nl.add_xpath("name", "div/text()") nl.add_css("name_div", "#id") nl.add_value("name_value", nl.selector.xpath('div[@id = "id"]/text()').getall()) diff --git a/tests/test_pipeline_crawl.py b/tests/test_pipeline_crawl.py index 0681371ef..8b522255a 100644 --- a/tests/test_pipeline_crawl.py +++ b/tests/test_pipeline_crawl.py @@ -23,7 +23,10 @@ if TYPE_CHECKING: class MediaDownloadSpider(SimpleSpider): name = "mediadownload" - def _process_url(self, url): + media_key: str + media_urls_key: str + + def _process_url(self, url: str) -> str: return url def parse(self, response): @@ -44,14 +47,15 @@ class MediaDownloadSpider(SimpleSpider): class BrokenLinksMediaDownloadSpider(MediaDownloadSpider): name = "brokenmedia" - def _process_url(self, url): + def _process_url(self, url: str) -> str: return url + ".foo" class RedirectedMediaDownloadSpider(MediaDownloadSpider): name = "redirectedmedia" - def _process_url(self, url): + def _process_url(self, url: str) -> str: + assert self.mockserver return add_or_replace_parameter( self.mockserver.url("/redirect-to"), "goto", url ) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4e7fb118b..3a8a3d0f7 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -22,6 +22,7 @@ from itemadapter import ItemAdapter from twisted.internet.defer import Deferred from twisted.python.failure import Failure +from scrapy.crawler import Crawler from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request, Response from scrapy.item import Field, Item @@ -75,7 +76,7 @@ class DeferredFSFilesStore(FSFilesStore): """A simple store with persist_file() returning a deferred.""" def persist_file(self, path, buf, info, meta=None, headers=None): - deferred = Deferred() + deferred: Deferred[None] = Deferred() # short-hand super() doesn't work in nested functions parent_persist_file = super().persist_file @@ -152,7 +153,7 @@ class TestFilesPipeline: file_path( Request("http://www.dorma.co.uk/images/product_details/2532"), response=Response("http://www.dorma.co.uk/images/product_details/2532"), - info=object(), + info=object(), # type: ignore[arg-type] ) == "full/244e0dd7d96a3b7b01f54eded250c9e272577aa1" ) @@ -383,7 +384,7 @@ class TestFilesPipeline: """ class CustomFilesPipeline(FilesPipeline): - def file_path(self, request, response=None, info=None, item=None): + def file_path(self, request, response=None, info=None, item=None) -> str: return f"full/{item.get('path')}" file_path = CustomFilesPipeline.from_crawler( @@ -476,7 +477,7 @@ class TestFilesPipeline: item["file_urls"] = bad_type with pytest.raises(TypeError, match="file_urls must be a list of URLs"): - list(pipeline.get_media_requests(item, None)) + list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] class TestFilesPipelineFieldsMixin(ABC): @@ -491,10 +492,10 @@ class TestFilesPipelineFieldsMixin(ABC): pipeline = FilesPipeline.from_crawler( get_crawler(None, {"FILES_STORE": tmp_path}) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] assert requests[0].url == url results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + item = pipeline.item_completed(results, item, None) # type: ignore[arg-type] files = ItemAdapter(item).get("files") assert files == [results[0][1]] assert isinstance(item, self.item_class) @@ -512,10 +513,10 @@ class TestFilesPipelineFieldsMixin(ABC): }, ) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] assert requests[0].url == url results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + item = pipeline.item_completed(results, item, None) # type: ignore[arg-type] custom_files = ItemAdapter(item).get("custom_files") assert custom_files == [results[0][1]] assert isinstance(item, self.item_class) @@ -581,8 +582,10 @@ class TestFilesPipelineCustomSettings: ("FILES_RESULT_FIELD", "FILES_RESULT_FIELD", "files_result_field"), } - def _generate_fake_settings(self, tmp_path, prefix=None): - def random_string(): + def _generate_fake_settings( + self, tmp_path: Path, prefix: str | None = None + ) -> dict[str, Any]: + def random_string() -> str: return "".join([chr(random.randint(97, 123)) for _ in range(10)]) settings = { @@ -599,7 +602,7 @@ class TestFilesPipelineCustomSettings: for k, v in settings.items() } - def _generate_fake_pipeline(self): + def _generate_fake_pipeline(self) -> type[FilesPipeline]: class UserDefinedFilePipeline(FilesPipeline): EXPIRES = 1001 FILES_URLS_FIELD = "alfa" @@ -739,14 +742,14 @@ class TestFilesPipelineCustomSettings: def test_file_pipeline_using_pathlike_objects(self, tmp_path): class CustomFilesPipelineWithPathLikeDir(FilesPipeline): - def file_path(self, request, response=None, info=None, *, item=None): - return Path("subdir") / Path(request.url).name + def file_path(self, request, response=None, info=None, *, item=None) -> str: + return str(Path("subdir") / Path(request.url).name) pipeline = CustomFilesPipelineWithPathLikeDir.from_crawler( get_crawler(None, {"FILES_STORE": tmp_path}) ) request = Request("http://example.com/image01.jpg") - assert pipeline.file_path(request) == Path("subdir/image01.jpg") + assert pipeline.file_path(request) == str(Path("subdir/image01.jpg")) class TestFSFilesStore: @@ -1092,7 +1095,7 @@ class TestFTPFileStore: store.port, store.username, store.password, - store.USE_ACTIVE_MODE, + bool(store.USE_ACTIVE_MODE), ) assert data == content @@ -1160,7 +1163,7 @@ class TestBuildFromCrawler: _from_crawler_called = False @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> "Pipeline": settings = crawler.settings store_uri = settings["FILES_STORE"] o = cls(store_uri, crawler=crawler) @@ -1179,7 +1182,7 @@ def test_files_pipeline_raises_notconfigured_when_files_store_invalid(store): settings = Settings() settings.clear() settings.set("FILES_STORE", store, priority="cmdline") - crawler = get_crawler(settings_dict=settings) + crawler = get_crawler(settings_dict=dict(settings)) with pytest.raises(NotConfigured): FilesPipeline.from_crawler(crawler) diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 19e61579f..43316f125 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -91,7 +91,7 @@ class TestImagesPipeline: file_path( Request("http://www.dorma.co.uk/images/product_details/2532"), response=Response("http://www.dorma.co.uk/images/product_details/2532"), - info=object(), + info=DUMMY_SPIDER_INFO, ) == "full/244e0dd7d96a3b7b01f54eded250c9e272577aa1.jpg" ) @@ -120,7 +120,7 @@ class TestImagesPipeline: Request("file:///tmp/some.name/foo"), name, response=Response("file:///tmp/some.name/foo"), - info=object(), + info=DUMMY_SPIDER_INFO, ) == "thumbs/50/850233df65a5b83361798f532f1fc549cd13cbe9.jpg" ) @@ -133,7 +133,7 @@ class TestImagesPipeline: class CustomImagesPipeline(ImagesPipeline): def thumb_path( self, request, thumb_id, response=None, info=None, item=None - ): + ) -> str: return f"thumb/{thumb_id}/{item.get('path')}" thumb_path = CustomImagesPipeline.from_crawler( @@ -159,11 +159,23 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp1, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp1, request=req, info=DUMMY_SPIDER_INFO + ) + ) with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp2, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp2, request=req, info=DUMMY_SPIDER_INFO + ) + ) with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp3, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp3, request=req, info=DUMMY_SPIDER_INFO + ) + ) def test_get_images(self): self.pipeline.min_width = 0 @@ -176,7 +188,7 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") get_images_gen = self.pipeline.get_images( - response=resp, request=req, info=object() + response=resp, request=req, info=DUMMY_SPIDER_INFO ) path, new_im, new_buf = next(get_images_gen) @@ -201,7 +213,7 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") get_images_gen = self.pipeline.get_images( - response=resp, request=req, info=object() + response=resp, request=req, info=DUMMY_SPIDER_INFO ) path, new_im, _ = next(get_images_gen) @@ -230,7 +242,7 @@ class TestImagesPipeline: def test_convert_image(self): SIZE = (100, 100) # straight forward case: RGB and JPEG - COLOUR = (0, 127, 255) + COLOUR: tuple[int, ...] = (0, 127, 255) im, buf = _create_image("JPEG", "RGB", SIZE, COLOUR) converted, converted_buf = self.pipeline.convert_image(im, response_body=buf) assert converted.mode == "RGB" @@ -296,7 +308,7 @@ class TestImagesPipeline: item["image_urls"] = bad_type with pytest.raises(TypeError, match="image_urls must be a list of URLs"): - list(pipeline.get_media_requests(item, None)) + list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) class TestImagesPipelineFieldsMixin(ABC): @@ -311,10 +323,10 @@ class TestImagesPipelineFieldsMixin(ABC): pipeline = ImagesPipeline.from_crawler( get_crawler(None, {"IMAGES_STORE": "s3://example/images/"}) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) assert requests[0].url == url - results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + results: Any = [(True, {"url": url})] + item = pipeline.item_completed(results, item, DUMMY_SPIDER_INFO) images = ItemAdapter(item).get("images") assert images == [results[0][1]] assert isinstance(item, self.item_class) @@ -332,10 +344,10 @@ class TestImagesPipelineFieldsMixin(ABC): }, ) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) assert requests[0].url == url - results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + results: Any = [(True, {"url": url})] + item = pipeline.item_completed(results, item, DUMMY_SPIDER_INFO) custom_images = ItemAdapter(item).get("custom_images") assert custom_images == [results[0][1]] assert isinstance(item, self.item_class) @@ -410,13 +422,15 @@ class TestImagesPipelineCustomSettings: "IMAGES_RESULT_FIELD": "images", } - def _generate_fake_settings(self, tmp_path, prefix=None): + def _generate_fake_settings( + self, tmp_path: Path, prefix: str | None = None + ) -> dict[str, Any]: """ :param prefix: string for setting keys :return: dictionary of image pipeline settings """ - def random_string(): + def random_string() -> str: return "".join([chr(random.randint(97, 123)) for _ in range(10)]) settings = { @@ -439,7 +453,7 @@ class TestImagesPipelineCustomSettings: for k, v in settings.items() } - def _generate_fake_pipeline_subclass(self): + def _generate_fake_pipeline_subclass(self) -> type[ImagesPipeline]: """ :return: ImagePipeline class will all uppercase attributes set. """ diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index ba1c18006..50787d7b7 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from typing import TYPE_CHECKING, Any, cast from unittest.mock import MagicMock import pytest @@ -10,7 +11,12 @@ from scrapy import signals from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.pipelines.files import FileException -from scrapy.pipelines.media import MediaPipeline, _MediaRequestFiltered +from scrapy.pipelines.media import ( + FileInfo, + FileInfoOrError, + MediaPipeline, + _MediaRequestFiltered, +) from scrapy.utils.defer import _defer_sleep_async from scrapy.utils.log import failure_to_exc_info from scrapy.utils.signal import disconnect_all @@ -19,21 +25,48 @@ from scrapy.utils.test import get_crawler from tests.utils.decorators import coroutine_test from tests.utils.media_pipelines import mocked_download_func +if TYPE_CHECKING: + from collections.abc import Awaitable + + from twisted.internet.defer import Deferred + + from scrapy.crawler import Crawler + class UserDefinedPipeline(MediaPipeline): - def media_to_download(self, request, info, *, item=None): - pass + def media_to_download( + self, request: Request, info: MediaPipeline.SpiderInfo, *, item: Any = None + ) -> Deferred[FileInfo | None] | None: + return None - def get_media_requests(self, item, info): - pass + def get_media_requests( + self, item: Any, info: MediaPipeline.SpiderInfo + ) -> list[Request]: + return [] - def media_downloaded(self, response, request, info, *, item=None): - return {} + def media_downloaded( + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: + return cast("FileInfo", {}) - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: failure.raiseException() - def file_path(self, request, response=None, info=None, *, item=None): + def file_path( + self, + request: Request, + response: Response | None = None, + info: MediaPipeline.SpiderInfo | None = None, + *, + item: Any = None, + ) -> str: return "" @@ -48,8 +81,14 @@ class TestBaseMediaPipeline: self.pipe = self.pipeline_class.from_crawler(crawler) self.pipe.open_spider() self.info = self.pipe.spiderinfo + assert crawler.request_fingerprinter is not None self.fingerprint = crawler.request_fingerprinter.fingerprint + @property + def mocked_pipe(self) -> MockedMediaPipeline: + assert isinstance(self.pipe, MockedMediaPipeline) + return self.pipe + def teardown_method(self): for name, signal in vars(signals).items(): if not name.startswith("_"): @@ -121,11 +160,13 @@ class TestBaseMediaPipeline: # When calling the method that caches the Request's result ... self.pipe._cache_result_and_execute_waiters(failure, fp, info) # ... it should store the Twisted Failure ... - assert info.downloaded[fp] == failure + downloaded = info.downloaded[fp] + assert downloaded == failure # ... encapsulating the original FileException ... - assert info.downloaded[fp].value == file_exc + assert isinstance(downloaded, Failure) + assert downloaded.value == file_exc # ... but it should not store the StopIteration exception on its context - context = getattr(info.downloaded[fp].value, "__context__", None) + context = getattr(downloaded.value, "__context__", None) assert context is None def test_default_item_completed(self, caplog: pytest.LogCaptureFixture) -> None: @@ -134,7 +175,7 @@ class TestBaseMediaPipeline: # Check that failures are logged by default fail = Failure(Exception()) - results = [(True, 1), (False, fail)] + results: Any = [(True, 1), (False, fail)] caplog.clear() new_item = self.pipe.item_completed(results, item, self.info) @@ -158,7 +199,7 @@ class TestBaseMediaPipeline: by item_completed(), as they are not download errors.""" item = {"name": "name"} fail = Failure(_MediaRequestFiltered("Filtered offsite request")) - results = [(True, 1), (False, fail)] + results: Any = [(True, 1), (False, fail)] with caplog.at_level(logging.DEBUG): new_item = self.pipe.item_completed(results, item, self.info) @@ -174,29 +215,44 @@ class TestBaseMediaPipeline: class MockedMediaPipeline(UserDefinedPipeline): - def __init__(self, *args, crawler=None, **kwargs): + def __init__(self, *args: Any, crawler: Crawler, **kwargs: Any): super().__init__(*args, crawler=crawler, **kwargs) - self._mockcalled = [] + self._mockcalled: list[str] = [] - def media_to_download(self, request, info, *, item=None): + def media_to_download( + self, request: Request, info: MediaPipeline.SpiderInfo, *, item: Any = None + ) -> Deferred[FileInfo | None] | None: self._mockcalled.append("media_to_download") if "result" in request.meta: return request.meta.get("result") return super().media_to_download(request, info) - def get_media_requests(self, item, info): + def get_media_requests( + self, item: Any, info: MediaPipeline.SpiderInfo + ) -> list[Request]: self._mockcalled.append("get_media_requests") - return item.get("requests") + return item.get("requests") # type: ignore[no-any-return] - def media_downloaded(self, response, request, info, *, item=None): + def media_downloaded( + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: self._mockcalled.append("media_downloaded") return super().media_downloaded(response, request, info) - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: self._mockcalled.append("media_failed") return super().media_failed(failure, request, info) - def item_completed(self, results, item, info): + def item_completed( + self, results: list[FileInfoOrError], item: Any, info: MediaPipeline.SpiderInfo + ) -> Any: self._mockcalled.append("item_completed") item = super().item_completed(results, item, info) item["results"] = results @@ -204,7 +260,14 @@ class MockedMediaPipeline(UserDefinedPipeline): class AsyncMediaDownloadedPipeline(MockedMediaPipeline): - async def media_downloaded(self, response, request, info, *, item=None): + async def media_downloaded( # type: ignore[override] + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: return super().media_downloaded(response, request, info) @@ -212,7 +275,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): pipeline_class = MockedMediaPipeline def _errback(self, result): - self.pipe._mockcalled.append("request_errback") + self.mocked_pipe._mockcalled.append("request_errback") return result @coroutine_test @@ -226,7 +289,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): item = {"requests": req} new_item = await self.pipe.process_item(item) assert new_item["results"] == [(True, {})] - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_downloaded", @@ -248,7 +311,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): assert new_item["results"][0][0] is False assert isinstance(new_item["results"][0][1], Failure) assert new_item["results"][0][1].value == exc - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", @@ -270,7 +333,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): assert new_item["results"][1][0] is False assert isinstance(new_item["results"][1][1], Failure) assert new_item["results"][1][1].value == exc - m = self.pipe._mockcalled + m = self.mocked_pipe._mockcalled # only once assert m[0] == "get_media_requests" # first hook called assert m.count("get_media_requests") == 1 @@ -294,7 +357,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): # returns iterable of Requests req1 = Request("http://url1") req2 = Request("http://url2") - item = {"requests": iter([req1, req2])} + item = {"requests": iter([req1, req2])} # type: ignore[dict-item] new_item = await self.pipe.process_item(item) assert new_item is item assert self.fingerprint(req1) in self.info.downloaded @@ -304,7 +367,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): async def test_results_are_cached_across_multiple_items(self): rsp1 = Response("http://url1") req1 = Request("http://url1", meta={"response": rsp1}) - item = {"requests": req1} + item: dict[str, Any] = {"requests": req1} new_item = await self.pipe.process_item(item) assert new_item is item assert new_item["results"] == [(True, {})] @@ -335,7 +398,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): 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 + assert self.mocked_pipe._mockcalled.count("media_to_download") == 1 @coroutine_test async def test_cached_failure_calls_errback(self): @@ -347,13 +410,13 @@ class TestMediaPipeline(TestBaseMediaPipeline): ) def errback(failure): - self.pipe._mockcalled.append("request_errback") + self.mocked_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 + assert self.mocked_pipe._mockcalled.count("request_errback") == 1 @coroutine_test async def test_results_are_cached_for_requests_of_single_item(self): @@ -362,14 +425,14 @@ class TestMediaPipeline(TestBaseMediaPipeline): req2 = Request( req1.url, meta={"response": Response("http://donot.download.me")} ) - item = {"requests": [req1, req2]} + item: dict[str, Any] = {"requests": [req1, req2]} new_item = await self.pipe.process_item(item) assert new_item is item assert new_item["results"] == [(True, {}), (True, {})] @coroutine_test async def test_wait_if_request_is_downloading(self): - def _check_downloading(response): + def _check_downloading(response: Response) -> Response: fp = self.fingerprint(req1) assert fp in self.info.downloading assert fp in self.info.waiting @@ -398,7 +461,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): item = {"requests": req} new_item = await self.pipe.process_item(item) assert new_item["results"] == [(True, "ITSME")] - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "item_completed", @@ -422,7 +485,9 @@ class TestAsyncMediaDownloaded(TestMediaPipeline): class TestMediaPipelineAllowRedirectSettings: - def _assert_request_no3xx(self, pipeline_class, settings): + def _assert_request_no3xx( + self, pipeline_class: type[MediaPipeline], settings: dict[str, Any] + ) -> None: pipe = pipeline_class(crawler=get_crawler(None, settings)) request = Request("http://url") pipe._modify_media_request(request) @@ -477,7 +542,7 @@ class TestBuildFromCrawler: self._init_called = True @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> Pipeline: settings = crawler.settings store_uri = settings["FILES_STORE"] o = cls(store_uri, settings=settings, crawler=crawler) @@ -493,9 +558,10 @@ class TestBuildFromCrawler: def test_has_from_crawler(self): class Pipeline(UserDefinedPipeline): _from_crawler_called = False + store_uri: str @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> Pipeline: settings = crawler.settings o = super().from_crawler(crawler) o._from_crawler_called = True @@ -509,7 +575,9 @@ class TestBuildFromCrawler: class MediaFailedNonePipeline(MockedMediaPipeline): - def media_failed(self, failure, request, info): + def media_failed( # type: ignore[override] + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> None: self._mockcalled.append("media_failed") @@ -524,7 +592,7 @@ class TestMediaFailedNone(TestBaseMediaPipeline): 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 == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", @@ -533,7 +601,9 @@ class TestMediaFailedNone(TestBaseMediaPipeline): class MediaFailedFailurePipeline(MockedMediaPipeline): - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: self._mockcalled.append("media_failed") return failure # deprecated @@ -544,7 +614,7 @@ class TestMediaFailedFailure(TestBaseMediaPipeline): pipeline_class = MediaFailedFailurePipeline def _errback(self, result): - self.pipe._mockcalled.append("request_errback") + self.mocked_pipe._mockcalled.append("request_errback") return result @coroutine_test @@ -565,7 +635,7 @@ class TestMediaFailedFailure(TestBaseMediaPipeline): assert new_item["results"][0][0] is False assert isinstance(new_item["results"][0][1], Failure) assert new_item["results"][0][1].value == exc - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index e65b5bc32..4b34946ae 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -47,7 +47,7 @@ class DeferredPipeline: return succeed(None) def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() d.addCallback(self.cb) d.callback(item) return d @@ -55,7 +55,7 @@ class DeferredPipeline: class AsyncDefPipeline: async def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() call_later(0, d.callback, None) await maybe_deferred_to_future(d) item["pipeline_passed"] = True @@ -64,7 +64,7 @@ class AsyncDefPipeline: class AsyncDefAsyncioPipeline: async def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() loop = asyncio.get_event_loop() loop.call_later(0, d.callback, None) await deferred_to_future(d) @@ -75,12 +75,12 @@ class AsyncDefAsyncioPipeline: class AsyncDefNotAsyncioPipeline: async def process_item(self, item): - d1 = Deferred() + d1: Deferred[Any] = Deferred() from twisted.internet import reactor reactor.callLater(0, d1.callback, None) await d1 - d2 = Deferred() + d2: Deferred[Any] = Deferred() reactor.callLater(0, d2.callback, None) await maybe_deferred_to_future(d2) item["pipeline_passed"] = True @@ -120,6 +120,8 @@ class OpenSpiderExceptionAsyncPipeline: class ItemSpider(Spider): name = "itemspider" + mockserver: MockServer + async def start(self): yield Request(self.mockserver.url("/status?n=200"))