Repository navigation
fix: reject a provider named like an attribute of the target module in expose() - #70
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #68
expose()installed each fixture withsetattr, so a provider whose name was already an attribute of the target module replaced it without a message. A conftest that defined its ownsettingsfixture and then calledexpose(Deps)with asettingsprovider lost its fixture.expose()now raisesValueErrorwhen a provider name is already defined on the target module, whether that attribute is a fixture, an imported name or anything else. The message names the attribute and the group, says the module already defines it, and points to renaming ormodern_di_fixture(..., name=...). The check runs before anything is installed, like the existing collision checks.Design notes:
_collect_fixturesstays pure. It takes a newexisting_namesargument, andexpose()passesvars(module).keys(). To do that,expose()now resolves the target module before collecting. An empty call still raisesTypeErrorfirst:expose()checks for groups before it looks up the module.expose()twice with the same group on one module now raises, since the second call finds the names from the first.README: a new "Fixture names and overrides" section says exposed fixtures follow pytest's override rules for fixtures from pytest, plugins and parent conftests, how to avoid a clash, and that
expose()should come after the conftest's own fixtures because a later definition cannot be detected.Out of scope, per the issue: detecting pytest built-in, plugin or parent conftest fixtures, a
pytest11entry point, and rename or prefix options forexpose().