Skip to content

fix: expose() skipped providers inherited from a parent Group - #38

Merged
lesnik512 merged 1 commit into
mainfrom
fix/expose-inherited-providers
Sep 6, 2026
Merged

fix: expose() skipped providers inherited from a parent Group#38
lesnik512 merged 1 commit into
mainfrom
fix/expose-inherited-providers

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Why

_collect_fixtures discovered providers by walking vars(group) — that is
cls.__dict__, the class's own body only. A provider inherited from a parent
Group was silently skipped, so the fixture set disagreed with the container:

modern-di sees:               ['base_repo', 'child_repo']   # Child.get_named_providers()
expose installed (before):    ['child_repo']
container resolves base_repo: Repo(label='real')

The user gets base_repo registered and resolvable from the container, but no
base_repo fixture, with no error to say so.

This was drift, not a decision. The vars() walk dates to the initial commit
(2026-05-24); modern-di added the public Group.get_named_providers() a month
later (modern-python/modern-di#242, released in 2.20.0), which returns exactly
the name -> provider mapping this function built by hand, walking the full
MRO. The later seam-adoption pass (589674d) reviewed resolve_dependency and
add_providers and adopted the first; it never considered the group walk,
though the method already existed. Nothing in planning/decisions/ or
deferred.md records the own-body-only behaviour as intended.

Design

_collect_fixtures now asks each group for its named providers instead of
walking attributes itself. The cross-group collision check and the
raise-before-returning contract are unchanged. The local skip-non-Provider
branch is gone — get_named_providers() already filters to AbstractProvider,
and name shadowing now follows MRO order, so a child redefining a parent's
provider name yields the child's.

No dependency bump: the floor is already modern-di>=3,<4, well above the
2.20.0 that first shipped the method.

The README's expose() paragraph — "one pytest fixture per Provider class
attribute" — becomes true as written, so it needs no edit.

Compatibility

This can only add fixtures. The one visible edge: a newly-surfaced inherited
name can now collide across groups and raise the existing ValueError, where
before it was invisible.

Non-goals

  • The inject/register wording in the README and in factory.py belongs to
    the in-flight vocabulary migration on docs/migrate-off-planning; the error
    string is left byte-identical here so the two do not conflict.
  • The README does not mention that expose() with no groups raises
    TypeError. Real but separate; not fixed here.
  • No planning/changes/ file. By the convention live on main this is the
    Lightweight lane and would want one, but docs/migrate-off-planning deletes
    that directory and is rebased and ready, so adding a file there buys a
    guaranteed conflict. The spec is this PR body instead, matching where the repo
    is going. Say the word and I'll add the change file.

Verification

just lint-ci clean. just test-ci and just test-branch pass at 100%
line and branch coverage, 17 tests. The new invariant test fails on the old
vars() walk with exactly the three missing inherited names.

@lesnik512
lesnik512 merged commit 8cbe9cb into main Sep 6, 2026
6 checks passed
@lesnik512
lesnik512 deleted the fix/expose-inherited-providers branch September 6, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant