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

Don't delete FixtureDef.cached_result, set it to None instead #6737

Merged
merged 1 commit into from Feb 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/6737.breaking.rst
@@ -0,0 +1,7 @@
The ``cached_result`` attribute of ``FixtureDef`` is now set to ``None`` when
the result is unavailable, instead of being deleted.

If your plugin perform checks like ``hasattr(fixturedef, 'cached_result')``,
for example in a ``pytest_fixture_post_finalizer`` hook implementation, replace
it with ``fixturedef.cached_result is not None``. If you ``del`` the attribute,
set it to ``None`` instead.
12 changes: 6 additions & 6 deletions src/_pytest/fixtures.py
Expand Up @@ -855,6 +855,7 @@ def __init__(
self.argnames = getfuncargnames(func, name=argname, is_method=unittest)
self.unittest = unittest
self.ids = ids
self.cached_result = None
self._finalizers = []

def addfinalizer(self, finalizer):
Expand All @@ -881,8 +882,7 @@ def finish(self, request):
# the cached fixture value and remove
# all finalizers because they may be bound methods which will
# keep instances alive
if hasattr(self, "cached_result"):
del self.cached_result
self.cached_result = None
self._finalizers = []

def execute(self, request):
Expand All @@ -894,9 +894,8 @@ def execute(self, request):
fixturedef.addfinalizer(functools.partial(self.finish, request=request))

my_cache_key = self.cache_key(request)
cached_result = getattr(self, "cached_result", None)
if cached_result is not None:
result, cache_key, err = cached_result
if self.cached_result is not None:
result, cache_key, err = self.cached_result
# note: comparison with `==` can fail (or be expensive) for e.g.
# numpy arrays (#6497)
if my_cache_key is cache_key:
Expand All @@ -908,7 +907,7 @@ def execute(self, request):
# 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")
assert self.cached_result is None

hook = self._fixturemanager.session.gethookproxy(request.node.fspath)
return hook.pytest_fixture_setup(fixturedef=self, request=request)
Expand Down Expand Up @@ -953,6 +952,7 @@ def pytest_fixture_setup(fixturedef, request):
kwargs = {}
for argname in fixturedef.argnames:
fixdef = request._get_active_fixturedef(argname)
assert fixdef.cached_result is not None
result, arg_cache_key, exc = fixdef.cached_result
request._check_scope(argname, request.scope, fixdef.scope)
kwargs[argname] = result
Expand Down
6 changes: 3 additions & 3 deletions src/_pytest/hookspec.py
Expand Up @@ -423,9 +423,9 @@ def pytest_fixture_setup(fixturedef, request):


def pytest_fixture_post_finalizer(fixturedef, request):
""" called after fixture teardown, but before the cache is cleared so
the fixture result cache ``fixturedef.cached_result`` can
still be accessed."""
"""Called after fixture teardown, but before the cache is cleared, so
the fixture result ``fixturedef.cached_result`` is still available (not
``None``)."""


# -------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions src/_pytest/setuponly.py
Expand Up @@ -34,8 +34,8 @@ def pytest_fixture_setup(fixturedef, request):
_show_fixture_action(fixturedef, "SETUP")


def pytest_fixture_post_finalizer(fixturedef):
if hasattr(fixturedef, "cached_result"):
def pytest_fixture_post_finalizer(fixturedef) -> None:
if fixturedef.cached_result is not None:
config = fixturedef._fixturemanager.config
if config.option.setupshow:
_show_fixture_action(fixturedef, "TEARDOWN")
Expand Down