diff --git a/changelog/14770.misc.rst b/changelog/14770.misc.rst new file mode 100644 index 00000000000..2d3d8ac9165 --- /dev/null +++ b/changelog/14770.misc.rst @@ -0,0 +1,3 @@ +Moved `FixtureDef.cached_result` to `SetupState`. + +This is part of the effort to add thread safety in pytest. diff --git a/pyproject.toml b/pyproject.toml index a0e2c40f4d4..0489043d714 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -480,7 +480,7 @@ exclude_lines = [ '^\s*assert False(,|$)', '^\s*case unreachable:', '^\s*assert_never\(', - '^\s*if TYPE_CHECKING:', + '^\s*if TYPE_CHECKING\s*.*', '^\s*(el)?if TYPE_CHECKING:', '^\s*@overload( |$)', '^\s*def .+: \.\.\.$', diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 3f1964f2d06..02be13d79ac 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -26,6 +26,7 @@ from typing import final from typing import Generic from typing import Literal +from typing import NamedTuple from typing import NoReturn from typing import overload from typing import TYPE_CHECKING @@ -96,23 +97,32 @@ FixtureFunction = Callable[..., object] # The type of a fixture function (type alias generic in fixture value). _FixtureFunc = Callable[..., FixtureValue] | Callable[..., Generator[FixtureValue]] -# The type of FixtureDef.cached_result (type alias generic in fixture value). -_FixtureCachedResult = ( - tuple[ - # The result. - FixtureValue, - # Cache key. - object, - None, - ] - | tuple[ - None, - # Cache key. - object, - # The exception and the original traceback. - tuple[BaseException, types.TracebackType | None], - ] -) +# Sentinel value for unset fixture params +_NO_PARAM = object() + + +# NamedTuples cannot take generic arguments before Python 3.11 +if TYPE_CHECKING and sys.version_info >= (3, 11): + + class _FixtureResult(NamedTuple, Generic[FixtureValue]): + value: FixtureValue + param: object + exception_and_traceback: None +else: + + class _FixtureResult(NamedTuple): + value: Any + param: object + exception_and_traceback: None + + +class _FixtureException(NamedTuple): + value: None + param: object + exception_and_traceback: tuple[BaseException, types.TracebackType | None] + + +_FixtureCachedResult = _FixtureResult[FixtureValue] | _FixtureException # type: ignore[type-arg] def pytest_sessionstart(session: Session) -> None: @@ -629,11 +639,44 @@ def getfixturevalue(self, argname: str) -> Any: # (using function parameters, autouse, etc). fixturedef = self._get_active_fixturedef(argname) - assert fixturedef.cached_result is not None, ( + fixture_result = self._get_cached_result(fixturedef) + assert fixture_result is not None, ( f'The fixture value for "{argname}" is not available. ' "This can happen when the fixture has already been torn down." ) - return fixturedef.cached_result[0] + return fixture_result.value + + def _get_cached_result( + self, fixturedef: FixtureDef[FixtureValue] + ) -> _FixtureCachedResult[FixtureValue] | None: + return self.session._setupstate.fixture_cache.get(fixturedef) + + def _cache_value( + self, + fixturedef: FixtureDef[FixtureValue], + value: FixtureValue, # type: ignore[misc] + ) -> None: + """Write a value into the cache for a fixture definition.""" + self.session._setupstate.fixture_cache[fixturedef] = _FixtureResult( + value, self._active_param, None + ) + + def _cache_exception( + self, + fixturedef: FixtureDef[FixtureValue], + exception: BaseException, + ) -> None: + """Write an exception result into the cache for a fixture definition.""" + self.session._setupstate.fixture_cache[fixturedef] = _FixtureException( + None, self._active_param, (exception, exception.__traceback__) + ) + + @property + def _active_param(self) -> object: + return getattr(self, "param", _NO_PARAM) + + def _invalidate_fixture_cache(self, fixturedef: FixtureDef[FixtureValue]) -> None: + del self.session._setupstate.fixture_cache[fixturedef] def _iter_chain(self) -> Iterator[SubRequest]: """Yield all SubRequests in the chain, from self up. @@ -1123,9 +1166,6 @@ def __init__( self.ids: Final = ids # The names requested by the fixtures. self.argnames: Final = getfuncargnames(func, name=argname) - # If the fixture was executed, the current value of the fixture. - # Can change if the fixture is executed with different parameters. - self.cached_result: _FixtureCachedResult[FixtureValue] | None = None self._finalizers: Final[list[Callable[[], object]]] = [] # only used to emit a deprecationwarning, can be removed in pytest9 @@ -1145,7 +1185,7 @@ def addfinalizer(self, finalizer: Callable[[], object]) -> None: self._finalizers.append(finalizer) def finish(self, request: SubRequest) -> None: - if self.cached_result is None: + if request._get_cached_result(self) is None: # Already finished. It is assumed that finalizers cannot be added in # this state. return @@ -1161,7 +1201,7 @@ def finish(self, request: SubRequest) -> None: # Even if finalization fails, we invalidate the cached fixture # value and remove all finalizers because they may be bound methods # which will keep instances alive. - self.cached_result = None + request._invalidate_fixture_cache(self) self._finalizers.clear() if len(exceptions) == 1: raise exceptions[0] @@ -1175,7 +1215,8 @@ def execute(self, request: SubRequest) -> FixtureValue: # This needs to be done before checking if we have a cached value, since # if a dependent fixture has their cache invalidated, e.g. due to # parametrization, they finalize themselves and fixtures depending on it - # (which will likely include this fixture) setting `self.cached_result = None`. + # (which will likely include this fixture), invalidating the respective + # cached values. # See #4871 requested_fixtures_that_should_finalize_us = [] for argname in self.argnames: @@ -1188,27 +1229,27 @@ def execute(self, request: SubRequest) -> FixtureValue: requested_fixtures_that_should_finalize_us.append(fixturedef) # Check for (and return) cached value/exception. - if self.cached_result is not None: - request_cache_key = self.cache_key(request) - cache_key = self.cached_result[1] + if (cached_result := request._get_cached_result(self)) is not None: + active_param = request._active_param + cached_param = cached_result.param try: # Attempt to make a normal == check: this might fail for objects # which do not implement the standard comparison (like numpy arrays -- #6497). - cache_hit = bool(request_cache_key == cache_key) + cache_hit = bool(active_param == cached_param) except (ValueError, RuntimeError): # If the comparison raises, use 'is' as fallback. - cache_hit = request_cache_key is cache_key + cache_hit = active_param is cached_param if cache_hit: - if self.cached_result[2] is not None: - exc, exc_tb = self.cached_result[2] + if cached_result.exception_and_traceback is not None: + exc, exc_tb = cached_result.exception_and_traceback raise exc.with_traceback(exc_tb) else: - return self.cached_result[0] + return cached_result.value # type: ignore[no-any-return] # We have a previous but differently parametrized fixture instance # so we need to tear it down before creating a new one. self.finish(request) - assert self.cached_result is None + assert request._get_cached_result(self) is None # Add finalizer to requested fixtures we saved previously. # We make sure to do this after checking for cached value to avoid @@ -1229,7 +1270,6 @@ def execute(self, request: SubRequest) -> FixtureValue: ihook = request.node.ihook try: # Setup the fixture, run the code in it, and cache the value - # in self.cached_result. result: FixtureValue = ihook.pytest_fixture_setup( fixturedef=self, request=request ) @@ -1239,9 +1279,6 @@ def execute(self, request: SubRequest) -> FixtureValue: return result - def cache_key(self, request: SubRequest) -> object: - return getattr(request, "param", None) - def __repr__(self) -> str: return f"" @@ -1263,7 +1300,7 @@ def __init__(self, request: FixtureRequest) -> None: node=request.node, _ispytest=True, ) - self.cached_result = (request, [0], None) + request._cache_value(self, request) def addfinalizer(self, finalizer: Callable[[], object]) -> None: pass @@ -1321,7 +1358,6 @@ def pytest_fixture_setup( kwargs[argname] = request.getfixturevalue(argname) fixturefunc = resolve_fixture_function(fixturedef, request) - my_cache_key = fixturedef.cache_key(request) if inspect.isasyncgenfunction(fixturefunc) or inspect.iscoroutinefunction( fixturefunc @@ -1342,9 +1378,12 @@ def pytest_fixture_setup( # Don't show the fixture as the skip location, as then the user # wouldn't know which test skipped. e._use_item_location = True - fixturedef.cached_result = (None, my_cache_key, (e, e.__traceback__)) + request._cache_exception( + fixturedef, + e, + ) raise - fixturedef.cached_result = (result, my_cache_key, None) + request._cache_value(fixturedef, result) return result diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index ba7ac2b76e3..104a8d7dbe8 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -867,8 +867,8 @@ def pytest_fixture_post_finalizer( fixturedef: FixtureDef[Any], request: SubRequest ) -> None: """Called after fixture teardown, but before the cache is cleared, so - the fixture result ``fixturedef.cached_result`` is still available (not - ``None``). + the fixture result ``request._get_cached_result(fixturedef)`` is still + available (not ``None``). :param fixturedef: The fixture definition object. diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index 3f03cfaff77..799fe8e3dcf 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -511,6 +511,9 @@ def __init__(self) -> None: tuple[OutcomeException | Exception, types.TracebackType | None] | None, ], ] = {} + # Importing the appropriate types from the fixtures module lead to circular + # imports, so we leave the cache untyped for now + self.fixture_cache = {} # type: ignore[var-annotated] def is_node_active(self, node: Node) -> bool: """Check if a node is currently active in the stack -- set up and not diff --git a/src/_pytest/setuponly.py b/src/_pytest/setuponly.py index 7e6b46bcdb4..e8c34c1d49a 100644 --- a/src/_pytest/setuponly.py +++ b/src/_pytest/setuponly.py @@ -6,6 +6,8 @@ from _pytest.config import Config from _pytest.config import ExitCode from _pytest.config.argparsing import Parser +from _pytest.fixtures import _FixtureResult +from _pytest.fixtures import _NO_PARAM from _pytest.fixtures import FixtureDef from _pytest.fixtures import SubRequest from _pytest.scope import Scope @@ -46,23 +48,30 @@ def pytest_fixture_setup( param = fixturedef.ids[request.param_index] else: param = request.param - fixturedef.cached_param = param # type: ignore[attr-defined] - _show_fixture_action(fixturedef, request.config, "SETUP") + # Use None as a dummy value for resolving/caching the fixture + # --setup-show does not care about the actual value, only about the param + request.session._setupstate.fixture_cache[fixturedef] = _FixtureResult( + None, param, None + ) + else: + param = _NO_PARAM + _show_fixture_action(request.config, fixturedef, param, "SETUP") def pytest_fixture_post_finalizer( fixturedef: FixtureDef[object], request: SubRequest ) -> None: - if fixturedef.cached_result is not None: - config = request.config - if config.option.setupshow: - _show_fixture_action(fixturedef, request.config, "TEARDOWN") - if hasattr(fixturedef, "cached_param"): - del fixturedef.cached_param + cached_result = request._get_cached_result(fixturedef) + assert cached_result is not None, ( + "As per the definition of this hook the fixture cache should not have been cleared" + ) + config = request.config + if config.option.setupshow: + _show_fixture_action(config, fixturedef, cached_result.param, "TEARDOWN") def _show_fixture_action( - fixturedef: FixtureDef[object], config: Config, msg: str + config: Config, fixturedef: FixtureDef[object], param: object, msg: str ) -> None: capman = config.pluginmanager.getplugin("capturemanager") if capman: @@ -82,8 +91,8 @@ def _show_fixture_action( if deps: tw.write(" (fixtures used: {})".format(", ".join(deps))) - if hasattr(fixturedef, "cached_param"): - tw.write(f"[{saferepr(fixturedef.cached_param, maxsize=42)}]") + if param is not _NO_PARAM: + tw.write(f"[{saferepr(param, maxsize=42)}]") tw.flush() diff --git a/src/_pytest/setupplan.py b/src/_pytest/setupplan.py index 4e124cce243..156af485a3d 100644 --- a/src/_pytest/setupplan.py +++ b/src/_pytest/setupplan.py @@ -25,9 +25,8 @@ def pytest_fixture_setup( ) -> object | None: # Will return a dummy fixture if the setuponly option is provided. if request.config.option.setupplan: - my_cache_key = fixturedef.cache_key(request) - fixturedef.cached_result = (None, my_cache_key, None) - return fixturedef.cached_result + request._cache_value(fixturedef, None) + return request._get_cached_result(fixturedef) return None diff --git a/testing/test_setuponly.py b/testing/test_setuponly.py index 1d2261f75a8..b9c39668013 100644 --- a/testing/test_setuponly.py +++ b/testing/test_setuponly.py @@ -259,6 +259,25 @@ def test_capturing(two): ) +def test_suppress_capturing(pytester: Pytester) -> None: + p = pytester.makepyfile( + """ + import pytest, sys + @pytest.fixture() + def one(): + sys.stdout.write('this should not be captured') + @pytest.fixture() + def two(one): + assert 0 + def test_capturing(two): + pass + """ + ) + + result = pytester.runpytest("--setup-only", "-s", p) + result.stdout.no_fnmatch_line("this should not be captured") + + def test_show_fixtures_and_execute_test(pytester: Pytester) -> None: """Verify that setups are shown and tests are executed.""" p = pytester.makepyfile(