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

Fix rosbag2_tests resource files and play_end_to_end test #362

Merged
merged 6 commits into from
Apr 10, 2020
Merged

Fix rosbag2_tests resource files and play_end_to_end test #362

merged 6 commits into from
Apr 10, 2020

Conversation

mabelzhang
Copy link
Contributor

Re-recorded the cdr_test and wrong_rmw_test bag files in rosbag2_tests with the latest master branch, metadata version 4.
Updated play_end_to_end test to reflect the changes and re-enabled this test in CMakeLists.txt.

Only change in the bag is that the field test_msgs::msg::BasicTypes::string_value doesn't exist anymore, so it is replaced with a int32_value.
The messages are produced with a simple script here.

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Copy link
Collaborator

@Karsten1987 Karsten1987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with green CI

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang
Copy link
Contributor Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@mabelzhang
Copy link
Contributor Author

Updated .repos file. Retry:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Contributor

@piraka9011 piraka9011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warnings were addressed in #360. If you rebase, you should have green CI.

@mabelzhang
Copy link
Contributor Author

mabelzhang commented Apr 10, 2020

Rebased and new set of CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Actually it looks like the version run with CI above has already been rebased, for I only forked this branch today, and the fix was yesterday. I had to update my workspace to build with ament_export_targets. That was before I ran the first set of CI here.

So I don't know where that warning is coming from. Not from this branch - grep doesn't even show ament_export_interfaces anymore.

I'm looking at all the CIs here https://ci.ros2.org/job/ci_linux/ , and there hasn't been one green. I clicked on a few random ones, and they all have the same warning. Maybe there is one repo that hasn't been updated with the new command.

@Karsten1987
Copy link
Collaborator

That's a known issue. There's an outstanding PR for tracing tools to address the deprecation warning.

@Karsten1987
Copy link
Collaborator

Looking at CI, I think the play test doesn't pass neither on Linux nor on Windows.

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
@mabelzhang
Copy link
Contributor Author

No more test failures on Linux & macOS.
Windows tests fail on record_end_to_end. Is that new?

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@piraka9011
Copy link
Contributor

@mabelzhang yes that's a known issue on Windows.
Splitting + Compression E2E tests are flaky since there's no support for ctrl-c (SIGINT) preventing the rosbag2 node from shutting down properly and cleaning up.

This solution is to refactor the E2E tests to use launch_testing (Some of the work started in #351 and #347).

@mabelzhang mabelzhang merged commit c2b161d into ros2:master Apr 10, 2020
@mabelzhang mabelzhang deleted the fix_play_end_to_end_test branch April 10, 2020 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants