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

[iron] Fix for regression in open_succeeds_twice and minimal_writer_example tests (backport #1667) #1669

Merged
merged 2 commits into from
May 22, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented May 21, 2024

The error message from CI reference build:

/home/jenkins-agent/workspace/nightly_linux-aarch64_repeated/ws/src/ros2/rosbag2/rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp:242
Expected: writer_->open(storage_options, {serialization_format_, serialization_format_}) doesn't throw an exception.
  Actual: it throws std::runtime_error with description "Bag directory already exists (/tmp/path_not_empty), can't overwrite existing bag
  • This PR addresses the failure in the rosbag2_compression.SequentialCompressionWriterTest.open_succeeds_twice on a second run due to the leftovers in the temp folder.
    It also addresses the flakiness in TestRosbag2CPPAPI::minimal_writer_example due to using shared ptr to the serialized message twice in the writer.write(msg) call.
    Note. The "serialized_msg2" in the TestRosbag2CPPAPI::minimal_writer_example does not own the serialized data after the first call writer.write(serialized_msg2,..). i.e. need to use another message or another API in the test for the second call to the writer.write(msg).
    This is an automatic backport of pull request Fix for regression in open_succeeds_twice and minimal_writer_example tests #1667 done by Mergify.

@mergify mergify bot requested a review from a team as a code owner May 21, 2024 20:24
@mergify mergify bot added the conflicts label May 21, 2024
@mergify mergify bot requested review from emersonknapp and hidmic and removed request for a team May 21, 2024 20:24
Copy link
Author

mergify bot commented May 21, 2024

Cherry-pick of 8523995 has failed:

On branch mergify/bp/iron/pr-1667
Your branch is up to date with 'origin/iron'.

You are currently cherry-picking commit 8523995b.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   rosbag2_tests/test/rosbag2_tests/test_rosbag2_cpp_api.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@MichaelOrlov MichaelOrlov changed the title Fix for regression in open_succeeds_twice and minimal_writer_example tests (backport #1667) [iron] Fix for regression in open_succeeds_twice and minimal_writer_example tests (backport #1667) May 21, 2024
- Use std::filesystem for temp files and folders operation. For some
reason rcpputils::fs::delete_all(folder_name) wasn't able to delete temp
folder with subfolders.

Signed-off-by: Michael Orlov <[email protected]>
- The `serialized_msg2` is not owning the serialized data after the
first call writer.write(serialized_msg2,..). i.e. need to use another
message or another API in test for second call to writer.write(msg).

Signed-off-by: Michael Orlov <[email protected]>
@MichaelOrlov
Copy link
Contributor

Pulls: #1669
Gist: https://gist.githubusercontent.com/MichaelOrlov/bc10c58d4968ea164f392b6877f0d2af/raw/bd3bec476e142e94a00a027da603319f1b6d72e2/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_compression rosbag2_tests
TEST args: --packages-above rosbag2_compression rosbag2_tests
ROS Distro: iron
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/13941

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

@MichaelOrlov MichaelOrlov merged commit 44bfa78 into iron May 22, 2024
14 checks passed
@delete-merged-branch delete-merged-branch bot deleted the mergify/bp/iron/pr-1667 branch May 22, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant