From 1568bb7871a237fe315907b61c80fbd5691df4c2 Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Fri, 18 Sep 2020 15:40:07 +0000 Subject: [PATCH 1/5] Uses bag_command.wait_for_output with expected string instead of time.sleep in tests Signed-off-by: Jaison Titus --- ros2bag/test/test_record_qos_profiles.py | 35 ++++++++++++------------ 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/ros2bag/test/test_record_qos_profiles.py b/ros2bag/test/test_record_qos_profiles.py index 6eb632a013..952b2dee42 100644 --- a/ros2bag/test/test_record_qos_profiles.py +++ b/ros2bag/test/test_record_qos_profiles.py @@ -81,52 +81,53 @@ def test_qos_simple(self): output_path = Path(self.tmpdir.name) / 'ros2bag_test_basic' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] + + expected_string_regex = re.compile(r"\[rosbag2_storage]: Opened database .* for READ_WRITE") + output_condition = lambda output: expected_string_regex.search(output) is not None with self.launch_bag_command(arguments=arguments) as bag_command: - time.sleep(3) + condition_result = bag_command.wait_for_output(condition=output_condition, timeout=3) + assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated - expected_string_regex = re.compile(ERROR_STRING) - matches = expected_string_regex.search(bag_command.output) - assert not matches, print('ros2bag CLI did not produce the expected output') def test_incomplete_qos_profile(self): profile_path = PROFILE_PATH / 'incomplete_qos_profile.yaml' output_path = Path(self.tmpdir.name) / 'ros2bag_test_incomplete' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] + expected_string_regex = re.compile(r"\[rosbag2_storage]: Opened database .* for READ_WRITE") + output_condition = lambda output: expected_string_regex.search(output) is not None with self.launch_bag_command(arguments=arguments) as bag_command: - time.sleep(3) + condition_result = bag_command.wait_for_output(condition=output_condition, timeout=3) + assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated - expected_string_regex = re.compile(ERROR_STRING) - matches = expected_string_regex.search(bag_command.output) - assert not matches, print('ros2bag CLI did not produce the expected output') def test_incomplete_qos_duration(self): profile_path = PROFILE_PATH / 'incomplete_qos_duration.yaml' output_path = Path(self.tmpdir.name) / 'ros2bag_test_incomplete_duration' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] + expected_string_regex = re.compile(r"\[ERROR] \[ros2bag]: Time overrides must include both") + output_condition = lambda output: expected_string_regex.search(output) is not None with self.launch_bag_command(arguments=arguments) as bag_command: - time.sleep(3) + condition_result = bag_command.wait_for_output(condition=output_condition, timeout=3) + assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated assert bag_command.exit_code != launch_testing.asserts.EXIT_OK - expected_string_regex = re.compile(ERROR_STRING) - matches = expected_string_regex.search(bag_command.output) - assert matches, print('ros2bag CLI did not produce the expected output') def test_nonexistent_qos_profile(self): profile_path = PROFILE_PATH / 'foobar.yaml' output_path = Path(self.tmpdir.name) / 'ros2bag_test_nonexistent' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] + expected_string_regex = re.compile( + r"ros2 bag record: error: argument --qos-profile-overrides-path: can't open") + output_condition = lambda output: expected_string_regex.search(output) is not None with self.launch_bag_command(arguments=arguments) as bag_command: - time.sleep(3) + condition_result = bag_command.wait_for_output(condition=output_condition, timeout=3) + assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated assert bag_command.exit_code != launch_testing.asserts.EXIT_OK - expected_string_regex = re.compile( - r"ros2 bag record: error: argument --qos-profile-overrides-path: can't open") - matches = expected_string_regex.search(bag_command.output) - assert matches, print('ros2bag CLI did not produce the expected output') From e2ce0407930b698ea99521b7740c65b107bf4320 Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Fri, 18 Sep 2020 10:29:05 -0700 Subject: [PATCH 2/5] Fixes code style errors Signed-off-by: Jaison Titus --- ros2bag/test/test_record_qos_profiles.py | 32 ++++++++++++++---------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/ros2bag/test/test_record_qos_profiles.py b/ros2bag/test/test_record_qos_profiles.py index 952b2dee42..fcd444a59a 100644 --- a/ros2bag/test/test_record_qos_profiles.py +++ b/ros2bag/test/test_record_qos_profiles.py @@ -81,11 +81,12 @@ def test_qos_simple(self): output_path = Path(self.tmpdir.name) / 'ros2bag_test_basic' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] - - expected_string_regex = re.compile(r"\[rosbag2_storage]: Opened database .* for READ_WRITE") - output_condition = lambda output: expected_string_regex.search(output) is not None + expected_string_regex = re.compile( + r'\[rosbag2_storage]: Opened database .* for READ_WRITE') with self.launch_bag_command(arguments=arguments) as bag_command: - condition_result = bag_command.wait_for_output(condition=output_condition, timeout=3) + condition_result = bag_command.wait_for_output( + condition=lambda output: expected_string_regex.search(output) is not None, + timeout=5) assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated @@ -95,10 +96,12 @@ def test_incomplete_qos_profile(self): output_path = Path(self.tmpdir.name) / 'ros2bag_test_incomplete' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] - expected_string_regex = re.compile(r"\[rosbag2_storage]: Opened database .* for READ_WRITE") - output_condition = lambda output: expected_string_regex.search(output) is not None + expected_string_regex = re.compile( + r'\[rosbag2_storage]: Opened database .* for READ_WRITE') with self.launch_bag_command(arguments=arguments) as bag_command: - condition_result = bag_command.wait_for_output(condition=output_condition, timeout=3) + condition_result = bag_command.wait_for_output( + condition=lambda output: expected_string_regex.search(output) is not None, + timeout=5) assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated @@ -108,10 +111,12 @@ def test_incomplete_qos_duration(self): output_path = Path(self.tmpdir.name) / 'ros2bag_test_incomplete_duration' arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] - expected_string_regex = re.compile(r"\[ERROR] \[ros2bag]: Time overrides must include both") - output_condition = lambda output: expected_string_regex.search(output) is not None + expected_string_regex = re.compile( + r'\[ERROR] \[ros2bag]: Time overrides must include both') with self.launch_bag_command(arguments=arguments) as bag_command: - condition_result = bag_command.wait_for_output(condition=output_condition, timeout=3) + condition_result = bag_command.wait_for_output( + condition=lambda output: expected_string_regex.search(output) is not None, + timeout=5) assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated @@ -123,10 +128,11 @@ def test_nonexistent_qos_profile(self): arguments = ['record', '-a', '--qos-profile-overrides-path', profile_path.as_posix(), '--output', output_path.as_posix()] expected_string_regex = re.compile( - r"ros2 bag record: error: argument --qos-profile-overrides-path: can't open") - output_condition = lambda output: expected_string_regex.search(output) is not None + r'ros2 bag record: error: argument --qos-profile-overrides-path: can\'t open') with self.launch_bag_command(arguments=arguments) as bag_command: - condition_result = bag_command.wait_for_output(condition=output_condition, timeout=3) + condition_result = bag_command.wait_for_output( + condition=lambda output: expected_string_regex.search(output) is not None, + timeout=5) assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated From b15c9fc3382e5fd84268aa49781e481f1341b98b Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Fri, 18 Sep 2020 17:46:50 +0000 Subject: [PATCH 3/5] Moves to asserting expected output match outside of the process context to account for cases where wait_for_output is maybe called after the expected output is already printed. Signed-off-by: Jaison Titus --- ros2bag/test/test_record_qos_profiles.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/ros2bag/test/test_record_qos_profiles.py b/ros2bag/test/test_record_qos_profiles.py index fcd444a59a..e892892743 100644 --- a/ros2bag/test/test_record_qos_profiles.py +++ b/ros2bag/test/test_record_qos_profiles.py @@ -36,7 +36,6 @@ PROFILE_PATH = Path(__file__).parent / 'resources' TEST_NODE = 'ros2bag_record_qos_profile_test_node' TEST_NAMESPACE = 'ros2bag_record_qos_profile' -ERROR_STRING = r'\[ERROR] \[ros2bag]:' @pytest.mark.rostest @@ -84,12 +83,13 @@ def test_qos_simple(self): expected_string_regex = re.compile( r'\[rosbag2_storage]: Opened database .* for READ_WRITE') with self.launch_bag_command(arguments=arguments) as bag_command: - condition_result = bag_command.wait_for_output( + bag_command.wait_for_output( condition=lambda output: expected_string_regex.search(output) is not None, timeout=5) - assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated + matches = expected_string_regex.search(bag_command.output) + assert matches, print('ros2bag CLI did not produce the expected output') def test_incomplete_qos_profile(self): profile_path = PROFILE_PATH / 'incomplete_qos_profile.yaml' @@ -99,12 +99,13 @@ def test_incomplete_qos_profile(self): expected_string_regex = re.compile( r'\[rosbag2_storage]: Opened database .* for READ_WRITE') with self.launch_bag_command(arguments=arguments) as bag_command: - condition_result = bag_command.wait_for_output( + bag_command.wait_for_output( condition=lambda output: expected_string_regex.search(output) is not None, timeout=5) - assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated + matches = expected_string_regex.search(bag_command.output) + assert matches, print('ros2bag CLI did not produce the expected output') def test_incomplete_qos_duration(self): profile_path = PROFILE_PATH / 'incomplete_qos_duration.yaml' @@ -114,13 +115,14 @@ def test_incomplete_qos_duration(self): expected_string_regex = re.compile( r'\[ERROR] \[ros2bag]: Time overrides must include both') with self.launch_bag_command(arguments=arguments) as bag_command: - condition_result = bag_command.wait_for_output( + bag_command.wait_for_output( condition=lambda output: expected_string_regex.search(output) is not None, timeout=5) - assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated assert bag_command.exit_code != launch_testing.asserts.EXIT_OK + matches = expected_string_regex.search(bag_command.output) + assert matches, print('ros2bag CLI did not produce the expected output') def test_nonexistent_qos_profile(self): profile_path = PROFILE_PATH / 'foobar.yaml' @@ -130,10 +132,11 @@ def test_nonexistent_qos_profile(self): expected_string_regex = re.compile( r'ros2 bag record: error: argument --qos-profile-overrides-path: can\'t open') with self.launch_bag_command(arguments=arguments) as bag_command: - condition_result = bag_command.wait_for_output( + bag_command.wait_for_output( condition=lambda output: expected_string_regex.search(output) is not None, timeout=5) - assert condition_result, print('ros2bag CLI did not produce the expected output') bag_command.wait_for_shutdown(timeout=5) assert bag_command.terminated assert bag_command.exit_code != launch_testing.asserts.EXIT_OK + matches = expected_string_regex.search(bag_command.output) + assert matches, print('ros2bag CLI did not produce the expected output') From a7881f547e59d1cc965d27520d0dcda9ee78f158 Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Mon, 21 Sep 2020 16:36:00 +0000 Subject: [PATCH 4/5] Defines timeout with variables and better error messages for failed tests. Signed-off-by: Jaison Titus --- ros2bag/test/test_record_qos_profiles.py | 33 +++++++++++++++--------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/ros2bag/test/test_record_qos_profiles.py b/ros2bag/test/test_record_qos_profiles.py index e892892743..f89124c669 100644 --- a/ros2bag/test/test_record_qos_profiles.py +++ b/ros2bag/test/test_record_qos_profiles.py @@ -36,7 +36,11 @@ PROFILE_PATH = Path(__file__).parent / 'resources' TEST_NODE = 'ros2bag_record_qos_profile_test_node' TEST_NAMESPACE = 'ros2bag_record_qos_profile' +ERROR_STRING_MSG = 'ros2bag CLI did not produce the expected output'\ + '\n Expected output pattern: {}\n Actual output: {}' +OUTPUT_WAIT_TIMEOUT = 10 +SHUTDOWN_TIMEOUT = 5 @pytest.mark.rostest @launch_testing.markers.keep_alive @@ -85,11 +89,12 @@ def test_qos_simple(self): with self.launch_bag_command(arguments=arguments) as bag_command: bag_command.wait_for_output( condition=lambda output: expected_string_regex.search(output) is not None, - timeout=5) - bag_command.wait_for_shutdown(timeout=5) + timeout=OUTPUT_WAIT_TIMEOUT) + bag_command.wait_for_shutdown(timeout=SHUTDOWN_TIMEOUT) assert bag_command.terminated matches = expected_string_regex.search(bag_command.output) - assert matches, print('ros2bag CLI did not produce the expected output') + assert matches, print( + ERROR_STRING_MSG.format(expected_string_regex.pattern, bag_command.output)) def test_incomplete_qos_profile(self): profile_path = PROFILE_PATH / 'incomplete_qos_profile.yaml' @@ -101,11 +106,12 @@ def test_incomplete_qos_profile(self): with self.launch_bag_command(arguments=arguments) as bag_command: bag_command.wait_for_output( condition=lambda output: expected_string_regex.search(output) is not None, - timeout=5) - bag_command.wait_for_shutdown(timeout=5) + timeout=OUTPUT_WAIT_TIMEOUT) + bag_command.wait_for_shutdown(timeout=SHUTDOWN_TIMEOUT) assert bag_command.terminated matches = expected_string_regex.search(bag_command.output) - assert matches, print('ros2bag CLI did not produce the expected output') + assert matches, print( + ERROR_STRING_MSG.format(expected_string_regex.pattern, bag_command.output)) def test_incomplete_qos_duration(self): profile_path = PROFILE_PATH / 'incomplete_qos_duration.yaml' @@ -117,12 +123,13 @@ def test_incomplete_qos_duration(self): with self.launch_bag_command(arguments=arguments) as bag_command: bag_command.wait_for_output( condition=lambda output: expected_string_regex.search(output) is not None, - timeout=5) - bag_command.wait_for_shutdown(timeout=5) + timeout=OUTPUT_WAIT_TIMEOUT) + bag_command.wait_for_shutdown(timeout=SHUTDOWN_TIMEOUT) assert bag_command.terminated assert bag_command.exit_code != launch_testing.asserts.EXIT_OK matches = expected_string_regex.search(bag_command.output) - assert matches, print('ros2bag CLI did not produce the expected output') + assert matches, print( + ERROR_STRING_MSG.format(expected_string_regex.pattern, bag_command.output)) def test_nonexistent_qos_profile(self): profile_path = PROFILE_PATH / 'foobar.yaml' @@ -134,9 +141,11 @@ def test_nonexistent_qos_profile(self): with self.launch_bag_command(arguments=arguments) as bag_command: bag_command.wait_for_output( condition=lambda output: expected_string_regex.search(output) is not None, - timeout=5) - bag_command.wait_for_shutdown(timeout=5) + timeout=OUTPUT_WAIT_TIMEOUT) + bag_command.wait_for_shutdown(timeout=SHUTDOWN_TIMEOUT) assert bag_command.terminated assert bag_command.exit_code != launch_testing.asserts.EXIT_OK matches = expected_string_regex.search(bag_command.output) - assert matches, print('ros2bag CLI did not produce the expected output') + assert matches, print( + ERROR_STRING_MSG.format(expected_string_regex.pattern, bag_command.output)) + From 6e9d96659bb571d12a0d5b9ebdb293a884bed5e0 Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Mon, 21 Sep 2020 09:39:57 -0700 Subject: [PATCH 5/5] Fixes flake8 errors Signed-off-by: Jaison Titus --- ros2bag/test/test_record_qos_profiles.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ros2bag/test/test_record_qos_profiles.py b/ros2bag/test/test_record_qos_profiles.py index f89124c669..91beba614f 100644 --- a/ros2bag/test/test_record_qos_profiles.py +++ b/ros2bag/test/test_record_qos_profiles.py @@ -38,10 +38,10 @@ TEST_NAMESPACE = 'ros2bag_record_qos_profile' ERROR_STRING_MSG = 'ros2bag CLI did not produce the expected output'\ '\n Expected output pattern: {}\n Actual output: {}' - OUTPUT_WAIT_TIMEOUT = 10 SHUTDOWN_TIMEOUT = 5 + @pytest.mark.rostest @launch_testing.markers.keep_alive def generate_test_description(): @@ -148,4 +148,3 @@ def test_nonexistent_qos_profile(self): matches = expected_string_regex.search(bag_command.output) assert matches, print( ERROR_STRING_MSG.format(expected_string_regex.pattern, bag_command.output)) -