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 #1649

Closed
wants to merge 2 commits into from
Closed

rostest: use AnyMsg in publishtest #1649

wants to merge 2 commits into from

Conversation

beetleskin
Copy link
Contributor

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.

@beetleskin
Copy link
Contributor Author

beetleskin commented Mar 12, 2019

Uff .. the failing test is a hztest, without retry argument. This is unfortunate, this never works reliably on CI with shared resources. I could add a retry argument, but this change is not about fixing those tests for CI.

Other instances of this from the recent PRs:

How can I just trigger the build again?

@beetleskin
Copy link
Contributor Author

beetleskin commented Mar 12, 2019

How can I just trigger the build again?

https://stackoverflow.com/a/28703197/1880234
.. but now we have this empty commit in there :/

@beetleskin beetleskin closed this Mar 15, 2019
@dirk-thomas
Copy link
Member

dirk-thomas commented Mar 15, 2019

I am not sure why you closed the ticket. The empty commit is not a problem - at the end the PR will be squash merged anyway. Do you want to reopen this?

Nvm, just saw that you also deleted the fork...

@beetleskin
Copy link
Contributor Author

messed up my fork, there you go: #1659

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