-
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
Test consistent #37
Test consistent #37
Conversation
I'm not sure why this would be, but the test fails for me if I specifically do
versus
|
@ericbarefoot nice catch, I think this had something to do with the fact that the delta object was instantiated outside of the test. I have updated the test (and another few tests which check model "state") to instantiate inside of the tests. For simple "exists" checks like in |
well this lowered coverage for some weird reason, but maybe you can check it out again and test locally, @ericbarefoot, if things pass, I think it can be merged. |
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 fantastic to me. I am not so worried about small decreases in coverage. We are going to increase coverage by a bunch eventually when we make a consistent effort to provide unit tests for everything.
I'll go ahead and merge this, then rebase my branch off of this. That way we can see if the jitting is affecting the consistency. To eye it looks fine, but you never know...
add a test to check a small slice of the
eta
matrix, so we can be sure the model is not changed by any of our steps (or at least we can tell when things are changed...).Fix a small warning about literal checking.
@ericbarefoot can you verify the test passes on your local?