Skip to content

Explicitly handle session- and module- scoped fixtures with @saved_fixture #17

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
smarie opened this issue Dec 19, 2018 · 7 comments
Closed
Labels
enhancement New feature or request

Comments

@smarie
Copy link
Owner

smarie commented Dec 19, 2018

As of today @saved_fixture can only be used for function-scoped fixtures, but the error message is cryptic:

"Internal Error - This fixture 'xxxxxxx' was already stored for test id ''"

We could at least detect this and raise a better exception.

We could also go further and propose specific ways to store session- or module-scoped fixtures.
The main problem is that session-scoped fixtures are not guaranteed to be unique by session, and module-scoped fixtures are not guaranteed to be unique for an unique parameter, in pytest. See pytest-dev/pytest#2846

@Sup3rGeo
Copy link

Sup3rGeo commented May 8, 2019

Hi @smarie! Just a feedback that I was so happy that pytest-harvest was exactly what we need for one use case we have, until I hit the message (already improved it seems) that @saved_fixtures only work for function-scoped fixtures, which was then a deal breaker and led us to implement this manually.

module-scoped fixtures are not guaranteed to be unique for an unique parameter

hmm I think they definitely should, at least theoretically.

@Sup3rGeo
Copy link

I was giving this a little more thought, and I think we could do at least some of the following things. Either:

  • Only raise if in fact there is more than one invocation for the fixture (i.e. not unique)
  • Add keys for each invocation of the fixture (e.g. myfixture for the first one and then myfixture[1] myfixture[2] and so on for the following invocations)
  • Only keep the returned value for the last invocation. Meaning that invocations overwrite the value if already existing.

I think that I would prefer alternative 2, because then pytest-harvest can still be useful and is up to the user to understand how pytest works.

@smarie
Copy link
Owner Author

smarie commented May 20, 2019

Sorry for the delay in answering, I was away - nice proposal indeed.

hmm I think they definitely should, at least theoretically.

Yes I was surprised too but apparently there is a "good reason" : if a fixture has two parameters (a, b) and the test order uses a, then b, then a. When current test requires parameter b, then the previously created fixture for parameter a is torn down, even if it is used afterwards again by the following test in the same module.

You have probably seen this answer. I personally do not like this idea but their philosophy seems that a fixture should not be "alive" in memory if current test does not use it. That's why the fixture for param 'a' is torn down then recreated in the above example. We should maybe ask for an option to "leave the session and module fixtures alive even if they are not used". That (and only that) will guarantee uniqueness. I mention it at the end of pytest-dev/pytest#3393.

@smarie
Copy link
Owner Author

smarie commented May 20, 2019

I suggest to allow session and module scoped fixtures to be stored but to store them exactly the same way than function-scope: with an inner key equal to the test id they where created for.

That mean that if you have a fixture with session scope used in two tests but not recreated, only the first test id will appear. However if it is recreated, it will appear under each test id where it was recreated.

Would that suit your need ?

@smarie
Copy link
Owner Author

smarie commented May 20, 2019

Example 1: if the fixture does not change only a single test entry will appear:

@pytest.fixture(scope='session')
@saved_fixture
def my_fix(request):
    return 1

def test_foo(my_fix):
    assert my_fix == 1

def test_bar(my_fix):
    assert my_fix == 1

def test_synthesis(fixture_store):
    print(fixture_store['my_fix'])

The print should contain only one entry with name test_foo:

{'pytest_harvest/tests_raw/test_saved_fixture_session_no_params.py::test_foo': 1}

@smarie
Copy link
Owner Author

smarie commented May 20, 2019

Example 2: re-creation. If the fixture is instantiated by pytest several times (whatever the reason),there will be an entry for each time it was setup.

@pytest.fixture(scope='module', params=[1, 2])
@saved_fixture
def my_fix():
    return 1

@pytest.fixture(scope='module', params=['a', 'b'])
@saved_fixture
def my_fix2():
    return 1

def test_foo(my_fix):
    assert my_fix == 1

def test_bar(my_fix, my_fix2):
    assert my_fix == 1

def test_synthesis(fixture_store):
    print(list(fixture_store['my_fix'].keys()))
    print(list(fixture_store['my_fix2'].keys()))

Yields the following "strange but true" pytest execution order as explained by this post:

test_saved_fixture_module_params.py::test_foo[1] 
test_saved_fixture_module_params.py::test_bar[1-a] 
test_saved_fixture_module_params.py::test_bar[2-a] 
test_saved_fixture_module_params.py::test_foo[2] 
test_saved_fixture_module_params.py::test_bar[2-b] 
test_saved_fixture_module_params.py::test_bar[1-b]

And as proposed, the fixtures are only stored when they are re-created:

(this is my_fix, that is created 3 times for parameter 1, 2, 1)
['pytest_harvest/tests_raw/test_saved_fixture_module_params.py::test_foo[1]', 
 'pytest_harvest/tests_raw/test_saved_fixture_module_params.py::test_bar[2-a]', 
 'pytest_harvest/tests_raw/test_saved_fixture_module_params.py::test_bar[1-b]']

(this is my_fix2 that is created 2 times for parameter a, b)
['pytest_harvest/tests_raw/test_saved_fixture_module_params.py::test_bar[1-a]', 
'pytest_harvest/tests_raw/test_saved_fixture_module_params.py::test_bar[2-b]']

Let me know if that is ok for you. In that case, I'll commit and push this in a new release.

@Sup3rGeo
Copy link

Hi @smarie, glad you are back!
Yes, it would work for me like this. In fact it is probably the best way of handling this situation.
Waiting to test it :)

@smarie smarie closed this as completed in 2fa0b7f May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants