Fix test_intra_process_manager.cpp with rmw_zenoh_cpp#2653
Merged
Conversation
In particular, make every pub and sub have to pass in both a topic name and a QoS when they are constructing mock pubs and subs for the intra-process manager test. This just makes it easier to tell whether the pubs and subs should be matched or not. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
It turns out that the intra_process_manager will attempt to match QoS between publishers and subscriptions as they are added to the IPM. This is exactly correct, but the tests were not following the same pattern. Thus, when running these tests under Zenoh, the tests would fail because more things would match than the tests were expecting. Update the test to take the differences in QoS into account, which fixes the test under rmw_zenoh_cpp (and keeps it working for the existing DDS RMWs). Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Contributor
Author
|
Pulls: #2653
|
Yadunund
approved these changes
Oct 16, 2024
fujitatomoya
approved these changes
Oct 16, 2024
Collaborator
fujitatomoya
left a comment
There was a problem hiding this comment.
lgtm with green CI
alsora
reviewed
Oct 16, 2024
| explicit PublisherBase(rclcpp::QoS qos = rclcpp::QoS(10)) | ||
| : qos_profile(qos), | ||
| topic_name("topic") | ||
| explicit PublisherBase(const std::string & topic, rclcpp::QoS qos) |
Collaborator
There was a problem hiding this comment.
minor
Suggested change
| explicit PublisherBase(const std::string & topic, rclcpp::QoS qos) | |
| PublisherBase(const std::string & topic, const rclcpp::QoS & qos) |
the same change applies to the other constructors
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Contributor
|
Yadunund
approved these changes
Oct 18, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
There are 2 patches here: