moveit2 Feature/adds attached body regression tests

Description

I'm planning to implement the proposal made in #3060 (hierarchical collision objects). I figured it'd first be useful to add some tests for the existing functionality, check for regressions whilst making the changes, and as a basis to extend with tests checking the correctness of collision object -> collision object transformation calculations.

Checklist

[ ] Required by CI: Code is auto formatted using clang-format
[ ] Extend the tutorials / documentation reference
[ ] Document API changes relevant to the user in the MIGRATION.md notes
[ ] Create tests, which fail without this PR reference
[ ] Include a screenshot if changing a GUI
[ ] While waiting for someone to review your request, please help review another open pull request to support the maintainers

Dec 04 '24 19:12 TSNoble

The tests are currently segfaulting, with the following output from gtest:

root@2e16105760d5:~/ws_moveit# ./build/moveit_core/robot_state/test_attached_body 
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from SingleAttachedBody
[ RUN      ] SingleAttachedBody.RootBodyHasCorrecAttachedLink
SingleAttachedBody
Stack trace (most recent call last):
#12   Object "", at 0xffffffffffffffff, in 
#11   Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f370fd61e4, in _start
#10   Object "/usr/lib/x86_64-linux-gnu/libc.so.6", at 0x7f8d20cfc28a, in __libc_start_main
#9    Object "/usr/lib/x86_64-linux-gnu/libc.so.6", at 0x7f8d20cfc1c9, in 
#8    Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f370fd5a92, in main
#7    Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f370ffb41e, in testing::UnitTest::Run()
#6    Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f37100ad7b, in testing::internal::UnitTestImpl::RunAllTests()
#5    Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f370ffb236, in testing::TestSuite::Run()
#4    Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f370ffaff4, in testing::TestInfo::Run()
#3    Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f370ffadc0, in testing::Test::Run()
#2    Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f371014990, in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)
#1    Object "/root/ws_moveit/build/moveit_core/robot_state/test_attached_body", at 0x55f370fda2bb, in SingleAttachedBody::SetUp()
#0    Object "/root/ws_moveit/build/moveit_core/robot_model/libmoveit_robot_model.so.2.12.0", at 0x7f8d21358cc0, in moveit::core::RobotModel::getLinkModel(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool*)
Segmentation fault (Address not mapped to object [0x78])
Segmentation fault

It seems to me that the SingleLinkRobot fixture's setup isn't being called. This leads to a segfault when the SingleAttachedBody setup attempts to fetch a link, due to the robot model not yet being initialised.

My knowledge of gtest is a little rusty, but I believe inheriting from a fixture should call the parent's SetUp methods?

Dec 04 '24 19:12 TSNoble

Also, I'm not a URDF expert, I'd appreciate any help simplifying it. After making the #3060 changes, I'm planning to mainly test attached body to attached body transforms, and the ability to look up child objects from parents, etc... so ideally the robot should just be the simplest possible example that a body can be attached to.

Dec 04 '24 19:12 TSNoble

Adding an explicit call to the parent's SetUp(), and fixing some URDF issues seems to have fixed the issue, though I'm still unsure if this call is necessary

Dec 04 '24 19:12 TSNoble

It seems this is just standard C++ behaviour. I wasn't sure if gtest had some magic under the hood for auto calling parent SetUp()s.

I've replaced SetUp() with constructors to get the desired effect.

Dec 04 '24 20:12 TSNoble

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 46.24%. Comparing base (7d6e2ab) to head (061c82b). :warning: Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3149      +/-   ##
==========================================
+ Coverage   46.22%   46.24%   +0.02%     
==========================================
  Files         720      721       +1     
  Lines       62713    62748      +35     
  Branches     7594     7594              
==========================================
+ Hits        28985    29009      +24     
- Misses      33562    33572      +10     
- Partials      166      167       +1

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

:snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Dec 04 '24 21:12 codecov-commenter

Thanks @sea-bass for the comments. Will take a better look at them after work today. This PR is still very much in the early stages, just wanted to get a simple test passing and go from there. Raised as a regular PR to demonstrate the segfault I was getting in the workflow, but since that's fixed now (and I can run the tests pretty easy locally) I'm happy to move it to a draft.