Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

rostest: use AnyMsg in publishtest#1659

Merged
dirk-thomas merged 2 commits intoros:melodic-develfrom
beetleskin:fix_publishtest
Mar 26, 2019
Merged

rostest: use AnyMsg in publishtest#1659
dirk-thomas merged 2 commits intoros:melodic-develfrom
beetleskin:fix_publishtest

Conversation

@beetleskin
Copy link
Copy Markdown
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
Copy Markdown
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.

@beetleskin beetleskin changed the title publishtest: use AnyMsg in publishtest rostest: use AnyMsg in publishtest Mar 15, 2019
@beetleskin
Copy link
Copy Markdown
Contributor Author

Done, anything missing?

@dirk-thomas
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

any plans to release this?

@dirk-thomas
Copy link
Copy Markdown
Member

any plans to release this?

See #1802.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants