-
Notifications
You must be signed in to change notification settings - Fork 245
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
Fix #381 unstable play_end_to_end test #396
Conversation
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang can you run CI on this? |
rosbag2_tests/test/rosbag2_tests/test_rosbag2_play_end_to_end.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
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 - on green CI
I am a bit concerned about what I see in the GitHub actions:
That Can anybody reproduce that return code? |
I can't reproduce this locally on my Ubuntu18.04. |
Fair - I'm trying to scrape the logs to find out how often this occurs |
Added some more context here #381 (comment) - i saw the -11 return code in about a quarter of the runs over the last 24 hours. |
So looks like on both Linux CIs, it is passing 10/10. It seems the version here fails less often than |
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
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 think that's okay for now to disable the tests (again) on windows.
@emersonknapp what do you think?
@emersonknapp If I get an okay about disabling the Windows test, I can merge this. |
yeah, I have no problem with it |
I think this should fix #381. The failing test was
play_filters_by_topic
inrosbag2_tests
.I took the same kind of fix from
rosbag2_transport
https://github.com/ros2/rosbag2/blob/master/rosbag2_transport/test/rosbag2_transport/test_play.cpp#L140 , so I think it should make the tests more stable.I ran GitHub Actions on this branch in my fork (temporarily the default branch to run the scheduled Actions) https://github.com/mabelzhang/rosbag2/actions , and the ros-tooling/action-ros-ci passed most of the time. The occasional times it fails are at
ament_cmake_core
, which I think is a glitch in Actions. The next Actions check,actions/upload-artifact
, always fails in my fork. I think that's not relevant to the test failures. The tests are passing.For example, these are passing runs scheduled ~5 minutes apart
348 https://github.com/mabelzhang/rosbag2/actions/runs/91812093
349 https://github.com/mabelzhang/rosbag2/actions/runs/91814897
350 https://github.com/mabelzhang/rosbag2/actions/runs/91818893
351 https://github.com/mabelzhang/rosbag2/actions/runs/91821981
352 https://github.com/mabelzhang/rosbag2/actions/runs/91825552
354 https://github.com/mabelzhang/rosbag2/actions/runs/91839933
355 https://github.com/mabelzhang/rosbag2/actions/runs/91843979
356 https://github.com/mabelzhang/rosbag2/actions/runs/91847046
As @Karsten1987 mentioned, the problem is probably due to publishers and subscribers not starting at the same time. A real fix is to revive and finish the
start_paused
branch, to add a feature that starts bag playback paused.