From 02deb38133345d2424e2d361d2d98559f93398ca Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Fri, 12 Jan 2024 00:07:36 -0500 Subject: [PATCH 1/2] feat(dispatcher): constrained global arguments Fixes #219 --- craft_cli/dispatcher.py | 73 +++++++++++++++++++++++++---------- craft_cli/utils.py | 29 ++++++++++++++ docs/.wordlist.txt | 1 + pyproject.toml | 2 + tests/unit/test_dispatcher.py | 2 +- tests/unit/test_utils.py | 55 ++++++++++++++++++++++++++ 6 files changed, 140 insertions(+), 22 deletions(-) create mode 100644 craft_cli/utils.py create mode 100644 tests/unit/test_utils.py diff --git a/craft_cli/dispatcher.py b/craft_cli/dispatcher.py index 7a90bbf..59b5b21 100644 --- a/craft_cli/dispatcher.py +++ b/craft_cli/dispatcher.py @@ -18,10 +18,11 @@ from __future__ import annotations import argparse +import dataclasses import difflib -from typing import Any, Literal, NamedTuple, NoReturn, Optional, Sequence +from typing import Any, Callable, Literal, NamedTuple, NoReturn, Optional, Sequence -from craft_cli import EmitterMode, emit +from craft_cli import EmitterMode, emit, utils from craft_cli.errors import ArgumentParsingError, ProvideHelpException from craft_cli.helptexts import HelpBuilder, OutputFormat @@ -43,7 +44,8 @@ class CommandGroup(NamedTuple): """Whether the commands in this group are already in the correct order (defaults to False).""" -class GlobalArgument(NamedTuple): +@dataclasses.dataclass +class GlobalArgument: """Definition of a global argument to be handled by the Dispatcher.""" name: str @@ -64,6 +66,27 @@ class GlobalArgument(NamedTuple): help_message: str """the one-line text that describes the argument, for building the help texts.""" + choices: Sequence[str] | None = dataclasses.field(default=None) + """Valid choices for this option.""" + + validator: Callable[[str], Any] | None = dataclasses.field(default=None) + """A validator callable that converts the option input to the correct value. + + The validator is called when parsing the argument. If it raises an exception, the + exception message will be used as part of the usage output. Otherwise, the return + value will be used as the content of this option. + """ + + case_sensitive: bool = True + """Whether the choices are case sensitive. Only used if choices are set.""" + + def __post_init__(self) -> None: + if self.type == "flag": + if self.choices is not None or self.validator is not None: + raise TypeError("A flag argument cannot have choices or a validator.") + elif self.choices and not self.case_sensitive: + self.choices = [choice.lower() for choice in self.choices] + _DEFAULT_GLOBAL_ARGS = [ GlobalArgument( @@ -93,6 +116,9 @@ class GlobalArgument(NamedTuple): None, "--verbosity", "Set the verbosity level to 'quiet', 'brief', 'verbose', 'debug' or 'trace'", + choices=[mode.name.lower() for mode in EmitterMode], + validator=lambda mode: EmitterMode[mode.upper()], + case_sensitive=False, ), ] @@ -397,20 +423,32 @@ def _parse_options( # noqa: PLR0912 (too many branches) arg = arg_per_option[sysarg] if arg.type == "flag": global_args[arg.name] = True - else: - try: - global_args[arg.name] = next(sysargs_it) - except StopIteration: - msg = f"The {arg.name!r} option expects one argument." - raise self._build_usage_exc(msg) # noqa: TRY200 (use 'raise from') + continue + option = sysarg + try: + value = next(sysargs_it) + except StopIteration: + msg = f"The {arg.name!r} option expects one argument." + raise self._build_usage_exc(msg) # noqa: TRY200 (use 'raise from') elif sysarg.startswith(tuple(options_with_equal)): option, value = sysarg.split("=", 1) - arg = arg_per_option[option] - if not value: - raise self._build_usage_exc(f"The {arg.name!r} option expects one argument.") - global_args[arg.name] = value else: filtered_sysargs.append(sysarg) + continue + arg = arg_per_option[option] + if not value: + raise self._build_usage_exc(f"The {arg.name!r} option expects one argument.") + if arg.choices is not None: + if not arg.case_sensitive: + value = value.lower() + if value not in arg.choices: + choices = utils.humanise_list([f"'{choice}'" for choice in arg.choices]) + raise self._build_usage_exc( + f"Bad {arg.name} {value!r}; valid values are {choices}." + ) + + validator = arg.validator or str + global_args[arg.name] = validator(value) return global_args, filtered_sysargs def pre_parse_args(self, sysargs: list[str]) -> dict[str, Any]: @@ -436,14 +474,7 @@ def pre_parse_args(self, sysargs: list[str]) -> dict[str, Any]: elif global_args["verbose"]: emit.set_mode(EmitterMode.VERBOSE) elif global_args["verbosity"]: - try: - verbosity_level = EmitterMode[global_args["verbosity"].upper()] - except KeyError: - raise self._build_usage_exc( # noqa: TRY200 (use 'raise from') - "Bad verbosity level; valid values are " - "'quiet', 'brief', 'verbose', 'debug' and 'trace'." - ) - emit.set_mode(verbosity_level) + emit.set_mode(global_args["verbosity"]) emit.trace(f"Raw pre-parsed sysargs: args={global_args} filtered={filtered_sysargs}") # handle requested help through -h/--help options diff --git a/craft_cli/utils.py b/craft_cli/utils.py new file mode 100644 index 0000000..551a941 --- /dev/null +++ b/craft_cli/utils.py @@ -0,0 +1,29 @@ +# Copyright 2024 Canonical Ltd. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License version 3 as published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +"""Utility functions for craft_cli.""" +from typing import Sequence + + +def humanise_list(values: Sequence[str], conjunction: str = "and") -> str: + """Convert a collection of values to a string that lists the values. + + :param values: The values to turn into a list + :param conjunction: The conjunction to use between the last two items + :returns: A string containing the list phrase ready to insert into a sentence. + """ + if not values: + return "" + start = ", ".join(values[:-1]) + return f"{start} {conjunction} {values[-1]}" diff --git a/docs/.wordlist.txt b/docs/.wordlist.txt index aa3251f..f849b68 100644 --- a/docs/.wordlist.txt +++ b/docs/.wordlist.txt @@ -33,6 +33,7 @@ RTD subdirectories subtree subfolders +utils UI VM YAML diff --git a/pyproject.toml b/pyproject.toml index d4bb2a5..0fccfb6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,7 +44,9 @@ emitter = "craft_cli.pytest_plugin" [project.optional-dependencies] dev = [ "coverage[toml]==7.3.2", + "hypothesis==6.92.9", "pytest==7.4.3", + "pytest-check==2.2.4", "pytest-cov==4.1.0", "pytest-mock==3.12.0", "pytest-subprocess" diff --git a/tests/unit/test_dispatcher.py b/tests/unit/test_dispatcher.py index 983fe9c..c57e491 100644 --- a/tests/unit/test_dispatcher.py +++ b/tests/unit/test_dispatcher.py @@ -322,7 +322,7 @@ def test_dispatcher_generic_setup_verbosity_levels_wrong(): Usage: appname [options] command [args]... Try 'appname -h' for help. - Error: Bad verbosity level; valid values are 'quiet', 'brief', 'verbose', 'debug' and 'trace'. + Error: Bad verbosity 'yelling'; valid values are 'quiet', 'brief', 'verbose', 'debug' and 'trace'. """ ) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py new file mode 100644 index 0000000..b648939 --- /dev/null +++ b/tests/unit/test_utils.py @@ -0,0 +1,55 @@ +# Copyright 2024 Canonical Ltd. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License version 3 as published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +"""Unit tests for utility functions.""" +import re + +import pytest +import pytest_check +from hypothesis import given, strategies + +from craft_cli import utils + + +@pytest.mark.parametrize( + "values", + [ + [], + ["one-thing"], + ["two", "things"], + ], +) +@pytest.mark.parametrize("conjunction", ["and", "or", "but not"]) +def test_humanise_list_success(values, conjunction): + actual = utils.humanise_list(values, conjunction) + + pytest_check.equal(actual.count(","), max((len(values) - 2, 0))) + with pytest_check.check: + assert actual == "" or conjunction in actual + for value in values: + pytest_check.is_in(value, actual) + + +@given( + values=strategies.lists(strategies.text()), + conjunction=strategies.text(), +) +def test_humanise_list_fuzzy(values, conjunction): + actual = utils.humanise_list(values, conjunction) + + pytest_check.greater_equal(actual.count(","), max((len(values) - 2, 0))) + with pytest_check.check: + assert actual == "" or conjunction in actual + for value in values: + pytest_check.is_in(value, actual) From 499ed143456363ee65874d4f276d8e2fcd46c58b Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Fri, 12 Jan 2024 13:30:53 -0500 Subject: [PATCH 2/2] test(dispatcher): GlobalArgument tests --- tests/unit/test_dispatcher.py | 112 ++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/tests/unit/test_dispatcher.py b/tests/unit/test_dispatcher.py index c57e491..c044571 100644 --- a/tests/unit/test_dispatcher.py +++ b/tests/unit/test_dispatcher.py @@ -30,6 +30,118 @@ from craft_cli.errors import ArgumentParsingError from tests.factory import create_command +# --- Tests for global arguments + + +@pytest.mark.parametrize( + ("params", "expected"), + [ + pytest.param( + {"type": "flag"}, + GlobalArgument( + name="my-arg", + short_option=None, + long_option="--long-option", + help_message="A global argument for testing", + type="flag", + ), + id="basic-flag", + ), + pytest.param( + {"type": "option"}, + GlobalArgument( + name="my-arg", + short_option=None, + long_option="--long-option", + help_message="A global argument for testing", + type="option", + ), + id="basic-option", + ), + pytest.param( + {"type": "option", "choices": ["a", "b", "c"]}, + GlobalArgument( + name="my-arg", + short_option=None, + long_option="--long-option", + help_message="A global argument for testing", + type="option", + choices=["a", "b", "c"], + ), + id="choices", + ), + pytest.param( + {"type": "option", "choices": "ABC", "case_sensitive": False}, + GlobalArgument( + name="my-arg", + short_option=None, + long_option="--long-option", + help_message="A global argument for testing", + type="option", + choices=["a", "b", "c"], + case_sensitive=False, + ), + id="case-insensitive", + ), + pytest.param( + {"type": "option", "validator": int}, + GlobalArgument( + name="my-arg", + short_option=None, + long_option="--long-option", + help_message="A global argument for testing", + type="option", + validator=int, + ), + id="int-validator", + ), + pytest.param( + {"type": "option", "choices": "123", "validator": int}, + GlobalArgument( + name="my-arg", + short_option=None, + long_option="--long-option", + help_message="A global argument for testing", + type="option", + choices="123", + validator=int, + ), + id="limited-ints", + ), + ], +) +def test_global_argument_init_success(params, expected): + my_params = { + "name": "my-arg", + "short_option": None, + "long_option": "--long-option", + "help_message": "A global argument for testing", + } + my_params.update(params) + assert GlobalArgument(**my_params) == expected + + +@pytest.mark.parametrize( + ("params", "match"), + [ + ( + {"type": "flag", "choices": "something"}, + "A flag argument cannot have choices or a validator.", + ) + ], +) +def test_global_argument_init_error(params, match): + my_params = { + "name": "my-arg", + "short_option": None, + "long_option": "--long-option", + "help_message": "A global argument for testing", + } + my_params.update(params) + with pytest.raises(TypeError, match=match): + GlobalArgument(**my_params) + + # --- Tests for the Dispatcher