From bb0b523a8a2cf2b40022c51781ad9b99a4a058a1 Mon Sep 17 00:00:00 2001 From: Nate Parsons Date: Wed, 2 Nov 2022 09:03:55 -0700 Subject: [PATCH 1/8] removes more false positives for unused-argument --- pylint_pytest/checkers/fixture.py | 17 ++++++++----- .../func_param_as_fixture_arg.py | 24 +++++++++++++++++++ tests/test_unused_argument.py | 5 ++++ 3 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 tests/input/unused-argument/func_param_as_fixture_arg.py diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index c2a00a0..51c1a94 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -110,10 +110,10 @@ def visit_module(self, node): is_test_module = True break + stdout, stderr = sys.stdout, sys.stderr try: with open(os.devnull, 'w') as devnull: # suppress any future output from pytest - stdout, stderr = sys.stdout, sys.stderr sys.stderr = sys.stdout = devnull # run pytest session with customized plugin to collect fixtures @@ -238,11 +238,16 @@ def patch_add_message(self, msgid, line=None, node=None, args=None, return # check W0613 unused-argument - if 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 msgid == 'unused-argument': + if _can_use_fixture(node.parent.parent): + if isinstance(node.parent, astroid.Arguments): + if node.name in FixtureChecker._pytest_fixtures: + return # argument is used as a fixture + else: + 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: + return # argument is used by a fixture # check W0621 redefined-outer-name if msgid == 'redefined-outer-name' and \ diff --git a/tests/input/unused-argument/func_param_as_fixture_arg.py b/tests/input/unused-argument/func_param_as_fixture_arg.py new file mode 100644 index 0000000..2ef5f10 --- /dev/null +++ b/tests/input/unused-argument/func_param_as_fixture_arg.py @@ -0,0 +1,24 @@ +""" +This module illustrates a situation in which unused-argument should be +suppressed, but is not. +""" + +import pytest + + +@pytest.fixture +def myfix(arg): + """A fixture that requests a function param""" + print("arg is ", arg) + return True + + +@pytest.mark.parametrize("arg", [1, 2, 3]) +def test_myfix(myfix, arg): + """A test function that uses the param through a fixture""" + assert myfix + +@pytest.mark.parametrize("narg", [4, 5, 6]) +def test_nyfix(narg): # unused-argument + """A test function that does not use its param""" + assert True diff --git a/tests/test_unused_argument.py b/tests/test_unused_argument.py index 7ab2747..529e764 100644 --- a/tests/test_unused_argument.py +++ b/tests/test_unused_argument.py @@ -28,3 +28,8 @@ 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_func_param_as_fixture_arg(self, enable_plugin): + self.run_linter(enable_plugin) + self.verify_messages(1 if enable_plugin else 2) From 360461c01214aa9f4f605f4212b4063ccbcedb91 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Mon, 23 Oct 2023 11:43:03 +0300 Subject: [PATCH 2/8] Make `nsp/nate/better-unused-argument` merge-able with `origin/master` Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- pylint_pytest/checkers/fixture.py | 28 +++++++++++-------- .../func_param_as_fixture_arg.py | 1 + tests/test_unused_argument.py | 12 ++++---- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 51c1a94..0d4362a 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -196,7 +196,7 @@ def visit_functiondef(self, node): for arg in node.args.args: self._invoked_with_func_args.add(arg.name) - # pylint: disable=protected-access,bad-staticmethod-argument + # pylint: disable=bad-staticmethod-argument,too-many-branches # 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): @@ -238,16 +238,22 @@ def patch_add_message(self, msgid, line=None, node=None, args=None, return # check W0613 unused-argument - if msgid == 'unused-argument': - if _can_use_fixture(node.parent.parent): - if isinstance(node.parent, astroid.Arguments): - if node.name in FixtureChecker._pytest_fixtures: - return # argument is used as a fixture - else: - 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: - return # argument is used by a fixture + if ( + msgid == "unused-argument" + and _can_use_fixture(node.parent.parent) + and isinstance(node.parent, astroid.Arguments) + ): + 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 msgid == 'redefined-outer-name' and \ diff --git a/tests/input/unused-argument/func_param_as_fixture_arg.py b/tests/input/unused-argument/func_param_as_fixture_arg.py index 2ef5f10..43f01dd 100644 --- a/tests/input/unused-argument/func_param_as_fixture_arg.py +++ b/tests/input/unused-argument/func_param_as_fixture_arg.py @@ -18,6 +18,7 @@ def test_myfix(myfix, arg): """A test function that uses the param through a fixture""" assert myfix + @pytest.mark.parametrize("narg", [4, 5, 6]) def test_nyfix(narg): # unused-argument """A test function that does not use its param""" diff --git a/tests/test_unused_argument.py b/tests/test_unused_argument.py index 529e764..8ce8a8b 100644 --- a/tests/test_unused_argument.py +++ b/tests/test_unused_argument.py @@ -7,29 +7,29 @@ class TestUnusedArgument(BasePytestTester): CHECKER_CLASS = FixtureChecker IMPACTED_CHECKER_CLASSES = [VariablesChecker] - MSG_ID = 'unused-argument' + MSG_ID = "unused-argument" - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_smoke(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(0 if enable_plugin else 2) - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_caller_yield_fixture(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(0 if enable_plugin else 1) - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_caller_not_a_test_func(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(1) - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_args_and_kwargs(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(2) - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_func_param_as_fixture_arg(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(1 if enable_plugin else 2) From 8756cc4627a66f150197519a2588512511f0c4f3 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Mon, 6 Nov 2023 16:34:32 +0200 Subject: [PATCH 3/8] Extend coverage for `if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames:` Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- tests/input/unused-argument/func_param_as_fixture_arg.py | 6 ++++++ tests/test_unused_argument.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/input/unused-argument/func_param_as_fixture_arg.py b/tests/input/unused-argument/func_param_as_fixture_arg.py index 43f01dd..003d194 100644 --- a/tests/input/unused-argument/func_param_as_fixture_arg.py +++ b/tests/input/unused-argument/func_param_as_fixture_arg.py @@ -23,3 +23,9 @@ def test_myfix(myfix, arg): def test_nyfix(narg): # unused-argument """A test function that does not use its param""" assert True + + +@pytest.mark.parametrize("arg", [1, 2, 3]) +def test_narg_is_used_nowhere(myfix, narg): + """A test function that uses the param through a fixture""" + assert myfix diff --git a/tests/test_unused_argument.py b/tests/test_unused_argument.py index a5224e1..a164672 100644 --- a/tests/test_unused_argument.py +++ b/tests/test_unused_argument.py @@ -33,4 +33,4 @@ def test_args_and_kwargs(self, enable_plugin): @pytest.mark.parametrize("enable_plugin", [True, False]) def test_func_param_as_fixture_arg(self, enable_plugin): self.run_linter(enable_plugin) - self.verify_messages(1 if enable_plugin else 2) + self.verify_messages(2 if enable_plugin else 3) From 106c3b8fac3b9eb1dbbbcc0a0979140fac1d48a1 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Mon, 6 Nov 2023 16:59:13 +0200 Subject: [PATCH 4/8] Release v1.1.4 * anis-campos/fix_is_pytest_fixture: https://github.com/pylint-dev/pylint-pytest/pull/2 * stdedos/ci/pre-commit/add-MOAR: https://github.com/pylint-dev/pylint-pytest/pull/20 Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- CHANGELOG.md | 32 ++++++++++++++++++++++++++++++++ setup.py | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a62d64..b2fce98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,38 @@ ## [Unreleased] +## [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 This is the first release after maintenance was assumed by https://github.com/stdedos. diff --git a/setup.py b/setup.py index f30541f..2337f38 100755 --- a/setup.py +++ b/setup.py @@ -11,7 +11,7 @@ setup( name="pylint-pytest", - version="1.1.3", + version="1.1.4", author="Stavros Ntentos", author_email="133706+stdedos@users.noreply.github.com", license="MIT", From d11811ed3c7cdcb274702847358fb0f29dd353c1 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Mon, 23 Oct 2023 10:36:47 +0300 Subject: [PATCH 5/8] Windows Artifacts have incorrect Slugification e.g. https://github.com/pylint-dev/pylint-pytest/actions/runs/6592065982?pr=2 Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- .github/workflows/run-tests.yaml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 656f11f..eced9b9 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -42,7 +42,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 }} From 90e4141cfd235b090ea868b153bd2fd10c73c04f Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Mon, 6 Nov 2023 17:26:15 +0200 Subject: [PATCH 6/8] Move `--cov-report=html` to `addopts` Non-`--cov` invocation is unaffected; "no reason" not to generate `--cov-report=html` in an untracked directory AFAIS. Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- .github/workflows/run-tests.yaml | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 656f11f..476acaa 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -60,7 +60,7 @@ 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 + PYTEST_CI_ARGS: --cov-report=xml --junitxml=test_artifacts/test_report.xml --color=yes run: tox ${{ matrix.python-version == '3.6' && '--skip-missing-interpreters=true' || '' }} - name: Upload coverage reports to Codecov diff --git a/pyproject.toml b/pyproject.toml index 9f4abc8..059659c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,7 +68,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 From 8edad6065f348759d20583b570db9156dba2bee7 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Mon, 23 Oct 2023 17:21:45 +0300 Subject: [PATCH 7/8] Fix tests running via PyCharm Would've been better to get the path via `requests.config.rootdir`, but it looks like a pain to inject the `requests` fixture here. Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- tests/base_tester.py | 11 ++++++++++- tests/base_tester_test.py | 7 ++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/base_tester.py b/tests/base_tester.py index 12d2e63..deed382 100644 --- a/tests/base_tester.py +++ b/tests/base_tester.py @@ -1,6 +1,7 @@ import os import sys from abc import ABC +from pathlib import Path from pprint import pprint from typing import Any, Dict, List @@ -21,6 +22,11 @@ pylint_pytest.checkers.fixture.FILE_NAME_PATTERNS = ("*",) +def get_test_root_path() -> Path: + """Assumes ``base_tester.py`` is at ``/tests``.""" + return Path(__file__).parent + + class BasePytestTester(ABC): CHECKER_CLASS = BaseChecker IMPACTED_CHECKER_CLASSES: List[BaseChecker] = [] @@ -41,7 +47,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: diff --git a/tests/base_tester_test.py b/tests/base_tester_test.py index 4cf43ff..b40d510 100644 --- 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" From d8db8e4cdca1c01ad66c74ba8923949a8aab35c8 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Sat, 11 Nov 2023 23:53:27 +0200 Subject: [PATCH 8/8] Improve copy-paste docstring from 8756cc4627a66f150197519a2588512511f0c4f3 Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- tests/input/unused-argument/func_param_as_fixture_arg.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/input/unused-argument/func_param_as_fixture_arg.py b/tests/input/unused-argument/func_param_as_fixture_arg.py index 003d194..90f0404 100644 --- a/tests/input/unused-argument/func_param_as_fixture_arg.py +++ b/tests/input/unused-argument/func_param_as_fixture_arg.py @@ -27,5 +27,8 @@ def test_nyfix(narg): # unused-argument @pytest.mark.parametrize("arg", [1, 2, 3]) def test_narg_is_used_nowhere(myfix, narg): - """A test function that uses the param through a fixture""" + """ + A test function that does not use its param (``narg``): + Not itself, nor through a fixture (``myfix``). + """ assert myfix