-
Notifications
You must be signed in to change notification settings - Fork 50
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
Disable any cross-vendor communication tests for Fast-RTPS. #322
Conversation
Builds are actually failing for all cross-vendor tests involving rmw_fastrtps_cpp not just those between Connext and Fast-RTPS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Linter passes locally with 323e45d. Merging
|
(rmw_implementation1_is_connext AND rmw_implementation2_is_fastrtps) OR | ||
(rmw_implementation1_is_fastrtps AND rmw_implementation2_is_connext)) | ||
) | ||
if(WIN32 AND (rmw_implementation1_is_fastrtps OR rmw_implementation2_is_fastrtps)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition skips the FastRTPS to FastRTPS pub/sub tests on Windows. I assume that was unintentional?
See #377 for the propose fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that seems unintentional, I'll review #377.
Builds are actually failing for all cross-vendor tests involving rmw_fastrtps_cpp not just those between Connext and Fast-RTPS.