Repository: pytest-dev/pytest
File: src/_pytest/fixtures.py (Python)

Code before:
        exceptions: List[BaseException] = []
        while self._finalizers:
            fin = self._finalizers.pop()
            try:
                fin()
            except BaseException as e:
                exceptions.append(e)
        node = request.node
        node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
        # 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
        self._finalizers.clear()
        if len(exceptions) == 1:
            raise exceptions[0]
        elif len(exceptions) > 1:
            msg = f'errors while tearing down fixture "{self.argname}" of {node}'
            raise BaseExceptionGroup(msg, exceptions[::-1])

    def execute(self, request: SubRequest) -> FixtureValue:
        # Ensure arguments (parent fixtures) are loaded.
        # If their cache has been invalidated it will finish itself and subfixtures,
        # which will set our self.cached_result = None.
        # If/when parent fixture parametrization is included in our cache key this
        # can be moved after checking our cache key and not require saving in a list.
        parent_fixtures_to_add_finalizer = []
        for argname in self.argnames:
            fixturedef = request._get_active_fixturedef(argname)
            if not isinstance(fixturedef, PseudoFixtureDef):
                # save fixture as one to add our finalizer to, if we're not cached
                # resolves #12135
                parent_fixtures_to_add_finalizer.append(fixturedef)

        my_cache_key = self.cache_key(request)
        if self.cached_result is not None:
            cache_key = self.cached_result[1]
            # note: comparison with `==` can fail (or be expensive) for e.g.
            # numpy arrays (#6497).
            if my_cache_key is cache_key:
                if self.cached_result[2] is not None:
                    exc = self.cached_result[2]
                    raise exc
                else:
                    result = self.cached_result[0]
                    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(request)
            assert self.cached_result is None


Review comment (performance):
- Let's avoid the term "subfixtures" since it's not a term we use and it's not very clear (to me at least), similar to "parent".
- I think the "if/when" comment is more of a wishlist item so better to keep it in the issue.

Suggested rewording:

```suggestion
        # Ensure fixtures that this fixture requests are loaded (XXX: figure out
        # why this is needed even in the cached case).
        #
        # Also make it such that whenever a fixture we request is torn down,
        # this fixture is torn down first. This is generally handled by
        # SetupState, but still currently needed when the requested fixture is
        # not parameterized itself but depends on a parameterized fixture (see
        # #4871 for more details).
        # Only do this in the non-cached case. It's not needed in the cached
        # case because the finalizers had already been registered.
```

Code after:
        exceptions: List[BaseException] = []
        while self._finalizers:
            fin = self._finalizers.pop()
            try:
                fin()
            except BaseException as e:
                exceptions.append(e)
        node = request.node
        node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
        # 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
        self._finalizers.clear()
        if len(exceptions) == 1:
            raise exceptions[0]
        elif len(exceptions) > 1:
            msg = f'errors while tearing down fixture "{self.argname}" of {node}'
            raise BaseExceptionGroup(msg, exceptions[::-1])

    def execute(self, request: SubRequest) -> FixtureValue:
        """Return the value of this fixture, executing it if not cached."""
        # Ensure that the dependent fixtures requested by this fixture are loaded.
        # 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`.
        # See #4871
        requested_fixtures_that_should_finalize_us = []
        for argname in self.argnames:
            fixturedef = request._get_active_fixturedef(argname)
            # Saves requested fixtures in a list so we later can add our finalizer
            # to them, ensuring that if a requested fixture gets torn down we get torn
            # down first. This is generally handled by SetupState, but still currently
            # needed when this fixture is not parametrized but depends on a parametrized
            # fixture.
            if not isinstance(fixturedef, PseudoFixtureDef):
                requested_fixtures_that_should_finalize_us.append(fixturedef)

        # Check for (and return) cached value/exception.
        my_cache_key = self.cache_key(request)
        if self.cached_result is not None:
            cache_key = self.cached_result[1]
            # note: comparison with `==` can fail (or be expensive) for e.g.
            # numpy arrays (#6497).
            if my_cache_key is cache_key:
                if self.cached_result[2] is not None:
                    exc = self.cached_result[2]
                    raise exc
                else:
                    result = self.cached_result[0]