search

ros-controls / ros_control

Generic and simple controls framework for ROS
http://wiki.ros.org/ros_control
BSD 3-Clause "New" or "Revised" License
467 stars 307 forks source link

Investigate running tests out of the devel space #410

Open matthew-reynolds opened 4 years ago

matthew-reynolds commented 4 years ago

Following up from https://github.com/ros-controls/ros_control/pull/404#discussion_r364924536:

Looks like tests fail if we don't install plugins. Will investigate.

Originally posted by @matthew-reynolds

This seems that the way industrial_ci and catkin_tools work together causes us to require installing libraries and plugin .xml files, even if they're only used for tests.

Here is a summary of my understanding:

For now, we're installing the required libraries and the plugin .xml files. Until catkin_tools is updated or unless industrial_ci implements a workaround, I don't think there's much we can do to avoid these installs.

I'm mainly opening this issue to track the behaviour and watch for upstream changes, but if there's a better solution that I've missed, I'd love to hear it.

mathias-luedtke commented 4 years ago

The devel space should not be used, and in general all plugin files must be installed. It looks like #404 does not work with colcon anymore (https://travis-ci.com/ros-industrial/industrial_ci/jobs/275277364)

matthew-reynolds commented 4 years ago

The devel space should not be used

Interesting, can you elaborate on that? I got the impression from that issue that it would be preferable to use the devel space, if possible. My understanding is that running tests out of the install space has the benefit of testing that files are actually installed correctly, but the disadvantage of requiring test resources and files be installed

It looks like #404 does not work with colcon anymore

Looks like this is due to the test nodes not being installed anymore. We ended up removing the installs for non-pluginlib-related files, sounds like that was not a good idea. Any idea why this works with catkin_make and catkin_tools but not colcon? I haven't played with colcon yet and don't know how it differs under the hood.

mathias-luedtke commented 4 years ago

The devel space should not be used

Interesting, can you elaborate on that?

The devel space was mostly a hack, and covers a lot of problems. That's why it got removed in ROS2. In addition, colcon does not support it anymore.

Looks like this is due to the test nodes not being installed anymore.

The tests do not have to get installed necessarily, because they can get started from the build space (?). However, for rostest the situation might be a little bit more complicated, as normal node discovery might be used.

Any idea why this works with catkin_make and catkin_tools but not colcon?

I guess they don't depend on the tests target?

matthew-reynolds commented 4 years ago

The devel space was mostly a hack [...] In addition, colcon does not support it anymore.

Got it. In that case, once we resolve this colcon build problem, I will close this issue. Seems it is not worth looking into it further, particularly since colcon doesn't support it.

The tests do not have to get installed necessarily [...] However, for rostest the situation might be a little bit more complicated

I don't know rostest or colcon well enough to have a more rigorous approach than just trying it and seeing if it works.

I will test with only correcting the tests target issues discussed here, without installing these nodes, and see if the tests still fail.