Add demos for using logger service#611
Conversation
Signed-off-by: Barry Xu <barry.xu@sony.com>
fujitatomoya
left a comment
There was a problem hiding this comment.
overall looks good to me, some changes are requested.
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
|
I updated code according to your comments. |
|
|
@clalancette CI green, can you do final review? |
| @@ -0,0 +1,18 @@ | |||
| Output with default logger level: | |||
There was a problem hiding this comment.
I'm noticing that when running the test here, the test itself takes like 1 second to run, but then it sits around for an additional 5 seconds before the test completes. That means each invocation of this test takes at least 6 seconds, and across 4 RMWs (rmw_fastrtps_cpp, rmw_fastrtps_dynamic_cpp, rmw_cyclonedds_cpp, and rmw_connextdds), this will add at least 24 seconds to our CI times.
Do you mind taking a look here and seeing what is causing that extra 5 second delay?
There was a problem hiding this comment.
Not only did this test take an extra 5 seconds, but I also noticed that all the other tests took at least 5 seconds as well. So, the issue is likely within the testing framework itself.
Currently, I am not familiar with the current testing framework, so it may take some time to investigate
There was a problem hiding this comment.
Find the below code to sleep 5 seconds in test framework. I think we can reduce the sleep time.
demos/demo_nodes_cpp/test/test_executables_tutorial.py.in
Lines 66 to 70 in 74738d8
There was a problem hiding this comment.
@clalancette this is not really related to this PR, can we have another issue to follow-up? or if you have suggestions, can you share it here?
There was a problem hiding this comment.
@clalancette this is not really related to this PR, can we have another issue to follow-up? or if you have suggestions, can you share it here?
Yes, you are correct. If you could please open a separate issue to reduce this timeout in the framework, then I think we are fine to move forward here.
There was a problem hiding this comment.
If you could please open a separate issue to reduce this timeout in the framework, then I think we are fine to move forward here.
I created a new issue #628 to trace this problem. @clalancette
Signed-off-by: Barry Xu <barry.xu@sony.com>
clalancette
left a comment
There was a problem hiding this comment.
One very minor thing to fix, then we'll be good to run another CI on this and merge.
Signed-off-by: Barry Xu <barry.xu@sony.com>
|
CI:
|
Depend on