Skip to content

Commit

Permalink
Don't delete FixtureDef.cached_result, set it to None instead
Browse files Browse the repository at this point in the history
Previously `cached_result` was either set or deleted. Type annotations
cannot handle this, so use `None` for the non-set state instead.
  • Loading branch information
bluetech committed Feb 14, 2020
1 parent 56a5dbe commit a7f53d9
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 11 deletions.
7 changes: 7 additions & 0 deletions changelog/TODO.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

0 comments on commit a7f53d9

Please sign in to comment.