[FIX] odev: parsing of unknown arguments in python 3.12.7+

Breaking change: passing unknown (odoo-bin) arguments before positional arguments is not supported in Python >= 3.12.7. In the same idea, it is also not supported anymore to pass known arguments after positional arguments. It is not planned to reimplement this feature in the later versions as it was taking advantage of a bug not compliant with common CLI rulings and RFCs. When in doubt, please refer to the command's help and follow the proposed order for arguments. See: python/cpython#59317
odoo-odev · Nov 6, 2024 · 07f0533 · 07f0533
1 parent 8d4eecd
commit 07f0533
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 32 deletions.
diff --git a/.github/workflows/odev.yml b/.github/workflows/odev.yml
@@ -102,7 +102,7 @@ jobs:
     runs-on: ${{ matrix.os }}-latest
 
     strategy:
-      fail-fast: true
+      fail-fast: false
       matrix:
         os:
           - ubuntu

diff --git a/odev/commands/database/run.py b/odev/commands/database/run.py
@@ -45,6 +45,6 @@ def run(self):
                 with progress.spinner(f"Reset database {self._database.name!r} from template {self._template.name!r}"):
                     self._database.drop()
 
-            self.odev.run_command("create", self._database.name, "--from-template", self._template.name)
+            self.odev.run_command("create", "--from-template", self._template.name, self._database.name)
 
         self.odoobin.run(args=self.args.odoo_args, progress=self.odoobin_progress)
diff --git a/odev/common/commands/base.py b/odev/common/commands/base.py
@@ -238,7 +238,11 @@ def prepare_arguments(cls, parser: ArgumentParser) -> None:
 
             if params.get("nargs") == "*...":
                 cls._unknown_arguments_dest = aliases[0]
-                params["nargs"] = "*"
+
+                # A bug in standard library argparse before python 3.12.7 causes `...` to not work as expected
+                # when used in conjunction with positional and optional arguments.
+                # See: https://github.com/python/cpython/issues/59317
+                params["nargs"] = "*" if sys.version_info < (3, 12, 7) else "..."
 
             if "action" in params:
                 params["action"] = ACTIONS_MAPPING.get(params["action"], params["action"])
@@ -281,7 +285,11 @@ def parse_arguments(cls, argv: Sequence[str]) -> Namespace:
                     arguments = parser.parse_args(argv)
                 else:
                     arguments, unknown = parser.parse_known_args(argv)
-                    setattr(arguments, cls._unknown_arguments_dest, unknown)
+                    setattr(
+                        arguments,
+                        cls._unknown_arguments_dest,
+                        getattr(arguments, cls._unknown_arguments_dest, []) + unknown,
+                    )
             except SystemExit as exception:
                 error_message = stderr.getvalue()
                 error = re.search(rf"{cls._name}: error: (.*)$", error_message, re.MULTILINE)

diff --git a/odev/common/commands/odoobin.py b/odev/common/commands/odoobin.py
@@ -279,26 +279,18 @@ class OdoobinTemplateCommand(OdoobinCommand):
     from_template = args.String(
         aliases=["-t", "--from-template"],
         description="Name of an existing PostgreSQL database to copy.",
-        nargs="?",
-        default="",
     )
 
     def infer_template_instance(self):
         """Infer the template database from the command line arguments."""
-
-        # Command called with --from-template and no value: append the suffix to the database name
-        if self.args.from_template is None:
-            template_name = self._database.name + TEMPLATE_SUFFIX
-            self._template = LocalDatabase(template_name)
-
-        # Command called with --from-template and a value: use the provided template as the base database
-        elif self.args.from_template:
+        if self.args.from_template:
             self._template = LocalDatabase(self.args.from_template)
 
             if not self._template.exists and not self._template.name.endswith(TEMPLATE_SUFFIX):
                 self._template = LocalDatabase(self.args.from_template + TEMPLATE_SUFFIX)
-
-        # Command called without --from-template: no template database
+        elif self.args.from_template is not None:
+            template_name = self._database.name + TEMPLATE_SUFFIX
+            self._template = LocalDatabase(template_name)
         else:
             self._template = None
 

diff --git a/tests/commands/test_database.py b/tests/commands/test_database.py
@@ -50,7 +50,7 @@ def test_01_cloc(self):
     def test_02_cloc_csv(self):
         """Command `odev cloc --csv` should display line of codes count by module in csv format."""
         with self.patch(ODOOBIN_PATH, "run", CompletedProcess(args=["cloc"], returncode=0, stdout=self.CLOC_RESULT)):
-            stdout, _ = self.dispatch_command("cloc", self.database.name, "--csv")
+            stdout, _ = self.dispatch_command("cloc", "--csv", self.database.name)
 
         self.assertIn("test_module_01", stdout)
         self.assertEqual(len(stdout.strip().splitlines()), 6)
@@ -96,7 +96,7 @@ def test_01_create_bare(self):
         self.assert_database(self.create_database_name, exists=False)
 
         with self.wrap("odev.common.bash", "stream") as stream:
-            self.dispatch_command("create", self.create_database_name, "--bare")
+            self.dispatch_command("create", "--bare", self.create_database_name)
             stream.assert_not_called()
 
         self.assert_database(self.create_database_name, exists=True, is_odoo=False)
@@ -107,7 +107,12 @@ def test_02_create_odoo(self):
 
         with self.wrap("odev.common.bash", "stream") as stream:
             stdout, _ = self.dispatch_command(
-                "create", self.create_database_name, "--version", "17.0", "--without-demo", "all"
+                "create",
+                "--version",
+                "17.0",
+                self.create_database_name,
+                "--without-demo",
+                "all",
             )
             stream.assert_called_with(
                 OdoobinMatch(
@@ -126,7 +131,12 @@ def test_03_create_from_template(self):
         self.assert_database(self.create_database_name, exists=False)
 
         with self.wrap("odev.common.bash", "stream") as stream:
-            self.dispatch_command("create", self.create_database_name, "--from-template", template_database.name)
+            self.dispatch_command(
+                "create",
+                "--from-template",
+                template_database.name,
+                self.create_database_name,
+            )
             stream.assert_not_called()
 
         self.assert_database(self.create_database_name, version=str(template_database.version))
@@ -138,7 +148,7 @@ def test_04_create_new_template(self):
         self.assert_database(self.template_database_name, exists=False)
 
         with self.wrap("odev.common.bash", "stream") as stream:
-            self.dispatch_command("create", self.create_database_name, "--create-template")
+            self.dispatch_command("create", "--create-template", self.create_database_name)
             stream.assert_not_called()
 
         self.assert_database(self.template_database_name, version=str(database.version))
@@ -151,7 +161,13 @@ def test_05_overwrite(self):
 
         with self.wrap("odev.common.bash", "stream") as stream:
             stdout, _ = self.dispatch_command(
-                "create", self.create_database_name, "--force", "--version", "16.0", "--without-demo", "all"
+                "create",
+                "--force",
+                "--version",
+                "16.0",
+                self.create_database_name,
+                "--without-demo",
+                "all",
             )
             stream.assert_called_with(
                 OdoobinMatch(
@@ -168,15 +184,29 @@ def test_06_with_venv(self):
         """Command `odev create` should create a new database with a specific virtual environment."""
         self.assert_database(self.create_database_name, is_odoo=False)
         self.dispatch_command(
-            "create", self.create_database_name, "--venv", self.venv.name, "--version", "17.0", "--without-demo", "all"
+            "create",
+            "--venv",
+            self.venv.name,
+            "--version",
+            "17.0",
+            self.create_database_name,
+            "--without-demo",
+            "all",
         )
         self.assert_database(self.create_database_name, is_odoo=True)
         self.assertEqual(LocalDatabase(self.create_database_name).venv.name, self.venv.name)
 
     def test_07_with_version(self):
         """Command `odev create` should create a new database with a specific Odoo version."""
         self.assert_database(self.create_database_name, is_odoo=False)
-        self.dispatch_command("create", self.create_database_name, "--version", "17.0", "--without-demo", "all")
+        self.dispatch_command(
+            "create",
+            "--version",
+            "17.0",
+            self.create_database_name,
+            "--without-demo",
+            "all",
+        )
         self.assert_database(self.create_database_name, is_odoo=True, version="17.0")
         database = LocalDatabase(self.create_database_name)
         self.assertEqual(database.venv.name, "17.0")
@@ -186,7 +216,14 @@ def test_08_with_worktree(self):
         """Command `odev create` should create a new database with a specific worktree."""
         self.assert_database(self.create_database_name, is_odoo=False)
         self.dispatch_command(
-            "create", self.create_database_name, "--worktree", "test", "--version", "17.0", "--without-demo", "all"
+            "create",
+            "--worktree",
+            "test",
+            "--version",
+            "17.0",
+            self.create_database_name,
+            "--without-demo",
+            "all",
         )
         self.assert_database(self.create_database_name, is_odoo=True)
         database = LocalDatabase(self.create_database_name)
@@ -200,7 +237,7 @@ class TestCommandDatabaseTest(OdevCommandTestCase):
     def test_01_test(self):
         """Command `odev test` should run tests on a database."""
         with self.wrap("odev.common.bash", "stream") as stream:
-            stdout, _ = self.dispatch_command("test", self.database.name, "--tags", ":TestSafeEval.test_expr")
+            stdout, _ = self.dispatch_command("test", "--tags", ":TestSafeEval.test_expr", self.database.name)
             stream.assert_called_with(
                 OdoobinMatch(
                     self.database.name,
@@ -278,15 +315,19 @@ def test_01_run(self):
     def test_02_run_from_template(self):
         """Command `odev run` should run Odoo in a database from a template."""
         self.assert_database(self.database.name, is_odoo=True)
-        self.dispatch_command("create", self.database.name, "--create-template")
+        self.dispatch_command("create", "--create-template", self.database.name)
         template_name = f"{self.database.name}:template"
         self.assert_database(template_name, is_odoo=True)
         self.database.query("CREATE TABLE test_table (id SERIAL PRIMARY KEY);")
         self.assertTrue(self.database.table_exists("test_table"))
 
         with self.wrap("odev.common.bash", "stream") as stream:
             stdout, _ = self.dispatch_command(
-                "run", self.database.name, "--stop-after-init", "--from-template", template_name
+                "run",
+                "--from-template",
+                template_name,
+                self.database.name,
+                "--stop-after-init",
             )
             stream.assert_called_with(OdoobinMatch(self.database.name, ["--stop-after-init"]))
 
@@ -302,13 +343,13 @@ def test_03_run_from_template_no_value(self):
         self.assert_database(self.database.name, is_odoo=True)
         template_name = f"{self.database.name}:template"
         self.assert_database(template_name, exists=False)
-        self.dispatch_command("create", self.database.name, "--create-template")
+        self.dispatch_command("create", "--create-template", self.database.name)
         self.assert_database(template_name, is_odoo=True)
         self.database.query("CREATE TABLE test_table (id SERIAL PRIMARY KEY);")
         self.assertTrue(self.database.table_exists("test_table"))
 
         with self.wrap("odev.common.bash", "stream") as stream:
-            stdout, _ = self.dispatch_command("run", self.database.name, "--stop-after-init", "--from-template")
+            stdout, _ = self.dispatch_command("run", "--from-template", "", self.database.name, "--stop-after-init")
             stream.assert_called_with(OdoobinMatch(self.database.name, ["--stop-after-init"]))
 
         self.assert_database(self.database.name, is_odoo=True)
@@ -324,7 +365,11 @@ def test_04_run_from_invalid_template(self):
         self.database.query("CREATE TABLE test_table (id SERIAL PRIMARY KEY);")
         self.assertTrue(self.database.table_exists("test_table"))
         stdout, _ = self.dispatch_command(
-            "run", self.database.name, "--stop-after-init", "--from-template", template_name
+            "run",
+            "--from-template",
+            template_name,
+            self.database.name,
+            "--stop-after-init",
         )
         self.assert_database(self.database.name, is_odoo=True)
         self.assertTrue(self.database.table_exists("test_table"))

diff --git a/tests/fixtures/case.py b/tests/fixtures/case.py
@@ -363,5 +363,5 @@ def create_odoo_database(cls, name: str) -> LocalDatabase:
         if (database := LocalDatabase(name)).exists:
             database.drop()
 
-        cls.odev.run_command("create", name, "--version", "17.0", "--without-demo", "all", "--community")
+        cls.odev.run_command("create", "--version", "17.0", "--community", name, "--without-demo", "all")
         return LocalDatabase(name)