-
Notifications
You must be signed in to change notification settings - Fork 146
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
Enable reuse of launch testing functionality #236
Conversation
@@ -45,7 +45,7 @@ def _match(expected, actual): | |||
return lambda expected, actual: expected in actual | |||
elif hasattr(expected_output, 'search'): | |||
return lambda expected, actual: ( | |||
expected.match(actual.replace(os.linesep, '\n')) is not None | |||
expected.search(actual.replace(os.linesep, '\n')) is not None |
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.
I encountered this same issue while trying to improve the test coverage on launch_testing. A simple test that catches this issue and prevents regression would be something like
def test_works_with_regex(self):
assertInStdout(
self.proc_output,
re.compile(r'Called with arguments \S+'),
'terminating_proc-2'
)
You can add it to the bottom of test_io_handler_and_assertions.py
You'll also need to import re
at the top
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.
Sounds good!
Signed-off-by: Michel Hidalgo <[email protected]>
…nStdout Signed-off-by: Michel Hidalgo <[email protected]>
Signed-off-by: Michel Hidalgo <[email protected]>
Signed-off-by: Michel Hidalgo <[email protected]>
0d6686c
to
69ab824
Compare
This reverts commit 7329a3e.
Connected to ros2/ros_testing#1. This pull request enables reuse of the
launch_test
CLI andadd_launch_test
CMake macro functionality, to be leveraged by theros2test
andros_testing
packages.Incidentally, a bug was found during the testing phase and thus this PR includes a fix for it, in it's own separate commit.