Skip to content

Commit 1a5e0eb

Browse files
committed
unittest: make obj work more like Function/Class
Previously, the `obj` of a `TestCaseFunction` (the unittest plugin item type) was the unbound method. This is unlike regular `Class` where the `obj` is a bound method to a fresh instance. This difference necessitated several special cases in in places outside of the unittest plugin, such as `FixtureDef` and `FixtureRequest`, and made things a bit harder to understand. Instead, match how the python plugin does it, including collecting fixtures from a fresh instance. The downside is that now this instance for fixture-collection is kept around in memory, but it's the same as `Class` so nothing new. Users should only initialize stuff in `setUp`/`setUpClass` and similar methods, and not in `__init__` which is generally off-limits in `TestCase` subclasses. I am not sure why there was a difference in the first place, though I will say the previous unittest approach is probably the preferable one, but first let's get consistency.
1 parent 03e5471 commit 1a5e0eb

File tree

5 files changed

+52
-70
lines changed

5 files changed

+52
-70
lines changed

src/_pytest/compat.py

+3-5
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ def getfuncargnames(
8686
function: Callable[..., object],
8787
*,
8888
name: str = "",
89-
is_method: bool = False,
9089
cls: type | None = None,
9190
) -> tuple[str, ...]:
9291
"""Return the names of a function's mandatory arguments.
@@ -97,9 +96,8 @@ def getfuncargnames(
9796
* Aren't bound with functools.partial.
9897
* Aren't replaced with mocks.
9998
100-
The is_method and cls arguments indicate that the function should
101-
be treated as a bound method even though it's not unless, only in
102-
the case of cls, the function is a static method.
99+
The cls arguments indicate that the function should be treated as a bound
100+
method even though it's not unless the function is a static method.
103101
104102
The name parameter should be the original name in which the function was collected.
105103
"""
@@ -137,7 +135,7 @@ def getfuncargnames(
137135
# If this function should be treated as a bound method even though
138136
# it's passed as an unbound method or function, remove the first
139137
# parameter name.
140-
if is_method or (
138+
if (
141139
# Not using `getattr` because we don't want to resolve the staticmethod.
142140
# Not using `cls.__dict__` because we want to check the entire MRO.
143141
cls

src/_pytest/fixtures.py

+17-40
Original file line numberDiff line numberDiff line change
@@ -462,12 +462,8 @@ def cls(self):
462462
@property
463463
def instance(self):
464464
"""Instance (can be None) on which test function was collected."""
465-
# unittest support hack, see _pytest.unittest.TestCaseFunction.
466-
try:
467-
return self._pyfuncitem._testcase # type: ignore[attr-defined]
468-
except AttributeError:
469-
function = getattr(self, "function", None)
470-
return getattr(function, "__self__", None)
465+
function = getattr(self, "function", None)
466+
return getattr(function, "__self__", None)
471467

472468
@property
473469
def module(self):
@@ -965,7 +961,6 @@ def __init__(
965961
func: "_FixtureFunc[FixtureValue]",
966962
scope: Union[Scope, _ScopeName, Callable[[str, Config], _ScopeName], None],
967963
params: Optional[Sequence[object]],
968-
unittest: bool = False,
969964
ids: Optional[
970965
Union[Tuple[Optional[object], ...], Callable[[Any], Optional[object]]]
971966
] = None,
@@ -1011,9 +1006,7 @@ def __init__(
10111006
# a parameter value.
10121007
self.ids: Final = ids
10131008
# The names requested by the fixtures.
1014-
self.argnames: Final = getfuncargnames(func, name=argname, is_method=unittest)
1015-
# Whether the fixture was collected from a unittest TestCase class.
1016-
self.unittest: Final = unittest
1009+
self.argnames: Final = getfuncargnames(func, name=argname)
10171010
# If the fixture was executed, the current value of the fixture.
10181011
# Can change if the fixture is executed with different parameters.
10191012
self.cached_result: Optional[_FixtureCachedResult[FixtureValue]] = None
@@ -1092,25 +1085,20 @@ def resolve_fixture_function(
10921085
"""Get the actual callable that can be called to obtain the fixture
10931086
value, dealing with unittest-specific instances and bound methods."""
10941087
fixturefunc = fixturedef.func
1095-
if fixturedef.unittest:
1096-
if request.instance is not None:
1097-
# Bind the unbound method to the TestCase instance.
1098-
fixturefunc = fixturedef.func.__get__(request.instance) # type: ignore[union-attr]
1099-
else:
1100-
# The fixture function needs to be bound to the actual
1101-
# request.instance so that code working with "fixturedef" behaves
1102-
# as expected.
1103-
if request.instance is not None:
1104-
# Handle the case where fixture is defined not in a test class, but some other class
1105-
# (for example a plugin class with a fixture), see #2270.
1106-
if hasattr(fixturefunc, "__self__") and not isinstance(
1107-
request.instance,
1108-
fixturefunc.__self__.__class__, # type: ignore[union-attr]
1109-
):
1110-
return fixturefunc
1111-
fixturefunc = getimfunc(fixturedef.func)
1112-
if fixturefunc != fixturedef.func:
1113-
fixturefunc = fixturefunc.__get__(request.instance) # type: ignore[union-attr]
1088+
# The fixture function needs to be bound to the actual
1089+
# request.instance so that code working with "fixturedef" behaves
1090+
# as expected.
1091+
if request.instance is not None:
1092+
# Handle the case where fixture is defined not in a test class, but some other class
1093+
# (for example a plugin class with a fixture), see #2270.
1094+
if hasattr(fixturefunc, "__self__") and not isinstance(
1095+
request.instance,
1096+
fixturefunc.__self__.__class__, # type: ignore[union-attr]
1097+
):
1098+
return fixturefunc
1099+
fixturefunc = getimfunc(fixturedef.func)
1100+
if fixturefunc != fixturedef.func:
1101+
fixturefunc = fixturefunc.__get__(request.instance) # type: ignore[union-attr]
11141102
return fixturefunc
11151103

11161104

@@ -1614,7 +1602,6 @@ def _register_fixture(
16141602
Union[Tuple[Optional[object], ...], Callable[[Any], Optional[object]]]
16151603
] = None,
16161604
autouse: bool = False,
1617-
unittest: bool = False,
16181605
) -> None:
16191606
"""Register a fixture
16201607
@@ -1635,8 +1622,6 @@ def _register_fixture(
16351622
The fixture's IDs.
16361623
:param autouse:
16371624
Whether this is an autouse fixture.
1638-
:param unittest:
1639-
Set this if this is a unittest fixture.
16401625
"""
16411626
fixture_def = FixtureDef(
16421627
config=self.config,
@@ -1645,7 +1630,6 @@ def _register_fixture(
16451630
func=func,
16461631
scope=scope,
16471632
params=params,
1648-
unittest=unittest,
16491633
ids=ids,
16501634
_ispytest=True,
16511635
)
@@ -1667,8 +1651,6 @@ def _register_fixture(
16671651
def parsefactories(
16681652
self,
16691653
node_or_obj: nodes.Node,
1670-
*,
1671-
unittest: bool = ...,
16721654
) -> None:
16731655
raise NotImplementedError()
16741656

@@ -1677,17 +1659,13 @@ def parsefactories(
16771659
self,
16781660
node_or_obj: object,
16791661
nodeid: Optional[str],
1680-
*,
1681-
unittest: bool = ...,
16821662
) -> None:
16831663
raise NotImplementedError()
16841664

16851665
def parsefactories(
16861666
self,
16871667
node_or_obj: Union[nodes.Node, object],
16881668
nodeid: Union[str, NotSetType, None] = NOTSET,
1689-
*,
1690-
unittest: bool = False,
16911669
) -> None:
16921670
"""Collect fixtures from a collection node or object.
16931671
@@ -1739,7 +1717,6 @@ def parsefactories(
17391717
func=func,
17401718
scope=marker.scope,
17411719
params=marker.params,
1742-
unittest=unittest,
17431720
ids=marker.ids,
17441721
autouse=marker.autouse,
17451722
)

src/_pytest/python.py

-1
Original file line numberDiff line numberDiff line change
@@ -1314,7 +1314,6 @@ def parametrize(
13141314
func=get_direct_param_fixture_func,
13151315
scope=scope_,
13161316
params=None,
1317-
unittest=False,
13181317
ids=None,
13191318
_ispytest=True,
13201319
)

src/_pytest/unittest.py

+27-23
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
from typing import Union
1616

1717
import _pytest._code
18-
from _pytest.compat import getimfunc
1918
from _pytest.compat import is_async_function
2019
from _pytest.config import hookimpl
2120
from _pytest.fixtures import FixtureRequest
@@ -63,6 +62,14 @@ class UnitTestCase(Class):
6362
# to declare that our children do not support funcargs.
6463
nofuncargs = True
6564

65+
def newinstance(self):
66+
# TestCase __init__ takes the method (test) name. The TestCase
67+
# constructor treats the name "runTest" as a special no-op, so it can be
68+
# used when a dummy instance is needed. While unittest.TestCase has a
69+
# default, some subclasses omit the default (#9610), so always supply
70+
# it.
71+
return self.obj("runTest")
72+
6673
def collect(self) -> Iterable[Union[Item, Collector]]:
6774
from unittest import TestLoader
6875

@@ -76,15 +83,15 @@ def collect(self) -> Iterable[Union[Item, Collector]]:
7683
self._register_unittest_setup_class_fixture(cls)
7784
self._register_setup_class_fixture()
7885

79-
self.session._fixturemanager.parsefactories(self, unittest=True)
86+
self.session._fixturemanager.parsefactories(self.newinstance(), self.nodeid)
87+
8088
loader = TestLoader()
8189
foundsomething = False
8290
for name in loader.getTestCaseNames(self.obj):
8391
x = getattr(self.obj, name)
8492
if not getattr(x, "__test__", True):
8593
continue
86-
funcobj = getimfunc(x)
87-
yield TestCaseFunction.from_parent(self, name=name, callobj=funcobj)
94+
yield TestCaseFunction.from_parent(self, name=name)
8895
foundsomething = True
8996

9097
if not foundsomething:
@@ -169,31 +176,28 @@ def unittest_setup_method_fixture(
169176
class TestCaseFunction(Function):
170177
nofuncargs = True
171178
_excinfo: Optional[List[_pytest._code.ExceptionInfo[BaseException]]] = None
172-
_testcase: Optional["unittest.TestCase"] = None
173179

174180
def _getobj(self):
175-
assert self.parent is not None
176-
# Unlike a regular Function in a Class, where `item.obj` returns
177-
# a *bound* method (attached to an instance), TestCaseFunction's
178-
# `obj` returns an *unbound* method (not attached to an instance).
179-
# This inconsistency is probably not desirable, but needs some
180-
# consideration before changing.
181-
return getattr(self.parent.obj, self.originalname) # type: ignore[attr-defined]
181+
assert isinstance(self.parent, UnitTestCase)
182+
testcase = self.parent.obj(self.name)
183+
return getattr(testcase, self.name)
184+
185+
# Backward compat for pytest-django; can be removed after pytest-django
186+
# updates + some slack.
187+
@property
188+
def _testcase(self):
189+
return self._obj.__self__
182190

183191
def setup(self) -> None:
184192
# A bound method to be called during teardown() if set (see 'runtest()').
185193
self._explicit_tearDown: Optional[Callable[[], None]] = None
186-
assert self.parent is not None
187-
self._testcase = self.parent.obj(self.name) # type: ignore[attr-defined]
188-
self._obj = getattr(self._testcase, self.name)
189194
super().setup()
190195

191196
def teardown(self) -> None:
192197
super().teardown()
193198
if self._explicit_tearDown is not None:
194199
self._explicit_tearDown()
195200
self._explicit_tearDown = None
196-
self._testcase = None
197201
self._obj = None
198202

199203
def startTest(self, testcase: "unittest.TestCase") -> None:
@@ -292,14 +296,14 @@ def addDuration(self, testcase: "unittest.TestCase", elapsed: float) -> None:
292296
def runtest(self) -> None:
293297
from _pytest.debugging import maybe_wrap_pytest_function_for_tracing
294298

295-
assert self._testcase is not None
299+
testcase = self.obj.__self__
296300

297301
maybe_wrap_pytest_function_for_tracing(self)
298302

299303
# Let the unittest framework handle async functions.
300304
if is_async_function(self.obj):
301305
# Type ignored because self acts as the TestResult, but is not actually one.
302-
self._testcase(result=self) # type: ignore[arg-type]
306+
testcase(result=self) # type: ignore[arg-type]
303307
else:
304308
# When --pdb is given, we want to postpone calling tearDown() otherwise
305309
# when entering the pdb prompt, tearDown() would have probably cleaned up
@@ -311,16 +315,16 @@ def runtest(self) -> None:
311315
assert isinstance(self.parent, UnitTestCase)
312316
skipped = _is_skipped(self.obj) or _is_skipped(self.parent.obj)
313317
if self.config.getoption("usepdb") and not skipped:
314-
self._explicit_tearDown = self._testcase.tearDown
315-
setattr(self._testcase, "tearDown", lambda *args: None)
318+
self._explicit_tearDown = testcase.tearDown
319+
setattr(testcase, "tearDown", lambda *args: None)
316320

317321
# We need to update the actual bound method with self.obj, because
318322
# wrap_pytest_function_for_tracing replaces self.obj by a wrapper.
319-
setattr(self._testcase, self.name, self.obj)
323+
setattr(testcase, self.name, self.obj)
320324
try:
321-
self._testcase(result=self) # type: ignore[arg-type]
325+
testcase(result=self) # type: ignore[arg-type]
322326
finally:
323-
delattr(self._testcase, self.name)
327+
delattr(testcase, self.name)
324328

325329
def _traceback_filter(
326330
self, excinfo: _pytest._code.ExceptionInfo[BaseException]

testing/test_unittest.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,14 @@ def test_demo(self):
208208
209209
"""
210210
)
211+
211212
pytester.inline_run("-s", testpath)
212213
gc.collect()
214+
215+
# Either already destroyed, or didn't run setUp.
213216
for obj in gc.get_objects():
214-
assert type(obj).__name__ != "TestCaseObjectsShouldBeCleanedUp"
217+
if type(obj).__name__ == "TestCaseObjectsShouldBeCleanedUp":
218+
assert not hasattr(obj, "an_expensive_obj")
215219

216220

217221
def test_unittest_skip_issue148(pytester: Pytester) -> None:

0 commit comments

Comments
 (0)