m-burst / flake8-pytest-style

A flake8 plugin checking common style issues or inconsistencies with pytest-based tests.
MIT License
230 stars 15 forks source link

[false-positive][PT019] When I have a test with an underscored parameter name, it incorrectly suggests switching to `usefixtures` #276

Open webknjaz opened 9 months ago

webknjaz commented 9 months ago

Bug report

What's wrong

I get

PT019 fixture _set without value is injected as parameter, use @pytest.mark.usefixtures instead

with

...
    @pytest.mark.parametrize(("_set", "expected"), (({"key2"}, True), ({"key"}, False)))
    def test_isdisjoint(
        self, cls: Type[MutableMultiMapping[str]], _set: Set[str], expected: bool
    ) -> None:
        ...

How it should work

PT019 shouldn't be triggered. Replacing _set with x_set works, so the plugin incorrectly assumes that there's no value provided for _set while it is, and incorrectly suggests using @pytest.mark.usefixtures, which would make it impossible to provide it with a value.

System information

m-burst commented 9 months ago

Hi @webknjaz ,

I would argue that names prefixed with an underscore are designed to be unused, and to avoid clashing with builtin names I personally would suffix the name with the underscore instead of prefixing (i.e. set_ instead of _set). I understand this is opinionated, but it is consistent with rules PT004 and PT005 which check for leading underscores in fixture names.

webknjaz commented 9 months ago

Maybe, but it's still a bug. The error is lying to my face implying that it doesn't work in runtime.

m-burst commented 9 months ago

I agree that the error message is misleading in this case.

In simple cases we can analyse the names configured in parametrize and avoid triggering PT019 for them. I'm not sure when I can get around to it myself, but I would gladly accept a PR.