Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagating fixtures to test class attribute #2618

Closed
skraynev opened this issue Jul 26, 2017 · 26 comments
Closed

Propagating fixtures to test class attribute #2618

skraynev opened this issue Jul 26, 2017 · 26 comments

Comments

@skraynev
Copy link

Hi there.

I'd like to share my thought about using fixture in tests classes and maybe ask some advise. Current issue is related to two useful methods for newcomers from setuptools: (mark.usefixture and autouse - option for fixture) and using this stuff in Test classes.

First thing - (mark.usefixture) allows to define fixtures fro whole test class, but it does not provide ability to get access to this fixture from the test. For this purposes I have to define fixtures as parameters of test methods, like:

class TestExample():                  
    def test_one(self, param):               
        param.return_value = 100

honestly it's not a big deal, when you have couple of the test. However for huge numbers of the test, where I have the same fixtures, I'd like to hide avoid using fixture parameter. So I have to do it in the such way:

class TestExample():                  
   @pytest.fixture(autouse=True)    
    def init_fixture(param1, param2):
        self.param1 = param1         
        self.param2 = param2         

    def test_one(self):               
        self.param1.return_value = 100
        self.param2.return_value = 200

So the main idea -to hide explicitly mentioned fixtures of the function and providing them as class attribute.
It's also can be useful, when you port some test from testscenario extension, where parameters are available as class attributes.

I suppose, that it can be done by improving "mark.usefixtures" decorator, where it allows to add fixtures as class attributes. So potentially decorator can have syntax, like:
mark.usefixtures(*args, as_attrs=False), where *args - is list of fixtures, as previously and
as_attrs is a flag (defailt is False), which save fixtures as class attributes.
In case if function has not class it may be ignored or raise warning message.

Finally I suppose, that it can be done by adding minimal logic here: https://github.com/pytest-dev/pytest/blob/master/_pytest/python.py#L143

def pytest_pyfunc_call(pyfuncitem):                 
    testfunction = pyfuncitem.obj                   
+   if pyfuncitem.cls:
+         for k, v in pyfuncitem.funcargs:
+             setattr(pyfuncitem.cls, k, v) 
    if pyfuncitem._isyieldedfunction():             
        testfunction(*pyfuncitem._args)             
    else:                                           
        funcargs = pyfuncitem.funcargs              
        testargs = {}                               
        for arg in pyfuncitem._fixtureinfo.argnames:
            testargs[arg] = funcargs[arg]           
        testfunction(**testargs)                    
    return True        

Please correct me if I wrong somewhere or share your opinion about such change in the project.

@RonnyPfannschmidt
Copy link
Member

from my pov a feature like that is absolutely unacceptable as it is a start to introduce hidden dependencies

one point of fixtures is to show dependencies explicitly, so people make less of a mess, making them secretly be added into objects and just appear at a distance is the antithesis to this

@skraynev
Copy link
Author

Ronny, thank you for the feedback.

I agree, that it may confuse people if it be default. That's why I propose, to disable it by default and add ability to enable it by request.
Also "mark.usefixtures" already add hidden fixtures, which are not accessible. My idea is just add ability to use these fixtures from tests.
Moreover looks like "mark.usefixtures" is already does not work according philosophy mentioned by you, or may be I missed something?

@The-Compiler
Copy link
Member

I like the idea FWIW. Usually, explicit is better than implicit, but this still strikes me as explicit enough and removes quite some boilerplate.

@nicoddemus
Copy link
Member

IIRC, the new as_attrs of @pytest.mark.usefixtures would only work when used as a class decorator, and it would set the declared fixtures in, and only in, @pytest.mark.usefixtures as instance attributes.

I'm +0, IMHO this is not much more implicit than @pytest.mark.usefixtures works today anyway; all the warnings of using the usefixtures mark woud apply to the as_attrs option.

Having said that are you still strictly opposed @RonnyPfannschmidt?

@RonnyPfannschmidt
Copy link
Member

yes, imho a explicit class level fixture that sets the attribute is waaaaay more transparent and already supported directly without anyhacks

from my pov usefixtures is a unfortunate hack and we should completely remove the need to have it

@nicoddemus
Copy link
Member

yes, imho a explicit class level fixture that sets the attribute is waaaaay more transparent and already supported directly without anyhacks

Indeed it might be better to support this using something else other than pytest.mark.usefixtures, I don't know for certain until we see some working code though.

But not sure how much more transparent using something else other than pytest.mark.usefixtures would be, to be honest.

Instead of:

@pytest.mark.usefixtures('tmpdir', 'foobar', as_attrs=True)
class Test:
    ...

It would be some other construct at the class level anyway, perhaps:

@pytest.fixtures_as_attrs('tmpdir', 'foobar')
class Test:
    ...

Why would be the first be more transparent than the second, from a user's point of view?

Unless your point is that using marks for this would be problematic from an implementation POV, but that's a different discussion.

from my pov usefixtures is a unfortunate hack and we should completely remove the need to have it

I'm curious, was @pytest.mark.usefixtures added with the intent of making it easy for groups of tests to "auto use" fixtures without declaring them explicitly? Because for a single test case I don't see much value in mark.usefixtures as you can just declare that parameter there, but for marking tests in classes and modules it makes more sense. Just curious because I don't know pytest for so long. 😁

@RonnyPfannschmidt
Copy link
Member

class test:
  @pytest.fixture
  def setup_attributes(self, tmpdir, foobar):
    self.tmpdir = tmpdir
    self.foobar = foobar

is while a bit more verbose order of magnitude more simple to comprehend

@nicoddemus
Copy link
Member

I see, that's what I do currently when I need something like this (except I make setup_attributes be autouse) and it doesn't need any change in pytest.

But I thought that's what @The-Compiler commented about being a little verbose?

@skraynev were you aware of the above solution?

@skraynev
Copy link
Author

skraynev commented Jul 26, 2017

@nicoddemus I read you message and understand, that I do the same right now :)
Regarding solution for mentioned issue, we can ignore it, if you have idea, how to apply your approach for several classes (without copy-pasting). For example: I have 10 classes with some tests and each of them have fixture:

@pytest.fixture(autouse=True)
   def init_attrs(self, attr1, attr2):
         self.attr1 = attr1
         self.attr2 = attr2

It really irritate to copy paste the same structure for each class. I tried to use some parent class, but I have not any idea how should look common fixture, which allow to pass any kind of named parameters.
Obviously I can not have one fixture with the same signature always.

@RonnyPfannschmidt
Copy link
Member

RonnyPfannschmidt commented Jul 26, 2017

i believe its possible to use a mixin class (but i haven't tried yet)

@skraynev
Copy link
Author

@RonnyPfannschmidt Could you try it? because I am not sure, which parent classes for such mixin you plan to use.

@RonnyPfannschmidt
Copy link
Member

nope

@nicoddemus
Copy link
Member

nicoddemus commented Jul 28, 2017

I cooked up this solution using a class-decorator:

from functools import partial

import pytest


def _inject(cls, names):
    @pytest.fixture(autouse=True)
    def auto_injector_fixture(self, request):
        for name in names:
            setattr(self, name, request.getfixturevalue(name))

    cls._auto_injector_fixture = auto_injector_fixture
    return cls


def auto_inject_fixtures(*names):
    return partial(_inject, names=names)


@auto_inject_fixtures('tmpdir')
class Test:
    def test_foo(self):
        assert self.tmpdir.isdir()

It also works for unittest subclasses:

@auto_inject_fixtures('tmpdir')
class Test2(unittest.TestCase):
    def test_foo(self):
        self.assertTrue(self.tmpdir.isdir())

@skraynev
Copy link
Author

@nicoddemus Awesome. You are rock. I have not known about 'getfixturevalue' .

I tried and it perfectly works for me. Also I a little bit modified it to apply for case, when you have one main Test class. So it looks like:

class TestBase(object):                                       
                                                              
    fixture_names = ()                                        
                                                              
    @pytest.fixture(autouse=True)                             
    def auto_injector_fixture(self, request):                 
        if hasattr(self, 'scenarios'):                        
            names = self.scenarios[0][1].keys()               
        else:                                                 
            names = self.fixture_names                        
        for name in names:                                    
            setattr(self, name, request.getfixturevalue(name))

As you can see, it uses class attribute, (fixture_names) where user may define his fixtures for visibility.
Also I added optimization for my scenario tests, it discover list of fixtures automatically.
I hope it will be also useful for someone. Thank you for the help, probably we can close this issue for now.

@RonnyPfannschmidt
Copy link
Member

@skraynev thanks for the follow-up that includes your working solution, those follow-ups are rare and commendable 👍

@skraynev
Copy link
Author

@RonnyPfannschmidt np. Thank you all guys for the fast response and help. I forgot to ask, what do you think about adding some of these snippets to official doc? I suppose, that it can be mentioned near Fast port of testscenarios. Honestly I searched something like that there before starting this issue.

@RonnyPfannschmidt
Copy link
Member

personally i am biased a bit against it, @nicoddemus whats your oppinion?

@nicoddemus
Copy link
Member

@skraynev great, glad it worked out.

Can I persuade you to write a short post about this to the pytest-tricks blog? 😉

cc @hackebrot

@nicoddemus
Copy link
Member

Oh sorry missed the last two comments.

Hmm I'm not against adding it to the docs, we should only warn the user about the downsides (like we do anyway with autouse fixtures). @skraynev would like to contribute a PR with that addition to the docs then?

@skraynev
Copy link
Author

@nicoddemus Honestly I have not heard about pytest-tricks blog. IMHO, it should be better, then docs.
So my answer - I can prepare short note about it for blog mentioned by you. In this case we can avoid adding this info to official doc.

@nicoddemus
Copy link
Member

@skraynev sounds good, thanks!

@skraynev
Copy link
Author

@nicoddemus I have one partially related question. Is it expected behavior, that for TestCase class method: pytest_generate_tests , will not be run ?
I see, that my scenario tests works by using parametrize which is called inside my pytest_generate_tests in conftest. However If I add TestCase as a parent for my class, this section will be ignored. I tried to find some info in source code, but I can not find any condition which prohibits run pytest_generate_tests

@RonnyPfannschmidt
Copy link
Member

@skraynev pytest_generate_tests is incompatible with unittest TestCase

@nicoddemus
Copy link
Member

nicoddemus commented Jul 28, 2017

Hi @skraynev

Is it expected behavior, that for TestCase class method: pytest_generate_tests , will not be run ?

Probably not.

In general, pytest aims to support standard TestCase subclasses, but not to support all pytest features for them. So some features work (like autouse fixtures and markers), but most of them don't (parametrize for example).

We probably should explicitly state what is supported in the unittest docs though.

@nicoddemus
Copy link
Member

Created #2626 to improve the docs regarding that topic.

@skraynev
Copy link
Author

@RonnyPfannschmidt @nicoddemus Got it. Thank you guys for the clear explanation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants