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

Use session.config.hook instead of ihook. Fixes #2124 #2127

Merged
merged 17 commits into from
Nov 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
522d59e
Use session.config.hook instead of ihook. Fixes #2124
malinoff Dec 10, 2016
a63b34c
Switch to item fspath
malinoff Dec 20, 2016
14b6380
Fix #2775 - running pytest with "--pyargs" will result in Items with …
Sep 13, 2017
794d458
Remove unnecessary complexity in _check_initialpaths_for_relpath().
Sep 28, 2017
76f3be4
remove unused _pytest.runner.NodeInfo class
RonnyPfannschmidt Nov 10, 2017
742f9cb
Merge pull request #2911 from RonnyPfannschmidt/remove-nodeinfo
nicoddemus Nov 10, 2017
f320686
Make SubRequest.addfinalizer an explicit method
nicoddemus Nov 11, 2017
b671c5a
Merge pull request #2914 from nicoddemus/addfinalizer-refactor
RonnyPfannschmidt Nov 11, 2017
f0f2d2b
Merge branch 'master' into fix-missing-nodeid-with-pyargs
RonnyPfannschmidt Nov 11, 2017
259b86b
Merge pull request #2776 from cryporchild/fix-missing-nodeid-with-pyargs
nicoddemus Nov 12, 2017
258031a
Merge remote-tracking branch 'upstream/master' into malinoff/fix-2124
nicoddemus Nov 12, 2017
6550b99
pytest_fixture_post_finalizer now receives a request argument
nicoddemus Nov 12, 2017
f074fd9
Merge remote-tracking branch 'upstream/features' into malinoff/fix-2124
nicoddemus Nov 12, 2017
063335a
Add changelog entries for #2124
nicoddemus Nov 12, 2017
bdad345
Fix passing request to finish() in FixtureDef
nicoddemus Nov 12, 2017
6d3fe0b
Explicitly clear finalizers list in finalize to ensure cleanup
nicoddemus Nov 12, 2017
a6f2d2d
Rename FixtureDef.finalizer to FixtureDef.finalizers
nicoddemus Nov 12, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 22 additions & 18 deletions _pytest/fixtures.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
from __future__ import absolute_import, division, print_function

import functools
import inspect
import sys
import warnings
from collections import OrderedDict

import attr
import py
from py._code.code import FormattedExcinfo

import attr
import _pytest
from _pytest import nodes
from _pytest._code.code import TerminalRepr
Expand All @@ -22,9 +24,6 @@
from _pytest.outcomes import fail, TEST_OUTCOME


from collections import OrderedDict


def pytest_sessionstart(session):
import _pytest.python

Expand Down Expand Up @@ -519,7 +518,7 @@ def _getfixturevalue(self, fixturedef):
val = fixturedef.execute(request=subrequest)
finally:
# if fixture function failed it might have registered finalizers
self.session._setupstate.addfinalizer(fixturedef.finish,
self.session._setupstate.addfinalizer(functools.partial(fixturedef.finish, request=subrequest),
subrequest.node)
return val

Expand Down Expand Up @@ -573,7 +572,6 @@ def __init__(self, request, scope, param, param_index, fixturedef):
self.param_index = param_index
self.scope = scope
self._fixturedef = fixturedef
self.addfinalizer = fixturedef.addfinalizer
self._pyfuncitem = request._pyfuncitem
self._fixture_values = request._fixture_values
self._fixture_defs = request._fixture_defs
Expand All @@ -584,6 +582,9 @@ def __init__(self, request, scope, param, param_index, fixturedef):
def __repr__(self):
return "<SubRequest %r for %r>" % (self.fixturename, self._pyfuncitem)

def addfinalizer(self, finalizer):
self._fixturedef.addfinalizer(finalizer)


class ScopeMismatchError(Exception):
""" A fixture function tries to use a different fixture function which
Expand Down Expand Up @@ -734,17 +735,17 @@ def __init__(self, fixturemanager, baseid, argname, func, scope, params,
self.argnames = getfuncargnames(func, is_method=unittest)
self.unittest = unittest
self.ids = ids
self._finalizer = []
self._finalizers = []

def addfinalizer(self, finalizer):
self._finalizer.append(finalizer)
self._finalizers.append(finalizer)

def finish(self):
def finish(self, request):
exceptions = []
try:
while self._finalizer:
while self._finalizers:
try:
func = self._finalizer.pop()
func = self._finalizers.pop()
func()
except: # noqa
exceptions.append(sys.exc_info())
Expand All @@ -754,20 +755,23 @@ def finish(self):
py.builtin._reraise(*e)

finally:
ihook = self._fixturemanager.session.ihook
ihook.pytest_fixture_post_finalizer(fixturedef=self)
hook = self._fixturemanager.session.gethookproxy(request.node.fspath)
hook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
# even if finalization fails, we invalidate
# the cached fixture value
# 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._finalizers = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RonnyPfannschmidt I realized we should also do this at this point to avoid keeping instances of bound methods alive in case they are registered as finalizers and an exception occurred during teardown, do you agree?


def execute(self, request):
# get required arguments and register our own finish()
# with their finalization
for argname in self.argnames:
fixturedef = request._get_active_fixturedef(argname)
if argname != "request":
fixturedef.addfinalizer(self.finish)
fixturedef.addfinalizer(functools.partial(self.finish, request=request))

my_cache_key = request.param_index
cached_result = getattr(self, "cached_result", None)
Expand All @@ -780,11 +784,11 @@ def execute(self, request):
return result
# we have a previous but differently parametrized fixture instance
# so we need to tear it down before creating a new one
self.finish()
self.finish(request)
assert not hasattr(self, "cached_result")

ihook = self._fixturemanager.session.ihook
return ihook.pytest_fixture_setup(fixturedef=self, request=request)
hook = self._fixturemanager.session.gethookproxy(request.node.fspath)
return hook.pytest_fixture_setup(fixturedef=self, request=request)

def __repr__(self):
return ("<FixtureDef name=%r scope=%r baseid=%r >" %
Expand Down
2 changes: 1 addition & 1 deletion _pytest/hookspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ def pytest_fixture_setup(fixturedef, request):
Stops at first non-None result, see :ref:`firstresult` """


def pytest_fixture_post_finalizer(fixturedef):
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."""
Expand Down
8 changes: 8 additions & 0 deletions _pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,16 @@ def __init__(self, fspath, parent=None, config=None, session=None):
super(FSCollector, self).__init__(name, parent, config, session)
self.fspath = fspath

def _check_initialpaths_for_relpath(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I first merged with upstream/master while working here so this PR is bringing #2776 into features as well. Because it is a small patch I don't think this will get into the way of reviewing things so if you guys are OK with it I won't bother trying to fix history in this case.

for initialpath in self.session._initialpaths:
if self.fspath.common(initialpath) == initialpath:
return self.fspath.relto(initialpath.dirname)

def _makeid(self):
relpath = self.fspath.relto(self.config.rootdir)

if not relpath:
relpath = self._check_initialpaths_for_relpath()
if os.sep != nodes.SEP:
relpath = relpath.replace(os.sep, nodes.SEP)
return relpath
Expand Down
5 changes: 0 additions & 5 deletions _pytest/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ def pytest_sessionfinish(session):
session._setupstate.teardown_all()


class NodeInfo:
def __init__(self, location):
self.location = location


def pytest_runtest_protocol(item, nextitem):
item.ihook.pytest_runtest_logstart(
nodeid=item.nodeid, location=item.location,
Expand Down
1 change: 1 addition & 0 deletions changelog/2124.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
``pytest_fixture_setup`` and ``pytest_fixture_post_finalizer`` hooks are now called for all ``conftest.py`` files.
1 change: 1 addition & 0 deletions changelog/2124.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
``pytest_fixture_post_finalizer`` hook can now receive a ``request`` argument.
1 change: 1 addition & 0 deletions changelog/2775.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix the bug where running pytest with "--pyargs" will result in Items with empty "parent.nodeid" if run from a different root directory.
4 changes: 3 additions & 1 deletion testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,10 @@ def join_pythonpath(*dirs):
for p in search_path:
monkeypatch.syspath_prepend(p)

os.chdir('world')
# mixed module and filenames:
result = testdir.runpytest("--pyargs", "-v", "ns_pkg.hello", "world/ns_pkg")
result = testdir.runpytest("--pyargs", "-v", "ns_pkg.hello", "ns_pkg/world")
testdir.chdir()
assert result.ret == 0
result.stdout.fnmatch_lines([
"*test_hello.py::test_hello*PASSED",
Expand Down
40 changes: 40 additions & 0 deletions testing/python/fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -3121,3 +3121,43 @@ def test_foo(request):
E*{1}:5
*1 failed*
""".format(fixfile.strpath, testfile.basename))


def test_pytest_fixture_setup_and_post_finalizer_hook(testdir):
testdir.makeconftest("""
from __future__ import print_function
def pytest_fixture_setup(fixturedef, request):
print('ROOT setup hook called for {0} from {1}'.format(fixturedef.argname, request.node.name))
def pytest_fixture_post_finalizer(fixturedef, request):
print('ROOT finalizer hook called for {0} from {1}'.format(fixturedef.argname, request.node.name))
""")
testdir.makepyfile(**{
'tests/conftest.py': """
from __future__ import print_function
def pytest_fixture_setup(fixturedef, request):
print('TESTS setup hook called for {0} from {1}'.format(fixturedef.argname, request.node.name))
def pytest_fixture_post_finalizer(fixturedef, request):
print('TESTS finalizer hook called for {0} from {1}'.format(fixturedef.argname, request.node.name))
""",
'tests/test_hooks.py': """
from __future__ import print_function
import pytest

@pytest.fixture()
def my_fixture():
return 'some'

def test_func(my_fixture):
print('TEST test_func')
assert my_fixture == 'some'
"""
})
result = testdir.runpytest("-s")
assert result.ret == 0
result.stdout.fnmatch_lines([
"*TESTS setup hook called for my_fixture from test_func*",
"*ROOT setup hook called for my_fixture from test_func*",
"*TEST test_func*",
"*TESTS finalizer hook called for my_fixture from test_func*",
"*ROOT finalizer hook called for my_fixture from test_func*",
])