-
Notifications
You must be signed in to change notification settings - Fork 1
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 back date validation #264
Conversation
willronchetti
commented
Aug 9, 2023
- Adds non-GPL validation component, tests for date/date-time
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.
My comments are not blocking.
snovault/tests/test_schema_utils.py
Outdated
'date_property': invalid_date | ||
}) | ||
date_error = str(errors[0]) | ||
assert f"'{invalid_date}' is not a 'date'" in date_error |
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.
The error message shows the offending string by calling repr
or using %r
or {...!r}
, and it makes more sense to me to do the same thing here. The difference is subtle, but if you had an invalid date with a single quote mark in it, the way it would show up in the error message would involve escaping (presumably, or the use of doublequotes instead) and not just the addition of single quotes on the outside.
assert f"'{invalid_date}' is not a 'date'" in date_error | |
assert f"{invalid_date!r} is not a 'date'" in date_error |
snovault/tests/test_schema_utils.py
Outdated
'date_time_property': invalid_date_time | ||
}) | ||
date_error = str(errors[0]) | ||
assert f"'{invalid_date_time}' is not a 'date-time'" in date_error |
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.
assert f"'{invalid_date_time}' is not a 'date-time'" in date_error | |
assert f"{invalid_date_time!r} is not a 'date-time'" in date_error |
@@ -403,7 +400,7 @@ def validate(schema, data, current=None, validate_current=False): | |||
dict validated contents, list of errors | |||
""" | |||
resolver = NoRemoteResolver.from_schema(schema) | |||
sv = SchemaValidator(schema, resolver=resolver, format_checker=format_checker) | |||
sv = SchemaValidator(schema, resolver=resolver, format_checker=Draft202012Validator.FORMAT_CHECKER) |
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 assume this is something that the format-nongpl
option sets up, but maybe a comment line explaining what this is about is useful.
snovault/tests/test_schema_utils.py
Outdated
_update_resolved_data, _handle_list_or_string_value, resolve_merge_refs, | ||
validate |
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 predict PyCharm and perhaps make lint
will fuss about you importing things with leading _
, so I would write:
_update_resolved_data, _handle_list_or_string_value, resolve_merge_refs, | |
validate | |
_update_resolved_data, _handle_list_or_string_value, # noQA - testing protected members | |
resolve_merge_refs, validate |