Skip to content

Commit

Permalink
fix(cli): allow exclusive arguments as optional (#2770)
Browse files Browse the repository at this point in the history
* fix(cli): allow exclusive arguments as optional

The CLI takes its arguments from the RequiredOptional, which has three fields: required, optional, and exclusive. In practice, the exclusive options are not defined as either required or optional, and would not be allowed in the CLI. This changes that, so that exclusive options are also added to the argument parser.

  * fix(cli): inform argument parser that options are mutually exclusive

  * fix(cli): use correct exclusive options, add unit test

Closes #2769
  • Loading branch information
Sjord committed Jan 29, 2024
1 parent 4e68d32 commit 7ec3189
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 0 deletions.
9 changes: 9 additions & 0 deletions gitlab/v4/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ def _populate_sub_parser_by_class(
sub_parser_action.add_argument(
f"--{x.replace('_', '-')}", required=False
)
if mgr_cls._create_attrs.exclusive:
group = sub_parser_action.add_mutually_exclusive_group()
for x in mgr_cls._create_attrs.exclusive:
group.add_argument(f"--{x.replace('_', '-')}")

if action_name == "update":
if cls._id_attr is not None:
Expand All @@ -280,6 +284,11 @@ def _populate_sub_parser_by_class(
f"--{x.replace('_', '-')}", required=False
)

if mgr_cls._update_attrs.exclusive:
group = sub_parser_action.add_mutually_exclusive_group()
for x in mgr_cls._update_attrs.exclusive:
group.add_argument(f"--{x.replace('_', '-')}")

if cls.__name__ in cli.custom_actions:
name = cls.__name__
for action_name in cli.custom_actions[name]:
Expand Down
61 changes: 61 additions & 0 deletions tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import gitlab.base
from gitlab import cli
from gitlab.exceptions import GitlabError
from gitlab.mixins import CreateMixin, UpdateMixin
from gitlab.types import RequiredOptional
from gitlab.v4 import cli as v4_cli


Expand Down Expand Up @@ -157,6 +159,65 @@ def test_v4_parser():
assert actions["--name"].required


def test_extend_parser():
class ExceptionArgParser(argparse.ArgumentParser):
def error(self, message):
"Raise error instead of exiting on invalid arguments, to make testing easier"
raise ValueError(message)

class Fake:
_id_attr = None

class FakeManager(gitlab.base.RESTManager, CreateMixin, UpdateMixin):
_obj_cls = Fake
_create_attrs = RequiredOptional(
required=("create",),
optional=("opt_create",),
exclusive=("create_a", "create_b"),
)
_update_attrs = RequiredOptional(
required=("update",),
optional=("opt_update",),
exclusive=("update_a", "update_b"),
)

parser = ExceptionArgParser()
with mock.patch.dict(
"gitlab.v4.objects.__dict__", {"FakeManager": FakeManager}, clear=True
):
v4_cli.extend_parser(parser)

assert parser.parse_args(["fake", "create", "--create", "1"])
assert parser.parse_args(["fake", "create", "--create", "1", "--opt-create", "1"])
assert parser.parse_args(["fake", "create", "--create", "1", "--create-a", "1"])
assert parser.parse_args(["fake", "create", "--create", "1", "--create-b", "1"])

with pytest.raises(ValueError):
# missing required "create"
parser.parse_args(["fake", "create", "--opt_create", "1"])

with pytest.raises(ValueError):
# both exclusive options
parser.parse_args(
["fake", "create", "--create", "1", "--create-a", "1", "--create-b", "1"]
)

assert parser.parse_args(["fake", "update", "--update", "1"])
assert parser.parse_args(["fake", "update", "--update", "1", "--opt-update", "1"])
assert parser.parse_args(["fake", "update", "--update", "1", "--update-a", "1"])
assert parser.parse_args(["fake", "update", "--update", "1", "--update-b", "1"])

with pytest.raises(ValueError):
# missing required "update"
parser.parse_args(["fake", "update", "--opt_update", "1"])

with pytest.raises(ValueError):
# both exclusive options
parser.parse_args(
["fake", "update", "--update", "1", "--update-a", "1", "--update-b", "1"]
)


@pytest.mark.skipif(sys.version_info < (3, 8), reason="added in 3.8")
def test_legacy_display_without_fields_warns(fake_object_no_id):
printer = v4_cli.LegacyPrinter()
Expand Down

0 comments on commit 7ec3189

Please sign in to comment.