Rostest example #71

ValRat · 2017-01-26T07:06:42Z

Closes #66, closes #67

ValRat · 2017-01-26T15:51:28Z

Basics have been written, need to change some CMakeList entries:
instead of catkin_add_gtest(), do add_rostest_gtest() since the former runs the node external of the ros computational network (which isn't what you want to do when doing integrated testing)
Reference

Still need to add a blurb about where results are located.

garethellis0 · 2017-01-27T01:09:32Z

README.md

@@ -108,11 +109,19 @@ some_ros_package
 |   | <b>my_node.cpp</b>
 </pre>

+## Launch files


😬 can't believe I didn't put a section on this in the readme..... good idea! 😃

garethellis0 · 2017-01-27T01:14:58Z

Also, in the example given in sample_package, I'd just be able to launch it, point it at some gtest tests, and it'd run roscore, allowing me to run my node and test methods directly, rather then testing the external publish/subscribe interface?

ValRat · 2017-01-30T02:38:01Z

In response to above (for documentation of discussion sake):
With rostest you'd be able to launch the node and test the publish/subscribe interface. Testing member methods should be done at the gtest unit test levels.

garethellis0

I'll do a full review, just happened to spot these while I was using your PR as reference for how to use setup / test fixtures

garethellis0 · 2017-01-30T03:20:39Z

src/sample_package/test/my_node_rostest.cpp

+    }
+};
+
+TEST_F(MyClassTest, MyNodeTest){


I'm thinking that perhaps we should name the test class MyNodeTest, with the second name being reserved to indicate very briefly the functionality you're trying to test.

garethellis0 · 2017-01-30T03:50:33Z

src/sample_package/test/my_node_rostest.cpp

+
+/**
+ * This is the helper class which will publish and subscribe messages which will test the node being instantiated
+ * It contains on the minimum:


"at the minimum" might be a better way of phrasing it

garethellis0 · 2017-02-04T07:43:54Z

README.md

@@ -94,7 +95,8 @@ some_ros_package
 |   | <b>MyNode.h</b>
 | 
 └───test
-|   | <b>my-node-test.cpp</b>
+|   | <b>my_node_test.cpp</b>


As we've decided to only change the naming convention to use underscores for ros-tests, this should still be my-node-test.cpp

Also, my_node_rostest.cpp should be in this hierarchy

garethellis0 · 2017-02-04T07:46:26Z

README.md

 ## Testing
 - GTest is our primary testing tool at the moment. The ROS wiki has a quick intro to it [here](http://wiki.ros.org/gtest), and we also strongly recommend you read Google's introduction to it [here] (https://github.com/google/googletest/blob/master/googletest/docs/Primer.md), then setup and write a few example tests before you start using it with ROS.
 - Once you've setup your tests in ROS, run `catkin_make run_tests` to run them
 - To run the tests for a specific package, run `catkin_make run_tests_MY_PACKAGE_NAME`

+### Rostest
+- For tests which require more than one active node, i.e. integrated testing, the rostest framework provides a way to launch your test alongside all the nodes it requires. This is  an extension on roslaunch enabling it to run test nodes. Special test nodes are nested within a `<test></test>` tag. This also needs a special entry under CMakelists as shown in the sample package. See more details [here](http://wiki.ros.org/rostest)


"This is an extension..." has an extra space

garethellis0 · 2017-02-04T07:50:16Z

src/sample_package/test/my_node_rostest.cpp

+
+    // spinOnce allows ros to actually process your callbacks
+    // for the curious:
+    //      http://answers.ros.org/question/11887/significance-of-rosspinonce/


Nitpicky thing, but lines 60 & 61 can be one line

garethellis0 · 2017-02-04T23:20:33Z

src/sample_package/test/my_node_rostest.cpp

+        messageOutput = msg->data.c_str();
+    }
+
+    std::string getMessage(){


I don't think we need this function. I believe test fixtures have access to protected variables. Might just be cleaner to make it a direct access. (that being said, that's kind of icky) or just making the messageOutput private.

garethellis0 · 2017-02-07T04:41:35Z

src/sample_package/test/my_node_rostest.cpp

+    ros::NodeHandle nh_;
+    std::string messageOutput;
+    ros::Publisher testPublisher;
+    ros::Subscriber testSubscriber;


Almost last thing - promise. Variables should be written_like_this

garethellis0 · 2017-02-07T04:42:06Z

src/sample_package/test/my_node_rostest.cpp

+    }
+};
+
+TEST_F(MyNodeTest, ExclamationMarkAppend){


~~just going off how tests have been named so far, this should be exclamationMarkAppendTest~~

Actually, thinking about it some more, that convention is pretty stupid. There's really no reason to have the word "test" on the end of each test, so exclamationMarkAppend would be fine (note changes to capitalization)

garethellis0 · 2017-02-07T04:43:14Z

I keep coming back to approve it and seeing stuff... sorry... 😆

garethellis0 · 2017-02-11T19:03:10Z

What could possibly go wrong?

ValRat added 6 commits January 23, 2017 23:14

Double check syntaxes

72efdd6

rostest blurb

f81ce9c

Added example commands

680a401

Readme formatting

22e4495

FORMAT

3c35a68

Format 2

c6a0302

garethellis0 reviewed Jan 27, 2017

View reviewed changes

Pass work onto desktop - investigate timing

9f5df76

garethellis0 mentioned this pull request Jan 30, 2017

Add info on launchfiles to readme #75

Closed

ValRat mentioned this pull request Jan 30, 2017

Update .launch file in sample_package with examples of topic remapping and multiple node instances #76

Open

ValRat added 3 commits January 29, 2017 18:07

Finalized timing constraints, added docs

1c09323

Reverted unintentional name change of original test node

7d94299

amended gtest and rostest naming conventions

698a008

garethellis0 reviewed Jan 30, 2017

View reviewed changes

garethellis0 requested changes Feb 4, 2017

View reviewed changes

Convention amendments

5737358

garethellis0 requested changes Feb 4, 2017

View reviewed changes

Removed unecessary getter

e115908

garethellis0 requested changes Feb 7, 2017

View reviewed changes

Variable naming conventions

70fa6bf

garethellis0 approved these changes Feb 11, 2017

View reviewed changes

garethellis0 merged commit 451f557 into master Feb 11, 2017

ValRat deleted the rostest_example branch August 13, 2017 23:41

garethellis0 mentioned this pull request Sep 9, 2017

Add info on testing with setup function to readme #77

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rostest example #71

Rostest example #71

ValRat commented Jan 26, 2017

ValRat commented Jan 26, 2017 •

edited

Loading

garethellis0 Jan 27, 2017

garethellis0 commented Jan 27, 2017

ValRat commented Jan 30, 2017

garethellis0 left a comment

garethellis0 Jan 30, 2017

garethellis0 Jan 30, 2017

garethellis0 Feb 4, 2017

garethellis0 Feb 4, 2017

garethellis0 Feb 4, 2017

garethellis0 Feb 4, 2017

garethellis0 Feb 4, 2017 •

edited

Loading

garethellis0 Feb 7, 2017

garethellis0 Feb 7, 2017 •

edited

Loading

garethellis0 Feb 7, 2017

garethellis0 commented Feb 7, 2017

garethellis0 commented Feb 11, 2017

Rostest example #71

Rostest example #71

Conversation

ValRat commented Jan 26, 2017

ValRat commented Jan 26, 2017 • edited Loading

Choose a reason for hiding this comment

garethellis0 commented Jan 27, 2017

ValRat commented Jan 30, 2017

garethellis0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garethellis0 Feb 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garethellis0 Feb 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garethellis0 commented Feb 7, 2017

garethellis0 commented Feb 11, 2017

ValRat commented Jan 26, 2017 •

edited

Loading

garethellis0 Feb 4, 2017 •

edited

Loading

garethellis0 Feb 7, 2017 •

edited

Loading