From afccf1879463b208229ac6dad0f08f7edf754e68 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 10 Aug 2026 18:25:28 +0200 Subject: [PATCH] Improve test coverage --- scrapy/utils/benchserver.py | 4 ++- scrapy/utils/ossignal.py | 7 +--- scrapy/utils/reactor.py | 12 +++---- tests/test_linkextractors.py | 13 ++++++++ tests/test_squeues.py | 25 ++++++++++++++ tests/test_squeues_request.py | 58 ++++++++------------------------- tests/test_utils_benchserver.py | 36 ++++++++++++++++++++ tests/test_utils_console.py | 20 ++++++++++++ tests/test_utils_defer.py | 15 +++++++++ tests/test_utils_deprecate.py | 5 +++ 10 files changed, 138 insertions(+), 57 deletions(-) create mode 100644 tests/test_utils_benchserver.py diff --git a/scrapy/utils/benchserver.py b/scrapy/utils/benchserver.py index 403cd54a8..4055b6820 100644 --- a/scrapy/utils/benchserver.py +++ b/scrapy/utils/benchserver.py @@ -33,7 +33,9 @@ def _getarg( return type_(request.args[name][0]) if name in request.args else default -if __name__ == "__main__": +# The bench command kills this process, so coverage data is never written for +# the lines below. +if __name__ == "__main__": # pragma: no cover from twisted.internet import reactor root = Root() # type: ignore[no-untyped-call] diff --git a/scrapy/utils/ossignal.py b/scrapy/utils/ossignal.py index 4eda29e25..4ad8e5741 100644 --- a/scrapy/utils/ossignal.py +++ b/scrapy/utils/ossignal.py @@ -10,12 +10,7 @@ SignalHandlerT: TypeAlias = ( Callable[[int, FrameType | None], Any] | int | signal.Handlers | None ) -signal_names: dict[int, str] = {} -for signame in dir(signal): - if signame.startswith("SIG") and not signame.startswith("SIG_"): - signum = getattr(signal, signame) - if isinstance(signum, int): - signal_names[signum] = signame +signal_names: dict[int, str] = {member.value: member.name for member in signal.Signals} def install_shutdown_handlers( diff --git a/scrapy/utils/reactor.py b/scrapy/utils/reactor.py index 5499e3f48..217249ee5 100644 --- a/scrapy/utils/reactor.py +++ b/scrapy/utils/reactor.py @@ -27,7 +27,7 @@ _T = TypeVar("_T") _P = ParamSpec("_P") -def listen_tcp(portrange: list[int], host: str, factory: ServerFactory) -> Port: # type: ignore[return] # noqa: RET503 +def listen_tcp(portrange: list[int], host: str, factory: ServerFactory) -> Port: """Like reactor.listenTCP but tries different ports in a range.""" from twisted.internet import reactor @@ -37,14 +37,14 @@ def listen_tcp(portrange: list[int], host: str, factory: ServerFactory) -> Port: raise ValueError(f"invalid portrange: {portrange}") if not portrange: return reactor.listenTCP(0, factory, interface=host) # type: ignore[no-any-return] - if len(portrange) == 1: - return reactor.listenTCP(portrange[0], factory, interface=host) # type: ignore[no-any-return] - for x in range(portrange[0], portrange[1] + 1): + for x in range(portrange[0], portrange[-1]): try: return reactor.listenTCP(x, factory, interface=host) # type: ignore[no-any-return] except error.CannotListenError: - if x == portrange[1]: - raise + pass + # The last port of the range is tried outside the loop so that its + # CannotListenError propagates to the caller. + return reactor.listenTCP(portrange[-1], factory, interface=host) # type: ignore[no-any-return] class CallLaterOnce(Generic[_T]): diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 7b73a133f..bc8a4d636 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -908,6 +908,19 @@ class TestLxmlParserLinkExtractor: Link(url="http://example.com/page.html", text="Link", nofollow=False), ] + def test_deduplicates_by_canonical_url(self): + # With canonicalized=False, the default, URLs are canonicalized to + # decide whether two extracted links are the same one. + html = ( + b'1' + b'2' + ) + response = HtmlResponse("http://example.com/", body=html) + lx = LxmlParserLinkExtractor(unique=True) + assert lx.extract_links(response) == [ + Link(url="http://example.com/page.html?b=2&a=1", text="1", nofollow=False), + ] + def test_strip_false(self): # With strip=False, trailing whitespace on a relative href survives urljoin # and is visible to process_value (safe_url_string cleans it up afterward). diff --git a/tests/test_squeues.py b/tests/test_squeues.py index 8544602af..6bbc44829 100644 --- a/tests/test_squeues.py +++ b/tests/test_squeues.py @@ -1,5 +1,6 @@ import pickle import sys +from typing import Any, cast import pytest from queuelib.tests import test_queue as t @@ -13,6 +14,8 @@ from scrapy.squeues import ( _MarshalLifoSerializationDiskQueue, _PickleFifoSerializationDiskQueue, _PickleLifoSerializationDiskQueue, + _scrapy_non_serialization_queue, + _serializable_queue, ) @@ -20,6 +23,28 @@ class MyItem(Item): name = Field() +class NoPeekQueue: + """Queue class without the optional peek method.""" + + +_no_peek_queue = cast("Any", NoPeekQueue) + + +@pytest.mark.parametrize( + "queue_class", + [ + _serializable_queue(_no_peek_queue, pickle.dumps, pickle.loads), + _scrapy_non_serialization_queue(_no_peek_queue), + ], +) +def test_peek_unsupported(queue_class): + with pytest.raises( + NotImplementedError, + match="The underlying queue class does not implement 'peek'", + ): + queue_class().peek() + + def _test_procesor(x): return x + x diff --git a/tests/test_squeues_request.py b/tests/test_squeues_request.py index c779b005f..637b31314 100644 --- a/tests/test_squeues_request.py +++ b/tests/test_squeues_request.py @@ -8,7 +8,6 @@ from abc import ABC, abstractmethod from typing import TYPE_CHECKING import pytest -import queuelib from scrapy.http import Request from scrapy.spiders import Spider @@ -23,12 +22,11 @@ from scrapy.squeues import ( from scrapy.utils.test import get_crawler if TYPE_CHECKING: + import queuelib + from scrapy.crawler import Crawler -HAVE_PEEK = hasattr(queuelib.queue.FifoMemoryQueue, "peek") - - @pytest.fixture def crawler() -> Crawler: return get_crawler(Spider) @@ -40,46 +38,26 @@ class TestRequestQueueBase(ABC): def is_fifo(self) -> bool: raise NotImplementedError - @pytest.mark.parametrize("test_peek", [True, False]) - def test_one_element(self, q: queuelib.queue.BaseQueue, test_peek: bool): - if test_peek and not HAVE_PEEK: - pytest.skip("The queuelib queues do not define peek") - if not test_peek and HAVE_PEEK: - pytest.skip("The queuelib queues define peek") + def test_one_element(self, q: queuelib.queue.BaseQueue): assert len(q) == 0 - if test_peek: - assert q.peek() is None + assert q.peek() is None assert q.pop() is None req = Request("http://www.example.com") q.push(req) assert len(q) == 1 - if test_peek: - result = q.peek() - assert result is not None - assert result.url == req.url - else: - with pytest.raises( - NotImplementedError, - match="The underlying queue class does not implement 'peek'", - ): - q.peek() + result = q.peek() + assert result is not None + assert result.url == req.url result = q.pop() assert result is not None assert result.url == req.url assert len(q) == 0 - if test_peek: - assert q.peek() is None + assert q.peek() is None assert q.pop() is None - @pytest.mark.parametrize("test_peek", [True, False]) - def test_order(self, q: queuelib.queue.BaseQueue, test_peek: bool): - if test_peek and not HAVE_PEEK: - pytest.skip("The queuelib queues do not define peek") - if not test_peek and HAVE_PEEK: - pytest.skip("The queuelib queues define peek") + def test_order(self, q: queuelib.queue.BaseQueue): assert len(q) == 0 - if test_peek: - assert q.peek() is None + assert q.peek() is None assert q.pop() is None req1 = Request("http://www.example.com/1") req2 = Request("http://www.example.com/2") @@ -87,25 +65,17 @@ class TestRequestQueueBase(ABC): q.push(req1) q.push(req2) q.push(req3) - if not test_peek: - with pytest.raises( - NotImplementedError, - match="The underlying queue class does not implement 'peek'", - ): - q.peek() reqs = [req1, req2, req3] if self.is_fifo else [req3, req2, req1] for i, req in enumerate(reqs): assert len(q) == 3 - i - if test_peek: - result = q.peek() - assert result is not None - assert result.url == req.url + result = q.peek() + assert result is not None + assert result.url == req.url result = q.pop() assert result is not None assert result.url == req.url assert len(q) == 0 - if test_peek: - assert q.peek() is None + assert q.peek() is None assert q.pop() is None diff --git a/tests/test_utils_benchserver.py b/tests/test_utils_benchserver.py new file mode 100644 index 000000000..830cccf85 --- /dev/null +++ b/tests/test_utils_benchserver.py @@ -0,0 +1,36 @@ +from __future__ import annotations + +import re +from typing import Any + +from twisted.web.test.requesthelper import DummyRequest + +from scrapy.utils.benchserver import Root, _getarg + + +def _request(**args: bytes) -> Any: + request = DummyRequest([b""]) + request.args = {name.encode(): [value] for name, value in args.items()} + return request + + +def test_getarg() -> None: + request = _request(total=b"5") + assert _getarg(request, b"total", 100, int) == 5 + assert _getarg(request, b"show", 100, int) == 100 + assert _getarg(request, b"missing") is None + + +def test_render() -> None: + root = Root() # type: ignore[no-untyped-call] + request = _request(total=b"5", show=b"2") + assert root.getChild("follow", request) is root + assert root.render(request) == b"" + body = b"".join(request.written).decode() + assert body.startswith("") + assert body.endswith("") + numbers = re.findall( + r"follow \1", body + ) + assert len(numbers) == 2 + assert all(1 <= int(number) <= 5 for number in numbers) diff --git a/tests/test_utils_console.py b/tests/test_utils_console.py index e92ecd93d..5ff1b0173 100644 --- a/tests/test_utils_console.py +++ b/tests/test_utils_console.py @@ -4,6 +4,7 @@ import subprocess import sys from importlib.util import find_spec from io import BytesIO +from types import ModuleType from typing import TYPE_CHECKING import pytest @@ -67,6 +68,25 @@ def test_get_shell_embed_func_bpython(): assert shell.__name__ == "_embed_bpython_shell" +def test_embed_bpython_shell(monkeypatch: pytest.MonkeyPatch) -> None: + # bpython is never the default shell, so a stand-in module takes the place + # of the interactive session the IPython tests below use. + calls: list[tuple[dict[str, object], str]] = [] + bpython = ModuleType("bpython") + monkeypatch.setattr( + bpython, + "embed", + lambda locals_, banner: calls.append((locals_, banner)), + raising=False, + ) + monkeypatch.setitem(sys.modules, "bpython", bpython) + + shell = get_shell_embed_func(["bpython"]) + assert shell is not None + shell({"a": 1}, "SHELL-READY") + assert calls == [({"a": 1}, "SHELL-READY")] + + def test_get_shell_embed_func_ipython(): pytest.importorskip("IPython") shell = get_shell_embed_func(["ipython"]) diff --git a/tests/test_utils_defer.py b/tests/test_utils_defer.py index 3327b4052..e61741fec 100644 --- a/tests/test_utils_defer.py +++ b/tests/test_utils_defer.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio import random +import warnings from asyncio import Future from typing import TYPE_CHECKING, Any @@ -16,6 +17,7 @@ from scrapy.utils.defer import ( deferred_to_future, iter_errback, maybe_deferred_to_future, + maybeDeferred_coro, mustbe_deferred, parallel_async, ) @@ -409,3 +411,16 @@ class TestMaybeDeferredToFutureNotAsyncio: result = maybe_deferred_to_future(d) assert isinstance(result, Deferred) assert result is d + + +def test_maybe_deferred_coro_deferred() -> None: + d: Deferred[int] = Deferred() + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + assert maybeDeferred_coro(lambda: d) is d + # Only the deprecation of maybeDeferred_coro() itself is reported; callables + # that return a Deferred are the reason it exists. + assert [str(record.message) for record in records] == [ + "maybeDeferred_coro() is deprecated and will be removed in a future" + " Scrapy version." + ] diff --git a/tests/test_utils_deprecate.py b/tests/test_utils_deprecate.py index c877f5a9e..5aa221414 100644 --- a/tests/test_utils_deprecate.py +++ b/tests/test_utils_deprecate.py @@ -252,6 +252,11 @@ class TestWarnWhenSubclassed: ): create_deprecated_class("DeprecatedName", NewName) + def test_unknown_parent_module(self): + with mock.patch("inspect.getmodule", return_value=None): + cls = create_deprecated_class("DeprecatedName", NewName) + assert cls.__module__ == "scrapy.utils.deprecate" + @mock.patch( "scrapy.utils.deprecate.DEPRECATION_RULES",