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

use transient_local and longer keep-alive for pub tests #546

Merged
merged 2 commits into from
Jul 6, 2020

Conversation

dirk-thomas
Copy link
Member

Follow up of #544. Fix flakiness of pub_once test added in #304.

E.g. https://ci.ros2.org/view/nightly/job/nightly_linux-centos_extra_rmw_release/459/testReport/ros2topic.ros2topic.test/test_cli/test_cli/

CI build testing ros2topic:

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

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas self-assigned this Jun 30, 2020
@dirk-thomas dirk-thomas requested a review from hidmic July 1, 2020 16:43
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM!

ros2topic/test/test_cli.py Outdated Show resolved Hide resolved
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas merged commit 95bfb05 into master Jul 6, 2020
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/use-transient_local-for-pub-tests branch July 6, 2020 17:21
sloretz pushed a commit that referenced this pull request Sep 10, 2020
* use transient_local and longer keep-alive for pub tests

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* add comment to document unit of --keep-alive

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
sloretz added a commit that referenced this pull request Sep 10, 2020
…nsient_local and longer keep-alive for pub tests (#546) Use reliable QoS for ros2topic tests (#555) (#565)

* add --keep-alive option to 'topic pub' (#544)

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Give kwarg default value for backporting

keep_alive is made a keyword argument with a default value of 0.1 so the
pull request can be backported.

Signed-off-by: Shane Loretz<sloretz@openrobotics.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* use transient_local and longer keep-alive for pub tests (#546)

* use transient_local and longer keep-alive for pub tests

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* add comment to document unit of --keep-alive

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Use reliable QoS for ros2topic tests (#555)

The TestROS2TopicCLI tests perform feature testing of the ros2topic
command line interface. If the system is under stress during these
tests, messages may be lost (by design). If that happens, there is a
fairly high likelihood that the test_topic_pub_once test will fail
because there is only one opportunity for the message to be successfully
transported. We're likely dropping other messages in this suite, but the
other tests continuously publish until one of the messages is received
(or a timeout occurs), making them significantly less likely to fail.

Using the 'reliable' setting for QoS reliability seems to make the tests
consistently pass, even when the system is placed under additional
stress.

Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Co-authored-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Co-authored-by: Scott K Logan <logans@cottsay.net>
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.

2 participants