From f6a343d72b425e632db1a02b9b065d07fba93b72 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Wed, 18 Oct 2023 21:27:08 +0300 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20Ignore=20collection=20failur?= =?UTF-8?q?es=20in=20non-tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change applies the pre-existing patterns to identify if the files with collection problems are tests. It is then used to eliminate the false-positives of F6401 (cannot-enumerate-pytest-fixtures). As a side effect, this patch also includes precise file paths that may be used to reproduce the problem. Fixes https://github.com/reverbc/pylint-pytest/issues/20 Fixes https://github.com/reverbc/pylint-pytest/issues/21 Signed-off-by: Sviatoslav Sydorenko _Replayed; Source PR: https://github.com/reverbc/pylint-pytest/pull/22_ Additionally, satisfied the https://github.com/pylint-dev/pylint-pytest's `.pre-commit-config.yaml` Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- pylint_pytest/checkers/fixture.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 32ec894..9af22a7 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -68,8 +68,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" ), "cannot-enumerate-pytest-fixtures", "Used when pylint-pytest has been unable to enumerate and collect pytest fixtures.", @@ -143,8 +143,23 @@ 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: + self.add_message( + "cannot-enumerate-pytest-fixtures", + args=" ".join(legitimate_failure_paths | {node.file}), + node=node, + ) finally: # restore output devices sys.stdout, sys.stderr = stdout, stderr From abc9f2220d9407e252a520f596344341b5915dcb Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:14:31 +0300 Subject: [PATCH 2/2] Improve `F6401`:`cannot-enumerate-pytest-fixtures`: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Capture and return `stdout` and `stderr` (instead of trashing them) * Fix the 'duplicate-path' error by `relative`-izing the paths before `union`ing them * Update tests to test for both conditions Additionally, fix two `used-before-assignment` pylint issues (`stdout`, `stderr`) 🤪 Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- pylint_pytest/checkers/fixture.py | 22 +++++++++++----- tests/test_cannot_enumerate_fixtures.py | 34 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 9af22a7..79b9fb8 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -1,5 +1,5 @@ import fnmatch -import os +import io import sys from pathlib import Path from typing import Set, Tuple @@ -69,7 +69,7 @@ class FixtureChecker(BasePytestChecker): ( "pylint-pytest plugin cannot enumerate and collect pytest fixtures. " "Please run `pytest --fixtures --collect-only %s` and resolve " - "any potential syntax error or package dependency issues" + "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.", @@ -116,11 +116,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() @@ -155,9 +156,18 @@ def visit_module(self, node): ) ) 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(legitimate_failure_paths | {node.file}), + args=( + " ".join(files_to_report), + captured_stdout.getvalue(), + captured_stderr.getvalue(), + ), node=node, ) finally: diff --git a/tests/test_cannot_enumerate_fixtures.py b/tests/test_cannot_enumerate_fixtures.py index 4ad241e..362ab2a 100644 --- a/tests/test_cannot_enumerate_fixtures.py +++ b/tests/test_cannot_enumerate_fixtures.py @@ -1,3 +1,5 @@ +import re + import pytest from base_tester import BasePytestTester @@ -13,7 +15,39 @@ def test_no_such_package(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(1 if enable_plugin else 0) + if enable_plugin: + msg = self.msgs[0] + + # Asserts/Fixes duplicate filenames in output: + # https://github.com/reverbc/pylint-pytest/pull/22/files#r698204470 + filename_arg = msg.args[0] + assert len(re.findall(r"\.py", filename_arg)) == 1 + + # Asserts that path is relative (usually to the root of the repository). + assert filename_arg[0] != "/" + + # Assert `stdout` is non-empty. + assert msg.args[1] + # Assert `stderr` is empty (pytest runs stably, even though fixture collection fails). + assert not msg.args[2] + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_import_corrupted_module(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(1 if enable_plugin else 0) + + if enable_plugin: + msg = self.msgs[0] + + # ... somehow, since `import_corrupted_module.py` imports `no_such_package.py` + # both of their names are returned in the message. + filename_arg = msg.args[0] + assert len(re.findall(r"\.py", filename_arg)) == 2 + + # Asserts that paths are relative (usually to the root of the repository). + assert not [x for x in filename_arg.split(" ") if x[0] == "/"] + + # Assert `stdout` is non-empty. + assert msg.args[1] + # Assert `stderr` is empty (pytest runs stably, even though fixture collection fails). + assert not msg.args[2]