5 files changed, 131 insertions, 116 deletions

diff --git a/src/mailman/utilities/i18n.py b/src/mailman/utilities/i18n.py
index f3c239d51..31c371172 100644
--- a/src/mailman/utilities/i18n.py
+++ b/src/mailman/utilities/i18n.py
@@ -35,7 +35,7 @@ class TemplateNotFoundError(MailmanError):
     def __init__(self, template_file):
         self.template_file = template_file
 
-    def __str__(self):                              # pragma: no cover
+    def __str__(self):                              # pragma: nocover
         return self.template_file
 
 
diff --git a/src/mailman/utilities/modules.py b/src/mailman/utilities/modules.py
index 83a4384b7..155a34ed2 100644
--- a/src/mailman/utilities/modules.py
+++ b/src/mailman/utilities/modules.py
@@ -20,6 +20,8 @@
 import os
 import sys
 
+from contextlib import contextmanager
+from importlib import import_module
 from pkg_resources import resource_filename, resource_listdir
 from public import public
 
@@ -46,9 +48,9 @@ def find_name(dotted_name):
     :return: The object.
     :rtype: object
     """
-    package_path, dot, object_name = dotted_name.rpartition('.')
-    __import__(package_path)
-    return getattr(sys.modules[package_path], object_name)
+    module_path, dot, object_name = dotted_name.rpartition('.')
+    module = import_module(module_path)
+    return getattr(module, object_name)
 
 
 @public
@@ -87,7 +89,7 @@ def scan_module(module, interface):
     for name in module.__all__:
         component = getattr(module, name, missing)
         assert component is not missing, (
-            '%s has bad __all__: %s' % (module, name))   # pragma: no cover
+            '%s has bad __all__: %s' % (module, name))   # pragma: nocover
         if (interface.implementedBy(component)
                 # We cannot use getattr() here because that will return True
                 # for all subclasses.  __abstract_component__ should *not* be
@@ -166,3 +168,17 @@ def expand_path(url):
         return resource_filename(package, resource + '.cfg')
     else:
         return url
+
+
+@public
+@contextmanager
+def hacked_sys_modules(name, module):
+    old_module = sys.modules.get(name)
+    sys.modules[name] = module
+    try:
+        yield
+    finally:
+        if old_module is None:
+            del sys.modules[name]
+        else:
+            sys.modules[name] = old_module
diff --git a/src/mailman/utilities/options.py b/src/mailman/utilities/options.py
index 3d8392486..7f1f5a1c5 100644
--- a/src/mailman/utilities/options.py
+++ b/src/mailman/utilities/options.py
@@ -15,123 +15,59 @@
 # You should have received a copy of the GNU General Public License along with
 # GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Common argument parsing."""
+"""Argument parsing utilities."""
 
-import os
-import sys
+import click
 
-from copy import copy
-from mailman.config import config
 from mailman.core.i18n import _
-from mailman.core.initialize import initialize
-from mailman.version import MAILMAN_VERSION
-from optparse import Option, OptionParser, OptionValueError
 from public import public
 
 
-def check_unicode(option, opt, value):
-    """Check that the value is a unicode string."""
-    if not isinstance(value, bytes):
+@public
+def validate_runner_spec(ctx, param, value):
+    # This validator handles two cases.  First, for the runner script where
+    # only a single --runner option is allowed, and second the master script
+    # where multiple --runner options are allowed.  When no --runner options
+    # are given, we'll either get None or the empty tuple.  That's why we use
+    # false-iness here.
+    if not value:
         return value
-    try:
-        return value.decode(sys.getdefaultencoding())
-    except UnicodeDecodeError:
-        raise OptionValueError(
-            'option {}: Cannot decode: {}'.format(opt, value))
-
-
-def check_yesno(option, opt, value):
-    """Check that the value is 'yes' or 'no'."""
-    value = value.lower()
-    if value not in ('yes', 'no', 'y', 'n'):
-        raise OptionValueError('option {}: invalid: {}'.format(opt, value))
-    return value[0] == 'y'
-
-
-class MailmanOption(Option):
-    """Extension types for unicode options."""
-    TYPES = Option.TYPES + ('unicode', 'yesno')
-    TYPE_CHECKER = copy(Option.TYPE_CHECKER)
-    TYPE_CHECKER['unicode'] = check_unicode
-    TYPE_CHECKER['yesno'] = check_yesno
-
-
-class SafeOptionParser(OptionParser):
-    """A unicode-compatible `OptionParser`.
-
-    Python's standard option parser does not accept unicode options.  Rather
-    than try to fix that, this class wraps the add_option() method and saves
-    having to wrap the options in str() calls.
-    """
-    def add_option(self, *args, **kwargs):
-        """See `OptionParser`."""
-        # Check to see if the first or first two options are unicodes and turn
-        # them into 8-bit strings before calling the superclass's method.
-        if len(args) == 0:
-            return OptionParser.add_option(self, *args, **kwargs)
-        old_args = list(args)
-        new_args = []
-        arg0 = old_args.pop(0)
-        new_args.append(str(arg0))
-        if len(old_args) > 0:
-            arg1 = old_args.pop(0)
-            new_args.append(str(arg1))
-        new_args.extend(old_args)
-        return OptionParser.add_option(self, *new_args, **kwargs)
+    specs = []
+    for spec in ([value] if isinstance(value, str) else value):
+        parts = spec.split(':')
+        if len(parts) == 1:
+            specs.append((parts[0], 1, 1))
+        elif len(parts) == 3:
+            runner = parts[0]
+            try:
+                rslice = int(parts[1])
+                rrange = int(parts[2])
+            except ValueError:
+                raise click.BadParameter(
+                    _('slice and range must be integers: $value'))
+            specs.append((runner, rslice, rrange))
+        else:
+            raise click.UsageError(_('Bad runner spec: $value'))
+    return specs[0] if isinstance(value, str) else specs
 
 
 @public
-class Options:
-    """Common argument parser."""
-
-    # Subclasses should override.
-    usage = None
-
-    def __init__(self):
-        self.parser = SafeOptionParser(
-            version=MAILMAN_VERSION,
-            option_class=MailmanOption,
-            usage=self.usage)
-        self.add_common_options()
-        self.add_options()
-        options, arguments = self.parser.parse_args()
-        self.options = options
-        self.arguments = arguments
-        # Also, for convenience, place the options in the configuration file
-        # because occasional global uses are necessary.
-        config.options = self
-
-    def add_options(self):
-        """Allow the subclass to add its own specific arguments."""
-        pass
-
-    def sanity_check(self):
-        """Allow subclasses to do sanity checking of arguments."""
-        pass
-
-    def add_common_options(self):
-        """Add options common to all scripts."""
-        # Python requires str types here.
-        self.parser.add_option(
-            '-C', '--config',
-            help=_('Alternative configuration file to use'))
-
-    def initialize(self, propagate_logs=None):
-        """Initialize the configuration system.
-
-        After initialization of the configuration system, perform sanity
-        checks.  We do it in this order because some sanity checks require the
-        configuration to be initialized.
-
-        :param propagate_logs: Optional flag specifying whether log messages
-            in sub-loggers should be propagated to the master logger (and
-            hence to the root logger).  If not given, propagation is taken
-            from the configuration files.
-        :type propagate_logs: bool or None.
-        """
-        # Fall back to using the environment variable if -C is not given.
-        config_file = (os.getenv('MAILMAN_CONFIG_FILE')
-                       if self.options.config is None
-                       else self.options.config)
-        initialize(config_file, propagate_logs=propagate_logs)
-        self.sanity_check()
+class I18nCommand(click.Command):                   # pragma: nocover
+    # https://github.com/pallets/click/issues/834
+    #
+    # Note that this handles the case for the `mailman <subcommand> --help`
+    # output.  To handle `mailman --help` we override the same method in the
+    # `Subcommands` subclass over in src/mailman/bin/mailman.py.  The test
+    # suite doesn't cover *this* copy of the method but who cares, since it
+    # will hopefully go away some day.
+    def format_options(self, ctx, formatter):
+        """Writes all the options into the formatter if they exist."""
+        opts = []
+        for param in self.get_params(ctx):
+            rv = param.get_help_record(ctx)
+            if rv is not None:
+                part_a, part_b = rv
+                opts.append((part_a, part_b.replace('\n', ' ')))
+        if opts:
+            with formatter.section('Options'):
+                formatter.write_dl(opts)
diff --git a/src/mailman/utilities/tests/test_modules.py b/src/mailman/utilities/tests/test_modules.py
index 20735486c..82d44cff4 100644
--- a/src/mailman/utilities/tests/test_modules.py
+++ b/src/mailman/utilities/tests/test_modules.py
@@ -23,7 +23,8 @@ import unittest
 
 from contextlib import ExitStack, contextmanager
 from mailman.interfaces.styles import IStyle
-from mailman.utilities.modules import add_components, find_components
+from mailman.utilities.modules import (
+    add_components, find_components, hacked_sys_modules)
 from pathlib import Path
 from tempfile import TemporaryDirectory
 
@@ -228,3 +229,15 @@ class StyleA2:
                     'previously <mypackage.components.StyleA1 object at '
                     '.*>'):
                 add_components('mypackage', IStyle, {})
+
+    def test_hacked_sys_modules(self):
+        self.assertIsNone(sys.modules.get('mailman.not_a_module'))
+        with hacked_sys_modules('mailman.not_a_module', object()):
+            self.assertIsNotNone(sys.modules.get('mailman.not_a_module'))
+
+    def test_hacked_sys_modules_restore(self):
+        email_package = sys.modules['email']
+        sentinel = object()
+        with hacked_sys_modules('email', sentinel):
+            self.assertEqual(sys.modules.get('email'), sentinel)
+        self.assertEqual(sys.modules.get('email'), email_package)
diff --git a/src/mailman/utilities/tests/test_options.py b/src/mailman/utilities/tests/test_options.py
new file mode 100644
index 000000000..5885972fa
--- /dev/null
+++ b/src/mailman/utilities/tests/test_options.py
@@ -0,0 +1,50 @@
+# Copyright (C) 2017 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman 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 General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test option utilities."""
+
+import click
+import unittest
+
+from mailman.utilities.options import validate_runner_spec
+
+
+class TestValidateRunnerSpec(unittest.TestCase):
+    def test_false_value(self):
+        self.assertIsNone(validate_runner_spec(None, None, None))
+
+    def test_runner_only(self):
+        specs = validate_runner_spec(None, None, 'incoming')
+        self.assertEqual(specs, ('incoming', 1, 1))
+
+    def test_full_runner_spec(self):
+        specs = validate_runner_spec(None, None, 'incoming:2:4')
+        self.assertEqual(specs, ('incoming', 2, 4))
+
+    def test_bad_runner_spec(self):
+        with self.assertRaises(click.BadParameter) as cm:
+            validate_runner_spec(None, None, 'incoming:not:int')
+        self.assertEqual(
+            cm.exception.message,
+            'slice and range must be integers: incoming:not:int')
+
+    def test_bad_runner_spec_parts(self):
+        with self.assertRaises(click.UsageError) as cm:
+            validate_runner_spec(None, None, 'incoming:2')
+        self.assertEqual(
+            cm.exception.message,
+            'Bad runner spec: incoming:2')