Test for Optical Flow checks correct camera position #23266
Conversation
|
|
||
|
|
||
| TEST_F(VehicleOpticalFlowTest, whenCameraDownFacing_ThenSensorSelected) | ||
| { |
There was a problem hiding this comment.
can you add some comments to the test methods? Usually best practice is to structure it with
// GIVEN
...
// WHEN <>
...
// THEN
EXPECT...
There was a problem hiding this comment.
Test name contains "WHEN" and "THEN" section:
whenCameraDownFacing_ThenSensorSelected
With such approach, comments are unnecessary.
There was a problem hiding this comment.
Hi @sfuhrer , can you please take a look on my latest answer?
There was a problem hiding this comment.
I still think adding some simple //GIVEN, //WHEN, //THEN comments increase the readability of the code and don't find them unnecessary. It's a pattern that we establish for all unit tests and makes it very easy to understand the aim of a test without having to read actual code.
There was a problem hiding this comment.
Hi @sfuhrer, testcase name was changed, and //GIVEN, //WHEN, //THEN comments were added, please take a look.
src/modules/sensors/vehicle_optical_flow/test/VehicleOpticalFlowTest.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Mathieu Bresciani <brescianimathieu@gmail.com>
…owTest.cpp Co-authored-by: Mathieu Bresciani <brescianimathieu@gmail.com>
|
|
||
|
|
||
| TEST_F(VehicleOpticalFlowTest, whenCameraDownFacing_ThenSensorSelected) | ||
| { |
|
|
||
|
|
||
| TEST_F(VehicleOpticalFlowTest, whenCameraDownFacing_ThenSensorSelected) | ||
| { |
There was a problem hiding this comment.
I still think adding some simple //GIVEN, //WHEN, //THEN comments increase the readability of the code and don't find them unnecessary. It's a pattern that we establish for all unit tests and makes it very easy to understand the aim of a test without having to read actual code.
src/modules/sensors/vehicle_optical_flow/test/VehicleOpticalFlowTest.cpp
Outdated
Show resolved
Hide resolved
| TEST_F(VehicleOpticalFlowTest, whenCameraDownFacing_ThenSensorSelected) | ||
| { | ||
| distance_sensor_s message = createDistanceSensorMessage(distance_sensor_s::ROTATION_DOWNWARD_FACING); | ||
| orb_advertise(ORB_ID(distance_sensor), &message); |
There was a problem hiding this comment.
You're missing the .publish step no? Have a look here:
There was a problem hiding this comment.
For this functionality orb_advertise is enough, please take a look on VehicleOpticalFlow::UpdateDistanceSensor().
There was a problem hiding this comment.
Also testcase name was changed, and //GIVEN, //WHEN, //THEN comments added
…owTest.cpp Co-authored-by: Silvan Fuhrer <silvan@auterion.com>
Solved Problem
OpticalFlow tests coverage improved with additional tests.
Fixes #{Github issue ID}
Solution
New test 'VehicleOpticalFlowTest' which checks camera orientation for Optical Flow.
Changelog Entry
For release notes:
Test coverage
VehicleOpticalFlowTest - new test