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

fix test_urdf for travis #243

Merged
merged 1 commit into from Nov 9, 2017

Conversation

Projects
None yet
6 participants
@fmessmer
Copy link
Member

commented Nov 7, 2017

fixes #242

this works for us
@mgruhler does this work for you, too?

@fmessmer

This comment has been minimized.

Copy link
Member Author

commented Nov 7, 2017

@ipa-rmb @ipa-jba @ipa-fez @ipa-fmw @ipa-bnm this is a hotfix for fixing our unittests...please merge asap!

@ipa-fez

ipa-fez approved these changes Nov 8, 2017

Copy link

left a comment

Not relying on having /opt/ros hardcoded would be nicer, so if you find a way, would be nice to fix it later.

@fmessmer

This comment has been minimized.

Copy link
Member Author

commented Nov 8, 2017

@ipa-fez

  • approve with owner account or merge directly
  • neither rosrun or rospack works in travis...I don't have the time - I just fixed it with the solution that is already used in cob_calibration_data by @ipa-mdl
@ipa-fez

This comment has been minimized.

Copy link

commented Nov 8, 2017

feel free to merge now

@ipa-mdl

This comment has been minimized.

Copy link
Member

commented Nov 8, 2017

stop!

@mgruhler

This comment has been minimized.

Copy link
Member

commented Nov 8, 2017

seems me fixing a not-working test script results in lots of problems 😭

@ipa-mdl

This comment has been minimized.

Copy link
Member

commented Nov 8, 2017

Please try setting ADDITIONAL_DEBS=ros-indigo-roslib. (see below)

I just fixed it with the solution that is already used in cob_calibration_data

This fix was added in a travis-only script, not in a catkin-triggered test.

@ipa-mdl

This comment has been minimized.

Copy link
Member

commented Nov 8, 2017

And add a test_depend on rosbash.

@ipa-mdl

This comment has been minimized.

Copy link
Member

commented Nov 8, 2017

Instead of the ADDITIONAL_DEBS setting, a test_depend on roslib might be even better.

Background: rospack (used by rosrun) needs some files from roslib to actually work.
However, roslib already depends on rospack, so this dependency cannot be added to the package.

@ipa-mdl

This comment has been minimized.

Copy link
Member

commented Nov 8, 2017

I just found out that this is script is not a test, but an actual script. (which should go into the scripts folder!).
In this case add exec_depends.

@fmessmer

This comment has been minimized.

Copy link
Member Author

commented Nov 8, 2017

re-opening ipa320/cob_robots#738 - for testing rosrun-variant

@@ -20,6 +20,7 @@

<exec_depend>gazebo_ros</exec_depend>
<exec_depend>rospy</exec_depend>
<exec_depend>roslib</exec_depend>

This comment has been minimized.

Copy link
@ipa-mdl

ipa-mdl Nov 8, 2017

Member

and rosbash

This comment has been minimized.

Copy link
@ipa-mdl

ipa-mdl Nov 8, 2017

Member

Actually, rospy already depends on roslib.

@@ -22,6 +22,7 @@

import rospy
import rosunit
import roslib

This comment has been minimized.

Copy link
@ipa-mdl

ipa-mdl Nov 8, 2017

Member

no need to import it.
The files from roslib will be used by catkin to set-up ROS_PACKAGE_PATH automatically.

This comment has been minimized.

Copy link
@ipa-mdl

ipa-mdl Nov 8, 2017

Member

And you don't need to import rospy..

@fmessmer fmessmer force-pushed the fmessmer:fix/test_urdf_travis branch from 3ded5dd to 0f3d3c7 Nov 8, 2017

@fmessmer fmessmer force-pushed the fmessmer:fix/test_urdf_travis branch from 0f3d3c7 to ec445fa Nov 8, 2017

@fmessmer

This comment has been minimized.

Copy link
Member Author

commented Nov 8, 2017

thanks @ipa-mdl
adding rosbash did the trick

@mgruhler

This comment has been minimized.

Copy link
Member

commented Nov 8, 2017

@ipa-fxm @ipa-mdl thanks. This version works for me perfectly!

@ipa-mdl

ipa-mdl approved these changes Nov 8, 2017

@@ -19,6 +19,7 @@
<buildtool_depend>catkin</buildtool_depend>

<exec_depend>gazebo_ros</exec_depend>
<exec_depend>rosbash</exec_depend>
<exec_depend>rospy</exec_depend>

This comment has been minimized.

Copy link
@ipa-mdl

ipa-mdl Nov 8, 2017

Member

extra picky: you can substitute rospy with roslib

@fmessmer

This comment has been minimized.

Copy link
Member Author

commented Nov 8, 2017

@ipa-rmb @ipa-jba @ipa-fez @ipa-fmw @ipa-bnm this is now good to merge!

@floweisshardt floweisshardt merged commit 672ad16 into ipa320:indigo_dev Nov 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@fmessmer fmessmer deleted the fmessmer:fix/test_urdf_travis branch Nov 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.