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

Code before:

    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]

            # First attempt to use 'is' for performance reasons (for example numpy arrays (#6497)).
            cache_hit = my_cache_key is cache_key
            if not cache_hit:
                # If they are not the same, fallback to a bool comparison (#12600).
                try:
                    cache_hit = bool(my_cache_key == cache_key)
                except (ValueError, RuntimeError):
                    cache_hit = False

            if cache_hit:
                if self.cached_result[2] is not None:
                    exc, exc_tb = self.cached_result[2]
                    raise exc.with_traceback(exc_tb)
                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

        # Add finalizer to requested fixtures we saved previously.
        # We make sure to do this after checking for cached value to avoid
        # adding our finalizer multiple times. (#12135)
        finalizer = functools.partial(self.finish, request=request)
        for parent_fixture in requested_fixtures_that_should_finalize_us:

Review comment (suggestion):
I think this actually muddies the semantic water somewhat. It is possible for an object to not equal itself (i.e. `obj is obj` is true, but `obj == obj` is false). Even though that's an odd thing for an object to do, it does indicate to me that we shouldn't do it. If we go with `==` I prefer we go do it the whole way without the `is` optimization. (The optimization wouldn't help avoid the pathological case anyway since a miss is assured when the param changes).

The `is` should be done as a fallback when `==` doesn't work.

Code after:

    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.
        if self.cached_result is not None:
            request_cache_key = self.cache_key(request)
            cache_key = self.cached_result[1]
            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)
            except (ValueError, RuntimeError):
                # If the comparison raises, use 'is' as fallback.
                cache_hit = request_cache_key is cache_key

            if cache_hit:
                if self.cached_result[2] is not None:
                    exc, exc_tb = self.cached_result[2]
                    raise exc.with_traceback(exc_tb)
                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

        # Add finalizer to requested fixtures we saved previously.
        # We make sure to do this after checking for cached value to avoid
        # adding our finalizer multiple times. (#12135)
        finalizer = functools.partial(self.finish, request=request)
        for parent_fixture in requested_fixtures_that_should_finalize_us:
            parent_fixture.addfinalizer(finalizer)