* `bin/runner` command has been simplified and its command line options

   reduced.  Now, only one `-r/--runner` option may be provided and the
   round-robin feature has been removed.
 * Fixed REST server crash on `reopen` command.  Identification and test
   provided by Aurélien Bompard.  (LP: #1184376)

Also:

 * bin/runner now uses standard argparse instead of ScriptOptions.
 * The entire bin/runner machinery has bee reorganized and simplified.  There
 * is no more Loop class.  Signal setting is moved directly into the base
   Runner class and overrided in specific subclasses (e.g. RESTRunner which
   must cleanly shutdown its TCPServer).  The runner exit status is now set
   directly on the Runner instance.
 * Fixed a few minor style issues.
 * In order to cleanly shutdown the RESTRunner's WSGI server, we must start a
   subthread which only watches for an Event and then calls the server's
   shutdown() method.  It has to be this way because the WSGI server itself
   (due to interactions with SQLite), and the signal handlers (due to Python's
   signal handling semantics) must both run in the main thread.  However, the
   shutdown() must be invoked from a subthread in order to prevent deadlock.
 * Refactor the RESTLayer to eliminate duplication of code.

10 files changed, 243 insertions, 247 deletions

diff --git a/src/mailman/bin/runner.py b/src/mailman/bin/runner.py
index 47133bff7..d6a1dd6db 100644
--- a/src/mailman/bin/runner.py
+++ b/src/mailman/bin/runner.py
@@ -29,115 +29,37 @@ import os
 import sys
 import signal
 import logging
+import argparse
 import traceback
 
 from mailman.config import config
 from mailman.core.i18n import _
-from mailman.core.logging import reopen
-from mailman.options import Options
+from mailman.core.initialize import initialize
 from mailman.utilities.modules import find_name
+from mailman.version import MAILMAN_VERSION_FULL
 
 
 log = None
 
 
 
-def r_callback(option, opt, value, parser):
+class ROptionAction(argparse.Action):
     """Callback for -r/--runner option."""
-    dest = getattr(parser.values, option.dest)
-    parts = value.split(':')
-    if len(parts) == 1:
-        runner = parts[0]
-        rslice = rrange = 1
-    elif len(parts) == 3:
-        runner = parts[0]
-        try:
-            rslice = int(parts[1])
-            rrange = int(parts[2])
-        except ValueError:
-            parser.print_help()
-            print >> sys.stderr, _('Bad runner specification: $value')
-            sys.exit(1)
-    else:
-        parser.print_help()
-        print >> sys.stderr, _('Bad runner specification: $value')
-        sys.exit(1)
-    dest.append((runner, rslice, rrange))
-
-
-
-class ScriptOptions(Options):
-    """Options for bin/runner."""
-    usage = _("""\
-Start one or more runners.
-
-The runner named on the command line is started, and it can either run through
-its main loop once (for those runners that support this) or continuously.  The
-latter is how the master runner starts all its subprocesses.
-
-When more than one runner is specified on the command line, they are each run
-in round-robin fashion.  All runners must support running its main loop once.
-In other words, the first named runner is run once.  When that runner is done,
-the next one is run to consume all the files in *its* directory, and so on.
-The number of total iterations can be given on the command line.  This mode of
-operation is primarily for debugging purposes.
-
-Usage: %prog [options]
-
--r is required unless -l or -h is given, and its argument must be one of the
-names displayed by the -l switch.
-
-Normally, this script should be started from 'bin/mailman start'.  Running it
-separately or with -o is generally useful only for debugging.  When run this
-way, the environment variable $MAILMAN_UNDER_MASTER_CONTROL will be set which
-subtly changes some error handling behavior.
-""")
-
-    def add_options(self):
-        """See `Options`."""
-        self.parser.add_option(
-            '-r', '--runner',
-            metavar='runner[:slice:range]', dest='runners',
-            type='string', default=[],
-            action='callback', callback=r_callback,
-            help=_("""\
-Start the named runner, which must be one of the strings returned by the -l
-option.
-
-For runners that manage a queue directory, optional slice:range if given, is
-used to assign multiple runner processes to that queue.  range is the total
-number of runners for the queue while slice is the number of this runner from
-[0..range).  For runners that do not manage a queue, slice and range are
-ignored.
-
-When using the slice:range form, you must ensure that each runner for the
-queue is given the same range value.  If slice:runner is not given, then 1:1
-is used.
-
-Multiple -r options may be given, in which case each runner will run once in
-round-robin fashion.  The special runner 'All' is shorthand for running all
-named runners listed by the -l option."""))
-        self.parser.add_option(
-            '-o', '--once',
-            default=False, action='store_true', help=_("""\
-Run each named runner exactly once through its main loop.  Otherwise, each
-runner runs indefinitely, until the process receives signal.  This is not
-compatible with runners that cannot be run once."""))
-        self.parser.add_option(
-            '-l', '--list',
-            default=False, action='store_true',
-            help=_('List the available runner names and exit.'))
-        self.parser.add_option(
-            '-v', '--verbose',
-            default=0, action='count', help=_("""\
-Display more debugging information to the log file."""))
-
-    def sanity_check(self):
-        """See `Options`."""
-        if self.arguments:
-            self.parser.error(_('Unexpected arguments'))
-        if not self.options.runners and not self.options.list:
-            self.parser.error(_('No runner name given.'))
+    def __call__(self, parser, namespace, values, option_string=None):
+        parts = values.split(':')
+        if len(parts) == 1:
+            runner = parts[0]
+            rslice = rrange = 1
+        elif len(parts) == 3:
+            runner = parts[0]
+            try:
+                rslice = int(parts[1])
+                rrange = int(parts[2])
+            except ValueError:
+                parser.error(_('Bad runner specification: $value'))
+        else:
+            parser.error(_('Bad runner specification: $value'))
+        setattr(namespace, self.dest, (runner, rslice, rrange))
 
 
 
@@ -175,53 +97,90 @@ def make_runner(name, slice, range, once=False):
 
 
 
-def set_signals(loop):
-    """Set up the signal handlers.
+def main():
+    global log
 
-    Signals caught are: SIGTERM, SIGINT, SIGUSR1 and SIGHUP.  The latter is
-    used to re-open the log files.  SIGTERM and SIGINT are treated exactly the
-    same -- they cause the runner to exit with no restart from the master.
-    SIGUSR1 also causes the runner to exit, but the master watcher will
-    restart it in that case.
+    parser = argparse.ArgumentParser(
+        description=_("""\
+        Start a runner
 
-    :param loop: A runner instance.
-    :type loop: `IRunner`
-    """
-    def sigterm_handler(signum, frame):
-        # Exit the runner cleanly
-        loop.stop()
-        loop.status = signal.SIGTERM
-        log.info('%s runner caught SIGTERM.  Stopping.', loop.name())
-    signal.signal(signal.SIGTERM, sigterm_handler)
-    def sigint_handler(signum, frame):
-        # Exit the runner cleanly
-        loop.stop()
-        loop.status = signal.SIGINT
-        log.info('%s runner caught SIGINT.  Stopping.', loop.name())
-    signal.signal(signal.SIGINT, sigint_handler)
-    def sigusr1_handler(signum, frame):
-        # Exit the runner cleanly
-        loop.stop()
-        loop.status = signal.SIGUSR1
-        log.info('%s runner caught SIGUSR1.  Stopping.', loop.name())
-    signal.signal(signal.SIGUSR1, sigusr1_handler)
-    # SIGHUP just tells us to rotate our log files.
-    def sighup_handler(signum, frame):
-        reopen()
-        log.info('%s runner caught SIGHUP.  Reopening logs.', loop.name())
-    signal.signal(signal.SIGHUP, sighup_handler)
+        The runner named on the command line is started, and it can
+        either run through its main loop once (for those runners that
+        support this) or continuously.  The latter is how the master
+        runner starts all its subprocesses.
 
+        -r is required unless -l or -h is given, and its argument must
+        be one of the names displayed by the -l switch.
 
-
-def main():
-    global log
+        Normally, this script should be started from 'bin/mailman
+        start'.  Running it separately or with -o is generally useful
+        only for debugging.  When run this way, the environment variable
+        $MAILMAN_UNDER_MASTER_CONTROL will be set which subtly changes
+        some error handling behavior.
+        """))
+    parser.add_argument(
+        '--version',
+        action='version', version=MAILMAN_VERSION_FULL,
+        help=_('Print this version string and exit'))
+    parser.add_argument(
+        '-C', '--config',
+        help=_("""\
+        Configuration file to use.  If not given, the environment variable
+        MAILMAN_CONFIG_FILE is consulted and used if set.  If neither are
+        given, a default configuration file is loaded."""))
+    parser.add_argument(
+        '-r', '--runner',
+        metavar='runner[:slice:range]', dest='runner',
+        action=ROptionAction, default=None,
+        help=_("""\
+        Start the named runner, which must be one of the strings
+        returned by the -l option.
+
+        For runners that manage a queue directory, optional
+        `slice:range` if given is used to assign multiple runner
+        processes to that queue.  range is the total number of runners
+        for the queue while slice is the number of this runner from
+        [0..range).  For runners that do not manage a queue, slice and
+        range are ignored.
 
-    options = ScriptOptions()
-    options.initialize()
+        When using the `slice:range` form, you must ensure that each
+        runner for the queue is given the same range value.  If
+        `slice:runner` is not given, then 1:1 is used.
+        """))
+    parser.add_argument(
+        '-o', '--once',
+        default=False, action='store_true', help=_("""\
+        Run the named runner exactly once through its main loop.
+        Otherwise, the runner runs indefinitely until the process
+        receives a signal.  This is not compatible with runners that
+        cannot be run once."""))
+    parser.add_argument(
+        '-l', '--list',
+        default=False, action='store_true',
+        help=_('List the available runner names and exit.'))
+    parser.add_argument(
+        '-v', '--verbose',
+        default=False, action='store_true', help=_("""\
+        Display more debugging information to the log file."""))
 
+    args = parser.parse_args()
+    if args.runner is None and not args.list:
+        parser.error(_('No runner name given.'))
+
+    # Initialize the system.  Honor the -C flag if given.
+    config_path = (None if args.config is None
+                   else os.path.abspath(os.path.expanduser(args.config)))
+    initialize(config_path, args.verbose)
     log = logging.getLogger('mailman.runner')
+    if args.verbose:
+        console = logging.StreamHandler(sys.stderr)
+        formatter = logging.Formatter(config.logging.root.format,
+                                      config.logging.root.datefmt)
+        console.setFormatter(formatter)
+        logging.getLogger().addHandler(console)
+        logging.getLogger().setLevel(logging.DEBUG)
 
-    if options.options.list:
+    if args.list:
         descriptions = {}
         for section in config.runner_configs:
             ignore, dot, shortname = section.name.rpartition('.')
@@ -234,56 +193,10 @@ def main():
             print _('$name runs $classname')
         sys.exit(0)
 
-    # Fast track for one infinite runner.
-    if len(options.options.runners) == 1 and not options.options.once:
-        runner = make_runner(*options.options.runners[0])
-        class Loop:
-            status = 0
-            def __init__(self, runner):
-                self._runner = runner
-            def name(self):
-                return self._runner.__class__.__name__
-            def stop(self):
-                self._runner.stop()
-        loop = Loop(runner)
-        if runner.intercept_signals:
-            set_signals(loop)
-        # Now start up the main loop
-        log.info('%s runner started.', loop.name())
-        runner.run()
-        log.info('%s runner exiting.', loop.name())
-    else:
-        # Anything else we have to handle a bit more specially.
-        runners = []
-        for runner, rslice, rrange in options.options.runners:
-            runner = make_runner(runner, rslice, rrange, once=True)
-            runners.append(runner)
-        # This class is used to manage the main loop.
-        class Loop:
-            status = 0
-            def __init__(self):
-                self._isdone = False
-            def name(self):
-                return 'Main loop'
-            def stop(self):
-                self._isdone = True
-            def isdone(self):
-                return self._isdone
-        loop = Loop()
-        if runner.intercept_signals:
-            set_signals(loop)
-        log.info('Main runner loop started.')
-        while not loop.isdone():
-            for runner in runners:
-                # In case the SIGTERM came in the middle of this iteration.
-                if loop.isdone():
-                    break
-                if options.options.verbose:
-                    log.info('Now doing a %s runner iteration',
-                             runner.__class__.__bases__[0].__name__)
-                runner.run()
-            if options.options.once:
-                break
-        log.info('Main runner loop exiting.')
-    # All done
-    sys.exit(loop.status)
+    runner = make_runner(*args.runner, once=args.once)
+    runner.set_signals()
+    # Now start up the main loop
+    log.info('%s runner started.', runner.name)
+    runner.run()
+    log.info('%s runner exiting.', runner.name)
+    sys.exit(runner.status)
diff --git a/src/mailman/core/runner.py b/src/mailman/core/runner.py
index a8412195d..347726b85 100644
--- a/src/mailman/core/runner.py
+++ b/src/mailman/core/runner.py
@@ -26,6 +26,7 @@ __all__ = [
 
 
 import time
+import signal
 import logging
 import traceback
 
@@ -37,6 +38,7 @@ from zope.interface import implementer
 
 from mailman.config import config
 from mailman.core.i18n import _
+from mailman.core.logging import reopen
 from mailman.core.switchboard import Switchboard
 from mailman.interfaces.languages import ILanguageManager
 from mailman.interfaces.listmanager import IListManager
@@ -46,12 +48,12 @@ from mailman.utilities.string import expand
 
 dlog = logging.getLogger('mailman.debug')
 elog = logging.getLogger('mailman.error')
+rlog = logging.getLogger('mailman.runner')
 
 
 
 @implementer(IRunner)
 class Runner:
-    intercept_signals = True
     is_queue_runner = True
 
     def __init__(self, name, slice=None):
@@ -85,10 +87,32 @@ class Runner:
         self.max_restarts = int(section.max_restarts)
         self.start = as_boolean(section.start)
         self._stop = False
+        self.status = 0
 
     def __repr__(self):
         return '<{0} at {1:#x}>'.format(self.__class__.__name__, id(self))
 
+    def signal_handler(self, signum, frame):
+        signame = {
+            signal.SIGTERM: 'SIGTERM',
+            signal.SIGINT: 'SIGINT',
+            signal.SIGUSR1: 'SIGUSR1',
+            }.get(signum, signum)
+        if signum in (signal.SIGTERM, signal.SIGINT, signal.SIGUSR1):
+            self.stop()
+            self.status = signum
+            rlog.info('%s runner caught %s.  Stopping.', self.name, signame)
+        elif signum == signal.SIGHUP:
+            reopen()
+            rlog.info('%s runner caught SIGHUP.  Reopening logs.', self.name)
+
+    def set_signals(self):
+        """See `IRunner`."""
+        signal.signal(signal.SIGHUP, self.signal_handler)
+        signal.signal(signal.SIGINT, self.signal_handler)
+        signal.signal(signal.SIGTERM, self.signal_handler)
+        signal.signal(signal.SIGUSR1, self.signal_handler)
+
     def stop(self):
         """See `IRunner`."""
         self._stop = True
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 97478e2ce..8faeb3074 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -130,6 +130,9 @@ Commands
    the Postfix `relay_domains` support.
  * `bin/mailman start` was passing the wrong relative path to its runner
    subprocesses when -C was given.  (LP: #982551)
+ * `bin/runner` command has been simplified and its command line options
+   reduced.  Now, only one `-r/--runner` option may be provided and the
+   round-robin feature has been removed.
 
 Other
 -----
@@ -144,6 +147,8 @@ Other
 Bugs
 ----
  * Fixed `send_goodbye_message()`.  (LP: #1091321)
+ * Fixed REST server crash on `reopen` command.  Identification and test
+   provided by Aurélien Bompard.  (LP: #1184376)
 
 
 3.0 beta 2 -- "Freeze"
diff --git a/src/mailman/handlers/cook_headers.py b/src/mailman/handlers/cook_headers.py
index 07235d6bd..6269f4c45 100644
--- a/src/mailman/handlers/cook_headers.py
+++ b/src/mailman/handlers/cook_headers.py
@@ -155,8 +155,8 @@ def process(mlist, msg, msgdata):
         # above code?
         # Also skip Cc if this is an anonymous list as list posting address
         # is already in From and Reply-To in this case.
-        if (mlist.personalize == Personalization.full and
-            mlist.reply_goes_to_list != ReplyToMunging.point_to_list and
+        if (mlist.personalize is Personalization.full and
+            mlist.reply_goes_to_list is not ReplyToMunging.point_to_list and
             not mlist.anonymous_list):
             # Watch out for existing Cc headers, merge, and remove dups.  Note
             # that RFC 2822 says only zero or one Cc header is allowed.
diff --git a/src/mailman/interfaces/runner.py b/src/mailman/interfaces/runner.py
index c54415c7a..a8b76652c 100644
--- a/src/mailman/interfaces/runner.py
+++ b/src/mailman/interfaces/runner.py
@@ -63,15 +63,16 @@ class IRunner(Interface):
         through the main loop.
         """)
 
-    # BAW 2013-05-30: but see LP: #1184376 for why this perhaps should be
-    # removed entirely.
-    intercept_signals = Attribute("""\
-        Should the runner mechanism intercept signals?
+    def set_signals():
+        """Set up the signal handlers necessary to control the runner.
 
-        In general, the runner catches SIGINT, SIGTERM, SIGUSR1, and SIGHUP to
-        manage the process.  Some runners need to manage their own signals,
-        and set this attribute to False.
-        """)
+        The runner should catch the following signals:
+        - SIGTERM and SIGINT: treated exactly the same, they cause the runner
+          to exit with no restart from the master.
+        - SIGUSR1: Also causes the runner to exit, but the master watcher will
+          retart it.
+        - SIGHUP: Re-open the log files.
+        """
 
     def _one_iteration():
         """The work done in one iteration of the main loop.
diff --git a/src/mailman/runners/lmtp.py b/src/mailman/runners/lmtp.py
index 31d7723f5..2bc3af979 100644
--- a/src/mailman/runners/lmtp.py
+++ b/src/mailman/runners/lmtp.py
@@ -156,12 +156,13 @@ class LMTPRunner(Runner, smtpd.SMTPServer):
 
     is_queue_runner = False
 
-    def __init__(self, slice=None, numslices=1):
+    def __init__(self, name, slice=None):
         localaddr = config.mta.lmtp_host, int(config.mta.lmtp_port)
         # Do not call Runner's constructor because there's no QDIR to create
         qlog.debug('LMTP server listening on %s:%s',
                    localaddr[0], localaddr[1])
         smtpd.SMTPServer.__init__(self, localaddr, remoteaddr=None)
+        super(LMTPRunner, self).__init__(name, slice)
 
     def handle_accept(self):
         conn, addr = self.accept()
diff --git a/src/mailman/runners/rest.py b/src/mailman/runners/rest.py
index 252e41ef0..67987805d 100644
--- a/src/mailman/runners/rest.py
+++ b/src/mailman/runners/rest.py
@@ -25,11 +25,9 @@ __all__ = [
     ]
 
 
-import sys
-import errno
-import select
 import signal
 import logging
+import threading
 
 from mailman.core.runner import Runner
 from mailman.rest.wsgiapp import make_server
@@ -40,25 +38,47 @@ log = logging.getLogger('mailman.http')
 
 
 class RESTRunner(Runner):
-    #intercept_signals = False
+    # Don't install the standard signal handlers because as defined, they
+    # won't actually stop the TCPServer started by .serve_forever().
     is_queue_runner = False
 
+    def __init__(self, name, slice=None):
+        """See `IRunner`."""
+        super(RESTRunner, self).__init__(name, slice)
+        # Both the REST server and the signal handlers must run in the main
+        # thread; the former because of SQLite requirements (objects created
+        # in one thread cannot be shared with the other threads), and the
+        # latter because of Python's signal handling semantics.
+        #
+        # Unfortunately, we cannot issue a TCPServer shutdown in the main
+        # thread, because that will cause a deadlock.  Yay.   So what we do is
+        # to use the signal handler to notify a shutdown thread that the
+        # shutdown should happen.  That thread will wake up and stop the main
+        # server.
+        self._server = make_server()
+        self._event = threading.Event()
+        def stopper(event, server):
+            event.wait()
+            server.shutdown()
+        self._thread = threading.Thread(
+            target=stopper, args=(self._event, self._server))
+        self._thread.start()
+
     def run(self):
-        log.info('Starting REST server')
-        # Handle SIGTERM the same way as SIGINT.
-        def stop_server(signum, frame):
-            log.info('REST server shutdown')
-            sys.exit(signal.SIGTERM)
-        signal.signal(signal.SIGTERM, stop_server)
-        try:
-            make_server().serve_forever()
-        except KeyboardInterrupt:
-            log.info('REST server interrupted')
-            sys.exit(signal.SIGINT)
-        except select.error as (errcode, message):
-            if errcode == errno.EINTR:
-                log.info('REST server exiting')
-                sys.exit(errno.EINTR)
-            raise
-        except:
-            raise
+        """See `IRunner`."""
+        self._server.serve_forever()
+
+    def signal_handler(self, signum, frame):
+        super(RESTRunner, self).signal_handler(signum, frame)
+        if signum in (signal.SIGTERM, signal.SIGINT, signal.SIGUSR1):
+            # Set the flag that will terminate the TCPserver loop.
+            self._event.set()
+
+    def _one_iteration(self):
+        # Just keep going
+        if self._thread.is_alive():
+            self._thread.join(timeout=0.1)
+        return 1
+
+    def _snooze(self, filecnt):
+        pass
diff --git a/src/mailman/runners/tests/test_rest.py b/src/mailman/runners/tests/test_rest.py
new file mode 100644
index 000000000..a7b51f720
--- /dev/null
+++ b/src/mailman/runners/tests/test_rest.py
@@ -0,0 +1,52 @@
+# Copyright (C) 2013 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 the REST runner."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'TestRESTRunner',
+    ]
+
+
+import os
+import signal
+import unittest
+
+from mailman.testing.helpers import call_api, wait_for_webservice
+from mailman.testing.layers import RESTLayer
+
+
+
+class TestRESTRunner(unittest.TestCase):
+    """Test the REST runner."""
+
+    layer = RESTLayer
+
+    def test_sighup_restart(self):
+        # The REST runner must survive a SIGHUP.
+        wait_for_webservice()
+        for pid in self.layer.server.runner_pids:
+            os.kill(pid, signal.SIGHUP)
+        wait_for_webservice()
+        # This should not raise an exception.  The best way to assert this is
+        # to ensure that the response is valid.
+        response, json = call_api('http://localhost:9001/3.0/system')
+        self.assertEqual(json['content-location'],
+                         'http://localhost:9001/3.0/system')
diff --git a/src/mailman/runners/tests/test_retry.py b/src/mailman/runners/tests/test_retry.py
index 5869f42a2..99e87235e 100644
--- a/src/mailman/runners/tests/test_retry.py
+++ b/src/mailman/runners/tests/test_retry.py
@@ -21,6 +21,7 @@ from __future__ import absolute_import, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'TestRetryRunner',
     ]
 
 
diff --git a/src/mailman/testing/layers.py b/src/mailman/testing/layers.py
index bee116b6c..e47d5c9e0 100644
--- a/src/mailman/testing/layers.py
+++ b/src/mailman/testing/layers.py
@@ -45,11 +45,9 @@ import logging
 import datetime
 import tempfile
 
-from base64 import b64encode
-from lazr.config import as_boolean, as_timedelta
+from lazr.config import as_boolean
 from pkg_resources import resource_string
 from textwrap import dedent
-from urllib2 import Request, URLError, urlopen
 from zope.component import getUtility
 
 from mailman.config import config
@@ -59,7 +57,7 @@ from mailman.core.logging import get_handler
 from mailman.database.transaction import transaction
 from mailman.interfaces.domain import IDomainManager
 from mailman.testing.helpers import (
-    TestableMaster, get_lmtp_client, reset_the_world)
+    TestableMaster, get_lmtp_client, reset_the_world, wait_for_webservice)
 from mailman.testing.mta import ConnectionCountingController
 from mailman.utilities.string import expand
 
@@ -315,29 +313,10 @@ class RESTLayer(SMTPLayer):
 
     server = None
 
-    @staticmethod
-    def _wait_for_rest_server():
-        until = datetime.datetime.now() + as_timedelta(config.devmode.wait)
-        while datetime.datetime.now() < until:
-            try:
-                request = Request('http://localhost:9001/3.0/system')
-                basic_auth = '{0}:{1}'.format(config.webservice.admin_user,
-                                              config.webservice.admin_pass)
-                request.add_header('Authorization',
-                                   'Basic ' + b64encode(basic_auth))
-                fp = urlopen(request)
-            except URLError:
-                pass
-            else:
-                fp.close()
-                break
-        else:
-            raise RuntimeError('REST server did not start up')
-
     @classmethod
     def setUp(cls):
         assert cls.server is None, 'Layer already set up'
-        cls.server = TestableMaster(cls._wait_for_rest_server)
+        cls.server = TestableMaster(wait_for_webservice)
         cls.server.start('rest')
 
     @classmethod