Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate running tests out of the devel space #410

Open
matthew-reynolds opened this issue Jan 12, 2020 · 4 comments
Open

Investigate running tests out of the devel space #410

matthew-reynolds opened this issue Jan 12, 2020 · 4 comments
Assignees

Comments

@matthew-reynolds
Copy link
Member

Following up from #404 (comment):

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:

  • industrial_ci calls catkin config --install before building or running any tests. This is a good thing for building, but causes some hiccups when running tests.
  • catkin_tools, when running tests, doesn't properly source the devel space
  • This combination of behaviours results in test resources not being found unless they're installed. However, since these resources are only for testing, it isn't ideal to be installing them.

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants