-
Notifications
You must be signed in to change notification settings - Fork 18
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 tests using Infinity.jl #107
Conversation
5b38559
to
4a95e07
Compare
Codecov Report
@@ Coverage Diff @@
## master #107 +/- ##
==========================================
- Coverage 74.39% 74.23% -0.17%
==========================================
Files 10 10
Lines 457 458 +1
==========================================
Hits 340 340
- Misses 117 118 +1
Continue to review full report at Codecov.
|
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.
Looks great. Let's hold off on merging until #104 is in and we'll go from there. Thanks for this :)
4a95e07
to
0e051de
Compare
I'll merge this today. I kind of want to tag it as a feature but since it only effects tests I could do it as a bugfix. |
Sorry this totally fell off my radar. I believe we may need some tests for |
0e051de
to
9b68184
Compare
During the course of completing this PR I noticed some additional work to be done to Infinity.jl: |
The CI failures were hard to track down but it ended up being an issue in Infinity.jl: cjdoris/Infinity.jl#23. The issue has a fix cjdoris/Infinity.jl#24 but we'll require that fix to be merged and tagged before we can merge this change. |
Looks like we also need: cjdoris/Infinity.jl#30 |
Version fixes cjdoris/Infinity.jl#29 which was blocking this PR.
ed4e5cc
to
93b79df
Compare
Finally going to be able to wrap this one up. |
Depends on cjdoris/Infinity.jl#16 and cjdoris/Infinity.jl#18 to be merged in.
This adds tests using Infinity.jl to confirm that it works with intervals.