fix: bug in parsing services name

Author: gmmcosta15

tests/updater/__init__.py

@@ -1,28 +1,27 @@
-"""Unit tests for updater.components """
+"""Unit tests for updater.components"""
+
 import logging
-import pytest
+from os.path import commonpath
 from pathlib import Path
 from unittest.mock import patch, mock_open
 
-from updater.components import (
-    ComponentConfig, ComponentStatus, load_components
-)
+from updater.components import load_components
 
 BUNDLE_YAML = """
-pool_internal_minutes: 30
+poll_interval_minutes: 1440
 components:
   - name: klipper
-  - type: git
-  - path: ~/klipper
-  - service: klipper.service
-  - reset_mode: hard
-  - order: 1
-  - name: BlocksScreen
-  - type: git
-  - path: ~/BlocksScreen
-  - service: BlocksScreen.service
-  - reset_mode: hard
-  - order: 99
+    type: git
+    path: ~/klipper
+    service: klipper.service
+    reset_mode: hard
+    order: 1
+  - name: blockscreen
+    type: git
+    path: ~/BlocksScreen
+    service: BlocksScreen.service
+    reset_mode: hard
+    order: 99
 """
 
 OVERRIDE_YAML = """
@@ -37,53 +36,83 @@
       order: 5
 """
 
+
 def _mock_load(bundled: str, override: str | None = None):
     import updater.components as mod
+
     bundled_path = Path(mod.__file__).parent / "components.yaml"
 
     def fake_open(path, *a, **kw):
         if Path(path) == bundled_path:
             return mock_open(read_data=bundled)()
         if override is not None:
             return mock_open(read_data=override)()
-        raise FileNotFoundError(Path)
+        raise FileNotFoundError(path)
+
     return fake_open
 
 
 class TestLoadComponents:
     def test_load_return_sorted_by_order(self):
-        with patch("builtins.open", _mock_load(BUNDLE_YAML)), patch("pathlib.Path.exists", return_value=False), patch("pathlib.Path.is_dir", return_value=True):
-                components = load_components()
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML)),
+            patch("pathlib.Path.exists", return_value=False),
+            patch("pathlib.Path.is_dir", return_value=True),
+        ):
+            components = load_components()
         names = [c.name for c in components]
-        assert names.index("klipper") < names.index("BlocksScreen")
-    
+        assert names.index("klipper") < names.index("blockscreen")
+
     def test_missing_override_file_silently_ignored(self):
-        with patch("builtins.open", _mock_load(BUNDLE_YAML)), patch("pathlib.Path.exists", return_value=False), patch("pathlib.Path.is_dir", return_value=True):
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML)),
+            patch("pathlib.Path.exists", return_value=False),
+            patch("pathlib.Path.is_dir", return_value=True),
+        ):
             components = load_components()
         assert len(components) >= 2
 
+
 class TestYamlMerge:
-    def test_ovvrides_path_by_name(self):
-        with patch("builtins.open", _mock_load(BUNDLE_YAML)), patch("pathlib.Path.exists", return_value=True), patch("pathlib.Path.is_dir", return_value=True):
+    def test_overrides_path_by_name(self):
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)),
+            patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.is_dir", return_value=True),
+        ):
             components = load_components()
         klipper = next(c for c in components if c.name == "klipper")
         assert str(klipper.path).endswith("costum_klipper")
         assert klipper.service == "klipper.service"
 
-    def test_appends_new_componentself(self):
-        with patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)), patch("pathlib.Path.exists", return_value=True), patch("pathlib.Path.is_dir", return_value=True):
+    def test_appends_new_component(self):
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)),
+            patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.is_dir", return_value=True),
+        ):
             components = load_components()
         assert any(c.name == "my-plugin" for c in components)
+
     def test_keeps_bundled_fields_not_in_override(self):
-        with patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)), patch("pathlib.Path.exists", return_value=True), patch("pathlib.Path.is_dir", return_value=True):
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)),
+            patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.is_dir", return_value=True),
+        ):
             components = load_components()
         klipper = next(c for c in components if c.name == "klipper")
-        assert klipper.type == "git"
+        assert klipper.kind == "git"
         assert klipper.reset_mode == "hard"
 
     def test_syntax_error_falls_back_to_bundled(self, caplog):
-        with patch("builtins.open", _mock_load(BUNDLE_YAML, OVERRIDE_YAML)), patch("pathlib.Path.exists", return_value=True), patch("pathlib.Path.is_dir", return_value=True), \
-            caplog.at_level(logging.ERROR, logger="updater.components"):
+        invalid_yaml = "key: [unclosed"
+        with (
+            patch("builtins.open", _mock_load(BUNDLE_YAML, invalid_yaml)),
+            patch("pathlib.Path.exists", return_value=True),
+            patch("pathlib.Path.is_dir", return_value=True),
+            caplog.at_level(logging.ERROR, logger="updater.components"),
+        ):
             components = load_components()
         names = [c.name for c in components]
         assert "klipper" in names
@@ -100,9 +129,11 @@ def test_invalid_component_skipped_with_warning(self, caplog):
       service: bad.service
       order: 10
 """
-        with patch("builtins.open", _mock_load(bad_yaml)), \
-            patch("pathlib.Path.exists", return_value=True), \
-            caplog.at_level(logging.WARNING, logger="updater.components"):
+        with (
+            patch("builtins.open", _mock_load(bad_yaml)),
+            patch("pathlib.Path.exists", return_value=True),
+            caplog.at_level(logging.WARNING, logger="updater.components"),
+        ):
             components = load_components()
         assert not any(c.name == "bad" for c in components)
         assert any("bad" in r.message for r in caplog.records)
@@ -116,8 +147,10 @@ def test_path_outside_home_rejected(self):
       service: outside.service
       order: 10
 """
-        with patch("builtins.open", _mock_load(outside_yaml)), \
-            patch("pathlib.Path.exists", return_value=False):
+        with (
+            patch("builtins.open", _mock_load(outside_yaml)),
+            patch("pathlib.Path.exists", return_value=False),
+        ):
             components = load_components()
         assert not any(c.name == "outside" for c in components)
 
@@ -130,25 +163,31 @@ def test_service_name_with_slash_rejected(self):
       service: ../bad.service
       order: 10
 """
-        with patch("builtins.open", _mock_load(bad_yaml)), \
-            patch("pathlib.Path.exists", return_value=False):
+        with (
+            patch("builtins.open", _mock_load(bad_yaml)),
+            patch("pathlib.Path.exists", return_value=False),
+        ):
             components = load_components()
-        assert not any(c.name == "evil" for c in components)
+        assert not any(c.name == "bad" for c in components)
 
     def test_service_name_with_shell_metachar_rejected(self):
         for metachar in [";", "&", "|", "$", "`"]:
             bad_yaml = f"""
 components:
-- name: bad
-type: git
-path: ~/bad
-service: bad{metachar}cmd.service
-order: 10
+  - name: bad
+    type: git
+    path: ~/bad
+    service: bad{metachar}cmd.service
+    order: 10
 """
-            with patch("builtins.open", _mock_load(bad_yaml)), \
-                patch("pathlib.Path.exists", return_value=False):
+            with (
+                patch("builtins.open", _mock_load(bad_yaml)),
+                patch("pathlib.Path.exists", return_value=False),
+            ):
                 components = load_components()
-            assert not any(c.name == "bad" for c in components), f"metachar {metachar!r} not rejected"
+            assert not any(c.name == "bad" for c in components), (
+                f"metachar {metachar!r} not rejected"
+            )
 
     def test_service_name_valid_pattern_accepted(self):
         good_yaml = """
@@ -159,9 +198,10 @@ def test_service_name_valid_pattern_accepted(self):
       service: my-plugin.service
       order: 5
 """
-        with patch("builtins.open", _mock_load(good_yaml)), \
-            patch("pathlib.Path.exists", return_value=False), \
-            patch("pathlib.Path.is_dir", return_value=True):
+        with (
+            patch("builtins.open", _mock_load(good_yaml)),
+            patch("pathlib.Path.exists", return_value=False),
+            patch("pathlib.Path.is_dir", return_value=True),
+        ):
             components = load_components()
         assert any(c.name == "my-plugin" for c in components)
- 

tests/updater/test_components_unit.py

@@ -13,7 +13,7 @@
 logger = logging.getLogger(__name__)
 
 _SERVICE_RE = re.compile(r"^[a-zA-Z0-9@:._-]+\.service$")
-_SERVICE_BANNED = set("/\\..;&|$`") | {" ", "\t"}
+_SERVICE_BANNED = set("/\\;&|$`") | {" ", "\t"}
 
 OVERRIDE_PATH = Path("~/printer_data/config/blockscreen_updater.yaml").expanduser()