RomanNum3ral
/
scrapy
mirror of https://github.com/scrapy/scrapy.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve test coverage for scrapy.pipelines (#7798) 

* Improve test coverage for scrapy.pipelines

* Restore old Pillow support
This commit is contained in:
Adrian 2026-07-29 11:32:18 +02:00 committed by GitHub
parent bc5b5fb1f6
commit 8b5147ae2e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 321 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

220
tests/test_pipeline_files.py
View File

@ -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()

52
tests/test_pipeline_images.py
View File

@ -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)

60
tests/test_pipeline_media.py
View File

@ -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")

6
tests/utils/media_pipelines.py
View File

@ -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: