Merge ppa-dev-tools:use-subparsers-for-args into ppa-dev-tools:main

[Bryce Harrington]

Modifies command argument handling to allow each command to have its own set of options. I had been originally planning to have a single parser and just avoid overloading args, but doing this allows for say, -N to mean one thing for one command and something totally different for another.

Two other more practical effects of this is that it should make the help text better organized (so it'll be clearer for users which args are relevant for each command), as well as writing test cases.

[Christian Ehrhardt  (paelzer)]

Generally ok with this split, with the low amount of suboptions it looks odd, but makes sense IMHO.
One request on better wording or fixing bugs or both (Sorry, didn't track their status if they are already fixed).

Plus one potentially wrong nit pick (you are the native speaker after all).

Either way, none of this is strong enough to stop this +1 overall.

review: Approve

[Bryce Harrington (bryce)]

Thanks, yeah the options will be growing this branch just sets the framework to place them in.

[Bryce Harrington (bryce)]

Pushed:
To git+ssh://git.launchpad.net/ppa-dev-tools
   8ec7d86..0e673cc main -> main

diff --git a/scripts/ppa b/scripts/ppa
index 1607342..ec66f03 100755
--- a/scripts/ppa
+++ b/scripts/ppa
@@ -130,14 +130,65 @@ def create_arg_parser():
                         action='version',
                         version='%(prog)s {version}'.format(version=__version__),
                         help="Version information")
-    parser.add_argument('--dry-run',
-                        dest='dry_run', action='store_true',
-                        help="Simulate command without modifying anything")
-    parser.add_argument('ppa_name', metavar='ppa-name',
-                        action='store',
-                        help="Name of the PPA to be created")
-    parser.add_argument('remainder',
-                        nargs=argparse.REMAINDER)
+    parser.add_argument('-v', '--verbose',
+                        dest='verbose', action='store_true',
+                        help="Print more information during processing")
+    parser.add_argument('-q', '--quiet',
+                        dest='quiet', action='store_true',
+                        help="Minimize output during processing")
+
+    subparser = parser.add_subparsers(dest='command')
+
+    # Create Command
+    create_parser = subparser.add_parser('create', help='create help')
+    create_parser.add_argument('ppa_name', metavar='ppa-name',
+                               action='store',
+                               help="Name of the PPA to be created")
+
+    # Desc Command
+    desc_parser = subparser.add_parser('desc', help='desc help')
+    desc_parser.add_argument('ppa_name', metavar='ppa-name',
+                             action='store',
+                             help="Name of the PPA to describe")
+    desc_parser.add_argument('description',
+                             nargs=argparse.REMAINDER)
+
+    # Destroy Command
+    destroy_parser = subparser.add_parser('destroy', help='destroy help')
+    destroy_parser.add_argument('ppa_name', metavar='ppa-name',
+                                action='store',
+                                help="Name of the PPA to destroy")
+
+    # List Command
+    list_parser = subparser.add_parser('list', help='list help')
+    list_parser.add_argument('ppa_name', metavar='ppa-name',
+                             action='store',
+                             help="Name of the PPA to list")
+
+    # Show Command
+    show_parser = subparser.add_parser('show', help='show help')
+    show_parser.add_argument('ppa_name', metavar='ppa-name',
+                             action='store',
+                             help="Name of the PPA to show")
+
+    # Status Command
+    status_parser = subparser.add_parser('status', help='status help')
+    status_parser.add_argument('ppa_name', metavar='ppa-name',
+                               action='store',
+                               help="Name of the PPA to report status of")
+
+    # Tests Command
+    tests_parser = subparser.add_parser('tests', help='tests help')
+    tests_parser.add_argument('ppa_name', metavar='ppa-name',
+                              action='store',
+                              help="Name of the PPA to view tests")
+
+    # Wait Command
+    wait_parser = subparser.add_parser('wait', help='wait help')
+    wait_parser.add_argument('ppa_name', metavar='ppa-name',
+                             action='store',
+                             help="Name of the PPA to wait on")
+
     return parser
 
 
@@ -274,7 +325,7 @@ def command_desc(lp, config):
     if not sys.stdin.isatty():
         description = sys.stdin.read()
     else:
-        description = ' '.join(config.get('remainder', None))
+        description = ' '.join(config.get('description', None))
 
     if not description or len(description) < 3:
         warn('No description provided')
diff --git a/tests/test_scripts_ppa.py b/tests/test_scripts_ppa.py
index d344340..a582fca 100644
--- a/tests/test_scripts_ppa.py
+++ b/tests/test_scripts_ppa.py
@@ -8,11 +8,14 @@
 # Released under GNU GPLv2 or later, read the file 'LICENSE.GPLv2+' for
 # more information.
 
+"""ppa command-line script tests"""
+
 import os
 import sys
 import types
 
 import importlib.machinery
+import argparse
 import pytest
 
 SCRIPT_NAME = 'ppa'
@@ -85,16 +88,63 @@ def fake_bug_report():
 
 
 def test_create_arg_parser():
-    # parser = script.create_arg_parser()
-
-    # TODO: Don't I have an existing example arg-parse test somewhere?
-    # TODO: Check command is recognized
-    # TODO: Check --config
-    # TODO: Check --debug
-    # TODO: Check --version
-    # TODO: Check --dry-run
-    # TODO: Check ppa_name
-    # TODO: Check remainder
+    """
+    Checks that the main argument processor is created properly.
+    It must support the top level options as well as the expected
+    set of subparsers.
+
+    Note we don't test --help or --version since these are built-ins
+    from argparse so we don't control their behavior.  They also both
+    exit when done which is not conducive to testing.
+    """
+    parser = script.create_arg_parser()
+
+    # Check command is recognized
+    args = parser.parse_args(['test-command'])
+    assert args.command[0] == 'test-command'
+
+    # Check -D, --debug
+    args = parser.parse_args(['cmd', '-D'])
+    assert args.debug == True
+    args.debug = None
+    args = parser.parse_args(['cmd', '--debug'])
+    assert args.debug == True
+    args.debug = None
+
+    # Check -v, --verbose
+    args = parser.parse_args(['cmd', '-v'])
+    assert args.verbose == True
+    args.verbose = None
+    args = parser.parse_args(['cmd', '--verbose'])
+    assert args.verbose == True
+    args.verbose = None
+
+    # Check -q, --quiet
+    args = parser.parse_args(['cmd', '-q'])
+    assert args.quiet == True
+    args.quiet = None
+    args = parser.parse_args(['cmd', '--quiet'])
+    assert args.quiet == True
+    args.quiet = None
+
+    # Verify all expected subparsers are present
+    subparsers_actions = [
+        action for action in parser._actions
+        if isinstance(action, argparse._SubParsersAction)]
+    subparsers = []
+    for subparsers_action in subparsers_actions:
+        for choice, subparser in subparsers_action.choices.items():
+            subparsers.append(choice)
+    assert subparsers == [
+        'create',
+        'desc',
+        'destroy',
+        'list',
+        'show',
+        'status',
+        'tests',
+        'wait'
+    ]
 
     pass