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

Stop installing test resources #17

Merged
merged 1 commit into from
Apr 8, 2021
Merged

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Apr 8, 2021

During colcon builds, colcon is using the CMAKE_INSTALL_PREFIX to dictate where the files should be installed to. During Debian and RPM builds, the DESTDIR environment variable is used. The latter approach prefixes all of the install paths, including absolute paths like the destination for the test test resource installation.

The end result is that they get installed into the staging directory for the platform package, and typically end up in the package at some unexpected location.

For RPM builds, this is a fatal build failure, as ROS packages are not expected to install anything outside of /opt/ros/.

It doesn't appear that installing these resources is even necessary because the working directory of the test means that they're already available to the test that needs them without copying.

You can see these extra files if you list the contents of the deb:

$ dpkg-query -L ros-rolling-domain-bridge
/.
/opt
/opt/ros
/opt/ros/rolling
/opt/ros/rolling/include
/opt/ros/rolling/include/domain_bridge

...

/tmp
/tmp/binarydeb
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/default_domain_ids.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/empty.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/invalid_depth.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/invalid_durability.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/invalid_history.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/invalid_reliability.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/missing_from_domain.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/missing_to_domain.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/missing_type.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/name.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/qos_as_list.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/topic_options.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/topics.yaml
/tmp/binarydeb/ros-rolling-domain-bridge-0.1.0/obj-x86_64-linux-gnu/test/config/topics_as_list.yaml
/usr
/usr/share
/usr/share/doc
/usr/share/doc/ros-rolling-domain-bridge
/usr/share/doc/ros-rolling-domain-bridge/changelog.Debian.gz

During colcon builds, colcon is using the CMAKE_INSTALL_PREFIX to
dictate where the files should be installed to. During Debian and RPM
builds, the DESTDIR environment variable is used. The latter approach
prefixes all of the install paths, including absolute paths like the
destination for the test test resource installation.

The end result is that they get installed into the staging directory for
the platform package, and typically end up in the package at some
unexpected location.

For RPM builds, this is a fatal build failure, as ROS packages are not
expected to install anything outside of /opt/ros/<foo>.

It doesn't appear that installing these resources is even necessary
because the working directory of the test means that they're already
available to the test that needs them without copying.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay added the bug Something isn't working label Apr 8, 2021
@cottsay cottsay requested a review from jacobperron April 8, 2021 20:26
@cottsay cottsay self-assigned this Apr 8, 2021
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally installed the test resources to the build directory for convenience. I.e. we can then run the test directly from the build directory: ./test_parse_domain_bridge_yaml_config

I realize now that we can still run the single test with ctest's -R option: ctest -R test_parse_domain_bridge_yaml_config

@jacobperron jacobperron merged commit d811197 into main Apr 8, 2021
@jacobperron jacobperron deleted the cottsay/dont_install_tests branch April 8, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants