FIX: add ValueError when empty string is inputted in get_absolute_node_name

Signed-off-by: Decwest <fumiyaonishi1016@gmail.com>
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>

Author: decwest

ros2log/ros2log/api/__init__.py

@@ -32,14 +32,21 @@
 LOGGER_SET_SERVICE_TYPE = 'rcl_interfaces/srv/SetLoggerLevels'
 
 
+def _require_absolute_node_name(node_name: str) -> str:
+    absolute_node_name = get_absolute_node_name(node_name)
+    if absolute_node_name is None:
+        raise ValueError('node_name must not be empty')
+    return absolute_node_name
+
+
 def get_get_logger_levels_service_name(node_name: str) -> str:
     """Return the get-logger-levels service name for a node."""
-    return f'{get_absolute_node_name(node_name)}{LOGGER_GET_SERVICE_SUFFIX}'
+    return f'{_require_absolute_node_name(node_name)}{LOGGER_GET_SERVICE_SUFFIX}'
 
 
 def get_set_logger_levels_service_name(node_name: str) -> str:
     """Return the set-logger-levels service name for a node."""
-    return f'{get_absolute_node_name(node_name)}{LOGGER_SET_SERVICE_SUFFIX}'
+    return f'{_require_absolute_node_name(node_name)}{LOGGER_SET_SERVICE_SUFFIX}'
 
 
 def get_logger_name_for_node(node_name: str) -> str:
@@ -49,15 +56,13 @@ def get_logger_name_for_node(node_name: str) -> str:
     ROS node logger names use dot-separated namespaces, e.g. `/demo/talker`
     becomes `demo.talker`.
     """
-    absolute_node_name = get_absolute_node_name(node_name)
-    if absolute_node_name is None:
-        raise ValueError('node_name must not be empty')
+    absolute_node_name = _require_absolute_node_name(node_name)
     return absolute_node_name.lstrip('/').replace('/', '.')
 
 
 def format_logger_service_unavailable_error(node_name: str) -> str:
     """Return the user-facing error for nodes without logger services."""
-    absolute_node_name = get_absolute_node_name(node_name)
+    absolute_node_name = _require_absolute_node_name(node_name)
     return (
         f"Logger service not available for node '{absolute_node_name}'.\n"
         "The 'enable_logger_service' node option must be enabled for this node."

ros2log/test/test_api.py

@@ -16,14 +16,36 @@
 from unittest.mock import patch
 
 from ros2log.api import format_logger_service_unavailable_error
+from ros2log.api import get_get_logger_levels_service_name
 from ros2log.api import get_logger_name_for_node
+from ros2log.api import get_set_logger_levels_service_name
 from ros2log.api import get_target_node_names
 from ros2node.api import NodeName
 
 
+class TestLoggerServiceNameHelpers(unittest.TestCase):
+    """Test logger service name helpers."""
+
+    def test_empty_node_name_raises_value_error_for_get_service_name(self):
+        with self.assertRaisesRegex(ValueError, 'node_name must not be empty'):
+            get_get_logger_levels_service_name('')
+
+    def test_empty_node_name_raises_value_error_for_set_service_name(self):
+        with self.assertRaisesRegex(ValueError, 'node_name must not be empty'):
+            get_set_logger_levels_service_name('')
+
+    def test_empty_node_name_raises_value_error_for_unavailable_error(self):
+        with self.assertRaisesRegex(ValueError, 'node_name must not be empty'):
+            format_logger_service_unavailable_error('')
+
+
 class TestGetLoggerNameForNode(unittest.TestCase):
     """Test node-name to logger-name conversion helpers."""
 
+    def test_empty_node_name_raises_value_error(self):
+        with self.assertRaisesRegex(ValueError, 'node_name must not be empty'):
+            get_logger_name_for_node('')
+
     def test_root_namespace(self):
         self.assertEqual(get_logger_name_for_node('/talker'), 'talker')