From 5d97234fd5694f4d3c35d20e069c0c07a5508e74 Mon Sep 17 00:00:00 2001 From: ymski <114902604+ymski@users.noreply.github.com> Date: Tue, 24 Oct 2023 16:42:55 +0900 Subject: [PATCH 1/7] fix(check_caret_rclcpp): skip check_caret_rclcpp for ROS Distributions after iron Signed-off-by: ymski <114902604+ymski@users.noreply.github.com> --- ros2caret/verb/check_caret_rclcpp.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ros2caret/verb/check_caret_rclcpp.py b/ros2caret/verb/check_caret_rclcpp.py index 7a5025c7..ebc58053 100644 --- a/ros2caret/verb/check_caret_rclcpp.py +++ b/ros2caret/verb/check_caret_rclcpp.py @@ -71,6 +71,7 @@ class RclcppCheck(): def __init__(self, root_dir_path: str) -> None: RclcppCheck._ensure_dir_exist(root_dir_path) + RclcppCheck._validate_ros_distrinution(root_dir_path) get_package_name = RclcppCheck._create_get_package_name(root_dir_path) ros_obj_paths = RclcppCheck._get_obj_paths(root_dir_path) @@ -171,6 +172,16 @@ def _ensure_dir_exist(path: str): 'where the build command completed.') exit(1) + @staticmethod + def _validate_ros_distrinution(path: str): + if os.environ['ROS_DISTRO'][0] < 'i': + return + logger.info('For ROS Distributions after iron, ' + 'applications can be evaluated ' + 'without building with caret-rclcpp. ' + 'For this reason, the verification procedure is skipped.') + exit(0) + @staticmethod def get_package_name_from_path(root_dir_path: str, path: str) -> str: package_name = path.replace(root_dir_path, '').split('/')[0] From 4e748db7b621d66217a5ba9d95ca6982910b650d Mon Sep 17 00:00:00 2001 From: ymski <114902604+ymski@users.noreply.github.com> Date: Thu, 21 Dec 2023 19:56:19 +0900 Subject: [PATCH 2/7] refactor: fix log message Signed-off-by: ymski <114902604+ymski@users.noreply.github.com> --- ros2caret/verb/check_caret_rclcpp.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ros2caret/verb/check_caret_rclcpp.py b/ros2caret/verb/check_caret_rclcpp.py index ebc58053..66e67363 100644 --- a/ros2caret/verb/check_caret_rclcpp.py +++ b/ros2caret/verb/check_caret_rclcpp.py @@ -176,10 +176,9 @@ def _ensure_dir_exist(path: str): def _validate_ros_distrinution(path: str): if os.environ['ROS_DISTRO'][0] < 'i': return - logger.info('For ROS Distributions after iron, ' - 'applications can be evaluated ' - 'without building with caret-rclcpp. ' - 'For this reason, the verification procedure is skipped.') + logger.info('If you are using a ROS distribution after iron, ' \ + 'you no longer need to rebuild with caret-rclcpp. ' \ + 'Thus, there is no need to validate the workspace.') exit(0) @staticmethod From cee62918d6cb12350bd749b90671eeb8e4e855ff Mon Sep 17 00:00:00 2001 From: ymski <114902604+ymski@users.noreply.github.com> Date: Thu, 21 Dec 2023 19:57:42 +0900 Subject: [PATCH 3/7] fix typo Signed-off-by: ymski <114902604+ymski@users.noreply.github.com> --- ros2caret/verb/check_caret_rclcpp.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ros2caret/verb/check_caret_rclcpp.py b/ros2caret/verb/check_caret_rclcpp.py index 66e67363..3cff13d3 100644 --- a/ros2caret/verb/check_caret_rclcpp.py +++ b/ros2caret/verb/check_caret_rclcpp.py @@ -71,7 +71,7 @@ class RclcppCheck(): def __init__(self, root_dir_path: str) -> None: RclcppCheck._ensure_dir_exist(root_dir_path) - RclcppCheck._validate_ros_distrinution(root_dir_path) + RclcppCheck._validate_ros_distribution(root_dir_path) get_package_name = RclcppCheck._create_get_package_name(root_dir_path) ros_obj_paths = RclcppCheck._get_obj_paths(root_dir_path) @@ -173,7 +173,7 @@ def _ensure_dir_exist(path: str): exit(1) @staticmethod - def _validate_ros_distrinution(path: str): + def _validate_ros_distribution(path: str): if os.environ['ROS_DISTRO'][0] < 'i': return logger.info('If you are using a ROS distribution after iron, ' \ From 950a4c3114d6ccc4997d674b526b1bede2d42b80 Mon Sep 17 00:00:00 2001 From: ymski <114902604+ymski@users.noreply.github.com> Date: Thu, 21 Dec 2023 20:12:46 +0900 Subject: [PATCH 4/7] fix style Signed-off-by: ymski <114902604+ymski@users.noreply.github.com> --- ros2caret/verb/check_caret_rclcpp.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ros2caret/verb/check_caret_rclcpp.py b/ros2caret/verb/check_caret_rclcpp.py index 3cff13d3..fa76af15 100644 --- a/ros2caret/verb/check_caret_rclcpp.py +++ b/ros2caret/verb/check_caret_rclcpp.py @@ -176,8 +176,8 @@ def _ensure_dir_exist(path: str): def _validate_ros_distribution(path: str): if os.environ['ROS_DISTRO'][0] < 'i': return - logger.info('If you are using a ROS distribution after iron, ' \ - 'you no longer need to rebuild with caret-rclcpp. ' \ + logger.info('If you are using a ROS distribution after iron, ' + 'you no longer need to rebuild with caret-rclcpp. ' 'Thus, there is no need to validate the workspace.') exit(0) From 552702edf96935708dc98f00c30dab5a0c429968 Mon Sep 17 00:00:00 2001 From: ymski <114902604+ymski@users.noreply.github.com> Date: Thu, 21 Dec 2023 21:11:24 +0900 Subject: [PATCH 5/7] add test Signed-off-by: ymski <114902604+ymski@users.noreply.github.com> --- test/verb/test_check_caret_rclcpp.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/verb/test_check_caret_rclcpp.py b/test/verb/test_check_caret_rclcpp.py index 84bdeabc..01dbb6aa 100644 --- a/test/verb/test_check_caret_rclcpp.py +++ b/test/verb/test_check_caret_rclcpp.py @@ -21,6 +21,8 @@ class TestCheckCaretRclcpp: def test_file_exist(self, caplog, mocker): mocker.patch('os.path.exists', return_value=True) + mocker.patch('ros2caret.verb.check_caret_rclcpp.RclcppCheck._validate_ros_distribution', + return_value=None) mocker.patch( 'ros2caret.verb.check_caret_rclcpp.RclcppCheck._get_obj_paths', return_value=[]) RclcppCheck('') @@ -30,6 +32,8 @@ def test_file_exist(self, caplog, mocker): def test_ng_case(self, caplog, mocker): mocker.patch('os.path.exists', return_value=True) + mocker.patch('ros2caret.verb.check_caret_rclcpp.RclcppCheck._validate_ros_distribution', + return_value=None) base_path = 'ros2caret.verb.check_caret_rclcpp.RclcppCheck.' mocker.patch(base_path+'_get_obj_paths', return_value=['']) mocker.patch(base_path+'_has_caret_rclcpp_tp', return_value=False) @@ -40,6 +44,8 @@ def test_ng_case(self, caplog, mocker): assert record.levelno == WARNING def test_ok_case(self, caplog, mocker): + mocker.patch('ros2caret.verb.check_caret_rclcpp.RclcppCheck._validate_ros_distribution', + return_value=None) mocker.patch('os.path.exists', return_value=True) base_path = 'ros2caret.verb.check_caret_rclcpp.RclcppCheck.' mocker.patch(base_path+'_get_obj_paths', return_value=['']) @@ -69,3 +75,13 @@ def test_get_package_name(self): get_package_name = RclcppCheck._create_get_package_name('foo/bar/') assert get_package_name('foo/bar/baz') == 'baz' + + def test_validate_ros_distribution(self, mocker, caplog): + mocker.patch('os.path.exists', return_value=True) + mocker.patch.dict('os.environ', {'ROS_DISTRO': 'iron'}) + + try: + RclcppCheck('') + except SystemExit: + assert len(caplog.records) == 1 + assert 'there is no need to validate the workspace.' in caplog.messages[0] From 45eeff78fab77f7b256b803290345982f0126682 Mon Sep 17 00:00:00 2001 From: ymski <114902604+ymski@users.noreply.github.com> Date: Mon, 25 Dec 2023 09:59:32 +0900 Subject: [PATCH 6/7] modify message Signed-off-by: ymski <114902604+ymski@users.noreply.github.com> --- ros2caret/verb/check_caret_rclcpp.py | 8 ++++---- test/verb/test_check_caret_rclcpp.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ros2caret/verb/check_caret_rclcpp.py b/ros2caret/verb/check_caret_rclcpp.py index fa76af15..0dcaf9fc 100644 --- a/ros2caret/verb/check_caret_rclcpp.py +++ b/ros2caret/verb/check_caret_rclcpp.py @@ -174,11 +174,11 @@ def _ensure_dir_exist(path: str): @staticmethod def _validate_ros_distribution(path: str): - if os.environ['ROS_DISTRO'][0] < 'i': + distribution = os.environ['ROS_DISTRO'] + if distribution[0] < 'i': return - logger.info('If you are using a ROS distribution after iron, ' - 'you no longer need to rebuild with caret-rclcpp. ' - 'Thus, there is no need to validate the workspace.') + logger.info('There is no need to build packages ' + f'using caret-rclcpp under ROS 2 {distribution}.') exit(0) @staticmethod diff --git a/test/verb/test_check_caret_rclcpp.py b/test/verb/test_check_caret_rclcpp.py index 01dbb6aa..351c1c96 100644 --- a/test/verb/test_check_caret_rclcpp.py +++ b/test/verb/test_check_caret_rclcpp.py @@ -84,4 +84,4 @@ def test_validate_ros_distribution(self, mocker, caplog): RclcppCheck('') except SystemExit: assert len(caplog.records) == 1 - assert 'there is no need to validate the workspace.' in caplog.messages[0] + assert f'There is no need to build packages using caret-rclcpp under ROS 2 iron.' in caplog.messages[0] From 0a3d08b3d2d3ea1039d06bcd1e27e9a55a470eba Mon Sep 17 00:00:00 2001 From: ymski <114902604+ymski@users.noreply.github.com> Date: Mon, 25 Dec 2023 10:07:36 +0900 Subject: [PATCH 7/7] style Signed-off-by: ymski <114902604+ymski@users.noreply.github.com> --- test/verb/test_check_caret_rclcpp.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/verb/test_check_caret_rclcpp.py b/test/verb/test_check_caret_rclcpp.py index 351c1c96..962a68f5 100644 --- a/test/verb/test_check_caret_rclcpp.py +++ b/test/verb/test_check_caret_rclcpp.py @@ -84,4 +84,5 @@ def test_validate_ros_distribution(self, mocker, caplog): RclcppCheck('') except SystemExit: assert len(caplog.records) == 1 - assert f'There is no need to build packages using caret-rclcpp under ROS 2 iron.' in caplog.messages[0] + assert 'There is no need to build packages ' \ + 'using caret-rclcpp under ROS 2 iron.' in caplog.messages[0]