From 522d59e844abe815790acf733a0aa13b597acfac Mon Sep 17 00:00:00 2001 From: Dmitry Malinovsky Date: Sat, 10 Dec 2016 16:45:40 +0600 Subject: [PATCH 01/11] Use session.config.hook instead of ihook. Fixes #2124 --- _pytest/fixtures.py | 8 +++---- testing/python/fixture.py | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 28bcd4d8d..c2a27b83d 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -754,8 +754,8 @@ class FixtureDef: func = self._finalizer.pop() func() finally: - ihook = self._fixturemanager.session.ihook - ihook.pytest_fixture_post_finalizer(fixturedef=self) + hook = self._fixturemanager.session.config.hook + hook.pytest_fixture_post_finalizer(fixturedef=self) # even if finalization fails, we invalidate # the cached fixture value if hasattr(self, "cached_result"): @@ -783,8 +783,8 @@ class FixtureDef: self.finish() assert not hasattr(self, "cached_result") - ihook = self._fixturemanager.session.ihook - return ihook.pytest_fixture_setup(fixturedef=self, request=request) + hook = self._fixturemanager.session.config.hook + return hook.pytest_fixture_setup(fixturedef=self, request=request) def __repr__(self): return ("" % diff --git a/testing/python/fixture.py b/testing/python/fixture.py index be99ed833..3e84be138 100644 --- a/testing/python/fixture.py +++ b/testing/python/fixture.py @@ -2984,4 +2984,49 @@ class TestParameterizedSubRequest: """.format(fixfile.strpath, testfile.basename)) +def test_pytest_fixture_setup_hook(testdir): + testdir.makeconftest(""" + import pytest + def pytest_fixture_setup(): + print('pytest_fixture_setup hook called') + """) + testdir.makepyfile(""" + import pytest + + @pytest.fixture() + def some(): + return 'some' + + def test_func(some): + assert some == 'some' + """) + result = testdir.runpytest("-s") + assert result.ret == 0 + result.stdout.fnmatch_lines([ + "*pytest_fixture_setup hook called*", + ]) + + +def test_pytest_fixture_post_finalizer_hook(testdir): + testdir.makeconftest(""" + import pytest + + def pytest_fixture_post_finalizer(): + print('pytest_fixture_post_finalizer hook called') + """) + testdir.makepyfile(""" + import pytest + + @pytest.fixture() + def some(): + return 'some' + + def test_func(some): + assert some == 'some' + """) + result = testdir.runpytest("-s") + assert result.ret == 0 + result.stdout.fnmatch_lines([ + "*pytest_fixture_post_finalizer hook called*", + ]) From a63b34c68596e4cddec6d899d8b27417ea4ce72c Mon Sep 17 00:00:00 2001 From: Dmitry Malinovsky Date: Tue, 20 Dec 2016 10:44:09 +0600 Subject: [PATCH 02/11] Switch to item fspath --- _pytest/fixtures.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index c2a27b83d..b951ae818 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -783,7 +783,9 @@ class FixtureDef: self.finish() assert not hasattr(self, "cached_result") - hook = self._fixturemanager.session.config.hook + hook = self._fixturemanager.session.gethookproxy( + request._pyfuncitem.fspath + ) return hook.pytest_fixture_setup(fixturedef=self, request=request) def __repr__(self): From 14b6380e5f8c2e26aa518de8a499978eb9601848 Mon Sep 17 00:00:00 2001 From: Christian Boelsen Date: Wed, 13 Sep 2017 17:14:24 +0100 Subject: [PATCH 03/11] Fix #2775 - running pytest with "--pyargs" will result in Items with empty "parent.nodeid" if run from a different root directory --- _pytest/main.py | 10 ++++++++++ changelog/2775.bugfix | 1 + testing/acceptance_test.py | 4 +++- 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 changelog/2775.bugfix diff --git a/_pytest/main.py b/_pytest/main.py index 4bddf1e2d..6203f15c5 100644 --- a/_pytest/main.py +++ b/_pytest/main.py @@ -520,8 +520,18 @@ class FSCollector(Collector): super(FSCollector, self).__init__(name, parent, config, session) self.fspath = fspath + def _check_initialpaths_for_relpath(self): + for initialpath in self.session._initialpaths: + parent_path = self.fspath + for _ in parent_path.parts(): + if parent_path.samefile(initialpath): + return self.fspath.relto(initialpath.dirname) + parent_path = parent_path.__class__(parent_path.dirname) + def _makeid(self): relpath = self.fspath.relto(self.config.rootdir) + if not relpath: + relpath = self._check_initialpaths_for_relpath() if os.sep != "/": relpath = relpath.replace(os.sep, "/") return relpath diff --git a/changelog/2775.bugfix b/changelog/2775.bugfix new file mode 100644 index 000000000..8123522ac --- /dev/null +++ b/changelog/2775.bugfix @@ -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. diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 712776906..8a8c32762 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -624,8 +624,10 @@ class TestInvocationVariants(object): 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", From 794d4585d3b0e3c892ee97142e5c1d392928d372 Mon Sep 17 00:00:00 2001 From: Christian Boelsen Date: Thu, 28 Sep 2017 20:53:50 +0100 Subject: [PATCH 04/11] Remove unnecessary complexity in _check_initialpaths_for_relpath(). --- _pytest/main.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/_pytest/main.py b/_pytest/main.py index 6203f15c5..472cf77b1 100644 --- a/_pytest/main.py +++ b/_pytest/main.py @@ -522,11 +522,8 @@ class FSCollector(Collector): def _check_initialpaths_for_relpath(self): for initialpath in self.session._initialpaths: - parent_path = self.fspath - for _ in parent_path.parts(): - if parent_path.samefile(initialpath): - return self.fspath.relto(initialpath.dirname) - parent_path = parent_path.__class__(parent_path.dirname) + if self.fspath.common(initialpath) == initialpath: + return self.fspath.relto(initialpath.dirname) def _makeid(self): relpath = self.fspath.relto(self.config.rootdir) From 76f3be452a01441d6ee1d9c53d7f1fefaa5c8f45 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Fri, 10 Nov 2017 17:48:52 +0100 Subject: [PATCH 05/11] remove unused _pytest.runner.NodeInfo class --- _pytest/runner.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/_pytest/runner.py b/_pytest/runner.py index ed58aceec..53f6824da 100644 --- a/_pytest/runner.py +++ b/_pytest/runner.py @@ -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, From f320686fe06bb6a9a9c2009613206114edc55d40 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 11 Nov 2017 03:07:34 -0200 Subject: [PATCH 06/11] Make SubRequest.addfinalizer an explicit method This implicit definition really tripped me while debugging #2127, unfortunately hidden as it was in the middle of all the variable declarations. I think the explicit definition is much easier for the eyes and IDEs to find. --- _pytest/fixtures.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index f71f35768..966913fa6 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -575,7 +575,6 @@ class SubRequest(FixtureRequest): 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 @@ -586,6 +585,9 @@ class SubRequest(FixtureRequest): def __repr__(self): return "" % (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 From 6550b9911bd38e3cb01fab0af388530b46619d4b Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 12 Nov 2017 11:14:55 -0200 Subject: [PATCH 07/11] pytest_fixture_post_finalizer now receives a request argument --- _pytest/fixtures.py | 19 +++++------ _pytest/hookspec.py | 2 +- testing/python/fixture.py | 68 +++++++++++++++++---------------------- 3 files changed, 41 insertions(+), 48 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 7b50b8574..5e3474ac3 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -4,6 +4,7 @@ import inspect import sys import warnings +import functools import py from py._code.code import FormattedExcinfo @@ -521,7 +522,7 @@ class FixtureRequest(FuncargnamesCompatAttr): 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 @@ -744,7 +745,7 @@ class FixtureDef: def addfinalizer(self, finalizer): self._finalizer.append(finalizer) - def finish(self): + def finish(self, request): exceptions = [] try: while self._finalizer: @@ -759,10 +760,12 @@ class FixtureDef: py.builtin._reraise(*e) finally: - hook = self._fixturemanager.session.config.hook - hook.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 @@ -772,7 +775,7 @@ class FixtureDef: 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) @@ -788,9 +791,7 @@ class FixtureDef: self.finish() assert not hasattr(self, "cached_result") - hook = self._fixturemanager.session.gethookproxy( - request._pyfuncitem.fspath - ) + hook = self._fixturemanager.session.gethookproxy(request.node.fspath) return hook.pytest_fixture_setup(fixturedef=self, request=request) def __repr__(self): diff --git a/_pytest/hookspec.py b/_pytest/hookspec.py index e5c966e58..ceb87a3c2 100644 --- a/_pytest/hookspec.py +++ b/_pytest/hookspec.py @@ -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.""" diff --git a/testing/python/fixture.py b/testing/python/fixture.py index b351eeeca..f843e1948 100644 --- a/testing/python/fixture.py +++ b/testing/python/fixture.py @@ -3127,49 +3127,41 @@ class TestParameterizedSubRequest(object): """.format(fixfile.strpath, testfile.basename)) -def test_pytest_fixture_setup_hook(testdir): +def test_pytest_fixture_setup_and_post_finalizer_hook(testdir): testdir.makeconftest(""" - import pytest - - def pytest_fixture_setup(): - print('pytest_fixture_setup hook called') + 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(""" - import pytest + 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 some(): - return 'some' + @pytest.fixture() + def my_fixture(): + return 'some' - def test_func(some): - assert some == '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([ - "*pytest_fixture_setup hook called*", - ]) - - -def test_pytest_fixture_post_finalizer_hook(testdir): - testdir.makeconftest(""" - import pytest - - def pytest_fixture_post_finalizer(): - print('pytest_fixture_post_finalizer hook called') - """) - testdir.makepyfile(""" - import pytest - - @pytest.fixture() - def some(): - return 'some' - - def test_func(some): - assert some == 'some' - """) - result = testdir.runpytest("-s") - assert result.ret == 0 - result.stdout.fnmatch_lines([ - "*pytest_fixture_post_finalizer hook called*", + "*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*", ]) From 063335a715bce042e0c9e591ab8ad406488cb5dc Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 12 Nov 2017 11:19:02 -0200 Subject: [PATCH 08/11] Add changelog entries for #2124 --- changelog/2124.bugfix | 1 + changelog/2124.feature | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog/2124.bugfix create mode 100644 changelog/2124.feature diff --git a/changelog/2124.bugfix b/changelog/2124.bugfix new file mode 100644 index 000000000..e1c5e044c --- /dev/null +++ b/changelog/2124.bugfix @@ -0,0 +1 @@ +``pytest_fixture_setup`` and ``pytest_fixture_post_finalizer`` hooks are now called for all ``conftest.py`` files. diff --git a/changelog/2124.feature b/changelog/2124.feature new file mode 100644 index 000000000..267fdabc9 --- /dev/null +++ b/changelog/2124.feature @@ -0,0 +1 @@ +``pytest_fixture_post_finalizer`` hook can now receive a ``request`` argument. From bdad345f991cae76896476a1b574269b23085bc3 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 12 Nov 2017 11:28:26 -0200 Subject: [PATCH 09/11] Fix passing request to finish() in FixtureDef --- _pytest/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index f1374b20f..74efd309d 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -785,7 +785,7 @@ class FixtureDef: 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") hook = self._fixturemanager.session.gethookproxy(request.node.fspath) From 6d3fe0b826c2aacf1eec573ed0fd50eb8774ee77 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 12 Nov 2017 11:28:57 -0200 Subject: [PATCH 10/11] Explicitly clear finalizers list in finalize to ensure cleanup --- _pytest/fixtures.py | 1 + 1 file changed, 1 insertion(+) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 74efd309d..ceae79cb4 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -765,6 +765,7 @@ class FixtureDef: # keep instances alive if hasattr(self, "cached_result"): del self.cached_result + self._finalizer = [] def execute(self, request): # get required arguments and register our own finish() From a6f2d2d2c958d743813414e46041adf3e47fb997 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 12 Nov 2017 11:35:46 -0200 Subject: [PATCH 11/11] Rename FixtureDef.finalizer to FixtureDef.finalizers --- _pytest/fixtures.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index ceae79cb4..19b977224 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -1,14 +1,15 @@ from __future__ import absolute_import, division, print_function +import functools import inspect import sys import warnings +from collections import OrderedDict -import functools +import attr import py from py._code.code import FormattedExcinfo -import attr import _pytest from _pytest import nodes from _pytest._code.code import TerminalRepr @@ -23,9 +24,6 @@ from _pytest.compat import ( from _pytest.outcomes import fail, TEST_OUTCOME -from collections import OrderedDict - - def pytest_sessionstart(session): import _pytest.python @@ -737,17 +735,17 @@ class FixtureDef: 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, 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()) @@ -765,7 +763,7 @@ class FixtureDef: # keep instances alive if hasattr(self, "cached_result"): del self.cached_result - self._finalizer = [] + self._finalizers = [] def execute(self, request): # get required arguments and register our own finish()