RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/humble/Concepts/Basic/About-Client-Libraries.html?highlight=rclnodejs#community-maintained
Apache License 2.0
316 stars 70 forks source link

fix tests in foxy/ubuntu-20.04 #749

Open koonpeng opened 3 years ago

koonpeng commented 3 years ago

I'm not sure if I'm the only one having this problem, the tests are failing for me in foxy/ubuntu-20.04. I actually saw in problem before in other projects and it is due to fastRTPS not correctly exported some cmake files. The simple workaround is to add AMENT_PREFIX_PATH to CMAKE_PREFIX_PATH.

I also try to simplify the compiling step by using colcon and sourcing the output instead of copying files around.

Edit: Actually all the previous build have been failing but it seems tests still passes

> rclnodejs@0.17.0 test /root/rclnodejs
> node ./scripts/compile_tests.js && node --expose-gc ./scripts/run_test.js && npm run dtslint

Starting >>> cpp_nodes

Starting >>> rclnodejs_test_msgs

--- stderr: cpp_nodes
/root/rclnodejs/test/cpp/add_two_ints_client.cpp: In function ‘example_interfaces::srv::AddTwoInts_Response_<std::allocator<void> >::SharedPtr send_request(rclcpp::Node::SharedPtr, rclcpp::Client<example_interfaces::srv::AddTwoInts>::SharedPtr, example_interfaces::srv::AddTwoInts_Request_<std::allocator<void> >::SharedPtr)’:
/root/rclnodejs/test/cpp/add_two_ints_client.cpp:48:23: error: ‘rclcpp::executor::FutureReturnCode’ has not been declared
   48 |     rclcpp::executor::FutureReturnCode::SUCCESS)
      |                       ^~~~~~~~~~~~~~~~
/root/rclnodejs/test/cpp/add_two_ints_client.cpp: In function ‘int main(int, char**)’:
/root/rclnodejs/test/cpp/add_two_ints_client.cpp:94:23: error: ‘rclcpp::executor::FutureReturnCode’ has not been declared
   94 |     rclcpp::executor::FutureReturnCode::SUCCESS)
      |                       ^~~~~~~~~~~~~~~~
/root/rclnodejs/test/cpp/add_two_ints_client.cpp: In function ‘example_interfaces::srv::AddTwoInts_Response_<std::allocator<void> >::SharedPtr send_request(rclcpp::Node::SharedPtr, rclcpp::Client<example_interfaces::srv::AddTwoInts>::SharedPtr, example_interfaces::srv::AddTwoInts_Request_<std::allocator<void> >::SharedPtr)’:
/root/rclnodejs/test/cpp/add_two_ints_client.cpp:54:1: warning: control reaches end of non-void function [-Wreturn-type]
   54 | }
      | ^
make[2]: *** [CMakeFiles/add_two_ints_client.dir/build.make:63: CMakeFiles/add_two_ints_client.dir/add_two_ints_client.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:84: CMakeFiles/add_two_ints_client.dir/all] Error 2
make: *** [Makefile:95: all] Error 2
---
Failed   <<< cpp_nodes [7.06s, exited with code 2]
coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 91.268% when pulling c78abbdb5c5c0dbefb39de2822de2f356765de29 on build/fix-tests-foxy into 45d7bd0a5cbc7b9d5bd6b397b33abc770845771b on develop.

koonpeng commented 3 years ago

The windows ci is using a ci build of ros2 which mistakenly exports local directories.

  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include;C:/ci/ws/install/include"

This causes cpp_nodes to fail, I tried with a release build of ros2 and it works. The test still passes because it seems cpp_nodes is not used in windows tests. Should I use the ci build and exclude cpp_nodes from the build, or switch to a release build? The downside of the release build is that we need to update it when new ros version gets released.

minggangw commented 3 years ago

I prefer we use the rolling-distribution for develop branch, meanwhile, stable release matches its corresponding brach of rclnodejs, e.g. foxy-fitzroy uses ROS 2 Foxy Fitzroy - Patch Release 3 for ci. Thus, we could track the latest changes and fix the problem before it's officially released on develop branch.

But the rolling-distribution (or nightly build) is not stable although it's called stable. I ever met similar issues before, and the problems often got fixed in the next package release. I think we could exclude cpp_nodes because those tests that use cpp_nodes are not running any more, or we could wait a few days until the next ROS2 package (https://ci.ros2.org/view/packaging/job/packaging_windows/lastSuccessfulBuild/artifact/ws/ros2-package-windows-AMD64.zip) is ready.

koonpeng commented 3 years ago

I went back a few weeks and they all have the same error https://ci.appveyor.com/project/minggangw/rclnodejs/builds/36303858

CMake Error in CMakeLists.txt:
  Imported target "rclcpp::rclcpp" includes non-existent path
    "C:/ci/ws/install/include"
  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:
  * The path was deleted, renamed, or moved to another location.
  * An install or uninstall procedure did not complete successfully.
  * The installation package was faulty and references files it does not
  provide.

maybe it's a unreported bug?

koonpeng commented 3 years ago

made an issue https://github.com/ros2/ros2/issues/1068

minggangw commented 3 years ago

I suspect it was misconfigured, let's wait for the reply from the upstream.

minggangw commented 3 years ago

I notice there is a new Window ROS2 package generated (https://ci.ros2.org/view/packaging/job/packaging_windows/1960/), so the problem may have been fixed.

koonpeng commented 3 years ago

It looks like the issue still isn't fixed :disappointed: