Repository: scrapy/scrapy (python)

Issue:
Investigate off-by-1 in `scrapy.cmdline._pop_command_name()`
It looks like `del argv[i]` removes the wrong item in `scrapy.cmdline._pop_command_name()` but as we don't seem to see any problems because of this it's worth investigating what exactly happens here and either fixing or refactoring the code.

Pull request:
Fix _pop_command_name and include tests
This PR resolves #6600. I have also created a test file called test_cmdline_pop_command_name.py within tests/test_cmdline

Patch:
diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py
index 065adccfb..b08fd3409 100644
--- a/scrapy/cmdline.py
+++ b/scrapy/cmdline.py
@@ -96,10 +96,9 @@ def _get_project_only_cmds(settings: BaseSettings) -> set[str]:
 
 
 def _pop_command_name(argv: list[str]) -> str | None:
-    for i, arg in enumerate(argv[1:]):
-        if not arg.startswith("-"):
-            del argv[i]
-            return arg
+    for i in range(1, len(argv)):
+        if not argv[i].startswith("-"):
+            return argv.pop(i)
     return None
 
 


Test patch:
diff --git a/tests/test_commands.py b/tests/test_commands.py
index 1aae3222e..50f093043 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -23,7 +23,7 @@ from pytest import mark
 from twisted.trial import unittest
 
 import scrapy
-from scrapy.cmdline import _print_unknown_command_msg
+from scrapy.cmdline import _pop_command_name, _print_unknown_command_msg
 from scrapy.commands import ScrapyCommand, ScrapyHelpFormatter, view
 from scrapy.commands.startproject import IGNORE
 from scrapy.settings import Settings
@@ -1163,3 +1163,29 @@ class HelpMessageTest(CommandTest):
         for command in self.commands:
             _, out, _ = self.proc(command, "-h")
             self.assertIn("Usage", out)
+
+
+class PopCommandNameTest(unittest.TestCase):
+    def test_valid_command(self):
+        argv = ["scrapy", "crawl", "my_spider"]
+        command = _pop_command_name(argv)
+        self.assertEqual(command, "crawl")
+        self.assertEqual(argv, ["scrapy", "my_spider"])
+
+    def test_no_command(self):
+        argv = ["scrapy"]
+        command = _pop_command_name(argv)
+        self.assertIsNone(command)
+        self.assertEqual(argv, ["scrapy"])
+
+    def test_option_before_command(self):
+        argv = ["scrapy", "-h", "crawl"]
+        command = _pop_command_name(argv)
+        self.assertEqual(command, "crawl")
+        self.assertEqual(argv, ["scrapy", "-h"])
+
+    def test_option_after_command(self):
+        argv = ["scrapy", "crawl", "-h"]
+        command = _pop_command_name(argv)
+        self.assertEqual(command, "crawl")
+        self.assertEqual(argv, ["scrapy", "-h"])