From 3d031e76e007555a7ac8f62b49c78c23117dcd04 Mon Sep 17 00:00:00 2001 From: avs-odoo Date: Tue, 29 Oct 2024 17:01:19 +0100 Subject: [PATCH] [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: https://github.com/python/cpython/issues/59317 --- .github/workflows/odev.yml | 2 +- odev/common/commands/base.py | 12 +++++- odev/common/commands/odoobin.py | 1 - tests/commands/test_database.py | 75 ++++++++++++++++++++++++++------- 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/.github/workflows/odev.yml b/.github/workflows/odev.yml index 97dd539..ae13545 100644 --- 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/common/commands/base.py b/odev/common/commands/base.py index fbcef9e..c8505d4 100644 --- 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 index 71a72ae..9c817c0 100644 --- a/odev/common/commands/odoobin.py +++ b/odev/common/commands/odoobin.py @@ -279,7 +279,6 @@ class OdoobinTemplateCommand(OdoobinCommand): from_template = args.String( aliases=["-t", "--from-template"], description="Name of an existing PostgreSQL database to copy.", - nargs="?", default="", ) diff --git a/tests/commands/test_database.py b/tests/commands/test_database.py index cdff3a2..8368e91 100644 --- 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,7 +184,14 @@ 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) @@ -176,7 +199,14 @@ def test_06_with_venv(self): 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,7 +315,7 @@ 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);") @@ -286,7 +323,11 @@ def test_02_run_from_template(self): 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"))