-
Notifications
You must be signed in to change notification settings - Fork 144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make RegisterEventHandler describe its sub-entities #386
Make RegisterEventHandler describe its sub-entities #386
Conversation
- Also wire up the correct description method for the launch_testing stdout ready listener event handler Signed-off-by: Pete Baughman <peter.c.baughman@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Can you add a test case?
Yes! let me put a little thought into it first though. |
Signed-off-by: Pete Baughman <peter.c.baughman@gmail.com>
@ivanpauno Ok - I didn't do any assertions about the text part of the description - that's not meant to be a thing you use programmatically. I did check that the actions you give to the constructor come out the other side. |
@ivanpauno poke |
CI:
I was OoO yesterday. |
@ivanpauno This kind of looks like an issue with CI:
Have you seen something like that before? |
I get the same errors when I build and test launch_ros master in osrf/ros2:nightly - I think something is broken upstream
|
Opened an issue here to try to get this fixed |
Thanks for the fix @pbaughman ! |
stdout ready listener event handler
This is needed so that launch_testing can find a ReadyToTest action that's conditional on seeing some stdout from one of the processes under test
Signed-off-by: Pete Baughman peter.c.baughman@gmail.com