-
Notifications
You must be signed in to change notification settings - Fork 138
types : definition : Add validate to definition #357
Conversation
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.
Sweet! Can you add some unit tests? One for an input with validation and one without (to make sure it's not changing).
import inspect | ||
import pathlib | ||
|
||
import unittest |
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 accidentally removed these when I went through and removed all unused import. The CI wasn't failing... weird. I figured out what was wrong: #358
If you merge master into this branch we'll be good to go.
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.
Last thing. Looking great :D Thanks!
tests/test_types.py
Outdated
|
||
|
||
class TestDefintion(AsyncTestCase): | ||
async def test_validate(self): |
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.
Let's add another test_
where the pie
datatype raises a InputValidationError
(add this new error class to dffml/exceptions.py
).
Use with self.assertRaises(InputValidationError)
in the new test to make sure that we catch it.
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.
SWEET
fixes #349