Commits on Sep 18, 2024

1. Improve the reliability of rosbag2 tests (#1796) …

   * Remove wait_until_shutdown.
   
   This has almost exactly the same functionality as wait_for_condition,
   except for two things:
   
   1.  It is templated on the Timeout type.
   2.  It calls rclcpp::shutdown after the loop completes.
   
   However, neither of those is necessary; all callers to it use
   a std::chrono::duration, and all of the test fixtures already
   call rclcpp::shutdown.  Thus, just remove it and make all
   callers use wait_for_condition instead.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Shutdown the async spinner node without rclcpp::shutdown.
   
   That is, we really don't actually want to do a full
   rclcpp shutdown here; we only want to stop spinning.
   Accomplish that with an executor, and timing out
   every 100 milliseconds to check if we are done yet.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Small fixes to start_async_spin in rosbag2_tests.
   
   Make sure it only spins as long as we haven't shutdown,
   and that it wakes up every so often to check that fact.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Wait for topics to be discovered during recorder->record().
   
   The main reason for that is that these tests generally want
   to test certain expectations around how many messages were
   received.  However, if discovery takes longer than we expect,
   then it could be the case that we "missed" messages at the
   beginning because discovery hadn't yet completed.
   
   Fix this by just waiting around for the recorder to get all
   the subscriptions it expects before moving on with the test.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Feedback from review.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Switch to using MockRecorder.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Fixes from review.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Feedback from review.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Apply suggestions from code review
   
   Co-authored-by: Michael Orlov <[email protected]>
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Switch to using spin, rather than spin_some.
   
   That's because there is currently at least one bug
   associated with spin_some in rclcpp.  However, it turns
   out that we don't even need to use it, as we can just as
   easily use spin() along with exec.cancel().
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Make sure to stop_spinning when we tear down the test.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Use scopes to shutdown spinning.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Nested contexts just to explicitly cleanup the async spinners.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Update rosbag2_transport/test/rosbag2_transport/record_integration_fixture.hpp
   
   Co-authored-by: Michael Orlov <[email protected]>
   Signed-off-by: Chris Lalancette <[email protected]>
   
   * Apply the same fix to rosbag2_tests.
   
   Signed-off-by: Chris Lalancette <[email protected]>
   
   ---------
   
   Signed-off-by: Chris Lalancette <[email protected]>
   Co-authored-by: Michael Orlov <[email protected]>

   

   clalancette and MichaelOrlov committed Sep 18, 2024
   Configuration menu

   • View commit details
   • Copy full SHA for 60f1042
   • Browse repository at this point

   Copy the full SHA

   60f1042 View commit details

   Browse the repository at this point in the history