python: fix instance handling in static and class method tests

and also fixes a regression in pytest 8.0.0 where `setup_method` crashes
if the class has static or class method tests.

It is allowed to have a test class with static/class methods which
request non-static/class method fixtures (including `setup_method`
xunit-fixture). I take it as a given that we need to support this
somewhat odd scenario (stdlib unittest also supports it).

This raises a question -- when a staticmethod test requests a bound
fixture, what is that fixture's `self`?

stdlib unittest says - a fresh instance for the test.

Previously, pytest said - some instance that is shared by all
static/class methods. This is definitely broken since it breaks test
isolation.

Change pytest to behave like stdlib unittest here.

In practice, this means stopping to rely on `self.obj.__self__` to get
to the instance from the test function's binding. This doesn't work
because staticmethods are not bound to anything.

Instead, keep the instance explicitly and use that.

BTW, I think this will allow us to change `Class`'s fixture collection
(`parsefactories`) to happen on the class itself instead of a class
instance, allowing us to avoid one class instantiation. But needs more
work.

Fixes #12065.
This commit is contained in:
Ran Benita 2024-03-09 09:08:44 +02:00
parent 774f0c44e6
commit 0dc0360351
6 changed files with 100 additions and 15 deletions

View File

@ -0,0 +1,4 @@
Fixed a regression in pytest 8.0.0 where test classes containing ``setup_method`` and tests using ``@staticmethod`` or ``@classmethod`` would crash with ``AttributeError: 'NoneType' object has no attribute 'setup_method'``.
Now the :attr:`request.instance <pytest.FixtureRequest.instance>` attribute of tests using ``@staticmethod`` and ``@classmethod`` is no longer ``None``, but a fresh instance of the class, like in non-static methods.
Previously it was ``None``, and all fixtures of such tests would share a single ``self``.

View File

@ -470,8 +470,9 @@ class FixtureRequest(abc.ABC):
@property @property
def instance(self): def instance(self):
"""Instance (can be None) on which test function was collected.""" """Instance (can be None) on which test function was collected."""
function = getattr(self, "function", None) if self.scope != "function":
return getattr(function, "__self__", None) return None
return getattr(self._pyfuncitem, "instance", None)
@property @property
def module(self): def module(self):

View File

@ -302,10 +302,10 @@ class PyobjMixin(nodes.Node):
"""Python instance object the function is bound to. """Python instance object the function is bound to.
Returns None if not a test method, e.g. for a standalone test function, Returns None if not a test method, e.g. for a standalone test function,
a staticmethod, a class or a module. a class or a module.
""" """
node = self.getparent(Function) # Overridden by Function.
return getattr(node.obj, "__self__", None) if node is not None else None return None
@property @property
def obj(self): def obj(self):
@ -1702,7 +1702,8 @@ class Function(PyobjMixin, nodes.Item):
super().__init__(name, parent, config=config, session=session) super().__init__(name, parent, config=config, session=session)
if callobj is not NOTSET: if callobj is not NOTSET:
self.obj = callobj self._obj = callobj
self._instance = getattr(callobj, "__self__", None)
#: Original function name, without any decorations (for example #: Original function name, without any decorations (for example
#: parametrization adds a ``"[...]"`` suffix to function names), used to access #: parametrization adds a ``"[...]"`` suffix to function names), used to access
@ -1752,12 +1753,31 @@ class Function(PyobjMixin, nodes.Item):
"""Underlying python 'function' object.""" """Underlying python 'function' object."""
return getimfunc(self.obj) return getimfunc(self.obj)
def _getobj(self): @property
assert self.parent is not None def instance(self):
try:
return self._instance
except AttributeError:
if isinstance(self.parent, Class): if isinstance(self.parent, Class):
# Each Function gets a fresh class instance. # Each Function gets a fresh class instance.
parent_obj = self.parent.newinstance() self._instance = self._getinstance()
else: else:
self._instance = None
return self._instance
def _getinstance(self):
if isinstance(self.parent, Class):
# Each Function gets a fresh class instance.
return self.parent.newinstance()
else:
return None
def _getobj(self):
instance = self.instance
if instance is not None:
parent_obj = instance
else:
assert self.parent is not None
parent_obj = self.parent.obj # type: ignore[attr-defined] parent_obj = self.parent.obj # type: ignore[attr-defined]
return getattr(parent_obj, self.originalname) return getattr(parent_obj, self.originalname)

View File

@ -177,16 +177,15 @@ class TestCaseFunction(Function):
nofuncargs = True nofuncargs = True
_excinfo: Optional[List[_pytest._code.ExceptionInfo[BaseException]]] = None _excinfo: Optional[List[_pytest._code.ExceptionInfo[BaseException]]] = None
def _getobj(self): def _getinstance(self):
assert isinstance(self.parent, UnitTestCase) assert isinstance(self.parent, UnitTestCase)
testcase = self.parent.obj(self.name) return self.parent.obj(self.name)
return getattr(testcase, self.name)
# Backward compat for pytest-django; can be removed after pytest-django # Backward compat for pytest-django; can be removed after pytest-django
# updates + some slack. # updates + some slack.
@property @property
def _testcase(self): def _testcase(self):
return self._obj.__self__ return self.instance
def setup(self) -> None: def setup(self) -> None:
# A bound method to be called during teardown() if set (see 'runtest()'). # A bound method to be called during teardown() if set (see 'runtest()').
@ -296,7 +295,8 @@ class TestCaseFunction(Function):
def runtest(self) -> None: def runtest(self) -> None:
from _pytest.debugging import maybe_wrap_pytest_function_for_tracing from _pytest.debugging import maybe_wrap_pytest_function_for_tracing
testcase = self.obj.__self__ testcase = self.instance
assert testcase is not None
maybe_wrap_pytest_function_for_tracing(self) maybe_wrap_pytest_function_for_tracing(self)

View File

@ -4577,3 +4577,48 @@ def test_deduplicate_names() -> None:
assert items == ("a", "b", "c", "d") assert items == ("a", "b", "c", "d")
items = deduplicate_names((*items, "g", "f", "g", "e", "b")) items = deduplicate_names((*items, "g", "f", "g", "e", "b"))
assert items == ("a", "b", "c", "d", "g", "f", "e") assert items == ("a", "b", "c", "d", "g", "f", "e")
def test_staticmethod_classmethod_fixture_instance(pytester: Pytester) -> None:
"""Ensure that static and class methods get and have access to a fresh
instance.
This also ensures `setup_method` works well with static and class methods.
Regression test for #12065.
"""
pytester.makepyfile(
"""
import pytest
class Test:
ran_setup_method = False
ran_fixture = False
def setup_method(self):
assert not self.ran_setup_method
self.ran_setup_method = True
@pytest.fixture(autouse=True)
def fixture(self):
assert not self.ran_fixture
self.ran_fixture = True
def test_method(self):
assert self.ran_setup_method
assert self.ran_fixture
@staticmethod
def test_1(request):
assert request.instance.ran_setup_method
assert request.instance.ran_fixture
@classmethod
def test_2(cls, request):
assert request.instance.ran_setup_method
assert request.instance.ran_fixture
"""
)
result = pytester.runpytest()
assert result.ret == ExitCode.OK
result.assert_outcomes(passed=3)

View File

@ -410,22 +410,37 @@ def test_function_instance(pytester: Pytester) -> None:
items = pytester.getitems( items = pytester.getitems(
""" """
def test_func(): pass def test_func(): pass
class TestIt: class TestIt:
def test_method(self): pass def test_method(self): pass
@classmethod @classmethod
def test_class(cls): pass def test_class(cls): pass
@staticmethod @staticmethod
def test_static(): pass def test_static(): pass
""" """
) )
assert len(items) == 4 assert len(items) == 4
assert isinstance(items[0], Function) assert isinstance(items[0], Function)
assert items[0].name == "test_func" assert items[0].name == "test_func"
assert items[0].instance is None assert items[0].instance is None
assert isinstance(items[1], Function) assert isinstance(items[1], Function)
assert items[1].name == "test_method" assert items[1].name == "test_method"
assert items[1].instance is not None assert items[1].instance is not None
assert items[1].instance.__class__.__name__ == "TestIt" assert items[1].instance.__class__.__name__ == "TestIt"
# Even class and static methods get an instance!
# This is the instance used for bound fixture methods, which
# class/staticmethod tests are perfectly able to request.
assert isinstance(items[2], Function)
assert items[2].name == "test_class"
assert items[2].instance is not None
assert isinstance(items[3], Function) assert isinstance(items[3], Function)
assert items[3].name == "test_static" assert items[3].name == "test_static"
assert items[3].instance is None assert items[3].instance is not None
assert items[1].instance is not items[2].instance is not items[3].instance