Tidy up Camera-related tests and add a name property #253
Conversation
The goal here was to make it as easy as possible to revise tests if new camera properties are added by removing duplication.
|
@JuanCab -- it may be easier to look at the two commits separately. The first commit condenses the tests, the second commit add the name property. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #253 +/- ##
==========================================
+ Coverage 55.90% 55.92% +0.01%
==========================================
Files 25 25
Lines 2962 2963 +1
==========================================
+ Hits 1656 1657 +1
Misses 1306 1306 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
It looked good, in fact, I rather like the 'trimming' you have been doing tightening up this code (particularly the testing code). I also like how the test for alt units being ok (test_camera_altunitscheck) is now just to assert the dictionary for the camera is still the same one you used to create it (I assume this is thanks to pydantic's checking).
There was a problem hiding this comment.
Rather like this approach of having a ZERO_CAMERA and then just copying it and changing the attributes. Cleaner test code!
My original intent here was to just a dd a name property; that allows storing at least a little information that is human-readable to identify the camera if looking at the settings much later.
Since that would have meant many, many changes to the tests I first rewrote the tests a bit so that
Cameraonly needed to be changed for the new property in one or two places.I think that ended up making the tests a little bit clearer because it is now more explicit what property of the camera is being changed/modified in a particular ttest.