Migrates legacy launch API tests (#167)

* Emits events.ExecutionComplete on Action completion.

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

* Makes LaunchTestService handle non-process actions.

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

* Makes LaunchTestService handle fixture process actions.

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

* Adds output checks to LaunchTestService.

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

* Updates launch_testing package tests.

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

* Applies fixes after launch_testing refactor.

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

* Adds OpaqueCoroutine action subclass.

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

* Apply suggestions from code review

Co-Authored-By: hidmic <michel@ekumenlabs.com>

* Addresses peer review comments.

- Emit ExecutionComplete events on demand.
- Shutdown OpaqueCoroutine gracefully.
- Fix failing test cases.

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

* Refactors launch_testing API a bit.

To cope with more test cases.

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

* Applies style fixes to launch_testing.

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

* Deal with non zero exit on shutdown.

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

* Avoids output tests' races.

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

* Applies misc fixes after Windows triaging.

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

* Applies more fixes after Windows triaging.

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

* Fixes linter issue.

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

* Addresses peer review comments.

- Improved OpaqueCoroutine documentation.
- Added test for launch.event_handlers.OnExecutionComplete.

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

launch/launch/action.py

@@ -56,7 +56,19 @@ def describe(self) -> Text:
     def visit(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEntity]]:
         """Override visit from LaunchDescriptionEntity so that it executes."""
         if self.__condition is None or self.__condition.evaluate(context):
-            return cast(Optional[List[LaunchDescriptionEntity]], self.execute(context))
+            try:
+                return cast(Optional[List[LaunchDescriptionEntity]], self.execute(context))
+            finally:
+                from .events import ExecutionComplete  # noqa
+                event = ExecutionComplete(action=self)
+                if context.would_handle_event(event):
+                    future = self.get_asyncio_future()
+                    if future is not None:
+                        future.add_done_callback(
+                            lambda _: context.emit_event_sync(event)
+                        )
+                    else:
+                        context.emit_event_sync(event)
         return None
 
     def execute(self, context: LaunchContext) -> Optional[List['Action']]:

launch/launch/actions/__init__.py

@@ -20,6 +20,7 @@
 from .group_action import GroupAction
 from .include_launch_description import IncludeLaunchDescription
 from .log_info import LogInfo
+from .opaque_coroutine import OpaqueCoroutine
 from .opaque_function import OpaqueFunction
 from .pop_launch_configurations import PopLaunchConfigurations
 from .push_launch_configurations import PushLaunchConfigurations
@@ -37,6 +38,7 @@
     'GroupAction',
     'IncludeLaunchDescription',
     'LogInfo',
+    'OpaqueCoroutine',
     'OpaqueFunction',
     'PopLaunchConfigurations',
     'PushLaunchConfigurations',

launch/launch/actions/execute_process.py

@@ -44,8 +44,8 @@
 from ..event_handler import EventHandler
 from ..event_handlers import OnProcessExit
 from ..event_handlers import OnShutdown
+from ..events import matches_action
 from ..events import Shutdown
-from ..events.process import matches_action
 from ..events.process import ProcessExited
 from ..events.process import ProcessStarted
 from ..events.process import ProcessStderr
@@ -257,7 +257,7 @@ def __on_signal_process_event(
             raise RuntimeError('Signal event received before subprocess transport available.')
         if self._subprocess_protocol.complete.done():
             # the process is done or is cleaning up, no need to signal
-            _logger.debug("signal '{}' not set to '{}' because it is already closing".format(
+            _logger.debug("signal '{}' not sent to '{}' because it is already closing".format(
                 typed_event.signal_name, self.process_details['name']
             ))
             return None
@@ -272,11 +272,16 @@ def __on_signal_process_event(
         _logger.info("sending signal '{}' to process[{}]".format(
             typed_event.signal_name, self.process_details['name']
         ))
-        if typed_event.signal_name == 'SIGKILL':
-            self._subprocess_transport.kill()  # works on both Windows and POSIX
+        try:
+            if typed_event.signal_name == 'SIGKILL':
+                self._subprocess_transport.kill()  # works on both Windows and POSIX
+                return None
+            self._subprocess_transport.send_signal(typed_event.signal)
             return None
-        self._subprocess_transport.send_signal(typed_event.signal)
-        return None
+        except ProcessLookupError:
+            _logger.debug("signal '{}' not sent to '{}' because it has closed already".format(
+                typed_event.signal_name, self.process_details['name']
+            ))
 
     def __on_process_stdin_event(
         self,

launch/launch/actions/opaque_coroutine.py

@@ -0,0 +1,115 @@
+# Copyright 2019 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Module for the OpaqueCoroutine action."""
+
+import asyncio
+import collections.abc
+from typing import Any
+from typing import Coroutine
+from typing import Dict
+from typing import Iterable
+from typing import List
+from typing import Optional
+from typing import Text
+
+from ..action import Action
+from ..event import Event
+from ..event_handlers import OnShutdown
+from ..launch_context import LaunchContext
+from ..some_actions_type import SomeActionsType
+from ..utilities import ensure_argument_type
+
+
+class OpaqueCoroutine(Action):
+    """
+    Action that adds a Python coroutine to the launch run loop.
+
+    The signature of a coroutine should be:
+
+    .. code-block:: python
+
+        async def coroutine(
+            context: LaunchContext,
+            *args,
+            **kwargs
+        ):
+            ...
+
+    if ignore_context is False on construction (currently the default), or
+
+    .. code-block:: python
+
+        async def coroutine(
+            *args,
+            **kwargs
+        ):
+            ...
+
+    if ignore_context is True on construction.
+    """
+
+    def __init__(
+        self, *,
+        coroutine: Coroutine,
+        args: Optional[Iterable[Any]] = None,
+        kwargs: Optional[Dict[Text, Any]] = None,
+        ignore_context: bool = False,
+        **left_over_kwargs
+    ) -> None:
+        """Constructor."""
+        super().__init__(**left_over_kwargs)
+        if not asyncio.iscoroutinefunction(coroutine):
+            raise TypeError(
+                "OpaqueCoroutine expected a coroutine for 'coroutine', got '{}'".format(
+                    type(coroutine)
+                )
+            )
+        ensure_argument_type(
+            args, (collections.abc.Iterable, type(None)), 'args', 'OpaqueCoroutine'
+        )
+        ensure_argument_type(kwargs, (dict, type(None)), 'kwargs', 'OpaqueCoroutine')
+        ensure_argument_type(ignore_context, bool, 'ignore_context', 'OpaqueCoroutine')
+        self.__coroutine = coroutine
+        self.__args = []  # type: Iterable
+        if args is not None:
+            self.__args = args
+        self.__kwargs = {}  # type: Dict[Text, Any]
+        if kwargs is not None:
+            self.__kwargs = kwargs
+        self.__ignore_context = ignore_context  # type: bool
+        self.__future = None  # type: Optional[asyncio.Future]
+
+    def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]:
+        """Cancel ongoing coroutine upon shutdown."""
+        if self.__future is not None:
+            self.__future.cancel()
+        return None
+
+    def execute(self, context: LaunchContext) -> Optional[List[Action]]:
+        """Execute the action."""
+        args = self.__args
+        if not self.__ignore_context:
+            args = [context, *self.__args]
+        self.__future = context.asyncio_loop.create_task(
+            self.__coroutine(*args, **self.__kwargs)
+        )
+        context.register_event_handler(
+            OnShutdown(on_shutdown=self.__on_shutdown)
+        )
+        return None
+
+    def get_asyncio_future(self) -> Optional[asyncio.Future]:
+        """Return an asyncio Future, used to let the launch system know when we're done."""
+        return self.__future

launch/launch/event_handlers/__init__.py

@@ -15,13 +15,15 @@
 """Package for event_handlers."""
 
 from .event_named import event_named
+from .on_execution_complete import OnExecutionComplete
 from .on_include_launch_description import OnIncludeLaunchDescription
 from .on_process_exit import OnProcessExit
 from .on_process_io import OnProcessIO
 from .on_shutdown import OnShutdown
 
 __all__ = [
     'event_named',
+    'OnExecutionComplete',
     'OnIncludeLaunchDescription',
     'OnProcessExit',
     'OnProcessIO',

launch/launch/event_handlers/on_execution_complete.py

@@ -0,0 +1,122 @@
+# Copyright 2019 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import collections.abc
+from typing import Callable
+from typing import cast
+from typing import List  # noqa
+from typing import Optional
+from typing import overload
+from typing import Text
+
+from ..event import Event
+from ..event_handler import EventHandler
+from ..events import ExecutionComplete
+from ..launch_context import LaunchContext
+from ..launch_description_entity import LaunchDescriptionEntity
+from ..some_actions_type import SomeActionsType
+
+
+class OnExecutionComplete(EventHandler):
+    """
+    Convenience class for handling an action completion event.
+
+    It may be configured to only handle the completion of a specific action,
+    or to handle them all.
+    """
+
+    @overload
+    def __init__(
+        self, *,
+        target_action: Optional['Action'] = None,
+        on_completion: SomeActionsType,
+        **kwargs
+    ) -> None:
+        """Overload which takes just actions."""
+        ...
+
+    @overload  # noqa: F811
+    def __init__(
+        self,
+        *,
+        target_action: Optional['Action'] = None,
+        on_completion: Callable[[int], Optional[SomeActionsType]],
+        **kwargs
+    ) -> None:
+        """Overload which takes a callable to handle completion."""
+        ...
+
+    def __init__(self, *, target_action=None, on_completion, **kwargs) -> None:  # noqa: F811
+        """Constructor."""
+        from ..action import Action  # noqa
+        if not isinstance(target_action, (Action, type(None))):
+            raise ValueError("OnExecutionComplete requires an 'Action' as the target")
+        super().__init__(
+            matcher=(
+                lambda event: (
+                    isinstance(event, ExecutionComplete) and (
+                        target_action is None or
+                        event.action == target_action
+                    )
+                )
+            ),
+            entities=None,
+            **kwargs,
+        )
+        self.__target_action = target_action
+        # TODO(wjwwood) check that it is not only callable, but also a callable that matches
+        # the correct signature for a handler in this case
+        self.__on_completion = on_completion
+        self.__actions_on_completion = []  # type: List[LaunchDescriptionEntity]
+        if callable(on_completion):
+            # Then on_completion is a function or lambda, so we can just call it, but
+            # we don't put anything in self.__actions_on_completion because we cannot
+            # know what the function will return.
+            pass
+        else:
+            # Otherwise, setup self.__actions_on_completion
+            if isinstance(on_completion, collections.abc.Iterable):
+                for entity in on_completion:
+                    if not isinstance(entity, LaunchDescriptionEntity):
+                        raise ValueError(
+                            "expected all items in 'on_completion' iterable to be of type "
+                            "'LaunchDescriptionEntity' but got '{}'".format(type(entity)))
+                self.__actions_on_completion = list(on_completion)
+            else:
+                self.__actions_on_completion = [on_completion]
+            # Then return it from a lambda and use that as the self.__on_completion callback.
+            self.__on_completion = lambda event, context: self.__actions_on_completion
+
+    def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]:
+        """Handle the given event."""
+        return self.__on_completion(cast(ExecutionComplete, event), context)
+
+    @property
+    def handler_description(self) -> Text:
+        """Return the string description of the handler."""
+        # TODO(jacobperron): revisit how to describe known actions that are passed in.
+        #                    It would be nice if the parent class could output their description
+        #                    via the 'entities' property.
+        if self.__actions_on_completion:
+            return '<actions>'
+        return '{}'.format(self.__on_completion)
+
+    @property
+    def matcher_description(self) -> Text:
+        """Return the string description of the matcher."""
+        if self.__target_action is None:
+            return 'event == ExecutionComplete'
+        return 'event == ExecutionComplete and event.action == Action({})'.format(
+            hex(id(self.__target_action))
+        )

launch/launch/events/__init__.py

@@ -15,12 +15,16 @@
 """Package for events."""
 
 from . import process
+from .execution_complete import ExecutionComplete
 from .include_launch_description import IncludeLaunchDescription
+from .matchers import matches_action
 from .shutdown import Shutdown
 from .timer_event import TimerEvent
 
 __all__ = [
+    'matches_action',
     'process',
+    'ExecutionComplete',
     'IncludeLaunchDescription',
     'Shutdown',
     'TimerEvent',

launch/launch/events/execution_complete.py

@@ -0,0 +1,33 @@
+# Copyright 2019 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Module for ExecutionComplete event."""
+
+from ..action import Action
+from ..event import Event
+
+
+class ExecutionComplete(Event):
+    """Event that is emitted on action execution completion."""
+
+    name = 'launch.events.ExecutionComplete'
+
+    def __init__(self, *, action: 'Action') -> None:
+        """Constructor."""
+        self.__action = action
+
+    @property
+    def action(self):
+        """Getter for action."""
+        return self.__action