Add coverage analysis for moveit_core #1133
Conversation
There was a problem hiding this comment.
Thanks for this initial contribution. What is still missing from my point of view: How does this integrate with github? We want to include coverage badges in the README.md table.
Where are the results stored? And how can we access them for badges?
I think, the reference mentioned in #1 used a different approach.
moveit_core/package.xml
Outdated
| @@ -76,4 +76,5 @@ | |||
| <test_depend>orocos_kdl</test_depend> | |||
| <test_depend>rosunit</test_depend> | |||
| <test_depend>tf_conversions</test_depend> | |||
| <test_depend>code_coverage</test_depend> | |||
There was a problem hiding this comment.
Coverage testing is not enabled for testing by default, is it?
If so, we can drop this dependency.
agutenkunst
force-pushed
the
feature/code_coverage
branch
from
56645ba
to
6c41eac
Compare
|
For context: We are currently at the world moveit day Talked to @v4hn we agreed that this may not be exactly what was hoped for and misses on the nice to have badges. However maybe even for a short time this would enable to generation of some kind of coverage until some coveralls is made. Results are stored locally after generation, really not perfect but better than nothing. |
There was a problem hiding this comment.
Yes, this probably does not easily allow for coverall badges, but it allows to generate it for core easily (for now).
Looking at the generated coverage, we should probably focus on increasing numbers a bit before adding the badges to the website...
@rhaschke your concrete feedback is addressed.
|
agreed on reducing public eye of our shameful test coverage, but can you point us to a webpage with our current results? |
|
@davetcoleman the coverage report ends up locally in the workspace (it looks like this is using my code_coverage package -- which is still somewhat new). I'd be happy to accept changes to code_coverage if someone comes up with a good way to get badges/results out easily. |
Need to add to package.xml |
I explicitly suggested not to depend on this by default: #1133 (comment). Having this in package.xml only ensures that rosdep will install it. It's not needed for the normal build or test process. I suggest to install this package by default in MoveIt's docker containers. |
|
Ok, if we're not going to add it to package.xml please add a new short section at the bottom of this tutorial explaining how to use this experimental feature (including what to install first) https://ros-planning.github.io/moveit_tutorials/doc/tests/tests_tutorial.html |
|
Results: 51.2 % of our functions have tests in moveit_core
|
|
@davetcoleman you mean at https://ros-planning.github.io/moveit_tutorials/doc/tests/tests_tutorial.html somewhere? |
|
Yes |
|
ping re:tutorial :-) otherwise this feature will likely never be found again as its pretty esoteric |
|
@davetcoleman: You merged the tutorials-PR, what about the feature itself? |
|
Ooops indeed! Humans are faulty like that, hence my recommendation here: #1284 ;-) |
|
cherry picked to melodic |
Originated from: #1
Provides a CMake target to generate a coverage analysis of moveit_core.
Usage:
catkin_make -DENABLE_COVERAGE_TESTING=ON moveit_core_coverage