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
320 stars 70 forks source link

rm rclnodejs_test_msgs, compile script, cpp & py dirs, consolidates linux workflows [#856] #860

Closed wayneparrott closed 2 years ago

wayneparrott commented 2 years ago

The main objective of this PR is to remove the troublesome compile_scripts.js, rclnodejs_test_msgs pkg and related tests and cpp/py example clients from the repo.

  1. removed test/rclnodejs_test_msgs and related tests
    • removed scripts/compile_tests.js and package.json script references
    • replaced reference to Fibonacci action msgs in test-action-*.js and ts types test files with equivalent msgs in test_msgs package

2 - discontinued testing compatibility with cpp and py rcl ros clients

3 - Replace test-non-primitive-static-array.js with test-non-primitive-array.js

4 - Updated linux workflows

5 - updated npmjs-readme.md

6 - fixed rolling specific issues bindings.gyp test-raw-pub-sub.js

Fix #856

wayneparrott commented 2 years ago

Odd that only the windows CI is failing the test-raw-pub-sub.js test on the rolling . I would have expected linux CI to fail in similar manner.

1) rclnodejs publisher test suite
       Publish raw serialized messages:
      Uncaught AssertionError [ERR_ASSERTION]: distro: rolling, buffer: Hello ROS World, msg: Hello ROS World
      + expected - actual
      -1
      +0
      at Subscription._callback (test\test-raw-pub-sub.js:58:16)
...
wayneparrott commented 2 years ago

poop! the most recent change to test-raw-pub-sub just move the fail from windows to linux os. Will investigate this more but does this seem legit for there to be difference in msg content between win and linux os'es?

minggangw commented 2 years ago

I think we can disable test-raw-pub-sub.js temporarily on rolling if the failure is caused by the null-terminated issue because the ROS2 release changes back and forth and it seems that the behavior is different on Linux and Windows now. We don't have to change our tests accordingly, as the rolling itself is not a stable release. I don't want we spend so much time in solving kind of problem, what do you think?

wayneparrott commented 2 years ago

Agree with disabling the test for now. Will update shortly. Thx