[pytest-dev] fixtures and pylint W0621

[holger krekel]

Hi Floris, all,

On Fri, Dec 06, 2013 at 10:07 +0000, Floris Bruynooghe wrote:
> On 4 December 2013 15:05, lahwran <lahwran at lahwran.net> wrote:
> > perhaps pytest.fixture could return a special object that rejects all
> > operations, rather than the decorated function object; such that `theobject
> > == 5` would produce a big flashy error in theobject.__eq__ indicating you
> > need to actually use the fixture for this to work, maybe even with an
> > example. Repeat for all magic methods that make sense. this object would
> > maybe have a _pytest_fixture attribute, or conform to whatever protocol
> > already exists for detecting fixture objects, such that the only thing you
> > could do with such an object is check if it's a fixture and get the original
> > function object from somewhere inside it. Any other operation would result
> > in a flashy error.
> 
> This seems like a quite nice idea to me.

Not quite sure.  It would mean that if you want to unittest 
fixture functions, it's slightly hairy.  More importantly,
i think sometimes i use (and probably others as well) direct
calling of fixture functions for other purposes already:

    @pytest.fixture
    def arg(request):
        ...

    @pytest.fixture
    def arg2(request):
        if some_condition:
           val = arg(request)
           ...

which is currently valid code.  Arguably one could use getfuncargvalue
but IIRC i had some reasons to not do it at the time. (Can't locate the
code currently).

That being sad, i am open to trying it -- so if somebody does a PR
introducing the "exploding" @pytest.fixture return value, i'd merge
it temporarily and see if there are any problems.  It would be very
nice if random errors of forgetting to specify an argument would 
result in a clear error message.

> > On Tue, Dec 3, 2013 at 11:26 PM, holger krekel <holger at merlinux.eu> wrote:
> >> Well, ok.  Pending further input, i made pytest accept pytest.fixture
> >> decorated pytest_funcarg__ prefixed functions, see
> >>
> >> https://bitbucket.org/hpk42/pytest/commits/aa1f0505a3156b9feca43cd67c5afc95622b9ac5
> 
> I don't really like this change.  IIRC this was considered when the
> decorator was introduced and the reason it  was not allowed originally
> is because the signature is different between the different ways of
> defining fixtures.  That can be very confusing too.

Not sure i follow.  What is different?  The only difference is that
the decorator allows to specify a caching scope, params etc.  It does
not change anything about the fixture functions own signature.

> My first reaction on this pylint issue was to see if this can not be
> fixed wit a pylint plugin.  And I still think this would be nicer then
> the other proposals of trying to figure out how to name fixtures
> differently.  The way of declaring fixtures right now is very nice and
> readable.  Especially if lahwran's suggestion is implemented as that
> would make any mistakes obvious (and to an experience python developer
> they already are obvious).

Aiming for better pylint integration eg. through a plugin sounds like a
great plan.  And one that does not require deep pytest knowledge -- so
if someone here on the list is interested, i am willing to help design it.

> >> Then again, my general recommendation is to put fixture functions into
> >> conftest.py files and you then get a clean "NameError" in a test module
> >> for the above example.  The docs use the "everything contained in test
> >> module"
> >> method, though, to make it easier to present getting-started examples.
> >> Maybe it makes sense to mention this recommended conftest/test module
> >> split earlier on.
> 
> Personally I tend to recommend people to declare their fixtures as
> close to the code using it.  So if a fixture is only used in one class
> declare it in that class.  I find it easier to manage which fixtures
> are needed and know about the relevant ones without having to search
> around.  Also if you have all test modules in package, like py.test,
> you quickly end up with colliding fixture names and a giant and hard
> to manage collection of fixtures in conftest.py.

True as well.  Although for my 1K-10K LOC projects it's not really an issue 
yet.  Maybe some day we can think about namespacing in some way.

best,
holger