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

rostest: use AnyMsg in publishtest #1659

Merged
merged 2 commits into from
Mar 26, 2019
Merged

rostest: use AnyMsg in publishtest #1659

merged 2 commits into from
Mar 26, 2019

Conversation

beetleskin
Copy link
Contributor

@beetleskin beetleskin commented Mar 15, 2019

Right now publishtest only works correctly when a publisher of the given topic exists: If no publisher exists, the retrieval of the topic class blocks until the test times out. This leads to aborted tests when one would expect a failed one with correct error message, stating that no message on the given topic was published. Using rospy.AnyMsg fixes that.

This also makes it impossible to test that a certain topic is not being published, when no publisher exists.

Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

Please create separate PRs for unrelated changes: one for the rospy.AnyMsg change and one for the retry flag to address the flakiness.

tools/rostest/CMakeLists.txt Outdated Show resolved Hide resolved
tools/rostest/test/publishtest.test Outdated Show resolved Hide resolved
tools/rostest/test/hztest.test Outdated Show resolved Hide resolved
@beetleskin beetleskin changed the title publishtest: use AnyMsg in publishtest rostest: use AnyMsg in publishtest Mar 15, 2019
@beetleskin
Copy link
Contributor Author

Done, anything missing?

@dirk-thomas
Copy link
Member

LGTM. Thanks for the patch.

@dirk-thomas dirk-thomas merged commit 40d3ca4 into ros:melodic-devel Mar 26, 2019
@beetleskin beetleskin deleted the fix_publishtest branch March 26, 2019 23:09
tahsinkose pushed a commit to tahsinkose/ros_comm that referenced this pull request Apr 15, 2019
* publishtest: use AnyMsg in publishtest

* fix concerns
@beetleskin
Copy link
Contributor Author

any plans to release this?

@dirk-thomas
Copy link
Member

any plans to release this?

See #1802.

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

Successfully merging this pull request may close these issues.

3 participants