From ec4547aea68e27ee482cbd851effc496b8d68e25 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 13:36:19 +0300 Subject: [PATCH 1/2] fix: reject a provider named like an attribute of the target module in expose() --- README.md | 30 ++++++++++++++++++++++++--- modern_di_pytest/factory.py | 26 ++++++++++++++++++------ tests/test_collect_fixtures.py | 11 ++++++++++ tests/test_expose.py | 37 +++++++++++++++++++++++++++++++++- 4 files changed, 94 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 0f32be8..2948275 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,30 @@ def test_email(email_client: EmailClient) -> None: # generated manually email_client.send("hi") ``` +## Fixture names and overrides + +`expose()` names each fixture after its provider attribute, so an exposed fixture +follows pytest's usual override rules. It replaces any fixture with the same name +from pytest itself, from plugins, or from a parent conftest, for every test under +that conftest. A provider named `tmp_path` replaces pytest's `tmp_path`. + +`expose()` raises `ValueError` if the module it installs into already defines a +name it would install, whether that name is a fixture or any other attribute. It +cannot see fixtures defined elsewhere, so it does not report those. + +To avoid a clash, rename the provider or the existing attribute. To keep both +names, do not pass that group to `expose()` and create its fixtures with +`modern_di_fixture()`, giving the clashing one another name: + +```python +app_settings = modern_di_fixture(Dependencies.settings) +user_service = modern_di_fixture(Dependencies.user_service) +``` + +Call `expose()` after the conftest's own fixtures. A fixture defined after the +call replaces the exposed fixture of the same name, and `expose()` cannot detect +that. + ## Pointing a fixture at a child container ```python @@ -138,9 +162,9 @@ Walk each ``Group`` subclass in ``groups`` and install one pytest fixture per Provider class attribute onto the caller's module. Fixture names equal the class-attribute names. Non-Provider class attributes are skipped. A duplicate attribute name across groups raises ``ValueError``, as does a provider named -the same as ``container_fixture``. Calling it with no groups raises -``TypeError``. Pass ``module=`` explicitly when stack -introspection cannot identify the caller. +the same as ``container_fixture`` or a name the target module already defines. +Calling it with no groups raises ``TypeError``. Pass ``module=`` explicitly when +stack introspection cannot identify the caller. ## 📦 [PyPI](https://pypi.org/project/modern-di-pytest) diff --git a/modern_di_pytest/factory.py b/modern_di_pytest/factory.py index f6840fb..7916634 100644 --- a/modern_di_pytest/factory.py +++ b/modern_di_pytest/factory.py @@ -57,6 +57,7 @@ def _fixture(request: pytest.FixtureRequest) -> typing.Any: # noqa: ANN401 def _collect_fixtures( *groups: type[Group], container_fixture: str = "di_container", + existing_names: typing.Collection[str] = (), ) -> dict[str, AbstractProvider[typing.Any]]: """Decide which Providers to expose and under what names. @@ -67,6 +68,8 @@ def _collect_fixtures( - ``TypeError`` if no groups are given. - ``ValueError`` if a name is claimed by more than one group. - ``ValueError`` if a name equals ``container_fixture``. + - ``ValueError`` if a name is in ``existing_names``, the attributes the + target module already defines. """ if not groups: msg = "expose() requires at least one Group." @@ -83,6 +86,14 @@ def _collect_fixtures( "Rename the provider or pass a different container_fixture." ) raise ValueError(msg) + if attr_name in existing_names: + msg = ( + f"expose() cannot install {attr_name!r} from {group.__name__}: " + "the target module already defines it. Rename the provider or the " + "existing attribute, or create this group's fixtures with " + "modern_di_fixture(..., name=...)." + ) + raise ValueError(msg) if attr_name in source: prior = source[attr_name] msg = ( @@ -107,8 +118,9 @@ def expose( Args: *groups: One or more ``Group`` subclasses whose class attributes are - Providers. Attribute names must be unique across all groups; a - duplicate raises ``ValueError``. + Providers. Attribute names must be unique across all groups and + must not already be defined on the target module; either case + raises ``ValueError`` before anything is installed. container_fixture: Name of the pytest fixture yielding the container. A provider with this name raises ``ValueError``. pytest_scope: pytest fixture scope applied to every generated fixture. @@ -126,10 +138,6 @@ def expose( named after that attribute. Non-Provider class attributes are skipped. """ - # Resolve the full set of fixtures (and surface any error) before touching - # the module, so a collision leaves the target untouched. - providers = _collect_fixtures(*groups, container_fixture=container_fixture) - if module is None: frame = inspect.stack()[1].frame module = inspect.getmodule(frame) @@ -137,6 +145,12 @@ def expose( msg = "expose() could not determine the caller module; pass module=... explicitly." raise RuntimeError(msg) + providers = _collect_fixtures( + *groups, + container_fixture=container_fixture, + existing_names=vars(module).keys(), + ) + for attr_name, provider in providers.items(): fixture = modern_di_fixture( provider, diff --git a/tests/test_collect_fixtures.py b/tests/test_collect_fixtures.py index 4828eab..b5c97a1 100644 --- a/tests/test_collect_fixtures.py +++ b/tests/test_collect_fixtures.py @@ -57,3 +57,14 @@ class Colliding(Group): def test_raises_when_called_with_no_groups() -> None: with pytest.raises(TypeError, match="at least one Group"): _collect_fixtures() + + +def test_raises_on_name_already_defined_in_the_target_module() -> None: + with pytest.raises(ValueError, match=r"'service'.*Dependencies.*already defines"): + _collect_fixtures(Dependencies, existing_names={"service", "pytest"}) + + +def test_collects_when_existing_names_do_not_clash() -> None: + collected = _collect_fixtures(Dependencies, existing_names={"Repo", "pytest"}) + + assert set(collected) == {"repo", "service", "request_widget"} diff --git a/tests/test_expose.py b/tests/test_expose.py index 2fef6dc..451504a 100644 --- a/tests/test_expose.py +++ b/tests/test_expose.py @@ -10,7 +10,7 @@ import pytest from modern_di import Group, Scope, providers -from modern_di_pytest import expose +from modern_di_pytest import expose, modern_di_fixture from tests.sample import Dependencies, ExtraDependencies, Repo, Service @@ -100,3 +100,38 @@ class Named(Group): expose(Named, container_fixture="my_container", module=target) assert hasattr(target, "di_container") + + +def test_expose_rejects_provider_named_like_a_fixture_in_the_module() -> None: + target = types.ModuleType("_target") + service = modern_di_fixture(Repo, name="service") + vars(target)["service"] = service + + with pytest.raises(ValueError, match=r"'service'.*Dependencies.*already defines.*name="): + expose(Dependencies, module=target) + + assert target.service is service + assert not hasattr(target, "repo") + assert not hasattr(target, "request_widget") + + +def test_expose_rejects_provider_named_like_a_non_fixture_attribute() -> None: + target = types.ModuleType("_target") + vars(target)["repo"] = types + + with pytest.raises(ValueError, match=r"'repo'.*Dependencies.*already defines"): + expose(Dependencies, ExtraDependencies, module=target) + + assert target.repo is types + assert not hasattr(target, "service") + assert not hasattr(target, "extra_repo") + + +def test_expose_installs_alongside_unrelated_module_attributes() -> None: + target = types.ModuleType("_target") + vars(target)["Repo"] = Repo + + expose(Dependencies, module=target) + + assert target.Repo is Repo + assert hasattr(target, "repo") From ab8478d58ebf4572b5cdcf6122e17a32f4575ca1 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 13:40:35 +0300 Subject: [PATCH 2/2] fix: keep TypeError for an empty expose() call before the module lookup --- modern_di_pytest/factory.py | 11 ++++++++--- tests/test_expose.py | 6 ++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/modern_di_pytest/factory.py b/modern_di_pytest/factory.py index 7916634..1fb3476 100644 --- a/modern_di_pytest/factory.py +++ b/modern_di_pytest/factory.py @@ -54,6 +54,12 @@ def _fixture(request: pytest.FixtureRequest) -> typing.Any: # noqa: ANN401 return _fixture +def _require_groups(groups: tuple[type[Group], ...]) -> None: + if not groups: + msg = "expose() requires at least one Group." + raise TypeError(msg) + + def _collect_fixtures( *groups: type[Group], container_fixture: str = "di_container", @@ -71,9 +77,7 @@ def _collect_fixtures( - ``ValueError`` if a name is in ``existing_names``, the attributes the target module already defines. """ - if not groups: - msg = "expose() requires at least one Group." - raise TypeError(msg) + _require_groups(groups) providers: dict[str, AbstractProvider[typing.Any]] = {} source: dict[str, type[Group]] = {} @@ -138,6 +142,7 @@ def expose( named after that attribute. Non-Provider class attributes are skipped. """ + _require_groups(groups) if module is None: frame = inspect.stack()[1].frame module = inspect.getmodule(frame) diff --git a/tests/test_expose.py b/tests/test_expose.py index 451504a..0c8f7fa 100644 --- a/tests/test_expose.py +++ b/tests/test_expose.py @@ -69,6 +69,12 @@ def test_expose_raises_when_module_cannot_be_determined() -> None: exec(compile(src, "", "exec"), {}) # noqa: S102 +def test_expose_with_no_groups_raises_type_error_before_finding_the_module() -> None: + src = "from modern_di_pytest import expose\nexpose()\n" + with pytest.raises(TypeError, match="at least one Group"): + exec(compile(src, "", "exec"), {}) # noqa: S102 + + def test_expose_rejects_provider_named_like_the_container_fixture() -> None: class Shadowing(Group): di_container = providers.Factory(scope=Scope.APP, creator=Repo)