From cdd7b71deae1f9aad809fe7e50947da4e5d0d8f4 Mon Sep 17 00:00:00 2001 From: Renan Rodrigo Date: Mon, 1 Jul 2024 15:23:22 -0300 Subject: [PATCH] cli: refactor enable to the new approach --- features/help.feature | 24 +++++++ uaclient/cli/__init__.py | 6 +- uaclient/cli/enable.py | 90 ++++++++++++++------------- uaclient/cli/tests/test_cli.py | 24 +------ uaclient/cli/tests/test_cli_enable.py | 47 +------------- 5 files changed, 76 insertions(+), 115 deletions(-) diff --git a/features/help.feature b/features/help.feature index cb0a824e77..ff48c42136 100644 --- a/features/help.feature +++ b/features/help.feature @@ -54,6 +54,30 @@ Feature: Pro Client help text --purge disable the service and remove/downgrade related packages \(experimental\) """ + When I run `pro enable --help` as non-root + Then stdout matches regexp: + """ + usage: pro enable \[flags\] + + Enable an Ubuntu Pro service. + + positional arguments: + service the name\(s\) of the Ubuntu Pro services to enable. One + of: anbox-cloud, cc-eal, cis, esm-apps, esm-infra, + fips, fips-preview, fips-updates, landscape, livepatch, + realtime-kernel, ros, ros-updates + + (optional arguments|options): + -h, --help show this help message and exit + --assume-yes do not prompt for confirmation before performing the + enable + --access-only do not auto-install packages. Valid for cc-eal, cis and + realtime-kernel. + --beta allow beta service to be enabled + --format \{cli,json\} output in the specified format \(default: cli\) + --variant VARIANT The name of the variant to use when enabling the + service + """ Examples: ubuntu release | release | machine_type | diff --git a/uaclient/cli/__init__.py b/uaclient/cli/__init__.py index 01a0e77116..7988a4658d 100644 --- a/uaclient/cli/__init__.py +++ b/uaclient/cli/__init__.py @@ -46,11 +46,12 @@ _reboot_required, ) from uaclient.apt import AptProxyScope, setup_apt_proxy -from uaclient.cli import cli_util, enable, fix +from uaclient.cli import cli_util, fix from uaclient.cli.api import api_command from uaclient.cli.collect_logs import collect_logs_command from uaclient.cli.constants import NAME, USAGE_TMPL from uaclient.cli.disable import disable_command, perform_disable +from uaclient.cli.enable import enable_command from uaclient.data_types import AttachActionsConfigFile, IncorrectTypeError from uaclient.entitlements import ( create_enable_entitlements_not_found_error, @@ -73,7 +74,7 @@ event = event_logger.get_event_logger() LOG = logging.getLogger(util.replace_top_level_logger_name(__name__)) -COMMANDS = [api_command, collect_logs_command, disable_command] +COMMANDS = [api_command, collect_logs_command, disable_command, enable_command] class UAArgumentParser(argparse.ArgumentParser): @@ -908,7 +909,6 @@ def get_parser(cfg: config.UAConfig): detach_parser(parser_detach) parser_detach.set_defaults(action=action_detach) - enable.add_parser(subparsers, cfg) fix.add_parser(subparsers) parser_security_status = subparsers.add_parser( diff --git a/uaclient/cli/enable.py b/uaclient/cli/enable.py index a2350e3233..d10a3a2e71 100644 --- a/uaclient/cli/enable.py +++ b/uaclient/cli/enable.py @@ -28,7 +28,8 @@ _enabled_services, ) from uaclient.api.u.pro.status.is_attached.v1 import _is_attached -from uaclient.cli import cli_util, constants +from uaclient.cli import cli_util +from uaclient.cli.commands import ProArgument, ProCommand LOG = logging.getLogger(util.replace_top_level_logger_name(__name__)) @@ -505,47 +506,48 @@ def action_enable(args, *, cfg, **kwargs) -> int: return 0 if ret else 1 -def add_parser(subparsers, cfg: config.UAConfig): - parser = subparsers.add_parser("enable", help=messages.CLI_ROOT_ENABLE) - parser.set_defaults(action=action_enable) - parser.description = messages.CLI_ENABLE_DESC - parser.usage = constants.USAGE_TMPL.format( - name=constants.NAME, command="enable []" - ) - parser.prog = "enable" - parser._positionals.title = messages.CLI_ARGS - parser._optionals.title = messages.CLI_FLAGS - parser.add_argument( - "service", - action="store", - nargs="+", - help=( - messages.CLI_ENABLE_SERVICE.format( - options=", ".join(entitlements.valid_services(cfg=cfg)) - ) +enable_command = ProCommand( + "enable", + help=messages.CLI_ROOT_ENABLE, + description=messages.CLI_ENABLE_DESC, + action=action_enable, + arguments=[ + ProArgument( + "service", + help=messages.CLI_ENABLE_SERVICE.format( + options=", ".join( + entitlements.valid_services(cfg=config.UAConfig()) + ) + ), + action="store", + nargs="+", ), - ) - parser.add_argument( - "--assume-yes", - action="store_true", - help=messages.CLI_ASSUME_YES.format(command="enable"), - ) - parser.add_argument( - "--access-only", - action="store_true", - help=messages.CLI_ENABLE_ACCESS_ONLY, - ) - parser.add_argument( - "--beta", action="store_true", help=messages.CLI_ENABLE_BETA - ) - parser.add_argument( - "--format", - action="store", - choices=["cli", "json"], - default="cli", - help=messages.CLI_FORMAT_DESC.format(default="cli"), - ) - parser.add_argument( - "--variant", action="store", help=messages.CLI_ENABLE_VARIANT - ) - return parser + ProArgument( + "--assume-yes", + help=messages.CLI_ASSUME_YES.format(command="enable"), + action="store_true", + ), + ProArgument( + "--access-only", + help=messages.CLI_ENABLE_ACCESS_ONLY, + action="store_true", + ), + ProArgument( + "--beta", + help=messages.CLI_ENABLE_BETA, + action="store_true", + ), + ProArgument( + "--format", + help=messages.CLI_FORMAT_DESC.format(default="cli"), + action="store", + choices=["cli", "json"], + default="cli", + ), + ProArgument( + "--variant", + help=messages.CLI_ENABLE_VARIANT, + action="store", + ), + ], +) diff --git a/uaclient/cli/tests/test_cli.py b/uaclient/cli/tests/test_cli.py index e175ea94d7..172e069a7b 100644 --- a/uaclient/cli/tests/test_cli.py +++ b/uaclient/cli/tests/test_cli.py @@ -3,13 +3,12 @@ import json import logging import socket -import textwrap import mock import pytest from uaclient import defaults, exceptions, messages, status -from uaclient.cli import action_help, get_parser, main +from uaclient.cli import action_help, main from uaclient.entitlements import get_valid_entitlement_names from uaclient.exceptions import ( AlreadyAttachedError, @@ -421,27 +420,6 @@ def test_setup_logging_with_defaults( assert expected_setup_logging_calls == m_setup_logging.call_args_list - @mock.patch("uaclient.cli.contract.get_available_resources") - def test_argparse_errors_well_formatted( - self, _m_resources, capsys, FakeConfig - ): - cfg = FakeConfig() - parser = get_parser(cfg) - with mock.patch("sys.argv", ["pro", "enable"]): - with pytest.raises(SystemExit) as excinfo: - parser.parse_args() - assert 2 == excinfo.value.code - _, err = capsys.readouterr() - assert ( - textwrap.dedent( - """\ - usage: pro enable [] [flags] - the following arguments are required: service - """ - ) - == str(err) - ) - @pytest.mark.parametrize( "cli_args,is_tty,should_warn", ( diff --git a/uaclient/cli/tests/test_cli_enable.py b/uaclient/cli/tests/test_cli_enable.py index 856987bf14..dd5c5b0fb7 100644 --- a/uaclient/cli/tests/test_cli_enable.py +++ b/uaclient/cli/tests/test_cli_enable.py @@ -15,61 +15,18 @@ EnabledServicesResult, ) from uaclient.api.u.pro.status.is_attached.v1 import IsAttachedResult -from uaclient.cli import main from uaclient.cli.enable import ( _enable_landscape, _enable_one_service, _EnableOneServiceResult, _print_json_output, - action_enable, + enable_command, prompt_for_dependency_handling, ) from uaclient.testing.helpers import does_not_raise -HELP_OUTPUT = """\ -usage: pro enable [] [flags] - -Enable an Ubuntu Pro service. - -Arguments: - service the name(s) of the Ubuntu Pro services to enable. One - of: anbox-cloud, cc-eal, cis, esm-apps, esm-infra, - fips, fips-preview, fips-updates, landscape, livepatch, - realtime-kernel, ros, ros-updates - -Flags: - -h, --help show this help message and exit - --assume-yes do not prompt for confirmation before performing the - enable - --access-only do not auto-install packages. Valid for cc-eal, cis and - realtime-kernel. - --beta allow beta service to be enabled - --format {cli,json} output in the specified format (default: cli) - --variant VARIANT The name of the variant to use when enabling the - service -""" - class TestActionEnable: - @mock.patch("uaclient.log.setup_cli_logging") - @mock.patch("uaclient.cli.contract.get_available_resources") - def test_enable_help( - self, - _m_resources, - _m_setup_logging, - capsys, - FakeConfig, - ): - with pytest.raises(SystemExit): - with mock.patch("sys.argv", ["/usr/bin/ua", "enable", "--help"]): - with mock.patch( - "uaclient.config.UAConfig", - return_value=FakeConfig(), - ): - main() - out, _err = capsys.readouterr() - assert HELP_OUTPUT == out - @pytest.mark.parametrize( [ "is_attached", @@ -543,7 +500,7 @@ def test_action_enable( fake_machine_token_file.attached = True with expected_raises: - action_enable(args, cfg=FakeConfig(), **kwargs) + enable_command.action(args, cfg=FakeConfig(), **kwargs) assert ( expected_enable_one_service_calls