From b6c0cce3e6aa450a2d130bf59b19b7cd34f1291c Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Fri, 12 May 2017 20:37:16 -0500 Subject: [PATCH 1/3] Add lower bound on setuptools Partial #320 --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 5a40d9b..0bb9d2e 100644 --- a/setup.py +++ b/setup.py @@ -20,6 +20,7 @@ requires = [ "pyflakes >= 1.5.0, < 1.6.0", "pycodestyle >= 2.0.0, < 2.4.0", "mccabe >= 0.6.0, < 0.7.0", + "setuptools >= 30", ] if sys.version_info < (3, 4): From 15ddc3aa2e32187a2079d87f9e91fd2e6f03f60c Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 13 May 2017 06:52:08 -0500 Subject: [PATCH 2/3] Handle missing default formatter In the event that we cannot load our plugins, we shouldn't raise a cryptic KeyError from our formatter. Closes #320 --- src/flake8/exceptions.py | 4 ++++ src/flake8/main/application.py | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/flake8/exceptions.py b/src/flake8/exceptions.py index cf8aae3..13e8996 100644 --- a/src/flake8/exceptions.py +++ b/src/flake8/exceptions.py @@ -13,6 +13,10 @@ class EarlyQuit(Flake8Exception): pass +class ExecutionError(Flake8Exception): + """Exception raised during execution of Flake8.""" + + class FailedToLoadPlugin(Flake8Exception): """Exception raised when a plugin fails to load.""" diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 2eae382..01912b3 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -195,9 +195,19 @@ class Application(object): elif 2 <= self.options.quiet: format_plugin = 'quiet-nothing' + try: + default_formatter = self.formatting_plugins['default'] + except KeyError: + raise exceptions.ExecutionError( + "The 'default' Flake8 formatting plugin is unavailable. " + "This usually indicates that your setuptools is too old. " + "Please upgrade setuptools. If that does not fix the issue" + " please file an issue." + ) + if formatter_class is None: formatter_class = self.formatting_plugins.get( - format_plugin, self.formatting_plugins['default'] + format_plugin, default_formatter ).execute self.formatter = formatter_class(self.options) @@ -332,6 +342,11 @@ class Application(object): LOG.exception(exc) self.file_checker_manager._force_cleanup() self.catastrophic_failure = True + except exceptions.ExecutionError as exc: + print('There was a critical error during execution of Flake8:') + print(exc.message) + LOG.exception(exc) + self.catastrophic_failure = True except exceptions.EarlyQuit: self.catastrophic_failure = True print('... stopped while processing files') From c62de6acc3a4590c1558a6e8843d31ac2a8d701a Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 13 May 2017 08:00:43 -0500 Subject: [PATCH 3/3] Make formatting plugin logic easier to test By splitting out the logic to retrieve and return the formatting class for an application, we can test it more easily and increase our test coverage of this critical logic. Refs #320 --- src/flake8/main/application.py | 36 +++++++++------ tests/unit/test_application.py | 81 +++++++++++++++++++++++++--------- 2 files changed, 84 insertions(+), 33 deletions(-) diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 01912b3..fb8df28 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -185,6 +185,28 @@ class Application(object): self.options, self.args) + def formatter_for(self, formatter_plugin_name): + """Retrieve the formatter class by plugin name.""" + try: + default_formatter = self.formatting_plugins['default'] + except KeyError: + raise exceptions.ExecutionError( + "The 'default' Flake8 formatting plugin is unavailable. " + "This usually indicates that your setuptools is too old. " + "Please upgrade setuptools. If that does not fix the issue" + " please file an issue." + ) + + formatter_plugin = self.formatting_plugins.get(formatter_plugin_name) + if formatter_plugin is None: + LOG.warning( + '"%s" is an unknown formatter. Falling back to default.', + formatter_plugin_name, + ) + formatter_plugin = default_formatter + + return formatter_plugin.execute + def make_formatter(self, formatter_class=None): # type: () -> NoneType """Initialize a formatter based on the parsed options.""" @@ -195,20 +217,8 @@ class Application(object): elif 2 <= self.options.quiet: format_plugin = 'quiet-nothing' - try: - default_formatter = self.formatting_plugins['default'] - except KeyError: - raise exceptions.ExecutionError( - "The 'default' Flake8 formatting plugin is unavailable. " - "This usually indicates that your setuptools is too old. " - "Please upgrade setuptools. If that does not fix the issue" - " please file an issue." - ) - if formatter_class is None: - formatter_class = self.formatting_plugins.get( - format_plugin, default_formatter - ).execute + formatter_class = self.formatter_for(format_plugin) self.formatter = formatter_class(self.options) diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 6a3ef91..7930228 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -4,6 +4,7 @@ import optparse import mock import pytest +from flake8 import exceptions from flake8.main import application as app @@ -16,6 +17,17 @@ def options(**kwargs): return optparse.Values(kwargs) +@pytest.fixture +def mocked_application(): + """Create an application with a mocked OptionManager.""" + with mock.patch('flake8.options.manager.OptionManager') as optionmanager: + optmgr = optionmanager.return_value = mock.Mock() + optmgr.parse_known_args.return_value = (options(), []) + application = app.Application() + + return application + + @pytest.mark.parametrize( 'result_count, catastrophic, exit_zero', [ (0, True, True), @@ -23,18 +35,14 @@ def options(**kwargs): (2, True, True), ] ) -def test_exit_does_not_raise(result_count, catastrophic, exit_zero): +def test_exit_does_not_raise(result_count, catastrophic, exit_zero, + mocked_application): """Verify Application.exit doesn't raise SystemExit.""" - with mock.patch('flake8.options.manager.OptionManager') as optionmanager: - optmgr = optionmanager.return_value = mock.Mock() - optmgr.parse_known_args.return_value = (options(), []) - application = app.Application() + mocked_application.result_count = result_count + mocked_application.catastrophic_failure = catastrophic + mocked_application.options = options(exit_zero=exit_zero) - application.result_count = result_count - application.catastrophic_failure = catastrophic - application.options = options(exit_zero=exit_zero) - - assert application.exit() is None + assert mocked_application.exit() is None @pytest.mark.parametrize( @@ -45,18 +53,51 @@ def test_exit_does_not_raise(result_count, catastrophic, exit_zero): (2, True, False, True), ] ) -def test_exit_does_raise(result_count, catastrophic, exit_zero, value): +def test_exit_does_raise(result_count, catastrophic, exit_zero, value, + mocked_application): """Verify Application.exit doesn't raise SystemExit.""" - with mock.patch('flake8.options.manager.OptionManager') as optionmanager: - optmgr = optionmanager.return_value = mock.Mock() - optmgr.parse_known_args.return_value = (options(), []) - application = app.Application() - - application.result_count = result_count - application.catastrophic_failure = catastrophic - application.options = options(exit_zero=exit_zero) + mocked_application.result_count = result_count + mocked_application.catastrophic_failure = catastrophic + mocked_application.options = options(exit_zero=exit_zero) with pytest.raises(SystemExit) as excinfo: - application.exit() + mocked_application.exit() assert excinfo.value.args[0] is value + + +def test_missing_default_formatter(mocked_application): + """Verify we raise an ExecutionError when there's no default formatter.""" + mocked_application.formatting_plugins = {} + + with pytest.raises(exceptions.ExecutionError): + mocked_application.formatter_for('fake-plugin-name') + + +def test_warns_on_unknown_formatter_plugin_name(mocked_application): + """Verify we log a warning with an unfound plugin.""" + default = mock.Mock() + execute = default.execute + mocked_application.formatting_plugins = { + 'default': default, + } + with mock.patch.object(app.LOG, 'warning') as warning: + assert execute is mocked_application.formatter_for('fake-plugin-name') + + assert warning.called is True + assert warning.call_count == 1 + + +def test_returns_specified_plugin(mocked_application): + """Verify we get the plugin we want.""" + desired = mock.Mock() + execute = desired.execute + mocked_application.formatting_plugins = { + 'default': mock.Mock(), + 'desired': desired, + } + + with mock.patch.object(app.LOG, 'warning') as warning: + assert execute is mocked_application.formatter_for('desired') + + assert warning.called is False