Merge branch 'master' into migrate-to-setup.cfg

# By Stavros Ntentos (14) and others # Via GitHub (10) and Stavros Ntentos (1) * master: Update/Prepare Changelog for v1.1.7 use pylint v3 in the repo remove support of pylint v1 Support pylint v3 Release v1.1.6 Minor `.github/ISSUE_TEMPLATE/bug_report.md` improvement Release v1.1.5 Improve copy-paste docstring from 8756cc4 Fix tests running via PyCharm Move `--cov-report=html` to `addopts` Windows Artifacts have incorrect Slugification Release v1.1.4 Extend coverage for `if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames:` Make `nsp/nate/better-unused-argument` merge-able with `origin/master` Improve `F6401`:`cannot-enumerate-pytest-fixtures`: 🐛 Ignore collection failures in non-tests removes more false positives for unused-argument Signed-off-by: Stavros Ntentos <[email protected]> # Conflicts: # .github/workflows/run-tests.yaml # CHANGELOG.md # setup.py # tests/base_tester.py # tox.ini Additionally, some minor format modifications at `CHANGELOG.md`
pylint-dev · Dec 3, 2023 · 421809d · 421809d
2 parents 3cbbfc6 + 48212e2
commit 421809d
Show file tree

Hide file tree

Showing 14 changed files with 255 additions and 57 deletions.
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -12,14 +12,21 @@ A clear and concise description of what the bug is.
 
 **To Reproduce**
 Package versions
-- pylint
-- pytest
-- pylint-pytest
+
+<!-- e.g., get them by running
+pip list |& grep -E '\b(pylint|pytest|pylint-pytest)\b' | sed 's/^/* /'
+-->
+
+* pylint:
+* pytest:
+* pylint-pytest:
 
 (add any relevant pylint/pytest plugin here)
 
 Folder structure
-```
+```console
+$ tree -L 2
+
 ```
 
 File content
@@ -32,7 +39,7 @@ pylint output with the plugin
 
 (Optional) pytest output from fixture collection
 ```bash
-$ pytest <path/to/test/module.py> --fixtures --collect-only
+$ pytest --fixtures --collect-only <path/to/test/module.py>
 <paste output here, can omit the built-ins>
 ```
 

diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml
@@ -33,7 +33,14 @@ jobs:
     steps:
       - uses: actions/checkout@v3
 
-      - name: Slugify GITHUB_REPOSITORY
+      - name: Slugify GITHUB_REPOSITORY (win)
+        if: ${{ matrix.os == 'windows-latest' }}
+        run: |
+          $slug = $env:GITHUB_REPOSITORY -replace '/', '_'
+          echo "GITHUB_REPOSITORY_SLUG=$slug" | tee -Append $env:GITHUB_ENV
+
+      - name: Slugify GITHUB_REPOSITORY (non-win)
+        if: ${{ matrix.os != 'windows-latest' }}
         run: echo "GITHUB_REPOSITORY_SLUG=${GITHUB_REPOSITORY////_}" >> $GITHUB_ENV
 
       - name: Set up Python ${{ matrix.python-version }}
@@ -51,11 +58,8 @@ jobs:
       - name: Test with tox
         env:
           FORCE_COLOR: 1
-          PYTEST_CI_ARGS: --cov-report=xml --cov-report=html --junitxml=test_artifacts/test_report.xml --color=yes
-        run: |
-          TOX_ENV=$(echo "py${{ matrix.python-version }}" | tr -d .)
-          tox -e $TOX_ENV
-        shell: bash
+          PYTEST_CI_ARGS: --cov-report=xml --junitxml=test_artifacts/test_report.xml --color=yes
+        run: tox --skip-missing-interpreters=true
 
       - name: Upload coverage reports to Codecov
         uses: codecov/codecov-action@v3

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,12 +4,75 @@
 
 ### Added
 
-- Support for Python 3.12 (#24)
-- Support for Pylint 3 (#24)
+* Support for Python 3.12 (https://github.com/pylint-dev/pylint-pytest/pull/24)
+* Support for Pylint 3 (https://github.com/pylint-dev/pylint-pytest/pull/24)
 
 ### Removed
 
-- Support for Python 3.6 & 3.7 (#23)
+* Support for Python 3.6 & 3.7 (https://github.com/pylint-dev/pylint-pytest/pull/23)
+
+## [1.1.7] - 2023-12-04
+
+This is a small release to support additionally Pylint v3.
+It should be noted, however, that for linting, Pylint must be v3 or newer (due to backwards-incompatible changes).
+
+### Fixed
+
+* Support pylint v3 and drop v1 (https://github.com/pylint-dev/pylint-pytest/pull/27)
+
+## [1.1.6] - 2023-11-20
+
+This is a small bugfix release.
+
+This will probably be the last bugfix release in the v1 series.
+We MAY support Python 3.12 in the v1 series if support appears to be trivial.
+
+### Fixed
+
+* 🐛 Ignore collection failures in non-tests (https://github.com/pylint-dev/pylint-pytest/pull/15)
+* Minor `.github/ISSUE_TEMPLATE/bug_report.md` improvement (https://github.com/pylint-dev/pylint-pytest/commit/22650f9912bcdc6a1bc4b3166f70bba7339aba7c)
+
+## [1.1.5] - 2023-11-13
+
+This is a small bugfix release.
+
+### Fixed
+
+* removes more false positives for unused-argument (https://github.com/pylint-dev/pylint-pytest/pull/21)
+* A collection of minor improvements to tests (https://github.com/pylint-dev/pylint-pytest/pull/26)
+* Windows Artifacts have incorrect Slugification (https://github.com/pylint-dev/pylint-pytest/pull/25)
+
+## [1.1.4] - 2023-11-06
+
+This is a small bugfix release.
+
+### Fixed
+
+* `anis-campos/fix_is_pytest_fixture` (https://github.com/pylint-dev/pylint-pytest/pull/2)
+  Astroid has different semantics when using `import pytest` or `from pytest import ...`,
+  which affects the detection of pytest fixtures.
+
+### Improved
+
+* `pre-commit`: (https://github.com/pylint-dev/pylint-pytest/pull/20)
+  * Added more checks to the `pre-commit` hook.
+    ```yaml
+    repos:
+      - repo: https://github.com/pre-commit/pre-commit-hooks
+        hooks:
+        - id: check-yaml
+        - id: check-toml
+        - id: check-vcs-permalinks
+        - id: check-shebang-scripts-are-executable
+        - id: name-tests-test
+      - repo: https://github.com/jumanjihouse/pre-commit-hook-yamlfmt
+          - id: yamlfmt
+      - repo: local
+        hooks:
+          - id: python-no-log-fatal
+            name: avoid logger.fatal(
+    ```
+  * Unified formatting (always expanded arrays; not covered by linters, sadly)
 
 ## [1.1.3] - 2023-10-23
 

diff --git a/pylint_pytest/checkers/__init__.py b/pylint_pytest/checkers/__init__.py
@@ -1,5 +1,16 @@
 from pylint.checkers import BaseChecker
 
+from pylint_pytest.utils import PYLINT_VERSION_MAJOR
+
 
 class BasePytestChecker(BaseChecker):
+    if PYLINT_VERSION_MAJOR < 3:
+        # todo(maybe-remove): if we only support pylint>=3
+        # Since https://github.com/pylint-dev/pylint/pull/8404, pylint does not need this
+        # __implements__ pattern. keeping it for retro compatibility with pylint==2.x
+        # pylint: disable=import-outside-toplevel,no-name-in-module
+        from pylint.interfaces import IAstroidChecker
+
+        __implements__ = IAstroidChecker
+
     name = "pylint-pytest"
diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py
@@ -1,11 +1,10 @@
 import fnmatch
-import os
+import io
 import sys
 from pathlib import Path
 from typing import Set, Tuple
 
 import astroid
-import pylint
 import pytest
 from pylint.checkers.variables import VariablesChecker
 
@@ -66,8 +65,8 @@ class FixtureChecker(BasePytestChecker):
         "F6401": (
             (
                 "pylint-pytest plugin cannot enumerate and collect pytest fixtures. "
-                "Please run `pytest --fixtures --collect-only path/to/current/module.py`"
-                " and resolve any potential syntax error or package dependency issues"
+                "Please run `pytest --fixtures --collect-only %s` and resolve "
+                "any potential syntax error or package dependency issues. stdout: %s. stderr: %s."
             ),
             "cannot-enumerate-pytest-fixtures",
             "Used when pylint-pytest has been unable to enumerate and collect pytest fixtures.",
@@ -114,11 +113,12 @@ def visit_module(self, node):
                 is_test_module = True
                 break
 
+        stdout, stderr = sys.stdout, sys.stderr
         try:
-            with open(os.devnull, "w") as devnull:
+            with io.StringIO() as captured_stdout, io.StringIO() as captured_stderr:
                 # suppress any future output from pytest
-                stdout, stderr = sys.stdout, sys.stderr
-                sys.stderr = sys.stdout = devnull
+                sys.stderr = captured_stderr
+                sys.stdout = captured_stdout
 
                 # run pytest session with customized plugin to collect fixtures
                 fixture_collector = FixtureCollector()
@@ -141,8 +141,32 @@ def visit_module(self, node):
 
                 FixtureChecker._pytest_fixtures = fixture_collector.fixtures
 
-                if (ret != pytest.ExitCode.OK or fixture_collector.errors) and is_test_module:
-                    self.add_message("cannot-enumerate-pytest-fixtures", node=node)
+                legitimate_failure_paths = set(
+                    collection_report.nodeid
+                    for collection_report in fixture_collector.errors
+                    if any(
+                        fnmatch.fnmatch(
+                            Path(collection_report.nodeid).name,
+                            pattern,
+                        )
+                        for pattern in FILE_NAME_PATTERNS
+                    )
+                )
+                if (ret != pytest.ExitCode.OK or legitimate_failure_paths) and is_test_module:
+                    files_to_report = {
+                        str(Path(x).absolute().relative_to(Path.cwd()))
+                        for x in legitimate_failure_paths | {node.file}
+                    }
+
+                    self.add_message(
+                        "cannot-enumerate-pytest-fixtures",
+                        args=(
+                            " ".join(files_to_report),
+                            captured_stdout.getvalue(),
+                            captured_stderr.getvalue(),
+                        ),
+                        node=node,
+                    )
         finally:
             # restore output devices
             sys.stdout, sys.stderr = stdout, stderr
@@ -208,7 +232,7 @@ def visit_functiondef(self, node):
             for arg in node.args.args:
                 self._invoked_with_func_args.add(arg.name)
 
-    # pylint: disable=bad-staticmethod-argument
+    # pylint: disable=bad-staticmethod-argument # The function itself is an if-return logic.
     @staticmethod
     def patch_add_message(
         self, msgid, line=None, node=None, args=None, confidence=None, col_offset=None
@@ -265,9 +289,18 @@ def patch_add_message(
             msgid == "unused-argument"
             and _can_use_fixture(node.parent.parent)
             and isinstance(node.parent, astroid.Arguments)
-            and node.name in FixtureChecker._pytest_fixtures
         ):
-            return
+            if node.name in FixtureChecker._pytest_fixtures:
+                # argument is used as a fixture
+                return
+
+            fixnames = (
+                arg.name for arg in node.parent.args if arg.name in FixtureChecker._pytest_fixtures
+            )
+            for fixname in fixnames:
+                if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames:
+                    # argument is used by a fixture
+                    return
 
         # check W0621 redefined-outer-name
         if (
@@ -278,10 +311,4 @@ def patch_add_message(
         ):
             return
 
-        if int(pylint.__version__.split(".")[0]) >= 2:
-            FixtureChecker._original_add_message(
-                self, msgid, line, node, args, confidence, col_offset
-            )
-        else:
-            # python2 + pylint1.9 backward compatibility
-            FixtureChecker._original_add_message(self, msgid, line, node, args, confidence)
+        FixtureChecker._original_add_message(self, msgid, line, node, args, confidence, col_offset)
diff --git a/pylint_pytest/utils.py b/pylint_pytest/utils.py
@@ -1,6 +1,9 @@
 import inspect
 
 import astroid
+import pylint
+
+PYLINT_VERSION_MAJOR = int(pylint.__version__.split(".")[0])
 
 
 def _is_pytest_mark_usefixtures(decorator):

diff --git a/pyproject.toml b/pyproject.toml
@@ -127,7 +127,7 @@ module = [
 check_untyped_defs = true
 
 [tool.pytest.ini_options]
-addopts = "--verbose --cov-config=pyproject.toml"
+addopts = "--verbose --cov-config=pyproject.toml --cov-report=html"
 
 [tool.ruff]
 # ruff is less lenient than pylint and does not make any exceptions
@@ -230,4 +230,8 @@ output-format = "colorized"
 ignored-argument-names = "_.*"
 
 [tool.pylint."messages control"]
-enable = ["useless-suppression"]
+enable = [
+    "useless-suppression",
+    "use-implicit-booleaness-not-comparison-to-zero",
+    "use-implicit-booleaness-not-comparison-to-string",
+]
diff --git a/tests/base_tester.py b/tests/base_tester.py
@@ -1,32 +1,31 @@
 import os
 import sys
 from abc import ABC
+from pathlib import Path
 from pprint import pprint
-from typing import Any, Dict, List
+from typing import List, Type
 
 import astroid
-from pylint.testutils import MessageTest, UnittestLinter
-
-try:
-    from pylint.utils import ASTWalker
-except ImportError:
-    # for pylint 1.9
-    from pylint.utils import PyLintASTWalker as ASTWalker
-
 from pylint.checkers import BaseChecker
+from pylint.testutils import MessageTest, UnittestLinter
+from pylint.utils import ASTWalker
 
 import pylint_pytest.checkers.fixture
 
 # XXX: allow all file names
 pylint_pytest.checkers.fixture.FILE_NAME_PATTERNS = ("*",)
 
 
+def get_test_root_path() -> Path:
+    """Assumes ``base_tester.py`` is at ``<root>/tests``."""
+    return Path(__file__).parent
+
+
 class BasePytestTester(ABC):
     CHECKER_CLASS = BaseChecker
-    IMPACTED_CHECKER_CLASSES: List[BaseChecker] = []
+    IMPACTED_CHECKER_CLASSES: List[Type[BaseChecker]] = []
     MSG_ID: str
     msgs: List[MessageTest] = []
-    CONFIG: Dict[str, Any] = {}
 
     def __init_subclass__(cls, **kwargs):
         super().__init_subclass__(**kwargs)
@@ -41,7 +40,10 @@ def run_linter(self, enable_plugin):
         # pylint: disable-next=protected-access
         target_test_file = sys._getframe(1).f_code.co_name.replace("test_", "", 1)
         file_path = os.path.join(
-            os.getcwd(), "tests", "input", self.MSG_ID, target_test_file + ".py"
+            get_test_root_path(),
+            "input",
+            self.MSG_ID,
+            target_test_file + ".py",
         )
 
         with open(file_path) as fin:
@@ -69,8 +71,6 @@ def setup_method(self):
         self.checker = self.CHECKER_CLASS(self.linter)
         self.impacted_checkers = []
 
-        for key, value in self.CONFIG.items():
-            setattr(self.linter.config, key, value)
         self.checker.open()
 
         for checker_class in self.IMPACTED_CHECKER_CLASSES:

diff --git a/tests/base_tester_test.py b/tests/base_tester_test.py
@@ -1,9 +1,14 @@
 import pytest
-from base_tester import BasePytestTester
+from base_tester import BasePytestTester, get_test_root_path
 
 # pylint: disable=unused-variable
 
 
+def test_get_test_root_path():
+    assert get_test_root_path().name == "tests"
+    assert (get_test_root_path() / "input").is_dir()
+
+
 def test_init_subclass_valid_msg_id():
     some_string = "some_string"