From 51f5a0cb2a0776e2ab631c50864bfabe26ffd91f Mon Sep 17 00:00:00 2001 From: Michael Orlov Date: Tue, 19 Mar 2024 00:08:11 -0700 Subject: [PATCH 1/2] Fix for false negative tests in rosbag2_py - wait_for(condition: Callable, timout) was incorrectly returning True after the first iteration even if condition was false. Signed-off-by: Michael Orlov --- rosbag2_py/test/common.py | 2 +- rosbag2_py/test/test_transport.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rosbag2_py/test/common.py b/rosbag2_py/test/common.py index 2a606c2381..56056b71a0 100644 --- a/rosbag2_py/test/common.py +++ b/rosbag2_py/test/common.py @@ -51,4 +51,4 @@ def wait_for( if clock.now() - start > timeout: return False time.sleep(sleep_time) - return True + return True diff --git a/rosbag2_py/test/test_transport.py b/rosbag2_py/test/test_transport.py index fe01d28b93..57f714d66e 100644 --- a/rosbag2_py/test/test_transport.py +++ b/rosbag2_py/test/test_transport.py @@ -84,6 +84,6 @@ def test_record_cancel(tmp_path, storage_id): metadata = metadata_io.read_metadata(bag_path) assert len(metadata.relative_file_paths) - storage_path = Path(metadata.relative_file_paths[0]) + storage_path = Path(bag_path + '/' + metadata.relative_file_paths[0]) assert wait_for(lambda: storage_path.is_file(), timeout=rclpy.duration.Duration(seconds=3)) From c47b619bb389120023dca069ad4630ea2b3e1ce8 Mon Sep 17 00:00:00 2001 From: Michael Orlov Date: Wed, 20 Mar 2024 14:38:41 -0700 Subject: [PATCH 2/2] Address review comments in regards bag_path optimization Signed-off-by: Michael Orlov --- rosbag2_py/test/test_transport.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/rosbag2_py/test/test_transport.py b/rosbag2_py/test/test_transport.py index 57f714d66e..e33da3bdfe 100644 --- a/rosbag2_py/test/test_transport.py +++ b/rosbag2_py/test/test_transport.py @@ -13,7 +13,6 @@ # limitations under the License. import datetime -from pathlib import Path import threading from common import get_rosbag_options, wait_for @@ -44,8 +43,8 @@ def test_options_qos_conversion(): @pytest.mark.parametrize('storage_id', TESTED_STORAGE_IDS) def test_record_cancel(tmp_path, storage_id): - bag_path = str(tmp_path / 'test_record_cancel') - storage_options, converter_options = get_rosbag_options(bag_path, storage_id) + bag_path = tmp_path / 'test_record_cancel' + storage_options, converter_options = get_rosbag_options(str(bag_path), storage_id) recorder = rosbag2_py.Recorder() @@ -78,12 +77,12 @@ def test_record_cancel(tmp_path, storage_id): recorder.cancel() metadata_io = rosbag2_py.MetadataIo() - assert wait_for(lambda: metadata_io.metadata_file_exists(bag_path), + assert wait_for(lambda: metadata_io.metadata_file_exists(str(bag_path)), timeout=rclpy.duration.Duration(seconds=3)) record_thread.join() - metadata = metadata_io.read_metadata(bag_path) + metadata = metadata_io.read_metadata(str(bag_path)) assert len(metadata.relative_file_paths) - storage_path = Path(bag_path + '/' + metadata.relative_file_paths[0]) + storage_path = bag_path / metadata.relative_file_paths[0] assert wait_for(lambda: storage_path.is_file(), timeout=rclpy.duration.Duration(seconds=3))