-
Notifications
You must be signed in to change notification settings - Fork 37
Add more and improve testing #37
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
Conversation
|
Let me know how you feel about handling weird object keys in the attributes. Implementing that would be kinda dirty atm. |
|
I don't want to lump work on you but at some stage I'd like to remove the |
|
Rebased! No more extra test grouping and removed those |
|
Most of the tests fail, and not all of the tests finish. |
|
Most of the added tests concern handling invalid input, which crel doesn't do at all atm. The tests crash at the property manipulation, which is not implemented. I should probably move them over to #35, but for now you can just comment them out to see the rest of the tests. |
|
Rebased, removed property and invalid input tests, as those will be added in their own prs. (#35 and some other) All tests now run, and luckilly crel doesn't fail any of them |
|
@KoryNunn could you give this another look now that I've tidied it up |
|
Awesome work, thanks! |
This does a couple of things:
end()calls as requestedSome things still need work, mainly the proxy part, but I'll leave that to another pr, this is big enough as is. I left comments about these in the code.
Stuff to consider
Currently not that useful, but will be if testing for garbage input becomes a thing (Check for invalid input #43)
I don't know how essential these would be in the long run in a small project like Crel, but at least having a look locally should give important insight on how comprehensive the tests currently are