[#106] Finally fixing pyroma path ignoring issue, where it has to ove…

…rride the default ignored files; also added a bunch more testing for file ignoring

CHANGELOG.rst

@@ -16,7 +16,6 @@ However it may cause slightly different orders or reduce these duplicate warning
 The behaviour of prospector should be unchanged, apart from some bugfixes related to the old file discovery mechanism.
 
 Related bugs and PRs:
-* `#494 <https://github.com/PyCQA/prospector/issues/494>`_
 * `#480 <https://github.com/PyCQA/prospector/issues/480>`_
 * `#417 <https://github.com/PyCQA/prospector/issues/417>`_
 * `#199 <https://github.com/PyCQA/prospector/issues/199>`_

prospector/config/__init__.py

@@ -24,7 +24,6 @@ class ProspectorConfig:
 
     def __init__(self, workdir: Path = None):
         self.config, self.arguments = self._configure_prospector()
-
         self.paths = self._get_work_path(self.config, self.arguments)
         self.explicit_file_mode = all(p.is_file for p in self.paths)
         self.workdir = workdir or Path.cwd()
@@ -36,6 +35,15 @@ def __init__(self, workdir: Path = None):
         self.configured_by: Dict[str, str] = {}
         self.messages: List[Message] = []
 
+    def make_exclusion_filter(self):
+        def _filter(path: Path):
+            for ignore in self.ignores:
+                path = path.absolute().relative_to(self.workdir)
+                if ignore.match(str(path)):
+                    return True
+            return False
+        return _filter
+
     def get_tools(self, found_files):
         self.configured_by = {}
         runners = []
@@ -98,7 +106,7 @@ def _get_work_path(self, config, arguments) -> List[Path]:
             paths = [Path.cwd()]
         return [p.resolve() for p in paths]
 
-    def _get_profile(self, path, config):
+    def _get_profile(self, workdir: Path, config):
         # Use the specified profiles
         profile_provided = False
         if len(config.profiles) > 0:
@@ -110,7 +118,7 @@ def _get_profile(self, path, config):
         profile_name = None
         if not profile_provided:
             for possible_profile in AUTO_LOADED_PROFILES:
-                prospector_yaml = os.path.join(path, possible_profile)
+                prospector_yaml = os.path.join(workdir, possible_profile)
                 if os.path.exists(prospector_yaml) and os.path.isfile(prospector_yaml):
                     profile_provided = True
                     profile_name = possible_profile
@@ -154,11 +162,11 @@ def _get_profile(self, path, config):
         #   * prospector provided profiles
         profile_path = [Path(path).absolute() for path in config.profile_path]
 
-        prospector_dir = os.path.join(path, ".prospector")
+        prospector_dir = os.path.join(workdir, ".prospector")
         if os.path.exists(prospector_dir) and os.path.isdir(prospector_dir):
             profile_path.append(prospector_dir)
 
-        profile_path.append(path)
+        profile_path.append(workdir)
         profile_path.append(BUILTIN_PROFILE_PATH)
 
         try:

prospector/finder.py

@@ -28,7 +28,6 @@ def __init__(self, *provided_paths: Path, exclusion_filters: Iterable[Callable]
         self._provided_files = []
         self._provided_dirs = []
         self._exclusion_filters = list(exclusion_filters or [])
-
         for path in provided_paths:
             if not path.exists():
                 raise FileNotFoundError(path)
@@ -119,6 +118,7 @@ def directories(self) -> List[Path]:
         """
         dirs = set()
         for directory in self._provided_dirs:
+            dirs.add(directory)
             try:
                 for obj in self._walk(directory):
                     if obj.is_dir():

prospector/profiles/profiles/no_test_warnings.yaml

@@ -3,6 +3,7 @@ allow-shorthand: false
 ignore-patterns:
   - ^tests?/?
   - /tests?(/|$)
+  - .*/tests(/|$)
   - (^|/)test_[_a-zA-Z0-9]+.py$
   - (^|/)[_a-zA-Z0-9]+_tests?.py$
   - (^|/)tests?.py

prospector/run.py

@@ -46,16 +46,7 @@ def execute(self):
         summary.update(self.config.get_summary_information())
 
         paths = [Path(p) for p in self.config.paths]
-
-        def _ignores(path: Path):
-            for ignore in self.config.ignores:
-                if path != self.config.workdir:
-                    path = path.absolute().relative_to(self.config.workdir)
-                if ignore.match(str(path)):
-                    return True
-            return False
-
-        found_files = FileFinder(*paths, exclusion_filters=[_ignores])
+        found_files = FileFinder(*paths, exclusion_filters=[self.config.make_exclusion_filter()])
 
         messages = []
 

prospector/tools/pyroma/__init__.py

@@ -1,5 +1,8 @@
 import logging
+from typing import List
 
+from prospector.config import ProspectorConfig
+from prospector.finder import FileFinder
 from prospector.message import Location, Message
 from prospector.tools.base import ToolBase
 
@@ -58,29 +61,31 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.ignore_codes = ()
 
-    def configure(self, prospector_config, found_files):
+    def configure(self, prospector_config: ProspectorConfig, found_files: FileFinder):
         self.ignore_codes = prospector_config.get_disabled_messages("pyroma")
 
-    def run(self, found_files):
+    def run(self, found_files: FileFinder) -> List[Message]:
         messages = []
-        for module in found_files.python_modules:
-            dirname, filename = str(module.parent.absolute()), module.name
-            if filename != "setup.py":
-                continue
+        for directory in found_files.directories:
+            # just list directories which are not ignored, but find any `setup.py` ourselves
+            # as that is excluded by default
+            for filepath in directory.iterdir():
+                if filepath.is_dir() or filepath.name != "setup.py":
+                    continue
 
-            data = projectdata.get_data(dirname)
+                data = projectdata.get_data(directory.resolve())
 
-            all_tests = [m() for m in PYROMA_TEST_CLASSES]
-            for test in all_tests:
-                code = PYROMA_CODES.get(test.__class__, "PYRUNKNOWN")
+                all_tests = [m() for m in PYROMA_TEST_CLASSES]
+                for test in all_tests:
+                    code = PYROMA_CODES.get(test.__class__, "PYRUNKNOWN")
 
-                if code in self.ignore_codes:
-                    continue
+                    if code in self.ignore_codes:
+                        continue
 
-                passed = test.test(data)
-                if passed is False:  # passed can be True, False or None...
-                    loc = Location(module, "setup", None, -1, -1)
-                    msg = Message("pyroma", code, loc, test.message())
-                    messages.append(msg)
+                    passed = test.test(data)
+                    if passed is False:  # passed can be True, False or None...
+                        loc = Location(filepath, "setup", None, -1, -1)
+                        msg = Message("pyroma", code, loc, test.message())
+                        messages.append(msg)
 
         return messages

tests/config/test_config.py

@@ -1,35 +1,41 @@
 import re
 from pathlib import Path
-from unittest import TestCase
-from unittest.mock import patch
+
+from prospector.finder import FileFinder
+from ..utils import patch_execution
 
 from prospector.config import ProspectorConfig
 
 
-class TestProspectorConfig(TestCase):
-    def test_determine_ignores_all_str(self):
-        with patch("sys.argv", ["", "-P", "prospector-str-ignores"]), patch(
-            "pathlib.Path.cwd", return_value=Path(__file__).parent
-        ):
-            config = ProspectorConfig()
-        self.assertNotEqual(len(config.ignores), 0)
-        boundary = r"(^|/|\\)%s(/|\\|$)"
-        paths = ["2017", "2018"]
-        for path in paths:
-            compiled_ignored_path = re.compile(boundary % re.escape(path))
-            self.assertIn(compiled_ignored_path, config.ignores)
-
-    def test_determine_ignores_containing_int_values_wont_throw_attr_exc(self):
-        try:
-            with patch("sys.argv", ["", "-P", "prospector-int-ignores"]), patch(
-                "pathlib.Path.cwd", return_value=Path(__file__).parent
-            ):
-                config = ProspectorConfig()
-            self.assertNotEqual(len(config.ignores), 0)
-            boundary = r"(^|/|\\)%s(/|\\|$)"
-            paths = ["2017", "2018"]
-            for path in paths:
-                compiled_ignored_path = re.compile(boundary % re.escape(path))
-                self.assertIn(compiled_ignored_path, config.ignores)
-        except AttributeError as attr_exc:
-            self.fail(attr_exc)
+def test_relative_ignores():
+    """
+    Tests that if 'ignore-paths: something' is set, then it is ignored; that
+    is, paths relative to the working directory should be ignored too
+    """
+    workdir = Path(__file__).parent / 'testdata/test_relative_ignores'
+    with patch_execution(workdir, "-P", "profile_relative_ignores.yml"):
+        config = ProspectorConfig()
+        files = FileFinder(*config.paths, exclusion_filters=[config.make_exclusion_filter()])
+        assert 2 == len(files.python_modules)
+
+
+def test_determine_ignores_all_str():
+    with patch_execution(Path(__file__).parent, "-P", "prospector-str-ignores"):
+        config = ProspectorConfig()
+    assert len(config.ignores) > 0
+    boundary = r"(^|/|\\)%s(/|\\|$)"
+    paths = ["2017", "2018"]
+    for path in paths:
+        compiled_ignored_path = re.compile(boundary % re.escape(path))
+        assert compiled_ignored_path in config.ignores
+
+
+def test_determine_ignores_containing_int_values_wont_throw_attr_exc():
+    with patch_execution(Path(__file__).parent, "-P", "prospector-int-ignores"):
+        config = ProspectorConfig()
+    assert len(config.ignores) > 0
+    boundary = r"(^|/|\\)%s(/|\\|$)"
+    paths = ["2017", "2018"]
+    for path in paths:
+        compiled_ignored_path = re.compile(boundary % re.escape(path))
+        assert compiled_ignored_path in config.ignores

tests/config/testdata/test_relative_ignores/profile_relative_ignores.yml

@@ -0,0 +1,7 @@
+inherits:
+  - strictness_medium
+  - no_test_warnings
+
+
+ignore-paths:
+  - something.py

tests/config/testdata/test_relative_ignores/something.py

@@ -0,0 +1 @@
+x = 1 + unittes

tests/execution/__init__.py

@@ -0,0 +1,3 @@
+"""
+These are full "end-to-end" tests of a prospector run
+"""

tests/finder/test_execution.py → tests/execution/test_execution.py

@@ -2,13 +2,31 @@
 Tests that prospector raises the expected errors on the expected files depending on the
 configuration of the file finder
 """
+import contextlib
+from pathlib import Path
+from typing import Optional, List
 from unittest.mock import patch
 
 from prospector.config import ProspectorConfig
 from prospector.finder import FileFinder
 from prospector.tools import PylintTool
+from prospector.run import Prospector
+from ..utils import patch_execution
 
-from .utils import TEST_DATA
+
+TEST_DATA = Path(__file__).parent / "testdata"
+
+
+def test_ignored():
+    workdir = TEST_DATA / "ignore_test"
+    with patch_execution(workdir, '--profile', 'profile.yml', str(workdir)):
+        config = ProspectorConfig()
+        pros = Prospector(config)
+        pros.execute()
+        msgs = pros.get_messages()
+        # only the pkg3.broken should be picked up as everything else is ignored
+        assert len(msgs) == 1
+        assert msgs[0].location.module == 'pkg1.pkg2.pkg3.broken'
 
 
 def test_total_errors():
@@ -38,23 +56,22 @@ def test_relative_path_specified():
     (when running inside the prospector checkout
     """
     root = TEST_DATA / "relative_specified"
-    with patch("pathlib.Path.cwd", new=lambda: root):
-        # oddness here : Path.cwd() uses os.getcwd() under the hood in python<=3.9 but
-        # for python 3.10+, they return different things if only one is patched; therefore,
-        # for this test to work in all python versions prospector supports, both need to
-        # be patched (or, an "if python version" statement but it's easier to just patch both)
-        with patch("os.getcwd", new=lambda: str(root.absolute())):
-            with patch("sys.argv", ["prospector"]):
-                config1 = ProspectorConfig()
-                found_files1 = FileFinder(*config1.paths)
-
-            with patch("sys.argv", ["prospector", "../relative_specified"]):
-                config2 = ProspectorConfig()
-                found_files2 = FileFinder(*config1.paths)
-
-            with patch("sys.argv", ["prospector", "."]):
-                config3 = ProspectorConfig()
-                found_files3 = FileFinder(*config1.paths)
+    # oddness here : Path.cwd() uses os.getcwd() under the hood in python<=3.9 but
+    # for python 3.10+, they return different things if only one is patched; therefore,
+    # for this test to work in all python versions prospector supports, both need to
+    # be patched (or, an "if python version" statement but it's easier to just patch both)
+    with patch("pathlib.Path.cwd", new=lambda: root), patch("os.getcwd", new=lambda: str(root.absolute())):
+        with patch("sys.argv", ["prospector"]):
+            config1 = ProspectorConfig()
+            found_files1 = FileFinder(*config1.paths)
+
+        with patch("sys.argv", ["prospector", "../relative_specified"]):
+            config2 = ProspectorConfig()
+            found_files2 = FileFinder(*config1.paths)
+
+        with patch("sys.argv", ["prospector", "."]):
+            config3 = ProspectorConfig()
+            found_files3 = FileFinder(*config1.paths)
 
     assert root == config2.workdir == config1.workdir == config3.workdir
     assert config1.paths == config2.paths == config3.paths

tests/execution/testdata/ignore_test/pkg1/pkg2/asdr_partial_aserfg.py

@@ -0,0 +1 @@
+x += 1

tests/execution/testdata/ignore_test/pkg1/pkg2/pkg3/broken.py

@@ -0,0 +1 @@
+x = y+1

tests/execution/testdata/ignore_test/pkg1/pkg2/pkg3/safdv_partial_ag45.py

@@ -0,0 +1 @@
+x += 1

tests/execution/testdata/ignore_test/pkg1/pkg_ignore/broken2.py

@@ -0,0 +1 @@
+a = y + 3/0

tests/execution/testdata/ignore_test/profile.yml

@@ -0,0 +1,6 @@
+strictness: medium
+
+ignore-paths:
+  - pkg1/pkg_ignore
+ignore-patterns:
+  - .*partial.*

tests/tools/pyroma/test_pyroma_tool.py

@@ -0,0 +1,42 @@
+from pathlib import Path
+
+from prospector.config import ProspectorConfig
+from prospector.finder import FileFinder
+from prospector.tools.pyroma import PyromaTool
+from ...utils import patch_cwd, patch_cli
+
+
+def test_forced_include():
+    """
+    The built-in default profiles for prospector will ignore `setup.py` by default, but this needs
+    to be explicitly overridden for pyroma *only*
+
+    However, other ignore files should still not be returned. Only a setup.py in the root of the
+    working directory should be explicitly force-included.
+
+    see https://github.com/PyCQA/prospector/pull/106
+    """
+    test_data = Path(__file__).parent / 'testdata'
+
+    with patch_cwd(test_data):
+        # can't use the patch_execution shortcut due to pyroma playing with things itself too
+        with patch_cli('prospector', '--profile', 'test-pyroma-profile.yml'):
+            config = ProspectorConfig()
+            files = FileFinder(*config.paths, exclusion_filters=[config.make_exclusion_filter()])
+            # this should not return the root setup.py by default (using the strictness profile)
+            # but will return others (they might just be called setup.py by coincidence)
+            assert len(files.python_modules) == 3
+            tool = PyromaTool()
+            tool.configure(config, files)
+
+        # must do this outside of the CLI patch because pyroma does its own sys.argv patching...
+        messages = tool.run(files)
+
+        # this should still find errors in the setup.py, but not any of the others
+        assert len(messages) == 8
+        allowed = (
+                test_data / 'setup.py',
+                test_data / 'pkg1/this_one_is_fine/setup.py'
+        )
+        for message in messages:
+            assert message.location.path in allowed