Improve test coverage

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]

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(

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]):

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'<a href="http://example.com/page.html?b=2&amp;a=1">1</a>'
+            b'<a href="http://example.com/page.html?a=1&amp;b=2">2</a>'
+        )
+        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).

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
 

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
 
 

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("<html><head></head><body>")
+    assert body.endswith("</body></html>")
+    numbers = re.findall(
+        r"<a href='/follow\?total=5&show=2&n=(\d+)'>follow \1</a>", body
+    )
+    assert len(numbers) == 2
+    assert all(1 <= int(number) <= 5 for number in numbers)

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

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."
+    ]

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",