Omnibus fixes for running tests with Connext. #2684
Merged
Conversation
This file contains 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
clalancette
requested review from
ivanpauno,
hidmic and
wjwwood
as code owners
This was
linked to
issues
Nov 22, 2024
|
Pulls: #2684
|
clalancette
force-pushed
the
clalancette/connext-test-failure-fixes
branch
from
54455b3 to
871cd7a
Compare
|
@clalancette i was trying to fix #2613 with #2651, but i was able to reproduce the issue after QoS depth 10. i will try your patch to make sure if that problem goes away. (i have never tried ServiceQoSProfile yet.) |
|
@clalancette unfortunately still fails... export RMW_IMPLEMENTATION=rmw_connextdds
colcon test --event-handlers console_direct+ --packages-select rclcpp --ctest-args -R test_service_introspection --retest-until-fail 100
...
74: Test command: /usr/bin/python3 "-u" "/root/ros2_ws/work/install/ament_cmake_test/share/ament_cmake_test/cmake/run_test.py" "/root/ros2_ws/work/build/rclcpp/test_results/rclcpp/test_service_introspection.gtest.xml" "--package-name" "rclcpp" "--output-file" "/root/ros2_ws/work/build/rclcpp/ament_cmake_gmock/test_service_introspection.txt" "--command" "/root/ros2_ws/work/build/rclcpp/test/rclcpp/test_service_introspection" "--gtest_output=xml:/root/ros2_ws/work/build/rclcpp/test_results/rclcpp/test_service_introspection.gtest.xml"
74: Working Directory: /root/ros2_ws/work/build/rclcpp/test/rclcpp
74: Test timeout computed to be: 60
74: -- run_test.py: invoking following command in '/root/ros2_ws/work/build/rclcpp/test/rclcpp':
74: - /root/ros2_ws/work/build/rclcpp/test/rclcpp/test_service_introspection --gtest_output=xml:/root/ros2_ws/work/build/rclcpp/test_results/rclcpp/test_service_introspection.gtest.xml
74: Running main() from gmock_main.cc
74: [==========] Running 3 tests from 1 test suite.
74: [----------] Global test environment set-up.
74: [----------] 3 tests from TestServiceIntrospection
74: [ RUN ] TestServiceIntrospection.service_introspection_nominal
74: RTI Connext DDS Non-commercial license is for academic, research, evaluation and personal use only. USE FOR COMMERCIAL PURPOSES IS PROHIBITED. See RTI_LICENSE.TXT for terms. Download free tools at rti.com/ncl. License issued to Non-Commercial User [email protected] For non-production use only.
74: Expires on 00-jan-00 See www.rti.com for more information.
74: [ OK ] TestServiceIntrospection.service_introspection_nominal (305 ms)
74: [ RUN ] TestServiceIntrospection.service_introspection_enable_disable_events
74: RTI Connext DDS Non-commercial license is for academic, research, evaluation and personal use only. USE FOR COMMERCIAL PURPOSES IS PROHIBITED. See RTI_LICENSE.TXT for terms. Download free tools at rti.com/ncl. License issued to Non-Commercial User [email protected] For non-production use only.
74: Expires on 00-jan-00 See www.rti.com for more information.
74: /root/ros2_ws/work/src/ros2/rclcpp/rclcpp/test/rclcpp/test_service_introspection.cpp:232: Failure
74: Expected equality of these values:
74: events.size()
74: Which is: 3
74: 4U
74: Which is: 4
74:
74: [ FAILED ] TestServiceIntrospection.service_introspection_enable_disable_events (2305 ms) |
When running the tests with RTI Connext as the default RMW, some of the tests are failing. There are three different failures fixed here: 1. Setting the liveliness duration to a value smaller than a microsecond causes Connext to throw an error. Set it to a millisecond. 2. Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1. Connext is somewhat slow in this regard, so it can be the case that we are overwriting a previous service introspection event with the next one. Switch to the ServicesDefaultQoS in the test, which ensures we will not lose events. 3. Connext is slow to match publishers and subscriptions. Thus, when creating a subscription "on-the-fly", we should wait for the publisher to match it before expecting the subscription to actually receive data from it. With these fixes in place, the test_client_common, test_generic_service, test_service_introspection, and test_executors tests all pass for me with rmw_connextdds. Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
clalancette
force-pushed
the
clalancette/connext-test-failure-fixes
branch
from
871cd7a to
ceb7e76
Compare
Signed-off-by: Chris Lalancette <[email protected]>
Ah, sorry, I forgot about your PR.
Good call on this. I was able to reproduce, and I found out the actual problem. What we needed to do was wait until our subscription in the test got matched with the introspection publishers coming from the service introspection. I did that in 309cd85, and it now seems to work for me. Can you give it another shot and see if that fixes it for you? |
Signed-off-by: Chris Lalancette <[email protected]>
|
Pulls: #2684
|
fujitatomoya
approved these changes
Nov 30, 2024
|
@Mergifyio backport jazzy |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Nov 30, 2024
* Omnibus fixes for running tests with Connext. When running the tests with RTI Connext as the default RMW, some of the tests are failing. There are three different failures fixed here: 1. Setting the liveliness duration to a value smaller than a microsecond causes Connext to throw an error. Set it to a millisecond. 2. Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1. Connext is somewhat slow in this regard, so it can be the case that we are overwriting a previous service introspection event with the next one. Switch to the ServicesDefaultQoS in the test, which ensures we will not lose events. 3. Connext is slow to match publishers and subscriptions. Thus, when creating a subscription "on-the-fly", we should wait for the publisher to match it before expecting the subscription to actually receive data from it. With these fixes in place, the test_client_common, test_generic_service, test_service_introspection, and test_executors tests all pass for me with rmw_connextdds. Signed-off-by: Chris Lalancette <[email protected]> * Fixes for executors. Signed-off-by: Chris Lalancette <[email protected]> * One more fix for services. Signed-off-by: Chris Lalancette <[email protected]> * More fixes for service_introspection. Signed-off-by: Chris Lalancette <[email protected]> * More fixes for introspection. Signed-off-by: Chris Lalancette <[email protected]> --------- Signed-off-by: Chris Lalancette <[email protected]> (cherry picked from commit 9984197) # Conflicts: # rclcpp/test/rclcpp/executors/test_executors.cpp # rclcpp/test/rclcpp/test_generic_service.cpp
fujitatomoya
added a commit
that referenced
this pull request
Dec 3, 2024
* Omnibus fixes for running tests with Connext. (#2684) * Omnibus fixes for running tests with Connext. When running the tests with RTI Connext as the default RMW, some of the tests are failing. There are three different failures fixed here: 1. Setting the liveliness duration to a value smaller than a microsecond causes Connext to throw an error. Set it to a millisecond. 2. Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1. Connext is somewhat slow in this regard, so it can be the case that we are overwriting a previous service introspection event with the next one. Switch to the ServicesDefaultQoS in the test, which ensures we will not lose events. 3. Connext is slow to match publishers and subscriptions. Thus, when creating a subscription "on-the-fly", we should wait for the publisher to match it before expecting the subscription to actually receive data from it. With these fixes in place, the test_client_common, test_generic_service, test_service_introspection, and test_executors tests all pass for me with rmw_connextdds. Signed-off-by: Chris Lalancette <[email protected]> * Fixes for executors. Signed-off-by: Chris Lalancette <[email protected]> * One more fix for services. Signed-off-by: Chris Lalancette <[email protected]> * More fixes for service_introspection. Signed-off-by: Chris Lalancette <[email protected]> * More fixes for introspection. Signed-off-by: Chris Lalancette <[email protected]> --------- Signed-off-by: Chris Lalancette <[email protected]> (cherry picked from commit 9984197) # Conflicts: # rclcpp/test/rclcpp/executors/test_executors.cpp # rclcpp/test/rclcpp/test_generic_service.cpp * address backport merge conflicts. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Chris Lalancette <[email protected]> Co-authored-by: Tomoya Fujita <[email protected]>
HarunTeper
pushed a commit
to HarunTeper/rclcpp
that referenced
this pull request
Dec 9, 2024
* Omnibus fixes for running tests with Connext. When running the tests with RTI Connext as the default RMW, some of the tests are failing. There are three different failures fixed here: 1. Setting the liveliness duration to a value smaller than a microsecond causes Connext to throw an error. Set it to a millisecond. 2. Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1. Connext is somewhat slow in this regard, so it can be the case that we are overwriting a previous service introspection event with the next one. Switch to the ServicesDefaultQoS in the test, which ensures we will not lose events. 3. Connext is slow to match publishers and subscriptions. Thus, when creating a subscription "on-the-fly", we should wait for the publisher to match it before expecting the subscription to actually receive data from it. With these fixes in place, the test_client_common, test_generic_service, test_service_introspection, and test_executors tests all pass for me with rmw_connextdds. Signed-off-by: Chris Lalancette <[email protected]> * Fixes for executors. Signed-off-by: Chris Lalancette <[email protected]> * One more fix for services. Signed-off-by: Chris Lalancette <[email protected]> * More fixes for service_introspection. Signed-off-by: Chris Lalancette <[email protected]> * More fixes for introspection. Signed-off-by: Chris Lalancette <[email protected]> --------- Signed-off-by: Chris Lalancette <[email protected]> Signed-off-by: HarunTeper <[email protected]>
HarunTeper
pushed a commit
to HarunTeper/rclcpp
that referenced
this pull request
Dec 9, 2024
* Omnibus fixes for running tests with Connext. When running the tests with RTI Connext as the default RMW, some of the tests are failing. There are three different failures fixed here: 1. Setting the liveliness duration to a value smaller than a microsecond causes Connext to throw an error. Set it to a millisecond. 2. Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1. Connext is somewhat slow in this regard, so it can be the case that we are overwriting a previous service introspection event with the next one. Switch to the ServicesDefaultQoS in the test, which ensures we will not lose events. 3. Connext is slow to match publishers and subscriptions. Thus, when creating a subscription "on-the-fly", we should wait for the publisher to match it before expecting the subscription to actually receive data from it. With these fixes in place, the test_client_common, test_generic_service, test_service_introspection, and test_executors tests all pass for me with rmw_connextdds. Signed-off-by: Chris Lalancette <[email protected]> * Fixes for executors. Signed-off-by: Chris Lalancette <[email protected]> * One more fix for services. Signed-off-by: Chris Lalancette <[email protected]> * More fixes for service_introspection. Signed-off-by: Chris Lalancette <[email protected]> * More fixes for introspection. Signed-off-by: Chris Lalancette <[email protected]> --------- Signed-off-by: Chris Lalancette <[email protected]> Signed-off-by: HarunTeper <[email protected]>
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.
When running the tests with RTI Connext as the default RMW, some of the tests are failing. There are three different failures fixed here:
Setting the liveliness duration to a value smaller than a microsecond causes Connext to throw an error. Set it to a millisecond.
Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1. Connext is somewhat slow in this regard, so it can be the case that we are overwriting a previous service introspection event with the next one. Switch to the ServicesDefaultQoS in the test, which ensures we will not lose events.
Connext is slow to match publishers and subscriptions. Thus, when creating a subscription "on-the-fly", we should wait for the publisher to match it before expecting the subscription to actually receive data from it.
With these fixes in place, the test_client_common, test_generic_service, test_service_introspection, and test_executors tests all pass for me with rmw_connextdds.
This should fix #2613, #2611, and #2646. @Crola1702 FYI