Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes for tests in service recording #1

Merged
merged 8 commits into from
Nov 8, 2023
Merged

Fixes for tests in service recording #1

merged 8 commits into from
Nov 8, 2023

Conversation

MichaelOrlov
Copy link

List of fixes:

  • Address flakiness in newly added rosbag2_transport tests
  • Use wait_for_srvice_to_be_ready() instead of the check_service_ready()
  • Add rosbag2_test_common::wait_until_condition(..)
  • Rewrite get_service_info tests to be more deterministic
  • Renames in the get_service_info tests
  • Use std::filesystem instead of rcpputils::fs functions
  • Avoid extra metadata readout in ros2 bag info verb with --verbose
  • Add info_end_to_end test with --verbose parameter

@MichaelOrlov
Address flakiness in newly added rosbag2_transport tests
c551f87 
- Got rid form ambient sleep for waiting for messages to be recorded
- Exclude "/rosout" topic from recording

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Use wait_for_srvice_to_be_ready() instead check_service_ready()
9e637f1 
- Also add default timeout parameter for ClientManager::send_request(..)

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Add rosbag2_test_common::wait_until_condition(..)
96096ff 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Rewrite get_service_info tests to be more deterministic
01a48c3 
- Also parameterize tests to use default supported storage plugins

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Renames in the get_service_info tests
ef4c2c4 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Use std::filesystem instead of rcpputils::fs functions
975e075 
- Rationale: Deprecation of the rcpputils::fs in future
See ros2/rcpputils#164 for details

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Avoid extra metadata readout in ros2 bag info verb with --verbose
2600f9f 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Add info_end_to_end test with --verbose parameter
28decfb 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov marked this pull request as ready for review November 7, 2023 09:47
@MichaelOrlov
Copy link
Author

@Barry-Xu-2018 Please review and merge to your branch if you have no concerns or requests for changes.

@MichaelOrlov MichaelOrlov mentioned this pull request Nov 7, 2023
Merged
@Barry-Xu-2018
Copy link
Owner

@MichaelOrlov

Thank you for your assistance.
I checked each one, and they look good to me.
About info.py, how about to output a warning while users pass '-t' and '-v' at the same time.

@Barry-Xu-2018
Copy link
Owner

I merged them.

About info.py, how about to output a warning while users pass '-t' and '-v' at the same time.

If you think it is okay, I will add it in ros2#1480.

@Barry-Xu-2018 Barry-Xu-2018 merged commit 759064c into Barry-Xu-2018:review/rosbag2_service_record_and_show Nov 8, 2023
10 of 11 checks passed
@MichaelOrlov
Copy link
Author

Sorry for my late response. Yeah, with warning will be even better. 👍

@MichaelOrlov MichaelOrlov deleted the morlov/rosbag2_services_recording_review_fixes branch December 10, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Barry-Xu-2018 Barry-Xu-2018 Barry-Xu-2018 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MichaelOrlov @Barry-Xu-2018