Repository: pybuilder/pybuilder
Issue: flake8 script file filter properties misbehave

Tried excluding bash files without `.sh` extensions from testing for flake8 by appending to a property, but was unsuccessful, had to disable flake8 on scripts entirely.

Looks like a bug, need to investigate.


Changed files:
src/unittest/python/external_command_tests.py, src/unittest/python/plugins/python/python_plugin_helper_tests.py, src/main/python/pybuilder/pluginhelper/external_command.py, src/main/python/pybuilder/plugins/python/flake8_plugin.py, src/unittest/python/plugins/python/flake8_plugin_tests.py, src/main/python/pybuilder/plugins/python/python_plugin_helper.py

diff --git a/src/main/python/pybuilder/pluginhelper/external_command.py b/src/main/python/pybuilder/pluginhelper/external_command.py
index 2cd39d2..fe2efba 100644
--- a/src/main/python/pybuilder/pluginhelper/external_command.py
+++ b/src/main/python/pybuilder/pluginhelper/external_command.py
@@ -21,7 +21,6 @@ from pybuilder.utils import read_file, execute_command
 
 
 class ExternalCommandResult(object):
-
     def __init__(self, exit_code, report_file, report_lines, error_report_file, error_report_lines):
         self.exit_code = exit_code
         self.report_file = report_file
@@ -31,7 +30,6 @@ class ExternalCommandResult(object):
 
 
 class ExternalCommandBuilder(object):
-
     def __init__(self, command_name, project):
         self.command_name = command_name
         self.parts = [command_name]
@@ -73,13 +71,15 @@ class ExternalCommandBuilder(object):
                                      outfile_name, outfile_lines,
                                      error_file_name, error_file_lines)
 
-    def run_on_production_source_files(self, logger, include_test_sources=False, include_scripts=False):
+    def run_on_production_source_files(self, logger, include_test_sources=False, include_scripts=False,
+                                       include_dirs_only=False):
         execution_result = execute_tool_on_source_files(project=self.project,
                                                         name=self.command_name,
                                                         command_and_arguments=self.parts,
                                                         include_test_sources=include_test_sources,
                                                         include_scripts=include_scripts,
-                                                        logger=logger)
+                                                        logger=logger,
+                                                        include_dirs_only=include_dirs_only)
         exit_code, report_file = execution_result
         report_lines = read_file(report_file)
         error_report_file = '{0}.err'.format(report_file)  # TODO @mriehl not dry, execute_tool... should return this
diff --git a/src/main/python/pybuilder/plugins/python/flake8_plugin.py b/src/main/python/pybuilder/plugins/python/flake8_plugin.py
index afe2ba3..d750135 100644
--- a/src/main/python/pybuilder/plugins/python/flake8_plugin.py
+++ b/src/main/python/pybuilder/plugins/python/flake8_plugin.py
@@ -39,6 +39,7 @@ def initialize_flake8_plugin(project):
     project.build_depends_on("flake8")
     project.set_property_if_unset("flake8_break_build", False)
     project.set_property_if_unset("flake8_max_line_length", 120)
+    project.set_property_if_unset("flake8_include_patterns", None)
     project.set_property_if_unset("flake8_exclude_patterns", None)
     project.set_property_if_unset("flake8_include_test_sources", False)
     project.set_property_if_unset("flake8_include_scripts", False)
@@ -66,6 +67,7 @@ def analyze(project, logger):
     command = ExternalCommandBuilder('flake8', project)
     command.use_argument('--ignore={0}').formatted_with_truthy_property('flake8_ignore')
     command.use_argument('--max-line-length={0}').formatted_with_property('flake8_max_line_length')
+    command.use_argument('--filename={0}').formatted_with_truthy_property('flake8_include_patterns')
     command.use_argument('--exclude={0}').formatted_with_truthy_property('flake8_exclude_patterns')
 
     include_test_sources = project.get_property("flake8_include_test_sources")
@@ -73,7 +75,8 @@ def analyze(project, logger):
 
     result = command.run_on_production_source_files(logger,
                                                     include_test_sources=include_test_sources,
-                                                    include_scripts=include_scripts)
+                                                    include_scripts=include_scripts,
+                                                    include_dirs_only=True)
 
     count_of_warnings = len(result.report_lines)
     count_of_errors = len(result.error_report_lines)
diff --git a/src/main/python/pybuilder/plugins/python/python_plugin_helper.py b/src/main/python/pybuilder/plugins/python/python_plugin_helper.py
index 25f3d89..49c12ea 100644
--- a/src/main/python/pybuilder/plugins/python/python_plugin_helper.py
+++ b/src/main/python/pybuilder/plugins/python/python_plugin_helper.py
@@ -16,9 +16,10 @@
 #   See the License for the specific language governing permissions and
 #   limitations under the License.
 
-import os
 import itertools
 
+import os
+
 from pybuilder.utils import (discover_modules,
                              discover_files_matching,
                              execute_command,
@@ -50,13 +51,29 @@ def discover_affected_files(include_test_sources, include_scripts, project):
             files = itertools.chain(files, discover_python_files(integrationtest_dir))
     if include_scripts and project.get_property("dir_source_main_scripts"):
         scripts_dir = project.get_property("dir_source_main_scripts")
-        files = itertools.chain(files, discover_files_matching(scripts_dir, "*"))  # we have no idea how scripts might look
+        files = itertools.chain(files,
+                                discover_files_matching(scripts_dir, "*"))  # we have no idea how scripts might look
+    return files
+
+
+def discover_affected_dirs(include_test_sources, include_scripts, project):
+    files = [project.get_property("dir_source_main_python")]
+    if include_test_sources:
+        if project.get_property("dir_source_unittest_python"):
+            files.append(project.get_property("dir_source_unittest_python"))
+        if project.get_property("dir_source_integrationtest_python"):
+            files.append(project.get_property("dir_source_integrationtest_python"))
+    if include_scripts and project.get_property("dir_source_main_scripts"):
+        files.append(project.get_property("dir_source_main_scripts"))
     return files
 
 
 def execute_tool_on_source_files(project, name, command_and_arguments, logger=None,
-                                 include_test_sources=False, include_scripts=False):
-    files = discover_affected_files(include_test_sources, include_scripts, project)
+                                 include_test_sources=False, include_scripts=False, include_dirs_only=False):
+    if include_dirs_only:
+        files = discover_affected_dirs(include_test_sources, include_scripts, project)
+    else:
+        files = discover_affected_files(include_test_sources, include_scripts, project)
 
     command = as_list(command_and_arguments) + [f for f in files]
 
diff --git a/src/unittest/python/external_command_tests.py b/src/unittest/python/external_command_tests.py
index 25192b7..36a2941 100644
--- a/src/unittest/python/external_command_tests.py
+++ b/src/unittest/python/external_command_tests.py
@@ -113,6 +113,22 @@ class ExternalCommandExecutionTests(unittest.TestCase):
 
         execute_command.assert_called_with(['command-name', '--foo', '--bar'], 'any-outfile-name')
 
+    @patch('pybuilder.pluginhelper.external_command.read_file')
+    @patch('pybuilder.pluginhelper.external_command.execute_tool_on_source_files')
+    def test_should_execute_external_command_on_production_source_files_dirs_only(self, execution, read):
+        execution.return_value = 0, '/tmp/reports/command-name'
+        logger = Mock()
+        self.command.run_on_production_source_files(logger, include_dirs_only=True)
+
+        execution.assert_called_with(
+            include_dirs_only=True,
+            include_test_sources=False,
+            include_scripts=False,
+            project=self.project,
+            logger=logger,
+            command_and_arguments=['command-name', '--foo', '--bar'],
+            name='command-name')
+
     @patch('pybuilder.pluginhelper.external_command.read_file')
     @patch('pybuilder.pluginhelper.external_command.execute_tool_on_source_files')
     def test_should_execute_external_command_on_production_source_files(self, execution, read):
@@ -121,6 +137,7 @@ class ExternalCommandExecutionTests(unittest.TestCase):
         self.command.run_on_production_source_files(logger)
 
         execution.assert_called_with(
+            include_dirs_only=False,
             include_test_sources=False,
             include_scripts=False,
             project=self.project,
@@ -136,6 +153,7 @@ class ExternalCommandExecutionTests(unittest.TestCase):
         self.command.run_on_production_and_test_source_files(logger)
 
         execution.assert_called_with(
+            include_dirs_only=False,
             include_test_sources=True,
             include_scripts=False,
             project=self.project,
diff --git a/src/unittest/python/plugins/python/flake8_plugin_tests.py b/src/unittest/python/plugins/python/flake8_plugin_tests.py
index 298aca7..71f5111 100644
--- a/src/unittest/python/plugins/python/flake8_plugin_tests.py
+++ b/src/unittest/python/plugins/python/flake8_plugin_tests.py
@@ -19,6 +19,7 @@ class FlakePluginInitializationTests(TestCase):
         expected_properties = {
             "flake8_break_build": True,
             "flake8_max_line_length": 80,
+            "flake8_include_patterns": "*.py",
             "flake8_exclude_patterns": ".svn",
             "flake8_include_test_sources": True,
             "flake8_include_scripts": True
@@ -33,6 +34,8 @@ class FlakePluginInitializationTests(TestCase):
                 self.project.get_property("flake8_break_build"), True)
             self.assertEquals(
                 self.project.get_property("flake8_max_line_length"), 80)
+            self.assertEquals(
+                self.project.get_property("flake8_include_patterns"), "*.py")
             self.assertEquals(
                 self.project.get_property("flake8_exclude_patterns"), ".svn")
             self.assertEquals(
diff --git a/src/unittest/python/plugins/python/python_plugin_helper_tests.py b/src/unittest/python/plugins/python/python_plugin_helper_tests.py
index 85e237c..a540749 100644
--- a/src/unittest/python/plugins/python/python_plugin_helper_tests.py
+++ b/src/unittest/python/plugins/python/python_plugin_helper_tests.py
@@ -17,15 +17,16 @@
 #   limitations under the License.
 
 import unittest
+
 from mock import Mock, call, patch
 
 from pybuilder.plugins.python.python_plugin_helper import (log_report,
                                                            discover_affected_files,
+                                                           discover_affected_dirs,
                                                            execute_tool_on_source_files)
 
 
 class LogReportsTest(unittest.TestCase):
-
     def test_should_not_warn_when_report_lines_is_empty(self):
         logger = Mock()
         log_report(logger, 'name', [])
@@ -41,7 +42,6 @@ class LogReportsTest(unittest.TestCase):
 
 
 class DiscoverAffectedFilesTest(unittest.TestCase):
-
     @patch('pybuilder.plugins.python.python_plugin_helper.discover_python_files')
     def test_should_discover_source_files_when_test_sources_not_included(self, discover_python_files):
         project = Mock()
@@ -78,13 +78,15 @@ class DiscoverAffectedFilesTest(unittest.TestCase):
         discover_files_matching.assert_called_with('dir_source_main_scripts', '*')
 
     @patch('pybuilder.plugins.python.python_plugin_helper.discover_python_files')
-    def test_should_discover_source_files_when_test_sources_are_included_and_only_unittests(self, discover_python_files):
+    def test_should_discover_source_files_when_test_sources_are_included_and_only_unittests(self,
+                                                                                            discover_python_files):
         project = Mock()
 
         def get_property(property):
             if property == 'dir_source_integrationtest_python':
                 return None
             return property
+
         project.get_property.side_effect = get_property
 
         discover_affected_files(True, False, project)
@@ -94,13 +96,15 @@ class DiscoverAffectedFilesTest(unittest.TestCase):
                           call('dir_source_unittest_python')])
 
     @patch('pybuilder.plugins.python.python_plugin_helper.discover_python_files')
-    def test_should_discover_source_files_when_test_sources_are_included_and_only_integrationtests(self, discover_python_files):
+    def test_should_discover_source_files_when_test_sources_are_included_and_only_integrationtests(self,
+                                                                                                   discover_python_files):
         project = Mock()
 
         def get_property(property):
             if property == 'dir_source_unittest_python':
                 return None
             return property
+
         project.get_property.side_effect = get_property
 
         discover_affected_files(True, False, project)
@@ -117,6 +121,7 @@ class DiscoverAffectedFilesTest(unittest.TestCase):
             if property == 'dir_source_main_python':
                 return property
             return None
+
         project.get_property.side_effect = get_property
 
         discover_affected_files(True, False, project)
@@ -125,8 +130,79 @@ class DiscoverAffectedFilesTest(unittest.TestCase):
                          [call('dir_source_main_python')])
 
 
-class ExecuteToolOnSourceFilesTest(unittest.TestCase):
+class DiscoverAffectedDirsTest(unittest.TestCase):
+    def test_should_discover_source_dirs_when_test_sources_not_included(self):
+        project = Mock()
+        project.get_property.return_value = 'source_directory'
+
+        files = discover_affected_dirs(False, False, project)
+        self.assertEqual(files, ['source_directory'])
+
+    def test_should_discover_source_dirs_when_test_sources_are_included(self):
+        project = Mock()
+
+        project.get_property.side_effect = lambda _property: _property
 
+        files = discover_affected_dirs(True, False, project)
+
+        self.assertEqual(project.get_property.call_args_list,
+                         [call('dir_source_main_python'),
+                          call('dir_source_unittest_python'),
+                          call('dir_source_unittest_python'),
+                          call('dir_source_integrationtest_python'),
+                          call('dir_source_integrationtest_python')])
+        self.assertEquals(files,
+                          ['dir_source_main_python', 'dir_source_unittest_python', 'dir_source_integrationtest_python'])
+
+    def test_should_discover_source_dirs_when_test_sources_are_included_no_unittests(self):
+        project = Mock()
+
+        project.get_property.side_effect = lambda \\
+            _property: _property if _property != 'dir_source_unittest_python' else None
+
+        files = discover_affected_dirs(True, False, project)
+
+        self.assertEqual(project.get_property.call_args_list,
+                         [call('dir_source_main_python'),
+                          call('dir_source_unittest_python'),
+                          call('dir_source_integrationtest_python'),
+                          call('dir_source_integrationtest_python')])
+        self.assertEquals(files,
+                          ['dir_source_main_python', 'dir_source_integrationtest_python'])
+
+    def test_should_discover_source_dirs_when_test_sources_are_included_no_integrationtests(self):
+        project = Mock()
+
+        project.get_property.side_effect = lambda \\
+            _property: _property if _property != 'dir_source_integrationtest_python' else None
+
+        files = discover_affected_dirs(True, False, project)
+
+        self.assertEqual(project.get_property.call_args_list,
+                         [call('dir_source_main_python'),
+                          call('dir_source_unittest_python'),
+                          call('dir_source_unittest_python'),
+                          call('dir_source_integrationtest_python')])
+        self.assertEquals(files,
+                          ['dir_source_main_python', 'dir_source_unittest_python'])
+
+    def test_should_discover_source_dirs_when_script_sources_are_included(self):
+        project = Mock()
+
+        project.get_property.side_effect = lambda \\
+            _property: _property if _property != 'dir_source_integrationtest_python' else None
+
+        files = discover_affected_dirs(False, True, project)
+
+        self.assertEqual(project.get_property.call_args_list,
+                         [call('dir_source_main_python'),
+                          call('dir_source_main_scripts'),
+                          call('dir_source_main_scripts')])
+        self.assertEquals(files,
+                          ['dir_source_main_python', 'dir_source_main_scripts'])
+
+
+class ExecuteToolOnSourceFilesTest(unittest.TestCase):
     @patch('pybuilder.plugins.python.python_plugin_helper.log_report')
     @patch('pybuilder.plugins.python.python_plugin_helper.read_file')
     @patch('pybuilder.plugins.python.python_plugin_helper.execute_command')
@@ -141,6 +217,20 @@ class ExecuteToolOnSourceFilesTest(unittest.TestCase):
 
         execute.assert_called_with(['foo --bar', 'file1', 'file2'], '/path/to/report')
 
+    @patch('pybuilder.plugins.python.python_plugin_helper.log_report')
+    @patch('pybuilder.plugins.python.python_plugin_helper.read_file')
+    @patch('pybuilder.plugins.python.python_plugin_helper.execute_command')
+    @patch('pybuilder.plugins.python.python_plugin_helper.discover_affected_dirs')
+    def test_should_execute_tool_on_source_dirs(self, affected,
+                                                execute, read, log):
+        project = Mock()
+        project.expand_path.return_value = '/path/to/report'
+        affected.return_value = ['/dir1', '/dir2']
+
+        execute_tool_on_source_files(project, 'name', 'foo --bar', include_dirs_only=True)
+
+        execute.assert_called_with(['foo --bar', '/dir1', '/dir2'], '/path/to/report')
+
     @patch('pybuilder.plugins.python.python_plugin_helper.log_report')
     @patch('pybuilder.plugins.python.python_plugin_helper.read_file')
     @patch('pybuilder.plugins.python.python_plugin_helper.execute_command')