reverbc / pylint-pytest

A Pylint plugin to suppress pytest-related false positives.
https://pypi.org/project/pylint-pytest/
MIT License
3 stars 8 forks source link

Fixtures depended by other fixtures reported as unused imports #19

Open Jaakkonen opened 3 years ago

Jaakkonen commented 3 years ago

Describe the bug When using a imported fixture with dependencies its dependencies need to be imported too. These imported dependencies are reported as unused by pylint-pytest.

To Reproduce Package versions

(add any relevant pylint/pytest plugin here)

Folder structure

tests/__init__.py
        fixtures.py
        test_thing.py

File content

# __init__.py <EMPTY>
# fixtures.py
"A docstring"
import pytest

@pytest.fixture
def my_fixture():
    "My 1st fixture"
    print("Hello")

@pytest.fixture
def my_second_fixture(my_fixture):
    "2nd fixture"
    print("Bar")
# test_thing.py
"Test module"
from .fixtures import (
        my_fixture,  # Imported as dependency of my_second_fixture
        my_second_fixture
)

def test_foo(my_second_fixture):
    "A dummy test"

pylint output with the plugin

$ pylint --load-plugins=pylint_pytest tests
************* Module tests.test_thing
tests/test_thing.py:2:0: W0611: Unused my_fixture imported from fixtures (unused-import)

------------------------------------------------------------------
Your code has been rated at 8.57/10 (previous run: 8.57/10, +0.00)

(Optional) pytest output from fixture collection

$ pytest --fixtures --collect-only tests
--------------------------------------------------- fixtures defined from tests.fixtures ----------------------------------------------------
my_second_fixture
    2nd fixture

my_fixture
    My 1st fixture

========================================================= 1 test collected in 0.01s =========================================================

Expected behavior A linting warning is not raised from fixtures imported as dependencies if the dependent fixture is being used.

Additional context Commenting the my_fixture out in test_thing.py makes pylint-pytest pass but executing tests fails to my_fixture not being found.

$ pytest tests
============================================================ test session starts ============================================================
platform linux -- Python 3.6.13, pytest-6.2.3, py-1.10.0, pluggy-0.13.1
rootdir: /tmp/tmp.n6epzQnc41
collected 1 item

tests/test_thing.py .                                                                                                                 [100%]

============================================================= 1 passed in 0.01s =============================================================
$ pylint --load-plugins=pylint_pytest tests************* Module tests.test_thing
tests/test_thing.py:2:0: W0611: Unused my_fixture imported from fixtures (unused-import)

------------------------------------------------------------------
Your code has been rated at 8.57/10 (previous run: 8.57/10, +0.00)
$ sed -i 's/my_fixture/#my_fixture/' tests/test_thing.py
$ pytest tests
============================================================ test session starts ============================================================
platform linux -- Python 3.6.13, pytest-6.2.3, py-1.10.0, pluggy-0.13.1
rootdir: /tmp/tmp.n6epzQnc41
collected 1 item

tests/test_thing.py E                                                                                                                 [100%]

================================================================== ERRORS ===================================================================
________________________________________________________ ERROR at setup of test_foo _________________________________________________________
file /tmp/tmp.n6epzQnc41/tests/test_thing.py, line 7
  def test_foo(my_second_fixture):
file /tmp/tmp.n6epzQnc41/tests/fixtures.py, line 9
  @pytest.fixture
  def my_second_fixture(my_fixture):
E       fixture 'my_fixture' not found
>       available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, doctest_namespace, monkeypatch, my_second_fixture, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory
>       use 'pytest --fixtures [testpath]' for help on them.

/tmp/tmp.n6epzQnc41/tests/fixtures.py:9
========================================================== short test summary info ==========================================================
ERROR tests/test_thing.py::test_foo
============================================================= 1 error in 0.01s ==============================================================
$ pylint --load-plugins=pylint_pytest tests

-------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 8.57/10, +1.43)
webknjaz commented 3 years ago

Commenting the my_fixture out in test_thing.py makes pylint-pytest pass but executing tests fails to my_fixture not being found.

This is weird and sounds like a bug in pytest itself. The import from .fixtures evaluates the fixtures module meaning that both fixtures must get registered no matter what. My guess is that the scope of the default fixtures is the fixtures.py module somehow and by exposing them to the global scope of test_thing.py, it makes them available.

FWIW I think that what you're trying to do is an antipattern and you should probably move the fixture imports or even their declarations to conftest.py instead. I don't think that pylint-pytest should encourage this usage pattern.

reverbc commented 3 years ago

AFAIK, pytest will only register the fixtures that are available in the locals() of the current module, or from locals() of the magic conftest.py. If we don't explicitly import the my_fixture in this module it will not load it.

FWIW I think that what you're trying to do is an antipattern and you should probably move the fixture imports or even their declarations to conftest.py instead. I don't think that pylint-pytest should encourage this usage pattern.

Yes I do agree with that. In general we should register the fixtures to conftest.py, and we do avoid the plugin from complaining unused imported fixtures in conftest.py at https://github.com/reverbc/pylint-pytest/blob/5804967/pylint_pytest/checkers/fixture.py#L217-L222

I'm wondering if there's a specific use case that you don't want to import the fixtures into conftest.py?

webknjaz commented 3 years ago

When you have a fixture that is local to the test module, not global for the whole test session. Also, you may override the global fixtures with some values specific to your module (by redefining fixtures with the same name).

reverbc commented 3 years ago

When you have a fixture that is local to the test module, not global for the whole test session. Also, you may override the global fixtures with some values specific to your module (by redefining fixtures with the same name).

@webknjaz sorry for the confusion - that question was actually addressed to @Jaakkonen. I'm interested in the pattern they're using (manually import fixtures from fixtures.py vs. automatically imported by contest.py).

Jaakkonen commented 3 years ago

I'm wondering if there's a specific use case that you don't want to import the fixtures into conftest.py?

Essentially trying not to have implicit dependencies.

I think one option would be to make pylint-pytest to give a warning which is not the pylint default unused-import if fixtures are imported explicitly (and not via conftest.py) like here. That would allow projects that want to use explicit imports of fixtures to ignore that warning without throwing # pylint: disable=unused-import in all places fixtures are being imported.