pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
12.13k stars 2.69k forks source link

Warn if argument shadows fixture #10512

Open MarcoGorelli opened 2 years ago

MarcoGorelli commented 2 years ago

What's the problem this feature will solve?

Say I have the files:

$ cat test_t.py
import pytest

@pytest.mark.parametrize('closed', ['left', 'right', 'both', 'neither'])
def test_me(closed):
    assert closed in ['left', 'right', 'both', 'neither']

$ cat conftest.py
import pytest

@pytest.fixture(params=["left", "right", "both", "neither"])
def closed(request):
    return request.param

Describe the solution you'd like

Some warning that, in test_me, I have an argument whose name shadows that of a fixture

This came out of a PR in pandas https://github.com/pandas-dev/pandas/pull/49754#discussion_r1025656945

Alternative Solutions

I'm tempted to write my own tool to do this, just wanted to check it's not been done already. I tried searching "argument shadows fixture " and similar things in the issue tracker but didn't find anything.

Additional context

The-Compiler commented 2 years ago

I don't follow. This seems to be a test which uses a fixture, in exactly the way how fixtures are intended to be used? :thinking:

MarcoGorelli commented 2 years ago

sorry, I'd accidentally removed the part the test which was the issue to test that it was picking up the fixture 🤦

have updated the issue description now, hope it's clear. the argument in pytest.mark.parametrize is the same one that's in the fixture

The-Compiler commented 2 years ago

Ah, that makes more sense now. Seems reasonable to me to show a warning in this case, but note there's indirect fixture parametrization which still needs to work, and looks almost like this.

MarcoGorelli commented 1 year ago

Thanks @The-Compiler

So, in cases when indirect isn't equal to True, then it should be safe to warn?

The-Compiler commented 1 year ago

It seems to me that it should, yes. But there are certainly a lot of subtleties involved in how fixtures work. Maybe someone else could confirm this?