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

Make unit test run with catkin_make run_tests #15

Merged
merged 6 commits into from
Mar 29, 2017

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Mar 28, 2017

I notice the unit tests wasn't running when using catkin_make run_tests. I changed how the test was added to use the catkin cmake macros. I also added python-mock as a test dependency because the official ubuntu/trusty docker image does not have that installed by default.

@clalancette
Copy link
Contributor

I love the change. However, it looks like all CI has failed, with no yaml. I think we'll need to add python-yaml to the package.xml as well.

@clalancette
Copy link
Contributor

A couple of things:

  • While you are messing around with the package.xml, it might be worthwhile to convert it over to format 2, to make it easier to integrate with ROS2.
  • Travis is still failing, not being able to find urdf_parser_py. I've seen that locally when I forget to source devel/setup.bash in my catkin workspace; I think something similar must be going on with travis. Whether it is worthwhile to fix travis, however, I'm not sure.

The unit tests can fail in some cases with catkin_make.
When using catkin_make the unit tests cannot depend on an environment
being set up with a complete environment (thankes @dirk-thomas!)
This was not an issue with catkin_make_isolated or `catkin build`
@sloretz
Copy link
Contributor Author

sloretz commented Mar 28, 2017

@clalancette Will do with the package.xml!

I learned from @dirk-thomas that unit tests can't expect to have the python path set up for importing python packages from the current package in all cases when using catkin_make. The unit test now adds ../src to sys.path. catkin_make_isolated and catkin build don't have this issue.

@sloretz
Copy link
Contributor Author

sloretz commented Mar 28, 2017

@clalancette I updated the package.xml. An odd thing is it has a <depend>python</depend>. I'm 99% sure that is unnecessary.

@clalancette
Copy link
Contributor

I agree with you that the on python is a bit weird. However, there are other packages (vision_opencv, for instance), which also have that dependency. I'll leave it up to you, but I'd just leave it in for now; it probably doesn't hurt anything. Otherwise, I think this looks good to me, so feel free to merge it.

@sloretz sloretz merged commit 06dd28a into indigo-devel Mar 29, 2017
@sloretz sloretz deleted the catkinize_unittests branch March 29, 2017 14:47
miguelriemoliveira added a commit to afonsocastro/urdf_parser_py that referenced this pull request May 20, 2019
ros#25 first version of data collector

ros#15 and ros#21 usign a pr2 bag file downloaded from https://projects.csail.mit.edu/stata/downloads.php

ros#24 data collector is already prepared to handle dynamic transforms
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

Successfully merging this pull request may close these issues.

2 participants