make rostest in CMakeLists optional (ros/rosdistro#3010)

[bulwahn]

No description provided.

[trainman419]

I'm not sure I see the point here; rostest is still a build_depend in the relevant package.xml.

[bulwahn]

You are right. Removing rostest as build dependency requires REP 140 to be active, implemented and rolled out. So for now, this is only a first step that remains ineffective for most use cases, but mainly prepares for the roll-out of REP 140. However, the installation routines for OpenEmbedded [1] rely only on the CMakeLists, and not on the information in the package.xml. So, it can exploit this refined setup before REP 140 is in place, and explore the possible problematic cases for REP 140's roll-out.

[1] https://github.com/bmwcarit/meta-ros

[bulwahn]

Are there still open concerns for this pull request?

[DLu]

I defer to @mikeferguson, but what problem does this solve?

[mikeferguson]

@bulwahn I think we can merge this -- but:

I realize CATKIN_ENABLE_TESTING was added to catkin about 8 months ago -- but I think we should amend the package.xmls to require catkin > 0.5.72 so that people will at least get a warning or forced update if they happen to have a very old version of catkin on their system, rather than some odd cmake error. If you go ahead and update the pull request with that, I'll gladly merge.

[bulwahn]

@mikeferguson CATKIN_ENABLE_TESTING was added with catkin version 0.5.68. If you look at the package.xml of map_server and robot_pose_ekf, you will notice that there is already a version constraint for catkin >= 0.5.68, which was added when the CATKIN_ENABLE_TESTING block was added in the commit 8354ca3. So in this commit, there no action required for the package.xml.

[mikeferguson]

I honestly have no idea what commit I was looking at when I made that comment -- but looking back at it, it is clearly not the commit attached to this PR. Clearly we are already using CATKIN_ENABLE_TESTING and have the catkin gte tag....

[mikeferguson]

Apparently, I should have read ros/rosdistro#3010 .. this ended up breaking the build pretty badly (ros/rosdistro#4411 (comment)). I believe this will be fixed by 72310ff, but waiting for the buildfarm to tell me, since I can't quite reproduce on my local machine.