test: Add comprehensive unit tests for core modules

Add new test files for core classes:

  - tests/core/test_config_loader.py: 11 test cases for ConfigMerger

    - Deep merge functionality (basic and nested)

    - Config file loading (valid, invalid, missing)

    - Device config loading with merge logic

  - tests/core/test_shell.py: 17 test cases for ShellRunner

    - Platform detection and binary path resolution

    - Command execution (list and string formats)

    - Error handling and callback support

    - Environment variable and working directory support

  - tests/core/test_base_modifier.py: 10 test cases for BaseModifier

    - File and directory recursive finding

    - Multiple match handling

    - Non-existent path handling

Author: toraidl

tests/core/test_base_modifier.py

@@ -0,0 +1,99 @@
+"""Unit tests for BaseModifier class."""
+
+from pathlib import Path
+from unittest.mock import Mock
+
+import pytest
+
+from src.core.modifiers.base_modifier import BaseModifier
+
+
+class TestBaseModifier:
+    """Test cases for BaseModifier class."""
+
+    @pytest.fixture
+    def modifier(self):
+        """Create a BaseModifier instance with mock context."""
+        context = Mock()
+        context.logger = Mock()
+        return BaseModifier(context, "TestModifier")
+
+    def test_init(self, modifier):
+        """Test BaseModifier initialization."""
+        assert modifier.name == "TestModifier"
+        assert modifier.ctx is not None
+        assert modifier.logger is not None
+
+    def test_find_file_recursive_found(self, modifier, tmp_path):
+        """Test finding a file that exists."""
+        # Create nested directory structure
+        nested = tmp_path / "a" / "b" / "c"
+        nested.mkdir(parents=True)
+        target_file = nested / "target.txt"
+        target_file.write_text("content")
+
+        result = modifier._find_file_recursive(tmp_path, "target.txt")
+        assert result == target_file
+
+    def test_find_file_recursive_not_found(self, modifier, tmp_path):
+        """Test finding a file that doesn't exist."""
+        result = modifier._find_file_recursive(tmp_path, "nonexistent.txt")
+        assert result is None
+
+    def test_find_file_recursive_in_root(self, modifier, tmp_path):
+        """Test finding a file in root directory."""
+        target_file = tmp_path / "root_file.txt"
+        target_file.write_text("content")
+
+        result = modifier._find_file_recursive(tmp_path, "root_file.txt")
+        assert result == target_file
+
+    def test_find_file_recursive_multiple_matches(self, modifier, tmp_path):
+        """Test finding returns first match when multiple files exist."""
+        (tmp_path / "file1.txt").write_text("content")
+        nested = tmp_path / "subdir"
+        nested.mkdir()
+        (nested / "file1.txt").write_text("content2")
+
+        result = modifier._find_file_recursive(tmp_path, "file1.txt")
+        assert result is not None
+        assert result.name == "file1.txt"
+
+    def test_find_file_recursive_nonexistent_dir(self, modifier):
+        """Test finding in non-existent directory returns None."""
+        result = modifier._find_file_recursive(Path("/nonexistent/path"), "file.txt")
+        assert result is None
+
+    def test_find_dir_recursive_found(self, modifier, tmp_path):
+        """Test finding a directory that exists."""
+        nested = tmp_path / "level1" / "level2" / "target"
+        nested.mkdir(parents=True)
+
+        result = modifier._find_dir_recursive(tmp_path, "target")
+        assert result == nested
+
+    def test_find_dir_recursive_not_found(self, modifier, tmp_path):
+        """Test finding a directory that doesn't exist."""
+        result = modifier._find_dir_recursive(tmp_path, "nonexistent_dir")
+        assert result is None
+
+    def test_find_dir_recursive_exact_name_match(self, modifier, tmp_path):
+        """Test that directory name must match exactly."""
+        # Create dir with similar but different name
+        (tmp_path / "target_dir").mkdir()
+        (tmp_path / "target").mkdir()
+
+        result = modifier._find_dir_recursive(tmp_path, "target")
+        assert result is not None
+        assert result.name == "target"
+
+    def test_find_dir_recursive_prefers_deeper_match(self, modifier, tmp_path):
+        """Test finding directories at different levels."""
+        (tmp_path / "target").mkdir()
+        deep = tmp_path / "a" / "b" / "target"
+        deep.mkdir(parents=True)
+
+        result = modifier._find_dir_recursive(tmp_path, "target")
+        # Should find the first one encountered by rglob
+        assert result is not None
+        assert result.name == "target"

tests/core/test_config_loader.py

@@ -0,0 +1,129 @@
+"""Unit tests for ConfigMerger class."""
+
+import json
+import logging
+from pathlib import Path
+
+import pytest
+
+from src.core.config_loader import ConfigMerger, load_device_config
+
+
+class TestConfigMerger:
+    """Test cases for ConfigMerger class."""
+
+    @pytest.fixture
+    def merger(self, tmp_path):
+        """Create a ConfigMerger instance with temporary directories."""
+        logger = logging.getLogger("test")
+        return ConfigMerger(logger)
+
+    def test_deep_merge_basic(self, merger):
+        """Test basic deep merge functionality."""
+        base = {"a": 1, "b": {"c": 2, "d": 3}}
+        override = {"b": {"c": 10}, "e": 5}
+        result = merger.deep_merge(base, override)
+
+        assert result["a"] == 1
+        assert result["b"]["c"] == 10
+        assert result["b"]["d"] == 3
+        assert result["e"] == 5
+
+    def test_deep_merge_nested(self, merger):
+        """Test deep merge with nested dictionaries."""
+        base = {"level1": {"level2": {"value": "original"}}}
+        override = {"level1": {"level2": {"value": "overridden"}}}
+        result = merger.deep_merge(base, override)
+        assert result["level1"]["level2"]["value"] == "overridden"
+
+    def test_deep_merge_skips_metadata(self, merger):
+        """Test that metadata keys (starting with _) are skipped."""
+        base = {"a": 1}
+        override = {"_comment": "This should be ignored", "a": 2}
+        result = merger.deep_merge(base, override)
+
+        assert "_comment" not in result
+        assert result["a"] == 2
+
+    def test_load_config_file_not_found(self, merger, tmp_path):
+        """Test loading a non-existent config file."""
+        config_path = tmp_path / "nonexistent.json"
+        result = merger.load_config(config_path)
+        assert result == {}
+
+    def test_load_config_valid_json(self, merger, tmp_path):
+        """Test loading a valid JSON config file."""
+        config_path = tmp_path / "config.json"
+        config_data = {"key": "value", "nested": {"inner": "data"}}
+
+        with open(config_path, "w") as f:
+            json.dump(config_data, f)
+
+        result = merger.load_config(config_path)
+        assert result == config_data
+
+    def test_load_config_invalid_json(self, merger, tmp_path, caplog):
+        """Test loading an invalid JSON file logs error."""
+        config_path = tmp_path / "invalid.json"
+        config_path.write_text("not valid json")
+
+        with caplog.at_level(logging.ERROR):
+            result = merger.load_config(config_path)
+
+        assert result == {}
+        assert "Failed to parse" in caplog.text
+
+    def test_load_config_io_error(self, merger, tmp_path, caplog):
+        """Test handling IO errors during config loading."""
+        # Create a directory with the same name to cause read error
+        config_path = tmp_path / "config.json"
+        config_path.mkdir()
+
+        with caplog.at_level(logging.ERROR):
+            result = merger.load_config(config_path)
+
+        assert result == {}
+
+
+class TestLoadDeviceConfig:
+    """Test cases for load_device_config function."""
+
+    def test_load_device_config_nonexistent_device(self, tmp_path, monkeypatch, caplog):
+        """Test loading config for a non-existent device."""
+        monkeypatch.chdir(tmp_path)
+
+        # Create devices directory structure
+        devices_dir = tmp_path / "devices"
+        devices_dir.mkdir()
+        (devices_dir / "common").mkdir()
+        (devices_dir / "common" / "config.json").write_text('{"common_key": "value"}')
+
+        with caplog.at_level(logging.INFO):
+            result = load_device_config("nonexistent")
+
+        assert "common_key" in result
+
+    def test_load_device_config_merges_configs(self, tmp_path, monkeypatch):
+        """Test that device config properly merges with common config."""
+        monkeypatch.chdir(tmp_path)
+
+        devices_dir = tmp_path / "devices"
+        common_dir = devices_dir / "common"
+        device_dir = devices_dir / "testdevice"
+        common_dir.mkdir(parents=True)
+        device_dir.mkdir(parents=True)
+
+        # Write common config
+        with open(common_dir / "config.json", "w") as f:
+            json.dump({"wild_boost": {"enable": False}, "pack": {"type": "payload"}}, f)
+
+        # Write device config
+        with open(device_dir / "config.json", "w") as f:
+            json.dump({"wild_boost": {"enable": True}}, f)
+
+        result = load_device_config("testdevice")
+
+        # Device should override common
+        assert result["wild_boost"]["enable"] is True
+        # Common values should be preserved
+        assert result["pack"]["type"] == "payload"

tests/core/test_shell.py

@@ -0,0 +1,162 @@
+"""Unit tests for ShellRunner class."""
+
+import subprocess
+from pathlib import Path
+from unittest.mock import Mock, patch
+
+import pytest
+
+from src.utils.shell import ShellRunner
+
+
+class TestShellRunner:
+    """Test cases for ShellRunner class."""
+
+    @pytest.fixture
+    def runner(self):
+        """Create a ShellRunner instance."""
+        return ShellRunner()
+
+    def test_init_sets_platform(self, runner):
+        """Test that initialization sets platform attributes."""
+        assert runner.os_name in ["linux", "darwin", "windows"]
+        assert runner.arch in ["x86_64", "aarch64"]
+
+    def test_get_binary_path_platform_specific(self, runner, tmp_path, monkeypatch):
+        """Test finding binary in platform-specific directory."""
+        # Setup mock binary directory
+        runner.bin_dir = tmp_path / "bin"
+        tool_path = runner.bin_dir / "testtool"
+        runner.bin_dir.mkdir()
+        tool_path.touch()
+
+        result = runner.get_binary_path("testtool")
+        assert result == tool_path
+
+    def test_get_binary_path_otatools(self, runner, tmp_path, monkeypatch):
+        """Test finding binary in otatools directory."""
+        runner.otatools_bin = tmp_path / "otatools" / "bin"
+        runner.otatools_bin.mkdir(parents=True)
+        tool_path = runner.otatools_bin / "otatool"
+        tool_path.touch()
+
+        result = runner.get_binary_path("otatool")
+        assert result == tool_path
+
+    def test_get_binary_path_fallback(self, runner):
+        """Test fallback to command name when binary not found."""
+        result = runner.get_binary_path("nonexistent_tool_12345")
+        assert result == Path("nonexistent_tool_12345")
+
+    @patch("subprocess.run")
+    def test_run_with_list_command(self, mock_run, runner):
+        """Test running a command as a list."""
+        mock_run.return_value = subprocess.CompletedProcess(
+            args=["echo", "test"], returncode=0, stdout="test\n", stderr=""
+        )
+
+        result = runner.run(["echo", "test"])
+
+        mock_run.assert_called_once()
+        assert result.returncode == 0
+
+    @patch("subprocess.run")
+    def test_run_with_string_command(self, mock_run, runner):
+        """Test running a command as a string."""
+        mock_run.return_value = subprocess.CompletedProcess(
+            args="echo test", returncode=0, stdout="test\n", stderr=""
+        )
+
+        result = runner.run("echo test", shell=True)
+
+        mock_run.assert_called_once()
+
+    @patch("subprocess.run")
+    def test_run_raises_on_failure(self, mock_run, runner):
+        """Test that run raises exception on command failure."""
+        mock_run.side_effect = subprocess.CalledProcessError(
+            returncode=1, cmd=["false"], output="", stderr="error message"
+        )
+
+        with pytest.raises(subprocess.CalledProcessError):
+            runner.run(["false"])
+
+    @patch("subprocess.run")
+    def test_run_check_false_no_raise(self, mock_run, runner):
+        """Test that check=False doesn't raise on failure."""
+        mock_run.return_value = subprocess.CompletedProcess(
+            args=["false"], returncode=1, stdout="", stderr=""
+        )
+
+        result = runner.run(["false"], check=False)
+        assert result.returncode == 1
+
+    @patch("subprocess.run")
+    def test_run_with_cwd(self, mock_run, runner, tmp_path):
+        """Test running command with specific working directory."""
+        mock_run.return_value = subprocess.CompletedProcess(
+            args=["pwd"], returncode=0, stdout=str(tmp_path) + "\n", stderr=""
+        )
+
+        runner.run(["pwd"], cwd=tmp_path)
+
+        call_kwargs = mock_run.call_args[1]
+        assert call_kwargs["cwd"] == tmp_path
+
+    @patch("subprocess.run")
+    def test_run_with_env(self, mock_run, runner):
+        """Test running command with custom environment variables."""
+        mock_run.return_value = subprocess.CompletedProcess(
+            args=["env"], returncode=0, stdout="", stderr=""
+        )
+
+        runner.run(["env"], env={"TEST_VAR": "test_value"})
+
+        call_kwargs = mock_run.call_args[1]
+        assert "TEST_VAR" in call_kwargs["env"]
+        assert call_kwargs["env"]["TEST_VAR"] == "test_value"
+
+    @patch("subprocess.Popen")
+    def test_run_with_logger(self, mock_popen, runner, caplog):
+        """Test running command with logger streams output."""
+        mock_process = Mock()
+        mock_process.stdout = iter(["line1\n", "line2\n"])
+        mock_process.wait.return_value = 0
+        mock_popen.return_value = mock_process
+
+        logger = Mock()
+
+        runner.run(["test"], logger=logger)
+
+        mock_popen.assert_called_once()
+        assert logger.info.called
+
+    @patch("subprocess.Popen")
+    def test_run_with_callback(self, mock_popen, runner):
+        """Test running command with output callback."""
+        mock_process = Mock()
+        mock_process.stdout = iter(["line1\n", "line2\n"])
+        mock_process.wait.return_value = 0
+        mock_popen.return_value = mock_process
+
+        callback_lines = []
+        runner.run(["test"], on_line=callback_lines.append)
+
+        assert len(callback_lines) == 2
+        assert "line1" in callback_lines
+        assert "line2" in callback_lines
+
+    def test_run_java_jar(self, runner):
+        """Test helper for running Java JAR files."""
+        with patch.object(runner, "run") as mock_run:
+            mock_run.return_value = subprocess.CompletedProcess(
+                args=["java", "-jar", "test.jar", "arg1"], returncode=0, stdout="", stderr=""
+            )
+
+            result = runner.run_java_jar("test.jar", ["arg1"])
+
+            mock_run.assert_called_once()
+            call_args = mock_run.call_args[0][0]
+            assert call_args[0] == "java"
+            assert call_args[1] == "-jar"
+            assert "arg1" in call_args