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

test_subscription_topic_statistics fails with rmw_connext_cpp #1403

Closed
brawner opened this issue Oct 13, 2020 · 4 comments
Closed

test_subscription_topic_statistics fails with rmw_connext_cpp #1403

brawner opened this issue Oct 13, 2020 · 4 comments
Assignees
Labels
bug Something isn't working Connext Connext support Linux Linux support tests Failing or missing tests

Comments

@brawner
Copy link
Contributor

brawner commented Oct 13, 2020

This is a reoccuring failure over at:
http://build.ros2.org/view/Rci/job/Rci__nightly-connext_ubuntu_focal_amd64/106/testReport/rclcpp/TestSubscriptionTopicStatisticsFixture/test_receive_stats_for_message_no_header/

It was probably introduced in #1281, which aligns with when this failure started showing up.

@iuhilnehc-ynos
Copy link
Collaborator

iuhilnehc-ynos commented Oct 19, 2020

I can reproduce this failure, and the topic echo show the publisher for topic '/statistics' might publish message (nan members), please refer to

$ ros2 topic echo /statistics statistics_msgs/msg/MetricsMessage
measurement_source_name: test_sub_stats_node
metrics_source: message_age
unit: ms
window_start:
  sec: 1603101739
  nanosec: 842547404
window_stop:
  sec: 1603101740
  nanosec: 843078333
statistics:
- data_type: 1
  data: .nan
- data_type: 3
  data: .nan
- data_type: 2
  data: .nan
- data_type: 5
  data: 0.0
- data_type: 4
  data: .nan
---
measurement_source_name: test_sub_stats_node
metrics_source: message_period
unit: ms
window_start:
  sec: 1603101739
  nanosec: 842547404
window_stop:
  sec: 1603101740
  nanosec: 843078333
statistics:
- data_type: 1
  data: .nan
- data_type: 3
  data: .nan
- data_type: 2
  data: .nan
- data_type: 5
  data: 0.0
- data_type: 4
  data: .nan
---

I am sure it's introduced in #1281.

#1281 lost logic,

    if (!has_samples) {
      continue;
    }

@ivanpauno
Copy link
Member

@dabonnie does @iuhilnehc-ynos comment make sense to you?
Would reintroducing that logic fix the issue?

@dabonnie
Copy link
Contributor

dabonnie commented Oct 28, 2020

I don't think so (context here).

For that test I'm curious why it's only failing for connext. Notice how line 243 introduces a check for empty messages, which is used at line 416 to assert that empty header messages are empty. IMHO the continue block is just hiding a failure, where we don't expect a message to have empty fields at that point.

@chapulina chapulina added Connext Connext support tests Failing or missing tests Linux Linux support labels Nov 4, 2020
@clalancette
Copy link
Contributor

Since we aren't using rmw_connext_cpp anymore, this one isn't really that relevant. So I'll close it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Connext Connext support Linux Linux support tests Failing or missing tests
Projects
None yet
Development

No branches or pull requests

7 participants