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

[rostest] Add reusable node to test parameters.#859

Closed
130s wants to merge 3 commits intoros:indigo-develfrom
130s:add/paramtest
Closed

[rostest] Add reusable node to test parameters.#859
130s wants to merge 3 commits intoros:indigo-develfrom
130s:add/paramtest

Conversation

@130s
Copy link
Copy Markdown
Member

@130s 130s commented Aug 11, 2016

Adding a node paramtest, a reusable rostest node that checks if a designated parameter is registered and non-empty.

Inspired by: hztest, and the implementation and the usage is similar to hztest.

Motivation: It's cumbersome to write your own test node when you simply want to check if a parameter is registered and/or is not null.

Usage: Hopefully the usage is obvious enough from a test file that's also added in this PR.

@dirk-thomas
Copy link
Copy Markdown
Member

Depending on how it is supposed to be used would it make sense to optionally also check if the parameter has a specific value?

@dirk-thomas
Copy link
Copy Markdown
Member

I can cherry-pick this patch manually but please make future PRs against the latest branch, currently kinetic-devel for this repo.

# POSSIBILITY OF SUCH DAMAGE.

# Original copied from hztest node
# https://github.com/ros/ros_comm/blob/24e45419bdd4b0d588321e3b376650c7a51bf11c/tools/rostest/nodes/hztest
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the code was copied it should preserve the original copyright.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The original is Willow 2008, but I'll just revert.

@130s
Copy link
Copy Markdown
Member Author

130s commented Aug 26, 2016

I added a few commits that include the following changes:

  • addressed comments.
  • feature to test specific param value.
  • use double quote consistently instead of single quote.

Btw I added the changes without squashing but once it LGT I can rebase (or please rely on github's feature).

I can cherry-pick this patch manually but please make future PRs against the latest branch, currently kinetic-devel for this repo.

Sure. I'm still mostly on indigo but changes like this can be easily tested on cloud.

@dirk-thomas
Copy link
Copy Markdown
Member

The patch currently fails on test.

@130s
Copy link
Copy Markdown
Member Author

130s commented Aug 31, 2016

Btw where does buildfarm obtain the email that it sends out the build result to? It's using my old one (130s@lateeye.net) that I stopped using awhile ago.

@dirk-thomas
Copy link
Copy Markdown
Member

Some tests are still failing.

I assume Jenkins has fetched the email address of your user earlier and has cached that value since then. I think you should be able to update your profile yourself: http://build.ros.org/user/130s/configure If not please let me know what I should update the email address to.

@130s
Copy link
Copy Markdown
Member Author

130s commented Sep 10, 2016

Tests passed. And thanks, I was able to resume getting notifications from buildfarm.

@dirk-thomas
Copy link
Copy Markdown
Member

Thanks for iterating on this. I have squashed and cherry-picked the change to the kinetic-devel branch: c485654

@130s 130s deleted the add/paramtest branch September 14, 2016 22:56
@130s
Copy link
Copy Markdown
Member Author

130s commented Sep 15, 2016

Thank you for the review and acceptance. Do new functionality only go to the newest branch for ros_comm? While I now understand the PR should be targetted at the latest branch, I want non-api/abi breaking change like this to be ported to older supported distros.

@dirk-thomas
Copy link
Copy Markdown
Member

Any change should go into the latest development branch. From there certain changes can be backported. After they have been available in the latest distribution for a while and increased the confidence that they don't cause regressions.

But backports should happen only when there is a "good reason" to do so (e.g. fix a significant bug). Not every feature should be backported. The reasons not to do it are extra effort and chance of regressions. While I do understand that it would be nice to always have every change all distros the reason why we have distros is to keep already released distros as stable as possible and add new stuff only into new distros.

So each request to backport specific changes needs to be considered carefully.

130s added a commit to 130s/rtmros_nextage that referenced this pull request Oct 17, 2016
130s added a commit to 130s/rtmros_nextage that referenced this pull request Oct 25, 2016
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.

2 participants