From c62de6acc3a4590c1558a6e8843d31ac2a8d701a Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 13 May 2017 08:00:43 -0500 Subject: [PATCH] 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