From 2596870f21ceca207b0da9340201dc537fbe17f3 Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Tue, 22 Nov 2016 09:42:15 +0100 Subject: [PATCH 01/12] finder.find_python tests for the FoundFiles and SingleFiles classes. Adding tests for the iter_[file|package|directory|module]_paths() methods of FoundFiles and SingleFiles, with an appropriate test data directory tree. Tests the current status quo of method results. Adding myself to CONTRIBUTORS.md. --- CONTRIBUTORS.md | 1 + tests/finder/test_file_finder.py | 184 +++++++++++++++++- .../testdata/dirs_mods_packages/module.py | 1 + .../nonpackage/nonpackage_module.py | 1 + .../nonpackage/nonpackage_plain.txt | 1 + .../dirs_mods_packages/package/__init__.py | 1 + .../package/package_module.py | 1 + .../package/package_plain.txt | 1 + .../package/subpackage/__init__.py | 1 + .../package/subpackage/subpackage_module.py | 1 + .../package/subpackage/subpackage_plain.txt | 1 + .../testdata/dirs_mods_packages/plain.txt | 1 + 12 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 tests/finder/testdata/dirs_mods_packages/module.py create mode 100644 tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module.py create mode 100644 tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_plain.txt create mode 100644 tests/finder/testdata/dirs_mods_packages/package/__init__.py create mode 100644 tests/finder/testdata/dirs_mods_packages/package/package_module.py create mode 100644 tests/finder/testdata/dirs_mods_packages/package/package_plain.txt create mode 100644 tests/finder/testdata/dirs_mods_packages/package/subpackage/__init__.py create mode 100644 tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module.py create mode 100644 tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_plain.txt create mode 100644 tests/finder/testdata/dirs_mods_packages/plain.txt diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 685452fe..9615a80b 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -6,3 +6,4 @@ Contributors * Jeff Quast ([@jquast](https://github.com/jquast)) * Sam Spilsbury ([@smspillaz](https://github.com/smspillaz)) * Jose Antonio Perdiguero ([@PeRDy](https://github.com/PeRDy)) +* Holger Joukl ([@hjoukl](https://github.com/hjoukl)) diff --git a/tests/finder/test_file_finder.py b/tests/finder/test_file_finder.py index e31d99e6..22413bda 100644 --- a/tests/finder/test_file_finder.py +++ b/tests/finder/test_file_finder.py @@ -1,6 +1,6 @@ import os from unittest import TestCase -from prospector.finder import find_python +from prospector.finder import find_python, FoundFiles, SingleFiles from prospector.pathutils import is_virtualenv @@ -48,3 +48,185 @@ def test_long_path_not_a_venv(self): path.append('long_path_not_a_venv_long_path_not_a_v') path = os.path.join(*path) self.assertFalse(is_virtualenv(path)) + + +class TestPathFinderDir(TestCase): + + def _run_test(self, expected, method_under_test): + # method_under_test: unbound FoundFiles method + root = os.path.join( + os.path.dirname(__file__), 'testdata', 'dirs_mods_packages') + found = find_python( + ignores=[], paths=[root], explicit_file_mode=False) + + expected = [os.path.normpath(os.path.join(root, e)) for e in expected] + expected.sort() + actual = list(method_under_test(found)) + actual.sort() + self.assertEqual(actual, expected) + + def test_file_paths(self): + expected = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ] + self._run_test(expected, FoundFiles.iter_file_paths) + + def test_module_paths(self): + expected = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/package_module.py', + 'nonpackage/nonpackage_module.py', + 'module.py', + ] + self._run_test(expected, FoundFiles.iter_module_paths) + + def test_directory_paths(self): + expected = [ + 'package', + 'package/subpackage', + 'nonpackage', + ] + self._run_test(expected, FoundFiles.iter_directory_paths) + + def test_package_paths(self): + expected = [ + 'package', + 'package/subpackage', + ] + self._run_test(expected, FoundFiles.iter_package_paths) + + +class TestPathFinderSingleFiles(TestCase): + + def _run_test(self, paths, expected, method_under_test): + # method_under_test: unbound SingleFiles method + root = os.path.join( + os.path.dirname(__file__), 'testdata', 'dirs_mods_packages') + paths = [os.path.normpath(os.path.join(root, p)) for p in paths] + found = find_python( + ignores=[], paths=paths, explicit_file_mode=True) + expected = [os.path.normpath(os.path.join(root, e)) for e in expected] + expected.sort() + actual = list(method_under_test(found)) + actual.sort() + self.assertEqual(actual, expected) + + def test_file_paths(self): + paths = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ] + expected = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ] + self._run_test(paths, expected, SingleFiles.iter_file_paths) + + def test_module_paths(self): + paths = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ] + expected = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ] + self._run_test(paths, expected, SingleFiles.iter_module_paths) + + def test_directory_paths(self): + paths = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ] + expected = [ + 'package', + 'package/subpackage', + 'package/subpackage', + 'package/subpackage', + 'package', + 'package', + 'nonpackage', + 'nonpackage', + '', + '', + ] + self._run_test(paths, expected, SingleFiles.iter_directory_paths) + + def test_package_paths(self): + paths = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ] + expected = [ + 'package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ] + self._run_test(paths, expected, SingleFiles.iter_package_paths) + diff --git a/tests/finder/testdata/dirs_mods_packages/module.py b/tests/finder/testdata/dirs_mods_packages/module.py new file mode 100644 index 00000000..4adc9840 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/module.py @@ -0,0 +1 @@ +# Python module diff --git a/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module.py b/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module.py new file mode 100644 index 00000000..4adc9840 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module.py @@ -0,0 +1 @@ +# Python module diff --git a/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_plain.txt b/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_plain.txt new file mode 100644 index 00000000..820cbf75 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_plain.txt @@ -0,0 +1 @@ +# plain text file diff --git a/tests/finder/testdata/dirs_mods_packages/package/__init__.py b/tests/finder/testdata/dirs_mods_packages/package/__init__.py new file mode 100644 index 00000000..717d76b5 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/__init__.py @@ -0,0 +1 @@ +# Python package diff --git a/tests/finder/testdata/dirs_mods_packages/package/package_module.py b/tests/finder/testdata/dirs_mods_packages/package/package_module.py new file mode 100644 index 00000000..4adc9840 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/package_module.py @@ -0,0 +1 @@ +# Python module diff --git a/tests/finder/testdata/dirs_mods_packages/package/package_plain.txt b/tests/finder/testdata/dirs_mods_packages/package/package_plain.txt new file mode 100644 index 00000000..820cbf75 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/package_plain.txt @@ -0,0 +1 @@ +# plain text file diff --git a/tests/finder/testdata/dirs_mods_packages/package/subpackage/__init__.py b/tests/finder/testdata/dirs_mods_packages/package/subpackage/__init__.py new file mode 100644 index 00000000..717d76b5 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/subpackage/__init__.py @@ -0,0 +1 @@ +# Python package diff --git a/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module.py b/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module.py new file mode 100644 index 00000000..4adc9840 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module.py @@ -0,0 +1 @@ +# Python module diff --git a/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_plain.txt b/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_plain.txt new file mode 100644 index 00000000..820cbf75 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_plain.txt @@ -0,0 +1 @@ +# plain text file diff --git a/tests/finder/testdata/dirs_mods_packages/plain.txt b/tests/finder/testdata/dirs_mods_packages/plain.txt new file mode 100644 index 00000000..820cbf75 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/plain.txt @@ -0,0 +1 @@ +# plain text file From 872ea0b0d96bcf22c948db44f805c1afedca5478 Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Tue, 22 Nov 2016 09:52:43 +0100 Subject: [PATCH 02/12] Fix erroneous result variable / call argument order in find_python(). Fixing the order of the _find_paths() return value assignment to result variables and the parameter order for the FoundFiles initialization to the correct order files, modules, packages, directories. Note that the double error actually evened out i.e. the failure was not visible. --- prospector/finder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prospector/finder.py b/prospector/finder.py index dcd7e65b..34e8821a 100644 --- a/prospector/finder.py +++ b/prospector/finder.py @@ -198,5 +198,5 @@ def find_python(ignores, paths, explicit_file_mode, workdir=None): return SingleFiles(paths, workdir or os.getcwd()) else: assert len(paths) == 1 - files, modules, directories, packages = _find_paths(ignores, paths[0], paths[0]) - return FoundFiles(paths[0], files, modules, directories, packages, ignores) + files, modules, packages, directories = _find_paths(ignores, paths[0], paths[0]) + return FoundFiles(paths[0], files, modules, packages, directories, ignores) From d665daaff3d062911d1bcdf1a9499a34cb02f006 Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Tue, 22 Nov 2016 10:28:10 +0100 Subject: [PATCH 03/12] Comment current finder.SingleFiles API behaviour and thus test expectations. --- tests/finder/test_file_finder.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/finder/test_file_finder.py b/tests/finder/test_file_finder.py index 22413bda..9ef20c26 100644 --- a/tests/finder/test_file_finder.py +++ b/tests/finder/test_file_finder.py @@ -162,6 +162,8 @@ def test_module_paths(self): 'module.py', 'plain.txt', ] + # SingleFiles.iter_module_paths yields all file paths, not just Python + # modules expected = [ 'package/__init__.py', 'package/subpackage/__init__.py', @@ -189,6 +191,8 @@ def test_directory_paths(self): 'module.py', 'plain.txt', ] + # SingleFiles.iter_module_paths yields the directories of all file + # paths (duplicates are not filtered) expected = [ 'package', 'package/subpackage', @@ -216,6 +220,8 @@ def test_package_paths(self): 'module.py', 'plain.txt', ] + # SingleFiles.iter_module_paths yields all file paths, not just Python + # packages expected = [ 'package/__init__.py', 'package/subpackage/__init__.py', From 4425868f203bac9ae30b5b16ed01bf353a42935f Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Thu, 24 Nov 2016 14:50:05 +0100 Subject: [PATCH 04/12] Add pep8/pycodestyle tool test for testing the StyleGuide paths config. Test expections are set so this tests ok with the current behaviour, for now. This will need to change as pep8 tool ignores python modules in non-package directories when invoked with a single dir PATH command line arg. --- tests/tools/__init__.py | 0 tests/tools/test_pep8.py | 195 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 tests/tools/__init__.py create mode 100644 tests/tools/test_pep8.py diff --git a/tests/tools/__init__.py b/tests/tools/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/tools/test_pep8.py b/tests/tools/test_pep8.py new file mode 100644 index 00000000..53d91511 --- /dev/null +++ b/tests/tools/test_pep8.py @@ -0,0 +1,195 @@ +import os +import tempfile +from unittest import TestCase +from prospector.finder import find_python +from prospector.tools.pep8 import Pep8Tool + + +# A very basic stub to avoid creating a real ProspectorConfig, which wants +# command line (sys.argv) args and whatnot. +# Needed for handing to the Pep8Tool.configure(...) method, should expose the +# ProspectorConfig interface accessed in this method. +class StubProspectorConfig: + # not actually used for anything here, default to s.th. "big" + _max_line_length = 300 + + def __init__(self, external_config=None): + # external_config: optional file path to pep8/pycodestyle config file + self.external_config = external_config + + @property + def max_line_length(self): + return self._max_line_length + + def use_external_config(self, _): + return True + + def external_config_location(self, tool_name): + return self.external_config + + def get_disabled_messages(self, tool_name): + return [] + + def tool_options(self, tool_name): + return {} + + +# helper function +def rooted(root, paths): + """Return list of paths with root dir prepended to each path.""" + return [os.path.normpath(os.path.join(root, p)) for p in paths] + + +class TestProspectorStyleGuidePathConfig(TestCase): + # Test how prospector sets up the pep8/pycodestyle tool with regard to the + # paths to check. + # We need to create the ProspectorStyleGuide through a Pep8Tool instance + # and use finder.find_python to create the input for Pep8Tool.configure() + def _setup_styleguide(self, paths, external_config=None): + # A helper to create the StyleGuide object. + # root: the root directory for the relative paths given in the paths + # and expected parameters + pep8tool = Pep8Tool() + config = StubProspectorConfig(external_config=external_config) + found_python = find_python( + ignores=[], paths=paths, + explicit_file_mode=True if len(paths)>1 else False) + + configured_by, _ = pep8tool.configure(config, found_python) + if external_config is not None: + expected_configuration_message = ( + "Configuration found at %s" % external_config) + self.assertEqual(configured_by, expected_configuration_message) + prospector_styleguide = pep8tool.checker + return prospector_styleguide + + def test_paths_dir_arg(self): + # test corresponding to a single command line PATH dir arg + root = os.path.join( + os.path.dirname(__file__), '..', 'finder', 'testdata', + 'dirs_mods_packages') + # use the root directory itself as find_python input arg: + paths = rooted(root, ['']) + # all Python modules in the directory tree are expected (but nothing + # else) + expected = rooted( + root, + ['package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + # 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + # 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + # 'nonpackage/nonpackage_plain.txt', + 'module.py', + # 'plain.txt', + ]) + expected = rooted( + root, + ['package', + 'package/subpackage', + ]) + prospector_styleguide = self._setup_styleguide(paths) + self.assertEqual(sorted(prospector_styleguide.paths), sorted(expected)) + + def test_paths_dir_arg_pep8_exclude_cfg(self): + # test corresponding to a single command line PATH dir arg, with + # external pep8/pycodestyle config file + root = os.path.join( + os.path.dirname(__file__), '..', 'finder', 'testdata', + 'dirs_mods_packages') + # use the root directory itself as find_python input arg: + paths = rooted(root, ['']) + + # set pep8/pycodestyle exclude config option + external_config_contents = """ +[pep8] +exclude = subpackage + """ + expected = rooted( + root, + ['package', + ]) + with tempfile.NamedTemporaryFile(delete=False) as f: + f.write(external_config_contents) + f.flush() + prospector_styleguide = self._setup_styleguide( + paths, external_config=f.name) + # exclusion of paths happens when pydodestyle checking is actually + # run, simulate this by using the .excluded() method + paths_after_exclusion = [ + p for p in prospector_styleguide.paths + if not prospector_styleguide.excluded(p)] + self.assertEqual( + sorted(paths_after_exclusion), sorted(expected)) + + def test_paths_py_modules_arg(self): + # test corresponding to multiple Python module files PATHs args + root = os.path.join( + os.path.dirname(__file__), '..', 'finder', 'testdata', + 'dirs_mods_packages') + paths = rooted( + root, + ['package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/package_module.py', + 'nonpackage/nonpackage_module.py', + 'module.py', + ]) + + # all Python modules given as paths args are expected (but nothing + # else) + expected = rooted( + root, + ['package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/package_module.py', + 'nonpackage/nonpackage_module.py', + 'module.py', + ]) + prospector_styleguide = self._setup_styleguide(paths) + self.assertEqual(sorted(prospector_styleguide.paths), sorted(expected)) + + def test_paths_files_arg(self): + # test corresponding to multiple files PATHs args (Python modules and + # other files) + root = os.path.join( + os.path.dirname(__file__), '..', 'finder', 'testdata', + 'dirs_mods_packages') + paths = rooted( + root, + ['package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ]) + + # all files given as path args are expected (but nothing + # else) + # TODO: Is this how it should work? Should the tools cater for + # TODO: filtering non-Python files? + expected = rooted( + root, + ['package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_plain.txt', + 'package/package_module.py', + 'package/package_plain.txt', + 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_plain.txt', + 'module.py', + 'plain.txt', + ]) + prospector_styleguide = self._setup_styleguide(paths) + self.assertEqual(sorted(prospector_styleguide.paths), sorted(expected)) + From 51e39f9612654f76602ebf7fea16d9b3777ccebb Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Fri, 25 Nov 2016 14:34:23 +0100 Subject: [PATCH 05/12] Add more submodules to file finder test data. --- tests/finder/test_file_finder.py | 117 ++++++++++++------ .../{module.py => module1.py} | 0 .../nonpackage_module.py => module2.py} | 0 .../nonpackage_module1.py} | 0 .../nonpackage_module2.py} | 0 .../package/package_module1.py | 1 + .../package/package_module2.py | 1 + .../package/subpackage/subpackage_module1.py | 1 + .../package/subpackage/subpackage_module2.py | 1 + 9 files changed, 83 insertions(+), 38 deletions(-) rename tests/finder/testdata/dirs_mods_packages/{module.py => module1.py} (100%) rename tests/finder/testdata/dirs_mods_packages/{nonpackage/nonpackage_module.py => module2.py} (100%) rename tests/finder/testdata/dirs_mods_packages/{package/package_module.py => nonpackage/nonpackage_module1.py} (100%) rename tests/finder/testdata/dirs_mods_packages/{package/subpackage/subpackage_module.py => nonpackage/nonpackage_module2.py} (100%) create mode 100644 tests/finder/testdata/dirs_mods_packages/package/package_module1.py create mode 100644 tests/finder/testdata/dirs_mods_packages/package/package_module2.py create mode 100644 tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module1.py create mode 100644 tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module2.py diff --git a/tests/finder/test_file_finder.py b/tests/finder/test_file_finder.py index 9ef20c26..24949486 100644 --- a/tests/finder/test_file_finder.py +++ b/tests/finder/test_file_finder.py @@ -69,13 +69,17 @@ def test_file_paths(self): expected = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', ] self._run_test(expected, FoundFiles.iter_file_paths) @@ -84,10 +88,14 @@ def test_module_paths(self): expected = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', - 'package/package_module.py', - 'nonpackage/nonpackage_module.py', - 'module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', ] self._run_test(expected, FoundFiles.iter_module_paths) @@ -126,25 +134,33 @@ def test_file_paths(self): paths = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', ] expected = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', ] self._run_test(paths, expected, SingleFiles.iter_file_paths) @@ -153,13 +169,17 @@ def test_module_paths(self): paths = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', ] # SingleFiles.iter_module_paths yields all file paths, not just Python @@ -167,13 +187,17 @@ def test_module_paths(self): expected = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', ] self._run_test(paths, expected, SingleFiles.iter_module_paths) @@ -182,26 +206,34 @@ def test_directory_paths(self): paths = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', ] - # SingleFiles.iter_module_paths yields the directories of all file + # SingleFiles.iter_directory_paths yields the directories of all file # paths (duplicates are not filtered) expected = [ 'package', 'package/subpackage', 'package/subpackage', 'package/subpackage', + 'package/subpackage', + 'package', 'package', 'package', 'nonpackage', 'nonpackage', + 'nonpackage', + '', '', '', ] @@ -211,28 +243,37 @@ def test_package_paths(self): paths = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', ] - # SingleFiles.iter_module_paths yields all file paths, not just Python + # SingleFiles.iter_package_paths yields all file paths, not just Python # packages expected = [ 'package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', ] self._run_test(paths, expected, SingleFiles.iter_package_paths) + diff --git a/tests/finder/testdata/dirs_mods_packages/module.py b/tests/finder/testdata/dirs_mods_packages/module1.py similarity index 100% rename from tests/finder/testdata/dirs_mods_packages/module.py rename to tests/finder/testdata/dirs_mods_packages/module1.py diff --git a/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module.py b/tests/finder/testdata/dirs_mods_packages/module2.py similarity index 100% rename from tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module.py rename to tests/finder/testdata/dirs_mods_packages/module2.py diff --git a/tests/finder/testdata/dirs_mods_packages/package/package_module.py b/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module1.py similarity index 100% rename from tests/finder/testdata/dirs_mods_packages/package/package_module.py rename to tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module1.py diff --git a/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module.py b/tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module2.py similarity index 100% rename from tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module.py rename to tests/finder/testdata/dirs_mods_packages/nonpackage/nonpackage_module2.py diff --git a/tests/finder/testdata/dirs_mods_packages/package/package_module1.py b/tests/finder/testdata/dirs_mods_packages/package/package_module1.py new file mode 100644 index 00000000..4adc9840 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/package_module1.py @@ -0,0 +1 @@ +# Python module diff --git a/tests/finder/testdata/dirs_mods_packages/package/package_module2.py b/tests/finder/testdata/dirs_mods_packages/package/package_module2.py new file mode 100644 index 00000000..4adc9840 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/package_module2.py @@ -0,0 +1 @@ +# Python module diff --git a/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module1.py b/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module1.py new file mode 100644 index 00000000..4adc9840 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module1.py @@ -0,0 +1 @@ +# Python module diff --git a/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module2.py b/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module2.py new file mode 100644 index 00000000..4adc9840 --- /dev/null +++ b/tests/finder/testdata/dirs_mods_packages/package/subpackage/subpackage_module2.py @@ -0,0 +1 @@ +# Python module From d83f4dae4ecba158ac3104cdd17851a2094e010f Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Fri, 25 Nov 2016 15:06:44 +0100 Subject: [PATCH 06/12] Revamped pep8/pycodestyle test to expose correct module-based expectations. Now set up to fail for the current incorrect behaviour of silently ignoring modules in non-package directories (#179). --- tests/tools/test_pep8.py | 227 ++++++++++++++++++++++++++------------- 1 file changed, 153 insertions(+), 74 deletions(-) diff --git a/tests/tools/test_pep8.py b/tests/tools/test_pep8.py index 53d91511..a69f1a0b 100644 --- a/tests/tools/test_pep8.py +++ b/tests/tools/test_pep8.py @@ -1,22 +1,34 @@ import os import tempfile +import re from unittest import TestCase from prospector.finder import find_python from prospector.tools.pep8 import Pep8Tool +from prospector.config import ProspectorConfig -# A very basic stub to avoid creating a real ProspectorConfig, which wants -# command line (sys.argv) args and whatnot. -# Needed for handing to the Pep8Tool.configure(...) method, should expose the -# ProspectorConfig interface accessed in this method. -class StubProspectorConfig: - # not actually used for anything here, default to s.th. "big" - _max_line_length = 300 +# A very basic stub to avoid creating a real ProspectorConfig, which isn't +# esasily tweakable for tests "from the outside". +# Needed for handing into the Pep8Tool.configure(...) method, should expose at +# least the ProspectorConfig interface accessed by this method. +class StubProspectorConfig(object): - def __init__(self, external_config=None): + def __init__(self, external_config=None, ignores=None, + max_line_length=300): # external_config: optional file path to pep8/pycodestyle config file + # ignores: list of regular expressions strings of (relative) paths to + # ignore + # max_line_length: value for the max_line_length property, not + # currently actually used for anything here, default to s.th. + # "big" self.external_config = external_config + if ignores is None: + self.ignores = [] + else: + self.ignores = [re.compile(patt) for patt in ignores] + self._max_line_length = max_line_length + # stubbed ProspectorConfig API @property def max_line_length(self): return self._max_line_length @@ -34,10 +46,27 @@ def tool_options(self, tool_name): return {} +def dummy_checker(): + # create a fake checker that gathers the filenames into + # DummyChecker.filenames class attribute + class DummyChecker(object): + filenames = [] + + def __init__(self, filename, *args, **kwargs): + self.filenames.append(filename) + + def check_all(self, *args, **kwargs): + pass + + return DummyChecker + + # helper function -def rooted(root, paths): +def prepend_root(paths, root=None): """Return list of paths with root dir prepended to each path.""" - return [os.path.normpath(os.path.join(root, p)) for p in paths] + if root: + return [os.path.normpath(os.path.join(root, p)) for p in paths] + return paths class TestProspectorStyleGuidePathConfig(TestCase): @@ -45,14 +74,16 @@ class TestProspectorStyleGuidePathConfig(TestCase): # paths to check. # We need to create the ProspectorStyleGuide through a Pep8Tool instance # and use finder.find_python to create the input for Pep8Tool.configure() - def _setup_styleguide(self, paths, external_config=None): + + def _setup_styleguide(self, paths, ignores=None, external_config=None): # A helper to create the StyleGuide object. # root: the root directory for the relative paths given in the paths - # and expected parameters + # and expected parameters pep8tool = Pep8Tool() - config = StubProspectorConfig(external_config=external_config) + config = StubProspectorConfig( + external_config=external_config, ignores=ignores) found_python = find_python( - ignores=[], paths=paths, + ignores=config.ignores, paths=paths, explicit_file_mode=True if len(paths)>1 else False) configured_by, _ = pep8tool.configure(config, found_python) @@ -61,6 +92,9 @@ def _setup_styleguide(self, paths, external_config=None): "Configuration found at %s" % external_config) self.assertEqual(configured_by, expected_configuration_message) prospector_styleguide = pep8tool.checker + # inject file paths-gathering dummy checker and invoke it + prospector_styleguide.checker_class = dummy_checker() + prospector_styleguide.check_files() return prospector_styleguide def test_paths_dir_arg(self): @@ -69,29 +103,53 @@ def test_paths_dir_arg(self): os.path.dirname(__file__), '..', 'finder', 'testdata', 'dirs_mods_packages') # use the root directory itself as find_python input arg: - paths = rooted(root, ['']) + paths = prepend_root([''], root) # all Python modules in the directory tree are expected (but nothing # else) - expected = rooted( - root, + expected = prepend_root( ['package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', - # 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', - # 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', - # 'nonpackage/nonpackage_plain.txt', - 'module.py', - # 'plain.txt', - ]) - expected = rooted( - root, - ['package', - 'package/subpackage', - ]) + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', + ], root) + prospector_styleguide = self._setup_styleguide(paths) - self.assertEqual(sorted(prospector_styleguide.paths), sorted(expected)) + self.assertEqual( + sorted(prospector_styleguide.checker_class.filenames), + sorted(expected)) + + def test_paths_dir_arg_prospector_ignore_patterns_cfg(self): + # test corresponding to a single command line PATH dir arg, with + # with prospectorer ignore-patterns + root = os.path.join( + os.path.dirname(__file__), '..', 'finder', 'testdata', + 'dirs_mods_packages') + # use the root directory itself as find_python input arg: + paths = prepend_root([''], root) + + ignores = ['(^|/)subpackage(/|$)', 'nonpackage', '^package/module1.py$'] + expected = prepend_root( + ['package/__init__.py', + #'package/subpackage/__init__.py', + #'package/subpackage/subpackage_module1.py', + #'package/subpackage/subpackage_module2.py', + #'package/package_module1.py', + 'package/package_module2.py', + #'nonpackage/nonpackage_module1.py', + #'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', + ], root) + prospector_styleguide = self._setup_styleguide(paths, ignores=ignores) + self.assertEqual( + sorted(prospector_styleguide.checker_class.filenames), + sorted(expected)) def test_paths_dir_arg_pep8_exclude_cfg(self): # test corresponding to a single command line PATH dir arg, with @@ -100,58 +158,70 @@ def test_paths_dir_arg_pep8_exclude_cfg(self): os.path.dirname(__file__), '..', 'finder', 'testdata', 'dirs_mods_packages') # use the root directory itself as find_python input arg: - paths = rooted(root, ['']) + paths = prepend_root([''], root) # set pep8/pycodestyle exclude config option external_config_contents = """ [pep8] exclude = subpackage """ - expected = rooted( - root, - ['package', - ]) - with tempfile.NamedTemporaryFile(delete=False) as f: + expected = prepend_root( + ['package/__init__.py', + #'package/subpackage/__init__.py', + #'package/subpackage/subpackage_module1.py', + #'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', + ], root) + with tempfile.NamedTemporaryFile() as f: f.write(external_config_contents) f.flush() prospector_styleguide = self._setup_styleguide( paths, external_config=f.name) - # exclusion of paths happens when pydodestyle checking is actually - # run, simulate this by using the .excluded() method - paths_after_exclusion = [ - p for p in prospector_styleguide.paths - if not prospector_styleguide.excluded(p)] self.assertEqual( - sorted(paths_after_exclusion), sorted(expected)) + sorted(prospector_styleguide.checker_class.filenames), + sorted(expected)) def test_paths_py_modules_arg(self): # test corresponding to multiple Python module files PATHs args root = os.path.join( os.path.dirname(__file__), '..', 'finder', 'testdata', 'dirs_mods_packages') - paths = rooted( - root, + paths = prepend_root( ['package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', - 'package/package_module.py', - 'nonpackage/nonpackage_module.py', - 'module.py', - ]) + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', + ], root) # all Python modules given as paths args are expected (but nothing # else) - expected = rooted( - root, + expected = prepend_root( ['package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', - 'package/package_module.py', - 'nonpackage/nonpackage_module.py', - 'module.py', - ]) + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', + ], root) prospector_styleguide = self._setup_styleguide(paths) - self.assertEqual(sorted(prospector_styleguide.paths), sorted(expected)) + self.assertEqual( + sorted(prospector_styleguide.checker_class.filenames), + sorted(expected)) def test_paths_files_arg(self): # test corresponding to multiple files PATHs args (Python modules and @@ -159,37 +229,46 @@ def test_paths_files_arg(self): root = os.path.join( os.path.dirname(__file__), '..', 'finder', 'testdata', 'dirs_mods_packages') - paths = rooted( - root, + paths = prepend_root( ['package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', - ]) + ], root) # all files given as path args are expected (but nothing # else) # TODO: Is this how it should work? Should the tools cater for # TODO: filtering non-Python files? - expected = rooted( - root, + expected = prepend_root( ['package/__init__.py', 'package/subpackage/__init__.py', - 'package/subpackage/subpackage_module.py', 'package/subpackage/subpackage_plain.txt', - 'package/package_module.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', 'package/package_plain.txt', - 'nonpackage/nonpackage_module.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', 'nonpackage/nonpackage_plain.txt', - 'module.py', + 'module1.py', + 'module2.py', 'plain.txt', - ]) + ], root) + prospector_styleguide = self._setup_styleguide(paths) - self.assertEqual(sorted(prospector_styleguide.paths), sorted(expected)) + self.assertEqual( + sorted(prospector_styleguide.checker_class.filenames), + sorted(expected)) From 2181901d8b080c71910e19c4d59e8615d56bab25 Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Fri, 25 Nov 2016 17:26:45 +0100 Subject: [PATCH 07/12] Better comments. --- tests/tools/test_pep8.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_pep8.py b/tests/tools/test_pep8.py index a69f1a0b..235d53e6 100644 --- a/tests/tools/test_pep8.py +++ b/tests/tools/test_pep8.py @@ -153,7 +153,8 @@ def test_paths_dir_arg_prospector_ignore_patterns_cfg(self): def test_paths_dir_arg_pep8_exclude_cfg(self): # test corresponding to a single command line PATH dir arg, with - # external pep8/pycodestyle config file + # external pep8/pycodestyle config file to exclude the 'subpackage' + # folder from checking root = os.path.join( os.path.dirname(__file__), '..', 'finder', 'testdata', 'dirs_mods_packages') From 445b56124c07f480a6b0e3360e6a2915a09c1607 Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Mon, 28 Nov 2016 16:35:03 +0100 Subject: [PATCH 08/12] Fix for pycodestyle not picking up modules in non-package dirs (#179). Instead of iter_package_paths() use iter_module_paths() for explicit files mode and rootpath for "single directory arg" mode (leaving file lookup to pep8/pycodestyle). --- prospector/tools/pep8/__init__.py | 19 +++++++++++++++--- tests/tools/test_pep8.py | 32 +++++++++++++++---------------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/prospector/tools/pep8/__init__.py b/prospector/tools/pep8/__init__.py index 8781a65f..0d22050c 100644 --- a/prospector/tools/pep8/__init__.py +++ b/prospector/tools/pep8/__init__.py @@ -15,6 +15,7 @@ from prospector.message import Location, Message from prospector.tools.base import ToolBase +from prospector.finder import FoundFiles __all__ = ( @@ -82,10 +83,15 @@ def excluded(self, filename, parent=None): # If the file survived pep8's exclusion rules, check it against # prospector's patterns. - if os.path.isdir(os.path.join(self._files.rootpath, filename)): + # os.path.join() caters for parent or filename being absolute + fullpath = os.path.join( + self._files.rootpath, + '' if parent is None else parent, + filename) + + if os.path.isdir(fullpath): return False - fullpath = os.path.join(self._files.rootpath, parent, filename) if parent else filename return fullpath not in self._module_paths @@ -121,8 +127,15 @@ def configure(self, prospector_config, found_files): break # Instantiate our custom pep8 checker. + # TODO: Can this be implemented cleaner? Ultimately, pycodestyle needs + # TODO: to be driven differently for explicit paths vs root dir path + # TODO: to make both prospector and pycodestyle exclusion rules work. + if isinstance(found_files, FoundFiles): + checkpaths = [found_files.rootpath] + else: + checkpaths = list(found_files.iter_module_paths()) self.checker = ProspectorStyleGuide( - paths=list(found_files.iter_package_paths()), + paths=checkpaths, found_files=found_files, config_file=external_config ) diff --git a/tests/tools/test_pep8.py b/tests/tools/test_pep8.py index 235d53e6..86a1d901 100644 --- a/tests/tools/test_pep8.py +++ b/tests/tools/test_pep8.py @@ -85,7 +85,6 @@ def _setup_styleguide(self, paths, ignores=None, external_config=None): found_python = find_python( ignores=config.ignores, paths=paths, explicit_file_mode=True if len(paths)>1 else False) - configured_by, _ = pep8tool.configure(config, found_python) if external_config is not None: expected_configuration_message = ( @@ -120,9 +119,8 @@ def test_paths_dir_arg(self): ], root) prospector_styleguide = self._setup_styleguide(paths) - self.assertEqual( - sorted(prospector_styleguide.checker_class.filenames), - sorted(expected)) + actual = prospector_styleguide.checker_class.filenames + self.assertEqual(sorted(actual), sorted(expected)) def test_paths_dir_arg_prospector_ignore_patterns_cfg(self): # test corresponding to a single command line PATH dir arg, with @@ -133,7 +131,11 @@ def test_paths_dir_arg_prospector_ignore_patterns_cfg(self): # use the root directory itself as find_python input arg: paths = prepend_root([''], root) - ignores = ['(^|/)subpackage(/|$)', 'nonpackage', '^package/module1.py$'] + ignores = [ + '(^|/)subpackage(/|$)', + 'nonpackage', + '^package/package_module1.py$' + ] expected = prepend_root( ['package/__init__.py', #'package/subpackage/__init__.py', @@ -147,9 +149,8 @@ def test_paths_dir_arg_prospector_ignore_patterns_cfg(self): 'module2.py', ], root) prospector_styleguide = self._setup_styleguide(paths, ignores=ignores) - self.assertEqual( - sorted(prospector_styleguide.checker_class.filenames), - sorted(expected)) + actual = prospector_styleguide.checker_class.filenames + self.assertEqual(sorted(actual), sorted(expected)) def test_paths_dir_arg_pep8_exclude_cfg(self): # test corresponding to a single command line PATH dir arg, with @@ -183,9 +184,8 @@ def test_paths_dir_arg_pep8_exclude_cfg(self): f.flush() prospector_styleguide = self._setup_styleguide( paths, external_config=f.name) - self.assertEqual( - sorted(prospector_styleguide.checker_class.filenames), - sorted(expected)) + actual = prospector_styleguide.checker_class.filenames + self.assertEqual(sorted(actual), sorted(expected)) def test_paths_py_modules_arg(self): # test corresponding to multiple Python module files PATHs args @@ -220,9 +220,8 @@ def test_paths_py_modules_arg(self): 'module2.py', ], root) prospector_styleguide = self._setup_styleguide(paths) - self.assertEqual( - sorted(prospector_styleguide.checker_class.filenames), - sorted(expected)) + actual = prospector_styleguide.checker_class.filenames + self.assertEqual(sorted(actual), sorted(expected)) def test_paths_files_arg(self): # test corresponding to multiple files PATHs args (Python modules and @@ -269,7 +268,6 @@ def test_paths_files_arg(self): ], root) prospector_styleguide = self._setup_styleguide(paths) - self.assertEqual( - sorted(prospector_styleguide.checker_class.filenames), - sorted(expected)) + actual = prospector_styleguide.checker_class.filenames + self.assertEqual(sorted(actual), sorted(expected)) From 45387f966521cb008cafc319e088dde72305b791 Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Wed, 7 Dec 2016 13:48:25 +0100 Subject: [PATCH 09/12] Clarified comment. --- tests/tools/test_pep8.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tools/test_pep8.py b/tests/tools/test_pep8.py index 86a1d901..2609d5cc 100644 --- a/tests/tools/test_pep8.py +++ b/tests/tools/test_pep8.py @@ -10,7 +10,7 @@ # A very basic stub to avoid creating a real ProspectorConfig, which isn't # esasily tweakable for tests "from the outside". # Needed for handing into the Pep8Tool.configure(...) method, should expose at -# least the ProspectorConfig interface accessed by this method. +# least the ProspectorConfig interface accessed by said method. class StubProspectorConfig(object): def __init__(self, external_config=None, ignores=None, From 9f7e52371f99313421f490276a1b778e84a2275d Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Wed, 7 Dec 2016 14:04:56 +0100 Subject: [PATCH 10/12] Fix Python 3 compatibility issue with tempfile.write. --- tests/tools/test_pep8.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_pep8.py b/tests/tools/test_pep8.py index 2609d5cc..425bc614 100644 --- a/tests/tools/test_pep8.py +++ b/tests/tools/test_pep8.py @@ -163,7 +163,9 @@ def test_paths_dir_arg_pep8_exclude_cfg(self): paths = prepend_root([''], root) # set pep8/pycodestyle exclude config option - external_config_contents = """ + # this is ascii-only content, simply use "b'" byte string prefix for + # Python 2/3 compat + external_config_contents = b""" [pep8] exclude = subpackage """ From 0836e8fa4ce1263c14c84b34f2a90b049537dae1 Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Wed, 4 Jan 2017 15:14:29 +0100 Subject: [PATCH 11/12] Fix pep8/pycodestyle files-to-check detection for relative PATH arg(s). --- prospector/tools/pep8/__init__.py | 10 ++-- tests/tools/test_pep8.py | 85 ++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/prospector/tools/pep8/__init__.py b/prospector/tools/pep8/__init__.py index 0d22050c..33bda70d 100644 --- a/prospector/tools/pep8/__init__.py +++ b/prospector/tools/pep8/__init__.py @@ -83,11 +83,9 @@ def excluded(self, filename, parent=None): # If the file survived pep8's exclusion rules, check it against # prospector's patterns. - # os.path.join() caters for parent or filename being absolute - fullpath = os.path.join( - self._files.rootpath, - '' if parent is None else parent, - filename) + # self._module_paths contains absolute paths(!) + fullpath = os.path.abspath( + os.path.join('' if parent is None else parent, filename)) if os.path.isdir(fullpath): return False @@ -131,7 +129,7 @@ def configure(self, prospector_config, found_files): # TODO: to be driven differently for explicit paths vs root dir path # TODO: to make both prospector and pycodestyle exclusion rules work. if isinstance(found_files, FoundFiles): - checkpaths = [found_files.rootpath] + checkpaths = [os.path.abspath(found_files.rootpath)] else: checkpaths = list(found_files.iter_module_paths()) self.checker = ProspectorStyleGuide( diff --git a/tests/tools/test_pep8.py b/tests/tools/test_pep8.py index 425bc614..72b8d1c7 100644 --- a/tests/tools/test_pep8.py +++ b/tests/tools/test_pep8.py @@ -1,6 +1,7 @@ import os import tempfile import re +import contextlib from unittest import TestCase from prospector.finder import find_python from prospector.tools.pep8 import Pep8Tool @@ -61,7 +62,7 @@ def check_all(self, *args, **kwargs): return DummyChecker -# helper function +# helper functions def prepend_root(paths, root=None): """Return list of paths with root dir prepended to each path.""" if root: @@ -69,6 +70,17 @@ def prepend_root(paths, root=None): return paths +@contextlib.contextmanager +def chdir(path): + """Context manager for changing the current working directory to the + specified path and back when leaving the context. + """ + wd = os.getcwd() + os.chdir(path) + yield os.getcwd() + os.chdir(wd) + + class TestProspectorStyleGuidePathConfig(TestCase): # Test how prospector sets up the pep8/pycodestyle tool with regard to the # paths to check. @@ -122,6 +134,37 @@ def test_paths_dir_arg(self): actual = prospector_styleguide.checker_class.filenames self.assertEqual(sorted(actual), sorted(expected)) + def test_paths_relative_dir_arg(self): + # test corresponding to a single command line *relative* PATH dir arg + root = os.path.join( + os.path.dirname(__file__), '..', 'finder', 'testdata', + 'dirs_mods_packages') + parent, rootdir = os.path.split(root) + with chdir(parent): + # use the root directory itself as find_python input arg: + paths = prepend_root([''], rootdir) + # all Python modules in the directory tree are expected (but + # nothing else) + # prospector hands absolute file paths to the pycodestyle/pep8 + # backend, so expect full absolute paths + expected = prepend_root( + ['package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', + ], root) + + prospector_styleguide = self._setup_styleguide(paths) + actual = prospector_styleguide.checker_class.filenames + self.assertEqual(sorted(actual), sorted(expected)) + + def test_paths_dir_arg_prospector_ignore_patterns_cfg(self): # test corresponding to a single command line PATH dir arg, with # with prospectorer ignore-patterns @@ -225,6 +268,46 @@ def test_paths_py_modules_arg(self): actual = prospector_styleguide.checker_class.filenames self.assertEqual(sorted(actual), sorted(expected)) + def test_paths_relative_py_modules_arg(self): + # test corresponding to multiple Python module files PATHs args + root = os.path.join( + os.path.dirname(__file__), '..', 'finder', 'testdata', + 'dirs_mods_packages') + parent, rootdir = os.path.split(root) + with chdir(parent): + paths = prepend_root( + ['package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', + ], rootdir) + + # all Python modules given as paths args are expected (but nothing + # else) + # prospector hands absolute file paths to the pycodestyle/pep8 + # backend, so expect full absolute paths + expected = prepend_root( + ['package/__init__.py', + 'package/subpackage/__init__.py', + 'package/subpackage/subpackage_module1.py', + 'package/subpackage/subpackage_module2.py', + 'package/package_module1.py', + 'package/package_module2.py', + 'nonpackage/nonpackage_module1.py', + 'nonpackage/nonpackage_module2.py', + 'module1.py', + 'module2.py', + ], root) + prospector_styleguide = self._setup_styleguide(paths) + actual = prospector_styleguide.checker_class.filenames + self.assertEqual(sorted(actual), sorted(expected)) + def test_paths_files_arg(self): # test corresponding to multiple files PATHs args (Python modules and # other files) From a666605d3c140d27b703c620531b395ee026f59a Mon Sep 17 00:00:00 2001 From: Holger Joukl Date: Wed, 4 Jan 2017 16:19:16 +0100 Subject: [PATCH 12/12] Remove superfluous os.path.abspath() in excluded() method. This is not necessary here as os.path.abspath() is now ensured in the tool configuration, i.e. by _iter_module_paths() for multiple file PATH args and explicitly for a single dir PATH arg. --- prospector/tools/pep8/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/prospector/tools/pep8/__init__.py b/prospector/tools/pep8/__init__.py index 33bda70d..bfe03e30 100644 --- a/prospector/tools/pep8/__init__.py +++ b/prospector/tools/pep8/__init__.py @@ -83,9 +83,7 @@ def excluded(self, filename, parent=None): # If the file survived pep8's exclusion rules, check it against # prospector's patterns. - # self._module_paths contains absolute paths(!) - fullpath = os.path.abspath( - os.path.join('' if parent is None else parent, filename)) + fullpath = os.path.join('' if parent is None else parent, filename) if os.path.isdir(fullpath): return False