Skip to content

Parametrized fixture content depends on test function order. #5693

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

Closed
LaurentMarchelli opened this issue Aug 4, 2019 · 24 comments
Closed

Parametrized fixture content depends on test function order. #5693

LaurentMarchelli opened this issue Aug 4, 2019 · 24 comments
Labels
invalid topic: fixtures anything involving fixtures directly or indirectly type: question general question, might be closed after 2 weeks of inactivity

Comments

@LaurentMarchelli
Copy link

Fixture parametrized at the function definition, parameter scope='class'

When the same fixture (scope='class') is used by 2 functions:

  • function1 without parameters (path_default)
  • function2 with 2 parameters (path_param_0, path_param_1)

Result:

  • function1 (Passed)
  • function2['path_param_0'] (Failed)
    function2 gets the ids of the first parameter, however it receives the default value.
    The fixture is not called for the first parameter, the 'slot' seems to be already used by the default value.
  • function2['path_param_1'] (Passed)

But, if your reverse the call order, every test passes.

  • function2 with 2 parameters (path_param_0, path_param_1)
  • function1 without parameters (default value)

Code:

import pytest

@pytest.fixture(scope='class')
def menunode_path(request):
    result = getattr(request, 'param', None)
    if result is not None:
        assert(result in ('path_param_0', 'path_param_1'))
    else:
        result = 'path_default'
    return result

class TestFailed(object):

    def test_path(self, menunode_path):
        assert(menunode_path == 'path_default')

    @pytest.mark.parametrize('menunode_path', ('path_param_0', 'path_param_1'), 
                             indirect=True, scope='class')
    def test_path_param(self, menunode_path):
        assert(menunode_path in ('path_param_0', 'path_param_1'))

class TestPassed(object):

    @pytest.mark.parametrize('menunode_path', ('path_param_0', 'path_param_1'), 
                             indirect=True, scope='class')
    def test_path_param(self, menunode_path):
        assert(menunode_path in ('path_param_0', 'path_param_1'))

    def test_path(self, menunode_path):
        assert(menunode_path == 'path_default')

Result:

========================== test session starts ===========================
platform linux -- Python 3.6.8, pytest-5.0.1, py-1.8.0, pluggy-0.12.0 -- /home/laurent/Documents/Development/bboss/kconfigrobot/.venv/bin/python
cachedir: .pytest_cache
rootdir: /home/laurent/Documents/Development/bboss/kconfigrobot
collected 6 items                                                        

test_bugg.py::TestFailed::test_path PASSED                         [ 16%]
test_bugg.py::TestFailed::test_path_param[path_param_0] FAILED     [ 33%]
test_bugg.py::TestFailed::test_path_param[path_param_1] PASSED     [ 50%]
test_bugg.py::TestPassed::test_path_param[path_param_0] PASSED     [ 66%]
test_bugg.py::TestPassed::test_path_param[path_param_1] PASSED     [ 83%]
test_bugg.py::TestPassed::test_path PASSED                         [100%]

================================ FAILURES ================================
________________ TestFailed.test_path_param[path_param_0] ________________

self = <test_bugg.TestFailed object at 0x7f5337e96630>
menunode_path = 'path_default'

    @pytest.mark.parametrize('menunode_path', ('path_param_0', 'path_param_1'),
                             indirect=True, scope='class')
    def test_path_param(self, menunode_path):
>       assert(menunode_path in ('path_param_0', 'path_param_1'))
E       AssertionError: assert 'path_default' in ('path_param_0', 'path_param_1')

test_bugg.py:24: AssertionError
=================== 1 failed, 5 passed in 0.05 seconds ===================

System info:
Linux4.15.0-55-generic #60-Ubuntu SMP Tue Jul 2 18:22:20 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

anytree==2.6.0
atomicwrites==1.3.0
attrs==19.1.0
bleach==3.1.0
bumpversion==0.5.3
certifi==2019.6.16
chardet==3.0.4
Cython==0.29.12
docopt==0.6.2
docutils==0.15.1
idna==2.8
importlib-metadata==0.18
kconfiglib==12.12.1
more-itertools==7.2.0
packaging==19.0
pexpect==4.7.0
pkginfo==1.5.0.1
pluggy==0.12.0
ptyprocess==0.6.0
py==1.8.0
pydevd==1.6.1
Pygments==2.4.2
pyparsing==2.4.2a1
pytest==5.0.1
readme-renderer==24.0
requests==2.22.0
requests-toolbelt==0.9.1
robotframework==3.1.2
six==1.12.0
tqdm==4.32.2
twine==1.13.0
urllib3==1.25.3
wcwidth==0.1.7
webencodings==0.5.1
zipp==0.5.2
@asottile
Copy link
Member

asottile commented Aug 4, 2019

the output seems correct to me -- a class scoped fixture is only going to be constructed once per class -- you should refactor your tests to not have a backwards relation between fixtures and parameterization and/or refactor your fixture to not be dependent on call order and/or drop scope='class' such that the fixture is properly rebuilt for each test method

@asottile asottile added invalid topic: fixtures anything involving fixtures directly or indirectly type: question general question, might be closed after 2 weeks of inactivity labels Aug 4, 2019
@LaurentMarchelli
Copy link
Author

Hi asottile,

Could you please tell me, where I am wrong in my understanding:

  • A class fixture is built once per class and per parameter, not only once per class.
  • The parameter is a part of the fixture signature.
  • A fixture signature without a parameter must be different than a signature with a parameter.

Thank's.

@asottile
Copy link
Member

asottile commented Aug 5, 2019

A class fixture is built once per class and per parameter, not only once per class.

This is correct, but your fixture doesn't have parameters (the params= keyword to the fixture decorator), your test has parameters

@LaurentMarchelli
Copy link
Author

You're right it is not a Fixture problem, but a SubRequest bug:

class SubRequest(FixtureRequest):
   """ a sub request for handling getting a fixture from a
   test function/fixture. """

   def __init__(self, request, scope, param, param_index, fixturedef):
       self._parent_request = request
       self.fixturename = fixturedef.argname
       if param is not NOTSET:
           self.param = param
       self.param_index = param_index

A Subrequest without parameter must not have the same param_index as the first param in a list and should not have a param_index at all, otherwise FixtureDef.execute will return the cached value instead than calling hook.pytest_fixture_setup.

    def execute(self, request):
        # get required arguments and register our own finish()
        # with their finalization
        for argname in self.argnames:
            fixturedef = request._get_active_fixturedef(argname)
            if argname != "request":
                fixturedef.addfinalizer(functools.partial(self.finish, request=request))

        my_cache_key = request.param_index

A quick fix could be:

  • Indent self.param_index in SubRequest.init()
  • Replace line 867, and 921 with:
    my_cache_key = getattr(request, 'param_index', None)
  • Need to be tested if request param_index is used elsewhere without care.
class SubRequest(FixtureRequest):
    """ a sub request for handling getting a fixture from a
    test function/fixture. """

    def __init__(self, request, scope, param, param_index, fixturedef):
        self._parent_request = request
        self.fixturename = fixturedef.argname
        if param is not NOTSET:
            self.param = param
            self.param_index = param_index
    def execute(self, request):
        # get required arguments and register our own finish()
        # with their finalization
        for argname in self.argnames:
            fixturedef = request._get_active_fixturedef(argname)
            if argname != "request":
                fixturedef.addfinalizer(functools.partial(self.finish, request=request))

        my_cache_key = getattr(request, 'param_index', None)
def pytest_fixture_setup(fixturedef, request):
    """ Execution of fixture setup. """
    kwargs = {}
    for argname in fixturedef.argnames:
        fixdef = request._get_active_fixturedef(argname)
        result, arg_cache_key, exc = fixdef.cached_result
        request._check_scope(argname, request.scope, fixdef.scope)
        kwargs[argname] = result

    fixturefunc = resolve_fixture_function(fixturedef, request)
    my_cache_key = getattr(request, 'param_index', None)

Hope it helps ;-)
Regards,

@asottile
Copy link
Member

asottile commented Aug 5, 2019

I don't think that's right, but if you think it will fix the problem please propose a PR so it can be discussed there

@LaurentMarchelli
Copy link
Author

Could you please elaborate why you don't think that's right.

@asottile
Copy link
Member

asottile commented Aug 5, 2019

the fixture has no params= so it shouldn't be being rebuilt due to an related @parametrize -- at least that's my understanding of this

@LaurentMarchelli
Copy link
Author

So how do you explain indirect=True ?

@asottile
Copy link
Member

asottile commented Aug 5, 2019

that only makes sense with scope='function' right?

@RonnyPfannschmidt
Copy link
Member

Indirect is supposed to work with all scopes

@asottile
Copy link
Member

asottile commented Aug 5, 2019

@RonnyPfannschmidt won't that cause all fixtures of all scopes to constantly be recreated whenever using indirect? that doesn't seem right

@LaurentMarchelli
Copy link
Author

No it's seems to make sense with all scopes except the 'function', as the 'function' scope is the easier to manage, your data is not shared by anyone.

@LaurentMarchelli
Copy link
Author

My understanding, is the fixture is recreated when the cache does not contains the requested param_index.

    def execute(self, request):
        # get required arguments and register our own finish()
        # with their finalization
        for argname in self.argnames:
            fixturedef = request._get_active_fixturedef(argname)
            if argname != "request":
                fixturedef.addfinalizer(functools.partial(self.finish, request=request))

        my_cache_key = request.param_index
        cached_result = getattr(self, "cached_result", None)
        if cached_result is not None:
            result, cache_key, err = cached_result
            if my_cache_key == cache_key:
                if err is not None:
                    _, val, tb = err
                    raise val.with_traceback(tb)
                else:
                    return result
            # we have a previous but differently parametrized fixture instance
            # so we need to tear it down before creating a new one
            self.finish(request)
            assert not hasattr(self, "cached_result")

        hook = self._fixturemanager.session.gethookproxy(request.node.fspath)
        return hook.pytest_fixture_setup(fixturedef=self, request=request)

@asottile
Copy link
Member

asottile commented Aug 5, 2019

which would be always (unless your params is of length 1, which is not a useful parametrize):

            # we have a previous but differently parametrized fixture instance
            # so we need to tear it down before creating a new one
            self.finish(request)
            assert not hasattr(self, "cached_result")

@LaurentMarchelli
Copy link
Author

So, if we are in line, the fixture is created for each parameter as soon as his value is not in the cache, and the teardown is called for the one that "was" in the cached, whatever the scope.

@asottile
Copy link
Member

asottile commented Aug 5, 2019

yes, which would essentially demote all fixtures to function scoped

@LaurentMarchelli
Copy link
Author

Not really, only all fixtures that are called by functions indirect parameter.
But there is other big differences between these two scopes, the 'function' scope runs in its own private address space and all fixtures in the 'class' scope run in the same address space.

@asottile
Copy link
Member

asottile commented Aug 5, 2019

not sure what you mean by address space, there's only one address space per process

@asottile
Copy link
Member

asottile commented Aug 5, 2019

the scope really just controls the lifetime of a fixture

@LaurentMarchelli
Copy link
Author

Yes, it is also why they do not use the same 'address space'.
In the following example all 'class' fixtures share the same self but a 'function' fixture does not share it self with anyone.
It's what I meant by 'address space', in the following sample there is at least a 'class instance' for test execution, one for all 'class' fixtures and one per 'function' fixture.

import pytest

class TestSelf(object):

    @pytest.fixture(scope='class')
    def set_self_class(self):
        self.__class__._self_class = self
        return self

    @pytest.fixture(scope='class')
    def get_self_class(self):
        return self

    @pytest.fixture(scope='function')
    def set_self_func(self):
        self.__class__._self_func = self
        return self

    @pytest.fixture(scope='function')
    def get_self_func(self):
        return self

    # Tests function
    def test_set_class(self, set_self_class, get_self_class):
        assert(get_self_class != self)
        assert(get_self_class == set_self_class)
        assert(get_self_class == self.__class__._self_class)

    def test_set_func(self, set_self_func, get_self_func):
        assert(get_self_func == self)
        assert(get_self_func == set_self_func)
        assert(get_self_func == self.__class__._self_func)

    def test_get_class(self, get_self_class):
        assert(get_self_class != self)
        assert(get_self_class == self.__class__._self_class)
        assert(get_self_class != self.__class__._self_func)

    def test_get_func(self, get_self_func):
        assert(get_self_func == self)
        assert(get_self_func != self.__class__._self_class)
        assert(get_self_func != self.__class__._self_func)

    @pytest.mark.parametrize('get_self_class', ('One', 'Two'), 
                             indirect=True, scope='class')
    def test_get_class_scope1(self, get_self_class):
        assert(get_self_class != self)
        assert(get_self_class == self.__class__._self_class)
        assert(get_self_class != self.__class__._self_func)

    @pytest.mark.parametrize('get_self_class', ('One', 'Two'), 
                             indirect=True, scope='function')
    def test_get_class_scope2(self, get_self_class):
        assert(get_self_class == self)
        assert(get_self_class != self.__class__._self_class)
        assert(get_self_class != self.__class__._self_func)

@RonnyPfannschmidt
Copy link
Member

the general correct term for that is namespace

@LaurentMarchelli
Copy link
Author

@RonnyPfannschmidt
Thank's I will use it in the future if it makes more sense to you.
I you have a sec, have a look on the proposed fixed, I hope it will help.
I apologize, but I do not have any more available time to fill a PR.
Regards,

@RonnyPfannschmidt
Copy link
Member

i wont in near future, im still on my hiatus fro general project work

@LaurentMarchelli
Copy link
Author

Thank's guys for your time, it was a nice tchat.

As you can see below, it fixes at least the described problem.

=================================================== test session starts ====================================================
platform linux -- Python 3.6.8, pytest-5.0.2.dev122+g68c486a25.d20190806, py-1.8.0, pluggy-0.12.0 -- /home/laurent/Documents/Development/bboss/kconfigrobot/.venv/bin/python
cachedir: .pytest_cache
rootdir: /home/laurent/Documents/Development/bboss/kconfigrobot
collected 6 items                                                                                                          

bug_5693.py::TestFailed::test_path PASSED                                                                            [ 16%]
bug_5693.py::TestFailed::test_path_param[path_param_0] PASSED                                                        [ 33%]
bug_5693.py::TestFailed::test_path_param[path_param_1] PASSED                                                        [ 50%]
bug_5693.py::TestPassed::test_path_param[path_param_0] PASSED                                                        [ 66%]
bug_5693.py::TestPassed::test_path_param[path_param_1] PASSED                                                        [ 83%]
bug_5693.py::TestPassed::test_path PASSED                                                                            [100%]

================================================= 6 passed in 0.02 seconds =================================================

I attached the file with modifications from 3173815, and also the test file.
If someone has the time to create a PR and take the credit ... feel free.
By the way, it could also help [#4948]

Regards,
5693_bug_fixed.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid topic: fixtures anything involving fixtures directly or indirectly type: question general question, might be closed after 2 weeks of inactivity
Projects
None yet
Development

No branches or pull requests

3 participants