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 1 commit
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
8 changes: 4 additions & 4 deletions _pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -754,8 +754,8 @@ def finish(self):
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"):
Expand Down Expand Up @@ -783,8 +783,8 @@ def execute(self, request):
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
Copy link
Member

Choose a reason for hiding this comment

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

as far as i understand, at this place, config.hook is an incorrect choice, as it does not apply fs path filtering

@hpk42 please cross-check my concern

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't then session.gethookproxy(fspath) be used here instead?

Copy link
Member

Choose a reason for hiding this comment

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

@nicoddemus i think so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicoddemus @RonnyPfannschmidt I disagree - session.gethookproxy(fspath) is specifically what I'm changing in this PR, because session.ihook does precisely session.gethookproxy(self.fspath) (https://github.com/pytest-dev/pytest/blob/master/_pytest/main.py#L267-L270) and it doesn't work.

Copy link
Member

Choose a reason for hiding this comment

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

Session.fspath is indeeed broken, you need the item fspath, which has a different value

return hook.pytest_fixture_setup(fixturedef=self, request=request)

def __repr__(self):
return ("<FixtureDef name=%r scope=%r baseid=%r >" %
Expand Down
45 changes: 45 additions & 0 deletions testing/python/fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -2984,4 +2984,49 @@ def test_foo(request):
""".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*",
])