From a86b89e1e3f1ee2b33c97863f856634bbb619557 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Wed, 18 Aug 2021 01:24:44 +0200 Subject: [PATCH 1/3] =?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 Fixes #21 --- pylint_pytest/checkers/fixture.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index c2a00a0..738abb2 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -61,7 +61,7 @@ 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.', @@ -135,8 +135,22 @@ 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 27b27367e3a7529cd5930c75a12ee35cba64f2ee Mon Sep 17 00:00:00 2001 From: Leonidas Loucas Date: Fri, 18 Feb 2022 21:15:22 -0800 Subject: [PATCH 2/3] feat: Support for pytest_describe --- pylint_pytest/checkers/fixture.py | 14 ++++++ pylint_pytest/utils.py | 46 ++++++++++++++++++- setup.cfg | 3 +- setup.py | 7 ++- tests/base_tester.py | 1 + tests/input/unused-argument/describe.py | 9 ++++ .../input/unused-argument/describe_nested.py | 10 ++++ tests/input/unused-variable/describe.py | 9 ++++ .../input/unused-variable/describe_nested.py | 14 ++++++ tests/test_unused_argument.py | 10 ++++ tests/test_unused_variable.py | 20 ++++++++ tox.ini | 2 +- 12 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 tests/input/unused-argument/describe.py create mode 100644 tests/input/unused-argument/describe_nested.py create mode 100644 tests/input/unused-variable/describe.py create mode 100644 tests/input/unused-variable/describe_nested.py create mode 100644 tests/test_unused_variable.py diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 738abb2..6dabae5 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -1,3 +1,4 @@ +import re import os import sys from pathlib import Path @@ -10,6 +11,7 @@ import pytest from ..utils import ( _can_use_fixture, + _is_in_describe_section_when_enabled, _is_pytest_mark, _is_pytest_mark_usefixtures, _is_pytest_fixture, @@ -72,6 +74,7 @@ class FixtureChecker(BasePytestChecker): _invoked_with_func_args = set() _invoked_with_usefixtures = set() _original_add_message = callable + enable_plugin = True def open(self): # patch VariablesChecker.add_message @@ -217,6 +220,11 @@ def patch_add_message(self, msgid, line=None, node=None, args=None, ''' - intercept and discard unwanted warning messages ''' + if not FixtureChecker.enable_plugin: + FixtureChecker._original_add_message( + self, msgid, line, node, args, confidence, col_offset + ) + return # check W0611 unused-import if msgid == 'unused-import': # actual attribute name is not passed as arg so...dirty hack @@ -265,6 +273,12 @@ def patch_add_message(self, msgid, line=None, node=None, args=None, node.name in FixtureChecker._pytest_fixtures: return + # check W0612 unused-variable + if msgid == 'unused-variable' and \ + _is_in_describe_section_when_enabled(node): + return + + if int(pylint.__version__.split('.')[0]) >= 2: FixtureChecker._original_add_message( self, msgid, line, node, args, confidence, col_offset) diff --git a/pylint_pytest/utils.py b/pylint_pytest/utils.py index 7dac65f..4042e26 100644 --- a/pylint_pytest/utils.py +++ b/pylint_pytest/utils.py @@ -1,5 +1,49 @@ import inspect +import re import astroid +import pytest +import sys + + +PYTEST_LT_7_0 = getattr(pytest, 'version_tuple', (0, 0)) < (7, 0) + + +try: + import pytest_describe + PYTEST_DESCRIBE = True +except ImportError: + PYTEST_DESCRIBE = False + describe_prefixes_option = () + + +if PYTEST_DESCRIBE: + describe_prefix = "describe" + try: + import _pytest.config.findpaths + config = _pytest.config.findpaths.determine_setup([], sys.argv[1:]) + if config: + if PYTEST_LT_7_0: + describe_prefix = config[2].config.sections.get('tool:pytest', {}).get('describe_prefixes', describe_prefix) + else: + describe_prefix = config[2].get('describe_prefixes', describe_prefix) + finally: + describe_prefixes_option = tuple(describe_prefix.split(' ')) + + +describe_prefix_matcher = re.compile(fr'^{"|".join(describe_prefixes_option)}_.+$') + + +def _is_in_describe_section_when_enabled(node): + import _pytest.config.findpaths + describe_prefix = "describe" + config = _pytest.config.findpaths.determine_setup([], sys.argv[1:]) + if config: + if PYTEST_LT_7_0: + describe_prefix = config[2].config.sections.get('tool:pytest', {}).get('describe_prefixes', describe_prefix) + else: + describe_prefix = config[2].get('describe_prefixes', describe_prefix) + return (PYTEST_DESCRIBE and + (node.parent is not None and isinstance(node.parent, astroid.FunctionDef) and re.match(describe_prefix_matcher, node.parent.name))) def _is_pytest_mark_usefixtures(decorator): @@ -84,7 +128,7 @@ def _can_use_fixture(function): if isinstance(function, astroid.FunctionDef): # test_*, *_test - if function.name.startswith('test_') or function.name.endswith('_test'): + if function.name.startswith('test_') or function.name.endswith('_test') or _is_in_describe_section_when_enabled(function): return True if function.decorators: diff --git a/setup.cfg b/setup.cfg index 2a2ef5b..0bccc26 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,6 +4,7 @@ test = pytest [tool:pytest] addopts = --verbose python_files = tests/test_*.py +describe_prefixes = describe context [bdist_wheel] -universal = 1 +universal = 1 \ No newline at end of file diff --git a/setup.py b/setup.py index ae9e869..147cc38 100644 --- a/setup.py +++ b/setup.py @@ -12,7 +12,7 @@ setup( name='pylint-pytest', - version='1.1.2', + version='1.1.3', author='Reverb Chu', author_email='pylint-pytest@reverbc.tw', maintainer='Reverb Chu', @@ -27,6 +27,9 @@ 'pylint', 'pytest>=4.6', ], + extras_require={ + 'pytest_describe': ['pytest_describe'], + }, python_requires='>=3.6', classifiers=[ 'Development Status :: 5 - Production/Stable', @@ -43,6 +46,6 @@ 'Operating System :: OS Independent', 'License :: OSI Approved :: MIT License', ], - tests_require=['pytest', 'pylint'], + tests_require=['pytest', 'pytest_describe', 'pylint'], keywords=['pylint', 'pytest', 'plugin'], ) diff --git a/tests/base_tester.py b/tests/base_tester.py index cf8c178..00596e8 100644 --- a/tests/base_tester.py +++ b/tests/base_tester.py @@ -28,6 +28,7 @@ class BasePytestTester(object): def run_linter(self, enable_plugin, file_path=None): self.enable_plugin = enable_plugin + self.CHECKER_CLASS.enable_plugin = enable_plugin # pylint: disable=protected-access if file_path is None: diff --git a/tests/input/unused-argument/describe.py b/tests/input/unused-argument/describe.py new file mode 100644 index 0000000..3410d39 --- /dev/null +++ b/tests/input/unused-argument/describe.py @@ -0,0 +1,9 @@ +import pytest + +def describe_stuff(): + @pytest.fixture() + def fix(): + pass + + def run(fix): + pass \ No newline at end of file diff --git a/tests/input/unused-argument/describe_nested.py b/tests/input/unused-argument/describe_nested.py new file mode 100644 index 0000000..08f0e91 --- /dev/null +++ b/tests/input/unused-argument/describe_nested.py @@ -0,0 +1,10 @@ +import pytest + +def describe_stuff(): + @pytest.fixture() + def fix(): + pass + + def context_more(): + def run(fix): + pass \ No newline at end of file diff --git a/tests/input/unused-variable/describe.py b/tests/input/unused-variable/describe.py new file mode 100644 index 0000000..3410d39 --- /dev/null +++ b/tests/input/unused-variable/describe.py @@ -0,0 +1,9 @@ +import pytest + +def describe_stuff(): + @pytest.fixture() + def fix(): + pass + + def run(fix): + pass \ No newline at end of file diff --git a/tests/input/unused-variable/describe_nested.py b/tests/input/unused-variable/describe_nested.py new file mode 100644 index 0000000..9f3a3e8 --- /dev/null +++ b/tests/input/unused-variable/describe_nested.py @@ -0,0 +1,14 @@ +import pytest + +def describe_stuff(): + @pytest.fixture() + def fix(): + pass + + def context_more(): + @pytest.fixture() + def fix(fix): + pass + + def run(fix): + pass \ No newline at end of file diff --git a/tests/test_unused_argument.py b/tests/test_unused_argument.py index 7ab2747..81f50d9 100644 --- a/tests/test_unused_argument.py +++ b/tests/test_unused_argument.py @@ -28,3 +28,13 @@ def test_caller_not_a_test_func(self, enable_plugin): def test_args_and_kwargs(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(2) + + @pytest.mark.parametrize('enable_plugin', [True, False]) + def test_describe(self, enable_plugin): + self.run_linter(enable_plugin) + self.verify_messages(0 if enable_plugin else 1) + + @pytest.mark.parametrize('enable_plugin', [True, False]) + def test_describe_nested(self, enable_plugin): + self.run_linter(enable_plugin) + self.verify_messages(0 if enable_plugin else 1) diff --git a/tests/test_unused_variable.py b/tests/test_unused_variable.py new file mode 100644 index 0000000..d384d98 --- /dev/null +++ b/tests/test_unused_variable.py @@ -0,0 +1,20 @@ +import pytest +from pylint.checkers.variables import VariablesChecker +from base_tester import BasePytestTester +from pylint_pytest.checkers.fixture import FixtureChecker + + +class TestUnusedVariable(BasePytestTester): + CHECKER_CLASS = FixtureChecker + IMPACTED_CHECKER_CLASSES = [VariablesChecker] + MSG_ID = 'unused-variable' + + @pytest.mark.parametrize('enable_plugin', [True, False]) + def test_describe(self, enable_plugin): + self.run_linter(enable_plugin) + self.verify_messages(0 if enable_plugin else 1) + + @pytest.mark.parametrize('enable_plugin', [True, False]) + def test_describe_nested(self, enable_plugin): + self.run_linter(enable_plugin) + self.verify_messages(0 if enable_plugin else 2) diff --git a/tox.ini b/tox.ini index a7cfb5d..01b33d5 100644 --- a/tox.ini +++ b/tox.ini @@ -4,5 +4,5 @@ skipsdist = True [testenv] commands = - pip install ./ --upgrade + pip install .[pytest_describe] --upgrade pytest {posargs:tests} From fd22da179666793de80cdc18ae10470bd04aef2a Mon Sep 17 00:00:00 2001 From: Leonidas Loucas Date: Sat, 19 Feb 2022 16:12:15 -0800 Subject: [PATCH 3/3] test: Move setup and teardown to a autouse fixture Pytest docs say that setup and teardown functions run prior to fixtures, therefore the plugin was always patching (being enabled), and strong guards via fixture cacheing were making it look like setup and teardown were called at the right time. By promotoing enable_plugin to a fixture in all cases, and using a autouse yield fixture to call setup and teardown we can make this under our full control --- pylint_pytest/checkers/fixture.py | 5 ---- tests/base_tester.py | 26 +++++++++++----- ...est_pytest_fixture_positional_arguments.py | 18 +++++++---- tests/test_pytest_mark_for_fixtures.py | 30 +++++++++++-------- tests/test_pytest_yield_fixture.py | 14 ++++++--- 5 files changed, 59 insertions(+), 34 deletions(-) diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 6dabae5..efd98f5 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -220,11 +220,6 @@ def patch_add_message(self, msgid, line=None, node=None, args=None, ''' - intercept and discard unwanted warning messages ''' - if not FixtureChecker.enable_plugin: - FixtureChecker._original_add_message( - self, msgid, line, node, args, confidence, col_offset - ) - return # check W0611 unused-import if msgid == 'unused-import': # actual attribute name is not passed as arg so...dirty hack diff --git a/tests/base_tester.py b/tests/base_tester.py index 00596e8..5731643 100644 --- a/tests/base_tester.py +++ b/tests/base_tester.py @@ -10,6 +10,7 @@ # for pylint 1.9 from pylint.utils import PyLintASTWalker as ASTWalker from pylint.checkers import BaseChecker +import pytest import pylint_pytest.checkers.fixture @@ -56,14 +57,24 @@ def verify_messages(self, msg_count, msg_id=None): pprint(self.MESSAGES) assert matched_count == msg_count, f'expecting {msg_count}, actual {matched_count}' - def setup_method(self): + @pytest.fixture(scope="function", autouse=True) + def within(self, request, enable_plugin): + try: + request.instance._setup_method(enable_plugin) + yield request.instance + finally: + request.instance._teardown_method(enable_plugin) + + + def _setup_method(self, enable_plugin): self.linter = UnittestLinter() - self.checker = self.CHECKER_CLASS(self.linter) self.impacted_checkers = [] - for key, value in self.CONFIG.items(): - setattr(self.checker.config, key, value) - self.checker.open() + if enable_plugin: + self.checker = self.CHECKER_CLASS(self.linter) + for key, value in self.CONFIG.items(): + setattr(self.checker.config, key, value) + self.checker.open() for checker_class in self.IMPACTED_CHECKER_CLASSES: checker = checker_class(self.linter) @@ -72,8 +83,9 @@ def setup_method(self): checker.open() self.impacted_checkers.append(checker) - def teardown_method(self): - self.checker.close() + def _teardown_method(self, enable_plugin): + if enable_plugin: + self.checker.close() for checker in self.impacted_checkers: checker.close() diff --git a/tests/test_pytest_fixture_positional_arguments.py b/tests/test_pytest_fixture_positional_arguments.py index 6208d9f..e565789 100644 --- a/tests/test_pytest_fixture_positional_arguments.py +++ b/tests/test_pytest_fixture_positional_arguments.py @@ -1,3 +1,5 @@ +import pytest + from base_tester import BasePytestTester from pylint_pytest.checkers.fixture import FixtureChecker @@ -6,14 +8,18 @@ class TestDeprecatedPytestFixtureScopeAsPositionalParam(BasePytestTester): CHECKER_CLASS = FixtureChecker MSG_ID = 'deprecated-positional-argument-for-pytest-fixture' - def test_with_args_scope(self): - self.run_linter(enable_plugin=True) + @pytest.fixture + def enable_plugin(self): + return True + + def test_with_args_scope(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(1) - def test_with_kwargs_scope(self): - self.run_linter(enable_plugin=True) + def test_with_kwargs_scope(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(0) - def test_without_scope(self): - self.run_linter(enable_plugin=True) + def test_without_scope(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(0) diff --git a/tests/test_pytest_mark_for_fixtures.py b/tests/test_pytest_mark_for_fixtures.py index 2854126..086a748 100644 --- a/tests/test_pytest_mark_for_fixtures.py +++ b/tests/test_pytest_mark_for_fixtures.py @@ -1,3 +1,5 @@ +import pytest + from base_tester import BasePytestTester from pylint_pytest.checkers.fixture import FixtureChecker @@ -6,26 +8,30 @@ class TestPytestMarkUsefixtures(BasePytestTester): CHECKER_CLASS = FixtureChecker MSG_ID = 'useless-pytest-mark-decorator' - def test_mark_usefixture_using_for_test(self): - self.run_linter(enable_plugin=True) + @pytest.fixture + def enable_plugin(self): + return True + + def test_mark_usefixture_using_for_test(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(0) - def test_mark_usefixture_using_for_class(self): - self.run_linter(enable_plugin=True) + def test_mark_usefixture_using_for_class(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(0) - def test_mark_usefixture_using_for_fixture_attribute(self): - self.run_linter(enable_plugin=True) + def test_mark_usefixture_using_for_fixture_attribute(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(2) - def test_mark_usefixture_using_for_fixture_function(self): - self.run_linter(enable_plugin=True) + def test_mark_usefixture_using_for_fixture_function(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(2) - def test_other_marks_using_for_fixture(self): - self.run_linter(enable_plugin=True) + def test_other_marks_using_for_fixture(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(4) - def test_not_pytest_marker(self): - self.run_linter(enable_plugin=True) + def test_not_pytest_marker(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(0) diff --git a/tests/test_pytest_yield_fixture.py b/tests/test_pytest_yield_fixture.py index 0102b89..c893923 100644 --- a/tests/test_pytest_yield_fixture.py +++ b/tests/test_pytest_yield_fixture.py @@ -1,3 +1,5 @@ +import pytest + from base_tester import BasePytestTester from pylint_pytest.checkers.fixture import FixtureChecker @@ -7,10 +9,14 @@ class TestDeprecatedPytestYieldFixture(BasePytestTester): IMPACTED_CHECKER_CLASSES = [] MSG_ID = 'deprecated-pytest-yield-fixture' - def test_smoke(self): - self.run_linter(enable_plugin=True) + @pytest.fixture + def enable_plugin(self): + return True + + def test_smoke(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(1) - def test_func(self): - self.run_linter(enable_plugin=True) + def test_func(self, enable_plugin): + self.run_linter(enable_plugin) self.verify_messages(2)