ADVRHumanoids / ROSEndEffector

ROS End-Effector package: provides a ROS-based set of standard interfaces to command robotics end-effectors in an agnostic fashion
Apache License 2.0
27 stars 6 forks source link

[Tests] Running tests should not require external roscore #40

Closed alaurenzi closed 2 years ago

alaurenzi commented 4 years ago

Hi @torydebra, I propose you an enhancement of your unit tests. I've seen that they rely on the "external" ROS network running. However, this is not optimal since tests are not isolated from the environment (plus you need to manually launch the roscore maybe). My suggestion is:

You can have a look here

Interesting files:

torydebra commented 4 years ago

Thanks for the issue, you are right

I followed your code but a have a problem. The first test_f run smoothly, the other fails. Here is the output:

1: [ RUN      ] testComposedAction.checkNumberPrimitives
1: [ERROR] [1587720981.206676773]: [setParam] Failed to contact master at [localhost:11322].  Retrying...
1: [ WARN] [1587720981.206086064]: SETTING PARAMS!!!!!
1: [ INFO] [1587720982.213810437]: Connected to master at [localhost:11322]
1: [PARSER::init]: Parsed Model: two_finger_end_effector
1: ... logging to /home/tori/.ros/log/0e78ceec-860f-11ea-8f17-90489ada8949/roslaunch-Aspire-E15-25129.log
[...]
1: Killed process 'roscore' with signal 2
1: Child process 'roscore' exited with status 0
1: [       OK ] testComposedAction.checkNumberPrimitives (2338 ms)
1: [ RUN      ] testComposedAction.checkEmitParse
1:  [PARSER::init]: Fail To load robot model robot_description are you sure to have put both urdf and srdf files in the parameter server with names robot_description and robot_description_semantic, respectively?
1: /home/tori/ROSEE/src/ROSEndEffector/test/test_composedAction.cpp:51: Failure
1: Value of: parserMoveIt->init ("robot_description")
1:   Actual: false
1: Expected: true
1: ... logging to /home/tori/.ros/log/0fde48a2-860f-11ea-8f17-90489ada8949/roslaunch-Aspire-E15-25237.log

Here is the code:

virtual void SetUp() override {

        //run roscore
        _roscore.reset(new ROSEE::TestUtils::Process({"roscore", "-p", "11322"}));

        //fill ros param with file models, needed by moveit parserMoveIt
        std::string modelPath = ROSEE::Utils::getPackagePath() + "configs/urdf/" + HAND_NAME_TEST;

        //Is there a better way to parse?
        std::ifstream urdf(modelPath + ".urdf");
        std::ifstream srdf(modelPath + ".srdf");
        std::stringstream sUrdf, sSrdf;
        sUrdf << urdf.rdbuf();
        sSrdf << srdf.rdbuf();

        ROS_WARN_STREAM ("SETTING PARAMS!!!!!");

        ros::param::set("robot_description" , sUrdf.str());
        ros::param::set("robot_description_semantic" , sUrdf.str());

        std::shared_ptr <ROSEE::ParserMoveIt> parserMoveIt = std::make_shared <ROSEE::ParserMoveIt> ();

        //if return false, models are not found and it is useless to continue the test
        ASSERT_TRUE(parserMoveIt->init ("robot_description")) ;
        [...]

It seems that in the second test (checkEmitParse), it directly "jump" to ASSERT_TRUE(parserMoveIt->init ("robot_description")) ; . It does not even print the WARN I have put. It seems that the parameters are set only for the first test.

So nonsense for me, maybe I am missing something about google tests.

alaurenzi commented 4 years ago

What if you create the roscore inside main()?

torydebra commented 4 years ago

What if you create the roscore inside main()?

It works :+1:

torydebra commented 4 years ago

So, I have put the new versions of test on branch test_issue40. I won't still open the pull request on devel (to have also @liesrock check modifications) because I want to find a nice way to run all the test on different hands. Previously this was done putting in the param server the robot models and then call make test ARGS="-V". Being now the roscore "inside" the test, this is not possible anymore. For now I use a #define inside the test code to define which hand to test.

alaurenzi commented 4 years ago

A possible solution is to pass the hand name via command line. Then, from the cmake, you create different test targets from the same executable varying the argument, like this example from xbot2_wip which reuses the same executable TestIpcSenderCreate to test for different scenarios (shm, sock, mq, iddp)

add_executable(TestIpcSenderCreate ipc_test.cpp)
target_link_libraries(TestIpcSenderCreate ${TestLibs})
add_test(NAME TestIpcSenderCreateShm  COMMAND TestIpcSenderCreate shm)
add_test(NAME TestIpcSenderCreateSock COMMAND TestIpcSenderCreate sock)
add_test(NAME TestIpcSenderCreateMq   COMMAND TestIpcSenderCreate mq)
add_test(NAME TestIpcSenderCreateIddp COMMAND TestIpcSenderCreate iddp)
torydebra commented 4 years ago

Thanks! We have _catkin_addgtest instead of _addtest and it seems we can't put arguments link

I tried using your _addtest but it says:

1: Test command:  "two_finger"
Unable to find executable: /home/tori/ROSEE/devel/lib/ros_end_effector/EEInterface_test
1/4 Test #1: EEInterfaceTestTwoFinger ............................***Not Run   0.00 sec

I also tried to put all previous cmake stuff like your file here but nothing.

Plus, with this add_test, it compiles well with make tests (done inside build folder) but it can't find the various testing:: functions if I compile with catkin_make (maybe catkin_add_gtest hides some library additions?)

torydebra commented 4 years ago

Solved, I only needed to add gtest dependency. I also find an old bug by the way.

Now, the only annoying things is this print:

10: [ERROR] [1588160401.295087866]: [setParam] Failed to contact master at [localhost:11322].  Retrying...

This is caused because roscore need some time to start. It is only a print, after a while the test continues well, so it is not a big problem. I tried to "wait" for master with ros::master::check() but it seems to wait endlessy, maybe because the roscore is runned in the code with the hack?

torydebra commented 4 years ago

I found another issue with travis: If you see this build https://travis-ci.org/github/ADVRHumanoids/ROSEndEffector/builds/680967786 you can see that (at the end) one test fails ( the problem is solved however) but travis says the build is ok. Why?

torydebra commented 4 years ago

I found another issue with travis: If you see this build https://travis-ci.org/github/ADVRHumanoids/ROSEndEffector/builds/680967786 you can see that (at the end) one test fails ( the problem is solved however) but travis says the build is ok. Why?

I solved this using ctest --verbose instead of make test ARGS="-V" -i

Now pull request is ready

torydebra commented 2 years ago

Relative pull request #73 has been merged into master