-
Notifications
You must be signed in to change notification settings - Fork 9
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 unit tests!!! #11
Comments
Have added some fairly extensive unit tests that mostly test whether |
Just to second that, code coverage right now is sparse :) |
Yes, I've noticed that. I might actually consider getting rid of some of the constructors, there are probably too many of them, I feel like half of this package is constructors. The tests I've added so far were intended to make damn sure that this will never segfault (at least not because of Arrow, users can still cause it). Feather tests have also been very helpful with ensuring that. Fortunately 0.7 detects a large class of segfaults and throws a proper error for a large class of them at seemingly zero cost, so that's good. Tests that we are currently missing that I am most concerned about are for writing. |
Phew... ok I now have in a hell of a lot more unit tests. I haven't seen what the coverage is yet, but now pretty much all of the "important" stuff is being tested. Probably not going to do more myself for quite a while. |
We need proper unit testing so that we can make changes and merge PR's properly. I'll try to get to this right after I do 0.7 updates.
The text was updated successfully, but these errors were encountered: