Upstream quality changes from Apex.AI part-2 #1924
Merged
+87
−53
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MichaelOrlov
changed the title
Timple
reviewed
Apr 8, 2025
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
- Increase timeout for `expect_messages(..)` - Also add `player_->wait_for_playback_to_start();` in `setup_player()` Signed-off-by: Michael Orlov <morlovmr@gmail.com>
MichaelOrlov
force-pushed
the
morlov/upstream-quality-changes-from-apexai-2
branch
2 times, most recently
from
0022efc to
14405d4
Compare
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
- Reduce size of each std_string_msg to fit in 256 byte boundary Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
- Fix to avoid failure with remote test execution and to be consistent with oter similar tests. Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Increase timeout when waiting to stop discovery at least to the 0.5 sec. The record_all_include_unpublished_false_includes_later_published is tend to be flaky due to the insufficient timeout for waiting to stop discovery thread in the recorder. Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
- Address clang16 warning: "destructor called on non-final 'msg_utils::MessageProducer<T>' that has virtual functions but non-virtual destructor". Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Increase timeouts for service calls in rosbag2_transport `test_play_services` tests Signed-off-by: Michael Orlov <morlovmr@gmail.com>
- Flush output from Python print when printing warnings. Some tests rely on the warning output to be in the std::cout. Without flushing the warnings may not be captured by tests. Signed-off-by: Michael Orlov <morlovmr@gmail.com>
MichaelOrlov
force-pushed
the
morlov/upstream-quality-changes-from-apexai-2
branch
from
14405d4 to
a652fa3
Compare
MichaelOrlov
changed the title
Signed-off-by: Michael Orlov <morlovmr@gmail.com>
rosbag2_tests/test/rosbag2_tests/test_rosbag2_cpp_get_service_info.cpp
Outdated
Show resolved
Hide resolved
Timple
reviewed
Apr 24, 2025
Sort includes alphabetically in test_service_utils.cpp Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com>
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com>
christophebedard
approved these changes
Apr 24, 2025
|
@ros-pull-request-builder retest this please |
|
Pulls: #1924
|
|
https://github.com/Mergifyio backport kilted jazzy |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Apr 25, 2025
* Fix for incorrectly handling exit code in wait_until_completion(..) Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address flakiness in test_play_services - Increase timeout for `expect_messages(..)` - Also add `player_->wait_for_playback_to_start();` in `setup_player()` Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix CPP_LINT warnings about missing include headers Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address CPPLint warnings in the rosbag2_storage_sqlite3 Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Adjust test_rosbag2_storage_api.cpp for bounded strings - Reduce size of each std_string_msg to fit in 256 byte boundary Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Print warnings instead of errors for not found type definitions Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Add debug info to flaky `test_play_cancel` in rosbag2_py Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix for flaky play_respect_messages_timing_after_play_next Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Reduce runtime for the `PlayerTestFixture.playing_respects_delay` test Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address review comment by using `delay_margin(0.5s)` Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address linter warnings in the test_transport.py Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Use RESOURCES_PATH from envar in test_transport.py - Fix to avoid failure with remote test execution and to be consistent with oter similar tests. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Mitigate flakiness in record_all_include_unpublished_false Increase timeout when waiting to stop discovery at least to the 0.5 sec. The record_all_include_unpublished_false_includes_later_published is tend to be flaky due to the insufficient timeout for waiting to stop discovery thread in the recorder. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix linter errors in the `message_definition_encoding.md` Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Minor changes in rosbag2_examples_py Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Mark MessageProducer<T> as final to address clang16 warnings - Address clang16 warning: "destructor called on non-final 'msg_utils::MessageProducer<T>' that has virtual functions but non-virtual destructor". Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address unused variable warning on the clang16 build Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix for flaky `test_play_services` Increase timeouts for service calls in rosbag2_transport `test_play_services` tests Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix for record_if_topic_list_service_list_and_all_are_specified - Flush output from Python print when printing warnings. Some tests rely on the warning output to be in the std::cout. Without flushing the warnings may not be captured by tests. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Sort includes alphabetically in the `info.cpp` file Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Update rosbag2_cpp/test/rosbag2_cpp/test_service_utils.cpp Sort includes alphabetically in test_service_utils.cpp Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Sort includes in test_rosbag2_cpp_get_service_info.cpp Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Sort includes in rosbag2_py/_info.cpp Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> (cherry picked from commit 6f44ee0)
mergify bot
pushed a commit
that referenced
this pull request
Apr 25, 2025
* Fix for incorrectly handling exit code in wait_until_completion(..) Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address flakiness in test_play_services - Increase timeout for `expect_messages(..)` - Also add `player_->wait_for_playback_to_start();` in `setup_player()` Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix CPP_LINT warnings about missing include headers Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address CPPLint warnings in the rosbag2_storage_sqlite3 Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Adjust test_rosbag2_storage_api.cpp for bounded strings - Reduce size of each std_string_msg to fit in 256 byte boundary Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Print warnings instead of errors for not found type definitions Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Add debug info to flaky `test_play_cancel` in rosbag2_py Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix for flaky play_respect_messages_timing_after_play_next Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Reduce runtime for the `PlayerTestFixture.playing_respects_delay` test Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address review comment by using `delay_margin(0.5s)` Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address linter warnings in the test_transport.py Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Use RESOURCES_PATH from envar in test_transport.py - Fix to avoid failure with remote test execution and to be consistent with oter similar tests. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Mitigate flakiness in record_all_include_unpublished_false Increase timeout when waiting to stop discovery at least to the 0.5 sec. The record_all_include_unpublished_false_includes_later_published is tend to be flaky due to the insufficient timeout for waiting to stop discovery thread in the recorder. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix linter errors in the `message_definition_encoding.md` Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Minor changes in rosbag2_examples_py Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Mark MessageProducer<T> as final to address clang16 warnings - Address clang16 warning: "destructor called on non-final 'msg_utils::MessageProducer<T>' that has virtual functions but non-virtual destructor". Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Address unused variable warning on the clang16 build Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix for flaky `test_play_services` Increase timeouts for service calls in rosbag2_transport `test_play_services` tests Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Fix for record_if_topic_list_service_list_and_all_are_specified - Flush output from Python print when printing warnings. Some tests rely on the warning output to be in the std::cout. Without flushing the warnings may not be captured by tests. Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Sort includes alphabetically in the `info.cpp` file Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Update rosbag2_cpp/test/rosbag2_cpp/test_service_utils.cpp Sort includes alphabetically in test_service_utils.cpp Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Sort includes in test_rosbag2_cpp_get_service_info.cpp Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> * Sort includes in rosbag2_py/_info.cpp Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> Signed-off-by: Michael Orlov <morlovmr@gmail.com> --------- Signed-off-by: Michael Orlov <morlovmr@gmail.com> Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com> (cherry picked from commit 6f44ee0) # Conflicts: # ros2bag/ros2bag/verb/record.py # rosbag2_cpp/src/rosbag2_cpp/info.cpp
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR consolidates fixes related to the code quality and stability after backporting Rosbag2 PRs to Apex.AI. PART 2
List of changes:
record_if_topic_list_service_list_and_all_are_specifiedDetails: Flush output from Python print when printing warnings. Some tests rely on the warning output to be in the std::cout. Without flushing, the warnings may not be captured by tests.
test_play_servicesDetails: Increase timeouts for service calls in rosbag2_transport
test_play_servicestestsMessageProducer<T>as final to address clang16 warnings "destructor called on non-final 'msg_utils::MessageProducer' that has virtual functions but non-virtual destructor"message_definition_encoding.mdrecord_all_include_unpublished_falseDetails: Increase timeout when waiting to stop discovery at least to 0.5 sec. The
record_all_include_unpublished_false_includes_later_publishedtends to be flaky due to the insufficient timeout for waiting to stop the discovery thread in the recorder.RESOURCES_PATHfrom envar in thetest_transport.pyRationale: Fix to avoid failure with remote test execution and to be consistent with other similar tests.
PlayerTestFixture.playing_respects_delayplay_respect_messages_timing_after_play_next.Details: Take the start timestamp before calling resume for the player.
test_play_canceltest_rosbag2_storage_api.cppfor bounded strings.Details: Reduce the size of each
std_string_msgto fit in a 256-byte boundary.rosbag2_storage_sqlite3test_play_services.Details: Increase timeout for
expect_messages(..). Also addplayer_->wait_for_playback_to_start();insetup_player()wait_until_completion(..)