From bdf8b3778fcefc4a38cc769f603f10e064908f71 Mon Sep 17 00:00:00 2001 From: "Evgeny V. Generalov" Date: Sat, 12 Dec 2015 15:40:07 +0500 Subject: [PATCH] Refactored command line argument parsing (fixes #7) --- README.md | 8 ++ django_maven/compat.py | 10 +- django_maven/management/argparse_command.py | 13 ++ django_maven/management/commands/maven.py | 121 +++++++++--------- django_maven/management/optparse_command.py | 54 ++++++++ django_maven/tests.py | 72 +++++++++++ setup.py | 5 +- .../test_project => tests}/__init__.py | 0 {test_project => tests}/manage.py | 0 {test_project => tests}/requirements.txt | 0 tests/test_app/__init__.py | 0 tests/test_app/management/__init__.py | 0 .../test_app/management/commands/__init__.py | 0 tests/test_app/management/commands/testcmd.py | 24 ++++ tests/test_app/models.py | 1 + tests/test_app/tests.py | 68 ++++++++++ tests/test_project/__init__.py | 0 .../test_project/settings.py | 16 +-- {test_project => tests}/test_project/urls.py | 0 {test_project => tests}/test_project/wsgi.py | 0 tox.ini | 22 ++++ 21 files changed, 345 insertions(+), 69 deletions(-) create mode 100644 django_maven/management/argparse_command.py create mode 100644 django_maven/management/optparse_command.py create mode 100644 django_maven/tests.py rename {test_project/test_project => tests}/__init__.py (100%) rename {test_project => tests}/manage.py (100%) rename {test_project => tests}/requirements.txt (100%) create mode 100644 tests/test_app/__init__.py create mode 100644 tests/test_app/management/__init__.py create mode 100644 tests/test_app/management/commands/__init__.py create mode 100644 tests/test_app/management/commands/testcmd.py create mode 100644 tests/test_app/models.py create mode 100644 tests/test_app/tests.py create mode 100644 tests/test_project/__init__.py rename {test_project => tests}/test_project/settings.py (86%) rename {test_project => tests}/test_project/urls.py (100%) rename {test_project => tests}/test_project/wsgi.py (100%) create mode 100644 tox.ini diff --git a/README.md b/README.md index 71a5b8d..c722bd6 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,14 @@ And command with `django-maven`: If `rebuild_index` command raising exception (server die or error creating index) you see their in your Sentry. +Tests +----- + +Tests should run with tox >=1.8. Running `tox` will run all tests for all environments. +Use tox -e to run a certain environment, a list of all environments can be found with tox -l. + +Hint: it is possible to run all environments in parallel with `detox`. + The name -------- diff --git a/django_maven/compat.py b/django_maven/compat.py index baaa9b5..f7ceeed 100644 --- a/django_maven/compat.py +++ b/django_maven/compat.py @@ -1,5 +1,7 @@ from django import VERSION +from django.core.management.base import BaseCommand +__all__ = ['OutputWrapper', 'MavenBaseCommand'] if VERSION > (1, 5,): from django.core.management.base import OutputWrapper @@ -10,6 +12,7 @@ class OutputWrapper(object): """ Wrapper around stdout/stderr from django 1.5 """ + def __init__(self, out, style_func=None, ending='\n'): self._out = out self.style_func = None @@ -24,6 +27,11 @@ def write(self, msg, style_func=None, ending=None): ending = ending is None and self.ending or ending if ending and not msg.endswith(ending): msg += ending - style_func = [f for f in (style_func, self.style_func, lambda x:x) + style_func = [f for f in (style_func, self.style_func, lambda x: x) if f is not None][0] self._out.write(force_unicode(style_func(msg))) + +if hasattr(BaseCommand, 'use_argparse'): + from django_maven.management.argparse_command import MavenBaseCommand +else: + from django_maven.management.optparse_command import MavenBaseCommand diff --git a/django_maven/management/argparse_command.py b/django_maven/management/argparse_command.py new file mode 100644 index 0000000..452b8b7 --- /dev/null +++ b/django_maven/management/argparse_command.py @@ -0,0 +1,13 @@ +import argparse + +from django.core.management.base import BaseCommand + + +class MavenBaseCommand(BaseCommand): + """The *argparse* compatible command""" + + def add_arguments(self, parser): + """ + :type parser: argparse.ArgumentParser + """ + parser.add_argument('args', nargs=argparse.REMAINDER) diff --git a/django_maven/management/commands/maven.py b/django_maven/management/commands/maven.py index 8fa599f..e437720 100644 --- a/django_maven/management/commands/maven.py +++ b/django_maven/management/commands/maven.py @@ -1,77 +1,84 @@ import sys -from optparse import OptionParser from django.conf import settings from django.core.management import get_commands, load_command_class -from django.core.management.base import (BaseCommand, handle_default_options, - CommandError) - +from django.core.management.base import CommandError, handle_default_options +from django_maven.compat import MavenBaseCommand, OutputWrapper from raven import Client -from django_maven.compat import OutputWrapper - +VERBOSE_OUTPUT = 2 -class Command(BaseCommand): +class Command(MavenBaseCommand): help = 'Capture exceptions and send in Sentry' - args = '' - def _get_subcommand_class(self, command): - commands = get_commands() - app_name = commands[command] - return load_command_class(app_name, command) + def __init__(self, *args, **kw): + super(MavenBaseCommand, self).__init__(*args, **kw) + self._argv = None - def _write_error_in_stderr(self, exc): - stderr = getattr(self, 'stderr', OutputWrapper(sys.stderr, - self.style.ERROR)) - stderr.write('%s: %s' % (exc.__class__.__name__, exc)) - sys.exit(1) + def run_from_argv(self, argv): + self._argv = argv + return super(Command, self).run_from_argv(argv) - def usage(self, subcommand): - usage = 'Usage: %s %s [command options]' % (subcommand, self.args) - if self.help: - return '%s\n\n%s' % (usage, self.help) - else: - return usage + def execute(self, *args, **options): + if not args: + self.print_help(self._argv[0], self._argv[1]) + return + + subcommand = self._get_subcommand_class(args[0]) + subcommand_argv = [self._argv[0]] + list(args) + + # this is a lightweight version of the BaseCommand.run_from_argv + # it should be compatible with Django-1.5..1.9 + subcommand_args, subcommand_options = self._handle_argv( + subcommand, subcommand_argv) + try: + subcommand.execute(*subcommand_args, **subcommand_options) + except Exception as e: + if not isinstance(e, CommandError): + if int(options['verbosity']) >= VERBOSE_OUTPUT: + # self.stderr is not guaranteed to be set here + stderr = getattr(self, 'stderr', OutputWrapper( + sys.stderr, self.style.ERROR)) + stderr.write('Beautiful is better than %s. ' + 'Errors should never pass silently.' % + (e.__class__.__name__)) - def create_parser(self, prog_name, subcommand, subcommand_class): - if hasattr(self, 'use_argparse') and self.use_argparse: - return super(Command, self).create_parser(prog_name, subcommand) + sentry = self._get_sentry() + if sentry: + sentry.captureException() + # use default 'maven' options to deal with traceback + raise + + def _get_sentry(self): + if hasattr(settings, 'SENTRY_DSN'): + dsn = settings.SENTRY_DSN + elif hasattr(settings, 'RAVEN_CONFIG'): + dsn = settings.RAVEN_CONFIG.get('dsn') else: - return OptionParser(prog=prog_name, - usage=subcommand_class.usage(subcommand), - version=subcommand_class.get_version(), - option_list=subcommand_class.option_list) + return None - def run_from_argv(self, argv): - if len(argv) <= 2 or argv[2] in ['-h', '--help']: - stdout = OutputWrapper(sys.stdout) - stdout.write(self.usage(argv[1])) - sys.exit(1) + sentry = Client(dsn) + if not sentry.is_enabled(): + return None + return sentry + + def _get_subcommand_class(self, command): + commands = get_commands() + app_name = commands[command] + return load_command_class(app_name, command) - subcommand_class = self._get_subcommand_class(argv[2]) - parser = self.create_parser(argv[0], argv[2], subcommand_class) - if hasattr(self, 'use_argparse') and self.use_argparse: - subcommand_class.add_arguments(parser) - options = parser.parse_args(argv[3:]) + def _handle_argv(self, subcommand, argv): + """The universal Django command arguments parser.""" + parser = subcommand.create_parser(argv[0], argv[1]) + + if hasattr(subcommand, 'use_argparse') and subcommand.use_argparse: + options = parser.parse_args(argv[2:]) cmd_options = vars(options) + # Move positional args out of options to mimic legacy optparse args = cmd_options.pop('args', ()) else: - options, args = parser.parse_args(argv[3:]) + options, args = parser.parse_args(argv[2:]) + cmd_options = vars(options) handle_default_options(options) - try: - subcommand_class.execute(*args, **options.__dict__) - except Exception as e: - if not isinstance(e, CommandError): - if hasattr(settings, 'SENTRY_DSN'): - dsn = settings.SENTRY_DSN - elif hasattr(settings, 'RAVEN_CONFIG'): - dsn = settings.RAVEN_CONFIG.get('dsn') - else: - raise - sentry = Client(dsn) - if not sentry.is_enabled(): - raise - sentry.get_ident(sentry.captureException()) - - self._write_error_in_stderr(e) + return args, cmd_options diff --git a/django_maven/management/optparse_command.py b/django_maven/management/optparse_command.py new file mode 100644 index 0000000..df8d425 --- /dev/null +++ b/django_maven/management/optparse_command.py @@ -0,0 +1,54 @@ +import types +from optparse import AmbiguousOptionError, BadOptionError + +from django.core.management.base import BaseCommand + + +class MavenBaseCommand(BaseCommand): + """The *optparse* compatible command + + manage.py maven [maven options] subcommand [subcommand options] + """ + + @property + def use_argparse(self): + return False + + def create_parser(self, prog_name, subcommand): + parser = super(MavenBaseCommand, self).create_parser( + prog_name, subcommand) + parser._process_args = types.MethodType(_process_args, parser) + return parser + + +def _process_args(self, largs, rargs, values): + max_own_positional_args = 1 + + while rargs: + try: + arg = rargs[0] + # XXX: e.generalov@ added condition to skip processing + # arguments after subcommand name + if len(largs) >= max_own_positional_args: + return # stop now, leave this arg in rargs + # We handle bare "--" explicitly, and bare "-" is handled by the + # standard arg handler since the short arg case ensures that the + # len of the opt string is greater than 1. + elif arg == '--': + del rargs[0] + return + elif arg[0:2] == '--': + # process a single long option (possibly with value(s)) + self._process_long_opt(rargs, values) + elif arg[:1] == '-' and len(arg) > 1: + # process a cluster of short options (possibly with + # value(s) for the last one only) + self._process_short_opts(rargs, values) + elif self.allow_interspersed_args: + largs.append(arg) + del rargs[0] + else: + return # stop now, leave this arg in rargs + + except (BadOptionError, AmbiguousOptionError) as e: + largs.append(e.opt_str) diff --git a/django_maven/tests.py b/django_maven/tests.py new file mode 100644 index 0000000..6bd20b7 --- /dev/null +++ b/django_maven/tests.py @@ -0,0 +1,72 @@ +try: + from unittest.case import skipIf +except ImportError: + from unittest2.case import skipIf + +from django.core.management.base import BaseCommand, handle_default_options +from django.test import TestCase +from django_maven.management.argparse_command import \ + MavenBaseCommand as ArgparseMavenBaseCommand +from django_maven.management.optparse_command import \ + MavenBaseCommand as OptparseMavenBaseCommand + + +class TestCommandParserMixin(object): + + def test_parse_maven_default_options(self): + args, options = self._handle_argv(['--verbosity', '2']) + self.assertEqual(args, []) + self.assertEqual(int(options['verbosity']), 2) + + def test_parse_subcommand(self): + args, options = self._handle_argv(['subcommand']) + self.assertEqual(args, ['subcommand']) + + def test_should_keep_subcommand_default_options(self): + args, options = self._handle_argv(['subcommand', '--help']) + self.assertEqual(args, ['subcommand', '--help']) + + def test_should_keep_subcommand_custom_options(self): + args, options = self._handle_argv(['subcommand', '--foo']) + self.assertEqual(args, ['subcommand', '--foo']) + + def test_mavens_and_subcommands_default_options_should_not_conflict(self): + args, options = self._handle_argv( + ['--verbosity', '2', 'subcommand', '--verbosity', '3']) + self.assertEqual(int(options['verbosity']), 2) + self.assertEqual(args, ['subcommand', '--verbosity', '3']) + + def _handle_argv(self, argv): + raise NotImplementedError() + + +@skipIf(not hasattr(BaseCommand, 'use_argparse'), + 'This Django doesn\'t use argparse') +class ArgparseCommandParserTest(TestCommandParserMixin, TestCase): + """ + :type parser: django.core.management.base.CommandParser""" + + def setUp(self): + self.cmd = ArgparseMavenBaseCommand() + self.parser = self.cmd.create_parser('manage.py', 'maven') + + def _handle_argv(self, argv): + options = self.parser.parse_args(argv) + cmd_options = vars(options) + args = cmd_options.pop('args', ()) + handle_default_options(options) + return args, cmd_options + + +class OptparseCommandParserTest(TestCommandParserMixin, TestCase): + """:type parser: django.core.management.base.OptionParser""" + + def setUp(self): + self.cmd = OptparseMavenBaseCommand() + self.parser = self.cmd.create_parser('manage.py', 'maven') + + def _handle_argv(self, argv): + options, args = self.parser.parse_args(argv) + cmd_options = vars(options) + handle_default_options(options) + return args, cmd_options diff --git a/setup.py b/setup.py index a26f12b..0d62edd 100644 --- a/setup.py +++ b/setup.py @@ -1,5 +1,6 @@ import os -from setuptools import setup, find_packages + +from setuptools import find_packages, setup def read(filename): @@ -32,6 +33,8 @@ def read(filename): 'Programming Language :: Python', 'Programming Language :: Python :: 2.6', 'Programming Language :: Python :: 2.7', + 'Programming Language :: Python :: 3.4', + 'Programming Language :: Python :: 3.5', 'Topic :: Internet :: WWW/HTTP' ], ) diff --git a/test_project/test_project/__init__.py b/tests/__init__.py similarity index 100% rename from test_project/test_project/__init__.py rename to tests/__init__.py diff --git a/test_project/manage.py b/tests/manage.py similarity index 100% rename from test_project/manage.py rename to tests/manage.py diff --git a/test_project/requirements.txt b/tests/requirements.txt similarity index 100% rename from test_project/requirements.txt rename to tests/requirements.txt diff --git a/tests/test_app/__init__.py b/tests/test_app/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_app/management/__init__.py b/tests/test_app/management/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_app/management/commands/__init__.py b/tests/test_app/management/commands/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_app/management/commands/testcmd.py b/tests/test_app/management/commands/testcmd.py new file mode 100644 index 0000000..902e9af --- /dev/null +++ b/tests/test_app/management/commands/testcmd.py @@ -0,0 +1,24 @@ +from optparse import make_option + +from django.core.management.base import BaseCommand + + +class IgnoramusException(Exception): + pass + + +class Command(BaseCommand): + help = 'Test command' + + option_list = BaseCommand.option_list + ( + make_option('--fail', + action='store_true', + dest='fail', + default=False, + help='Raise exception'), + ) + + def handle(self, *args, **options): + if options['fail']: + raise IgnoramusException('use jQuery') + print ('OK') diff --git a/tests/test_app/models.py b/tests/test_app/models.py new file mode 100644 index 0000000..3997c55 --- /dev/null +++ b/tests/test_app/models.py @@ -0,0 +1 @@ +# naked file diff --git a/tests/test_app/tests.py b/tests/test_app/tests.py new file mode 100644 index 0000000..0a00e94 --- /dev/null +++ b/tests/test_app/tests.py @@ -0,0 +1,68 @@ +from __future__ import unicode_literals + +import locale +import os +import subprocess + +from django.test import TestCase + + +class MavenCommandTest(TestCase): + + def test_subcommand(self): + result = get_output(['django-admin.py', + 'testcmd', '--settings=test_project.settings']) + self.assertEqual('OK', result.output.strip()) + self.assertEqual(0, result.returncode) + + def test_maven_subcommand(self): + result = get_output(['django-admin.py', + 'maven', '--settings=test_project.settings', + 'testcmd']) + self.assertEqual('OK', result.output.strip()) + self.assertEqual(0, result.returncode) + + def test_maven_should_capture_exceptions_from_subcommand(self): + result = get_output(['django-admin.py', + 'maven', '--settings=test_project.settings', + '--traceback', '--verbosity=2', + 'testcmd', '--fail']) + self.assertIn('Beautiful is better than IgnoramusException. ' + 'Errors should never pass silently.', + result.output.strip()) + self.assertIn('Traceback', result.output.strip()) + self.assertNotEqual(0, result.returncode) + + +def get_output(*popenargs, **kwargs): + r"""Run command with arguments and return its output as a string.""" + + def decode(bytes): + return bytes.decode(locale.getpreferredencoding(False), 'ignore') + + process = subprocess.Popen(stdout=subprocess.PIPE, stderr=subprocess.PIPE, + env=dict(os.environ), + *popenargs, **kwargs) + output, err = process.communicate() + retcode = process.poll() + cmd = kwargs.get('args') + if cmd is None: + cmd = popenargs[0] + if retcode: + error = subprocess.CalledProcessError(retcode, cmd) + error.output = decode(err) + return error + else: + result = CalledProcessResult(cmd, decode(output)) + return result + + +class CalledProcessResult(object): + + def __init__(self, cmd, output=None): + self.cmd = cmd + self.output = output + self.returncode = 0 + + def __str__(self): + return "Command '%s' success" % (self.cmd) diff --git a/tests/test_project/__init__.py b/tests/test_project/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test_project/test_project/settings.py b/tests/test_project/settings.py similarity index 86% rename from test_project/test_project/settings.py rename to tests/test_project/settings.py index 8b3d383..e33434e 100644 --- a/test_project/test_project/settings.py +++ b/tests/test_project/settings.py @@ -11,14 +11,9 @@ DATABASES = { 'default': { - 'ENGINE': 'django.db.backends.sqlite3', # Add 'postgresql_psycopg2', 'mysql', 'sqlite3' or 'oracle'. - 'NAME': 'sqlite3.db', # Or path to database file if using sqlite3. - # The following settings are not used with sqlite3: - 'USER': '', - 'PASSWORD': '', - 'HOST': '', # Empty for localhost through domain sockets or '127.0.0.1' for localhost through TCP. - 'PORT': '', # Set to empty string for default. - } + 'ENGINE': 'django.db.backends.sqlite3', + 'NAME': ':memory:', + }, } # Hosts/domain names that are valid for this site; required if DEBUG is False @@ -79,7 +74,7 @@ STATICFILES_FINDERS = ( 'django.contrib.staticfiles.finders.FileSystemFinder', 'django.contrib.staticfiles.finders.AppDirectoriesFinder', -# 'django.contrib.staticfiles.finders.DefaultStorageFinder', + # 'django.contrib.staticfiles.finders.DefaultStorageFinder', ) # Make this unique, and don't share it with anybody. @@ -89,7 +84,7 @@ TEMPLATE_LOADERS = ( 'django.template.loaders.filesystem.Loader', 'django.template.loaders.app_directories.Loader', -# 'django.template.loaders.eggs.Loader', + # 'django.template.loaders.eggs.Loader', ) MIDDLEWARE_CLASSES = ( @@ -121,6 +116,7 @@ 'django.contrib.messages', 'django.contrib.staticfiles', 'django_maven', + 'test_app' # Uncomment the next line to enable the admin: # 'django.contrib.admin', # Uncomment the next line to enable admin documentation: diff --git a/test_project/test_project/urls.py b/tests/test_project/urls.py similarity index 100% rename from test_project/test_project/urls.py rename to tests/test_project/urls.py diff --git a/test_project/test_project/wsgi.py b/tests/test_project/wsgi.py similarity index 100% rename from test_project/test_project/wsgi.py rename to tests/test_project/wsgi.py diff --git a/tox.ini b/tox.ini new file mode 100644 index 0000000..e8ab86e --- /dev/null +++ b/tox.ini @@ -0,0 +1,22 @@ +[tox] +envlist = + py{26}-dj{15,16} + py{27,34}-dj{15,16,17,18,19} + py{33}-dj{15,16,17,18} + py{35}-dj{18,19} +skipsdist = True + +[testenv] +changedir = {toxinidir}/tests +commands = python manage.py test django_maven test_app +deps = + raven + py26: unittest2 + dj15: Django>=1.5,<1.6 + dj16: Django>=1.6,<1.7 + dj17: Django>=1.7,<1.8 + dj18: Django>=1.8,<1.9 + dj19: Django>=1.9,<1.10 +setenv = + PYTHONPATH = {toxinidir}:{toxinidir}/tests +usedevelop = True