From 39f7fd59553584e2e4d2dd812e61e335bc045eb5 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 14 Nov 2013 22:33:57 +0100 Subject: [PATCH] commands: Extend use of exit helper to parser errors This moves all the bad arugment handling into our class and makes sure to mark all the helpers as internal. --- mopidy/utils/command.py | 40 ++++++++--------- tests/utils/command_test.py | 85 +++++++++++++++++++------------------ 2 files changed, 60 insertions(+), 65 deletions(-) diff --git a/mopidy/utils/command.py b/mopidy/utils/command.py index df33b444..84f54a66 100644 --- a/mopidy/utils/command.py +++ b/mopidy/utils/command.py @@ -4,22 +4,17 @@ import os import sys -class CommandError(Exception): - def __init__(self, message, usage=None): - self.message = message - self.usage = usage - - def __str__(self): - return '%s\n\nerror: %s' % (self.usage, self.message) - - -class ArgumentParser(argparse.ArgumentParser): - def error(self, message): - raise CommandError(message) +class _ParserError(Exception): + pass class _HelpError(Exception): - """Internal exception used to trigger help code path.""" + pass + + +class _ArgumentParser(argparse.ArgumentParser): + def error(self, message): + raise _ParserError(message) class _HelpAction(argparse.Action): @@ -43,7 +38,7 @@ class Command(object): def _build(self): actions = [] - parser = ArgumentParser(add_help=False) + parser = _ArgumentParser(add_help=False) parser.register('action', 'help', _HelpAction) for args, kwargs in self._arguments: @@ -62,8 +57,9 @@ class Command(object): def set_defaults(self, **kwargs): self._defaults.update(kwargs) - def exit(self, return_code): - sys.exit(return_code) + def exit(self, status_code=0, message=None, usage=None): + print '\n\n'.join(m for m in (usage, message) if m.strip) + sys.exit(status_code) def format_usage(self, prog=None): actions = self._build()[1] @@ -124,8 +120,7 @@ class Command(object): return self._parse( args, argparse.Namespace(), self._defaults.copy(), prog) except _HelpError: - print self.format_help(prog) - self.exit(0) + self.exit(0, self.format_help(prog)) def _parse(self, args, namespace, defaults, prog): defaults.update(self._defaults) @@ -133,9 +128,8 @@ class Command(object): try: result = parser.parse_args(args, namespace) - except CommandError as e: - e.usage = self._usage(actions, prog) - raise + except _ParserError as e: + self.exit(1, e.message, self._usage(actions, prog)) if not result._args: for attr, value in defaults.items(): @@ -147,8 +141,8 @@ class Command(object): child = result._args.pop(0) if child not in self._children: - raise CommandError('unrecognized command: %s' % child, - usage=self._usage(actions, prog)) + usage = self._usage(actions, prog) + self.exit(1, 'unrecognized command: %s' % child, usage) return self._children[child]._parse( result._args, result, defaults, ' '.join([prog, child])) diff --git a/tests/utils/command_test.py b/tests/utils/command_test.py index 5410e8da..2794287d 100644 --- a/tests/utils/command_test.py +++ b/tests/utils/command_test.py @@ -8,6 +8,14 @@ from mopidy.utils import command class CommandParsingTest(unittest.TestCase): + def setUp(self): + self.exit_patcher = mock.patch.object(command.Command, 'exit') + self.exit_mock = self.exit_patcher.start() + self.exit_mock.side_effect = SystemExit + + def tearDown(self): + self.exit_patcher.stop() + def test_command_parsing_returns_namespace(self): cmd = command.Command() self.assertIsInstance(cmd.parse([]), argparse.Namespace) @@ -17,14 +25,14 @@ class CommandParsingTest(unittest.TestCase): result = cmd.parse([]) self.assertFalse(hasattr(result, '_args')) - def test_unknown_options_raises_error(self): + def test_unknown_options_bails(self): cmd = command.Command() - with self.assertRaises(command.CommandError): + with self.assertRaises(SystemExit): cmd.parse(['--foobar']) - def test_invalid_sub_command_raises_error(self): + def test_invalid_sub_command_bails(self): cmd = command.Command() - with self.assertRaises(command.CommandError): + with self.assertRaises(SystemExit): cmd.parse(['foo']) def test_command_arguments(self): @@ -94,12 +102,12 @@ class CommandParsingTest(unittest.TestCase): cmd = command.Command() cmd.add_argument('--bar', type=int) - with self.assertRaises(command.CommandError) as cm: + with self.assertRaises(SystemExit): cmd.parse(['--bar', b'zero'], prog='foo') - self.assertEqual(cm.exception.message, - "argument --bar: invalid int value: 'zero'") - self.assertEqual(cm.exception.usage, 'usage: foo [--bar BAR]') + self.exit_mock.assert_called_once_with( + 1, "argument --bar: invalid int value: 'zero'", + 'usage: foo [--bar BAR]') @mock.patch('sys.argv') def test_command_error_usage_prog(self, argv_mock): @@ -108,33 +116,37 @@ class CommandParsingTest(unittest.TestCase): cmd = command.Command() cmd.add_argument('--bar', required=True) - with self.assertRaises(command.CommandError) as cm: + with self.assertRaises(SystemExit): cmd.parse([]) - self.assertEqual(cm.exception.usage, 'usage: foo --bar BAR') + self.exit_mock.assert_called_once_with( + mock.ANY, mock.ANY, 'usage: foo --bar BAR') - with self.assertRaises(command.CommandError) as cm: + self.exit_mock.reset_mock() + with self.assertRaises(SystemExit): cmd.parse([], prog='baz') - self.assertEqual(cm.exception.usage, 'usage: baz --bar BAR') + + self.exit_mock.assert_called_once_with( + mock.ANY, mock.ANY, 'usage: baz --bar BAR') def test_missing_required(self): cmd = command.Command() cmd.add_argument('--bar', required=True) - with self.assertRaises(command.CommandError) as cm: + with self.assertRaises(SystemExit): cmd.parse([], prog='foo') - self.assertEqual(cm.exception.message, 'argument --bar is required') - self.assertEqual(cm.exception.usage, 'usage: foo --bar BAR') + self.exit_mock.assert_called_once_with( + 1, 'argument --bar is required', 'usage: foo --bar BAR') def test_missing_positionals(self): cmd = command.Command() cmd.add_argument('bar') - with self.assertRaises(command.CommandError) as cm: + with self.assertRaises(SystemExit): cmd.parse([], prog='foo') - self.assertEqual(cm.exception.message, 'too few arguments') - self.assertEqual(cm.exception.usage, 'usage: foo bar') + self.exit_mock.assert_called_once_with( + 1, 'too few arguments', 'usage: foo bar') def test_missing_positionals_subcommand(self): child = command.Command() @@ -143,31 +155,30 @@ class CommandParsingTest(unittest.TestCase): cmd = command.Command() cmd.add_child('bar', child) - with self.assertRaises(command.CommandError) as cm: + with self.assertRaises(SystemExit): cmd.parse(['bar'], prog='foo') - self.assertEqual(cm.exception.message, 'too few arguments') - self.assertEqual(cm.exception.usage, 'usage: foo bar baz') + self.exit_mock.assert_called_once_with( + 1, 'too few arguments', 'usage: foo bar baz') def test_unknown_command(self): cmd = command.Command() - with self.assertRaises(command.CommandError) as cm: + with self.assertRaises(SystemExit): cmd.parse(['--help'], prog='foo') - self.assertEqual( - cm.exception.message, 'unrecognized arguments: --help') - self.assertEqual(cm.exception.usage, 'usage: foo') + self.exit_mock.assert_called_once_with( + 1, 'unrecognized arguments: --help', 'usage: foo') def test_invalid_subcommand(self): cmd = command.Command() cmd.add_child('baz', command.Command()) - with self.assertRaises(command.CommandError) as cm: + with self.assertRaises(SystemExit): cmd.parse(['bar'], prog='foo') - self.assertEqual(cm.exception.message, 'unrecognized command: bar') - self.assertEqual(cm.exception.usage, 'usage: foo') + self.exit_mock.assert_called_once_with( + 1, 'unrecognized command: bar', 'usage: foo') def test_set_defaults(self): cmd = command.Command() @@ -201,11 +212,12 @@ class CommandParsingTest(unittest.TestCase): cmd = command.Command() cmd.add_argument('-h', action='help') cmd.format_help = mock.Mock() - cmd.exit = mock.Mock() - cmd.parse(['-h']) + with self.assertRaises(SystemExit): + cmd.parse(['-h']) + cmd.format_help.assert_called_once_with(mock.ANY) - cmd.exit.assert_called_once_with(0) + self.exit_mock.assert_called_once_with(0, cmd.format_help.return_value) class UsageTest(unittest.TestCase): @@ -435,17 +447,6 @@ class HelpTest(unittest.TestCase): self.assertEqual(expected, cmd.format_help('foo').strip()) -class CommandErrorTest(unittest.TestCase): - def test_args_get_stored(self): - error = command.CommandError('message', usage='usage: foo') - self.assertEqual(error.message, 'message') - self.assertEqual(error.usage, 'usage: foo') - - def test_str_command_error(self): - error = command.CommandError('message', usage='usage: foo') - self.assertEqual(str(error), 'usage: foo\n\nerror: message') - - class RunTest(unittest.TestCase): def test_default_implmentation_raises_error(self): with self.assertRaises(NotImplementedError):