skip test relying on source timestamps with Connext

[dirk-thomas]

Skip test with Connext which have been failing since they have been added:

• Feature/services timestamps #545: http://build.ros2.org/view/Rci/job/Rci__nightly-connext_ubuntu_focal_amd64/42/testReport/rclpy.rclpy.test.test_client/TestClient/test_service_timestamps/
• Add method to take with message_info #542: http://build.ros2.org/view/Rci/job/Rci__nightly-connext_ubuntu_focal_amd64/42/testReport/rclpy.rclpy.test.test_node/TestNodeAllowUndeclaredParameters/test_take/

The missing feature is ticketed in ros2/rmw_connext#405.

Before:
After:

[gbiggs]

The changes themselves LGTM.

Is there an alternative test for _rclpy.rclpy_take() available?

[dirk-thomas]

Is there an alternative test for _rclpy.rclpy_take() available?

I don't understand the question - what for?

[gbiggs]

I'm probably misunderstanding something, but it looks like you're skipping the only tests here that call rclpy_take() for Connext. I was asking if that function is tested with Connext somewhere else or if it's just left untested with Connext. If the function relies on source timestamps and is completely unusable with Connext then that's fine, but it's probably worth being more specific about that in the skip comment.

[iluetkeb]

One test is timestamp-specific (get_service_timestamps), but the other (test_take) isn't. In the C++ tests I've #ifdef'd out the check blocks only. Not sure if sth like that is possible for pyunit?

[dirk-thomas]

I'm probably misunderstanding something, but it looks like you're skipping the only tests here that call rclpy_take() for Connext. I was asking if that function is tested with Connext somewhere else or if it's just left untested with Connext.

The specific test was added in the above referenced PR and has failed for Connext since then. This patch only skips the always failing test.

Adding a separate test to check rclpy_take() without considering the source timestamp would be an option as well as modifying the test to skip part of the logic for Connext.

Since this change doesn't reduce the previously covered (and passed) code I don't think either should happen in this PR.