add tests for split_arguments argument in ExecuteLocal

Signed-off-by: William Woodall <[email protected]>
ros2 · Sep 20, 2023 · d8a323b · d8a323b
1 parent 159ee9c
commit d8a323b
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 4 deletions.
diff --git a/launch/launch/actions/execute_local.py b/launch/launch/actions/execute_local.py
@@ -83,8 +83,11 @@ def __init__(
         *,
         process_description: Executable,
         shell: bool = False,
-        split_arguments: SomeSubstitutionsType = LaunchConfiguration(
-            'split_arguments', default=False),
+        split_arguments: Union[
+            SomeSubstitutionsType,
+            bool
+        ] = LaunchConfiguration(
+            'split_arguments', default='False'),
         sigterm_timeout: SomeSubstitutionsType = LaunchConfiguration(
             'sigterm_timeout', default=5),
         sigkill_timeout: SomeSubstitutionsType = LaunchConfiguration(
@@ -209,6 +212,8 @@ def __init__(
         super().__init__(**kwargs)
         self.__process_description = process_description
         self.__shell = shell
+        if isinstance(split_arguments, bool):
+            split_arguments = 'True' if split_arguments else 'False'
         self.__split_arguments = normalize_to_list_of_substitutions(split_arguments)
         self.__sigterm_timeout = normalize_to_list_of_substitutions(sigterm_timeout)
         self.__sigkill_timeout = normalize_to_list_of_substitutions(sigkill_timeout)
@@ -633,8 +638,8 @@ async def __execute_process(self, context: LaunchContext) -> None:
         if returncode == 0:
             self.__logger.info('process has finished cleanly [pid {}]'.format(pid))
         else:
-            self.__logger.error("process has died [pid {}, exit code {}, cmd '{}'].".format(
-                pid, returncode, ' '.join(filter(lambda part: part.strip(), cmd))
+            self.__logger.error("process has died [pid {}, exit code {}, cmd ['{}']].".format(
+                pid, returncode, "', '".join(cmd)
             ))
         await context.emit_event(
                 ProcessExited(returncode=returncode, **process_event_args)

diff --git a/launch/test/launch/test_execute_process.py b/launch/test/launch/test_execute_process.py
@@ -318,3 +318,70 @@ def test_execute_process_prefix_filter_override_in_launch_file():
     test_process.execute(lc)
     assert 'echo' in test_process.process_details['cmd'] and \
         'time' not in test_process.process_details['cmd']
+
+
+preamble = [sys.executable, os.path.join(os.path.dirname(__file__), 'argv_echo.py')]
+
+
+@pytest.mark.parametrize('test_parameters', [
+    # This case will result in 2 arguments, keeping the --some-arg and "some string" together.
+    {
+        'cmd': preamble + ['--some-arg "some string"'],
+        'shell': False,
+        'split_arguments': False,
+        'expected_number_of_args': 2,
+    },
+    # This case will split the --some-arg and "some string" up, resulting in 3 args.
+    {
+        'cmd': preamble + ['--some-arg "some string"'],
+        'shell': False,
+        'split_arguments': True,
+        'expected_number_of_args': 3,
+    },
+    # This case the split_arguments is ignored, due to shell=True,
+    # and produces again 3 arguments, not 4.
+    {
+        'cmd': preamble + ['--some-arg "some string"'],
+        'shell': True,
+        'split_arguments': True,
+        'expected_number_of_args': 3,
+    },
+    # This is the "normal" shell=True behavior.
+    {
+        'cmd': preamble + ['--some-arg "some string"'],
+        'shell': True,
+        'split_arguments': False,
+        'expected_number_of_args': 3,
+    },
+    # Test single argument for cmd (still a list), which will require shell=True.
+    {
+        'cmd': [' '.join(preamble + ['--some-arg "some string"'])],
+        'shell': True,
+        'split_arguments': False,
+        'expected_number_of_args': 3,
+    },
+    # This case also ignores split_arguments.
+    {
+        'cmd': [' '.join(preamble + ['--some-arg "some string"'])],
+        'shell': True,
+        'split_arguments': True,
+        'expected_number_of_args': 3,
+    },
+])
+def test_execute_process_split_arguments(test_parameters):
+    """Test the use of the split_arguments option."""
+    execute_process_action = ExecuteProcess(
+        cmd=test_parameters['cmd'],
+        output='screen',
+        shell=test_parameters['shell'],
+        split_arguments=test_parameters['split_arguments'],
+    )
+
+    ld = LaunchDescription([
+        execute_process_action,
+    ])
+    ls = LaunchService()
+    ls.include_launch_description(ld)
+    assert 0 == ls.run(shutdown_when_idle=True)
+    assert execute_process_action.return_code == test_parameters['expected_number_of_args'], \
+        execute_process_action.process_details['cmd']