-
Notifications
You must be signed in to change notification settings - Fork 49
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
Support recursive wildcards in module ignores #174
Conversation
22f2656
to
155b61d
Compare
This is a duplicate test case which is already in the "wildcards" section above. It shouldn't be under the "invalid expressions" section.
If the test failed to raise an exception as expected the test would silently pass. Fix this by using `pytest.raises()`.
Tests of the forbidden contract could be unstable where an invalid chain had multiple subchains.
155b61d
to
806a598
Compare
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.
Thanks for this - just had a quick skim but this is looking nice. One comment that stood out.
I'll take a closer look in the next few days.
|
||
Examples: | ||
|
||
- ``mypackage.*``: matches ``mypackage.foo`` but not ``mypackage.foo.bar``. | ||
- ``mypackage.*.baz``: matches ``mypackage.foo.baz`` but not ``mypackage.foo.bar.baz``. | ||
- ``mypackage.*.*``: matches ``mypackage.foo.bar`` and ``mypackage.foobar.baz``. | ||
- ``mypackage.foo*``: not a valid expression. (The wildcard must replace a whole module name.) | ||
- ``mypackage.**``: matches ``mypackage.foo.bar`` and ``mypackage.foobar.baz``. |
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.
Should this be mypackage.foo.bar.baz
?
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.
Well, no, I guess it was highlighting that it'd give the same as mypackage.*.*
, but I guess also anything below that too. A bit of a tricky one to express sanely.
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.
Thanks for this - a pleasure to review.
@@ -138,10 +138,6 @@ class TestModuleField(BaseFieldTest): | |||
"mypackage.**.bar -> mypackage.baz", | |||
ValidationError("A wildcard can only replace a whole module."), | |||
), | |||
( |
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.
👏🏻
@@ -33,11 +34,9 @@ class BaseFieldTest: | |||
def test_field(self, raw_data, expected_value): | |||
field = self.field_class(**self.field_kwargs) | |||
|
|||
if isinstance(expected_value, ValidationError): | |||
try: | |||
if isinstance(expected_value, Exception): |
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.
Oops, thanks for spotting.
Closes #160.