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 can load tests from a dotted name #722

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@heuristicus
Copy link
Contributor

commented Dec 28, 2015

This is a simple solution to the issue, using unittest.TestLoader().loadTestsFromName. The dual-use (str or unittest.TestCase) of the test parameter could be changed to use a kwarg instead. That would necessitate test being an optional parameter which specifies the unittest.TestCase class, and some other parameter (test_from_name or something?) specifying a string from which tests should be loaded. That would probably require additional checks to ensure that at least one of the optional parameters existed.

You can check that the name-based loading works by changing line 56 in test_rosbag to

rostest.rosrun('test_rosbag', 'latched_pub', 'latched_pub.LatchedPub', sys.argv)

Here's a test you can run to check that suite loading works correctly. You can run it with rosrun test_rosbag tmp.py. Don't forget to chmod +x.

ros_comm/test/test_rosbag/test/tmp.py:

#!/usr/bin/env python

import unittest
import rostest
import rospy
import sys

class CaseA(unittest.TestCase):

    def runTest(self):
        self.assertTrue(True)

class CaseB(unittest.TestCase):

    def runTest(self):
        self.assertTrue(True)

class SuiteTest(unittest.TestSuite):

    def __init__(self):
        super(SuiteTest, self).__init__()
        self.addTest(CaseA())
        self.addTest(CaseB())

if __name__ == '__main__':
      rostest.rosrun('test_rosbag', 'temp_tests', 'tmp.SuiteTest', sys.argv)

Fixes #423.

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Dec 28, 2015

Can you please add a unit test for the new feature to this PR.

@heuristicus

This comment has been minimized.

Copy link
Contributor Author

commented Dec 29, 2015

I've added some tests, but couldn't really come up with a way of programmatically ensuring that the expected tests are actually loaded. I'm just assuming that loadTestsFromName will crash if you give it incorrect values for the dotted name. I'm not sure if there is a way to test expected failures, like giving a non-string, non-unittest.TestCase object to rosrun in the test parameter.

heuristicus added a commit to heuristicus/ros that referenced this pull request Dec 29, 2015

@heuristicus heuristicus force-pushed the heuristicus:indigo-devel branch 2 times, most recently from 5bb7578 to 6bdba57 Dec 29, 2015

@stonier

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2016

Had a chat about this - test should probably be testing the function itself (nosetest?) rather than running a rostest and crashing rostest itself.

@heuristicus heuristicus force-pushed the heuristicus:indigo-devel branch from 6bdba57 to 930a4a8 Jan 8, 2016

@heuristicus

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2016

I've moved the tests to using the python unittest framework, and added some test cases for expected failures.

Also squashed to a single commit.

@heuristicus heuristicus force-pushed the heuristicus:indigo-devel branch 4 times, most recently from 61ed672 to 0b693b4 Jan 8, 2016

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Mar 1, 2016

@ros-pull-request-builder test this please

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Mar 1, 2016

@ros-pull-request-builder retest this please

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Mar 2, 2016

Thank you for the improvement.

Jenkins is actually fine: http://build.ros.org/view/Jpr/job/Jpr__ros_comm__ubuntu_trusty_amd64/50/

I have cherry-picked the patch with the same modifications as ros/ros#104: 4c837a7

@dirk-thomas dirk-thomas closed this Mar 2, 2016

dirk-thomas added a commit that referenced this pull request Sep 19, 2016

@dirk-thomas

This comment has been minimized.

Copy link
Member

commented Sep 19, 2016

Several callers pass types instead of instances of unittest.TestCase. Therefore I had to relax the check: e341225

otamachan added a commit to otamachan/ros_comm that referenced this pull request Feb 1, 2017

dirk-thomas added a commit that referenced this pull request Mar 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.