fixtures: say where a fixture is defined, when it is missing and when it is doubled

changelog/10151.improvement.rst

@@ -0,0 +1,6 @@
+When a fixture is not found, the error now points at definitions of that name
+which exist elsewhere in the collection tree but are not visible to the test.
+
+Moving a fixture into a conftest one directory over leaves the name registered
+but out of scope, and the previous message only listed the fixtures that *were*
+available, giving no sign that the fixture was there all along.

changelog/1511.improvement.rst

@@ -0,0 +1,13 @@
+pytest now warns with :class:`~pytest.PytestImportedFixtureWarning` when two
+modules register the same fixture function, which is what importing a fixture in
+order to reuse it does.
+
+The second registration is a separate :class:`~pytest.FixtureDef` for the same
+function, so a session-scoped fixture reached that way runs once per registering
+module, and the duplicate is invisible in the source of either file. The warning
+names both modules and points at the line that brought the fixture in.
+
+Importing a fixture from a module that is not itself a plugin or a collected
+module registers it only once, and is not warned about. Neither is inheriting a
+fixture from a base class in another module, nor building one with a factory
+defined elsewhere.

doc/en/reference/fixtures.rst

@@ -141,6 +141,19 @@ You can have multiple nested directories/packages containing your tests, and
 each directory can have its own ``conftest.py`` with its own fixtures, adding on
 to the ones provided by the ``conftest.py`` files in parent directories.
 
+.. warning::
+
+    Do not ``import`` a fixture in order to reuse it. Importing binds the
+    fixture in the importing module as well, so if the module it came from is
+    itself a plugin or a collected module, the fixture ends up registered
+    *twice*: a session-scoped fixture registered twice runs twice, and neither
+    file shows why. pytest emits
+    :class:`~pytest.PytestImportedFixtureWarning` when two modules register the
+    same fixture function.
+
+    Reach the fixture through a ``conftest.py``, or make its module a plugin by
+    listing it in :globalvar:`pytest_plugins`, and drop the import.
+
 For example, given a test file structure like this:
 
 ::

doc/en/reference/reference.rst

@@ -1311,6 +1311,9 @@ Custom warnings generated in some situations such as improper usage or deprecate
 .. autoclass:: pytest.PytestExperimentalApiWarning
    :show-inheritance:
 
+.. autoclass:: pytest.PytestImportedFixtureWarning
+   :show-inheritance:
+
 .. autoclass:: pytest.PytestReturnNotNoneWarning
   :show-inheritance:
 

src/_pytest/fixtures.py

@@ -75,6 +75,7 @@
 from _pytest.scope import HIGH_SCOPES
 from _pytest.scope import Scope
 from _pytest.scope import ScopeName
+from _pytest.warning_types import PytestImportedFixtureWarning
 from _pytest.warning_types import PytestWarning
 from _pytest.warning_types import warn_explicit_for
 
@@ -958,6 +959,10 @@ def addfinalizer(self, finalizer: Callable[[], object]) -> None:
         self._fixturedef.addfinalizer(finalizer)
 
 
+#: Definitions listed in a fixture-not-found hint before it collapses to a count.
+_MAX_HINTED_DEFINITIONS = 5
+
+
 @final
 class FixtureLookupError(LookupError):
     """Could not return a requested fixture (missing or invalid)."""
@@ -1008,11 +1013,52 @@ def formatrepr(self) -> FixtureLookupErrorRepr:
                 )
             else:
                 msg = f"fixture '{self.argname}' not found"
+                hint = self._out_of_scope_definitions_hint()
+                if hint is not None:
+                    msg += f"\n {hint}"
             msg += "\n available fixtures: {}".format(", ".join(sorted(available)))
             msg += "\n use 'pytest --fixtures [testpath]' for help on them."
 
         return FixtureLookupErrorRepr(fspath, lineno, tblines, msg, self.argname)
 
+    def _out_of_scope_definitions_hint(self) -> str | None:
+        """Describe definitions of the missing name that exist but are out of scope.
+
+        A fixture that was moved to a sibling conftest is still registered, just
+        not visible from here, and a bare "not found" sends the reader looking
+        for a name that is in front of them. See #10151.
+
+        Only called once the name is known to be invisible to the requesting
+        node, so every definition found here is by construction out of scope.
+        """
+        assert self.argname is not None
+        fm = self.request._fixturemanager
+        invocation_dir = self.request._pyfuncitem.config.invocation_params.dir
+        locations = set()
+        for fixturedef in fm._get_all_fixture_defs_for_name(self.argname):
+            try:
+                locations.add(_pretty_fixture_path(invocation_dir, fixturedef.func))
+            except (AttributeError, TypeError):
+                # inspect.getfile() and __code__ are what can fail here. A
+                # fixture with no locatable source is worse than no hint, so
+                # drop that one rather than the whole hint.
+                continue
+        if not locations:
+            return None
+        shown = sorted(locations)[:_MAX_HINTED_DEFINITIONS]
+        hidden = len(locations) - len(shown)
+        places = ", ".join(shown)
+        if hidden:
+            places += f", and {hidden} more"
+        if len(locations) == 1:
+            return (
+                f"hint: '{self.argname}' is defined in {places}, but not visible here"
+            )
+        return (
+            f"hint: '{self.argname}' is defined in {len(locations)} places, "
+            f"none visible here: {places}"
+        )
+
 
 class FixtureLookupErrorRepr(TerminalRepr):
     def __init__(
@@ -1752,6 +1798,40 @@ def deduplicate_names(*seqs: Iterable[str]) -> tuple[str, ...]:
     return tuple(dict.fromkeys(name for seq in seqs for name in seq))
 
 
+def _warn_duplicate_fixture(
+    holderobj: types.ModuleType,
+    fixture_name: str,
+    func: Callable[..., object],
+    first: types.ModuleType,
+) -> None:
+    origin = getattr(func, "__module__", None) or "another module"
+    message = PytestImportedFixtureWarning(
+        f"fixture '{fixture_name}', defined in '{origin}', is registered twice: "
+        f"by '{first.__name__}' and by '{holderobj.__name__}'.\n"
+        f"Importing a fixture registers another copy of it, so it can run more "
+        f"than once and shadow other definitions of the same name.\n"
+        f"Drop the import and reach it through a conftest.py, or list "
+        f"'{origin}' in 'pytest_plugins'."
+    )
+    # Nothing records which line bound the name -- that is lost once the module
+    # is imported -- and the message already names both modules, so the file is
+    # the anchor. A module synthesised at runtime has no __file__; name it
+    # rather than drop the anchor entirely.
+    filename = getattr(holderobj, "__file__", None) or f"<{holderobj.__name__}>"
+    try:
+        warnings.warn_explicit(
+            message,
+            PytestImportedFixtureWarning,
+            filename=filename,
+            module=holderobj.__name__,
+            registry=holderobj.__dict__.setdefault("__warningregistry__", {}),
+            lineno=1,
+        )
+    except Warning as w:
+        # Under -W error the location is dropped, so carry it in the message.
+        raise type(w)(f"{w}\n at {filename}") from None
+
+
 class FixtureManager:
     """pytest fixture definitions and information is stored and managed
     from this class.
@@ -1801,6 +1881,13 @@ def __init__(self, session: Session) -> None:
         self._arg2nodeid2fixturedefs: Final[
             dict[str, dict[str, list[FixtureDef[Any]]]]
         ] = {}
+        # The first module holder each fixture function was registered from,
+        # keyed by id() and holding the function to keep that id valid.
+        # A second module registering the same function is importing it, which
+        # registers a duplicate -- see #1511.
+        self._fixturefunc2module: Final[
+            dict[int, tuple[types.ModuleType, Callable[..., object]]]
+        ] = {}
         # A mapping from a node to a list of autouse fixture names it defines.
         # The Session entry holds global usefixtures from config.
         self._node_autousenames: Final[dict[nodes.Node, list[str]]] = {
@@ -2329,6 +2416,16 @@ def parsefactories(
 
                 func = obj._get_wrapped_function()
 
+                # Only module holders: a class legitimately inherits fixtures
+                # from a base in another module, registering the same function
+                # once per subclass, and that is not a duplicate.
+                if isinstance(holderobj, types.ModuleType):
+                    first = self._fixturefunc2module.setdefault(
+                        id(func), (holderobj, func)
+                    )[0]
+                    if first is not holderobj:
+                        _warn_duplicate_fixture(holderobj, fixture_name, func, first)
+
                 self._register_fixture(
                     name=fixture_name,
                     func=func,
@@ -2352,6 +2449,16 @@ def _get_all_fixture_defs(self) -> Iterable[FixtureDef[Any]]:
             for fixturedefs in nodeid2fixturedefs.values():
                 yield from fixturedefs
 
+    def _get_all_fixture_defs_for_name(self, argname: str) -> Iterable[FixtureDef[Any]]:
+        """Get all FixtureDefs registered under a name, whatever their visibility.
+
+        The order is not guaranteed.
+        """
+        for fixturedefs in self._arg2node2fixturedefs.get(argname, {}).values():
+            yield from fixturedefs
+        for fixturedefs in self._arg2nodeid2fixturedefs.get(argname, {}).values():
+            yield from fixturedefs
+
     def _get_all_fixture_defs_for_node(
         self, node: nodes.Node
     ) -> Iterable[FixtureDef[Any]]:

src/_pytest/warning_types.py

@@ -82,6 +82,17 @@ class PytestReturnNotNoneWarning(PytestWarning):
     __module__ = "pytest"
 
 
+@final
+class PytestImportedFixtureWarning(PytestWarning):
+    """Warning emitted when two modules register the same fixture function,
+    which happens when one of them imported it from the other.
+
+    See :ref:`conftest` for details.
+    """
+
+    __module__ = "pytest"
+
+
 @final
 class PytestUnknownMarkWarning(PytestWarning):
     """Warning emitted on use of unknown markers.

src/pytest/__init__.py

@@ -87,6 +87,7 @@
 from _pytest.warning_types import PytestDeprecationWarning
 from _pytest.warning_types import PytestExperimentalApiWarning
 from _pytest.warning_types import PytestFDWarning
+from _pytest.warning_types import PytestImportedFixtureWarning
 from _pytest.warning_types import PytestRemovedIn10Warning
 from _pytest.warning_types import PytestReturnNotNoneWarning
 from _pytest.warning_types import PytestUnhandledThreadExceptionWarning
@@ -139,6 +140,7 @@
     "PytestDeprecationWarning",
     "PytestExperimentalApiWarning",
     "PytestFDWarning",
+    "PytestImportedFixtureWarning",
     "PytestPluginManager",
     "PytestRemovedIn10Warning",
     "PytestReturnNotNoneWarning",