ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
124 stars 139 forks source link

Allow ReadyToTest() usage in event handler #665

Closed nnmm closed 1 year ago

nnmm commented 1 year ago

This fixes https://github.com/ros2/launch/issues/578.

The LaunchDescription is searched for a ReadyToTest action recursively with the describe{_conditional}_sub_entities() functions of LaunchDescriptionEntity. For a RegisterEventHandler action, this calls the describe() function of the event handler, but the default implementation returns an empty list. So this PR overrides the describe() function for the OnActionEventBase class, which I think covers all the event handlers where you'd want to use ReadyToTest: OnProcessExit, OnProcessIO, OnProcessStart, OnExecutionComplete.

Apologies in advance for probably not using the right testing idioms, I'm not so familiar with the code base.

nnmm commented 1 year ago

@clalancette Could you suggest a reviewer?

vinnnyr commented 1 year ago

Wow this looks really useful. I was just needing to do this (signal ReadyToTest once a test asset download has been complete).

nnmm commented 1 year ago

Thanks! @cottsay could you review this?

methylDragon commented 1 year ago
methylDragon commented 1 year ago

Thank you for the contribution! :fire: