From 56c2a9cd9ba3b8ef4b470403fec5ba67cc5a9588 Mon Sep 17 00:00:00 2001 From: michalfrak Date: Thu, 12 Oct 2017 08:51:22 +0200 Subject: [PATCH 1/5] [PYB_01] Pylint plugin shows logs in shell and breaks build on certain conditions --- .../pybuilder/plugins/python/pylint_plugin.py | 65 ++++++++++++++++++- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/src/main/python/pybuilder/plugins/python/pylint_plugin.py b/src/main/python/pybuilder/plugins/python/pylint_plugin.py index ecd1543fd..b48d46154 100644 --- a/src/main/python/pybuilder/plugins/python/pylint_plugin.py +++ b/src/main/python/pybuilder/plugins/python/pylint_plugin.py @@ -17,8 +17,9 @@ # limitations under the License. from pybuilder.core import use_plugin, after, init, task -from pybuilder.utils import assert_can_execute +from pybuilder.errors import BuildFailedException from pybuilder.plugins.python.python_plugin_helper import execute_tool_on_modules +from pybuilder.utils import assert_can_execute use_plugin("python.core") use_plugin("analysis") @@ -44,4 +45,64 @@ def execute_pylint(project, logger): logger.info("Executing pylint on project sources") command_and_arguments = ["pylint"] + project.get_property("pylint_options") - execute_tool_on_modules(project, "pylint", command_and_arguments, True) + pylint_output_file_path = execute_tool_on_modules(project, "pylint", command_and_arguments, True)[1] + + with open(pylint_output_file_path, 'r') as f: + file_content = f.read().splitlines() + + module_name = '' + pylint_score = 0 + pylint_change = 0 + errors = 0 + errors_info = '' + warnings = 0 + warnings_info = '' + show_info_messages = project.get_property('pylint_show_info_messages') + show_warning_messages = project.get_property('pylint_show_warning_messages') + for line in file_content: + if line.startswith('************* Module'): + module_name = line.split(' ')[2] + + if line.startswith('E:') or line.startswith('F:'): + logger.error('Pylint: Module {}: '.format(module_name) + line) + errors += 1 + + if show_warning_messages and line.startswith('W:'): + logger.warn('Pylint: Module {}: '.format(module_name) + line) + warnings += 1 + + if show_info_messages and (line.startswith('C:') or line.startswith('R:')): + logger.info('Pylint: Module {}: '.format(module_name) + line) + + if line.startswith('Your code has been rated at '): + pylint_score = float(line.split(' ')[6].split('/')[0]) + pylint_change = float(line.split(' ')[10][:-1]) + + if errors > 0: + errors_info = ' / Errors: {}'.format(errors) + + if warnings > 0: + warnings_info = ' / Warnings {}'.format(warnings) + + logger.info( + 'Pylint ratio: {} / Pylint change: {}'.format(pylint_score, pylint_change) + errors_info + warnings_info + ) + + if errors > 0 and project.get_property('pylint_break_build_on_errors'): + raise BuildFailedException( + "Pylint: Building failed due to {} errors or fatal errors".format(errors) + ) + + pylint_expected_score = project.get_property('pylint_score_threshold') + pylint_expected_score_change = project.get_property('pylint_score_change_threshold') + if pylint_expected_score and pylint_score < pylint_expected_score: + raise BuildFailedException( + "Pylint: Building failed due to Pylint score({}) less then expected({})". + format(pylint_score, pylint_expected_score) + ) + + if pylint_expected_score_change and pylint_change < pylint_expected_score_change: + raise BuildFailedException( + "Pylint: Building failed due to Pylint score decrease({}) higher then allowed({})". + format(pylint_change, pylint_expected_score_change) + ) From 79c85ec3366e7d559189c3145536e44179aa608a Mon Sep 17 00:00:00 2001 From: michalfrak Date: Fri, 13 Oct 2017 10:15:48 +0200 Subject: [PATCH 2/5] [PYB_01] Unit tests --- .../pybuilder/plugins/python/pylint_plugin.py | 7 +- .../plugins/python/pylint_plugin_tests.py | 76 ++++++++++++++----- 2 files changed, 59 insertions(+), 24 deletions(-) diff --git a/src/main/python/pybuilder/plugins/python/pylint_plugin.py b/src/main/python/pybuilder/plugins/python/pylint_plugin.py index b48d46154..99f473af1 100644 --- a/src/main/python/pybuilder/plugins/python/pylint_plugin.py +++ b/src/main/python/pybuilder/plugins/python/pylint_plugin.py @@ -64,15 +64,15 @@ def execute_pylint(project, logger): module_name = line.split(' ')[2] if line.startswith('E:') or line.startswith('F:'): - logger.error('Pylint: Module {}: '.format(module_name) + line) + logger.error('Pylint: Module %s: ' % module_name + line) errors += 1 if show_warning_messages and line.startswith('W:'): - logger.warn('Pylint: Module {}: '.format(module_name) + line) + logger.warn('Pylint: Module %s: ' % module_name + line) warnings += 1 if show_info_messages and (line.startswith('C:') or line.startswith('R:')): - logger.info('Pylint: Module {}: '.format(module_name) + line) + logger.info('Pylint: Module %s: ' % module_name + line) if line.startswith('Your code has been rated at '): pylint_score = float(line.split(' ')[6].split('/')[0]) @@ -100,7 +100,6 @@ def execute_pylint(project, logger): "Pylint: Building failed due to Pylint score({}) less then expected({})". format(pylint_score, pylint_expected_score) ) - if pylint_expected_score_change and pylint_change < pylint_expected_score_change: raise BuildFailedException( "Pylint: Building failed due to Pylint score decrease({}) higher then allowed({})". diff --git a/src/unittest/python/plugins/python/pylint_plugin_tests.py b/src/unittest/python/plugins/python/pylint_plugin_tests.py index 80b1edb66..2aad7df93 100644 --- a/src/unittest/python/plugins/python/pylint_plugin_tests.py +++ b/src/unittest/python/plugins/python/pylint_plugin_tests.py @@ -15,45 +15,81 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - -from unittest import TestCase -from test_utils import Mock, patch +import os from logging import Logger +from tempfile import NamedTemporaryFile +from unittest import TestCase from pybuilder.core import Project +from pybuilder.errors import BuildFailedException from pybuilder.plugins.python.pylint_plugin import (check_pylint_availability, init_pylint, execute_pylint, DEFAULT_PYLINT_OPTIONS) +from test_utils import Mock, patch class PylintPluginTests(TestCase): + def setUp(self): + self.project = Project(".") + init_pylint(self.project) + self.mock_logger = Mock(Logger) + @patch('pybuilder.plugins.python.pylint_plugin.assert_can_execute') def test_should_check_that_pylint_can_be_executed(self, mock_assert_can_execute): + check_pylint_availability(self.mock_logger) + expected_command_line = ('pylint',) + mock_assert_can_execute.assert_called_with(expected_command_line, 'pylint', 'plugin python.pylint') - mock_logger = Mock(Logger) + def test_should_run_pylint_with_default_options(self): + self._make_temp_test_file('E:476, 8: Error message') + self._run_execute_tool().assert_called_with(self.project, "pylint", ["pylint"] + DEFAULT_PYLINT_OPTIONS, True) - check_pylint_availability(mock_logger) + def test_should_run_pylint_with_custom_options(self): + self._make_temp_test_file('E:476, 8: Error message') + self.project.set_property("pylint_options", ["--test", "-f", "--x=y"]) + self._run_execute_tool().assert_called_with(self.project, "pylint", ["pylint", "--test", "-f", "--x=y"], True) - expected_command_line = ('pylint',) - mock_assert_can_execute.assert_called_with(expected_command_line, 'pylint', 'plugin python.pylint') + def test_should_show_error_message_in_pyb_logs(self): + self._make_temp_test_file('E:476, 8: Error message') + self._run_execute_tool() + self.mock_logger.error.assert_called_with('Pylint: Module : E:476, 8: Error message') - @patch('pybuilder.plugins.python.pylint_plugin.execute_tool_on_modules') - def test_should_run_pylint_with_default_options(self, execute_tool): - project = Project(".") - init_pylint(project) + def test_should_show_warning_message_in_pyb_logs(self): + self._make_temp_test_file('W:2476, 8: Warning message') + self.project.set_property('pylint_show_warning_messages', True) + self._run_execute_tool() + self.mock_logger.warn.assert_called_with('Pylint: Module : W:2476, 8: Warning message') - execute_pylint(project, Mock(Logger)) + def test_should_break_build_on_errors(self): + self._make_temp_test_file('E:2476, 8: Warning message') + self.project.set_property('pylint_break_build_on_errors', True) + with self.assertRaises(BuildFailedException): + self._run_execute_tool() - execute_tool.assert_called_with(project, "pylint", ["pylint"] + DEFAULT_PYLINT_OPTIONS, True) + def test_should_break_build_on_too_low_pylint_score(self): + self._make_temp_test_file('Your code has been rated at 4.60/10 (previous run: 4.60/10, +0.00)') + self.project.set_property('pylint_score_threshold', 5.0) + with self.assertRaises(BuildFailedException): + self._run_execute_tool() - @patch('pybuilder.plugins.python.pylint_plugin.execute_tool_on_modules') - def test_should_run_pylint_with_custom_options(self, execute_tool): - project = Project(".") - init_pylint(project) - project.set_property("pylint_options", ["--test", "-f", "--x=y"]) + def test_should_break_build_on_too_high_pylint_score_decrease(self): + self._make_temp_test_file('Your code has been rated at 4.60/10 (previous run: 6.60/10, -2.00)') + self.project.set_property('pylint_score_change_threshold', -1.0) + with self.assertRaises(BuildFailedException): + self._run_execute_tool() - execute_pylint(project, Mock(Logger)) + @staticmethod + def _make_temp_test_file(test_file_content): + temp_file = NamedTemporaryFile(delete=False) + temp_file.name = 'test.txt' + with open('test.txt', 'w') as f: + f.write(test_file_content) - execute_tool.assert_called_with(project, "pylint", ["pylint", "--test", "-f", "--x=y"], True) + @patch('pybuilder.plugins.python.pylint_plugin.execute_tool_on_modules') + def _run_execute_tool(self, execute_tool): + execute_tool.return_value = (0, 'test.txt') + execute_pylint(self.project, self.mock_logger) + os.remove('test.txt') + return execute_tool From 01c9c592d9f71f72fbdef74b2ce1af8b7dade861 Mon Sep 17 00:00:00 2001 From: michalfrak Date: Wed, 18 Oct 2017 08:29:04 +0200 Subject: [PATCH 3/5] [PYB_01] Code review - patch over class --- .../plugins/python/pylint_plugin_tests.py | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/src/unittest/python/plugins/python/pylint_plugin_tests.py b/src/unittest/python/plugins/python/pylint_plugin_tests.py index 2aad7df93..cbd0901b8 100644 --- a/src/unittest/python/plugins/python/pylint_plugin_tests.py +++ b/src/unittest/python/plugins/python/pylint_plugin_tests.py @@ -15,7 +15,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import os from logging import Logger from tempfile import NamedTemporaryFile from unittest import TestCase @@ -29,6 +28,7 @@ from test_utils import Mock, patch +@patch('pybuilder.plugins.python.pylint_plugin.execute_tool_on_modules') class PylintPluginTests(TestCase): def setUp(self): @@ -37,59 +37,60 @@ def setUp(self): self.mock_logger = Mock(Logger) @patch('pybuilder.plugins.python.pylint_plugin.assert_can_execute') - def test_should_check_that_pylint_can_be_executed(self, mock_assert_can_execute): + def test_should_check_that_pylint_can_be_executed(self, mock_assert_can_execute, execute_tool): check_pylint_availability(self.mock_logger) expected_command_line = ('pylint',) mock_assert_can_execute.assert_called_with(expected_command_line, 'pylint', 'plugin python.pylint') - def test_should_run_pylint_with_default_options(self): + def test_should_run_pylint_with_default_options(self, execute_tool): self._make_temp_test_file('E:476, 8: Error message') - self._run_execute_tool().assert_called_with(self.project, "pylint", ["pylint"] + DEFAULT_PYLINT_OPTIONS, True) + self._run_execute_tool(execute_tool).assert_called_with( + self.project, "pylint", ["pylint"] + DEFAULT_PYLINT_OPTIONS, True + ) - def test_should_run_pylint_with_custom_options(self): + def test_should_run_pylint_with_custom_options(self, execute_tool): self._make_temp_test_file('E:476, 8: Error message') self.project.set_property("pylint_options", ["--test", "-f", "--x=y"]) - self._run_execute_tool().assert_called_with(self.project, "pylint", ["pylint", "--test", "-f", "--x=y"], True) + self._run_execute_tool(execute_tool).assert_called_with( + self.project, "pylint", ["pylint", "--test", "-f", "--x=y"], True + ) - def test_should_show_error_message_in_pyb_logs(self): + def test_should_show_error_message_in_pyb_logs(self, execute_tool): self._make_temp_test_file('E:476, 8: Error message') - self._run_execute_tool() + self._run_execute_tool(execute_tool) self.mock_logger.error.assert_called_with('Pylint: Module : E:476, 8: Error message') - def test_should_show_warning_message_in_pyb_logs(self): + def test_should_show_warning_message_in_pyb_logs(self, execute_tool): self._make_temp_test_file('W:2476, 8: Warning message') self.project.set_property('pylint_show_warning_messages', True) - self._run_execute_tool() + self._run_execute_tool(execute_tool) self.mock_logger.warn.assert_called_with('Pylint: Module : W:2476, 8: Warning message') - def test_should_break_build_on_errors(self): + def test_should_break_build_on_errors(self, execute_tool): self._make_temp_test_file('E:2476, 8: Warning message') self.project.set_property('pylint_break_build_on_errors', True) with self.assertRaises(BuildFailedException): - self._run_execute_tool() + self._run_execute_tool(execute_tool) - def test_should_break_build_on_too_low_pylint_score(self): + def test_should_break_build_on_too_low_pylint_score(self, execute_tool): self._make_temp_test_file('Your code has been rated at 4.60/10 (previous run: 4.60/10, +0.00)') self.project.set_property('pylint_score_threshold', 5.0) with self.assertRaises(BuildFailedException): - self._run_execute_tool() + self._run_execute_tool(execute_tool) - def test_should_break_build_on_too_high_pylint_score_decrease(self): + def test_should_break_build_on_too_high_pylint_score_decrease(self, execute_tool): self._make_temp_test_file('Your code has been rated at 4.60/10 (previous run: 6.60/10, -2.00)') self.project.set_property('pylint_score_change_threshold', -1.0) with self.assertRaises(BuildFailedException): - self._run_execute_tool() + self._run_execute_tool(execute_tool) - @staticmethod - def _make_temp_test_file(test_file_content): - temp_file = NamedTemporaryFile(delete=False) - temp_file.name = 'test.txt' - with open('test.txt', 'w') as f: + def _make_temp_test_file(self, test_file_content): + self.temp_file = NamedTemporaryFile() + with open(self.temp_file.name, 'w') as f: f.write(test_file_content) + return self.temp_file - @patch('pybuilder.plugins.python.pylint_plugin.execute_tool_on_modules') def _run_execute_tool(self, execute_tool): - execute_tool.return_value = (0, 'test.txt') + execute_tool.return_value = (0, self.temp_file.name) execute_pylint(self.project, self.mock_logger) - os.remove('test.txt') return execute_tool From dfc9b096257a43fb23b912982a4580322536bd89 Mon Sep 17 00:00:00 2001 From: michalfrak Date: Wed, 18 Oct 2017 09:07:19 +0200 Subject: [PATCH 4/5] [PYB_01] Code review - execution separated from assertion --- .../plugins/python/pylint_plugin_tests.py | 67 ++++++++++--------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/src/unittest/python/plugins/python/pylint_plugin_tests.py b/src/unittest/python/plugins/python/pylint_plugin_tests.py index cbd0901b8..c4d76d79b 100644 --- a/src/unittest/python/plugins/python/pylint_plugin_tests.py +++ b/src/unittest/python/plugins/python/pylint_plugin_tests.py @@ -37,60 +37,61 @@ def setUp(self): self.mock_logger = Mock(Logger) @patch('pybuilder.plugins.python.pylint_plugin.assert_can_execute') - def test_should_check_that_pylint_can_be_executed(self, mock_assert_can_execute, execute_tool): + def test_should_check_that_pylint_can_be_executed(self, mock_assert_can_execute, mock_execute_tool): check_pylint_availability(self.mock_logger) expected_command_line = ('pylint',) mock_assert_can_execute.assert_called_with(expected_command_line, 'pylint', 'plugin python.pylint') - def test_should_run_pylint_with_default_options(self, execute_tool): - self._make_temp_test_file('E:476, 8: Error message') - self._run_execute_tool(execute_tool).assert_called_with( - self.project, "pylint", ["pylint"] + DEFAULT_PYLINT_OPTIONS, True - ) + def test_should_run_pylint_with_default_options(self, mock_execute_tool): + self._create_pylint_log_file('E:476, 8: Error message') + execute_tool = self._run_execute_tool(mock_execute_tool) + execute_tool.assert_called_with(self.project, "pylint", ["pylint"] + DEFAULT_PYLINT_OPTIONS, True) - def test_should_run_pylint_with_custom_options(self, execute_tool): - self._make_temp_test_file('E:476, 8: Error message') + def test_should_run_pylint_with_custom_options(self, mock_execute_tool): + self._create_pylint_log_file('E:476, 8: Error message') self.project.set_property("pylint_options", ["--test", "-f", "--x=y"]) - self._run_execute_tool(execute_tool).assert_called_with( - self.project, "pylint", ["pylint", "--test", "-f", "--x=y"], True - ) + execute_tool = self._run_execute_tool(mock_execute_tool) + execute_tool.assert_called_with(self.project, "pylint", ["pylint", "--test", "-f", "--x=y"], True) - def test_should_show_error_message_in_pyb_logs(self, execute_tool): - self._make_temp_test_file('E:476, 8: Error message') - self._run_execute_tool(execute_tool) + def test_should_show_error_message_in_pyb_logs(self, mock_execute_tool): + self._create_pylint_log_file('E:476, 8: Error message') + self._run_execute_tool(mock_execute_tool) self.mock_logger.error.assert_called_with('Pylint: Module : E:476, 8: Error message') - def test_should_show_warning_message_in_pyb_logs(self, execute_tool): - self._make_temp_test_file('W:2476, 8: Warning message') + def test_should_show_warning_message_in_pyb_logs(self, mock_execute_tool): + self._create_pylint_log_file('W:2476, 8: Warning message') self.project.set_property('pylint_show_warning_messages', True) - self._run_execute_tool(execute_tool) + self._run_execute_tool(mock_execute_tool) self.mock_logger.warn.assert_called_with('Pylint: Module : W:2476, 8: Warning message') - def test_should_break_build_on_errors(self, execute_tool): - self._make_temp_test_file('E:2476, 8: Warning message') + def test_should_break_build_on_errors(self, mock_execute_tool): + self._create_pylint_log_file('E:2476, 8: Warning message') self.project.set_property('pylint_break_build_on_errors', True) - with self.assertRaises(BuildFailedException): - self._run_execute_tool(execute_tool) + expected_message = 'Pylint: Building failed due to 1 errors or fatal errors' + with self.assertRaisesRegexp(BuildFailedException, expected_message): + self._run_execute_tool(mock_execute_tool) - def test_should_break_build_on_too_low_pylint_score(self, execute_tool): - self._make_temp_test_file('Your code has been rated at 4.60/10 (previous run: 4.60/10, +0.00)') + def test_should_break_build_on_too_low_pylint_score(self, mock_execute_tool): + self._create_pylint_log_file('Your code has been rated at 4.60/10 (previous run: 4.60/10, +0.00)') self.project.set_property('pylint_score_threshold', 5.0) - with self.assertRaises(BuildFailedException): - self._run_execute_tool(execute_tool) + expected_message = 'Pylint: Building failed due to Pylint score\(4.6\) less then expected\(5.0\)' + with self.assertRaisesRegexp(BuildFailedException, expected_message): + self._run_execute_tool(mock_execute_tool) - def test_should_break_build_on_too_high_pylint_score_decrease(self, execute_tool): - self._make_temp_test_file('Your code has been rated at 4.60/10 (previous run: 6.60/10, -2.00)') + def test_should_break_build_on_too_high_pylint_score_decrease(self, mock_execute_tool): + self._create_pylint_log_file('Your code has been rated at 4.60/10 (previous run: 6.60/10, -2.00)') self.project.set_property('pylint_score_change_threshold', -1.0) - with self.assertRaises(BuildFailedException): - self._run_execute_tool(execute_tool) + expected_message = 'Pylint: Building failed due to Pylint score decrease\(-2.0\) higher then allowed\(-1.0\)' + with self.assertRaisesRegexp(BuildFailedException, expected_message): + self._run_execute_tool(mock_execute_tool) - def _make_temp_test_file(self, test_file_content): + def _create_pylint_log_file(self, test_file_content): self.temp_file = NamedTemporaryFile() with open(self.temp_file.name, 'w') as f: f.write(test_file_content) return self.temp_file - def _run_execute_tool(self, execute_tool): - execute_tool.return_value = (0, self.temp_file.name) + def _run_execute_tool(self, mock_execute_tool): + mock_execute_tool.return_value = (0, self.temp_file.name) execute_pylint(self.project, self.mock_logger) - return execute_tool + return mock_execute_tool From 7886ae633ee9213985c5d0048d311a9f14063096 Mon Sep 17 00:00:00 2001 From: michalfrak Date: Wed, 18 Oct 2017 11:08:56 +0200 Subject: [PATCH 5/5] [PYB_01] Code review --- .../python/plugins/python/pylint_plugin_tests.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/unittest/python/plugins/python/pylint_plugin_tests.py b/src/unittest/python/plugins/python/pylint_plugin_tests.py index c4d76d79b..b105de6d6 100644 --- a/src/unittest/python/plugins/python/pylint_plugin_tests.py +++ b/src/unittest/python/plugins/python/pylint_plugin_tests.py @@ -44,14 +44,14 @@ def test_should_check_that_pylint_can_be_executed(self, mock_assert_can_execute, def test_should_run_pylint_with_default_options(self, mock_execute_tool): self._create_pylint_log_file('E:476, 8: Error message') - execute_tool = self._run_execute_tool(mock_execute_tool) - execute_tool.assert_called_with(self.project, "pylint", ["pylint"] + DEFAULT_PYLINT_OPTIONS, True) + self._run_execute_tool(mock_execute_tool) + mock_execute_tool.assert_called_with(self.project, "pylint", ["pylint"] + DEFAULT_PYLINT_OPTIONS, True) def test_should_run_pylint_with_custom_options(self, mock_execute_tool): self._create_pylint_log_file('E:476, 8: Error message') self.project.set_property("pylint_options", ["--test", "-f", "--x=y"]) - execute_tool = self._run_execute_tool(mock_execute_tool) - execute_tool.assert_called_with(self.project, "pylint", ["pylint", "--test", "-f", "--x=y"], True) + self._run_execute_tool(mock_execute_tool) + mock_execute_tool.assert_called_with(self.project, "pylint", ["pylint", "--test", "-f", "--x=y"], True) def test_should_show_error_message_in_pyb_logs(self, mock_execute_tool): self._create_pylint_log_file('E:476, 8: Error message') @@ -89,9 +89,7 @@ def _create_pylint_log_file(self, test_file_content): self.temp_file = NamedTemporaryFile() with open(self.temp_file.name, 'w') as f: f.write(test_file_content) - return self.temp_file def _run_execute_tool(self, mock_execute_tool): mock_execute_tool.return_value = (0, self.temp_file.name) execute_pylint(self.project, self.mock_logger) - return mock_execute_tool