Generic improvements related to reactorless tests (#6968)

* Skip doctests that import the reactor.

* Extract FTPDownloadHandler tests.

* Skip TestHttps2ClientProtocol via pytestmark.

* Replace some explicit reactor.callLater() in tests.

* Simplify TestRequestSendOrder._test_request_order().
This commit is contained in:
Andrey Rakhmatullin 2025-07-25 13:35:00 +05:00 committed by GitHub
parent 8c8f4ff033
commit 552f2fb91e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 249 additions and 248 deletions

View File

@ -49,6 +49,7 @@ You can usually fix the issue by moving those offending module-level Twisted
imports to the method or function definitions where they are used. For example,
if you have something like:
.. skip: next
.. code-block:: python
from twisted.internet import reactor

View File

@ -2003,6 +2003,7 @@ reactor is installed.
In order to use the reactor installed by Scrapy:
.. skip: next
.. code-block:: python
import scrapy

View File

@ -30,6 +30,7 @@ from scrapy.crawler import (
from scrapy.exceptions import ScrapyDeprecationWarning
from scrapy.extensions.throttle import AutoThrottle
from scrapy.settings import Settings, default_settings
from scrapy.utils.asyncio import call_later
from scrapy.utils.defer import deferred_f_from_coro_f, deferred_from_coro
from scrapy.utils.log import configure_logging, get_scrapy_root_handler
from scrapy.utils.spider import DefaultSpider
@ -932,8 +933,6 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin):
@inlineCallbacks
def test_shutdown_forced(self):
from twisted.internet import reactor
sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK
args = self.get_script_args("sleeping.py", "10")
p = PopenSpawn(args, timeout=5)
@ -943,7 +942,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin):
p.expect_exact("shutting down gracefully")
# sending the second signal too fast often causes problems
d = Deferred()
reactor.callLater(0.01, d.callback, None)
call_later(0.01, d.callback, None)
yield d
p.kill(sig)
p.expect_exact("forcing unclean shutdown")

View File

@ -0,0 +1,194 @@
from __future__ import annotations
import os
import sys
from pathlib import Path
from tempfile import mkstemp
from typing import TYPE_CHECKING, Any
import pytest
from pytest_twisted import async_yield_fixture
from twisted.cred import checkers, credentials, portal
from scrapy.core.downloader.handlers.ftp import FTPDownloadHandler
from scrapy.http import HtmlResponse, Request, Response
from scrapy.http.response.text import TextResponse
from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future
from scrapy.utils.misc import build_from_crawler
from scrapy.utils.python import to_bytes
from scrapy.utils.spider import DefaultSpider
from scrapy.utils.test import get_crawler
if TYPE_CHECKING:
from collections.abc import AsyncGenerator, Generator
class TestFTPBase:
username = "scrapy"
password = "passwd"
req_meta: dict[str, Any] = {"ftp_user": username, "ftp_password": password}
test_files = (
("file.txt", b"I have the power!"),
("file with spaces.txt", b"Moooooooooo power!"),
("html-file-without-extension", b"<!DOCTYPE html>\n<title>.</title>"),
)
def _create_files(self, root: Path) -> None:
userdir = root / self.username
userdir.mkdir()
for filename, content in self.test_files:
(userdir / filename).write_bytes(content)
def _get_factory(self, root):
from twisted.protocols.ftp import FTPFactory, FTPRealm
realm = FTPRealm(anonymousRoot=str(root), userHome=str(root))
p = portal.Portal(realm)
users_checker = checkers.InMemoryUsernamePasswordDatabaseDontUse()
users_checker.addUser(self.username, self.password)
p.registerChecker(users_checker, credentials.IUsernamePassword)
return FTPFactory(portal=p)
@async_yield_fixture
async def server_url(self, tmp_path: Path) -> AsyncGenerator[str]:
from twisted.internet import reactor
self._create_files(tmp_path)
factory = self._get_factory(tmp_path)
port = reactor.listenTCP(0, factory, interface="127.0.0.1")
portno = port.getHost().port
yield f"https://127.0.0.1:{portno}/"
await port.stopListening()
@staticmethod
@pytest.fixture
def dh() -> Generator[FTPDownloadHandler]:
crawler = get_crawler()
dh = build_from_crawler(FTPDownloadHandler, crawler)
yield dh
# if the test was skipped, there will be no client attribute
if hasattr(dh, "client"):
assert dh.client.transport
dh.client.transport.loseConnection()
@staticmethod
async def download_request(dh: FTPDownloadHandler, request: Request) -> Response:
return await maybe_deferred_to_future(
dh.download_request(request, DefaultSpider())
)
@deferred_f_from_coro_f
async def test_ftp_download_success(
self, server_url: str, dh: FTPDownloadHandler
) -> None:
request = Request(url=server_url + "file.txt", meta=self.req_meta)
r = await self.download_request(dh, request)
assert r.status == 200
assert r.body == b"I have the power!"
assert r.headers == {b"Local Filename": [b""], b"Size": [b"17"]}
assert r.protocol is None
@deferred_f_from_coro_f
async def test_ftp_download_path_with_spaces(
self, server_url: str, dh: FTPDownloadHandler
) -> None:
request = Request(
url=server_url + "file with spaces.txt",
meta=self.req_meta,
)
r = await self.download_request(dh, request)
assert r.status == 200
assert r.body == b"Moooooooooo power!"
assert r.headers == {b"Local Filename": [b""], b"Size": [b"18"]}
@deferred_f_from_coro_f
async def test_ftp_download_nonexistent(
self, server_url: str, dh: FTPDownloadHandler
) -> None:
request = Request(url=server_url + "nonexistent.txt", meta=self.req_meta)
r = await self.download_request(dh, request)
assert r.status == 404
@deferred_f_from_coro_f
async def test_ftp_local_filename(
self, server_url: str, dh: FTPDownloadHandler
) -> None:
f, local_fname = mkstemp()
fname_bytes = to_bytes(local_fname)
local_path = Path(local_fname)
os.close(f)
meta = {"ftp_local_filename": fname_bytes}
meta.update(self.req_meta)
request = Request(url=server_url + "file.txt", meta=meta)
r = await self.download_request(dh, request)
assert r.body == fname_bytes
assert r.headers == {b"Local Filename": [fname_bytes], b"Size": [b"17"]}
assert local_path.exists()
assert local_path.read_bytes() == b"I have the power!"
local_path.unlink()
@pytest.mark.parametrize(
("filename", "response_class"),
[
("file.txt", TextResponse),
("html-file-without-extension", HtmlResponse),
],
)
@deferred_f_from_coro_f
async def test_response_class(
self,
filename: str,
response_class: type[Response],
server_url: str,
dh: FTPDownloadHandler,
) -> None:
f, local_fname = mkstemp()
local_fname_path = Path(local_fname)
os.close(f)
meta = {}
meta.update(self.req_meta)
request = Request(url=server_url + filename, meta=meta)
r = await self.download_request(dh, request)
assert type(r) is response_class # pylint: disable=unidiomatic-typecheck
local_fname_path.unlink()
class TestFTP(TestFTPBase):
@deferred_f_from_coro_f
async def test_invalid_credentials(
self, server_url: str, dh: FTPDownloadHandler, reactor_pytest: str
) -> None:
if reactor_pytest == "asyncio" and sys.platform == "win32":
pytest.skip(
"This test produces DirtyReactorAggregateError on Windows with asyncio"
)
from twisted.protocols.ftp import ConnectionLost
meta = dict(self.req_meta)
meta.update({"ftp_password": "invalid"})
request = Request(url=server_url + "file.txt", meta=meta)
with pytest.raises(ConnectionLost):
await self.download_request(dh, request)
class TestAnonymousFTP(TestFTPBase):
username = "anonymous"
req_meta = {}
def _create_files(self, root: Path) -> None:
for filename, content in self.test_files:
(root / filename).write_bytes(content)
def _get_factory(self, tmp_path):
from twisted.protocols.ftp import FTPFactory, FTPRealm
realm = FTPRealm(anonymousRoot=str(tmp_path))
p = portal.Portal(realm)
p.registerChecker(checkers.AllowAnonymousAccess(), credentials.IAnonymous)
return FTPFactory(portal=p, userAnonymous=self.username)

View File

@ -4,35 +4,25 @@ from __future__ import annotations
import contextlib
import os
import sys
from pathlib import Path
from tempfile import mkdtemp, mkstemp
from typing import TYPE_CHECKING, Any
from unittest import mock
import pytest
from pytest_twisted import async_yield_fixture
from twisted.cred import checkers, credentials, portal
from w3lib.url import path_to_file_uri
from scrapy.core.downloader.handlers import DownloadHandlers
from scrapy.core.downloader.handlers.datauri import DataURIDownloadHandler
from scrapy.core.downloader.handlers.file import FileDownloadHandler
from scrapy.core.downloader.handlers.ftp import FTPDownloadHandler
from scrapy.core.downloader.handlers.s3 import S3DownloadHandler
from scrapy.exceptions import NotConfigured
from scrapy.http import HtmlResponse, Request, Response
from scrapy.http.response.text import TextResponse
from scrapy.http import Request, Response
from scrapy.responsetypes import responsetypes
from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future
from scrapy.utils.misc import build_from_crawler
from scrapy.utils.python import to_bytes
from scrapy.utils.spider import DefaultSpider
from scrapy.utils.test import get_crawler
if TYPE_CHECKING:
from collections.abc import AsyncGenerator, Generator
class DummyDH:
lazy = False
@ -305,177 +295,6 @@ class TestS3:
)
class TestFTPBase:
username = "scrapy"
password = "passwd"
req_meta: dict[str, Any] = {"ftp_user": username, "ftp_password": password}
test_files = (
("file.txt", b"I have the power!"),
("file with spaces.txt", b"Moooooooooo power!"),
("html-file-without-extension", b"<!DOCTYPE html>\n<title>.</title>"),
)
def _create_files(self, root: Path) -> None:
userdir = root / self.username
userdir.mkdir()
for filename, content in self.test_files:
(userdir / filename).write_bytes(content)
def _get_factory(self, root):
from twisted.protocols.ftp import FTPFactory, FTPRealm
realm = FTPRealm(anonymousRoot=str(root), userHome=str(root))
p = portal.Portal(realm)
users_checker = checkers.InMemoryUsernamePasswordDatabaseDontUse()
users_checker.addUser(self.username, self.password)
p.registerChecker(users_checker, credentials.IUsernamePassword)
return FTPFactory(portal=p)
@async_yield_fixture
async def server_url(self, tmp_path: Path) -> AsyncGenerator[str]:
from twisted.internet import reactor
self._create_files(tmp_path)
factory = self._get_factory(tmp_path)
port = reactor.listenTCP(0, factory, interface="127.0.0.1")
portno = port.getHost().port
yield f"https://127.0.0.1:{portno}/"
await port.stopListening()
@staticmethod
@pytest.fixture
def dh() -> Generator[FTPDownloadHandler]:
crawler = get_crawler()
dh = build_from_crawler(FTPDownloadHandler, crawler)
yield dh
# if the test was skipped, there will be no client attribute
if hasattr(dh, "client"):
assert dh.client.transport
dh.client.transport.loseConnection()
@staticmethod
async def download_request(dh: FTPDownloadHandler, request: Request) -> Response:
return await maybe_deferred_to_future(
dh.download_request(request, DefaultSpider())
)
@deferred_f_from_coro_f
async def test_ftp_download_success(
self, server_url: str, dh: FTPDownloadHandler
) -> None:
request = Request(url=server_url + "file.txt", meta=self.req_meta)
r = await self.download_request(dh, request)
assert r.status == 200
assert r.body == b"I have the power!"
assert r.headers == {b"Local Filename": [b""], b"Size": [b"17"]}
assert r.protocol is None
@deferred_f_from_coro_f
async def test_ftp_download_path_with_spaces(
self, server_url: str, dh: FTPDownloadHandler
) -> None:
request = Request(
url=server_url + "file with spaces.txt",
meta=self.req_meta,
)
r = await self.download_request(dh, request)
assert r.status == 200
assert r.body == b"Moooooooooo power!"
assert r.headers == {b"Local Filename": [b""], b"Size": [b"18"]}
@deferred_f_from_coro_f
async def test_ftp_download_nonexistent(
self, server_url: str, dh: FTPDownloadHandler
) -> None:
request = Request(url=server_url + "nonexistent.txt", meta=self.req_meta)
r = await self.download_request(dh, request)
assert r.status == 404
@deferred_f_from_coro_f
async def test_ftp_local_filename(
self, server_url: str, dh: FTPDownloadHandler
) -> None:
f, local_fname = mkstemp()
fname_bytes = to_bytes(local_fname)
local_path = Path(local_fname)
os.close(f)
meta = {"ftp_local_filename": fname_bytes}
meta.update(self.req_meta)
request = Request(url=server_url + "file.txt", meta=meta)
r = await self.download_request(dh, request)
assert r.body == fname_bytes
assert r.headers == {b"Local Filename": [fname_bytes], b"Size": [b"17"]}
assert local_path.exists()
assert local_path.read_bytes() == b"I have the power!"
local_path.unlink()
@pytest.mark.parametrize(
("filename", "response_class"),
[
("file.txt", TextResponse),
("html-file-without-extension", HtmlResponse),
],
)
@deferred_f_from_coro_f
async def test_response_class(
self,
filename: str,
response_class: type[Response],
server_url: str,
dh: FTPDownloadHandler,
) -> None:
f, local_fname = mkstemp()
local_fname_path = Path(local_fname)
os.close(f)
meta = {}
meta.update(self.req_meta)
request = Request(url=server_url + filename, meta=meta)
r = await self.download_request(dh, request)
assert type(r) is response_class # pylint: disable=unidiomatic-typecheck
local_fname_path.unlink()
class TestFTP(TestFTPBase):
@deferred_f_from_coro_f
async def test_invalid_credentials(
self, server_url: str, dh: FTPDownloadHandler, reactor_pytest: str
) -> None:
if reactor_pytest == "asyncio" and sys.platform == "win32":
pytest.skip(
"This test produces DirtyReactorAggregateError on Windows with asyncio"
)
from twisted.protocols.ftp import ConnectionLost
meta = dict(self.req_meta)
meta.update({"ftp_password": "invalid"})
request = Request(url=server_url + "file.txt", meta=meta)
with pytest.raises(ConnectionLost):
await self.download_request(dh, request)
class TestAnonymousFTP(TestFTPBase):
username = "anonymous"
req_meta = {}
def _create_files(self, root: Path) -> None:
for filename, content in self.test_files:
(root / filename).write_bytes(content)
def _get_factory(self, tmp_path):
from twisted.protocols.ftp import FTPFactory, FTPRealm
realm = FTPRealm(anonymousRoot=str(tmp_path))
p = portal.Portal(realm)
p.registerChecker(checkers.AllowAnonymousAccess(), credentials.IAnonymous)
return FTPFactory(portal=p, userAnonymous=self.username)
class TestDataURI:
def setup_method(self):
crawler = get_crawler()

View File

@ -17,6 +17,7 @@ from twisted.web.http import _DataLoss
from scrapy.http import Headers, HtmlResponse, Request, Response, TextResponse
from scrapy.spiders import Spider
from scrapy.utils.asyncio import call_later
from scrapy.utils.defer import (
deferred_f_from_coro_f,
deferred_from_coro,
@ -305,8 +306,6 @@ class TestHttp11Base(TestHttpBase):
async def test_download_with_maxsize_very_large_file(
self, mockserver: MockServer, download_handler: DownloadHandlerProtocol
) -> None:
from twisted.internet import reactor
# TODO: the logger check is specific to scrapy.core.downloader.handlers.http11
with mock.patch("scrapy.core.downloader.handlers.http11.logger") as logger:
request = Request(
@ -326,7 +325,7 @@ class TestHttp11Base(TestHttpBase):
# after closing the connection.
d: defer.Deferred[mock.Mock] = defer.Deferred()
d.addCallback(check)
reactor.callLater(0.1, d.callback, logger)
call_later(0.1, d.callback, logger)
await maybe_deferred_to_future(d)
@deferred_f_from_coro_f

View File

@ -138,7 +138,6 @@ class TestRequestSendOrder:
def get_num(self, request_or_response: Request | Response):
return int(request_or_response.url.rsplit("&", maxsplit=1)[1])
@deferred_f_from_coro_f
async def _test_request_order(
self,
start_nums,
@ -218,14 +217,12 @@ class TestRequestSendOrder:
return
yield
await maybe_deferred_to_future(
self._test_request_order(
start_nums=nums,
settings={"CONCURRENT_REQUESTS": 1},
response_seconds=response_seconds,
start_fn=start,
parse_fn=parse,
)
await self._test_request_order(
start_nums=nums,
settings={"CONCURRENT_REQUESTS": 1},
response_seconds=response_seconds,
start_fn=start,
parse_fn=parse,
)
@deferred_f_from_coro_f
@ -260,17 +257,15 @@ class TestRequestSendOrder:
return
yield
await maybe_deferred_to_future(
self._test_request_order(
start_nums=nums,
settings={
"CONCURRENT_REQUESTS": 1,
"SCHEDULER_START_MEMORY_QUEUE": "scrapy.squeues.LifoMemoryQueue",
},
response_seconds=response_seconds,
start_fn=start,
parse_fn=parse,
)
await self._test_request_order(
start_nums=nums,
settings={
"CONCURRENT_REQUESTS": 1,
"SCHEDULER_START_MEMORY_QUEUE": "scrapy.squeues.LifoMemoryQueue",
},
response_seconds=response_seconds,
start_fn=start,
parse_fn=parse,
)
@deferred_f_from_coro_f
@ -320,17 +315,15 @@ class TestRequestSendOrder:
return
yield
await maybe_deferred_to_future(
self._test_request_order(
start_nums=nums,
settings={
"CONCURRENT_REQUESTS": 1,
"SCHEDULER_START_MEMORY_QUEUE": None,
},
response_seconds=response_seconds,
start_fn=start,
parse_fn=parse,
)
await self._test_request_order(
start_nums=nums,
settings={
"CONCURRENT_REQUESTS": 1,
"SCHEDULER_START_MEMORY_QUEUE": None,
},
response_seconds=response_seconds,
start_fn=start,
parse_fn=parse,
)
# Examples from the “Start requests” section of the documentation about
@ -350,14 +343,12 @@ class TestRequestSendOrder:
request = self.request(num, response_seconds, download_slots)
yield request
await maybe_deferred_to_future(
self._test_request_order(
start_nums=start_nums,
cb_nums=cb_nums,
settings={
"CONCURRENT_REQUESTS": 1,
},
response_seconds=response_seconds,
start_fn=start,
)
await self._test_request_order(
start_nums=start_nums,
cb_nums=cb_nums,
settings={
"CONCURRENT_REQUESTS": 1,
},
response_seconds=response_seconds,
start_fn=start,
)

View File

@ -44,6 +44,11 @@ if TYPE_CHECKING:
from scrapy.core.http2.protocol import H2ClientProtocol
pytestmark = pytest.mark.skipif(
not H2_ENABLED, reason="HTTP/2 support in Twisted is not enabled"
)
def generate_random_string(size: int) -> str:
return "".join(random.choices(string.ascii_uppercase + string.digits, k=size))
@ -187,7 +192,6 @@ async def make_request(client: H2ClientProtocol, request: Request) -> Response:
return await maybe_deferred_to_future(make_request_dfd(client, request))
@pytest.mark.skipif(not H2_ENABLED, reason="HTTP/2 support in Twisted is not enabled")
class TestHttps2ClientProtocol:
scheme = "https"
host = "localhost"

View File

@ -14,6 +14,7 @@ from scrapy.http.request import NO_CALLBACK
from scrapy.pipelines.files import FileException
from scrapy.pipelines.media import MediaPipeline
from scrapy.spiders import Spider
from scrapy.utils.asyncio import call_later
from scrapy.utils.log import failure_to_exc_info
from scrapy.utils.signal import disconnect_all
from scrapy.utils.test import get_crawler
@ -337,10 +338,8 @@ class TestMediaPipeline(TestBaseMediaPipeline):
rsp1 = Response("http://url")
def rsp1_func():
from twisted.internet import reactor
dfd = Deferred().addCallback(_check_downloading)
reactor.callLater(0.1, dfd.callback, rsp1)
call_later(0.1, dfd.callback, rsp1)
return dfd
def rsp2_func():

View File

@ -4,6 +4,7 @@ import pytest
from twisted.internet.defer import Deferred, inlineCallbacks
from scrapy import Request, Spider, signals
from scrapy.utils.asyncio import call_later
from scrapy.utils.defer import deferred_to_future, maybe_deferred_to_future
from scrapy.utils.test import get_crawler, get_from_asyncio_queue
from tests.mockserver.http import MockServer
@ -30,9 +31,7 @@ class DeferredPipeline:
class AsyncDefPipeline:
async def process_item(self, item, spider):
d = Deferred()
from twisted.internet import reactor
reactor.callLater(0, d.callback, None)
call_later(0, d.callback, None)
await maybe_deferred_to_future(d)
item["pipeline_passed"] = True
return item
@ -41,9 +40,8 @@ class AsyncDefPipeline:
class AsyncDefAsyncioPipeline:
async def process_item(self, item, spider):
d = Deferred()
from twisted.internet import reactor
reactor.callLater(0, d.callback, None)
loop = asyncio.get_event_loop()
loop.call_later(0, d.callback, None)
await deferred_to_future(d)
await asyncio.sleep(0.2)
item["pipeline_passed"] = await get_from_asyncio_queue(True)

View File

@ -14,6 +14,7 @@ from scrapy.exceptions import _InvalidOutput
from scrapy.http import Request, Response
from scrapy.spiders import Spider
from scrapy.utils.asyncgen import collect_asyncgen
from scrapy.utils.asyncio import call_later
from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future
from scrapy.utils.test import get_crawler
@ -220,9 +221,7 @@ class ProcessSpiderExceptionAsyncIteratorMiddleware:
async def process_spider_exception(self, response, exception, spider):
yield {"foo": 1}
d = defer.Deferred()
from twisted.internet import reactor
reactor.callLater(0, d.callback, None)
call_later(0, d.callback, None)
await maybe_deferred_to_future(d)
yield {"foo": 2}
yield {"foo": 3}

View File

@ -7,6 +7,7 @@ from twisted.internet import defer
from twisted.internet.defer import inlineCallbacks
from twisted.python.failure import Failure
from scrapy.utils.asyncio import call_later
from scrapy.utils.defer import deferred_from_coro
from scrapy.utils.signal import (
send_catch_log,
@ -65,12 +66,10 @@ class TestSendCatchLogDeferred(TestSendCatchLog):
class TestSendCatchLogDeferred2(TestSendCatchLogDeferred):
def ok_handler(self, arg, handlers_called):
from twisted.internet import reactor
handlers_called.add(self.ok_handler)
assert arg == "test"
d = defer.Deferred()
reactor.callLater(0, d.callback, "OK")
call_later(0, d.callback, "OK")
return d
@ -98,12 +97,10 @@ class TestSendCatchLogAsync(TestSendCatchLog):
class TestSendCatchLogAsync2(TestSendCatchLogAsync):
def ok_handler(self, arg, handlers_called):
from twisted.internet import reactor
handlers_called.add(self.ok_handler)
assert arg == "test"
d = defer.Deferred()
reactor.callLater(0, d.callback, "OK")
call_later(0, d.callback, "OK")
return d