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

[roscpp] throw exception on ros::init with empty node name#894

Closed
furushchev wants to merge 2 commits intoros:indigo-develfrom
furushchev:cpp-empty-node-name
Closed

[roscpp] throw exception on ros::init with empty node name#894
furushchev wants to merge 2 commits intoros:indigo-develfrom
furushchev:cpp-empty-node-name

Conversation

@furushchev
Copy link
Copy Markdown
Contributor

according to discussion at #891

@furushchev furushchev changed the title throw exception on ros::init with empty node name [roscpp] throw exception on ros::init with empty node name Sep 11, 2016
@furushchev
Copy link
Copy Markdown
Contributor Author

furushchev commented Sep 11, 2016

at Ipr__ros_comm__ubuntu_trusty_amd64:

Traceback (most recent call last):
  File "/usr/lib/python2.7/unittest/case.py", line 331, in run
    testMethod()
  File "/tmp/catkin_workspace/src/ros_comm/test/test_rosbag/test/test_bag.py", line 298, in test_get_compression_info
    self.assertLess(info.compressed, 1000)
  File "/usr/lib/python2.7/unittest/case.py", line 932, in assertLess
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python2.7/unittest/case.py", line 412, in fail
    raise self.failureException(msg)
AssertionError: 1006 not less than 1000

it looks not related to my changes.
need restarting test?

@dirk-thomas
Copy link
Copy Markdown
Member

That looks indeed like a flaky one. @ros-pull-request-builder retest this please

@furushchev
Copy link
Copy Markdown
Contributor Author

@dirk-thomas Thank you for restarting. However it looks still failed with the same error

Traceback (most recent call last):
  File "/usr/lib/python2.7/unittest/case.py", line 331, in run
    testMethod()
  File "/tmp/catkin_workspace/src/ros_comm/test/test_rosbag/test/test_bag.py", line 298, in test_get_compression_info
    self.assertLess(info.compressed, 1000)
  File "/usr/lib/python2.7/unittest/case.py", line 932, in assertLess
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python2.7/unittest/case.py", line 412, in fail
    raise self.failureException(msg)
AssertionError: 1006 not less than 1000


if (name.empty()) {
std::stringstream ss;
ss << "Name [" << name << "] must not be empty";
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.

Since the name is empty anyway (as checked two lines above) there is no need to build the message using stringstream. throw InvalidNameException("The node name must not be empty") should be sufficient.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dirk-thomas Thank you for reviewing! I updated my pull request.

@dirk-thomas
Copy link
Copy Markdown
Member

Thank you for the improvement. I have cherry picked the changes to the kinetic-devel branch: bd3af70

Please make future PRs against the latest development branch (currently kinetic-devel).

@furushchev furushchev deleted the cpp-empty-node-name branch September 15, 2016 05:38
@furushchev
Copy link
Copy Markdown
Contributor Author

@dirk-thomas Thank you for merging! OK, I'll create pr for kinetic from next time! 👍

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