-
Notifications
You must be signed in to change notification settings - Fork 187
Filter validation #697
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
Filter validation #697
Conversation
|
@m0nkeyc0de , looks like your changes have some linter errors. |
|
@jnovinger, my bad, I'm too used to pre-commit hooks who do that automagically Linting is done in commit 73d9821 |
|
@jnovinger code updated regarding your comments (except for the yet unresolved conversation) I also mentionned the purpose of |
jnovinger
left a comment
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.
This looks great, @m0nkeyc0de, thanks for the iteration and improvements!
I appreciate the separation of the strict validation tests in to a separate test class. Also, I did leave one comment on a completely optional approach to setting up mocks for the strict validation tests--happy to merge this with or without that.
| def test_filter_strict_valid_kwargs(self): | ||
| with patch( | ||
| "pynetbox.core.query.Request._make_call", return_value=Mock() | ||
| ) as mock: |
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.
This is totally optional, but you can also use patch as a function/method or class decorator, allowing you to reduce boilerplate and indentation in test code.
It would look something like this:
@patch("pynetbox.core.query.Request._make_call", return_value=Mock())
class StrictFilterTestCase(unittest.TestCase):
def test_filter_strict_valid_kwargs(self, mock):
# the rest of the test
...Again, totally optional, but thought it was worth sharing.
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 sharing this trick. I figured out that I wasn't using this mock variable as I'm only watching for the Exception beeing raised or not in my tests.
This unused boilerplate is now removed.
Fixes: #695
Validation of filter() kwargs is optionnaly done in pynetbox:
strict_filters=Truewhen instanciating the API objectstrict_filterson thefilter()orget()methodsParameterValidationErroris raised