robot_cal_tools icon indicating copy to clipboard operation
robot_cal_tools copied to clipboard

Fresh clone doesn't compile due to missing GTest

Open jdlangs opened this issue 5 years ago • 5 comments

When you clone the repo down and attempt to build as a catkin workspace, rct_ros_tools fails to build due to the local copy/build of GTest not being present. I was able to figure out I could build with RCT_BUILD_TESTS to get it working but that shouldn't be needed when starting with the repo.

jdlangs avatar Sep 03 '20 21:09 jdlangs

Did you happen to be building with CATKIN_ENABLE_TESTING=ON? I made a change recently that makes rct_ros_tools use the version of GTest that is cloned into the workspace by rct_common (which happens when RCT_BUILD_TESTS=ON). It seems like there is a scenario in which CATKIN_ENABLE_TESTING=ON but RCT_BUILD_TESTS=OFF in which the build would probably fail like you described.

A solution to this case might be amending the test section in rct_ros_tools/CMakeLists.txt like this:

if(CATKIN_ENABLE_TESTING && RCT_BUILD_TESTS)
...
endif()

marip8 avatar Sep 03 '20 22:09 marip8

Yes, that's the failure mode. Those are the default values for the variables, right? At least I didn't explicitly turn on CATKIN_ENABLE_TESTING.

Your proposed solution is probably the simplest. I was thinking of something like printing a warning that testing will be disabled until it's built with RCT_BUILD_TESTS on.

jdlangs avatar Sep 03 '20 22:09 jdlangs

A quick search seems to indicate that the default value of CATKIN_ENABLE_TESTING is ON.

Alternatively we could fix it with:

if(CATKIN_ENABLE_TESTING)
  find_package(rostest REQUIRED)
  if(RCT_BUILD_TESTS)
    find_package(GTest REQUIRED)
  endif()
  ...
endif()

I think this would make it use the version of GTest provided by rostest (which is different than the version we clone) in the case that RCT_BUILD_TESTS=OFF and would allow building the unit tests in the more standard catkin way where only the CATKIN_ENABLE_TESTING variable is required

marip8 avatar Sep 03 '20 22:09 marip8

IIRC, the default CMake variable for this would be BUILD_TESTING.

I seem to recall Catkin and Colcon both also set that variable, either depending on the value of CATKIN_ENABLE_TESTING in the case of Catkin, or some other conditional for Colcon.

Would perhaps renaming RCT_BUILD_TESTS to BUILD_TESTING solve this?

gavanderhoorn avatar Sep 04 '20 05:09 gavanderhoorn

I fixed this problem in this pull : https://github.com/Jmeyer1292/robot_cal_tools/pull/95

Reference : ros gtest

gachiemchiep avatar Mar 29 '21 01:03 gachiemchiep