Merge ppa-dev-tools:set-command-basic-options into ppa-dev-tools:main

[Bryce Harrington]

True testing of all this stuff requires running against Launchpad, which
the unit tests don't do. Here's some example commandlines I used in my
own testing:

    # Basic creation
    $ ./scripts/ppa create test-set-command

    # Architectures
    $ ./scripts/ppa set test-set-command --architecture armhf,powerpc,s390x
    $ ./scripts/ppa set test-set-command --default-architectures
    $ ./scripts/ppa set test-set-command --all-architectures

After running each command, load up the PPA webpage in the browser and
verify the settings took. I didn't run into any problems with any of
these, and would expect them to Just Work(tm) for everyone, so if you
spot any issues let me know.

[Andreas Hasenack (ahasenack)]

review: Needs Information

[Bryce Harrington (bryce)]

Fixed help text for --disable

[Andreas Hasenack (ahasenack)]

+1

review: Approve

[Bryce Harrington (bryce)]

Total 0 (delta 0), reused 0 (delta 0)
To git+ssh://git.launchpad.net/ppa-dev-tools
   01d9b53..321446a main -> main

diff --git a/ppa/constants.py b/ppa/constants.py
index f6e7b01..4270392 100644
--- a/ppa/constants.py
+++ b/ppa/constants.py
@@ -12,7 +12,8 @@
 """Global constants"""
 
 ARCHES_ALL = ["amd64", "arm64", "armhf", "armel", "i386", "powerpc", "ppc64el", "s390x", "riscv64"]
-ARCHES_PPA = ["amd64", "arm64", "armhf", "i386", "powerpc", "ppc64el", "s390x"]
+ARCHES_PPA_DEFAULT = ["amd64", "i386"]
+ARCHES_PPA_ALL = ["amd64", "arm64", "armhf", "i386", "powerpc", "ppc64el", "s390x"]
 ARCHES_PPA_EXTRA = ["riscv64"]
 ARCHES_AUTOPKGTEST = ["amd64", "arm64", "armhf", "i386", "ppc64el", "s390x"]
 
diff --git a/ppa/ppa_group.py b/ppa/ppa_group.py
index 622cf76..7bccbb9 100755
--- a/ppa/ppa_group.py
+++ b/ppa/ppa_group.py
@@ -88,7 +88,7 @@ class PpaGroup:
         """
         return self.service.people[self.name]
 
-    def create(self, ppa_name='ppa', ppa_description=None):
+    def create(self, ppa_name='ppa', ppa_description=None, **kwargs):
         """Registers a new PPA with Launchpad.
 
         If a description is not provided a default one will be generated.
@@ -100,16 +100,18 @@ class PpaGroup:
 
         :raises PpaAlreadyExists: Raised if a PPA by this name already exists in Launchpad.
         """
-        ppa_displayname = ''
-        if ppa_description is None:
-            ppa_description = ppa_name
-            ppa_displayname = ppa_name
+        ppa_settings = {
+            'description': ppa_description,
+            'displayname': ppa_name,
+        }
+        for k in kwargs.keys():
+            ppa_settings[k] = kwargs[k]
 
         try:
             self.team.createPPA(
                 name=ppa_name,
-                description=ppa_description,
-                displayname=ppa_displayname)
+                **ppa_settings
+            )
             self.team.lp_save()
         except BadRequest as e:
             if "You already have a PPA" in o2str(e.content):
diff --git a/scripts/ppa b/scripts/ppa
index a3bad54..fbf65f2 100755
--- a/scripts/ppa
+++ b/scripts/ppa
@@ -65,7 +65,8 @@ if '__file__' in globals():
 
 from ppa._version import __version__
 from ppa.constants import (
-    ARCHES_PPA,
+    ARCHES_PPA_ALL,
+    ARCHES_PPA_DEFAULT,
     ARCHES_AUTOPKGTEST,
     URL_AUTOPKGTEST,
 )
@@ -137,6 +138,69 @@ def add_global_options(parser) -> None:
                         help="Minimize output during processing")
 
 
+def add_basic_config_options(parser) -> None:
+    """Adds to a parser the command line options to configure the PPA.
+
+    The config options are supported by the 'create' and 'set' command,
+    to allow configuring the PPA at creation time, or after, respectively.
+
+    These options represent what can be set as a user from the Launchpad
+    web interface.
+    """
+    # Architectures
+    parser.add_argument(
+        '-a', '--arches', '--arch', '--architectures',
+        dest="architectures",
+        action='store',
+        default=None,
+        help="Comma-separated list of hardware architectures to use"
+    )
+    parser.add_argument(
+        '--all-arches', '--all-architectures',
+        dest="architectures",
+        action='store_const',
+        const=','.join(ARCHES_PPA_ALL),
+        help="Enable all available architectures for the PPA"
+    )
+    parser.add_argument(
+        '--default-arches', '--default-architectures',
+        dest="architectures",
+        action='store_const',
+        const=','.join(ARCHES_PPA_DEFAULT),
+        help="Enable all available architectures for the PPA"
+    )
+
+    # Displayname
+    parser.add_argument(
+        '--displayname',
+        dest="displayname",
+        action='store',
+        help="A short title for the PPA's web page."
+    )
+
+    # Description
+    parser.add_argument(
+        '--description',
+        dest="description",
+        action='store',
+        help="A short description of the archive.  URLs will be rendered as links.  (See also 'ppa desc'.)"
+    )
+
+    # Enable/Disable uploading
+    parser.add_argument(
+        '--enable',
+        dest="set_enabled",
+        action='store_true',
+        help="Accept and build packages uploaded to the PPA."
+    )
+    parser.add_argument(
+        '--disable',
+        dest="set_disabled",
+        action='store_true',
+        help="Do not accept or build packages uploaded to the PPA."
+    )
+
+
 def create_arg_parser():
     """Sets up the command line parser object.
 
@@ -164,9 +228,7 @@ def create_arg_parser():
     create_parser.add_argument('ppa_name', metavar='ppa-name',
                                action='store',
                                help="Name of the PPA to be created")
-    create_parser.add_argument('-a', '--arches', '--arch', '--architectures',
-                               dest="architectures", action='store',
-                               help="Comma-separated list of hardware architectures to use")
+    add_basic_config_options(create_parser)
 
     # Desc Command
     desc_parser = subparser.add_parser(
@@ -207,6 +269,19 @@ def create_arg_parser():
                              nargs='?', default='me',
                              help="Name of the PPA to list")
 
+    # Set Command
+    set_parser = subparser.add_parser(
+        'set',
+        argument_default=argparse.SUPPRESS,
+        help='set help',
+        prog=progname,
+    )
+    add_global_options(set_parser)
+    set_parser.add_argument('ppa_name', metavar='ppa-name',
+                            action='store',
+                            help="Name of the PPA to be set config values on")
+    add_basic_config_options(set_parser)
+
     # Show Command
     show_parser = subparser.add_parser(
         'show',
@@ -351,6 +426,13 @@ def create_config(lp, args):
             else:
                 warn(f"Invalid releases '{args.releases}'")
                 return None
+    elif args.command == "set":
+        if args.architectures is not None:
+            if args.architectures:
+                config['architectures'] = args.architectures.split(',')
+            else:
+                warn(f"Invalid architectures '{args.architectures}'")
+                return None
 
     return config
 
@@ -382,7 +464,7 @@ def command_create(lp, config):
         warn("Could not determine team name")
         return os.EX_USAGE
 
-    architectures = config.get('architectures', ARCHES_PPA)
+    architectures = config.get('architectures', ARCHES_PPA_ALL)
 
     try:
         if not config.get('dry_run', False):
@@ -519,6 +601,38 @@ def command_exists(lp, config):
     return 1
 
 
+def command_set(lp, config):
+    """Sets one or more properties of PPA in Launchpad.
+
+    :param Lp lp: The Launchpad wrapper object.
+    :param dict config: Configuration param:value map.
+    :rtype: int
+    :returns: Status code OK (0) on success, non-zero on error.
+    """
+    try:
+        the_ppa = get_ppa(lp, config)
+
+        if 'architectures' in config:
+            the_ppa.set_architectures(config['architectures'])
+
+        if 'description' in config:
+            the_ppa.archive.description = config['description']
+
+        if 'displayname' in config:
+            the_ppa.archive.displayname = config['displayname']
+
+        return the_ppa.archive.lp_save()
+    except PpaDoesNotExist as e:
+        print(e)
+    except ValueError as e:
+        print(f"Error: {e}")
+        return os.EX_USAGE
+    except KeyboardInterrupt:
+        return 2
+    print("Unhandled error")
+    return 1
+
+
 def command_show(lp, config):
     """Displays details about the given PPA.
 
@@ -752,6 +866,7 @@ COMMANDS = {
     'desc':       (command_desc, None),
     'destroy':    (command_destroy, None),
     'list':       (command_list, None),
+    'set':        (command_set, None),
     'show':       (command_show, None),
     'status':     (command_status, None),
     'tests':      (command_tests, None),
diff --git a/tests/helpers.py b/tests/helpers.py
index 30a82bb..c6a55eb 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -18,6 +18,16 @@ from ppa.ppa import Ppa
 from ppa.ppa_group import PpaAlreadyExists
 
 
+class ArchiveMock:
+    """A stand-in for a Launchpad Archive object."""
+    def __init__(self, name, description):
+        self.displayname = name
+        self.description = description
+
+    def lp_save(self):
+        return True
+
+
 class PersonMock:
     """A stand-in for a Launchpad Person object."""
     def __init__(self, name):
@@ -32,6 +42,12 @@ class PersonMock:
         self._ppas.append(new_ppa)
         return True
 
+    def getPPAByName(self, name):
+        for ppa in self._ppas:
+            if ppa.name == name:
+                return ArchiveMock(name, ppa.description)
+        return None
+
     def lp_save(self):
         return True
 
diff --git a/tests/test_ppa_group.py b/tests/test_ppa_group.py
index 9bfd382..d9d83de 100644
--- a/tests/test_ppa_group.py
+++ b/tests/test_ppa_group.py
@@ -69,24 +69,6 @@ def test_create_with_team():
     assert ppa.address == 'ppa:test_team_name/ppa_test_name'
 
 
-def test_create_for_lpbug():
-    """Check associating a bug # when creating a PPA."""
-    ppa_group = PpaGroup(service=LpServiceMock(), name='me')
-    lpbug = '1234567'
-    ppa = ppa_group.create('lp' + lpbug)
-    assert ppa is not None
-    assert lpbug in ppa.description
-
-
-def test_create_for_merge_proposal():
-    """Check associating a merge proposal when creating a PPA."""
-    ppa_group = PpaGroup(service=LpServiceMock(), name='me')
-    version = '1.2.3-4'
-    ppa = ppa_group.create('merge.' + version)
-    assert ppa is not None
-    assert version in ppa.description
-
-
 def test_list_ppas():
     """Check listing the PPAs for a PPA group."""
     test_ppa_list = ['a', 'b', 'c', 'd']
diff --git a/tests/test_scripts_ppa.py b/tests/test_scripts_ppa.py
index e393bd8..10acc30 100644
--- a/tests/test_scripts_ppa.py
+++ b/tests/test_scripts_ppa.py
@@ -11,6 +11,7 @@
 """ppa command-line script tests"""
 
 import os
+import io
 import sys
 import types
 
@@ -22,6 +23,12 @@ SCRIPT_NAME = "ppa"
 BASE_PATH = os.path.realpath(os.path.join(os.path.dirname(os.path.realpath(__file__)), ".."))
 sys.path.insert(0, BASE_PATH)
 
+from ppa.constants import (
+    ARCHES_PPA_ALL,
+    ARCHES_PPA_DEFAULT
+)
+from tests.helpers import LpServiceMock
+
 if '.pybuild' in BASE_PATH:
     python_version = '.'.join([str(v) for v in sys.version_info[0:2]])
     scripts_path = os.path.join(
@@ -41,7 +48,7 @@ loader.exec_module(script)
 def fake_config():
     return {
         'ppa_name': 'testing',
-        'team_name': 'tester',
+        'team_name': 'me',
         'wait_seconds': 0.1
     }
 
@@ -145,6 +152,7 @@ def test_create_arg_parser():
         'desc',
         'destroy',
         'list',
+        'set',
         'show',
         'status',
         'tests',
@@ -152,36 +160,80 @@ def test_create_arg_parser():
     ]
 
 
-def test_create_arg_parser_create():
-    """Checks argument parsing for the 'create' command."""
[e-mail]('command', ['create', 'set'])
+def test_create_arg_parser_basic_config(command):
+    """Checks argument parsing for the basic PPA config options.
+
+    This test covers the set of options used by 'create' and 'set' for
+    configuring various properties of the PPA's behaviors, such as
+    dependencies, publication policy, access control, etc.  It does not
+    cover settings that require Launchpad administrator involvement.
+
+    The testing checks only that the options are being received and
+    registered as expected, and does not cover the processing of the
+    inputs nor the actual underlying functionality.
+    """
     parser = script.create_arg_parser()
 
-    # Check ppa_name
-    args = parser.parse_args(['create', 'test-ppa'])
+    # Check command and ppa_name
+    args = parser.parse_args([command, 'test-ppa'])
+    assert args.command == command
     assert args.ppa_name == 'test-ppa'
 
     # Check that command args can come before or after the ppa name
-    args = parser.parse_args(['create', 'test-ppa', '-a', 'x'])
+    args = parser.parse_args([command, 'test-ppa', '-a', 'x'])
     assert args.architectures == 'x'
     args.architectures = None
-    args = parser.parse_args(['create', '-a', 'x', 'test-ppa'])
+    args = parser.parse_args([command, '-a', 'x', 'test-ppa'])
     assert args.architectures == 'x'
     args.architectures = None
 
+    # Check --all-arches and --default-arches
+    args = parser.parse_args([command, 'test-ppa', '--all-arches'])
+    assert args.architectures == ','.join(ARCHES_PPA_ALL)
+    args.architectures = None
+    args = parser.parse_args([command, 'test-ppa', '--all-architectures'])
+    assert args.architectures == ','.join(ARCHES_PPA_ALL)
+    args.architectures = None
+    args = parser.parse_args([command, 'test-ppa', '--default-arches'])
+    assert args.architectures == ','.join(ARCHES_PPA_DEFAULT)
+    args.architectures = None
+    args = parser.parse_args([command, 'test-ppa', '--default-architectures'])
+    assert args.architectures == ','.join(ARCHES_PPA_DEFAULT)
+    args.architectures = None
+
     # Check -a, --arch, --arches, --architectures
-    args = parser.parse_args(['create', 'test-ppa', '-a', 'x'])
+    args = parser.parse_args([command, 'test-ppa', '-a', 'x'])
     assert args.architectures == 'x'
     args.architectures = None
-    args = parser.parse_args(['create', 'test-ppa', '--arch', 'x'])
+    args = parser.parse_args([command, 'test-ppa', '--arch', 'x'])
     assert args.architectures == 'x'
     args.architectures = None
-    args = parser.parse_args(['create', 'test-ppa', '--arches', 'x'])
+    args = parser.parse_args([command, 'test-ppa', '--arches', 'x'])
     assert args.architectures == 'x'
     args.architectures = None
-    args = parser.parse_args(['create', 'test-ppa', '--architectures', 'a,b,c'])
+    args = parser.parse_args([command, 'test-ppa', '--architectures', 'a,b,c'])
     assert args.architectures == 'a,b,c'
     args.architectures = None
 
+    # Check --displayname
+    args = parser.parse_args([command, 'test-ppa', '--displayname', 'x'])
+    assert args.displayname == 'x'
+    args.displayname = None
+
+    # Check --description
+    args = parser.parse_args([command, 'test-ppa', '--description', 'x'])
+    assert args.description == 'x'
+    args.description = None
+
+    # Check --enable / -D|--disable
+    args = parser.parse_args([command, 'test-ppa', '--enable'])
+    assert args.set_enabled is True
+    args.set_enabled = False
+    args = parser.parse_args([command, 'test-ppa', '--disable'])
+    assert args.set_disabled is True
+    args.set_disabled = False
+
 
 def test_create_arg_parser_show():
     """Checks argument parsing for the 'show' command."""
@@ -312,16 +364,18 @@ def test_create_config():
     pass
 
 
[e-mail](reason="Unimplemented")
-def test_command_create(fake_config):
-    assert script.command_create(fake_config) == 0
-
-    # TODO: Specify a ppa_name and verify it gets used properly
-    # TODO: Specify a team name
-    # TODO: Verify if no team name specified, the lp_user is used
-    #   instead
-    # TODO: Verify the ppa_address is set as expected
-    pass
+def test_command_create(fake_config, monkeypatch):
+    lp = LpServiceMock()
+    monkeypatch.setattr("sys.stdin", io.StringIO('test description'))
+    assert script.command_create(lp, fake_config) == 0
+
+    the_ppa = lp.me.getPPAByName(fake_config['ppa_name'])
+
+    # Check basic attributes (further coverage is in test_ppa_group.py)
+    assert the_ppa.displayname == fake_config['ppa_name']
+    assert the_ppa.description == 'test description'
+
+    # TODO: Check processors
 
 
 @pytest.mark.xfail(reason="Unimplemented")