cli: refactor status to the new approach

Signed-off-by: Renan Rodrigo <[email protected]>

features/help.feature

@@ -177,6 +177,53 @@ Feature: Pro Client help text
         --no-related    If used, when fixing a USN, the command will not try to also
                         fix related USNs to the target USN.
       """
+    When I run `pro status --help` as non-root
+    Then stdout matches regexp:
+      """
+      usage: pro status \[flags\]
+
+      Report current status of Ubuntu Pro services on system.
+
+      This shows whether this machine is attached to an Ubuntu Advantage
+      support contract. When attached, the report includes the specific
+      support contract details including contract name, expiry dates, and the
+      status of each service on this system.
+
+      The attached status output has four columns:
+
+      * SERVICE: name of the service
+      * ENTITLED: whether the contract to which this machine is attached
+        entitles use of this service. Possible values are: yes or no
+      * STATUS: whether the service is enabled on this machine. Possible
+        values are: enabled, disabled, n/a \(if your contract entitles
+        you to the service, but it isn't available for this machine\) or — \(if
+        you aren't entitled to this service\)
+      * DESCRIPTION: a brief description of the service
+
+      The unattached status output instead has three columns. SERVICE
+      and DESCRIPTION are the same as above, and there is the addition
+      of:
+
+      * AVAILABLE: whether this service would be available if this machine
+        were attached. The possible values are yes or no.
+
+      If --simulate-with-token is used, then the output has five
+      columns. SERVICE, AVAILABLE, ENTITLED and DESCRIPTION are the same
+      as mentioned above, and AUTO_ENABLED shows whether the service is set
+      to be enabled when that token is attached.
+
+      If the --all flag is set, beta and unavailable services are also
+      listed in the output.
+
+      (optional arguments|options):
+        -h, --help            show this help message and exit
+        --wait                Block waiting on pro to complete
+        --format \{tabular,json,yaml\}
+                              output in the specified format \(default: tabular\)
+        --simulate-with-token TOKEN
+                              simulate the output status using a provided token
+        --all                 Include unavailable and beta services
+      """
 
     Examples: ubuntu release
       | release | machine_type  |

uaclient/cli/__init__.py

@@ -4,11 +4,10 @@
 import json
 import logging
 import sys
-import time
 from typing import Optional
 
+# This will be fixed when the help command goes away
 from uaclient import (
-    actions,
     apt,
     apt_news,
     config,
@@ -21,10 +20,9 @@
     lock,
     log,
     messages,
-    status,
-    util,
-    version,
 )
+from uaclient import status as status_module
+from uaclient import util, version
 from uaclient.api.u.pro.security.status.reboot_required.v1 import (
     _reboot_required,
 )
@@ -40,6 +38,7 @@
 from uaclient.cli.enable import enable_command
 from uaclient.cli.fix import fix_command
 from uaclient.cli.security_status import security_status_command
+from uaclient.cli.status import status_command
 from uaclient.entitlements.entitlement_status import ApplicationStatus
 from uaclient.files import state_files
 from uaclient.log import get_user_or_root_log_file_path
@@ -62,6 +61,7 @@
     enable_command,
     fix_command,
     security_status_command,
+    status_command,
 ]
 
 
@@ -249,41 +249,6 @@ def reboot_required_parser(parser):
     return parser
 
 
-def status_parser(parser):
-    """Build or extend an arg parser for status subcommand."""
-    usage = USAGE_TMPL.format(name=NAME, command="status")
-    parser.usage = usage
-    parser.prog = "status"
-    # This formatter_class ensures that our formatting below isn't lost
-    parser.formatter_class = argparse.RawDescriptionHelpFormatter
-    parser.description = messages.CLI_STATUS_DESC
-
-    parser.add_argument(
-        "--wait",
-        action="store_true",
-        default=False,
-        help=messages.CLI_STATUS_WAIT,
-    )
-    parser.add_argument(
-        "--format",
-        action="store",
-        choices=STATUS_FORMATS,
-        default=STATUS_FORMATS[0],
-        help=(messages.CLI_FORMAT_DESC.format(default=STATUS_FORMATS[0])),
-    )
-    parser.add_argument(
-        "--simulate-with-token",
-        metavar="TOKEN",
-        action="store",
-        help=messages.CLI_STATUS_SIMULATE_WITH_TOKEN,
-    )
-    parser.add_argument(
-        "--all", action="store_true", help=messages.CLI_STATUS_ALL
-    )
-    parser._optionals.title = "Flags"
-    return parser
-
-
 def _print_help_for_subcommand(
     cfg: config.UAConfig, cmd_name: str, subcmd_name: str
 ):
@@ -551,12 +516,6 @@ def get_parser(cfg: config.UAConfig):
     parser_refresh.set_defaults(action=action_refresh)
     refresh_parser(parser_refresh)
 
-    parser_status = subparsers.add_parser(
-        "status", help=messages.CLI_ROOT_STATUS
-    )
-    parser_status.set_defaults(action=action_status)
-    status_parser(parser_status)
-
     parser_version = subparsers.add_parser(
         "version", help=messages.CLI_ROOT_VERSION.format(name=NAME)
     )
@@ -571,35 +530,6 @@ def get_parser(cfg: config.UAConfig):
     return parser
 
 
-def action_status(args, *, cfg: config.UAConfig, **kwargs):
-    if not cfg:
-        cfg = config.UAConfig()
-    show_all = args.all if args else False
-    token = args.simulate_with_token if args else None
-    active_value = status.UserFacingConfigStatus.ACTIVE.value
-    status_dict, ret = actions.status(
-        cfg, simulate_with_token=token, show_all=show_all
-    )
-    config_active = bool(status_dict["execution_status"] == active_value)
-
-    if args and args.wait and config_active:
-        while status_dict["execution_status"] == active_value:
-            event.info(".", end="")
-            time.sleep(1)
-            status_dict, ret = actions.status(
-                cfg,
-                simulate_with_token=token,
-                show_all=show_all,
-            )
-        event.info("")
-
-    event.set_output_content(status_dict)
-    output = status.format_tabular(status_dict, show_all=show_all)
-    event.info(util.handle_unicode_characters(output))
-    event.process_events()
-    return ret
-
-
 def action_system(args, *, cfg, **kwargs):
     """Perform the system action.
 
@@ -704,7 +634,7 @@ def action_help(args, *, cfg, **kwargs):
     if not cfg:
         cfg = config.UAConfig()
 
-    help_response = status.help(cfg, service)
+    help_response = status_module.help(cfg, service)
 
     if args.format == "json":
         print(json.dumps(help_response))

uaclient/cli/status.py

@@ -0,0 +1,71 @@
+import time
+
+from uaclient import actions, config, event_logger, messages, status, util
+from uaclient.cli.commands import ProArgument, ProArgumentGroup, ProCommand
+
+event = event_logger.get_event_logger()
+
+
+def action_status(args, *, cfg: config.UAConfig, **kwargs):
+    if not cfg:
+        cfg = config.UAConfig()
+    show_all = args.all if args else False
+    token = args.simulate_with_token if args else None
+    active_value = status.UserFacingConfigStatus.ACTIVE.value
+    status_dict, ret = actions.status(
+        cfg, simulate_with_token=token, show_all=show_all
+    )
+    config_active = bool(status_dict["execution_status"] == active_value)
+
+    if args and args.wait and config_active:
+        while status_dict["execution_status"] == active_value:
+            event.info(".", end="")
+            time.sleep(1)
+            status_dict, ret = actions.status(
+                cfg,
+                simulate_with_token=token,
+                show_all=show_all,
+            )
+        event.info("")
+
+    event.set_output_content(status_dict)
+    output = status.format_tabular(status_dict, show_all=show_all)
+    event.info(util.handle_unicode_characters(output))
+    event.process_events()
+    return ret
+
+
+status_command = ProCommand(
+    "status",
+    help=messages.CLI_ROOT_STATUS,
+    description=messages.CLI_STATUS_DESC,
+    action=action_status,
+    preserve_description=True,
+    argument_groups=[
+        ProArgumentGroup(
+            arguments=[
+                ProArgument(
+                    "--wait",
+                    help=messages.CLI_STATUS_WAIT,
+                    action="store_true",
+                ),
+                ProArgument(
+                    "--format",
+                    help=messages.CLI_FORMAT_DESC.format(default="tabular"),
+                    action="store",
+                    choices=["tabular", "json", "yaml"],
+                    default="tabular",
+                ),
+                ProArgument(
+                    "--simulate-with-token",
+                    help=messages.CLI_STATUS_SIMULATE_WITH_TOKEN,
+                    metavar="TOKEN",
+                    action="store",
+                ),
+                ProArgument(
+                    "--all", help=messages.CLI_STATUS_ALL, action="store_true"
+                ),
+            ]
+        )
+    ],
+)

uaclient/cli/tests/test_cli.py

@@ -8,7 +8,7 @@
 import pytest
 
 from uaclient import defaults, exceptions, messages, status
-from uaclient.cli import action_help, main
+from uaclient.cli import _warn_about_output_redirection, action_help, main
 from uaclient.entitlements import get_valid_entitlement_names
 from uaclient.exceptions import (
     AlreadyAttachedError,
@@ -421,56 +421,60 @@ def test_setup_logging_with_defaults(
         assert expected_setup_logging_calls == m_setup_logging.call_args_list
 
     @pytest.mark.parametrize(
-        "cli_args,is_tty,should_warn",
+        "command,out_format,is_tty,should_warn",
         (
-            (["pro", "status"], True, False),
-            (["pro", "status"], False, True),
-            (["pro", "status", "--format", "tabular"], True, False),
-            (["pro", "status", "--format", "tabular"], False, True),
-            (["pro", "status", "--format", "json"], True, False),
-            (["pro", "status", "--format", "json"], False, False),
-            (["pro", "security-status"], True, False),
-            (["pro", "security-status"], False, True),
-            (["pro", "security-status", "--format", "json"], True, False),
-            (["pro", "security-status", "--format", "json"], False, False),
+            ("status", None, True, False),
+            ("status", None, False, True),
+            ("status", "tabular", True, False),
+            ("status", "tabular", False, True),
+            ("status", "json", True, False),
+            ("status", "json", False, False),
+            ("security-status", None, True, False),
+            ("security-status", None, False, True),
+            ("security-status", "json", True, False),
+            ("security-status", "json", False, False),
         ),
     )
+    @pytest.mark.parametrize("caplog_text", [logging.WARNING], indirect=True)
     @mock.patch("uaclient.cli.event.info")
-    @mock.patch("uaclient.cli.action_status")
-    @mock.patch("uaclient.cli.security_status.action_security_status")
-    @mock.patch("uaclient.log.setup_cli_logging")
     @mock.patch("sys.stdout.isatty")
     def test_status_human_readable_warning(
         self,
         m_tty,
-        _m_setup_logging,
-        _m_action_security_status,
-        _m_action_status,
         m_event_info,
-        cli_args,
+        command,
+        out_format,
         is_tty,
         should_warn,
-        FakeConfig,
+        caplog_text,
     ):
         m_tty.return_value = is_tty
-        with mock.patch("sys.argv", cli_args):
-            with mock.patch(
-                "uaclient.config.UAConfig",
-                return_value=FakeConfig(),
-            ):
-                main()
+
+        m_args = mock.MagicMock()
+        m_args.command = command
+        m_args.format = out_format
+
+        _warn_about_output_redirection(m_args)
 
         if should_warn:
             assert [
                 mock.call(
                     messages.WARNING_HUMAN_READABLE_OUTPUT.format(
-                        command=cli_args[1]
+                        command=command
                     ),
                     file_type=mock.ANY,
                 )
             ] == m_event_info.call_args_list
+            assert (
+                "Not in a tty and human-readable command called"
+                in caplog_text()
+            )
         else:
             assert [] == m_event_info.call_args_list
+            assert (
+                "Not in a tty and human-readable command called"
+                not in caplog_text()
+            )
 
 
 class TestGetValidEntitlementNames:

uaclient/cli/tests/test_cli_enable.py

@@ -702,7 +702,7 @@ def test_action_enable(
             ),
         ),
     )
-    @mock.patch("uaclient.cli.status.status")
+    @mock.patch("uaclient.cli.status_module.status")
     @mock.patch("uaclient.cli.enable._enable")
     @mock.patch("uaclient.cli.enable._enable_landscape")
     @mock.patch("uaclient.cli.enable.cli_util.CLIEnableDisableProgress")