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.

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

Add loop and sleep to test #109

Merged
merged 4 commits into from Oct 7, 2021
Merged

Conversation

gbiggs
Copy link
Collaborator

@gbiggs gbiggs commented Aug 20, 2021

Bug fix

Fixed bug

One of the tests in test_Transport.cpp in rmf_rxcpp was flaky due to a race condition between creating a subscriber to a topic and checking the count of subscribers to that topic. See here for one example of where this was failing.

Fix applied

A loop was added that sleeps briefly then checks if the subscription has been created. When the subscription exists the loop exits. The loop is limited to 10 iterations.

Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net>
@gbiggs gbiggs added the bug Something isn't working label Aug 20, 2021
@gbiggs gbiggs requested a review from mxgrey August 20, 2021 07:18
@gbiggs gbiggs self-assigned this Aug 20, 2021
@gbiggs gbiggs added this to In Review in Research & Development via automation Aug 20, 2021
Copy link
Contributor

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a tiny code linting error.

rmf_fleet_adapter/rmf_rxcpp/test/test_Transport.cpp Outdated Show resolved Hide resolved
Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net>
@gbiggs
Copy link
Collaborator Author

gbiggs commented Oct 7, 2021

@mxgrey The test that's failing appears to be unrelated to the changes in this PR.

@codecov
Copy link

codecov bot commented Oct 7, 2021

Codecov Report

Merging #109 (e3baa36) into main (fb9f934) will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #109      +/-   ##
==========================================
+ Coverage   21.48%   21.54%   +0.05%     
==========================================
  Files         211      844     +633     
  Lines       17153    68628   +51475     
  Branches     8129    32524   +24395     
==========================================
+ Hits         3686    14784   +11098     
- Misses       9507    37996   +28489     
- Partials     3960    15848   +11888     
Flag Coverage Δ
tests 21.54% <ø> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...apter/src/rmf_fleet_adapter/services/Negotiate.hpp
...adapter/src/rmf_fleet_adapter/phases/MoveRobot.hpp
...fleet_adapter/src/rmf_fleet_adapter/estimation.cpp
...xCpp-4.1.0/Rx/v2/src/rxcpp/operators/rx-concat.hpp
...f_fleet_adapter/agv/internal_RobotUpdateHandle.hpp
.../RxCpp-4.1.0/Rx/v2/src/rxcpp/operators/rx-lift.hpp
.../rmf_task_ros2/test/dispatcher/test_Dispatcher.cpp
...Cpp-4.1.0/Rx/v2/src/rxcpp/subjects/rx-behavior.hpp
...d4xr/rmf_ros2/rmf_fleet_adapter/test/test_Task.cpp
...s2/src/rmf_traffic_ros2/geometry/ShapeInternal.hpp
... and 1045 more

@mxgrey mxgrey merged commit 95ab29c into main Oct 7, 2021
Research & Development automation moved this from In Review to Done Oct 7, 2021
@mxgrey mxgrey deleted the gbiggs/fix-flaky-test-for-subscriber-count branch October 7, 2021 15:03
arjo129 pushed a commit that referenced this pull request Oct 12, 2021
Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

2 participants