Add support for launching nodes not in a package #82

hidmic · 2019-10-04T19:27:30Z

Precisely what the title says. Requiring nodes to be installed for launch_ros.actions.Node to be able to launch them is an unnecessary restriction. This way, packages with launch tests can have fixture node executables that do not get installed anywhere.

Follow-up PR after #80. Useful in upcoming PRs.

By making the package argument in launch_ros.actions.Node action optional. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

hidmic · 2019-10-04T19:30:10Z

CI up to launch_testing_ros:

Linux
Linux-aarch64
macOS
Windows

ivanpauno

LGTM!
I left a couple of minimal comments.

(I thought a time ago about doing the same thing).

launch_testing_ros/setup.py

launch_testing_ros/test/examples/talker_listener_launch_test.py

hidmic · 2019-10-07T13:43:34Z

Going in! Thanks for the reviews!

wjwwood · 2019-10-08T21:15:48Z

Is it possible this broken something? It's the only thing that's changed since yesterday when two new failures showed up on nightly/today's CI:

Starting >>> test_launch_ros
03:23:02 running pytest
03:23:02 running egg_info
03:23:02 writing /home/jenkins-agent/workspace/nightly_linux_release/ws/build/test_launch_ros/test_launch_ros.egg-info/PKG-INFO
03:23:02 writing dependency_links to /home/jenkins-agent/workspace/nightly_linux_release/ws/build/test_launch_ros/test_launch_ros.egg-info/dependency_links.txt
03:23:02 writing requirements to /home/jenkins-agent/workspace/nightly_linux_release/ws/build/test_launch_ros/test_launch_ros.egg-info/requires.txt
03:23:02 writing top-level names to /home/jenkins-agent/workspace/nightly_linux_release/ws/build/test_launch_ros/test_launch_ros.egg-info/top_level.txt
03:23:02 reading manifest file '/home/jenkins-agent/workspace/nightly_linux_release/ws/build/test_launch_ros/test_launch_ros.egg-info/SOURCES.txt'
03:23:02 writing manifest file '/home/jenkins-agent/workspace/nightly_linux_release/ws/build/test_launch_ros/test_launch_ros.egg-info/SOURCES.txt'
03:23:02 running build_ext
03:23:02 ============================= test session starts ==============================
03:23:02 platform linux -- Python 3.6.8, pytest-5.2.1, py-1.8.0, pluggy-0.13.0
03:23:02 cachedir: /home/jenkins-agent/workspace/nightly_linux_release/ws/build/test_launch_ros/.pytest_cache
03:23:02 rootdir: /home/jenkins-agent/workspace/nightly_linux_release/ws/src/ros2/launch_ros/test_launch_ros
03:23:02 plugins: ament-copyright-0.8.0, ament-pep257-0.8.0, ament-flake8-0.8.0, ament-lint-0.8.0, mock-1.11.1, repeat-0.8.0, rerunfailures-7.0, cov-2.8.1
03:23:03 collecting ... 
03:23:03 collected 50 items                                                             
03:23:03 
03:23:03 test/test_copyright.py .                                                 [  2%]
03:23:03 test/test_flake8.py .                                                    [  4%]
03:23:03 test/test_pep257.py .                                                    [  6%]
03:23:03 test/test_launch_ros/test_normalize_parameters.py .....................  [ 48%]
03:23:03 test/test_launch_ros/test_normalize_remap_rule.py .....                  [ 58%]
03:23:05 test/test_launch_ros/actions/test_node.py .........                      [ 76%]
03:23:05 test/test_launch_ros/actions/test_push_ros_namespace.py ........         [ 92%]
03:23:05 test/test_launch_ros/frontend/test_node_frontend.py RRRRRRRRRRFRRRRRRRRR [ 96%]
03:23:05 RF                                                                       [ 96%]
03:23:05 test/test_launch_ros/substitutions/test_find_package.py ..               [100%]
03:23:05 
03:23:05 =================================== FAILURES ===================================
03:23:05 _ test_node_frontend[\n<launch>\n    <let name="a_string" value="\\'[2, 5, 8]\\'"/>\n    <let name="a_list" value="[2, 5, 8]"/>\n    <node pkg="demo_nodes_py" exec="talker_qos" output="screen" name="my_node" namespace="my_ns" args="--number_of_cycles 1">\n        <param name="param1" value="ads"/>\n        <param name="param_group1">\n            <param name="param_group2">\n                <param name="param2" value="2"/>\n            </param>\n            <param name="param3" value="2, 5, 8" value-sep=", "/>\n            <param name="param4" value="$(var a_list)"/>\n            <param name="param5" value="$(var a_string)"/>\n            <param name="param6" value="2., 5., 8." value-sep=", "/>\n            <param name="param7" value="'2', '5', '8'" value-sep=", "/>\n            <param name="param8" value="''2'', ''5'', ''8''" value-sep=", "/>\n            <param name="param9" value="\\'2\\', \\'5\\', \\'8\\'" value-sep=", "/>\n            <param name="param10" value="''asd'', ''bsd'', ''csd''" value-sep=", "/>\n            <param name="param11" value="'\\asd', '\\bsd', '\\csd'" value-sep=", "/>\n            <param name="param12" value=""/>\n        </param>\n        <param from="/home/jenkins-agent/workspace/nightly_linux_release/ws/src/ros2/launch_ros/test_launch_ros/test/test_launch_ros/frontend/params.yaml"/>\n        <env name="var" value="1"/>\n    </node>\n</launch>\n] _
03:23:05 test/test_launch_ros/frontend/test_node_frontend.py:114: in test_node_frontend
03:23:05     ld = parser.parse_description(root_entity)
03:23:05 ../../../../install/launch/lib/python3.6/site-packages/launch/frontend/parser.py:102: in parse_description
03:23:05     actions = [self.parse_action(child) for child in entity.children]
03:23:05 ../../../../install/launch/lib/python3.6/site-packages/launch/frontend/parser.py:102: in <listcomp>
03:23:05     actions = [self.parse_action(child) for child in entity.children]
03:23:05 ../../../../install/launch/lib/python3.6/site-packages/launch/frontend/parser.py:85: in parse_action
03:23:05     return instantiate_action(entity, self)
03:23:05 ../../../../install/launch/lib/python3.6/site-packages/launch/frontend/expose.py:38: in instantiate_action
03:23:05     action_type, kwargs = action_parse_methods[entity.type_name](entity, parser)
03:23:05 ../../../../install/launch_ros/lib/python3.6/site-packages/launch_ros/actions/node.py:242: in parse
03:23:05     package = parser.parse_substitution(entity.get_attr('pkg'), optional=True)
03:23:05 E   TypeError: parse_substitution() got an unexpected keyword argument 'optional'
03:23:05 _ test_node_frontend[\nlaunch:\n    - let:\n        name: 'a_string'\n        value: "'[2, 5, 8]'"\n    - let:\n        name: 'a_list'\n        value: '[2, 5, 8]'\n    - node:\n        pkg: demo_nodes_py\n        exec: talker_qos\n        output: screen\n        name: my_node\n        namespace: my_ns\n        args: '--number_of_cycles 1'\n        param:\n            -   name: param1\n                value: ads\n            -   name: param_group1\n                param:\n                -   name: param_group2\n                    param:\n                    -   name: param2\n                        value: 2\n                -   name: param3\n                    value: [2, 5, 8]\n                -   name: param4\n                    value: $(var a_list)\n                -   name: param5\n                    value: $(var a_string)\n                -   name: param6\n                    value: [2., 5., 8.]\n                -   name: param7\n                    value: ['2', '5', '8']\n                -   name: param8\n                    value: ["'2'", "'5'", "'8'"]\n                -   name: param9\n                    value: ["\\\\'2\\\\'", "\\\\'5\\\\'", "\\\\'8\\\\'"]\n                -   name: param10\n                    value: ["'asd'", "'bsd'", "'csd'"]\n                -   name: param11\n                    value: ['\\asd', '\\bsd', '\\csd']\n                -   name: param12\n                    value: ''\n            -   from: /home/jenkins-agent/workspace/nightly_linux_release/ws/src/ros2/launch_ros/test_launch_ros/test/test_launch_ros/frontend/params.yaml\n        env:\n            -   name: var\n                value: '1'\n] _
03:23:05 test/test_launch_ros/frontend/test_node_frontend.py:114: in test_node_frontend
03:23:05     ld = parser.parse_description(root_entity)
03:23:05 ../../../../install/launch/lib/python3.6/site-packages/launch/frontend/parser.py:102: in parse_description
03:23:05     actions = [self.parse_action(child) for child in entity.children]
03:23:05 ../../../../install/launch/lib/python3.6/site-packages/launch/frontend/parser.py:102: in <listcomp>
03:23:05     actions = [self.parse_action(child) for child in entity.children]
03:23:05 ../../../../install/launch/lib/python3.6/site-packages/launch/frontend/parser.py:85: in parse_action
03:23:05     return instantiate_action(entity, self)
03:23:05 ../../../../install/launch/lib/python3.6/site-packages/launch/frontend/expose.py:38: in instantiate_action
03:23:05     action_type, kwargs = action_parse_methods[entity.type_name](entity, parser)
03:23:05 ../../../../install/launch_ros/lib/python3.6/site-packages/launch_ros/actions/node.py:242: in parse
03:23:05     package = parser.parse_substitution(entity.get_attr('pkg'), optional=True)
03:23:05 E   TypeError: parse_substitution() got an unexpected keyword argument 'optional'
03:23:05 =============================== warnings summary ===============================
03:23:05 test/test_launch_ros/actions/test_node.py::TestNode::test_launch_node_with_parameter_dict
03:23:05   /home/jenkins-agent/workspace/nightly_linux_release/ws/src/ros2/launch_ros/test_launch_ros/test/test_launch_ros/actions/test_node.py:164: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
03:23:05     expanded_parameters_dict = yaml.load(h)
03:23:05 
03:23:05 -- Docs: https://docs.pytest.org/en/latest/warnings.html
03:23:05 - generated xml file: /home/jenkins-agent/workspace/nightly_linux_release/ws/build/test_launch_ros/pytest.xml -
03:23:05 ============== 2 failed, 48 passed, 1 warnings, 20 rerun in 2.85s ==============
03:23:05 Finished <<< test_launch_ros [3.97s]	[ with test failures ]

wjwwood · 2019-10-08T21:18:49Z

launch_ros/launch_ros/actions/node.py

@@ -235,7 +239,9 @@ def parse(cls, entity: Entity, parser: Parser):
        node_name = entity.get_attr('node-name', optional=True)
        if node_name is not None:
            kwargs['node_name'] = node_name
-        kwargs['package'] = parser.parse_substitution(entity.get_attr('pkg'))
+        package = parser.parse_substitution(entity.get_attr('pkg'), optional=True)


This is the line where nightly/CI is failing.

Yes, that's wrong. I'll fix it and check why it didn't fail before.

Cool thanks, I've got it as part of this issue, so if you fix it could you comment there too? ros2/build_farmer#244

hidmic added 2 commits October 4, 2019 15:59

Add support for launching nodes not installed by a package.

3b6bf45

By making the package argument in launch_ros.actions.Node action optional. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

Stop installing launch_testing_ros test fixture nodes.

9e7292e

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

hidmic changed the title ~~Add support for launching nodes not installed by a package.~~ Add support for launching nodes not in a package Oct 4, 2019

hidmic requested review from ivanpauno and jacobperron October 4, 2019 19:30

ivanpauno reviewed Oct 4, 2019

View reviewed changes

launch_testing_ros/setup.py Show resolved Hide resolved

launch_testing_ros/test/examples/talker_listener_launch_test.py Show resolved Hide resolved

ivanpauno approved these changes Oct 4, 2019

View reviewed changes

jacobperron approved these changes Oct 4, 2019

View reviewed changes

hidmic merged commit e64cf54 into master Oct 7, 2019

delete-merged-branch bot deleted the hidmic/launch-not-packaged-nodes branch October 7, 2019 13:43

wjwwood mentioned this pull request Oct 8, 2019

new test failures on linux nightly ros2/build_farmer#244

Closed

wjwwood reviewed Oct 8, 2019

View reviewed changes

hidmic mentioned this pull request Oct 8, 2019

Fix launch_ros.actions.Node parsing function #83

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for launching nodes not in a package #82

Add support for launching nodes not in a package #82

hidmic commented Oct 4, 2019 •

edited

Loading

hidmic commented Oct 4, 2019

ivanpauno left a comment

hidmic commented Oct 7, 2019

wjwwood commented Oct 8, 2019

wjwwood Oct 8, 2019

hidmic Oct 8, 2019

wjwwood Oct 8, 2019

Add support for launching nodes not in a package #82

Add support for launching nodes not in a package #82

Conversation

hidmic commented Oct 4, 2019 • edited Loading

hidmic commented Oct 4, 2019

ivanpauno left a comment

Choose a reason for hiding this comment

hidmic commented Oct 7, 2019

wjwwood commented Oct 8, 2019

wjwwood Oct 8, 2019

Choose a reason for hiding this comment

hidmic Oct 8, 2019

Choose a reason for hiding this comment

wjwwood Oct 8, 2019

Choose a reason for hiding this comment

hidmic commented Oct 4, 2019 •

edited

Loading