fix "pass_all_args" for roslaunch-check#1368
Conversation
|
Thanks for this contribution. As the active branch has changed, please retarget this to Please also add a test-case which exposes the problem and is fixed by your proposed change. I would suggest doing so in this file: https://github.com/ros/ros_comm/blob/melodic-devel/tools/roslaunch/resources/example.launch There are several tests which load and process this launch file in different contexts; it shouldn't be hard to make one of them fail. |
1870017 to
c8ed560
Compare
|
I added a test which exposed the mentioned problem. However if i ran the test manually ( Somehow the implicitly defined arguments "__name" and "__log" for the node specified in the testfile get forwarded and end up in the test environment as well. They are blacklisted in 7fb090f Finally my original fix is applied in c8ed560 |
| self.assertIsNone(error_msg) | ||
|
|
||
| if __name__ == '__main__': | ||
| rostest.run(PKG, NAME, TestRoslaunchCheck, sys.argv) |
There was a problem hiding this comment.
You can filter argv using a rospy helper, see: http://wiki.ros.org/rospy/Overview/Initialization%20and%20Shutdown#Accessing_your_command-line_arguments
However, I don't think that's quite the right approach here— AFAICT this doesn't need to be a rostest at all. Rostests are specifically tests which take place in the full roslaunch environment (where a rosmaster and param server are present). What you have here could just be a regular nosetest. See catkin_add_nosetests documentation and some examples:
http://docs.ros.org/melodic/api/catkin/html/howto/format2/python_nose_configuration.html
https://github.com/ros/ros_comm/search?q=catkin_add_nosetests
This should allow you to drop the hack in xmlloader.py as well.
c8ed560 to
bb474ec
Compare
|
Looks great, thanks for your patience in iterating on this with me. |
the option "pass_all_args" was not recognized by roslaunch checks. This works as expected on "our" launch files which create a mixed tree using the "pass_all_args" option as well as explicit argument forwarding