Reactorless tests 2: enable pytest-asyncio. (#7233)

* Move no-reactor to pytest-asyncio.

* Unify the style.

* Skip a test that installs a reactor in no-reactor.

* Fix pinned env deps.

* Strict pytest-asyncio mode.

conftest.py

@@ -88,7 +88,10 @@ def pytest_runtest_setup(item):
     # Skip tests based on reactor markers
     reactor = item.config.getoption("--reactor")
 
-    if item.get_closest_marker("requires_reactor") and reactor == "none":
+    if (
+        item.get_closest_marker("requires_reactor")
+        or item.get_closest_marker("requires_http_handler")
+    ) and reactor == "none":
         pytest.skip('This test is only run when the --reactor value is not "none"')
 
     if item.get_closest_marker("only_asyncio") and reactor not in {"asyncio", "none"}:

pyproject.toml

@@ -230,13 +230,14 @@ markers = [
     "only_asyncio: marks tests that require the asyncio loop to be used",
     "only_not_asyncio: marks tests that require the asyncio loop to not be used",
     "requires_reactor: marks tests that require a reactor",
+    "requires_http_handler: marks tests that require a HTTP handler",
     "requires_uvloop: marks tests as only enabled when uvloop is known to be working",
     "requires_botocore: marks tests that need botocore (but not boto3)",
     "requires_boto3: marks tests that need botocore and boto3",
     "requires_mitmproxy: marks tests that need mitmproxy",
 ]
 filterwarnings = [
-    "ignore::DeprecationWarning:twisted.web.static"
+    "ignore::DeprecationWarning:twisted.web.static",
 ]
 
 [tool.ruff.lint]

tests/test_addons.py

@@ -199,7 +199,12 @@ class TestAddonManager:
         settings = Settings()
         settings.setdict(get_reactor_settings())
         settings.set("KEY", "default", priority="default")
-        runner = CrawlerRunner(settings)
+        runner_cls = (
+            CrawlerRunner
+            if settings.getbool("TWISTED_ENABLED", True)
+            else AsyncCrawlerRunner
+        )
+        runner = runner_cls(settings)
         crawler = runner.create_crawler(MySpider)
         assert crawler.settings.get("KEY") == "default"
         yield crawler.crawl()

tests/test_closespider.py

@@ -1,3 +1,5 @@
+import pytest
+
 from scrapy.utils.test import get_crawler
 from tests.mockserver.http import MockServer
 from tests.spiders import (
@@ -10,6 +12,7 @@ from tests.spiders import (
 from tests.utils.decorators import inlineCallbacks
 
 
+@pytest.mark.requires_http_handler
 class TestCloseSpider:
     @classmethod
     def setup_class(cls):

tests/test_contracts.py

@@ -501,6 +501,7 @@ class TestContractsManager:
         assert not self.results.failures
         assert self.results.errors
 
+    @pytest.mark.requires_http_handler
     @inlineCallbacks
     def test_same_url(self):
         class TestSameUrlSpider(Spider):

tests/test_core_downloader.py

@@ -38,6 +38,7 @@ class TestSlot:
         assert repr(slot) == "Slot(concurrency=8, delay=0.10, randomize_delay=True)"
 
 
+@pytest.mark.requires_reactor
 class TestContextFactoryBase:
     context_factory = None
 

tests/test_core_scraper.py

@@ -2,16 +2,17 @@ from __future__ import annotations
 
 from typing import TYPE_CHECKING
 
+import pytest
+
 from scrapy.utils.test import get_crawler
 from tests.spiders import SimpleSpider
 from tests.utils.decorators import deferred_f_from_coro_f
 
 if TYPE_CHECKING:
-    import pytest
-
     from tests.mockserver.http import MockServer
 
 
+@pytest.mark.requires_http_handler
 @deferred_f_from_coro_f
 async def test_scraper_exception(
     mockserver: MockServer,

tests/test_crawl.py

@@ -61,6 +61,7 @@ if TYPE_CHECKING:
     from scrapy.statscollectors import StatsCollector
 
 
+@pytest.mark.requires_http_handler  # easier than marking many individual tests
 class TestCrawl:
     mockserver: MockServer
 
@@ -428,6 +429,7 @@ with multiples lines
         assert "NotSupported: Unsupported URL scheme 'foo'" in caplog.text
 
 
+@pytest.mark.requires_http_handler
 class TestCrawlSpider:
     mockserver: MockServer
 

tests/test_crawler.py

@@ -646,12 +646,16 @@ class TestCrawlerProcess(TestBaseCrawler):
 
 @pytest.mark.only_asyncio
 class TestAsyncCrawlerProcess(TestBaseCrawler):
-    def test_crawler_process_accepts_dict(self):
-        runner = AsyncCrawlerProcess({"foo": "bar"}, install_root_handler=False)
+    def test_crawler_process_accepts_dict(self, reactor_pytest: str) -> None:
+        runner = AsyncCrawlerProcess(
+            {"foo": "bar", "TWISTED_ENABLED": reactor_pytest != "none"},
+            install_root_handler=False,
+        )
         assert runner.settings["foo"] == "bar"
         self.assertOptionIsDefault(runner.settings, "RETRY_ENABLED")
 
-    def test_crawler_process_accepts_None(self):
+    @pytest.mark.requires_reactor  # can't pass TWISTED_ENABLED=False
+    def test_crawler_process_accepts_None(self) -> None:
         runner = AsyncCrawlerProcess(install_root_handler=False)
         self.assertOptionIsDefault(runner.settings, "RETRY_ENABLED")
 
@@ -672,6 +676,7 @@ class NoRequestsSpider(scrapy.Spider):
         yield
 
 
+@pytest.mark.requires_reactor
 class TestCrawlerRunnerHasSpider:
     @staticmethod
     def _runner():

tests/test_downloader_handler_twisted_http2.py

@@ -200,7 +200,7 @@ class TestHttps2Proxy(H2DownloadHandlerMixin, TestHttpProxyBase):
     ) -> None:
         with pytest.raises(NotImplementedError):
             await maybe_deferred_to_future(
-                super().test_download_with_proxy_https_timeout(proxy_mockserver)
+                super().test_download_with_proxy_https_timeout(proxy_mockserver)  # type: ignore[arg-type]
             )
 
     @deferred_f_from_coro_f
@@ -209,5 +209,5 @@ class TestHttps2Proxy(H2DownloadHandlerMixin, TestHttpProxyBase):
     ) -> None:
         with pytest.raises(UnsupportedURLSchemeError):
             await maybe_deferred_to_future(
-                super().test_download_with_proxy_without_http_scheme(proxy_mockserver)
+                super().test_download_with_proxy_without_http_scheme(proxy_mockserver)  # type: ignore[arg-type]
             )

tests/test_downloaderslotssettings.py

@@ -67,6 +67,7 @@ class TestCrawl:
     def setup_method(self):
         self.runner = CrawlerRunner()
 
+    @pytest.mark.requires_http_handler
     @inlineCallbacks
     def test_delay(self):
         crawler = get_crawler(DownloaderSlotsSettingsTestSpider)
@@ -128,6 +129,7 @@ def test_get_slot_deprecated_spider_arg():
     assert slot1 == slot2
 
 
+@pytest.mark.requires_http_handler
 @pytest.mark.parametrize(
     "priority_queue_class",
     [

tests/test_engine.py

@@ -373,6 +373,7 @@ class TestEngineBase:
 
 
 class TestEngine(TestEngineBase):
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_crawler(self, mockserver: MockServer) -> None:
         for spider in (
@@ -390,6 +391,7 @@ class TestEngine(TestEngineBase):
             self._assert_signals_caught(run)
             self._assert_bytes_received(run)
 
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_crawler_dupefilter(self, mockserver: MockServer) -> None:
         run = CrawlerRun(DupeFilterSpider)
@@ -397,12 +399,14 @@ class TestEngine(TestEngineBase):
         self._assert_scheduled_requests(run, count=8)
         self._assert_dropped_requests(run)
 
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_crawler_itemerror(self, mockserver: MockServer) -> None:
         run = CrawlerRun(ItemZeroDivisionErrorSpider)
         await run.run(mockserver)
         self._assert_items_error(run)
 
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_crawler_change_close_reason_on_idle(
         self, mockserver: MockServer

tests/test_engine_loop.py

@@ -4,6 +4,7 @@ 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
@@ -14,8 +15,6 @@ from tests.test_scheduler import MemoryScheduler
 from tests.utils.decorators import deferred_f_from_coro_f
 
 if TYPE_CHECKING:
-    import pytest
-
     from scrapy.http import Response
 
 
@@ -28,6 +27,7 @@ async def sleep(seconds: float = 0.001) -> None:
 
 
 class TestMain:
+    @pytest.mark.requires_reactor  # TODO
     @deferred_f_from_coro_f
     async def test_sleep(self):
         """Neither asynchronous sleeps on Spider.start() nor the equivalent on
@@ -332,6 +332,7 @@ class TestRequestSendOrder:
     # Examples from the “Start requests” section of the documentation about
     # spiders.
 
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_lazy(self):
         start_nums = [1, 2, 4]

tests/test_engine_stop_download_bytes.py

@@ -2,6 +2,8 @@ from __future__ import annotations
 
 from typing import TYPE_CHECKING
 
+import pytest
+
 from scrapy.exceptions import StopDownload
 from tests.test_engine import (
     AttrsItemsSpider,
@@ -14,8 +16,6 @@ from tests.test_engine import (
 from tests.utils.decorators import deferred_f_from_coro_f
 
 if TYPE_CHECKING:
-    import pytest
-
     from tests.mockserver.http import MockServer
 
 
@@ -26,6 +26,7 @@ class BytesReceivedCrawlerRun(CrawlerRun):
 
 
 class TestBytesReceivedEngine(TestEngineBase):
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_crawler(
         self, mockserver: MockServer, caplog: pytest.LogCaptureFixture

tests/test_engine_stop_download_headers.py

@@ -2,6 +2,8 @@ from __future__ import annotations
 
 from typing import TYPE_CHECKING
 
+import pytest
+
 from scrapy.exceptions import StopDownload
 from tests.test_engine import (
     AttrsItemsSpider,
@@ -14,8 +16,6 @@ from tests.test_engine import (
 from tests.utils.decorators import deferred_f_from_coro_f
 
 if TYPE_CHECKING:
-    import pytest
-
     from tests.mockserver.http import MockServer
 
 
@@ -26,6 +26,7 @@ class HeadersReceivedCrawlerRun(CrawlerRun):
 
 
 class TestHeadersReceivedEngine(TestEngineBase):
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_crawler(
         self, mockserver: MockServer, caplog: pytest.LogCaptureFixture

tests/test_extension_telnet.py

@@ -6,6 +6,8 @@ from scrapy.extensions.telnet import TelnetConsole
 from scrapy.utils.test import get_crawler
 from tests.utils.decorators import inlineCallbacks
 
+pytestmark = pytest.mark.requires_reactor
+
 
 class TestTelnetExtension:
     def _get_console_and_portal(self, settings=None):

tests/test_feedexport.py

@@ -163,6 +163,7 @@ class TestFileFeedStorage:
         assert storage.path == path
 
 
+@pytest.mark.requires_reactor  # needs a reactor for BlockingFeedStorage
 class TestFTPFeedStorage:
     def get_test_spider(self, settings=None):
         class TestSpider(scrapy.Spider):
@@ -277,6 +278,7 @@ class TestBlockingFeedStorage:
 
 
 @pytest.mark.requires_boto3
+@pytest.mark.requires_reactor  # needs a reactor for BlockingFeedStorage
 class TestS3FeedStorage:
     def test_parse_credentials(self):
         aws_credentials = {
@@ -506,6 +508,7 @@ class TestS3FeedStorage:
         assert "S3 does not support appending to files" in str(log)
 
 
+@pytest.mark.requires_reactor  # needs a reactor for BlockingFeedStorage
 class TestGCSFeedStorage:
     def test_parse_settings(self):
         try:
@@ -846,6 +849,7 @@ class ExceptionJsonItemExporter(JsonItemExporter):
         raise RuntimeError("foo")
 
 
+@pytest.mark.requires_http_handler
 class TestFeedExport(TestFeedExportBase):
     async def run_and_export(
         self, spider_cls: type[Spider], settings: dict[str, Any]
@@ -1830,6 +1834,7 @@ class TestFeedExport(TestFeedExportBase):
         assert not Storage.file_was_closed
 
 
+@pytest.mark.requires_http_handler
 class TestFeedPostProcessedExports(TestFeedExportBase):
     items = [{"foo": "bar"}]
     expected = b"foo\r\nbar\r\n"
@@ -2348,6 +2353,7 @@ class TestFeedPostProcessedExports(TestFeedExportBase):
             assert result == expected
 
 
+@pytest.mark.requires_http_handler
 class TestBatchDeliveries(TestFeedExportBase):
     _file_mark = "_%(batch_time)s_#%(batch_id)02d_"
 

tests/test_logformatter.py

@@ -253,6 +253,7 @@ class DropSomeItemsPipeline:
         self.drop = True
 
 
+@pytest.mark.requires_http_handler
 class TestShowOrSkipMessages:
     @classmethod
     def setup_class(cls):

tests/test_pipeline_crawl.py

@@ -57,6 +57,7 @@ class RedirectedMediaDownloadSpider(MediaDownloadSpider):
         )
 
 
+@pytest.mark.requires_http_handler
 class TestFileDownloadCrawl:
     pipeline_class = "scrapy.pipelines.files.FilesPipeline"
     store_setting_key = "FILES_STORE"

tests/test_pipeline_files.py

@@ -691,6 +691,7 @@ class TestGCSFilesStore:
             store.bucket.get_blob.assert_called_with(expected_blob_path)
 
 
+@pytest.mark.requires_reactor  # needs a reactor for FTPFilesStore
 class TestFTPFileStore:
     @inlineCallbacks
     def test_persist(self):

tests/test_pipelines.py

@@ -127,6 +127,7 @@ class ItemSpider(Spider):
         return {"field": 42}
 
 
+@pytest.mark.requires_http_handler
 class TestPipeline:
     def _on_item_scraped(self, item):
         assert isinstance(item, dict)
@@ -265,6 +266,7 @@ class TestCustomPipelineManager:
         ):
             itemproc.process_item({}, crawler.spider)
 
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_integration_recommended(self, mockserver: MockServer) -> None:
         class CustomPipelineManager(ItemPipelineManager):
@@ -291,6 +293,7 @@ class TestCustomPipelineManager:
 
         assert len(items) == 1
 
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_integration_no_async_subclass(self, mockserver: MockServer) -> None:
         class CustomPipelineManager(ItemPipelineManager):
@@ -349,6 +352,7 @@ class TestCustomPipelineManager:
 
         assert len(items) == 1
 
+    @pytest.mark.requires_http_handler
     @deferred_f_from_coro_f
     async def test_integration_no_async_not_subclass(
         self, mockserver: MockServer

tests/test_proxy_connect.py

@@ -61,6 +61,7 @@ def _wrong_credentials(proxy_url):
     return urlunsplit(bad_auth_proxy)
 
 
+@pytest.mark.requires_http_handler
 @pytest.mark.requires_mitmproxy
 class TestProxyConnect:
     @classmethod

tests/test_request_attribute_binding.py

@@ -1,3 +1,4 @@
+import pytest
 from testfixtures import LogCapture
 
 from scrapy import Request, signals
@@ -62,6 +63,7 @@ class AlternativeCallbacksMiddleware:
         return response.replace(request=new_request)
 
 
+@pytest.mark.requires_http_handler
 class TestCrawl:
     @classmethod
     def setup_class(cls):

tests/test_request_cb_kwargs.py

@@ -1,3 +1,4 @@
+import pytest
 from testfixtures import LogCapture
 
 from scrapy.http import Request
@@ -148,6 +149,7 @@ class KeywordArgumentsSpider(MockServerSpider):
         self.crawler.stats.inc_value("boolean_checks", 1)
 
 
+@pytest.mark.requires_http_handler
 class TestCallbackKeywordArguments:
     @classmethod
     def setup_class(cls):

tests/test_scheduler.py

@@ -369,6 +369,7 @@ class TestIntegrationWithDownloaderAwareInMemory:
             },
         )
 
+    @pytest.mark.requires_http_handler
     @inlineCallbacks
     def test_integration_downloader_aware_priority_queue(self):
         with MockServer() as mockserver:

tests/test_scheduler_base.py

@@ -144,6 +144,7 @@ class TestSimpleScheduler(InterfaceCheckMixin):
         assert close_result == "close"
 
 
+@pytest.mark.requires_http_handler
 class TestMinimalSchedulerCrawl:
     scheduler_cls = MinimalScheduler
 

tests/test_signals.py

@@ -50,6 +50,7 @@ class TestMockServer:
         item = await get_from_asyncio_queue(item)
         self.items.append(item)
 
+    @pytest.mark.requires_http_handler
     @pytest.mark.only_asyncio
     @inlineCallbacks
     def test_simple_pipeline(self):

tests/test_spider_start.py

@@ -162,6 +162,7 @@ class TestMain:
 
         await self._test_start(start, [ITEM_A])
 
+    @pytest.mark.requires_reactor  # needs a reactor for twisted_sleep()
     @deferred_f_from_coro_f
     async def test_twisted_delayed(self):
         async def start(spider):

tests/test_spidermiddleware_httperror.py

@@ -191,6 +191,7 @@ class TestHttpErrorMiddlewareHandleAll:
         mw.process_spider_input(res402)
 
 
+@pytest.mark.requires_http_handler
 class TestHttpErrorMiddlewareIntegrational:
     @classmethod
     def setup_class(cls):

tests/test_spidermiddleware_output_chain.py

@@ -1,3 +1,4 @@
+import pytest
 from testfixtures import LogCapture
 
 from scrapy import Request, Spider
@@ -318,6 +319,7 @@ class NotGeneratorOutputChainSpider(Spider):
 
 
 # ================================================================================
+@pytest.mark.requires_http_handler
 class TestSpiderMiddleware:
     mockserver: MockServer
 

tests/test_spidermiddleware_process_start.py

@@ -341,10 +341,12 @@ class TestMain:
             [NoOpSpiderMiddleware, AsyncioSleepSpiderMiddleware, NoOpSpiderMiddleware]
         )
 
+    @pytest.mark.requires_reactor
     @deferred_f_from_coro_f
     async def test_twisted_sleep_single(self):
         await self._test_sleep([TwistedSleepSpiderMiddleware])
 
+    @pytest.mark.requires_reactor
     @deferred_f_from_coro_f
     async def test_twisted_sleep_multiple(self):
         await self._test_sleep(

tests/test_utils_defer.py

@@ -26,6 +26,7 @@ if TYPE_CHECKING:
     from collections.abc import AsyncGenerator, Awaitable, Callable, Generator
 
 
+@pytest.mark.requires_reactor
 @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning")
 class TestMustbeDeferred:
     @inlineCallbacks
@@ -153,6 +154,7 @@ class TestAsyncDefTestsuite:
         raise RuntimeError("This is expected to be raised")
 
 
+@pytest.mark.requires_reactor
 class TestParallelAsync:
     """This tests _AsyncCooperatorAdapter by testing parallel_async which is its only usage.
 

tests/test_utils_reactor.py

@@ -29,6 +29,7 @@ class TestAsyncio:
 
         assert original_reactor == reactor
 
+    @pytest.mark.requires_reactor
     @pytest.mark.only_asyncio
     @deferred_f_from_coro_f
     async def test_set_asyncio_event_loop(self):

tests/test_zz_resources.py

@@ -8,6 +8,7 @@ import logging
 import pytest
 
 from scrapy.utils.log import LogCounterHandler
+from scrapy.utils.reactor import is_asyncio_reactor_installed, is_reactor_installed
 
 
 def test_counter_handler() -> None:
@@ -35,3 +36,14 @@ def test_stderr_log_handler() -> None:
 def test_pending_asyncio_tasks() -> None:
     """Test that there are no pending asyncio tasks."""
     assert not asyncio.all_tasks()
+
+
+def test_installed_reactor(reactor_pytest: str) -> None:
+    """Test that the correct reactor is installed."""
+    match reactor_pytest:
+        case "asyncio":
+            assert is_asyncio_reactor_installed()
+        case "default":
+            assert not is_asyncio_reactor_installed()
+        case "none":
+            assert not is_reactor_installed()

tests/utils/decorators.py

@@ -7,7 +7,8 @@ import pytest
 from twisted.internet.defer import Deferred
 from twisted.internet.defer import inlineCallbacks as inlineCallbacks_orig
 
-from scrapy.utils.defer import deferred_from_coro
+from scrapy.utils.defer import deferred_from_coro, deferred_to_future
+from scrapy.utils.reactor import is_reactor_installed
 
 if TYPE_CHECKING:
     from collections.abc import Awaitable, Callable, Generator
@@ -18,28 +19,48 @@ _P = ParamSpec("_P")
 
 def inlineCallbacks(
     f: Callable[_P, Generator[Deferred[Any], Any, None]],
-) -> Callable[_P, Deferred[None]]:
-    """Like :func:`twisted.internet.defer.inlineCallbacks`, but marks the
-    decorated function with ``@pytest.mark.requires_reactor``."""
+) -> Callable[_P, Awaitable[None]]:
+    """Mark a test function written in a :func:`twisted.internet.defer.inlineCallbacks` style.
+
+    This calls :func:`twisted.internet.defer.inlineCallbacks` and then:
+
+    * with ``pytest-twisted`` this returns the resulting Deferred
+    * with ``pytest-asyncio`` this converts the resulting Deferred into a
+      coroutine
+    """
+
+    if not is_reactor_installed():
+
+        @pytest.mark.asyncio
+        @wraps(f)
+        async def wrapper_coro(*args: _P.args, **kwargs: _P.kwargs) -> None:
+            await deferred_to_future(inlineCallbacks_orig(f)(*args, **kwargs))
+
+        return wrapper_coro
 
-    @pytest.mark.requires_reactor
     @wraps(f)
     @inlineCallbacks_orig
-    def wrapper(
+    def wrapper_dfd(
         *args: _P.args, **kwargs: _P.kwargs
     ) -> Generator[Deferred[Any], Any, None]:
         return f(*args, **kwargs)
 
-    return wrapper
+    return wrapper_dfd
 
 
 def deferred_f_from_coro_f(
     coro_f: Callable[_P, Awaitable[None]],
-) -> Callable[_P, Deferred[None]]:
-    """Like :func:`scrapy.utils.defer.deferred_f_from_coro_f`, but marks the
-    decorated function with ``@pytest.mark.requires_reactor``."""
+) -> Callable[_P, Awaitable[None]]:
+    """Mark a test function that returns a coroutine.
+
+    * with ``pytest-twisted`` this converts a coroutine into a
+      :class:`twisted.internet.defer.Deferred`
+    * with ``pytest-asyncio`` this is a no-op
+    """
+
+    if not is_reactor_installed():
+        return pytest.mark.asyncio(coro_f)
 
-    @pytest.mark.requires_reactor
     @wraps(coro_f)
     def f(*coro_args: _P.args, **coro_kwargs: _P.kwargs) -> Deferred[None]:
         return deferred_from_coro(coro_f(*coro_args, **coro_kwargs))

tox.ini

@@ -162,6 +162,9 @@ commands =
     {[testenv]commands} --reactor=default
 
 [testenv:no-reactor]
+deps =
+    {[testenv]deps}
+    pytest-asyncio
 commands =
     {[testenv]commands} -p no:twisted --reactor=none
 
@@ -174,7 +177,9 @@ setenv =
 
 [testenv:no-reactor-pinned]
 basepython = {[pinned]basepython}
-deps = {[testenv:pinned]deps}
+deps =
+    {[testenv:pinned]deps}
+    pytest-asyncio
 commands = {[pinned]commands} -p no:twisted --reactor=none
 setenv =
     {[pinned]setenv}