importing pytest fixtures raises F401

[wasim42]

It appears that ruff isn't handling pytest fixtures correctly.

Consider the following files (adapted from a pytest example):

fruit.py

import pytest


class Fruit:
    def __init__(self, name):
        self.name = name

    def __eq__(self, other):
        return self.name == other.name


@pytest.fixture
def my_fruit():
    return Fruit("apple")


@pytest.fixture
def fruit_basket(my_fruit):
    return [Fruit("banana"), my_fruit]

and test_fruit.py:

from fruit import (
    my_fruit,
    fruit_basket
)

def test_my_fruit_in_basket(my_fruit, fruit_basket):
    assert my_fruit in fruit_basket

Running ruff --isolated test_fruit.py I get:

test_fruit.py:2:5: F401 [*] `fruit.my_fruit` imported but unused
test_fruit.py:3:5: F401 [*] `fruit.fruit_basket` imported but unused
test_fruit.py:6:29: F811 Redefinition of unused `my_fruit` from line 2
test_fruit.py:6:39: F811 Redefinition of unused `fruit_basket` from line 3
Found 4 errors.
[*] 2 potentially fixable with the --fix option.

I'm using ruff 0.0.262

[evanrittenhouse]

Thanks for filing. I'll look into this!

[charliermarsh]

I think this has been discussed before, but I’m on my phone and it may be hard to dig up the relevant issue.

[evanrittenhouse]

#3295 pointed me to this. Pytest suggests avoiding importing fixtures in favor of adding them to conftest.py. In that case, I'm personally a bit torn on implementing this - on one hand, it's definitely confusing behavior since the import is in fact used, but on the other it's not recommended to do so.

I'm almost leaning towards fixing the unused import bug, but maybe implementing a separate rule centered on avoiding fixture imports? FWIW, it seems like Pylint has the same issue as the one mentioned here. The suggested solution is to simply rename the fixture file (fruit.py, in this case) to conftest.py.

[charliermarsh]

Yeah, reading through the discussion, I think I agree with the conclusion that was reached in #3295: since Pytest itself recommends against this pattern, I'd rather not special-case in Ruff. (I'm not even sure how we could special-case it without introducing lots of false negatives. Best-case is that we could avoid autofixing these cases but still flag them.)

[charliermarsh]

I'm going to close, given that conclusion, but always happy to continue the discussion.

[wasim42]

Thank you very much for that! I’m a novice with pytest and this was a case of me using a linter to help me muddle through, and it has 😜

[jamesbraza]

Today, F401 autofixes wiped away my fixture imports a few times. I can think of several workarounds:

1. Use conftest.py (which I am avoiding for reasons beyond this issue's scope)
2. Manually insert # noqa: F401 before invoking ruff
3. Place F401 in unfixable aspect of config

I went with option 3, marking F401 as unfixable.