Improve test coverage for scrapy.commands (#7829)

scrapy/commands/genspider.py

@@ -32,10 +32,7 @@ def sanitize_module_name(module_name: str) -> str:
 
 def extract_domain(url: str) -> str:
     """Extract domain name from URL string"""
-    o = urlparse(url)
-    if o.scheme == "" and o.netloc == "":
-        o = urlparse("//" + url.lstrip("/"))
-    return o.netloc
+    return urlparse(url).netloc
 
 
 def verify_url_scheme(url: str) -> str:

scrapy/commands/parse.py

@@ -41,7 +41,7 @@ class Command(BaseRunSpiderCommand):
     spider: Spider | None = None
     items: ClassVar[dict[int, list[Any]]] = {}
     requests: ClassVar[dict[int, list[Request]]] = {}
-    spidercls: type[Spider] | None
+    spidercls: type[Spider] | None = None
 
     first_response = None
 

tests/test_command_crawl.py

@@ -23,6 +23,18 @@ class TestCrawlCommand(TestProjectBase):
         _, _, stderr = self.crawl(code, proj_path, args=args)
         return stderr
 
+    def test_no_spider(self, proj_path: Path) -> None:
+        returncode, out, _ = proc("crawl", cwd=proj_path)
+        assert returncode == 2
+        assert "Usage" in out
+
+    def test_multiple_spiders(self, proj_path: Path) -> None:
+        returncode, _, err = proc("crawl", "myspider", "myspider2", cwd=proj_path)
+        assert returncode == 2
+        assert (
+            "running 'scrapy crawl' with more than one spider is not supported" in err
+        )
+
     def test_no_output(self, proj_path: Path) -> None:
         spider_code = """
 import scrapy

tests/test_command_fetch.py

@@ -2,13 +2,24 @@ from __future__ import annotations
 
 from typing import TYPE_CHECKING
 
+import pytest
+
+from tests.utils.bases.commands import TestProjectBase
 from tests.utils.cmdline import proc
 
 if TYPE_CHECKING:
+    from pathlib import Path
+
     from tests.mockserver.http import MockServer
 
 
 class TestFetchCommand:
+    @pytest.mark.parametrize("args", [(), ("not-a-url",), ("a:b", "c:d")])
+    def test_bad_arguments(self, args: tuple[str, ...]) -> None:
+        returncode, out, _ = proc("fetch", *args)
+        assert returncode == 2
+        assert "Usage" in out
+
     def test_output(self, mockserver: MockServer) -> None:
         _, out, _ = proc("fetch", mockserver.url("/text"))
         assert out.strip() == "Works"
@@ -36,3 +47,24 @@ class TestFetchCommand:
             "fetch", "-s", "TWISTED_REACTOR_ENABLED=False", mockserver.url("/text")
         )
         assert out.strip() == "Works"
+
+
+class TestFetchCommandWithSpider(TestProjectBase):
+    @pytest.fixture(autouse=True)
+    def create_files(self, proj_path: Path) -> None:
+        (proj_path / self.project_name / "spiders" / "myspider.py").write_text(
+            """
+import scrapy
+
+class MySpider(scrapy.Spider):
+    name = "myspider"
+    custom_settings = {"USER_AGENT": "myspider-user-agent"}
+""",
+            encoding="utf-8",
+        )
+
+    def test_spider(self, proj_path: Path, mockserver: MockServer) -> None:
+        _, out, err = proc(
+            "fetch", "--spider", "myspider", mockserver.url("/echo"), cwd=proj_path
+        )
+        assert "myspider-user-agent" in out, err

tests/test_command_genspider.py

@@ -64,6 +64,24 @@ class TestGenspiderCommand(TestProjectBase):
         assert call("genspider", "--dump=basic", cwd=proj_path) == 0
         assert call("genspider", "-d", "basic", cwd=proj_path) == 0
 
+    @pytest.mark.parametrize(
+        "args",
+        [("--dump=nonexistent",), ("-t", "nonexistent", "test_name", "test.com")],
+    )
+    def test_unknown_template(self, args: tuple[str, ...], proj_path: Path) -> None:
+        returncode, out, err = proc("genspider", *args, cwd=proj_path)
+        assert returncode == 0, err
+        assert "Unable to find template: nonexistent" in out
+        assert not (proj_path / self.project_name / "spiders" / "test_name.py").exists()
+
+    def test_name_not_starting_with_a_letter(self, proj_path: Path) -> None:
+        """The module name, unlike the spider name, is prefixed with a letter."""
+        _, out, err = proc("genspider", "1st_spider", "test.com", cwd=proj_path)
+        assert "Created spider '1st_spider'" in out, err
+        spider = proj_path / self.project_name / "spiders" / "a1st_spider.py"
+        assert spider.exists()
+        assert find_in_file(spider, r'name\s*=\s*"1st_spider"') is not None
+
     @pytest.mark.skipif(
         sys.platform == "win32", reason="requires a POSIX shell editor script"
     )
@@ -87,7 +105,8 @@ class TestGenspiderCommand(TestProjectBase):
         )
 
     def test_same_name_as_project(self, proj_path: Path) -> None:
-        assert call("genspider", self.project_name, cwd=proj_path) == 2
+        _, out, err = proc("genspider", self.project_name, "test.com", cwd=proj_path)
+        assert "Cannot create a spider with the same name as your project" in out, err
         assert not (
             proj_path / self.project_name / "spiders" / f"{self.project_name}.py"
         ).exists()

tests/test_command_parse.py

@@ -3,6 +3,7 @@ from __future__ import annotations
 import argparse
 import re
 from typing import TYPE_CHECKING
+from urllib.parse import urlparse
 
 import pytest
 
@@ -552,6 +553,130 @@ ITEM_PIPELINES = {{'{self.project_name}.pipelines.MyPipeline': 1}}
         content = '[\n{},\n{"foo": "bar"}\n]'
         assert file_path.read_text(encoding="utf-8") == content
 
+    @pytest.mark.parametrize("args", [(), ("not-a-url",), ("a:b", "c:d")])
+    def test_bad_arguments(self, args: tuple[str, ...], proj_path: Path) -> None:
+        returncode, out, _ = proc("parse", *args, cwd=proj_path)
+        assert returncode == 2
+        assert "Usage" in out
+
+    @pytest.mark.parametrize(
+        ("option", "message"),
+        [
+            ("--meta", "Invalid -m/--meta value"),
+            ("-m", "Invalid -m/--meta value"),
+            ("--cbkwargs", "Invalid --cbkwargs value"),
+        ],
+    )
+    def test_invalid_json(
+        self, option: str, message: str, proj_path: Path, mockserver: MockServer
+    ) -> None:
+        returncode, _, err = proc(
+            "parse",
+            "--spider",
+            self.spider_name,
+            option,
+            "{invalid",
+            mockserver.url("/html"),
+            cwd=proj_path,
+        )
+        assert returncode == 2
+        assert message in err
+
+    def test_unknown_spider(self, proj_path: Path, mockserver: MockServer) -> None:
+        returncode, _, err = proc(
+            "parse",
+            "--spider",
+            "nonexistent",
+            mockserver.url("/html"),
+            cwd=proj_path,
+        )
+        assert returncode == 0, err
+        assert "Unable to find spider: nonexistent" in err
+
+    def test_spider_found_by_url(self, proj_path: Path, mockserver: MockServer) -> None:
+        """Without --spider, the spider is chosen based on the URL."""
+        url = mockserver.url("/html")
+        # The spider name doubles as a domain of the spider, and it is matched
+        # against the netloc of the URL, hence the port.
+        (proj_path / self.project_name / "spiders" / "urlspider.py").write_text(
+            f"""
+import scrapy
+
+class UrlSpider(scrapy.Spider):
+    name = "{urlparse(url).netloc}"
+
+    def parse(self, response):
+        return [{{"found_by_url": True}}]
+""",
+            encoding="utf-8",
+        )
+        returncode, out, err = proc("parse", url, cwd=proj_path)
+        assert returncode == 0, err
+        assert "Unable to find spider for" not in err
+        assert "{'found_by_url': True}" in out
+
+    def test_legacy_item_processor(
+        self, proj_path: Path, mockserver: MockServer
+    ) -> None:
+        """--pipelines supports an ITEM_PROCESSOR without process_item_async()."""
+        (proj_path / self.project_name / "legacy.py").write_text(
+            """
+import logging
+
+from twisted.internet.defer import succeed
+
+
+class LegacyItemProcessor:
+    @classmethod
+    def from_crawler(cls, crawler):
+        return cls()
+
+    def open_spider(self, spider):
+        return succeed(None)
+
+    def close_spider(self, spider):
+        return succeed(None)
+
+    def process_item(self, item, spider):
+        logging.info("Legacy item processor!")
+        return succeed(item)
+""",
+            encoding="utf-8",
+        )
+        _, _, stderr = proc(
+            "parse",
+            "--spider",
+            self.spider_name,
+            "--pipelines",
+            "-c",
+            "parse",
+            "-s",
+            f"ITEM_PROCESSOR={self.project_name}.legacy.LegacyItemProcessor",
+            mockserver.url("/html"),
+            cwd=proj_path,
+        )
+        assert "INFO: Legacy item processor!" in stderr
+
+    @pytest.mark.parametrize("verbose", [True, False])
+    def test_no_items_no_links(
+        self, verbose: bool, proj_path: Path, mockserver: MockServer
+    ) -> None:
+        args = ["--verbose"] if verbose else []
+        _, out, err = proc(
+            "parse",
+            "--spider",
+            self.spider_name,
+            "-c",
+            "parse",
+            "--noitems",
+            "--nolinks",
+            *args,
+            mockserver.url("/html"),
+            cwd=proj_path,
+        )
+        assert "# Scraped Items" not in out, err
+        assert "# Requests" not in out
+
     def test_parse_add_options(self):
         command = parse.Command()
         command.settings = Settings()

tests/test_command_runspider.py

@@ -136,6 +136,12 @@ class MySpider(scrapy.Spider):
         log = self.get_log(tmp_path, "from scrapy.spiders import Spider\n")
         assert "No spider found in file" in log
 
+    @pytest.mark.parametrize("args", [(), ("a.py", "b.py")])
+    def test_runspider_bad_arguments(self, args: tuple[str, ...]) -> None:
+        returncode, out, _ = proc("runspider", *args)
+        assert returncode == 2
+        assert "Usage" in out
+
     def test_runspider_file_not_found(self) -> None:
         _, _, log = proc("runspider", "some_non_existent_file")
         assert "File not found: some_non_existent_file" in log

tests/test_command_shell.py

@@ -18,6 +18,7 @@ from scrapy.shell import Shell, inspect_response
 from scrapy.utils.reactor import _asyncio_reactor_path
 from scrapy.utils.test import get_crawler
 from tests import NON_EXISTING_RESOLVABLE, tests_datadir
+from tests.utils.bases.commands import TestProjectBase
 from tests.utils.cmdline import proc
 from tests.utils.decorators import coroutine_test
 
@@ -162,6 +163,33 @@ class TestShellCommand:
         assert ret == 0, out
 
 
+class TestShellCommandWithSpider(TestProjectBase):
+    @pytest.fixture(autouse=True)
+    def create_files(self, proj_path: Path) -> None:
+        (proj_path / self.project_name / "spiders" / "myspider.py").write_text(
+            """
+import scrapy
+
+class MySpider(scrapy.Spider):
+    name = "myspider"
+""",
+            encoding="utf-8",
+        )
+
+    def test_spider(self, proj_path: Path, mockserver: MockServer) -> None:
+        ret, out, err = proc(
+            "shell",
+            "--spider",
+            "myspider",
+            mockserver.url("/text"),
+            "-c",
+            "spider.name",
+            cwd=proj_path,
+        )
+        assert ret == 0, err
+        assert out.strip() == "myspider"
+
+
 class TestInteractiveShell:
     def test_fetch(self, mockserver: MockServer) -> None:
         args = (

tests/test_commands.py

@@ -3,21 +3,27 @@ from __future__ import annotations
 import argparse
 import json
 import sys
+from pathlib import Path
 from typing import TYPE_CHECKING
 
 import pytest
 
 import scrapy
 from scrapy.cmdline import _pop_command_name, execute
-from scrapy.commands import ScrapyCommand, ScrapyHelpFormatter, view
+from scrapy.commands import ScrapyCommand, ScrapyHelpFormatter
 from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.settings import Settings
 from scrapy.utils.reactor import _asyncio_reactor_path
 from tests.utils.bases.commands import TestProjectBase
-from tests.utils.cmdline import call, proc, write_recording_editor
+from tests.utils.cmdline import (
+    call,
+    proc,
+    write_recording_browser,
+    write_recording_editor,
+)
 
 if TYPE_CHECKING:
-    from pathlib import Path
+    from tests.mockserver.http import MockServer
 
 
 class EmptyCommand(ScrapyCommand):
@@ -107,6 +113,93 @@ class TestCommandSettings:
         )
 
 
+class TestGlobalOptions:
+    """Tests for the options that every command supports."""
+
+    spider_code = """
+import scrapy
+
+class MySpider(scrapy.Spider):
+    name = "myspider"
+
+    async def start(self):
+        self.logger.debug("It works!")
+        return
+        yield
+"""
+
+    @pytest.fixture
+    def spider_path(self, tmp_path: Path) -> Path:
+        path = tmp_path / "myspider.py"
+        path.write_text(self.spider_code, encoding="utf-8")
+        return path
+
+    def test_invalid_set(self, spider_path: Path) -> None:
+        returncode, _, err = proc("runspider", str(spider_path), "-s", "FOO")
+        assert returncode == 2
+        assert "Invalid -s value, use -s NAME=VALUE" in err
+
+    def test_invalid_spider_argument(self, spider_path: Path) -> None:
+        returncode, _, err = proc("runspider", str(spider_path), "-a", "FOO")
+        assert returncode == 2
+        assert "Invalid -a value, use -a NAME=VALUE" in err
+
+    def test_logfile(self, tmp_path: Path, spider_path: Path) -> None:
+        logfile = tmp_path / "scrapy.log"
+        returncode, _, err = proc(
+            "runspider", str(spider_path), "--logfile", str(logfile)
+        )
+        assert returncode == 0, err
+        assert "It works!" in logfile.read_text(encoding="utf-8")
+        assert "It works!" not in err
+
+    def test_loglevel(self, spider_path: Path) -> None:
+        returncode, _, err = proc("runspider", str(spider_path), "--loglevel", "INFO")
+        assert returncode == 0, err
+        assert "It works!" not in err
+        assert "Spider closed (finished)" in err
+
+    def test_nolog(self, spider_path: Path) -> None:
+        returncode, _, err = proc("runspider", str(spider_path), "--nolog")
+        assert returncode == 0, err
+        assert not err
+
+    def test_pidfile(self, tmp_path: Path, spider_path: Path) -> None:
+        pidfile = tmp_path / "scrapy.pid"
+        returncode, _, err = proc(
+            "runspider", str(spider_path), "--pidfile", str(pidfile)
+        )
+        assert returncode == 0, err
+        assert pidfile.read_text(encoding="utf-8").strip().isdigit()
+
+    def test_pdb(self, spider_path: Path) -> None:
+        returncode, _, err = proc("runspider", str(spider_path), "--pdb")
+        assert returncode == 0, err
+        assert "It works!" in err
+
+
+class TestSettingsCommand:
+    @pytest.mark.parametrize(
+        ("option", "setting", "expected"),
+        [
+            ("--get", "BOT_NAME", "scrapybot"),
+            ("--getbool", "COOKIES_ENABLED", "True"),
+            ("--getint", "CONCURRENT_REQUESTS", "16"),
+            ("--getfloat", "DOWNLOAD_DELAY", "0.0"),
+            ("--getlist", "SPIDER_MODULES", "[]"),
+        ],
+    )
+    def test_get(self, option: str, setting: str, expected: str) -> None:
+        returncode, out, err = proc("settings", option, setting)
+        assert returncode == 0, err
+        assert out.startswith(expected)
+
+    def test_no_option(self) -> None:
+        returncode, out, err = proc("settings")
+        assert returncode == 0, err
+        assert not out
+
+
 class TestCommandCrawlerProcess(TestProjectBase):
     """Test that the command uses the expected kind of *CrawlerProcess
     and produces expected errors when needed."""
@@ -577,18 +670,31 @@ class TestBenchCommand:
 
 
 class TestViewCommand:
-    def test_methods(self) -> None:
-        command = view.Command()
-        command.settings = Settings()
-        parser = argparse.ArgumentParser(
-            prog="scrapy",
-            prefix_chars="-",
-            formatter_class=ScrapyHelpFormatter,
-            conflict_handler="resolve",
+    @pytest.mark.skipif(
+        sys.platform == "win32", reason="requires a POSIX shell browser script"
+    )
+    def test_view(
+        self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, mockserver: MockServer
+    ) -> None:
+        opened = tmp_path / "opened.txt"
+        browser = tmp_path / "fake-browser.sh"
+        write_recording_browser(browser, opened)
+        monkeypatch.setenv("BROWSER", str(browser))
+
+        returncode, _, err = proc("view", mockserver.url("/html"), cwd=tmp_path)
+
+        assert returncode == 0, err
+        url = opened.read_text(encoding="utf-8")
+        assert url.startswith("file://")
+        body = Path(url.removeprefix("file://")).read_text(encoding="utf-8")
+        assert "<p class='one'>Works</p>" in body
+
+    def test_non_text_response(self, mockserver: MockServer) -> None:
+        returncode, _, err = proc(
+            "view", mockserver.url("/static/files/images/scrapy.png")
         )
-        command.add_options(parser)
-        assert command.short_desc() == "Open URL in browser, as seen by Scrapy"
-        assert "URL using the Scrapy downloader and show its" in command.long_desc()
+        assert returncode == 0, err
+        assert "Cannot view a non-text response." in err
 
 
 class TestEditCommand(TestProjectBase):
@@ -615,6 +721,11 @@ class TestEditCommand(TestProjectBase):
         assert returncode == 1
         assert "Spider not found: nonexistent" in err
 
+    def test_edit_no_spider(self, proj_path: Path) -> None:
+        returncode, out, _ = proc("edit", cwd=proj_path)
+        assert returncode == 2
+        assert "Usage" in out
+
 
 class TestHelpMessage(TestProjectBase):
     @pytest.mark.parametrize(

tests/utils/cmdline.py

@@ -46,3 +46,16 @@ def write_recording_editor(editor: Path) -> None:
     open (its last argument) into the file given as its first argument."""
     editor.write_text('#!/bin/sh\nprintf "%s" "$2" > "$1"\n', encoding="utf-8")
     editor.chmod(0o755)
+
+
+def write_recording_browser(browser: Path, recorded: Path) -> None:
+    """Create an executable browser script that writes the URL it is asked to
+    open into *recorded*.
+
+    ``webbrowser`` only passes the URL to the command from the ``BROWSER``
+    environment variable, hence the hardcoded output path.
+    """
+    browser.write_text(
+        f'#!/bin/sh\nprintf "%s" "$1" > "{recorded}"\n', encoding="utf-8"
+    )
+    browser.chmod(0o755)