Skip to content

to fix flaky test about TestTimeSource.callbacks#2111

Merged
clalancette merged 1 commit intoros2:rollingfrom
iuhilnehc-ynos:topic-flaky-test-timesource-callbacks
Feb 24, 2023
Merged

to fix flaky test about TestTimeSource.callbacks#2111
clalancette merged 1 commit intoros2:rollingfrom
iuhilnehc-ynos:topic-flaky-test-timesource-callbacks

Conversation

@iuhilnehc-ynos
Copy link
Copy Markdown
Collaborator

@iuhilnehc-ynos iuhilnehc-ynos commented Feb 24, 2023

I encounter the falky test about TestTimeSource.callbacks some times.

/tmp/ws/src/rclcpp/rclcpp/test/rclcpp/test_time_source.cpp:426
Expected equality of these values:
  3
  cbo.last_precallback_id_
    Which is: 2

Flaky test failure history:
https://build.ros2.org/job/Rpr__rclcpp__ubuntu_jammy_amd64/602/
https://build.ros2.org/job/Rpr__rclcpp__ubuntu_jammy_amd64/597/
https://build.ros2.org/job/Rpr__rclcpp__ubuntu_jammy_amd64/547/
https://build.ros2.org/job/Rpr__rclcpp__ubuntu_jammy_amd64/530/

Let me explain why the flaky test happened.
If

trigger_clock_changes(node, ros_clock);
is called and
the ros_clock is set with 4.000001000s at this moment,
While calling
trigger_clock_changes(node, ros_clock);
,
and if the message published by clock_pub in
clock_pub->publish(msg);
can't be immediately received by clock_sub in some situation, the clock->now().nanoseconds() >= end_time.count() will be always True and return so quick that the new callback
std::bind(&CallbackObject::pre_callback, &cbo, 3),
will not be called.

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@iuhilnehc-ynos
Copy link
Copy Markdown
Collaborator Author

I mentioned this flaky test in #1995 (comment), which points to reason.

@iuhilnehc-ynos iuhilnehc-ynos marked this pull request as ready for review February 24, 2023 07:14
Copy link
Copy Markdown
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me with green CI.

Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@fujitatomoya
Copy link
Copy Markdown
Collaborator

this addresses #2093

@fujitatomoya
Copy link
Copy Markdown
Collaborator

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants