-
Notifications
You must be signed in to change notification settings - Fork 49
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 the cameras to the POCS tests #646
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #646 +/- ##
===========================================
+ Coverage 65.64% 69.08% +3.43%
===========================================
Files 66 65 -1
Lines 5792 5702 -90
Branches 810 798 -12
===========================================
+ Hits 3802 3939 +137
+ Misses 1780 1553 -227
Partials 210 210
Continue to review full report at Codecov.
|
That's the kind of test coverage bump I want... |
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.
Can't really tell what the effect of this change is.
Though I do see that coverage has gone up by more than 1%. Pretty good! |
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.
Approving because of the coverage increase alone, without fully understanding the impact.
Maybe add some comments in the new fixture?
This was merely a fix that should have been put in place with #612 (the camera dependency injection). Without this change the Observatory object is not getting passed any cameras so was erroring. Which, now that I write that, means that we should have a check in the Observatory such that it cannot leave the |
This was missed during #612 but is also indicative of #645.