diff options
Diffstat (limited to 'src/mailman/bin')
| -rw-r--r-- | src/mailman/bin/mailman.py | 142 | ||||
| -rw-r--r-- | src/mailman/bin/master.py | 222 | ||||
| -rw-r--r-- | src/mailman/bin/runner.py | 198 | ||||
| -rw-r--r-- | src/mailman/bin/tests/test_mailman.py | 62 | ||||
| -rw-r--r-- | src/mailman/bin/tests/test_master.py | 74 |
5 files changed, 400 insertions, 298 deletions
diff --git a/src/mailman/bin/mailman.py b/src/mailman/bin/mailman.py index 62066175e..e012237f3 100644 --- a/src/mailman/bin/mailman.py +++ b/src/mailman/bin/mailman.py @@ -17,81 +17,93 @@ """The 'mailman' command dispatcher.""" -import os -import argparse +import click -from functools import cmp_to_key +from contextlib import ExitStack +from mailman.config import config from mailman.core.i18n import _ from mailman.core.initialize import initialize from mailman.database.transaction import transaction from mailman.interfaces.command import ICLISubCommand -from mailman.utilities.modules import find_components +from mailman.utilities.modules import add_components from mailman.version import MAILMAN_VERSION_FULL from public import public -# --help should display the subcommands by alphabetical order, except that -# 'mailman help' should be first. -def _help_sorter(command, other): - """Sorting helper.""" - if command.name == 'help': - return -1 - elif other.name == 'help': - return 1 - elif command.name < other.name: - return -1 - elif command.name == other.name: - return 0 - else: - assert command.name > other.name - return 1 +class Subcommands(click.MultiCommand): + # Handle dynamic listing and loading of `mailman` subcommands. + def __init__(self, *args, **kws): + super().__init__(*args, **kws) + self._commands = {} + # Look at all modules in the mailman.bin package and if they are + # prepared to add a subcommand, let them do so. I'm still undecided as + # to whether this should be pluggable or not. If so, then we'll + # probably have to partially parse the arguments now, then initialize + # the system, then find the plugins. Punt on this for now. + add_components('mailman.commands', ICLISubCommand, self._commands) + def list_commands(self, ctx): + return sorted(self._commands) # pragma: nocover + def get_command(self, ctx, name): + try: + return self._commands[name].command + except KeyError as error: + # Returning None here signals click to report usage information + # and a "No such command" error message. + return None + + # This is here to hook command parsing into the Mailman database + # transaction system. If the subcommand succeeds, the transaction is + # committed, otherwise it's aborted. + def invoke(self, ctx): + with ExitStack() as resources: + # If given a bogus subcommand, the database won't have been + # initialized so there's no transaction to commit. + if config.db is not None: + resources.enter_context(transaction()) + return super().invoke(ctx) + + # https://github.com/pallets/click/issues/834 + # + # Note that this only handles the case for the `mailman --help` output. + # To handle `mailman <subcommand> --help` we create a custom click.Command + # subclass and override this method there too. See + # src/mailman/utilities/options.py + 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) + + +@click.group( + cls=Subcommands, + context_settings=dict(help_option_names=['-h', '--help'])) +@click.pass_context +@click.option( + '-C', '--config', 'config_file', + envvar='MAILMAN_CONFIG_FILE', + type=click.Path(exists=True, dir_okay=False, resolve_path=True), + 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.""")) +@click.version_option(MAILMAN_VERSION_FULL, message='%(version)s') @public -def main(): - """The `mailman` command dispatcher.""" - # Create the basic parser and add all globally common options. - parser = argparse.ArgumentParser( - description=_("""\ - The GNU Mailman mailing list management system - Copyright 1998-2017 by the Free Software Foundation, Inc. - http://www.list.org - """), - formatter_class=argparse.RawDescriptionHelpFormatter) - parser.add_argument( - '-v', '--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.""")) - # Look at all modules in the mailman.bin package and if they are prepared - # to add a subcommand, let them do so. I'm still undecided as to whether - # this should be pluggable or not. If so, then we'll probably have to - # partially parse the arguments now, then initialize the system, then find - # the plugins. Punt on this for now. - subparser = parser.add_subparsers(title='Commands') - subcommands = [] - for command in find_components('mailman.commands', ICLISubCommand): - subcommands.append(command) - subcommands.sort(key=cmp_to_key(_help_sorter)) - for command in subcommands: - command_parser = subparser.add_parser( - command.name, help=_(command.__doc__)) - command.add(parser, command_parser) - command_parser.set_defaults(func=command.process) - args = parser.parse_args() - if len(args.__dict__) <= 1: - # No arguments or subcommands were given. - parser.print_help() - parser.exit() +def main(ctx, config_file): + # XXX https://github.com/pallets/click/issues/303 + """\ + The GNU Mailman mailing list management system + Copyright 1998-2017 by the Free Software Foundation, Inc. + http://www.list.org + """ # 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) - # Perform the subcommand option. - with transaction(): - args.func(args) + initialize(config_file) + # click handles dispatching to the subcommand via the Subcommands class. diff --git a/src/mailman/bin/master.py b/src/mailman/bin/master.py index c6a8bcd2d..0b273d332 100644 --- a/src/mailman/bin/master.py +++ b/src/mailman/bin/master.py @@ -19,7 +19,7 @@ import os import sys -import errno +import click import signal import socket import logging @@ -30,8 +30,10 @@ from flufl.lock import Lock, NotLockedError, TimeOutError from lazr.config import as_boolean from mailman.config import config from mailman.core.i18n import _ +from mailman.core.initialize import initialize from mailman.core.logging import reopen -from mailman.utilities.options import Options +from mailman.utilities.options import I18nCommand, validate_runner_spec +from mailman.version import MAILMAN_VERSION_FULL from public import public @@ -44,67 +46,25 @@ SUBPROC_START_WAIT = timedelta(seconds=20) PRESERVE_ENVS = ( 'COVERAGE_PROCESS_START', 'MAILMAN_EXTRA_TESTING_CFG', + 'LANG', + 'LANGUAGE', + 'LC_CTYPE', + 'LC_NUMERIC', + 'LC_TIME', + 'LC_COLLATE', + 'LC_MONETARY', + 'LC_MESSAGES', + 'LC_PAPER', + 'LC_NAME', + 'LC_ADDRESS', + 'LC_TELEPHONE', + 'LC_MEASUREMENT', + 'LC_IDENTIFICATION', + 'LC_ALL', ) -class MasterOptions(Options): - """Options for the master watcher.""" - - usage = _("""\ -%prog [options] - -Master subprocess watcher. - -Start and watch the configured runners and ensure that they stay alive and -kicking. Each runner is forked and exec'd in turn, with the master waiting on -their process ids. When it detects a child runner has exited, it may restart -it. - -The runners respond to SIGINT, SIGTERM, SIGUSR1 and SIGHUP. SIGINT, SIGTERM -and SIGUSR1 all cause a runner to exit cleanly. The master will restart -runners that have exited due to a SIGUSR1 or some kind of other exit condition -(say because of an uncaught exception). SIGHUP causes the master and the -runners to close their log files, and reopen then upon the next printed -message. - -The master also responds to SIGINT, SIGTERM, SIGUSR1 and SIGHUP, which it -simply passes on to the runners. Note that the master will close and reopen -its own log files on receipt of a SIGHUP. The master also leaves its own -process id in the file `data/master.pid` but you normally don't need to use -this pid directly.""") - - def add_options(self): - """See `Options`.""" - self.parser.add_option( - '-n', '--no-restart', - dest='restartable', default=True, action='store_false', - help=_("""\ -Don't restart the runners when they exit because of an error or a SIGUSR1. -Use this only for debugging.""")) - self.parser.add_option( - '-f', '--force', - default=False, action='store_true', - help=_("""\ -If the master watcher finds an existing master lock, it will normally exit -with an error message. With this option,the master will perform an extra -level of checking. If a process matching the host/pid described in the lock -file is running, the master will still exit, requiring you to manually clean -up the lock. But if no matching process is found, the master will remove the -apparently stale lock and make another attempt to claim the master lock.""")) - self.parser.add_option( - '-r', '--runner', - dest='runners', action='append', default=[], - help=_("""\ -Override the default set of runners that the master will invoke, which is -typically defined in the configuration file. Multiple -r options may be -given. The values for -r are passed straight through to bin/runner.""")) - - def sanity_check(self): - """See `Options`.""" - if len(self.arguments) > 0: - self.parser.error(_('Too many arguments')) - - +@public class WatcherState(Enum): """Enum for the state of the master process watcher.""" # No lock has been acquired by any process. @@ -117,6 +77,7 @@ class WatcherState(Enum): host_mismatch = 3 +@public def master_state(lock_file=None): """Get the state of the master watcher. @@ -139,12 +100,9 @@ def master_state(lock_file=None): try: os.kill(pid, 0) return WatcherState.conflict, lock - except OSError as error: - if error.errno == errno.ESRCH: - # No matching process id. - return WatcherState.stale_lock, lock - # Some other error occurred. - raise + except ProcessLookupError: + # No matching process id. + return WatcherState.stale_lock, lock def acquire_lock_1(force, lock_file=None): @@ -211,17 +169,17 @@ Lock host: $hostname Exiting.""") else: assert status is WatcherState.none, ( - 'Invalid enum value: ${0}'.format(status)) + 'Invalid enum value: {}'.format(status)) hostname, pid, tempfile = lock.details message = _("""\ For unknown reasons, the master lock could not be acquired. - Lock file: $config.LOCK_FILE Lock host: $hostname Exiting.""") - config.options.parser.error(message) + print(message, file=sys.stderr) + sys.exit(1) class PIDWatcher: @@ -438,15 +396,12 @@ class Loop: while True: try: pid, status = os.wait() - except OSError as error: + except ChildProcessError: # No children? We're done. - if error.errno == errno.ECHILD: - break + break + except InterruptedError: # pragma: nocover # If the system call got interrupted, just restart it. - elif error.errno == errno.EINTR: - continue - else: - raise + continue # Find out why the subprocess exited by getting the signal # received or exit status. if os.WIFSIGNALED(status): @@ -497,40 +452,121 @@ Runner {0} reached maximum restart limit of {1:d}, not restarting.""", for pid in self._kids: try: os.kill(pid, signal.SIGTERM) - except OSError as error: - if error.errno == errno.ESRCH: - # The child has already exited. - log.info('ESRCH on pid: %d', pid) + except ProcessLookupError: # pragma: nocover + # The child has already exited. + log.info('ESRCH on pid: %d', pid) + except OSError: # pragma: nocover + # XXX I'm not so sure about this. It preserves the semantics + # before conversion to PEP 3151 exceptions. But is it right? + pass # Wait for all the children to go away. while self._kids: try: pid, status = os.wait() self._kids.drop(pid) - except OSError as error: - if error.errno == errno.ECHILD: - break - elif error.errno == errno.EINTR: - continue - raise + except ChildProcessError: + break + except InterruptedError: # pragma: nocover + continue + +@click.command( + cls=I18nCommand, + context_settings=dict(help_option_names=['-h', '--help']), + help=_("""\ + Master subprocess watcher. + Start and watch the configured runners, ensuring that they stay alive and + kicking. Each runner is forked and exec'd in turn, with the master waiting + on their process ids. When it detects a child runner has exited, it may + restart it. + + The runners respond to SIGINT, SIGTERM, SIGUSR1 and SIGHUP. SIGINT, + SIGTERM and SIGUSR1 all cause a runner to exit cleanly. The master will + restart runners that have exited due to a SIGUSR1 or some kind of other + exit condition (say because of an uncaught exception). SIGHUP causes the + master and the runners to close their log files, and reopen then upon the + next printed message. + + The master also responds to SIGINT, SIGTERM, SIGUSR1 and SIGHUP, which it + simply passes on to the runners. Note that the master will close and + reopen its own log files on receipt of a SIGHUP. The master also leaves + its own process id in the file specified in the configuration file but you + normally don't need to use this PID directly.""")) +@click.option( + '-C', '--config', 'config_file', + envvar='MAILMAN_CONFIG_FILE', + type=click.Path(exists=True, dir_okay=False, resolve_path=True), + 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.""")) +@click.option( + '--no-restart', '-n', 'restartable', + is_flag=True, default=True, + help=_("""\ + Don't restart the runners when they exit because of an error or a SIGUSR1. + Use this only for debugging.""")) +@click.option( + '--force', '-f', + is_flag=True, default=False, + help=_("""\ + If the master watcher finds an existing master lock, it will normally exit + with an error message. With this option,the master will perform an extra + level of checking. If a process matching the host/pid described in the + lock file is running, the master will still exit, requiring you to manually + clean up the lock. But if no matching process is found, the master will + remove the apparently stale lock and make another attempt to claim the + master lock.""")) +@click.option( + '--runners', '-r', + metavar='runner[:slice:range]', + callback=validate_runner_spec, default=None, + multiple=True, + help=_("""\ + Override the default set of runners that the master will invoke, which is + typically defined in the configuration file. Multiple -r options may be + given. The values for -r are passed straight through to bin/runner.""")) +@click.option( + '-v', '--verbose', + is_flag=True, default=False, + help=_('Display more debugging information to the log file.')) +@click.version_option(MAILMAN_VERSION_FULL) @public -def main(): - """Main process.""" +def main(config_file, restartable, force, runners, verbose): + # XXX https://github.com/pallets/click/issues/303 + """Master subprocess watcher. + + Start and watch the configured runners and ensure that they stay + alive and kicking. Each runner is forked and exec'd in turn, with + the master waiting on their process ids. When it detects a child + runner has exited, it may restart it. - options = MasterOptions() - options.initialize() + The runners respond to SIGINT, SIGTERM, SIGUSR1 and SIGHUP. SIGINT, + SIGTERM and SIGUSR1 all cause a runner to exit cleanly. The master + will restart runners that have exited due to a SIGUSR1 or some kind + of other exit condition (say because of an uncaught exception). + SIGHUP causes the master and the runners to close their log files, + and reopen then upon the next printed message. + + The master also responds to SIGINT, SIGTERM, SIGUSR1 and SIGHUP, + which it simply passes on to the runners. Note that the master will + close and reopen its own log files on receipt of a SIGHUP. The + master also leaves its own process id in the file `data/master.pid` + but you normally don't need to use this pid directly. + """ + initialize(config_file, verbose) # Acquire the master lock, exiting if we can't. We'll let the caller # handle any clean up or lock breaking. No `with` statement here because # Lock's constructor doesn't support a timeout. - lock = acquire_lock(options.options.force) + lock = acquire_lock(force) try: with open(config.PID_FILE, 'w') as fp: print(os.getpid(), file=fp) - loop = Loop(lock, options.options.restartable, options.options.config) + loop = Loop(lock, restartable, config.filename) loop.install_signal_handlers() try: - loop.start_runners(options.options.runners) + loop.start_runners(runners) loop.loop() finally: loop.cleanup() diff --git a/src/mailman/bin/runner.py b/src/mailman/bin/runner.py index 29b84034a..bf84bc3e5 100644 --- a/src/mailman/bin/runner.py +++ b/src/mailman/bin/runner.py @@ -19,15 +19,16 @@ import os import sys +import click import signal import logging -import argparse import traceback from mailman.config import config from mailman.core.i18n import _ from mailman.core.initialize import initialize from mailman.utilities.modules import find_name +from mailman.utilities.options import I18nCommand, validate_runner_spec from mailman.version import MAILMAN_VERSION_FULL from public import public @@ -41,44 +42,19 @@ if os.environ.get('COVERAGE_PROCESS_START') is not None: coverage.process_startup() -class ROptionAction(argparse.Action): - """Callback for -r/--runner option.""" - 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)) - - def make_runner(name, slice, range, once=False): - # Several conventions for specifying the runner name are supported. It - # could be one of the shortcut names. If the name is a full module path, - # use it explicitly. If the name starts with a dot, it's a class name - # relative to the Mailman.runner package. + # The runner name must be defined in the configuration. Only runner short + # names are supported. runner_config = getattr(config, 'runner.' + name, None) - if runner_config is not None: - # It was a shortcut name. - class_path = runner_config['class'] - elif name.startswith('.'): - class_path = 'mailman.runners' + name - else: - class_path = name + if runner_config is None: + print(_('Undefined runner name: $name'), file=sys.stderr) + # Exit with SIGTERM exit code so the master won't try to restart us. + sys.exit(signal.SIGTERM) + class_path = runner_config['class'] try: runner_class = find_name(class_path) except ImportError: if os.environ.get('MAILMAN_UNDER_MASTER_CONTROL') is not None: - # Exit with SIGTERM exit code so the master watcher won't try to - # restart us. print(_('Cannot import runner module: $class_path'), file=sys.stderr) traceback.print_exc() @@ -94,82 +70,98 @@ def make_runner(name, slice, range, once=False): return runner_class(name, slice) -@public -def main(): - global log +@click.command( + cls=I18nCommand, + context_settings=dict(help_option_names=['-h', '--help']), + help=_("""\ + Start a runner. + + 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. - parser = argparse.ArgumentParser( - description=_("""\ - Start a runner + -r is required unless -l or -h is given, and its argument must be one of + the names displayed by the -l switch. - 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. + Normally, this script should be started from `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. + """)) +@click.option( + '-C', '--config', 'config_file', + envvar='MAILMAN_CONFIG_FILE', + type=click.Path(exists=True, dir_okay=False, resolve_path=True), + 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.""")) +@click.option( + '-l', '--list', 'list_runners', + is_flag=True, is_eager=True, default=False, + help=_('List the available runner names and exit.')) +@click.option( + '-o', '--once', + is_flag=True, default=False, + 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.""")) +@click.option( + '-r', '--runner', 'runner_spec', + metavar='runner[:slice:range]', + callback=validate_runner_spec, default=None, + help=_("""\ - -r is required unless -l or -h is given, and its argument must be one - of the names displayed by the -l switch. + Start the named runner, which must be one of the strings returned by the -l + option. - Normally, this script should be started from '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. - 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. + """)) +@click.option( + '-v', '--verbose', + is_flag=True, default=False, + help=_('Display more debugging information to the log file.')) +@click.version_option(MAILMAN_VERSION_FULL) +@click.pass_context +@public +def main(ctx, config_file, verbose, list_runners, once, runner_spec): + # XXX https://github.com/pallets/click/issues/303 + """Start a runner. + + 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. - 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=None, action='store_true', help=_("""\ - Display more debugging information to the log file.""")) + Normally, this script should be started from '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. + """ - args = parser.parse_args() - if args.runner is None and not args.list: - parser.error(_('No runner name given.')) + global log + + if runner_spec is None and not list_runners: + ctx.fail(_('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) + + initialize(config_file, verbose) log = logging.getLogger('mailman.runner') - if args.verbose: + if verbose: console = logging.StreamHandler(sys.stderr) formatter = logging.Formatter(config.logging.root.format, config.logging.root.datefmt) @@ -177,7 +169,7 @@ def main(): logging.getLogger().addHandler(console) logging.getLogger().setLevel(logging.DEBUG) - if args.list: + if list_runners: descriptions = {} for section in config.runner_configs: ignore, dot, shortname = section.name.rpartition('.') @@ -191,10 +183,10 @@ def main(): print(_('$name runs $classname')) sys.exit(0) - runner = make_runner(*args.runner, once=args.once) + runner = make_runner(*runner_spec, once=once) runner.set_signals() # Now start up the main loop - log.info('%s runner started.', runner.name) + log.info('%s runner started.'.format(runner.name)) runner.run() - log.info('%s runner exiting.', runner.name) + log.info('%s runner exiting.'.format(runner.name)) sys.exit(runner.status) diff --git a/src/mailman/bin/tests/test_mailman.py b/src/mailman/bin/tests/test_mailman.py index 49a64fb5c..54ee54bce 100644 --- a/src/mailman/bin/tests/test_mailman.py +++ b/src/mailman/bin/tests/test_mailman.py @@ -19,9 +19,8 @@ import unittest -from contextlib import ExitStack +from click.testing import CliRunner from datetime import timedelta -from io import StringIO from mailman.app.lifecycle import create_list from mailman.bin.mailman import main from mailman.config import config @@ -34,17 +33,31 @@ from unittest.mock import patch class TestMailmanCommand(unittest.TestCase): layer = ConfigLayer + def setUp(self): + self._command = CliRunner() + def test_mailman_command_without_subcommand_prints_help(self): # Issue #137: Running `mailman` without a subcommand raises an # AttributeError. - testargs = ['mailman'] - output = StringIO() - with patch('sys.argv', testargs), patch('sys.stdout', output): - with self.assertRaises(SystemExit): - main() - self.assertIn('usage', output.getvalue()) + result = self._command.invoke(main) + lines = result.output.splitlines() + # "main" instead of "mailman" because of the way the click runner + # works. It does actually show the correct program when run from the + # command line. + self.assertEqual(lines[0], 'Usage: main [OPTIONS] COMMAND [ARGS]...') + + def test_mailman_command_with_bad_subcommand_prints_help(self): + # Issue #137: Running `mailman` without a subcommand raises an + # AttributeError. + result = self._command.invoke(main, ('not-a-subcommand',)) + lines = result.output.splitlines() + # "main" instead of "mailman" because of the way the click runner + # works. It does actually show the correct program when run from the + # command line. + self.assertEqual(lines[0], 'Usage: main [OPTIONS] COMMAND [ARGS]...') - def test_transaction_commit_after_successful_subcommand(self): + @patch('mailman.bin.mailman.initialize') + def test_transaction_commit_after_successful_subcommand(self, mock): # Issue #223: Subcommands which change the database need to commit or # abort the transaction. with transaction(): @@ -52,40 +65,23 @@ class TestMailmanCommand(unittest.TestCase): mlist.volume = 5 mlist.next_digest_number = 3 mlist.digest_last_sent_at = now() - timedelta(days=60) - testargs = ['mailman', 'digests', '-b', '-l', 'ant@example.com'] - output = StringIO() - with ExitStack() as resources: - enter = resources.enter_context - enter(patch('sys.argv', testargs)) - enter(patch('sys.stdout', output)) - # Everything is already initialized. - enter(patch('mailman.bin.mailman.initialize')) - main() + self._command.invoke(main, ('digests', '-b', '-l', 'ant@example.com')) # Clear the current transaction to force a database reload. config.db.abort() self.assertEqual(mlist.volume, 6) self.assertEqual(mlist.next_digest_number, 1) - def test_transaction_abort_after_failing_subcommand(self): + @patch('mailman.bin.mailman.initialize') + @patch('mailman.commands.cli_digests.maybe_send_digest_now', + side_effect=RuntimeError) + def test_transaction_abort_after_failing_subcommand(self, mock1, mock2): with transaction(): mlist = create_list('ant@example.com') mlist.volume = 5 mlist.next_digest_number = 3 mlist.digest_last_sent_at = now() - timedelta(days=60) - testargs = ['mailman', 'digests', '-b', '-l', 'ant@example.com', - '--send'] - output = StringIO() - with ExitStack() as resources: - enter = resources.enter_context - enter(patch('sys.argv', testargs)) - enter(patch('sys.stdout', output)) - # Force an exception in the subcommand. - enter(patch('mailman.commands.cli_digests.maybe_send_digest_now', - side_effect=RuntimeError)) - # Everything is already initialized. - enter(patch('mailman.bin.mailman.initialize')) - with self.assertRaises(RuntimeError): - main() + self._command.invoke( + main, ('digests', '-b', '-l', 'ant@example.com', '--send')) # Clear the current transaction to force a database reload. config.db.abort() # The volume and number haven't changed. diff --git a/src/mailman/bin/tests/test_master.py b/src/mailman/bin/tests/test_master.py index fb045f58f..27ea6d559 100644 --- a/src/mailman/bin/tests/test_master.py +++ b/src/mailman/bin/tests/test_master.py @@ -21,13 +21,28 @@ import os import tempfile import unittest -from contextlib import suppress +from click.testing import CliRunner +from contextlib import ExitStack, suppress from datetime import timedelta -from flufl.lock import Lock +from flufl.lock import Lock, TimeOutError +from io import StringIO from mailman.bin import master +from mailman.config import config +from mailman.testing.layers import ConfigLayer +from pkg_resources import resource_filename +from unittest.mock import patch -class TestMasterLock(unittest.TestCase): +class FakeLock: + details = ('host.example.com', 9999, '/tmp/whatever') + + def unlock(self): + pass + + +class TestMaster(unittest.TestCase): + layer = ConfigLayer + def setUp(self): fd, self.lock_file = tempfile.mkstemp() os.close(fd) @@ -59,4 +74,55 @@ class TestMasterLock(unittest.TestCase): finally: my_lock.unlock() self.assertEqual(state, master.WatcherState.conflict) - # XXX test stale_lock and host_mismatch states. + + def test_acquire_lock_timeout_reason_unknown(self): + stderr = StringIO() + with ExitStack() as resources: + resources.enter_context(patch( + 'mailman.bin.master.acquire_lock_1', + side_effect=TimeOutError)) + resources.enter_context(patch( + 'mailman.bin.master.master_state', + return_value=(master.WatcherState.none, FakeLock()))) + resources.enter_context(patch( + 'mailman.bin.master.sys.stderr', stderr)) + with self.assertRaises(SystemExit) as cm: + master.acquire_lock(False) + self.assertEqual(cm.exception.code, 1) + self.assertEqual(stderr.getvalue(), """\ +For unknown reasons, the master lock could not be acquired. + +Lock file: {} +Lock host: host.example.com + +Exiting. +""".format(config.LOCK_FILE)) + + def test_main_cli(self): + command = CliRunner() + fake_lock = FakeLock() + with ExitStack() as resources: + config_file = resource_filename( + 'mailman.testing', 'testing.cfg') + init_mock = resources.enter_context(patch( + 'mailman.bin.master.initialize')) + lock_mock = resources.enter_context(patch( + 'mailman.bin.master.acquire_lock', + return_value=fake_lock)) + start_mock = resources.enter_context(patch.object( + master.Loop, 'start_runners')) + loop_mock = resources.enter_context(patch.object( + master.Loop, 'loop')) + command.invoke( + master.main, + ('-C', config_file, + '--no-restart', '--force', + '-r', 'in:1:1', '--verbose')) + # We got initialized with the custom configuration file and the + # verbose flag. + init_mock.assert_called_once_with(config_file, True) + # We returned a lock that was force-acquired. + lock_mock.assert_called_once_with(True) + # We created a non-restartable loop. + start_mock.assert_called_once_with([('in', 1, 1)]) + loop_mock.assert_called_once_with() |