From 516793339589b9278eb99da68dd21f4d9b2ab2c8 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 22 Jul 2018 09:27:34 -0300 Subject: [PATCH 1/3] Move teardown code of yield fixtures to a partial to avoid leaks As it were before, it was keeping a reference to fixturefunc and it alive when an error occurred --- src/_pytest/fixtures.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 6f1a0880d..ca150580e 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -789,23 +789,26 @@ def call_fixture_func(fixturefunc, request, kwargs): if yieldctx: it = fixturefunc(**kwargs) res = next(it) - - def teardown(): - try: - next(it) - except StopIteration: - pass - else: - fail_fixturefunc( - fixturefunc, "yield_fixture function has more than one 'yield'" - ) - - request.addfinalizer(teardown) + finalizer = functools.partial(_teardown_yield_fixture, fixturefunc, it) + request.addfinalizer(finalizer) else: res = fixturefunc(**kwargs) return res +def _teardown_yield_fixture(fixturefunc, it): + """Executes the teardown of a fixture function by advancing the iterator after the + yield and ensure the iteration ends (if not it means there is more than one yield in the function""" + try: + next(it) + except StopIteration: + pass + else: + fail_fixturefunc( + fixturefunc, "yield_fixture function has more than one 'yield'" + ) + + class FixtureDef(object): """ A container for a factory definition. """ From c9a088130947dfb8453bbd0ccec6bd1de219fa18 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 22 Jul 2018 09:37:41 -0300 Subject: [PATCH 2/3] Isolate the code that resolves the fixturefunc to a separate function pytest_fixture_setup was somewhat convoluted because it was trying to do too many things. --- src/_pytest/fixtures.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index ca150580e..f384fa4c3 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -899,15 +899,10 @@ class FixtureDef(object): ) -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 - +def resolve_fixture_function(fixturedef, request): + """Gets the actual callable that can be called to obtain the fixture value, dealing with unittest-specific + instances and bound methods. + """ fixturefunc = fixturedef.func if fixturedef.unittest: if request.instance is not None: @@ -921,6 +916,19 @@ def pytest_fixture_setup(fixturedef, request): fixturefunc = getimfunc(fixturedef.func) if fixturefunc != fixturedef.func: fixturefunc = fixturefunc.__get__(request.instance) + return fixturefunc + + +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 = request.param_index try: result = call_fixture_func(fixturefunc, request, kwargs) From f5165064eedf852898ff0418f40e404a9b1eda19 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 22 Jul 2018 09:41:03 -0300 Subject: [PATCH 3/3] Make yield_fixture just call fixture to do its work Since fixture and yield_fixture are identical, they should call the same code; as it was, the code inside them was already starting to deviate. --- src/_pytest/fixtures.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index f384fa4c3..207bf27e4 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1027,13 +1027,7 @@ def yield_fixture(scope="function", params=None, autouse=False, ids=None, name=N .. deprecated:: 3.0 Use :py:func:`pytest.fixture` directly instead. """ - if callable(scope) and params is None and not autouse: - # direct decoration - return FixtureFunctionMarker("function", params, autouse, ids=ids, name=name)( - scope - ) - else: - return FixtureFunctionMarker(scope, params, autouse, ids=ids, name=name) + return fixture(scope=scope, params=params, autouse=autouse, ids=ids, name=name) defaultfuncargprefixmarker = fixture()