-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add optional uniqueness checks for Binder
#235
base: master
Are you sure you want to change the base?
Changes from 1 commit
cda04a3
212d38c
28522ca
3cbe776
8e2fffc
cc8cf6f
9e59d15
5784937
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ | |
Inject, | ||
Injector, | ||
NoInject, | ||
NonUniqueBinding, | ||
Scope, | ||
InstanceProvider, | ||
ClassProvider, | ||
|
@@ -1583,6 +1584,15 @@ def test_binder_has_implicit_binding_for_implicitly_bound_type(): | |
assert not injector.binder.has_explicit_binding_for(int) | ||
|
||
|
||
def test_binder_with_uniqueness_checking_raises_error(): | ||
def configure(binder): | ||
binder.bind(int, to=123) | ||
binder.bind(int, to=456) | ||
|
||
with pytest.raises(NonUniqueBinding): | ||
_ = Injector([configure], unique=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good! I would like to see how the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem! It looks like there are a couple of tests keying on similar concepts - I will borrow from them. |
||
|
||
|
||
def test_get_bindings(): | ||
def function1(a: int) -> None: | ||
pass | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to see a more human readable error message here. Our other error messages help the more novice users understad what is wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I completely agree! I will add a message that explains "Binding for 'X' already exists". Do you think that is detailed enough, or would you like to seeing some mention of the
unique
flag?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the first message would be good enough, but that it would be even better to also mention the flag.