From 6fe27ba33e9117a2d632396fe5e61d0fb32fa6cc Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 24 Mar 2026 15:29:22 +0500 Subject: [PATCH] Add more no-reactor tests (#7259) * Generic changes and scrapy bench. * scrapy check. * scrapy crawl. * scrapy fetch. * scrapy parse. * scrapy runspider. * scrapy shell. * Skip httpx tests on default-reactor. * Review requires_reactor marks. * Make test functions that require an event loop async def. * Improve test_pending_asyncio_tasks(). * Add Mac OS DNS error. * Refactor most of test_scheduler.py. * Finish refactoring DownloaderAwareSchedulerTestMixin. * Refactor test_engine_loop.py. * Add the no-reactor-extra-deps tox env, run no-reactor on macos. * Skip unhandled CancelledError when shutting down the engine. * Fix typing and pre-commit checks. * Fix typing problems in master. --------- Co-authored-by: Adrian --- .github/workflows/tests-macos.yml | 9 +- .github/workflows/tests-ubuntu.yml | 3 + scrapy/core/downloader/handlers/_httpx.py | 2 +- scrapy/core/engine.py | 4 +- scrapy/crawler.py | 4 + tests/test_command_check.py | 20 +- tests/test_command_crawl.py | 17 + tests/test_command_fetch.py | 6 + tests/test_command_parse.py | 16 + tests/test_command_runspider.py | 11 + tests/test_command_shell.py | 7 + tests/test_commands.py | 10 +- tests/test_core_downloader.py | 2 +- tests/test_crawler.py | 2 +- tests/test_downloader_handler_httpx.py | 2 + tests/test_downloader_handler_twisted_ftp.py | 2 +- .../test_downloader_handler_twisted_http10.py | 2 +- .../test_downloader_handler_twisted_http11.py | 2 +- .../test_downloader_handler_twisted_http2.py | 2 +- tests/test_downloaderslotssettings.py | 8 +- tests/test_engine.py | 4 +- tests/test_engine_loop.py | 29 +- tests/test_extension_periodic_log.py | 11 +- tests/test_extension_telnet.py | 2 +- tests/test_feedexport.py | 2 +- tests/test_feedexport_batch.py | 1 + tests/test_feedexport_storages.py | 6 +- tests/test_http2_client_protocol.py | 2 +- tests/test_logstats.py | 5 +- tests/test_mail.py | 2 +- tests/test_pipeline_files.py | 7 +- tests/test_pipelines.py | 6 +- tests/test_pqueues.py | 5 +- tests/test_scheduler.py | 333 +++++++++--------- tests/test_spidermiddleware_process_start.py | 4 +- tests/test_utils_asyncio.py | 22 +- tests/test_utils_defer.py | 46 +-- tests/test_utils_reactor.py | 6 +- tests/test_webclient.py | 1 + tests/test_zz_resources.py | 16 +- tests/utils/__init__.py | 15 +- tests/utils/decorators.py | 5 + tox.ini | 25 +- 43 files changed, 400 insertions(+), 286 deletions(-) diff --git a/.github/workflows/tests-macos.yml b/.github/workflows/tests-macos.yml index 3adf3de38..2a1c62833 100644 --- a/.github/workflows/tests-macos.yml +++ b/.github/workflows/tests-macos.yml @@ -19,6 +19,12 @@ jobs: fail-fast: false matrix: python-version: ["3.10", "3.11", "3.12", "3.13"] + env: + - TOXENV: py + include: + - python-version: '3.13' + env: + TOXENV: no-reactor steps: - uses: actions/checkout@v6 @@ -29,9 +35,10 @@ jobs: python-version: ${{ matrix.python-version }} - name: Run tests + env: ${{ matrix.env }} run: | pip install -U tox - tox -e py + tox - name: Upload coverage report uses: codecov/codecov-action@v5 diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index c07cd49f0..a34112698 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -64,6 +64,9 @@ jobs: - python-version: "3.13" env: TOXENV: extra-deps + - python-version: "3.13" + env: + TOXENV: no-reactor-extra-deps - python-version: pypy3.11 env: TOXENV: pypy3-extra-deps diff --git a/scrapy/core/downloader/handlers/_httpx.py b/scrapy/core/downloader/handlers/_httpx.py index 596e44f4c..916937152 100644 --- a/scrapy/core/downloader/handlers/_httpx.py +++ b/scrapy/core/downloader/handlers/_httpx.py @@ -72,7 +72,7 @@ class HttpxDownloadHandler(BaseHttpDownloadHandler): _DEFAULT_CONNECT_TIMEOUT = 10 def __init__(self, crawler: Crawler): - # we don't run extra-deps tests with the non-asyncio reactor + # we skip HttpxDownloadHandler tests with the non-asyncio reactor if not is_asyncio_available(): # pragma: no cover raise NotConfigured( f"{type(self).__name__} requires the asyncio support. Make" diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index dc4106963..94920e840 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -8,6 +8,7 @@ For more information see docs/topics/architecture.rst from __future__ import annotations import asyncio +import contextlib import logging import warnings from time import time @@ -197,7 +198,8 @@ class ExecutionEngine: self._start_request_processing_awaitable = asyncio.ensure_future(coro) else: self._start_request_processing_awaitable = Deferred.fromCoroutine(coro) - await maybe_deferred_to_future(self._closewait) + with contextlib.suppress(asyncio.exceptions.CancelledError): + await maybe_deferred_to_future(self._closewait) def stop(self) -> Deferred[None]: # pragma: no cover warnings.warn( diff --git a/scrapy/crawler.py b/scrapy/crawler.py index b2f08450b..2e07a3b87 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -143,6 +143,10 @@ class Crawler: change them here when the reactor is not used. """ self.settings.set("TELNETCONSOLE_ENABLED", False, priority="default") + for scheme in ("http", "https"): + self.settings["DOWNLOAD_HANDLERS_BASE"][scheme] = ( + "scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler" + ) # Cannot use @deferred_f_from_coro_f because that relies on the reactor # being installed already, which is done within _apply_settings(), inside diff --git a/tests/test_command_check.py b/tests/test_command_check.py index dc07ed695..c34e75624 100644 --- a/tests/test_command_check.py +++ b/tests/test_command_check.py @@ -46,10 +46,17 @@ class CheckSpider(scrapy.Spider): ) def _test_contract( - self, proj_path: Path, contracts: str = "", parse_def: str = "pass" + self, + proj_path: Path, + contracts: str = "", + parse_def: str = "pass", + use_reactor: bool = True, ) -> None: self._write_contract(proj_path, contracts, parse_def) - ret, out, err = proc("check", cwd=proj_path) + args = ["check"] + if not use_reactor: + args += ["-s", "TWISTED_ENABLED=False"] + ret, out, err = proc(*args, cwd=proj_path) assert "F" not in out assert "OK" in err assert ret == 0 @@ -63,6 +70,15 @@ class CheckSpider(scrapy.Spider): """ self._test_contract(proj_path, contracts, parse_def) + def test_check_no_reactor(self, proj_path: Path) -> None: + contracts = """ + @returns requests 1 + """ + parse_def = """ + yield scrapy.Request(url='http://next-url.com') + """ + self._test_contract(proj_path, contracts, parse_def, use_reactor=False) + def test_check_returns_items_contract(self, proj_path: Path) -> None: contracts = """ @returns items 1 diff --git a/tests/test_command_crawl.py b/tests/test_command_crawl.py index 5a223d122..1b5dee961 100644 --- a/tests/test_command_crawl.py +++ b/tests/test_command_crawl.py @@ -124,3 +124,20 @@ class MySpider(scrapy.Spider): not in log ) assert "Spider closed (finished)" in log + + def test_no_reactor(self, proj_path: Path) -> None: + spider_code = """ +import scrapy + +class MySpider(scrapy.Spider): + name = 'myspider' + + async def start(self): + self.logger.debug('It works!') + return + yield +""" + log = self.get_log(spider_code, proj_path, args=("-s", "TWISTED_ENABLED=False")) + assert "[myspider] DEBUG: It works!" in log + assert "Not using a Twisted reactor" in log + assert "Spider closed (finished)" in log diff --git a/tests/test_command_fetch.py b/tests/test_command_fetch.py index 4f4f19c3c..57db7dc50 100644 --- a/tests/test_command_fetch.py +++ b/tests/test_command_fetch.py @@ -30,3 +30,9 @@ class TestFetchCommand: out = out.replace("\r", "") # required on win32 assert "Server: TwistedWeb" in out assert "Content-Type: text/plain" in out + + def test_no_reactor(self, mockserver: MockServer) -> None: + _, out, _ = proc( + "fetch", "-s", "TWISTED_ENABLED=False", mockserver.url("/text") + ) + assert out.strip() == "Works" diff --git a/tests/test_command_parse.py b/tests/test_command_parse.py index 85529e607..d612b705b 100644 --- a/tests/test_command_parse.py +++ b/tests/test_command_parse.py @@ -513,3 +513,19 @@ ITEM_PIPELINES = {{'{self.project_name}.pipelines.MyPipeline': 1}} assert namespace.depth == 2 assert namespace.spider == self.spider_name assert namespace.verbose + + def test_no_reactor(self, proj_path: Path, mockserver: MockServer) -> None: + _, out, stderr = proc( + "parse", + "--spider", + "asyncdef_asyncio_return", + "-c", + "parse", + mockserver.url("/html"), + "-s", + "TWISTED_ENABLED=False", + cwd=proj_path, + ) + assert "INFO: Got response 200" in stderr + assert "{'id': 1}" in out + assert "{'id': 2}" in out diff --git a/tests/test_command_runspider.py b/tests/test_command_runspider.py index b5408ccfe..9bc65f152 100644 --- a/tests/test_command_runspider.py +++ b/tests/test_command_runspider.py @@ -213,6 +213,17 @@ class MySpider(scrapy.Spider): in log ) + def test_no_reactor(self, tmp_path: Path) -> None: + log = self.get_log( + tmp_path, + self.debug_log_spider, + args=[ + "-s", + "TWISTED_ENABLED=False", + ], + ) + assert "Not using a Twisted reactor" in log + def test_output(self, tmp_path: Path) -> None: spider_code = """ import scrapy diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 25262d809..ee515ede4 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -125,6 +125,13 @@ class TestShellCommand: assert ret == 0, err assert "RuntimeError: There is no current event loop in thread" not in err + @pytest.mark.xfail(reason="Not implemented yet", strict=True) + def test_shell_fetch_no_reactor(self, mockserver: MockServer) -> None: + url = mockserver.url("/html") + code = f"fetch('{url}')" + ret, _, err = proc("shell", "-c", code, "--set", "TWISTED_ENABLED=False") + assert ret == 0, err + class TestInteractiveShell: def test_fetch(self, mockserver: MockServer) -> None: diff --git a/tests/test_commands.py b/tests/test_commands.py index 07bd0b45c..209ffdc08 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -345,14 +345,18 @@ Unknown command: abc class TestBenchCommand: - def test_run(self) -> None: - _, _, err = proc( + @pytest.mark.parametrize("use_reactor", [True, False]) + def test_run(self, use_reactor: bool) -> None: + args: list[str] = [ "bench", "-s", "LOGSTATS_INTERVAL=0.001", "-s", "CLOSESPIDER_TIMEOUT=0.01", - ) + ] + if not use_reactor: + args += ["-s", "TWISTED_ENABLED=False"] + _, _, err = proc(*args) assert "INFO: Crawled" in err assert "Unhandled Error" not in err assert "log_count/ERROR" not in err diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index 35de52a4b..92991fb8f 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -39,7 +39,7 @@ class TestSlot: assert repr(slot) == "Slot(concurrency=8, delay=0.10, randomize_delay=True)" -@pytest.mark.requires_reactor +@pytest.mark.requires_reactor # this test is related to the Twisted HTTP code class TestContextFactoryBase: context_factory: ContextFactory | None = None diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 5c053089d..f4b4fbec9 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -666,7 +666,7 @@ class NoRequestsSpider(scrapy.Spider): yield -@pytest.mark.requires_reactor +@pytest.mark.requires_reactor # CrawlerRunner requires a reactor class TestCrawlerRunnerHasSpider: @staticmethod def _runner() -> CrawlerRunnerBase: diff --git a/tests/test_downloader_handler_httpx.py b/tests/test_downloader_handler_httpx.py index 8d146b496..1c11a1034 100644 --- a/tests/test_downloader_handler_httpx.py +++ b/tests/test_downloader_handler_httpx.py @@ -26,6 +26,8 @@ if TYPE_CHECKING: from tests.mockserver.http import MockServer +pytestmark = pytest.mark.only_asyncio + pytest.importorskip("httpx") diff --git a/tests/test_downloader_handler_twisted_ftp.py b/tests/test_downloader_handler_twisted_ftp.py index 957ad35c9..f39e5ecaf 100644 --- a/tests/test_downloader_handler_twisted_ftp.py +++ b/tests/test_downloader_handler_twisted_ftp.py @@ -25,7 +25,7 @@ if TYPE_CHECKING: from twisted.protocols.ftp import FTPFactory -pytestmark = pytest.mark.requires_reactor +pytestmark = pytest.mark.requires_reactor # FTPDownloadHandler requires a reactor class TestFTPBase(ABC): diff --git a/tests/test_downloader_handler_twisted_http10.py b/tests/test_downloader_handler_twisted_http10.py index 99f3697f8..a1cd0b7a6 100644 --- a/tests/test_downloader_handler_twisted_http10.py +++ b/tests/test_downloader_handler_twisted_http10.py @@ -16,7 +16,7 @@ if TYPE_CHECKING: from tests.mockserver.http import MockServer -pytestmark = pytest.mark.requires_reactor +pytestmark = pytest.mark.requires_reactor # HTTP10DownloadHandler requires a reactor class HTTP10DownloadHandlerMixin: diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index ffaa2c569..954dcd246 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -23,7 +23,7 @@ if TYPE_CHECKING: from scrapy.core.downloader.handlers import DownloadHandlerProtocol -pytestmark = pytest.mark.requires_reactor +pytestmark = pytest.mark.requires_reactor # HTTP11DownloadHandler requires a reactor class HTTP11DownloadHandlerMixin: diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 1b9dc21e7..b14356453 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -29,7 +29,7 @@ if TYPE_CHECKING: pytestmark = [ - pytest.mark.requires_reactor, + pytest.mark.requires_reactor, # H2DownloadHandler requires a reactor pytest.mark.skipif( not H2_ENABLED, reason="HTTP/2 support in Twisted is not enabled" ), diff --git a/tests/test_downloaderslotssettings.py b/tests/test_downloaderslotssettings.py index 1a3221dc9..a717b18a6 100644 --- a/tests/test_downloaderslotssettings.py +++ b/tests/test_downloaderslotssettings.py @@ -85,8 +85,8 @@ class TestCrawl: assert max(list(error_delta.values())) < tolerance -@pytest.mark.requires_reactor # needs a reactor or an event loop for Downloader._slot_gc_loop -def test_params(): +@coroutine_test +async def test_params(): params = { "concurrency": 1, "delay": 2, @@ -110,8 +110,8 @@ def test_params(): ) -@pytest.mark.requires_reactor # needs a reactor or an event loop for Downloader._slot_gc_loop -def test_get_slot_deprecated_spider_arg(): +@coroutine_test +async def test_get_slot_deprecated_spider_arg(): crawler = get_crawler(DefaultSpider) crawler.spider = crawler._create_spider() downloader = Downloader(crawler) diff --git a/tests/test_engine.py b/tests/test_engine.py index 131beeead..2cd583721 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -605,8 +605,8 @@ class TestEngineDownload(TestEngineDownloadAsync): return await maybe_deferred_to_future(engine.download(request)) -@pytest.mark.requires_reactor # needs a reactor or an event loop for _Slot.heartbeat -def test_request_scheduled_signal(caplog): +@coroutine_test +async def test_request_scheduled_signal(caplog): class TestScheduler(BaseScheduler): def __init__(self): self.enqueued = [] diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index 115deb821..8b5705487 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -4,30 +4,21 @@ from collections import deque from logging import ERROR from typing import TYPE_CHECKING -import pytest -from twisted.internet.defer import Deferred - from scrapy import Request, Spider, signals -from scrapy.utils.defer import maybe_deferred_to_future +from scrapy.utils.asyncio import call_later from scrapy.utils.test import get_crawler from tests.mockserver.http import MockServer from tests.test_scheduler import MemoryScheduler +from tests.utils import async_sleep from tests.utils.decorators import coroutine_test if TYPE_CHECKING: + import pytest + from scrapy.http import Response -async def sleep(seconds: float = 0.001) -> None: - from twisted.internet import reactor - - deferred: Deferred[None] = Deferred() - reactor.callLater(seconds, deferred.callback, None) - await maybe_deferred_to_future(deferred) - - class TestMain: - @pytest.mark.requires_reactor # TODO @coroutine_test async def test_sleep(self): """Neither asynchronous sleeps on Spider.start() nor the equivalent on @@ -40,27 +31,25 @@ class TestMain: name = "test" async def start(self): - from twisted.internet import reactor - yield Request("data:,a") - await sleep(seconds) + await async_sleep(seconds) self.crawler.engine._slot.scheduler.pause() self.crawler.engine._slot.scheduler.enqueue_request(Request("data:,b")) # During this time, the scheduler reports having requests but # returns None. - await sleep(seconds) + await async_sleep(seconds) self.crawler.engine._slot.scheduler.unpause() # The scheduler request is processed. - await sleep(seconds) + await async_sleep(seconds) yield Request("data:,c") - await sleep(seconds) + await async_sleep(seconds) self.crawler.engine._slot.scheduler.pause() self.crawler.engine._slot.scheduler.enqueue_request(Request("data:,d")) @@ -69,7 +58,7 @@ class TestMain: # delayed call below, proving that the start iteration can # finish before a scheduler “sleep” without causing the # scheduler to finish. - reactor.callLater(seconds, self.crawler.engine._slot.scheduler.unpause) + call_later(seconds, self.crawler.engine._slot.scheduler.unpause) def parse(self, response): pass diff --git a/tests/test_extension_periodic_log.py b/tests/test_extension_periodic_log.py index 86517e17b..18782fb70 100644 --- a/tests/test_extension_periodic_log.py +++ b/tests/test_extension_periodic_log.py @@ -3,12 +3,11 @@ from __future__ import annotations import datetime from typing import TYPE_CHECKING, Any -import pytest - from scrapy.extensions.periodic_log import PeriodicLog from scrapy.utils.test import get_crawler from .spiders import MetaSpider +from .utils.decorators import coroutine_test if TYPE_CHECKING: from collections.abc import Callable @@ -88,8 +87,8 @@ class TestPeriodicLog: assert extension({"PERIODIC_LOG_DELTA": True, "LOGSTATS_INTERVAL": 60}) assert extension({"PERIODIC_LOG_DELTA": "True", "LOGSTATS_INTERVAL": 60}) - @pytest.mark.requires_reactor # needs a reactor or an event loop for PeriodicLog.task - def test_log_delta(self): + @coroutine_test + async def test_log_delta(self): def emulate( settings: dict[str, Any] | None = None, ) -> tuple[PeriodicLog, dict[str, Any], dict[str, Any]]: @@ -154,8 +153,8 @@ class TestPeriodicLog: ), ) - @pytest.mark.requires_reactor # needs a reactor or an event loop for PeriodicLog.task - def test_log_stats(self): + @coroutine_test + async def test_log_stats(self): def emulate( settings: dict[str, Any] | None = None, ) -> tuple[PeriodicLog, dict[str, Any], dict[str, Any]]: diff --git a/tests/test_extension_telnet.py b/tests/test_extension_telnet.py index a956ab184..f1c86ce62 100644 --- a/tests/test_extension_telnet.py +++ b/tests/test_extension_telnet.py @@ -6,7 +6,7 @@ from scrapy.extensions.telnet import TelnetConsole from scrapy.utils.test import get_crawler from tests.utils.decorators import inline_callbacks_test -pytestmark = pytest.mark.requires_reactor +pytestmark = pytest.mark.requires_reactor # TelnetConsole requires a reactor class TestTelnetExtension: diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index ff9b0a3f9..e50b2be42 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -1151,7 +1151,7 @@ class TestFeedExport(TestFeedExportBase): data = await self.exported_no_data(settings) assert data["csv"] == b"" - @pytest.mark.requires_reactor # needs a reactor for BlockingFeedStorage + @pytest.mark.requires_reactor # TODO: needs a reactor for BlockingFeedStorage @coroutine_test async def test_multiple_feeds_success_logs_blocking_feed_storage(self): settings = { diff --git a/tests/test_feedexport_batch.py b/tests/test_feedexport_batch.py index 3b70e896c..16c4357bb 100644 --- a/tests/test_feedexport_batch.py +++ b/tests/test_feedexport_batch.py @@ -381,6 +381,7 @@ class TestBatchDeliveries(TestFeedExportBase): assert "feedexport/success_count/FileFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/success_count/FileFeedStorage") == 12 + @pytest.mark.requires_reactor # TODO: needs a reactor for BlockingFeedStorage @pytest.mark.requires_boto3 @inline_callbacks_test def test_s3_export(self): diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index b4da9bc68..c5cd5c782 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -116,7 +116,7 @@ class TestFileFeedStorage: assert storage.path == path -@pytest.mark.requires_reactor # needs a reactor for BlockingFeedStorage +@pytest.mark.requires_reactor # TODO: needs a reactor for BlockingFeedStorage class TestFTPFeedStorage: def get_test_spider(self, settings=None): class TestSpider(scrapy.Spider): @@ -231,7 +231,7 @@ class TestBlockingFeedStorage: @pytest.mark.requires_boto3 -@pytest.mark.requires_reactor # needs a reactor for BlockingFeedStorage +@pytest.mark.requires_reactor # TODO: needs a reactor for BlockingFeedStorage class TestS3FeedStorage: def test_parse_credentials(self): aws_credentials = { @@ -461,7 +461,7 @@ class TestS3FeedStorage: assert "S3 does not support appending to files" in str(log) -@pytest.mark.requires_reactor # needs a reactor for BlockingFeedStorage +@pytest.mark.requires_reactor # TODO: needs a reactor for BlockingFeedStorage class TestGCSFeedStorage: def test_parse_settings(self): try: diff --git a/tests/test_http2_client_protocol.py b/tests/test_http2_client_protocol.py index 49b9180b9..65da89472 100644 --- a/tests/test_http2_client_protocol.py +++ b/tests/test_http2_client_protocol.py @@ -40,7 +40,7 @@ if TYPE_CHECKING: pytestmark = [ - pytest.mark.requires_reactor, + pytest.mark.requires_reactor, # H2ClientProtocol requires a reactor pytest.mark.skipif( not H2_ENABLED, reason="HTTP/2 support in Twisted is not enabled" ), diff --git a/tests/test_logstats.py b/tests/test_logstats.py index c2e6d3456..370728e6a 100644 --- a/tests/test_logstats.py +++ b/tests/test_logstats.py @@ -5,6 +5,7 @@ import pytest from scrapy.extensions.logstats import LogStats from scrapy.utils.test import get_crawler from tests.spiders import SimpleSpider +from tests.utils.decorators import coroutine_test class TestLogStats: @@ -16,8 +17,8 @@ class TestLogStats: self.stats.set_value("response_received_count", 4802) self.stats.set_value("item_scraped_count", 3201) - @pytest.mark.requires_reactor # needs a reactor or an event loop for LogStats.task - def test_stats_calculations(self): + @coroutine_test + async def test_stats_calculations(self): logstats = LogStats.from_crawler(self.crawler) with pytest.raises(AttributeError): diff --git a/tests/test_mail.py b/tests/test_mail.py index c845ba320..61136dfbe 100644 --- a/tests/test_mail.py +++ b/tests/test_mail.py @@ -8,7 +8,7 @@ from twisted.internet._sslverify import ClientTLSOptions from scrapy.mail import MailSender -@pytest.mark.requires_reactor +@pytest.mark.requires_reactor # MailSender requires a reactor class TestMailSender: def test_send(self): mailsender = MailSender(debug=True) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 7f59af7de..201b29f83 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -584,6 +584,7 @@ class TestFilesPipelineCustomSettings: assert fs_store.basedir == str(tmp_path) +@pytest.mark.requires_reactor # TODO: needs a reactor for S3FilesStore @pytest.mark.requires_botocore class TestS3FilesStore: @inline_callbacks_test @@ -714,7 +715,7 @@ class TestGCSFilesStore: store.bucket.get_blob.assert_called_with(expected_blob_path) -@pytest.mark.requires_reactor # needs a reactor for FTPFilesStore +@pytest.mark.requires_reactor # TODO: needs a reactor for FTPFilesStore class TestFTPFileStore: @inline_callbacks_test def test_persist(self): @@ -752,13 +753,13 @@ class ItemWithFiles(Item): files = Field() -def _create_item_with_files(*files): +def _create_item_with_files(*files: str) -> ItemWithFiles: item = ItemWithFiles() item["file_urls"] = files return item -def _prepare_request_object(item_url, flags=None): +def _prepare_request_object(item_url: str, flags: list[str] | None = None) -> Request: return Request( item_url, meta={"response": Response(item_url, status=200, body=b"data", flags=flags)}, diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index 2d5836ad6..e65b5bc32 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -248,10 +248,8 @@ class TestPipeline: class TestCustomPipelineManager: - # needs a reactor or an event loop for is_asyncio_available() - # (for ItemPipelineManager.process_item()) - @pytest.mark.requires_reactor - def test_deprecated_process_item_spider_arg(self) -> None: + @coroutine_test + async def test_deprecated_process_item_spider_arg(self) -> None: class CustomPipelineManager(ItemPipelineManager): def process_item(self, item: Any, spider: Spider) -> Deferred[Any]: # pylint: disable=useless-parent-delegation return super().process_item(item, spider) diff --git a/tests/test_pqueues.py b/tests/test_pqueues.py index 2d42ba791..350b3e10d 100644 --- a/tests/test_pqueues.py +++ b/tests/test_pqueues.py @@ -1,4 +1,5 @@ import tempfile +from unittest.mock import Mock import pytest import queuelib @@ -9,7 +10,7 @@ from scrapy.spiders import Spider from scrapy.squeues import FifoMemoryQueue from scrapy.utils.misc import build_from_crawler, load_object from scrapy.utils.test import get_crawler -from tests.test_scheduler import MockDownloader, MockEngine +from tests.test_scheduler import MockDownloader class TestPriorityQueue: @@ -98,7 +99,7 @@ class TestPriorityQueue: class TestDownloaderAwarePriorityQueue: def setup_method(self): crawler = get_crawler(Spider) - crawler.engine = MockEngine(downloader=MockDownloader()) + crawler.engine = Mock(downloader=MockDownloader()) self.queue = DownloaderAwarePriorityQueue.from_crawler( crawler=crawler, downstream_queue_cls=FifoMemoryQueue, diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 0d2d8be93..7fec479f3 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -1,11 +1,11 @@ from __future__ import annotations -import shutil -import tempfile import warnings from abc import ABC, abstractmethod from collections import deque -from typing import Any, NamedTuple +from contextlib import AbstractAsyncContextManager, asynccontextmanager +from typing import TYPE_CHECKING, Any, NamedTuple +from unittest.mock import Mock import pytest @@ -14,12 +14,16 @@ from scrapy.core.scheduler import BaseScheduler, Scheduler from scrapy.crawler import Crawler from scrapy.http import Request from scrapy.spiders import Spider -from scrapy.utils.defer import _schedule_coro +from scrapy.utils.defer import ensure_awaitable from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.misc import load_object from scrapy.utils.test import get_crawler from tests.mockserver.http import MockServer -from tests.utils.decorators import inline_callbacks_test +from tests.utils.decorators import coroutine_test, inline_callbacks_test + +if TYPE_CHECKING: + from collections.abc import AsyncGenerator + from pathlib import Path class MemoryScheduler(BaseScheduler): @@ -54,78 +58,62 @@ class MemoryScheduler(BaseScheduler): self.paused = False -class MockEngine(NamedTuple): - downloader: MockDownloader - - class MockSlot(NamedTuple): active: list[Any] class MockDownloader: - def __init__(self): - self.slots = {} + def __init__(self) -> None: + self.slots: dict[str, MockSlot] = {} - def get_slot_key(self, request): + def get_slot_key(self, request: Request) -> str: if Downloader.DOWNLOAD_SLOT in request.meta: return request.meta[Downloader.DOWNLOAD_SLOT] return urlparse_cached(request).hostname or "" - def increment(self, slot_key): + def increment(self, slot_key: str) -> None: slot = self.slots.setdefault(slot_key, MockSlot(active=[])) slot.active.append(1) - def decrement(self, slot_key): - slot = self.slots.get(slot_key) + def decrement(self, slot_key: str) -> None: + slot = self.slots[slot_key] slot.active.pop() - def close(self): + def close(self) -> None: pass class MockCrawler(Crawler): - def __init__(self, priority_queue_cls, jobdir): + def __init__(self, priority_queue_cls: str, jobdir: Path | None): settings = { "SCHEDULER_DEBUG": False, "SCHEDULER_DISK_QUEUE": "scrapy.squeues.PickleLifoDiskQueue", "SCHEDULER_MEMORY_QUEUE": "scrapy.squeues.LifoMemoryQueue", "SCHEDULER_PRIORITY_QUEUE": priority_queue_cls, - "JOBDIR": jobdir, + "JOBDIR": str(jobdir) if jobdir is not None else None, "DUPEFILTER_CLASS": "scrapy.dupefilters.BaseDupeFilter", } super().__init__(Spider, settings) - self.engine = MockEngine(downloader=MockDownloader()) + self.engine = Mock(downloader=MockDownloader()) self.stats = load_object(self.settings["STATS_CLASS"])(self) -# needs a reactor or an event loop for is_asyncio_available() -# (for _schedule_coro()) -@pytest.mark.requires_reactor -class SchedulerHandler(ABC): - jobdir = None - - @property - @abstractmethod - def priority_queue_cls(self) -> str: - raise NotImplementedError - - def create_scheduler(self): - self.mock_crawler = MockCrawler(self.priority_queue_cls, self.jobdir) - self.scheduler = Scheduler.from_crawler(self.mock_crawler) - self.spider = Spider(name="spider") - self.scheduler.open(self.spider) - - def close_scheduler(self): - self.scheduler.close("finished") - _schedule_coro(self.mock_crawler.stop_async()) - self.mock_crawler.engine.downloader.close() - - def setup_method(self): - self.create_scheduler() - - def teardown_method(self): - self.close_scheduler() +@asynccontextmanager +async def create_scheduler( + priority_queue_cls: str, jobdir: Path | None +) -> AsyncGenerator[Scheduler]: + mock_crawler = MockCrawler(priority_queue_cls, jobdir) + scheduler = Scheduler.from_crawler(mock_crawler) + spider = Spider(name="spider") + await ensure_awaitable(scheduler.open(spider)) + try: + yield scheduler + finally: + await ensure_awaitable(scheduler.close("finished")) + await mock_crawler.stop_async() + assert mock_crawler.engine + mock_crawler.engine.downloader.close() _PRIORITIES = [ @@ -140,99 +128,118 @@ _PRIORITIES = [ _URLS = {"http://foo.com/a", "http://foo.com/b", "http://foo.com/c"} -class TestSchedulerInMemoryBase(SchedulerHandler): - def test_length(self): - assert not self.scheduler.has_pending_requests() - assert len(self.scheduler) == 0 +class TestSchedulerBase(ABC): + @property + @abstractmethod + def priority_queue_cls(self) -> str: + raise NotImplementedError - for url in _URLS: - self.scheduler.enqueue_request(Request(url)) + @pytest.fixture + def jobdir(self) -> Path | None: + return None - assert self.scheduler.has_pending_requests() - assert len(self.scheduler) == len(_URLS) + def create_scheduler( + self, jobdir: Path | None + ) -> AbstractAsyncContextManager[Scheduler]: + return create_scheduler(self.priority_queue_cls, jobdir) - def test_dequeue(self): - for url in _URLS: - self.scheduler.enqueue_request(Request(url)) + # TODO: unify test methods using "reopen" like in DownloaderAwareSchedulerTestMixin - urls = set() - while self.scheduler.has_pending_requests(): - urls.add(self.scheduler.next_request().url) + +class TestSchedulerInMemoryBase(TestSchedulerBase): + @coroutine_test + async def test_length(self, jobdir: Path | None) -> None: + async with self.create_scheduler(jobdir) as scheduler: + assert not scheduler.has_pending_requests() + assert len(scheduler) == 0 + + for url in _URLS: + scheduler.enqueue_request(Request(url)) + + assert scheduler.has_pending_requests() + assert len(scheduler) == len(_URLS) + + @coroutine_test + async def test_dequeue(self, jobdir: Path | None) -> None: + async with self.create_scheduler(jobdir) as scheduler: + for url in _URLS: + scheduler.enqueue_request(Request(url)) + + urls = set() + while scheduler.has_pending_requests(): + request = scheduler.next_request() + assert request is not None + urls.add(request.url) assert urls == _URLS - def test_dequeue_priorities(self): - for url, priority in _PRIORITIES: - self.scheduler.enqueue_request(Request(url, priority=priority)) + @coroutine_test + async def test_dequeue_priorities(self, jobdir: Path | None) -> None: + async with self.create_scheduler(jobdir) as scheduler: + for url, priority in _PRIORITIES: + scheduler.enqueue_request(Request(url, priority=priority)) - priorities = [] - while self.scheduler.has_pending_requests(): - priorities.append(self.scheduler.next_request().priority) + priorities = [] + while scheduler.has_pending_requests(): + request = scheduler.next_request() + assert request is not None + priorities.append(request.priority) assert priorities == sorted([x[1] for x in _PRIORITIES], key=lambda x: -x) -class TestSchedulerOnDiskBase(SchedulerHandler): - def setup_method(self): - self.jobdir = tempfile.mkdtemp() - self.create_scheduler() +class TestSchedulerOnDiskBase(TestSchedulerBase): + @pytest.fixture + def jobdir(self, tmp_path: Path) -> Path | None: + return tmp_path - def teardown_method(self): - self.close_scheduler() + @coroutine_test + async def test_length(self, jobdir: Path | None) -> None: + async with self.create_scheduler(jobdir) as scheduler: + assert not scheduler.has_pending_requests() + assert len(scheduler) == 0 + for url in _URLS: + scheduler.enqueue_request(Request(url)) - shutil.rmtree(self.jobdir) - self.jobdir = None + async with self.create_scheduler(jobdir) as scheduler: + assert scheduler.has_pending_requests() + assert len(scheduler) == len(_URLS) - def test_length(self): - assert not self.scheduler.has_pending_requests() - assert len(self.scheduler) == 0 - - for url in _URLS: - self.scheduler.enqueue_request(Request(url)) - - self.close_scheduler() - self.create_scheduler() - - assert self.scheduler.has_pending_requests() - assert len(self.scheduler) == len(_URLS) - - def test_dequeue(self): - for url in _URLS: - self.scheduler.enqueue_request(Request(url)) - - self.close_scheduler() - self.create_scheduler() + @coroutine_test + async def test_dequeue(self, jobdir: Path | None) -> None: + async with self.create_scheduler(jobdir) as scheduler: + for url in _URLS: + scheduler.enqueue_request(Request(url)) urls = set() - while self.scheduler.has_pending_requests(): - urls.add(self.scheduler.next_request().url) - + async with self.create_scheduler(jobdir) as scheduler: + while scheduler.has_pending_requests(): + request = scheduler.next_request() + assert request is not None + urls.add(request.url) assert urls == _URLS - def test_dequeue_priorities(self): - for url, priority in _PRIORITIES: - self.scheduler.enqueue_request(Request(url, priority=priority)) - - self.close_scheduler() - self.create_scheduler() + @coroutine_test + async def test_dequeue_priorities(self, jobdir: Path | None) -> None: + async with self.create_scheduler(jobdir) as scheduler: + for url, priority in _PRIORITIES: + scheduler.enqueue_request(Request(url, priority=priority)) priorities = [] - while self.scheduler.has_pending_requests(): - priorities.append(self.scheduler.next_request().priority) - + async with self.create_scheduler(jobdir) as scheduler: + while scheduler.has_pending_requests(): + request = scheduler.next_request() + assert request is not None + priorities.append(request.priority) assert priorities == sorted([x[1] for x in _PRIORITIES], key=lambda x: -x) class TestSchedulerInMemory(TestSchedulerInMemoryBase): - @property - def priority_queue_cls(self) -> str: - return "scrapy.pqueues.ScrapyPriorityQueue" + priority_queue_cls = "scrapy.pqueues.ScrapyPriorityQueue" class TestSchedulerOnDisk(TestSchedulerOnDiskBase): - @property - def priority_queue_cls(self) -> str: - return "scrapy.pqueues.ScrapyPriorityQueue" + priority_queue_cls = "scrapy.pqueues.ScrapyPriorityQueue" _URLS_WITH_SLOTS = [ @@ -246,39 +253,25 @@ _URLS_WITH_SLOTS = [ class TestMigration: - # needs a reactor or an event loop for is_asyncio_available() - # (for _schedule_coro()) - @pytest.mark.requires_reactor - def test_migration(self, tmpdir): - class PrevSchedulerHandler(SchedulerHandler): - jobdir = tmpdir + @coroutine_test + async def test_migration(self, tmp_path: Path) -> None: + async with create_scheduler( + "scrapy.pqueues.ScrapyPriorityQueue", tmp_path + ) as prev_scheduler: + for url in _URLS: + prev_scheduler.enqueue_request(Request(url)) - @property - def priority_queue_cls(self) -> str: - return "scrapy.pqueues.ScrapyPriorityQueue" - - class NextSchedulerHandler(SchedulerHandler): - jobdir = tmpdir - - @property - def priority_queue_cls(self) -> str: - return "scrapy.pqueues.DownloaderAwarePriorityQueue" - - prev_scheduler_handler = PrevSchedulerHandler() - prev_scheduler_handler.create_scheduler() - for url in _URLS: - prev_scheduler_handler.scheduler.enqueue_request(Request(url)) - prev_scheduler_handler.close_scheduler() - - next_scheduler_handler = NextSchedulerHandler() with pytest.raises( ValueError, match="DownloaderAwarePriorityQueue accepts ``slot_startprios`` as a dict", ): - next_scheduler_handler.create_scheduler() + async with create_scheduler( + "scrapy.pqueues.DownloaderAwarePriorityQueue", tmp_path + ): + pass -def _is_scheduling_fair(enqueued_slots, dequeued_slots): +def _is_scheduling_fair(enqueued_slots: list[str], dequeued_slots: list[str]) -> bool: """ We enqueued same number of requests for every slot. Assert correct order, e.g. @@ -303,39 +296,49 @@ def _is_scheduling_fair(enqueued_slots, dequeued_slots): return True -class DownloaderAwareSchedulerTestMixin: +class DownloaderAwareSchedulerTestMixin(TestSchedulerBase): reopen = False + priority_queue_cls = "scrapy.pqueues.DownloaderAwarePriorityQueue" - @property - def priority_queue_cls(self) -> str: - return "scrapy.pqueues.DownloaderAwarePriorityQueue" + @coroutine_test + async def test_logic(self, jobdir: Path | None) -> None: + def _setup(scheduler: Scheduler) -> None: + for url, slot in _URLS_WITH_SLOTS: + request = Request(url) + request.meta[Downloader.DOWNLOAD_SLOT] = slot + scheduler.enqueue_request(request) - def test_logic(self): - for url, slot in _URLS_WITH_SLOTS: - request = Request(url) - request.meta[Downloader.DOWNLOAD_SLOT] = slot - self.scheduler.enqueue_request(request) + def _assert(scheduler: Scheduler) -> None: + dequeued_slots: list[str] = [] + requests: list[Request] = [] + assert scheduler.crawler + assert scheduler.crawler.engine + downloader = scheduler.crawler.engine.downloader + assert isinstance(downloader, MockDownloader) + while scheduler.has_pending_requests(): + request = scheduler.next_request() + assert request is not None + slot = downloader.get_slot_key(request) + dequeued_slots.append(slot) + downloader.increment(slot) + requests.append(request) + + for request in requests: + slot = downloader.get_slot_key(request) + downloader.decrement(slot) + + assert _is_scheduling_fair([s for u, s in _URLS_WITH_SLOTS], dequeued_slots) + assert sum(len(s.active) for s in downloader.slots.values()) == 0 if self.reopen: - self.close_scheduler() - self.create_scheduler() - - dequeued_slots = [] - requests = [] - downloader = self.mock_crawler.engine.downloader - while self.scheduler.has_pending_requests(): - request = self.scheduler.next_request() - slot = downloader.get_slot_key(request) - dequeued_slots.append(slot) - downloader.increment(slot) - requests.append(request) - - for request in requests: - slot = downloader.get_slot_key(request) - downloader.decrement(slot) - - assert _is_scheduling_fair([s for u, s in _URLS_WITH_SLOTS], dequeued_slots) - assert sum(len(s.active) for s in downloader.slots.values()) == 0 + async with self.create_scheduler(jobdir) as scheduler: + _setup(scheduler) + async with self.create_scheduler(jobdir) as scheduler: + _assert(scheduler) + else: + async with self.create_scheduler(jobdir) as scheduler: + _setup(scheduler) + _assert(scheduler) class TestSchedulerWithDownloaderAwareInMemory( diff --git a/tests/test_spidermiddleware_process_start.py b/tests/test_spidermiddleware_process_start.py index 3fbe9c94e..ddece52c4 100644 --- a/tests/test_spidermiddleware_process_start.py +++ b/tests/test_spidermiddleware_process_start.py @@ -346,12 +346,12 @@ class TestMain: [NoOpSpiderMiddleware, AsyncioSleepSpiderMiddleware, NoOpSpiderMiddleware] ) - @pytest.mark.requires_reactor + @pytest.mark.requires_reactor # needs a reactor for twisted_sleep() @coroutine_test async def test_twisted_sleep_single(self): await self._test_sleep([TwistedSleepSpiderMiddleware]) - @pytest.mark.requires_reactor + @pytest.mark.requires_reactor # needs a reactor for twisted_sleep() @coroutine_test async def test_twisted_sleep_multiple(self): await self._test_sleep( diff --git a/tests/test_utils_asyncio.py b/tests/test_utils_asyncio.py index 5489fd948..a871a282e 100644 --- a/tests/test_utils_asyncio.py +++ b/tests/test_utils_asyncio.py @@ -21,10 +21,10 @@ if TYPE_CHECKING: class TestAsyncio: - @pytest.mark.requires_reactor # needs a reactor or an event loop for is_asyncio_available() - def test_is_asyncio_available(self, reactor_pytest: str) -> None: + @coroutine_test + async def test_is_asyncio_available(self, reactor_pytest: str) -> None: # the result should depend only on the pytest --reactor argument - assert is_asyncio_available() == (reactor_pytest == "asyncio") + assert is_asyncio_available() == (reactor_pytest != "default") @pytest.mark.only_asyncio @@ -104,10 +104,10 @@ class TestParallelAsyncio: assert max_parallel_count[0] <= self.CONCURRENT_ITEMS -@pytest.mark.requires_reactor # needs a running event loop for AsyncioLoopingCall.start() @pytest.mark.only_asyncio class TestAsyncioLoopingCall: - def test_looping_call(self): + @coroutine_test + async def test_looping_call(self): func = mock.MagicMock() looping_call = AsyncioLoopingCall(func) looping_call.start(1, now=False) @@ -116,21 +116,24 @@ class TestAsyncioLoopingCall: assert not looping_call.running assert not func.called - def test_looping_call_now(self): + @coroutine_test + async def test_looping_call_now(self): func = mock.MagicMock() looping_call = AsyncioLoopingCall(func) looping_call.start(1) looping_call.stop() assert func.called - def test_looping_call_already_running(self): + @coroutine_test + async def test_looping_call_already_running(self): looping_call = AsyncioLoopingCall(lambda: None) looping_call.start(1) with pytest.raises(RuntimeError): looping_call.start(1) looping_call.stop() - def test_looping_call_interval(self): + @coroutine_test + async def test_looping_call_interval(self): looping_call = AsyncioLoopingCall(lambda: None) with pytest.raises(ValueError, match="Interval must be greater than 0"): looping_call.start(0) @@ -138,7 +141,8 @@ class TestAsyncioLoopingCall: looping_call.start(-1) assert not looping_call.running - def test_looping_call_bad_function(self): + @coroutine_test + async def test_looping_call_bad_function(self): looping_call = AsyncioLoopingCall(Deferred) with pytest.raises(TypeError): looping_call.start(0.1) diff --git a/tests/test_utils_defer.py b/tests/test_utils_defer.py index 1b062d28d..978c24f5a 100644 --- a/tests/test_utils_defer.py +++ b/tests/test_utils_defer.py @@ -19,13 +19,13 @@ from scrapy.utils.defer import ( mustbe_deferred, parallel_async, ) -from tests.utils.decorators import inline_callbacks_test +from tests.utils.decorators import coroutine_test, inline_callbacks_test if TYPE_CHECKING: from collections.abc import AsyncGenerator, Awaitable, Callable, Generator -@pytest.mark.requires_reactor +@pytest.mark.requires_reactor # mustbe_deferred() requires a reactor @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") class TestMustbeDeferred: @inline_callbacks_test @@ -89,9 +89,8 @@ class TestIterErrback: assert isinstance(errors[0].value, ZeroDivisionError) -@pytest.mark.requires_reactor class TestAiterErrback: - @deferred_f_from_coro_f + @coroutine_test async def test_aiter_errback_good(self): async def itergood() -> AsyncGenerator[int, None]: for x in range(10): @@ -102,7 +101,7 @@ class TestAiterErrback: assert out == list(range(10)) assert not errors - @deferred_f_from_coro_f + @coroutine_test async def test_iter_errback_bad(self): async def iterbad() -> AsyncGenerator[int, None]: for x in range(10): @@ -117,23 +116,18 @@ class TestAiterErrback: assert isinstance(errors[0].value, ZeroDivisionError) -@pytest.mark.requires_reactor class TestAsyncDefTestsuite: - @deferred_f_from_coro_f - async def test_deferred_f_from_coro_f(self): + @coroutine_test + async def test_coroutine_test(self): pass - @deferred_f_from_coro_f - async def test_deferred_f_from_coro_f_generator(self): - yield - @pytest.mark.xfail(reason="Checks that the test is actually executed", strict=True) - @deferred_f_from_coro_f - async def test_deferred_f_from_coro_f_xfail(self): + @coroutine_test + async def test_coroutine_test_xfail(self): raise RuntimeError("This is expected to be raised") -@pytest.mark.requires_reactor +@pytest.mark.requires_reactor # parallel_async() requires a reactor class TestParallelAsync: """This tests _AsyncCooperatorAdapter by testing parallel_async which is its only usage. @@ -327,9 +321,8 @@ class TestDeferredFFromCoroF: @pytest.mark.only_asyncio -@pytest.mark.requires_reactor class TestDeferredToFuture: - @deferred_f_from_coro_f + @coroutine_test async def test_deferred(self): d = Deferred() result = deferred_to_future(d) @@ -338,7 +331,7 @@ class TestDeferredToFuture: future_result = await result assert future_result == 42 - @deferred_f_from_coro_f + @coroutine_test async def test_wrapped_coroutine(self): async def c_f() -> int: return 42 @@ -349,7 +342,7 @@ class TestDeferredToFuture: future_result = await result assert future_result == 42 - @deferred_f_from_coro_f + @coroutine_test async def test_wrapped_coroutine_asyncio(self): async def c_f() -> int: await asyncio.sleep(0.01) @@ -363,11 +356,8 @@ class TestDeferredToFuture: @pytest.mark.only_asyncio -# needs a reactor or an event loop for is_asyncio_available() -# (for maybe_deferred_to_future()) -@pytest.mark.requires_reactor class TestMaybeDeferredToFutureAsyncio: - @deferred_f_from_coro_f + @coroutine_test async def test_deferred(self): d = Deferred() result = maybe_deferred_to_future(d) @@ -376,7 +366,7 @@ class TestMaybeDeferredToFutureAsyncio: future_result = await result assert future_result == 42 - @deferred_f_from_coro_f + @coroutine_test async def test_wrapped_coroutine(self): async def c_f() -> int: return 42 @@ -387,7 +377,7 @@ class TestMaybeDeferredToFutureAsyncio: future_result = await result assert future_result == 42 - @deferred_f_from_coro_f + @coroutine_test async def test_wrapped_coroutine_asyncio(self): async def c_f() -> int: await asyncio.sleep(0.01) @@ -401,11 +391,9 @@ class TestMaybeDeferredToFutureAsyncio: @pytest.mark.only_not_asyncio -# needs a reactor or an event loop for is_asyncio_available() -# (for maybe_deferred_to_future()) -@pytest.mark.requires_reactor class TestMaybeDeferredToFutureNotAsyncio: - def test_deferred(self): + @coroutine_test + async def test_deferred(self): d = Deferred() result = maybe_deferred_to_future(d) assert isinstance(result, Deferred) diff --git a/tests/test_utils_reactor.py b/tests/test_utils_reactor.py index 3255d5940..d11fbc582 100644 --- a/tests/test_utils_reactor.py +++ b/tests/test_utils_reactor.py @@ -13,12 +13,12 @@ from tests.utils.decorators import coroutine_test class TestAsyncio: - @pytest.mark.requires_reactor + @pytest.mark.requires_reactor # needs a reactor def test_is_asyncio_reactor_installed(self, reactor_pytest: str) -> None: # the result should depend only on the pytest --reactor argument assert is_asyncio_reactor_installed() == (reactor_pytest == "asyncio") - @pytest.mark.requires_reactor + @pytest.mark.requires_reactor # installs a reactor def test_install_asyncio_reactor(self): from twisted.internet import reactor as original_reactor @@ -29,7 +29,7 @@ class TestAsyncio: assert original_reactor == reactor - @pytest.mark.requires_reactor + @pytest.mark.requires_reactor # installs a reactor @pytest.mark.only_asyncio @coroutine_test async def test_set_asyncio_event_loop(self): diff --git a/tests/test_webclient.py b/tests/test_webclient.py index 7834e3b31..b14d15be4 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -30,6 +30,7 @@ from tests.mockserver.http_resources import ( from tests.mockserver.utils import ssl_context_factory from tests.test_core_downloader import TestContextFactoryBase +# these tests are related to the Twisted HTTP code pytestmark = pytest.mark.requires_reactor diff --git a/tests/test_zz_resources.py b/tests/test_zz_resources.py index e6ff46f5c..1faa2487a 100644 --- a/tests/test_zz_resources.py +++ b/tests/test_zz_resources.py @@ -9,6 +9,7 @@ import pytest from scrapy.utils.log import LogCounterHandler from scrapy.utils.reactor import is_asyncio_reactor_installed, is_reactor_installed +from tests.utils.decorators import coroutine_test def test_counter_handler() -> None: @@ -31,11 +32,20 @@ def test_stderr_log_handler() -> None: assert c == 0 -@pytest.mark.requires_reactor # needs a running event loop for asyncio.all_tasks() @pytest.mark.only_asyncio -def test_pending_asyncio_tasks() -> None: +@coroutine_test +async def test_pending_asyncio_tasks() -> None: """Test that there are no pending asyncio tasks.""" - assert not asyncio.all_tasks() + # note that pytest-asyncio uses separate loops per function so this isn't as useful there + tasks = [] + for t in asyncio.all_tasks(): + coro = t.get_coro() + if ( + coro is not None + and getattr(coro, "__name__", None) != "test_pending_asyncio_tasks" + ): + tasks.append(t) + assert not tasks def test_installed_reactor(reactor_pytest: str) -> None: diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py index a81f2d1a2..5b7848d54 100644 --- a/tests/utils/__init__.py +++ b/tests/utils/__init__.py @@ -1,17 +1,28 @@ +import asyncio import os from pathlib import Path from twisted.internet.defer import Deferred +from scrapy.utils.asyncio import is_asyncio_available +from scrapy.utils.defer import maybe_deferred_to_future -def twisted_sleep(seconds): + +def twisted_sleep(seconds: float): from twisted.internet import reactor - d = Deferred() + d: Deferred[None] = Deferred() reactor.callLater(seconds, d.callback, None) return d +async def async_sleep(seconds: float) -> None: + if is_asyncio_available(): + await asyncio.sleep(seconds) + else: + await maybe_deferred_to_future(twisted_sleep(seconds)) + + def get_script_run_env() -> dict[str, str]: """Return a OS environment dict suitable to run scripts shipped with tests.""" diff --git a/tests/utils/decorators.py b/tests/utils/decorators.py index dd9043ab1..4750158e5 100644 --- a/tests/utils/decorators.py +++ b/tests/utils/decorators.py @@ -55,6 +55,11 @@ def coroutine_test( * with ``pytest-twisted`` this converts a coroutine into a :class:`twisted.internet.defer.Deferred` * with ``pytest-asyncio`` this is a no-op + + In addition to handling asynchronous test functions this can also be used + to mark "synchronous" test functions (they still need to be made + ``async def``) that call code that needs a reactor or a running event loop, + so that ``pytest-asyncio`` starts a loop for them too. """ if not is_reactor_installed(): diff --git a/tox.ini b/tox.ini index 1bebaf2aa..ca304330c 100644 --- a/tox.ini +++ b/tox.ini @@ -11,6 +11,7 @@ minversion = 1.7.0 deps = attrs coverage >= 7.10.6 + httpx pexpect >= 4.8.0 pyftpdlib >= 2.0.1 pygments @@ -107,6 +108,7 @@ deps = Twisted==21.7.0 cryptography==37.0.0 cssselect==0.9.1 + httpx==0.26.0 itemadapter==0.1.0 lxml==4.6.4 parsel==1.5.0 @@ -170,14 +172,6 @@ commands = {[pinned]commands} commands = {[testenv]commands} --reactor=default -[testenv:no-reactor] -deps = - {[testenv]deps} - httpx - pytest-asyncio -commands = - {[testenv]commands} -p no:twisted --reactor=none - [testenv:default-reactor-pinned] basepython = {[pinned]basepython} deps = {[testenv:pinned]deps} @@ -185,11 +179,24 @@ commands = {[pinned]commands} --reactor=default setenv = {[pinned]setenv} +[testenv:no-reactor] +deps = + {[testenv]deps} + pytest-asyncio +commands = + {[testenv]commands} -p no:twisted --reactor=none + +[testenv:no-reactor-extra-deps] +deps = + {[testenv:extra-deps]deps} + pytest-asyncio +commands = + {[testenv]commands} -p no:twisted --reactor=none + [testenv:no-reactor-pinned] basepython = {[pinned]basepython} deps = {[testenv:pinned]deps} - httpx==0.26.0 pytest-asyncio commands = {[pinned]commands} -p no:twisted --reactor=none setenv =