PilzDE / pilz_robots

PILZ robot manipulator module PRBT 6 in ROS
https://wiki.ros.org/pilz_robots
52 stars 25 forks source link

fatal error: 'gmock/gmock.h' file not found #415

Closed v4hn closed 4 years ago

v4hn commented 4 years ago

Commit

melodic-devel

Steps to reproduce

  1. Probably only by setting up a non-debian Linux
  2. Build

Observed behavior

pilz_robots/pilz_testutils/src/joint_state_publisher_mock.cpp:23:10: fatal error: 'gmock/gmock.h' file not found

Looking at the respective CMakeLists.txt, GMOCK_INCLUDE_DIRS is not mentioned anywhere. In another project I had to switch from catkin_add_gtest to catkin_add_gmock to fix this, but as this is a library, that wouldn't work. You probably have to add gmock manually.

I'm guessing Ubuntu/Debian might provide a symlink /usr/include/gmock?

Addendum:

Adding

target_include_directories(${PROJECT_NAME} PRIVATE "${GMOCK_INCLUDE_DIRS}" "${GTEST_INCLUDE_DIRS}")

fixes the problem on my side, but of course you would need to make sure that the variables are actually set (probably only if CATKIN_ENABLE_TESTING is set?)

hslusarek commented 4 years ago

Thanks for your input. I looked into joint_state_publisher_mock.cpp and discovered that the #include <gmock/gmock.h> is actually not needed. I opened a PR #425 to remove the include.

hslusarek commented 4 years ago

Ok, I also opened PR #426 which should add the missing includes, etc.

@v4hn Maybe you can try it out and see if it works for you.

v4hn commented 4 years ago

Sorry, but I'm confused. Why do you propose to add the dependencies when the include is not necessary?

hslusarek commented 4 years ago

Sorry, but I'm confused. Why do you propose to add the dependencies when the include is not necessary?

Sorry, for the confusing communication. The joint_state_publisher_mock.cpp file does not need the gmock include. However, the async_test does need the gmock include, although it's missing the corresponding gmock include right now. For more information see my comment on PR https://github.com/PilzDE/pilz_robots/pull/426#issuecomment-651722245

v4hn commented 4 years ago

The async_test does need the gmock include because it uses ::testing::InvokeWithoutArgs

Syntactically it does not need the include because it only defines preprocessor macros with the offending symbol, it does not use them. As far as I can see, the whole library does not use a single gtest or gmock symbol in preprocessed code.

So of course you can add the dependencies, but everything is fine without them as well. The only thing you gain would be a gmock include in async_test.h, which the user usually provides on their own anyway. Because this is only about preprocessor directives, it wouldn't even require a specific include order between them.

Joint pointing it out though, as long as it's resolved I don't mind either solution.

hslusarek commented 4 years ago

PR #430 is going to close this issue. Thx to everyone for the participation in the discussions.