From c5ce5cefcf9ad761f940818bd5c411bfbf697743 Mon Sep 17 00:00:00 2001 From: Lia Launtz Date: Fri, 20 Feb 2026 22:04:43 -0500 Subject: [PATCH 1/8] edit command tests with small precommit changes --- tests/test_command_edit.py | 74 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 tests/test_command_edit.py diff --git a/tests/test_command_edit.py b/tests/test_command_edit.py new file mode 100644 index 000000000..209fa16b3 --- /dev/null +++ b/tests/test_command_edit.py @@ -0,0 +1,74 @@ +from __future__ import annotations + +import os +from pathlib import Path +from unittest import mock + +import pytest + +from scrapy.cmdline import ScrapyArgumentParser +from scrapy.commands.edit import Command +from scrapy.utils.project import get_project_settings +from tests.test_commands import TestProjectBase +from tests.utils.cmdline import call, proc + + +class TestEditCommand(TestProjectBase): + """unit testing of the 'scrapy edit' command""" + + @pytest.fixture + def create_spider(self, proj_path: Path): + """setup creates spider, teardown removes spider""" + # setup: create spider to edit + test_name = "test_name" + call("genspider", test_name, "test.com", cwd=proj_path) + spider = proj_path / self.project_name / "spiders" / "test_name.py" + yield proj_path, spider, test_name + # teardown: remove spider from project + Path.unlink(spider) + + def test_edit_valid_spider(self, create_spider) -> None: + """test call to edit command with correct spider name""" + proj_path, spider, test_name = create_spider + assert spider.exists() + assert call("edit", test_name, cwd=proj_path) == 0 + + def test_edit_nospider(self, proj_path: Path) -> None: + """test call to edit if no spider has been specified""" + assert call("edit", "not_a_valid_spider", cwd=proj_path) == 1 + + def test_edit_implicit(self, create_spider): + """tests edit command calls editor""" + proj_path, spider, test_name = create_spider + + # change current working directory + os.chdir(proj_path) + + # create edit command object + cmd = Command() + cmd.settings = get_project_settings() + editor = cmd.settings.get("EDITOR") + + # parse commandline arguments + parser = ScrapyArgumentParser() + opts, _ = parser.parse_known_args(["edit", test_name]) + + # mock the operating system + with mock.patch("scrapy.commands.edit.os.system", return_value=0) as mock_sys: + cmd.run([test_name], opts) + mock_sys.assert_called_once_with(f'{editor} "{spider}"') + + def test_edit_help_syntax(self, proj_path: Path) -> None: + """Check that long description and syntax are included in edit -h""" + rtn_code, out, _ = proc("edit", "-h", cwd=proj_path) + assert rtn_code == 0 + cmd = Command() + assert cmd.long_desc() in out + assert cmd.syntax() in out + + def test_edit_short_desc(self, proj_path: Path) -> None: + """Check that short description included in scrapy -h""" + rtn_code, out, _ = proc("-h", cwd=proj_path) + assert rtn_code == 0 + cmd = Command() + assert cmd.short_desc() in out From d9b5a64e8fbdf5a3ca99ba531af1ed7156a976ca Mon Sep 17 00:00:00 2001 From: Lia Launtz Date: Sat, 21 Feb 2026 01:01:34 -0500 Subject: [PATCH 2/8] edit command tests adjusted for precommit --- tests/test_command_edit.py | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/tests/test_command_edit.py b/tests/test_command_edit.py index 209fa16b3..1a040912e 100644 --- a/tests/test_command_edit.py +++ b/tests/test_command_edit.py @@ -1,14 +1,10 @@ from __future__ import annotations -import os from pathlib import Path -from unittest import mock import pytest -from scrapy.cmdline import ScrapyArgumentParser from scrapy.commands.edit import Command -from scrapy.utils.project import get_project_settings from tests.test_commands import TestProjectBase from tests.utils.cmdline import call, proc @@ -18,7 +14,7 @@ class TestEditCommand(TestProjectBase): @pytest.fixture def create_spider(self, proj_path: Path): - """setup creates spider, teardown removes spider""" + """creates spider needed for tests""" # setup: create spider to edit test_name = "test_name" call("genspider", test_name, "test.com", cwd=proj_path) @@ -37,27 +33,6 @@ class TestEditCommand(TestProjectBase): """test call to edit if no spider has been specified""" assert call("edit", "not_a_valid_spider", cwd=proj_path) == 1 - def test_edit_implicit(self, create_spider): - """tests edit command calls editor""" - proj_path, spider, test_name = create_spider - - # change current working directory - os.chdir(proj_path) - - # create edit command object - cmd = Command() - cmd.settings = get_project_settings() - editor = cmd.settings.get("EDITOR") - - # parse commandline arguments - parser = ScrapyArgumentParser() - opts, _ = parser.parse_known_args(["edit", test_name]) - - # mock the operating system - with mock.patch("scrapy.commands.edit.os.system", return_value=0) as mock_sys: - cmd.run([test_name], opts) - mock_sys.assert_called_once_with(f'{editor} "{spider}"') - def test_edit_help_syntax(self, proj_path: Path) -> None: """Check that long description and syntax are included in edit -h""" rtn_code, out, _ = proc("edit", "-h", cwd=proj_path) From a9a36c6262788500074fde053263c5248d293222 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 23 Feb 2026 15:34:56 +0500 Subject: [PATCH 3/8] Pass correct env to PopenSpawn() in tests. (#7279) --- tests/test_crawler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index ba33d0bbe..9ade8472f 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -951,7 +951,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): def test_shutdown_graceful(self): sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK args = self.get_script_args("sleeping.py", "3") - p = PopenSpawn(args, timeout=5) + p = PopenSpawn(args, timeout=5, env=get_script_run_env()) p.expect_exact("Spider opened") p.expect_exact("Crawled (200)") p.kill(sig) @@ -963,7 +963,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): def test_shutdown_forced(self): sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK args = self.get_script_args("sleeping.py", "10") - p = PopenSpawn(args, timeout=5) + p = PopenSpawn(args, timeout=5, env=get_script_run_env()) p.expect_exact("Spider opened") p.expect_exact("Crawled (200)") p.kill(sig) From 353135cf10b69305a7881c093d4563e2699dd80c Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 23 Feb 2026 15:48:38 +0500 Subject: [PATCH 4/8] Bump ruff, fix some of rules (#7277) --- .pre-commit-config.yaml | 2 +- pyproject.toml | 41 ++++++++----------- scrapy/core/downloader/__init__.py | 4 +- scrapy/core/downloader/contextfactory.py | 6 +-- scrapy/core/downloader/handlers/file.py | 2 +- scrapy/core/engine.py | 4 +- scrapy/core/http2/agent.py | 4 +- scrapy/core/spidermw.py | 2 +- scrapy/http/request/__init__.py | 10 ++--- scrapy/linkextractors/lxmlhtml.py | 2 +- scrapy/responsetypes.py | 2 +- scrapy/shell.py | 1 - scrapy/spidermiddlewares/referer.py | 2 +- scrapy/utils/defer.py | 2 +- scrapy/utils/engine.py | 2 +- scrapy/utils/log.py | 4 +- scrapy/utils/reactor.py | 2 +- scrapy/utils/signal.py | 6 +-- tests/mockserver/http_resources.py | 2 +- tests/spiders.py | 2 +- tests/test_downloadermiddleware_retry.py | 2 +- tests/test_exporters.py | 4 +- tests/test_extension_periodic_log.py | 18 +++++--- tests/test_item.py | 2 +- tests/test_link.py | 2 +- tests/test_pipeline_files.py | 6 +-- tests/test_pipeline_images.py | 4 +- ...t_return_with_argument_inside_generator.py | 2 +- tests/test_utils_spider.py | 2 +- tests/test_webclient.py | 2 +- 30 files changed, 70 insertions(+), 76 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index dc4276cff..a2bf1be0c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,7 +6,7 @@ exclude: | ) repos: - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.14.2 + rev: v0.15.2 hooks: - id: ruff-check args: [ --fix ] diff --git a/pyproject.toml b/pyproject.toml index 7cb83c483..e4fae8bf4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -174,7 +174,6 @@ disable = [ "disallowed-name", "duplicate-code", # https://github.com/pylint-dev/pylint/issues/214 "fixme", - "import-outside-toplevel", "inherit-non-class", # false positives with create_deprecated_class() "invalid-name", "invalid-overridden-method", @@ -188,12 +187,10 @@ disable = [ "no-value-for-parameter", # https://github.com/pylint-dev/pylint/issues/3268 "not-callable", "protected-access", - "redefined-builtin", "redefined-outer-name", "too-few-public-methods", "too-many-ancestors", "too-many-arguments", - "too-many-branches", "too-many-function-args", "too-many-instance-attributes", "too-many-lines", @@ -202,12 +199,22 @@ disable = [ "too-many-public-methods", "too-many-return-statements", "unused-argument", - "unused-import", "unused-variable", "useless-import-alias", # used as a hint to mypy "useless-return", # https://github.com/pylint-dev/pylint/issues/6530 "wrong-import-position", + # Ones that are implemented in ruff and need to be disabled for some lines (listed here to avoid two disable comments) + "bare-except", + "eval-used", + "global-statement", + "import-outside-toplevel", + "import-self", + "inconsistent-return-statements", + "redefined-builtin", + "too-many-branches", + "unused-import", + # Ones that we may want to address (fix, ignore per-line or move to "don't want to fix") "abstract-method", "arguments-differ", @@ -345,34 +352,22 @@ ignore = [ "D403", # `try`-`except` within a loop incurs performance overhead "PERF203", - # Import alias does not rename original package - "PLC0414", # Too many return statements "PLR0911", - # Too many branches - "PLR0912", # Too many arguments in function definition "PLR0913", - # Too many statements - "PLR0915", # Magic value used in comparison "PLR2004", - # `for` loop variable overwritten by assignment target - "PLW2901", # String contains ambiguous {}. "RUF001", # Docstring contains ambiguous {}. "RUF002", # Comment contains ambiguous {}. "RUF003", - # Mutable class attributes should be annotated with `typing.ClassVar` - "RUF012", # Use of `assert` detected; needed for mypy "S101", # FTP-related functions are being called; https://github.com/scrapy/scrapy/issues/4180 "S321", - # Argument default set to insecure SSL protocol - "S503", # Use a context manager for opening files "SIM115", # Yoda condition detected @@ -384,18 +379,16 @@ ignore = [ "B003", # Do not use mutable data structures for argument defaults. "B006", - # Loop control variable not used within the loop body. - "B007", - # Do not perform function calls in argument defaults. - "B008", # Found useless expression. "B018", - # Star-arg unpacking after a keyword argument is strongly discouraged. - "B026", # No explicit stacklevel argument found. "B028", # Within an `except` clause, raise exceptions with `raise ... from` "B904", + # `for` loop variable overwritten by assignment target + "PLW2901", + # Mutable class attributes should be annotated with `typing.ClassVar` + "RUF012", # Use capitalized environment variable "SIM112", ] @@ -417,8 +410,8 @@ split-on-trailing-comma = false "scrapy/linkextractors/__init__.py" = ["E402"] "scrapy/spiders/__init__.py" = ["E402"] -# Skip bandit in tests -"tests/**" = ["S"] +# Skip bandit and allow blocking file I/O in tests +"tests/**" = ["ASYNC240", "S"] # Issues pending a review: "docs/conf.py" = ["E402"] diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index c862ecdf9..59871fdc3 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -157,9 +157,7 @@ class Downloader: if key not in self.slots: assert self.crawler.spider slot_settings = self.per_slot_settings.get(key, {}) - conc = ( - self.ip_concurrency if self.ip_concurrency else self.domain_concurrency - ) + conc = self.ip_concurrency or self.domain_concurrency conc, delay = _get_concurrency_delay( conc, self.crawler.spider, self.settings ) diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index 5f7c9a8c3..252f509bf 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -49,7 +49,7 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): def __init__( self, - method: int = SSL.SSLv23_METHOD, + method: int = SSL.SSLv23_METHOD, # noqa: S503 tls_verbose_logging: bool = False, tls_ciphers: str | None = None, *args: Any, @@ -75,7 +75,7 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): def from_crawler( cls, crawler: Crawler, - method: int = SSL.SSLv23_METHOD, + method: int = SSL.SSLv23_METHOD, # noqa: S503 *args: Any, **kwargs: Any, ) -> Self: @@ -84,10 +84,10 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): ) tls_ciphers: str | None = crawler.settings["DOWNLOADER_CLIENT_TLS_CIPHERS"] return cls( # type: ignore[misc] + *args, method=method, tls_verbose_logging=tls_verbose_logging, tls_ciphers=tls_ciphers, - *args, **kwargs, ) diff --git a/scrapy/core/downloader/handlers/file.py b/scrapy/core/downloader/handlers/file.py index 21fd2c353..a59aa722b 100644 --- a/scrapy/core/downloader/handlers/file.py +++ b/scrapy/core/downloader/handlers/file.py @@ -16,6 +16,6 @@ if TYPE_CHECKING: class FileDownloadHandler(BaseDownloadHandler): async def download_request(self, request: Request) -> Response: filepath = file_uri_to_path(request.url) - body = Path(filepath).read_bytes() + body = Path(filepath).read_bytes() # noqa: ASYNC240 respcls = responsetypes.from_args(filename=filepath, body=body) return respcls(url=request.url, body=body) diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 93fc64cdc..1555fc0d6 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -439,7 +439,7 @@ class ExecutionEngine: spider=self.spider, dont_log=IgnoreRequest, ) - for handler, result in request_scheduled_result: + for _, result in request_scheduled_result: if isinstance(result, Failure) and isinstance(result.value, IgnoreRequest): return if not self._slot.scheduler.enqueue_request(request): # type: ignore[union-attr] @@ -587,7 +587,7 @@ class ExecutionEngine: ) return deferred_from_coro(self.close_spider_async(reason=reason)) - async def close_spider_async(self, *, reason: str = "cancelled") -> None: + async def close_spider_async(self, *, reason: str = "cancelled") -> None: # noqa: PLR0912 """Close (cancel) spider and clear all its outstanding requests. .. versionadded:: 2.14 diff --git a/scrapy/core/http2/agent.py b/scrapy/core/http2/agent.py index 45f32daaa..8760b13ec 100644 --- a/scrapy/core/http2/agent.py +++ b/scrapy/core/http2/agent.py @@ -120,7 +120,7 @@ class H2Agent: self, reactor: ReactorBase, pool: H2ConnectionPool, - context_factory: BrowserLikePolicyForHTTPS = BrowserLikePolicyForHTTPS(), + context_factory: BrowserLikePolicyForHTTPS = BrowserLikePolicyForHTTPS(), # noqa: B008 connect_timeout: float | None = None, bind_address: bytes | None = None, ) -> None: @@ -164,7 +164,7 @@ class ScrapyProxyH2Agent(H2Agent): reactor: ReactorBase, proxy_uri: URI, pool: H2ConnectionPool, - context_factory: BrowserLikePolicyForHTTPS = BrowserLikePolicyForHTTPS(), + context_factory: BrowserLikePolicyForHTTPS = BrowserLikePolicyForHTTPS(), # noqa: B008 connect_timeout: float | None = None, bind_address: bytes | None = None, ) -> None: diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 6eaf84642..476d2e504 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -259,7 +259,7 @@ class SpiderMiddlewareManager(MiddlewareManager): # being available immediately which doesn't work when it's a wrapped coroutine. # It also needs @inlineCallbacks only because of downgrading so it can be removed when downgrading is removed. @inlineCallbacks - def _process_spider_output( + def _process_spider_output( # noqa: PLR0912 self, response: Response, result: Iterable[_T] | AsyncIterator[_T], diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 61e50927d..c4a0c8131 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -200,7 +200,7 @@ class Request(object_ref): #: .. seealso:: :ref:`topics-request-response-ref-errbacks` self.errback: Callable[[Failure], Any] | None = errback - self._cookies: CookiesT | None = cookies if cookies else None + self._cookies: CookiesT | None = cookies or None self._headers: Headers | None = ( Headers(headers, encoding=encoding) if headers else None ) @@ -244,7 +244,7 @@ class Request(object_ref): @cb_kwargs.setter def cb_kwargs(self, value: dict[str, Any] | None) -> None: - self._cb_kwargs = value if value else None + self._cb_kwargs = value or None @property def meta(self) -> dict[str, Any]: @@ -254,7 +254,7 @@ class Request(object_ref): @meta.setter def meta(self, value: dict[str, Any] | None) -> None: - self._meta = value if value else None + self._meta = value or None @property def url(self) -> str: @@ -292,7 +292,7 @@ class Request(object_ref): @flags.setter def flags(self, value: list[str] | None) -> None: - self._flags = value if value else None + self._flags = value or None @property def cookies(self) -> CookiesT: @@ -302,7 +302,7 @@ class Request(object_ref): @cookies.setter def cookies(self, value: CookiesT | None) -> None: - self._cookies = value if value else None + self._cookies = value or None @property def headers(self) -> Headers: diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 0ea78e35a..96a0f523b 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -110,7 +110,7 @@ class LxmlParserLinkExtractor: ) -> list[Link]: links: list[Link] = [] # hacky way to get the underlying lxml parsed document - for el, attr, attr_val in self._iter_links(selector.root): + for el, _, attr_val in self._iter_links(selector.root): # pseudo lxml.html.HtmlElement.make_links_absolute(base_url) try: if self.strip: diff --git a/scrapy/responsetypes.py b/scrapy/responsetypes.py index 3f6f030a5..3aaf17b53 100644 --- a/scrapy/responsetypes.py +++ b/scrapy/responsetypes.py @@ -54,7 +54,7 @@ class ResponseTypes: return Response if mimetype in self.classes: return self.classes[mimetype] - basetype = f"{mimetype.split('/')[0]}/*" + basetype = f"{mimetype.split('/', maxsplit=1)[0]}/*" return self.classes.get(basetype, Response) def from_content_type( diff --git a/scrapy/shell.py b/scrapy/shell.py index 4b2bdf6cf..00097d224 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -73,7 +73,6 @@ class Shell: else: self.populate_vars() if self.code: - # pylint: disable-next=eval-used print(eval(self.code, globals(), self.vars)) # noqa: S307 else: # Detect interactive shell setting in scrapy.cfg diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index e4d0f4014..ac340324a 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -97,7 +97,7 @@ class ReferrerPolicy(ABC): def potentially_trustworthy(self, url: str) -> bool: # Note: this does not follow https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy parsed_url = urlparse(url) - if parsed_url.scheme in ("data",): + if parsed_url.scheme == "data": return False return self.tls_protected(url) diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 5bd4506d6..0d937fea5 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -435,7 +435,7 @@ def _maybeDeferred_coro( """Copy of defer.maybeDeferred that also converts coroutines to Deferreds.""" try: result = f(*args, **kw) - except: # noqa: E722 # pylint: disable=bare-except + except: # noqa: E722 return fail(failure.Failure(captureVars=Deferred.debug)) # when the deprecation period has ended we need to make sure the behavior diff --git a/scrapy/utils/engine.py b/scrapy/utils/engine.py index 1e0c53212..085720f66 100644 --- a/scrapy/utils/engine.py +++ b/scrapy/utils/engine.py @@ -32,7 +32,7 @@ def get_engine_status(engine: ExecutionEngine) -> list[tuple[str, Any]]: checks: list[tuple[str, Any]] = [] for test in tests: try: - checks += [(test, eval(test))] # noqa: S307 # pylint: disable=eval-used + checks += [(test, eval(test))] # noqa: S307 except Exception as e: checks += [(test, f"{type(e).__name__} (exception)")] diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 07a9e5ff7..5f0d19913 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -138,7 +138,7 @@ _scrapy_root_handler: logging.Handler | None = None def install_scrapy_root_handler(settings: Settings) -> None: - global _scrapy_root_handler # noqa: PLW0603 # pylint: disable=global-statement + global _scrapy_root_handler # noqa: PLW0603 _uninstall_scrapy_root_handler() logging.root.setLevel(logging.NOTSET) @@ -147,7 +147,7 @@ def install_scrapy_root_handler(settings: Settings) -> None: def _uninstall_scrapy_root_handler() -> None: - global _scrapy_root_handler # noqa: PLW0603 # pylint: disable=global-statement + global _scrapy_root_handler # noqa: PLW0603 if ( _scrapy_root_handler is not None diff --git a/scrapy/utils/reactor.py b/scrapy/utils/reactor.py index f37c48a7d..6aa72064b 100644 --- a/scrapy/utils/reactor.py +++ b/scrapy/utils/reactor.py @@ -26,7 +26,7 @@ _T = TypeVar("_T") _P = ParamSpec("_P") -def listen_tcp(portrange: list[int], host: str, factory: ServerFactory) -> Port: # type: ignore[return] # pylint: disable=inconsistent-return-statements # noqa: RET503 +def listen_tcp(portrange: list[int], host: str, factory: ServerFactory) -> Port: # type: ignore[return] # noqa: RET503 """Like reactor.listenTCP but tries different ports in a range.""" from twisted.internet import reactor diff --git a/scrapy/utils/signal.py b/scrapy/utils/signal.py index 1f7426e59..997e8eba8 100644 --- a/scrapy/utils/signal.py +++ b/scrapy/utils/signal.py @@ -50,7 +50,7 @@ def send_catch_log( result: TypingAny try: response = robustApply( - receiver, signal=signal, sender=sender, *arguments, **named + receiver, *arguments, signal=signal, sender=sender, **named ) if isinstance(response, Deferred): logger.error( @@ -118,9 +118,9 @@ def _send_catch_log_deferred( robustApply, True, receiver, + *arguments, signal=signal, sender=sender, - *arguments, **named, ) d.addErrback(logerror, receiver) @@ -190,7 +190,7 @@ async def _send_catch_log_asyncio( try: result = await ensure_awaitable( robustApply( - receiver, signal=signal, sender=sender, *arguments, **named + receiver, *arguments, signal=signal, sender=sender, **named ), _warn=global_object_name(receiver), ) diff --git a/tests/mockserver/http_resources.py b/tests/mockserver/http_resources.py index 62a3146bf..91577aa6b 100644 --- a/tests/mockserver/http_resources.py +++ b/tests/mockserver/http_resources.py @@ -282,7 +282,7 @@ class LargeChunkedFileResource(resource.Resource): from twisted.internet import reactor def response(): - for i in range(1024): + for _ in range(1024): request.write(b"x" * 1024) request.finish() diff --git a/tests/spiders.py b/tests/spiders.py index 79565738d..363a09fad 100644 --- a/tests/spiders.py +++ b/tests/spiders.py @@ -389,7 +389,7 @@ class DuplicateStartSpider(MockServerSpider): async def start(self): for i in range(self.distinct_urls): - for j in range(self.dupe_factor): + for _ in range(self.dupe_factor): url = self.mockserver.url(f"/echo?headers=1&body=test{i}") yield Request(url, dont_filter=self.dont_filter) diff --git a/tests/test_downloadermiddleware_retry.py b/tests/test_downloadermiddleware_retry.py index 20e8fc407..50946899a 100644 --- a/tests/test_downloadermiddleware_retry.py +++ b/tests/test_downloadermiddleware_retry.py @@ -250,7 +250,7 @@ class TestMaxRetryTimes: ): middleware = middleware or self.mw - for i in range(max_retry_times): + for _ in range(max_retry_times): req = middleware.process_exception(req, exception) assert isinstance(req, Request) diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 5719d5bb0..2fded613d 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -195,9 +195,7 @@ class TestPprintItemExporter(TestBaseItemExporter): return PprintItemExporter(self.output, **kwargs) def _check_output(self): - self._assert_expected_item( - eval(self.output.getvalue()) # pylint: disable=eval-used - ) + self._assert_expected_item(eval(self.output.getvalue())) class TestPprintItemExporterDataclass(TestPprintItemExporter): diff --git a/tests/test_extension_periodic_log.py b/tests/test_extension_periodic_log.py index 4f368d5ec..0e2d4be28 100644 --- a/tests/test_extension_periodic_log.py +++ b/tests/test_extension_periodic_log.py @@ -122,8 +122,10 @@ class TestPeriodicLog: # include multiple check( {"PERIODIC_LOG_DELTA": {"include": ["downloader/", "scheduler/"]}}, - lambda k, v: isinstance(v, (int, float)) - and ("downloader/" in k or "scheduler/" in k), + lambda k, v: ( + isinstance(v, (int, float)) + and ("downloader/" in k or "scheduler/" in k) + ), ) # exclude @@ -135,15 +137,19 @@ class TestPeriodicLog: # exclude multiple check( {"PERIODIC_LOG_DELTA": {"exclude": ["downloader/", "scheduler/"]}}, - lambda k, v: isinstance(v, (int, float)) - and ("downloader/" not in k and "scheduler/" not in k), + lambda k, v: ( + isinstance(v, (int, float)) + and ("downloader/" not in k and "scheduler/" not in k) + ), ) # include exclude combined check( {"PERIODIC_LOG_DELTA": {"include": ["downloader/"], "exclude": ["bytes"]}}, - lambda k, v: isinstance(v, (int, float)) - and ("downloader/" in k and "bytes" not in k), + lambda k, v: ( + isinstance(v, (int, float)) + and ("downloader/" in k and "bytes" not in k) + ), ) @pytest.mark.requires_reactor # needs a reactor or an event loop for PeriodicLog.task diff --git a/tests/test_item.py b/tests/test_item.py index 94742bfed..4eb37a344 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -60,7 +60,7 @@ class TestItem: assert itemrepr == "{'name': 'John Doe', 'number': 123}" - i2 = eval(itemrepr) # pylint: disable=eval-used + i2 = eval(itemrepr) assert i2["name"] == "John Doe" assert i2["number"] == 123 diff --git a/tests/test_link.py b/tests/test_link.py index f96961075..c49e5c090 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -49,7 +49,7 @@ class TestLink: l1 = Link( "http://www.example.com", text="test", fragment="something", nofollow=True ) - l2 = eval(repr(l1)) # pylint: disable=eval-used + l2 = eval(repr(l1)) self._assert_same_links(l1, l2) def test_bytes_url(self): diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 7f5060701..57736e0ea 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -438,7 +438,7 @@ class TestFilesPipelineCustomSettings: """ pipe_cls = self._generate_fake_pipeline() pipe = pipe_cls.from_crawler(get_crawler(None, {"FILES_STORE": tmp_path})) - for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: + for pipe_attr, _, pipe_ins_attr in self.file_cls_attr_settings_map: custom_value = getattr(pipe, pipe_ins_attr) assert custom_value != self.default_cls_settings[pipe_attr] assert getattr(pipe, pipe_ins_attr) == getattr(pipe, pipe_attr) @@ -469,7 +469,7 @@ class TestFilesPipelineCustomSettings: user_pipeline = UserDefinedFilesPipeline.from_crawler( get_crawler(None, {"FILES_STORE": tmp_path}) ) - for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: + for pipe_attr, _, pipe_ins_attr in self.file_cls_attr_settings_map: # Values from settings for custom pipeline should be set on pipeline instance. custom_value = self.default_cls_settings.get(pipe_attr.upper()) assert getattr(user_pipeline, pipe_ins_attr) == custom_value @@ -541,7 +541,7 @@ class TestFilesPipelineCustomSettings: pipeline_cls = UserPipe.from_crawler(get_crawler(None, settings)) - for pipe_attr, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map: + for _, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map: expected_value = settings.get(settings_attr) assert getattr(pipeline_cls, pipe_inst_attr) == expected_value diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 8108f7000..199ec5afa 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -435,7 +435,7 @@ class TestImagesPipelineCustomSettings: pipeline = pipeline_cls.from_crawler( get_crawler(None, {"IMAGES_STORE": tmp_path}) ) - for pipe_attr, settings_attr in self.img_cls_attribute_names: + for pipe_attr, _ in self.img_cls_attribute_names: # Instance attribute (lowercase) must be equal to class attribute (uppercase). attr_value = getattr(pipeline, pipe_attr.lower()) assert attr_value != self.default_pipeline_settings[pipe_attr] @@ -469,7 +469,7 @@ class TestImagesPipelineCustomSettings: user_pipeline = UserDefinedImagePipeline.from_crawler( get_crawler(None, {"IMAGES_STORE": tmp_path}) ) - for pipe_attr, settings_attr in self.img_cls_attribute_names: + for pipe_attr, _ in self.img_cls_attribute_names: # Values from settings for custom pipeline should be set on pipeline instance. custom_value = self.default_pipeline_settings.get(pipe_attr.upper()) assert getattr(user_pipeline, pipe_attr.lower()) == custom_value diff --git a/tests/test_utils_misc/test_return_with_argument_inside_generator.py b/tests/test_utils_misc/test_return_with_argument_inside_generator.py index ad31e5185..3783416b9 100644 --- a/tests/test_utils_misc/test_return_with_argument_inside_generator.py +++ b/tests/test_utils_misc/test_return_with_argument_inside_generator.py @@ -185,7 +185,7 @@ https://example.org warn_on_generator_with_return_value(mock_spider, l2) assert len(w) == 0 - def test_generators_return_none_with_decorator(self, mock_spider): + def test_generators_return_none_with_decorator(self, mock_spider): # noqa: PLR0915 def decorator(func): def inner_func(): func() diff --git a/tests/test_utils_spider.py b/tests/test_utils_spider.py index 5efb29a49..05971f423 100644 --- a/tests/test_utils_spider.py +++ b/tests/test_utils_spider.py @@ -24,7 +24,7 @@ def test_iterate_spider_output(): def test_iter_spider_classes(): - import tests.test_utils_spider # noqa: PLW0406,PLC0415 # pylint: disable=import-self + import tests.test_utils_spider # noqa: PLW0406,PLC0415 it = iter_spider_classes(tests.test_utils_spider) assert set(it) == {MySpider1, MySpider2} diff --git a/tests/test_webclient.py b/tests/test_webclient.py index 0ad6bd9d1..7834e3b31 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -48,8 +48,8 @@ def getPage(url, contextFactory=None, response_transform=None, *args, **kwargs): return _makeGetterFactory( to_bytes(url), _clientfactory, - contextFactory=contextFactory, *args, + contextFactory=contextFactory, **kwargs, ).deferred From b358e813f7faeed2e6617a362628e97c43b4d388 Mon Sep 17 00:00:00 2001 From: Lia Launtz Date: Thu, 26 Feb 2026 13:42:03 -0500 Subject: [PATCH 5/8] not necessary for branch - Revert "Bump ruff, fix some of rules (#7277)" This reverts commit 353135cf10b69305a7881c093d4563e2699dd80c. --- .pre-commit-config.yaml | 2 +- pyproject.toml | 41 +++++++++++-------- scrapy/core/downloader/__init__.py | 4 +- scrapy/core/downloader/contextfactory.py | 6 +-- scrapy/core/downloader/handlers/file.py | 2 +- scrapy/core/engine.py | 4 +- scrapy/core/http2/agent.py | 4 +- scrapy/core/spidermw.py | 2 +- scrapy/http/request/__init__.py | 10 ++--- scrapy/linkextractors/lxmlhtml.py | 2 +- scrapy/responsetypes.py | 2 +- scrapy/shell.py | 1 + scrapy/spidermiddlewares/referer.py | 2 +- scrapy/utils/defer.py | 2 +- scrapy/utils/engine.py | 2 +- scrapy/utils/log.py | 4 +- scrapy/utils/reactor.py | 2 +- scrapy/utils/signal.py | 6 +-- tests/mockserver/http_resources.py | 2 +- tests/spiders.py | 2 +- tests/test_downloadermiddleware_retry.py | 2 +- tests/test_exporters.py | 4 +- tests/test_extension_periodic_log.py | 18 +++----- tests/test_item.py | 2 +- tests/test_link.py | 2 +- tests/test_pipeline_files.py | 6 +-- tests/test_pipeline_images.py | 4 +- ...t_return_with_argument_inside_generator.py | 2 +- tests/test_utils_spider.py | 2 +- tests/test_webclient.py | 2 +- 30 files changed, 76 insertions(+), 70 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a2bf1be0c..dc4276cff 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,7 +6,7 @@ exclude: | ) repos: - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.15.2 + rev: v0.14.2 hooks: - id: ruff-check args: [ --fix ] diff --git a/pyproject.toml b/pyproject.toml index e4fae8bf4..7cb83c483 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -174,6 +174,7 @@ disable = [ "disallowed-name", "duplicate-code", # https://github.com/pylint-dev/pylint/issues/214 "fixme", + "import-outside-toplevel", "inherit-non-class", # false positives with create_deprecated_class() "invalid-name", "invalid-overridden-method", @@ -187,10 +188,12 @@ disable = [ "no-value-for-parameter", # https://github.com/pylint-dev/pylint/issues/3268 "not-callable", "protected-access", + "redefined-builtin", "redefined-outer-name", "too-few-public-methods", "too-many-ancestors", "too-many-arguments", + "too-many-branches", "too-many-function-args", "too-many-instance-attributes", "too-many-lines", @@ -199,22 +202,12 @@ disable = [ "too-many-public-methods", "too-many-return-statements", "unused-argument", + "unused-import", "unused-variable", "useless-import-alias", # used as a hint to mypy "useless-return", # https://github.com/pylint-dev/pylint/issues/6530 "wrong-import-position", - # Ones that are implemented in ruff and need to be disabled for some lines (listed here to avoid two disable comments) - "bare-except", - "eval-used", - "global-statement", - "import-outside-toplevel", - "import-self", - "inconsistent-return-statements", - "redefined-builtin", - "too-many-branches", - "unused-import", - # Ones that we may want to address (fix, ignore per-line or move to "don't want to fix") "abstract-method", "arguments-differ", @@ -352,22 +345,34 @@ ignore = [ "D403", # `try`-`except` within a loop incurs performance overhead "PERF203", + # Import alias does not rename original package + "PLC0414", # Too many return statements "PLR0911", + # Too many branches + "PLR0912", # Too many arguments in function definition "PLR0913", + # Too many statements + "PLR0915", # Magic value used in comparison "PLR2004", + # `for` loop variable overwritten by assignment target + "PLW2901", # String contains ambiguous {}. "RUF001", # Docstring contains ambiguous {}. "RUF002", # Comment contains ambiguous {}. "RUF003", + # Mutable class attributes should be annotated with `typing.ClassVar` + "RUF012", # Use of `assert` detected; needed for mypy "S101", # FTP-related functions are being called; https://github.com/scrapy/scrapy/issues/4180 "S321", + # Argument default set to insecure SSL protocol + "S503", # Use a context manager for opening files "SIM115", # Yoda condition detected @@ -379,16 +384,18 @@ ignore = [ "B003", # Do not use mutable data structures for argument defaults. "B006", + # Loop control variable not used within the loop body. + "B007", + # Do not perform function calls in argument defaults. + "B008", # Found useless expression. "B018", + # Star-arg unpacking after a keyword argument is strongly discouraged. + "B026", # No explicit stacklevel argument found. "B028", # Within an `except` clause, raise exceptions with `raise ... from` "B904", - # `for` loop variable overwritten by assignment target - "PLW2901", - # Mutable class attributes should be annotated with `typing.ClassVar` - "RUF012", # Use capitalized environment variable "SIM112", ] @@ -410,8 +417,8 @@ split-on-trailing-comma = false "scrapy/linkextractors/__init__.py" = ["E402"] "scrapy/spiders/__init__.py" = ["E402"] -# Skip bandit and allow blocking file I/O in tests -"tests/**" = ["ASYNC240", "S"] +# Skip bandit in tests +"tests/**" = ["S"] # Issues pending a review: "docs/conf.py" = ["E402"] diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 59871fdc3..c862ecdf9 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -157,7 +157,9 @@ class Downloader: if key not in self.slots: assert self.crawler.spider slot_settings = self.per_slot_settings.get(key, {}) - conc = self.ip_concurrency or self.domain_concurrency + conc = ( + self.ip_concurrency if self.ip_concurrency else self.domain_concurrency + ) conc, delay = _get_concurrency_delay( conc, self.crawler.spider, self.settings ) diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index 252f509bf..5f7c9a8c3 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -49,7 +49,7 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): def __init__( self, - method: int = SSL.SSLv23_METHOD, # noqa: S503 + method: int = SSL.SSLv23_METHOD, tls_verbose_logging: bool = False, tls_ciphers: str | None = None, *args: Any, @@ -75,7 +75,7 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): def from_crawler( cls, crawler: Crawler, - method: int = SSL.SSLv23_METHOD, # noqa: S503 + method: int = SSL.SSLv23_METHOD, *args: Any, **kwargs: Any, ) -> Self: @@ -84,10 +84,10 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): ) tls_ciphers: str | None = crawler.settings["DOWNLOADER_CLIENT_TLS_CIPHERS"] return cls( # type: ignore[misc] - *args, method=method, tls_verbose_logging=tls_verbose_logging, tls_ciphers=tls_ciphers, + *args, **kwargs, ) diff --git a/scrapy/core/downloader/handlers/file.py b/scrapy/core/downloader/handlers/file.py index a59aa722b..21fd2c353 100644 --- a/scrapy/core/downloader/handlers/file.py +++ b/scrapy/core/downloader/handlers/file.py @@ -16,6 +16,6 @@ if TYPE_CHECKING: class FileDownloadHandler(BaseDownloadHandler): async def download_request(self, request: Request) -> Response: filepath = file_uri_to_path(request.url) - body = Path(filepath).read_bytes() # noqa: ASYNC240 + body = Path(filepath).read_bytes() respcls = responsetypes.from_args(filename=filepath, body=body) return respcls(url=request.url, body=body) diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 1555fc0d6..93fc64cdc 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -439,7 +439,7 @@ class ExecutionEngine: spider=self.spider, dont_log=IgnoreRequest, ) - for _, result in request_scheduled_result: + for handler, result in request_scheduled_result: if isinstance(result, Failure) and isinstance(result.value, IgnoreRequest): return if not self._slot.scheduler.enqueue_request(request): # type: ignore[union-attr] @@ -587,7 +587,7 @@ class ExecutionEngine: ) return deferred_from_coro(self.close_spider_async(reason=reason)) - async def close_spider_async(self, *, reason: str = "cancelled") -> None: # noqa: PLR0912 + async def close_spider_async(self, *, reason: str = "cancelled") -> None: """Close (cancel) spider and clear all its outstanding requests. .. versionadded:: 2.14 diff --git a/scrapy/core/http2/agent.py b/scrapy/core/http2/agent.py index 8760b13ec..45f32daaa 100644 --- a/scrapy/core/http2/agent.py +++ b/scrapy/core/http2/agent.py @@ -120,7 +120,7 @@ class H2Agent: self, reactor: ReactorBase, pool: H2ConnectionPool, - context_factory: BrowserLikePolicyForHTTPS = BrowserLikePolicyForHTTPS(), # noqa: B008 + context_factory: BrowserLikePolicyForHTTPS = BrowserLikePolicyForHTTPS(), connect_timeout: float | None = None, bind_address: bytes | None = None, ) -> None: @@ -164,7 +164,7 @@ class ScrapyProxyH2Agent(H2Agent): reactor: ReactorBase, proxy_uri: URI, pool: H2ConnectionPool, - context_factory: BrowserLikePolicyForHTTPS = BrowserLikePolicyForHTTPS(), # noqa: B008 + context_factory: BrowserLikePolicyForHTTPS = BrowserLikePolicyForHTTPS(), connect_timeout: float | None = None, bind_address: bytes | None = None, ) -> None: diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index 476d2e504..6eaf84642 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -259,7 +259,7 @@ class SpiderMiddlewareManager(MiddlewareManager): # being available immediately which doesn't work when it's a wrapped coroutine. # It also needs @inlineCallbacks only because of downgrading so it can be removed when downgrading is removed. @inlineCallbacks - def _process_spider_output( # noqa: PLR0912 + def _process_spider_output( self, response: Response, result: Iterable[_T] | AsyncIterator[_T], diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index c4a0c8131..61e50927d 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -200,7 +200,7 @@ class Request(object_ref): #: .. seealso:: :ref:`topics-request-response-ref-errbacks` self.errback: Callable[[Failure], Any] | None = errback - self._cookies: CookiesT | None = cookies or None + self._cookies: CookiesT | None = cookies if cookies else None self._headers: Headers | None = ( Headers(headers, encoding=encoding) if headers else None ) @@ -244,7 +244,7 @@ class Request(object_ref): @cb_kwargs.setter def cb_kwargs(self, value: dict[str, Any] | None) -> None: - self._cb_kwargs = value or None + self._cb_kwargs = value if value else None @property def meta(self) -> dict[str, Any]: @@ -254,7 +254,7 @@ class Request(object_ref): @meta.setter def meta(self, value: dict[str, Any] | None) -> None: - self._meta = value or None + self._meta = value if value else None @property def url(self) -> str: @@ -292,7 +292,7 @@ class Request(object_ref): @flags.setter def flags(self, value: list[str] | None) -> None: - self._flags = value or None + self._flags = value if value else None @property def cookies(self) -> CookiesT: @@ -302,7 +302,7 @@ class Request(object_ref): @cookies.setter def cookies(self, value: CookiesT | None) -> None: - self._cookies = value or None + self._cookies = value if value else None @property def headers(self) -> Headers: diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 96a0f523b..0ea78e35a 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -110,7 +110,7 @@ class LxmlParserLinkExtractor: ) -> list[Link]: links: list[Link] = [] # hacky way to get the underlying lxml parsed document - for el, _, attr_val in self._iter_links(selector.root): + for el, attr, attr_val in self._iter_links(selector.root): # pseudo lxml.html.HtmlElement.make_links_absolute(base_url) try: if self.strip: diff --git a/scrapy/responsetypes.py b/scrapy/responsetypes.py index 3aaf17b53..3f6f030a5 100644 --- a/scrapy/responsetypes.py +++ b/scrapy/responsetypes.py @@ -54,7 +54,7 @@ class ResponseTypes: return Response if mimetype in self.classes: return self.classes[mimetype] - basetype = f"{mimetype.split('/', maxsplit=1)[0]}/*" + basetype = f"{mimetype.split('/')[0]}/*" return self.classes.get(basetype, Response) def from_content_type( diff --git a/scrapy/shell.py b/scrapy/shell.py index 00097d224..4b2bdf6cf 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -73,6 +73,7 @@ class Shell: else: self.populate_vars() if self.code: + # pylint: disable-next=eval-used print(eval(self.code, globals(), self.vars)) # noqa: S307 else: # Detect interactive shell setting in scrapy.cfg diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index ac340324a..e4d0f4014 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -97,7 +97,7 @@ class ReferrerPolicy(ABC): def potentially_trustworthy(self, url: str) -> bool: # Note: this does not follow https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy parsed_url = urlparse(url) - if parsed_url.scheme == "data": + if parsed_url.scheme in ("data",): return False return self.tls_protected(url) diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 0d937fea5..5bd4506d6 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -435,7 +435,7 @@ def _maybeDeferred_coro( """Copy of defer.maybeDeferred that also converts coroutines to Deferreds.""" try: result = f(*args, **kw) - except: # noqa: E722 + except: # noqa: E722 # pylint: disable=bare-except return fail(failure.Failure(captureVars=Deferred.debug)) # when the deprecation period has ended we need to make sure the behavior diff --git a/scrapy/utils/engine.py b/scrapy/utils/engine.py index 085720f66..1e0c53212 100644 --- a/scrapy/utils/engine.py +++ b/scrapy/utils/engine.py @@ -32,7 +32,7 @@ def get_engine_status(engine: ExecutionEngine) -> list[tuple[str, Any]]: checks: list[tuple[str, Any]] = [] for test in tests: try: - checks += [(test, eval(test))] # noqa: S307 + checks += [(test, eval(test))] # noqa: S307 # pylint: disable=eval-used except Exception as e: checks += [(test, f"{type(e).__name__} (exception)")] diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 5f0d19913..07a9e5ff7 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -138,7 +138,7 @@ _scrapy_root_handler: logging.Handler | None = None def install_scrapy_root_handler(settings: Settings) -> None: - global _scrapy_root_handler # noqa: PLW0603 + global _scrapy_root_handler # noqa: PLW0603 # pylint: disable=global-statement _uninstall_scrapy_root_handler() logging.root.setLevel(logging.NOTSET) @@ -147,7 +147,7 @@ def install_scrapy_root_handler(settings: Settings) -> None: def _uninstall_scrapy_root_handler() -> None: - global _scrapy_root_handler # noqa: PLW0603 + global _scrapy_root_handler # noqa: PLW0603 # pylint: disable=global-statement if ( _scrapy_root_handler is not None diff --git a/scrapy/utils/reactor.py b/scrapy/utils/reactor.py index 6aa72064b..f37c48a7d 100644 --- a/scrapy/utils/reactor.py +++ b/scrapy/utils/reactor.py @@ -26,7 +26,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: # type: ignore[return] # pylint: disable=inconsistent-return-statements # noqa: RET503 """Like reactor.listenTCP but tries different ports in a range.""" from twisted.internet import reactor diff --git a/scrapy/utils/signal.py b/scrapy/utils/signal.py index 997e8eba8..1f7426e59 100644 --- a/scrapy/utils/signal.py +++ b/scrapy/utils/signal.py @@ -50,7 +50,7 @@ def send_catch_log( result: TypingAny try: response = robustApply( - receiver, *arguments, signal=signal, sender=sender, **named + receiver, signal=signal, sender=sender, *arguments, **named ) if isinstance(response, Deferred): logger.error( @@ -118,9 +118,9 @@ def _send_catch_log_deferred( robustApply, True, receiver, - *arguments, signal=signal, sender=sender, + *arguments, **named, ) d.addErrback(logerror, receiver) @@ -190,7 +190,7 @@ async def _send_catch_log_asyncio( try: result = await ensure_awaitable( robustApply( - receiver, *arguments, signal=signal, sender=sender, **named + receiver, signal=signal, sender=sender, *arguments, **named ), _warn=global_object_name(receiver), ) diff --git a/tests/mockserver/http_resources.py b/tests/mockserver/http_resources.py index 91577aa6b..62a3146bf 100644 --- a/tests/mockserver/http_resources.py +++ b/tests/mockserver/http_resources.py @@ -282,7 +282,7 @@ class LargeChunkedFileResource(resource.Resource): from twisted.internet import reactor def response(): - for _ in range(1024): + for i in range(1024): request.write(b"x" * 1024) request.finish() diff --git a/tests/spiders.py b/tests/spiders.py index 363a09fad..79565738d 100644 --- a/tests/spiders.py +++ b/tests/spiders.py @@ -389,7 +389,7 @@ class DuplicateStartSpider(MockServerSpider): async def start(self): for i in range(self.distinct_urls): - for _ in range(self.dupe_factor): + for j in range(self.dupe_factor): url = self.mockserver.url(f"/echo?headers=1&body=test{i}") yield Request(url, dont_filter=self.dont_filter) diff --git a/tests/test_downloadermiddleware_retry.py b/tests/test_downloadermiddleware_retry.py index 50946899a..20e8fc407 100644 --- a/tests/test_downloadermiddleware_retry.py +++ b/tests/test_downloadermiddleware_retry.py @@ -250,7 +250,7 @@ class TestMaxRetryTimes: ): middleware = middleware or self.mw - for _ in range(max_retry_times): + for i in range(max_retry_times): req = middleware.process_exception(req, exception) assert isinstance(req, Request) diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 2fded613d..5719d5bb0 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -195,7 +195,9 @@ class TestPprintItemExporter(TestBaseItemExporter): return PprintItemExporter(self.output, **kwargs) def _check_output(self): - self._assert_expected_item(eval(self.output.getvalue())) + self._assert_expected_item( + eval(self.output.getvalue()) # pylint: disable=eval-used + ) class TestPprintItemExporterDataclass(TestPprintItemExporter): diff --git a/tests/test_extension_periodic_log.py b/tests/test_extension_periodic_log.py index 0e2d4be28..4f368d5ec 100644 --- a/tests/test_extension_periodic_log.py +++ b/tests/test_extension_periodic_log.py @@ -122,10 +122,8 @@ class TestPeriodicLog: # include multiple check( {"PERIODIC_LOG_DELTA": {"include": ["downloader/", "scheduler/"]}}, - lambda k, v: ( - isinstance(v, (int, float)) - and ("downloader/" in k or "scheduler/" in k) - ), + lambda k, v: isinstance(v, (int, float)) + and ("downloader/" in k or "scheduler/" in k), ) # exclude @@ -137,19 +135,15 @@ class TestPeriodicLog: # exclude multiple check( {"PERIODIC_LOG_DELTA": {"exclude": ["downloader/", "scheduler/"]}}, - lambda k, v: ( - isinstance(v, (int, float)) - and ("downloader/" not in k and "scheduler/" not in k) - ), + lambda k, v: isinstance(v, (int, float)) + and ("downloader/" not in k and "scheduler/" not in k), ) # include exclude combined check( {"PERIODIC_LOG_DELTA": {"include": ["downloader/"], "exclude": ["bytes"]}}, - lambda k, v: ( - isinstance(v, (int, float)) - and ("downloader/" in k and "bytes" not in k) - ), + lambda k, v: isinstance(v, (int, float)) + and ("downloader/" in k and "bytes" not in k), ) @pytest.mark.requires_reactor # needs a reactor or an event loop for PeriodicLog.task diff --git a/tests/test_item.py b/tests/test_item.py index 4eb37a344..94742bfed 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -60,7 +60,7 @@ class TestItem: assert itemrepr == "{'name': 'John Doe', 'number': 123}" - i2 = eval(itemrepr) + i2 = eval(itemrepr) # pylint: disable=eval-used assert i2["name"] == "John Doe" assert i2["number"] == 123 diff --git a/tests/test_link.py b/tests/test_link.py index c49e5c090..f96961075 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -49,7 +49,7 @@ class TestLink: l1 = Link( "http://www.example.com", text="test", fragment="something", nofollow=True ) - l2 = eval(repr(l1)) + l2 = eval(repr(l1)) # pylint: disable=eval-used self._assert_same_links(l1, l2) def test_bytes_url(self): diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 57736e0ea..7f5060701 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -438,7 +438,7 @@ class TestFilesPipelineCustomSettings: """ pipe_cls = self._generate_fake_pipeline() pipe = pipe_cls.from_crawler(get_crawler(None, {"FILES_STORE": tmp_path})) - for pipe_attr, _, pipe_ins_attr in self.file_cls_attr_settings_map: + for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: custom_value = getattr(pipe, pipe_ins_attr) assert custom_value != self.default_cls_settings[pipe_attr] assert getattr(pipe, pipe_ins_attr) == getattr(pipe, pipe_attr) @@ -469,7 +469,7 @@ class TestFilesPipelineCustomSettings: user_pipeline = UserDefinedFilesPipeline.from_crawler( get_crawler(None, {"FILES_STORE": tmp_path}) ) - for pipe_attr, _, pipe_ins_attr in self.file_cls_attr_settings_map: + for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map: # Values from settings for custom pipeline should be set on pipeline instance. custom_value = self.default_cls_settings.get(pipe_attr.upper()) assert getattr(user_pipeline, pipe_ins_attr) == custom_value @@ -541,7 +541,7 @@ class TestFilesPipelineCustomSettings: pipeline_cls = UserPipe.from_crawler(get_crawler(None, settings)) - for _, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map: + for pipe_attr, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map: expected_value = settings.get(settings_attr) assert getattr(pipeline_cls, pipe_inst_attr) == expected_value diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 199ec5afa..8108f7000 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -435,7 +435,7 @@ class TestImagesPipelineCustomSettings: pipeline = pipeline_cls.from_crawler( get_crawler(None, {"IMAGES_STORE": tmp_path}) ) - for pipe_attr, _ in self.img_cls_attribute_names: + for pipe_attr, settings_attr in self.img_cls_attribute_names: # Instance attribute (lowercase) must be equal to class attribute (uppercase). attr_value = getattr(pipeline, pipe_attr.lower()) assert attr_value != self.default_pipeline_settings[pipe_attr] @@ -469,7 +469,7 @@ class TestImagesPipelineCustomSettings: user_pipeline = UserDefinedImagePipeline.from_crawler( get_crawler(None, {"IMAGES_STORE": tmp_path}) ) - for pipe_attr, _ in self.img_cls_attribute_names: + for pipe_attr, settings_attr in self.img_cls_attribute_names: # Values from settings for custom pipeline should be set on pipeline instance. custom_value = self.default_pipeline_settings.get(pipe_attr.upper()) assert getattr(user_pipeline, pipe_attr.lower()) == custom_value diff --git a/tests/test_utils_misc/test_return_with_argument_inside_generator.py b/tests/test_utils_misc/test_return_with_argument_inside_generator.py index 3783416b9..ad31e5185 100644 --- a/tests/test_utils_misc/test_return_with_argument_inside_generator.py +++ b/tests/test_utils_misc/test_return_with_argument_inside_generator.py @@ -185,7 +185,7 @@ https://example.org warn_on_generator_with_return_value(mock_spider, l2) assert len(w) == 0 - def test_generators_return_none_with_decorator(self, mock_spider): # noqa: PLR0915 + def test_generators_return_none_with_decorator(self, mock_spider): def decorator(func): def inner_func(): func() diff --git a/tests/test_utils_spider.py b/tests/test_utils_spider.py index 05971f423..5efb29a49 100644 --- a/tests/test_utils_spider.py +++ b/tests/test_utils_spider.py @@ -24,7 +24,7 @@ def test_iterate_spider_output(): def test_iter_spider_classes(): - import tests.test_utils_spider # noqa: PLW0406,PLC0415 + import tests.test_utils_spider # noqa: PLW0406,PLC0415 # pylint: disable=import-self it = iter_spider_classes(tests.test_utils_spider) assert set(it) == {MySpider1, MySpider2} diff --git a/tests/test_webclient.py b/tests/test_webclient.py index 7834e3b31..0ad6bd9d1 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -48,8 +48,8 @@ def getPage(url, contextFactory=None, response_transform=None, *args, **kwargs): return _makeGetterFactory( to_bytes(url), _clientfactory, - *args, contextFactory=contextFactory, + *args, **kwargs, ).deferred From 2e708278f1994dd7a347ce2a674069fa093eac35 Mon Sep 17 00:00:00 2001 From: Lia Launtz Date: Thu, 26 Feb 2026 13:43:24 -0500 Subject: [PATCH 6/8] not necessary for branch - Revert "Pass correct env to PopenSpawn() in tests. (#7279)" This reverts commit a9a36c6262788500074fde053263c5248d293222. --- tests/test_crawler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 9ade8472f..ba33d0bbe 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -951,7 +951,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): def test_shutdown_graceful(self): sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK args = self.get_script_args("sleeping.py", "3") - p = PopenSpawn(args, timeout=5, env=get_script_run_env()) + p = PopenSpawn(args, timeout=5) p.expect_exact("Spider opened") p.expect_exact("Crawled (200)") p.kill(sig) @@ -963,7 +963,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): def test_shutdown_forced(self): sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK args = self.get_script_args("sleeping.py", "10") - p = PopenSpawn(args, timeout=5, env=get_script_run_env()) + p = PopenSpawn(args, timeout=5) p.expect_exact("Spider opened") p.expect_exact("Crawled (200)") p.kill(sig) From fbb4aedf4e51763b5a2a416144a5e8e6b0cc096f Mon Sep 17 00:00:00 2001 From: Lia Launtz Date: Thu, 26 Feb 2026 13:58:33 -0500 Subject: [PATCH 7/8] removed edit help syntax test --- tests/test_command_edit.py | 93 ++++++++++++++++++++++++++++++++++---- 1 file changed, 85 insertions(+), 8 deletions(-) diff --git a/tests/test_command_edit.py b/tests/test_command_edit.py index 1a040912e..c4160c201 100644 --- a/tests/test_command_edit.py +++ b/tests/test_command_edit.py @@ -1,10 +1,15 @@ from __future__ import annotations +import os +from contextlib import suppress from pathlib import Path +from unittest import mock import pytest +from scrapy.cmdline import ScrapyArgumentParser from scrapy.commands.edit import Command +from scrapy.utils.project import get_project_settings from tests.test_commands import TestProjectBase from tests.utils.cmdline import call, proc @@ -15,14 +20,45 @@ class TestEditCommand(TestProjectBase): @pytest.fixture def create_spider(self, proj_path: Path): """creates spider needed for tests""" + # setup: add "cat" as test environment editor + try: + editor_to_restore = os.environ["EDITOR"] + except KeyError: + editor_to_restore = None + finally: + os.environ["EDITOR"] = "cat" + + # setup: preserve scrapy settings in local environment + try: + scrapy_settings_to_restore = os.environ["SCRAPY_SETTINGS_MODULE"] + except KeyError: + scrapy_settings_to_restore = None + # setup: create spider to edit test_name = "test_name" call("genspider", test_name, "test.com", cwd=proj_path) spider = proj_path / self.project_name / "spiders" / "test_name.py" yield proj_path, spider, test_name + # teardown: remove spider from project Path.unlink(spider) + # teardown: restore previous editor + if editor_to_restore is not None: + os.environ["EDITOR"] = editor_to_restore + else: + # remove editor from os.environ if it exists + with suppress(KeyError): + os.environ.pop("EDITOR") + + # teardown: restore project settings in local environment + if scrapy_settings_to_restore is not None: + os.environ["SCRAPY_SETTINGS_MODULE"] = scrapy_settings_to_restore + else: + # remove "SCRAPY_SETTINGS_MODULE" from os.environ if it exists + with suppress(KeyError): + os.environ.pop("SCRAPY_SETTINGS_MODULE") + def test_edit_valid_spider(self, create_spider) -> None: """test call to edit command with correct spider name""" proj_path, spider, test_name = create_spider @@ -33,17 +69,58 @@ class TestEditCommand(TestProjectBase): """test call to edit if no spider has been specified""" assert call("edit", "not_a_valid_spider", cwd=proj_path) == 1 - def test_edit_help_syntax(self, proj_path: Path) -> None: - """Check that long description and syntax are included in edit -h""" - rtn_code, out, _ = proc("edit", "-h", cwd=proj_path) - assert rtn_code == 0 - cmd = Command() - assert cmd.long_desc() in out - assert cmd.syntax() in out - def test_edit_short_desc(self, proj_path: Path) -> None: """Check that short description included in scrapy -h""" rtn_code, out, _ = proc("-h", cwd=proj_path) assert rtn_code == 0 cmd = Command() assert cmd.short_desc() in out + + def test_edit_command_valid_directory(self, create_spider): + """calls editor command directly from project directory""" + proj_path, spider, test_name = create_spider + + # change into cwd + current = Path.cwd() + os.chdir(proj_path) + + # create edit command object + # teardown required as get_project_settings() mutates os.environ + cmd = Command() + cmd.settings = get_project_settings() + # grabs system editor to mock + editor = cmd.settings.get("EDITOR") + + # parse commandline arguments + parser = ScrapyArgumentParser() + opts, _ = parser.parse_known_args(["edit", test_name]) + + with mock.patch("scrapy.commands.edit.os.system", return_value=0) as mock_sys: + cmd.run([test_name], opts) + mock_sys.assert_called_once_with(f'{editor} "{spider}"') + + # move back into previous cwd + os.chdir(current) + + def test_edit_as_subprocess(self, create_spider): + """check that subprocess calls editor""" + proj_path, _, test_name = create_spider + + spider_text = """ +class TestNameSpider(scrapy.Spider): + name = "test_name" +""" + + _, out, _ = proc("edit", test_name, cwd=proj_path) + assert spider_text in out + + def test_edit_subprocess_no_project(self, create_spider): + """check that subprocess does not call editor outside project""" + proj_path, _, test_name = create_spider + + no_proj = """ +The edit command is not available from this location. +These commands are only available from within a project: check, crawl, edit, list, parse. +""" + _, out, _ = proc("edit", test_name, cwd=proj_path.parent) + assert no_proj in out From 74d62309d3bb917057427f42dc22f1c12dcc2f13 Mon Sep 17 00:00:00 2001 From: Lia Launtz Date: Thu, 26 Mar 2026 15:29:16 -0400 Subject: [PATCH 8/8] changes to fixture and edit_command_valid_directory --- tests/test_command_edit.py | 92 ++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 39 deletions(-) diff --git a/tests/test_command_edit.py b/tests/test_command_edit.py index c4160c201..da6b9a0a1 100644 --- a/tests/test_command_edit.py +++ b/tests/test_command_edit.py @@ -21,43 +21,44 @@ class TestEditCommand(TestProjectBase): def create_spider(self, proj_path: Path): """creates spider needed for tests""" # setup: add "cat" as test environment editor - try: + editor_to_restore = None + if "EDITOR" in os.environ: editor_to_restore = os.environ["EDITOR"] - except KeyError: - editor_to_restore = None - finally: - os.environ["EDITOR"] = "cat" + + os.environ["EDITOR"] = "cat" # setup: preserve scrapy settings in local environment - try: + scrapy_settings_to_restore = None + if "SCRAPY_SETTINGS_MODULE" in os.environ: scrapy_settings_to_restore = os.environ["SCRAPY_SETTINGS_MODULE"] - except KeyError: - scrapy_settings_to_restore = None # setup: create spider to edit test_name = "test_name" call("genspider", test_name, "test.com", cwd=proj_path) spider = proj_path / self.project_name / "spiders" / "test_name.py" - yield proj_path, spider, test_name - # teardown: remove spider from project - Path.unlink(spider) + try: + yield proj_path, spider, test_name - # teardown: restore previous editor - if editor_to_restore is not None: - os.environ["EDITOR"] = editor_to_restore - else: - # remove editor from os.environ if it exists - with suppress(KeyError): - os.environ.pop("EDITOR") + finally: + # teardown: remove spider from project + Path.unlink(spider) - # teardown: restore project settings in local environment - if scrapy_settings_to_restore is not None: - os.environ["SCRAPY_SETTINGS_MODULE"] = scrapy_settings_to_restore - else: - # remove "SCRAPY_SETTINGS_MODULE" from os.environ if it exists - with suppress(KeyError): - os.environ.pop("SCRAPY_SETTINGS_MODULE") + # teardown: restore previous editor + if editor_to_restore is not None: + os.environ["EDITOR"] = editor_to_restore + else: + # remove editor from os.environ if it exists + with suppress(KeyError): + os.environ.pop("EDITOR") + + # teardown: restore project settings in local environment + if scrapy_settings_to_restore is not None: + os.environ["SCRAPY_SETTINGS_MODULE"] = scrapy_settings_to_restore + else: + # remove "SCRAPY_SETTINGS_MODULE" from os.environ if it exists + with suppress(KeyError): + os.environ.pop("SCRAPY_SETTINGS_MODULE") def test_edit_valid_spider(self, create_spider) -> None: """test call to edit command with correct spider name""" @@ -79,28 +80,41 @@ class TestEditCommand(TestProjectBase): def test_edit_command_valid_directory(self, create_spider): """calls editor command directly from project directory""" proj_path, spider, test_name = create_spider + failures = [] # change into cwd current = Path.cwd() os.chdir(proj_path) - # create edit command object - # teardown required as get_project_settings() mutates os.environ - cmd = Command() - cmd.settings = get_project_settings() - # grabs system editor to mock - editor = cmd.settings.get("EDITOR") + try: + # create edit command object + # teardown required as get_project_settings() mutates os.environ + cmd = Command() + cmd.settings = get_project_settings() + # grabs system editor to mock + editor = cmd.settings.get("EDITOR") - # parse commandline arguments - parser = ScrapyArgumentParser() - opts, _ = parser.parse_known_args(["edit", test_name]) + # parse commandline arguments + parser = ScrapyArgumentParser() + opts, _ = parser.parse_known_args(["edit", test_name]) - with mock.patch("scrapy.commands.edit.os.system", return_value=0) as mock_sys: - cmd.run([test_name], opts) - mock_sys.assert_called_once_with(f'{editor} "{spider}"') + with mock.patch( + "scrapy.commands.edit.os.system", return_value=0 + ) as mock_sys: + cmd.run([test_name], opts) + mock_sys.assert_called_once_with(f'{editor} "{spider}"') - # move back into previous cwd - os.chdir(current) + # catch failure so we can restore cwd + except AssertionError as e: + failures.append(e) + + finally: + # restore previous cwd + os.chdir(current) + + # report test failure + if failures: + pytest.fail(f"{failures[0]}") def test_edit_as_subprocess(self, create_spider): """check that subprocess calls editor"""