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
11.96k stars 2.66k forks source link

Session scoped fixtures can leak out of a class #2145

Open jinty opened 7 years ago

jinty commented 7 years ago

Best explained with a failing test, see:

https://github.com/jinty/pytest/commit/c0048b115314dd4a54d1fea5032d85f52efd221b

or inline for the lazy:

import pytest

@pytest.fixture(scope='session')
def foo():
    return 'foo'

@pytest.fixture(scope='session')
def bar(foo):
    return foo + 'bar'

@pytest.fixture(scope='session')
def baz(bar):
    return bar + 'baz'

class TestSpecific:

    @pytest.fixture(scope='session')
    def foo(self):
        return 'FOO'

    @pytest.fixture(scope='session')
    def baz(self, bar):
        return bar + 'BAZ'

    def test_b(self, foo, bar, baz):
        pass

def test_c(foo, bar, baz):
    assert foo == 'foo'
    assert bar == 'foobar'
    assert baz == 'foobarbaz'

Fails with the current master.

decentral1se commented 7 years ago

Yep, reproduced this one with wget goo.gl/CDgb12 -O test_2145.py && pytest.

I thought scope=session fixtures were global, so this is expected?

jinty commented 7 years ago

On Mon, Dec 19, 2016 at 04:30:29PM -0800, Luke Murphy wrote:

Yep, reproduced this one with wget goo.gl/CDgb12 -O test_2145.py && pytest.

I thought scope=session fixtures were global, so this is expected?

At least I don't expected that the result changes depending on test ordering and the dependency graph of the fixtures.

I would prefer an explicit error (i.e. "don't do that, it's silly") rather than the current behaviour which took quite a while to debug.

-- Brian Sutherland

RonnyPfannschmidt commented 7 years ago

the short story is, fixture declaration and mapping declarations to locations and caches is a complete mess and needs major refactoring ^^

nicoddemus commented 7 years ago

While I agree with @RonnyPfannschmidt that the fixture system could use a refactoring to account for the features added in the last years, this warrants more investigation because I think there's a bug that should be solvable here; test_c should not have been able to see the fixtures declared by TestSpecific.