Merge branch 'dev' into sha256_ota

tests/unit_tests/conftest.py

@@ -87,3 +87,17 @@ def mock_run_external_command() -> Generator[Mock, None, None]:
     """Mock run_external_command for platformio_api."""
     with patch("esphome.platformio_api.run_external_command") as mock:
         yield mock
+
+
+@pytest.fixture
+def mock_run_git_command() -> Generator[Mock, None, None]:
+    """Mock run_git_command for git module."""
+    with patch("esphome.git.run_git_command") as mock:
+        yield mock
+
+
+@pytest.fixture
+def mock_get_idedata() -> Generator[Mock, None, None]:
+    """Mock get_idedata for platformio_api."""
+    with patch("esphome.platformio_api.get_idedata") as mock:
+        yield mock

tests/unit_tests/test_git.py

@@ -0,0 +1,246 @@
+"""Tests for git.py module."""
+
+from datetime import datetime, timedelta
+import hashlib
+import os
+from pathlib import Path
+from unittest.mock import Mock
+
+from esphome import git
+from esphome.core import CORE, TimePeriodSeconds
+
+
+def test_clone_or_update_with_never_refresh(
+    tmp_path: Path, mock_run_git_command: Mock
+) -> None:
+    """Test that NEVER_REFRESH skips updates for existing repos."""
+    # Set up CORE.config_path so data_dir uses tmp_path
+    CORE.config_path = tmp_path / "test.yaml"
+
+    # Compute the expected repo directory path
+    url = "https://github.com/test/repo"
+    ref = None
+    key = f"{url}@{ref}"
+    domain = "test"
+
+    # Compute hash-based directory name (matching _compute_destination_path logic)
+    h = hashlib.new("sha256")
+    h.update(key.encode())
+    repo_dir = tmp_path / ".esphome" / domain / h.hexdigest()[:8]
+
+    # Create the git repo directory structure
+    repo_dir.mkdir(parents=True)
+    git_dir = repo_dir / ".git"
+    git_dir.mkdir()
+
+    # Create FETCH_HEAD file with current timestamp
+    fetch_head = git_dir / "FETCH_HEAD"
+    fetch_head.write_text("test")
+
+    # Call with NEVER_REFRESH
+    result_dir, revert = git.clone_or_update(
+        url=url,
+        ref=ref,
+        refresh=git.NEVER_REFRESH,
+        domain=domain,
+    )
+
+    # Should NOT call git commands since NEVER_REFRESH and repo exists
+    mock_run_git_command.assert_not_called()
+    assert result_dir == repo_dir
+    assert revert is None
+
+
+def test_clone_or_update_with_refresh_updates_old_repo(
+    tmp_path: Path, mock_run_git_command: Mock
+) -> None:
+    """Test that refresh triggers update for old repos."""
+    # Set up CORE.config_path so data_dir uses tmp_path
+    CORE.config_path = tmp_path / "test.yaml"
+
+    # Compute the expected repo directory path
+    url = "https://github.com/test/repo"
+    ref = None
+    key = f"{url}@{ref}"
+    domain = "test"
+
+    # Compute hash-based directory name (matching _compute_destination_path logic)
+    h = hashlib.new("sha256")
+    h.update(key.encode())
+    repo_dir = tmp_path / ".esphome" / domain / h.hexdigest()[:8]
+
+    # Create the git repo directory structure
+    repo_dir.mkdir(parents=True)
+    git_dir = repo_dir / ".git"
+    git_dir.mkdir()
+
+    # Create FETCH_HEAD file with old timestamp (2 days ago)
+    fetch_head = git_dir / "FETCH_HEAD"
+    fetch_head.write_text("test")
+    old_time = datetime.now() - timedelta(days=2)
+    fetch_head.touch()  # Create the file
+    # Set modification time to 2 days ago
+    os.utime(fetch_head, (old_time.timestamp(), old_time.timestamp()))
+
+    # Mock git command responses
+    mock_run_git_command.return_value = "abc123"  # SHA for rev-parse
+
+    # Call with refresh=1d (1 day)
+    refresh = TimePeriodSeconds(days=1)
+    result_dir, revert = git.clone_or_update(
+        url=url,
+        ref=ref,
+        refresh=refresh,
+        domain=domain,
+    )
+
+    # Should call git fetch and update commands since repo is older than refresh
+    assert mock_run_git_command.called
+    # Check for fetch command
+    fetch_calls = [
+        call
+        for call in mock_run_git_command.call_args_list
+        if len(call[0]) > 0 and "fetch" in call[0][0]
+    ]
+    assert len(fetch_calls) > 0
+
+
+def test_clone_or_update_with_refresh_skips_fresh_repo(
+    tmp_path: Path, mock_run_git_command: Mock
+) -> None:
+    """Test that refresh doesn't update fresh repos."""
+    # Set up CORE.config_path so data_dir uses tmp_path
+    CORE.config_path = tmp_path / "test.yaml"
+
+    # Compute the expected repo directory path
+    url = "https://github.com/test/repo"
+    ref = None
+    key = f"{url}@{ref}"
+    domain = "test"
+
+    # Compute hash-based directory name (matching _compute_destination_path logic)
+    h = hashlib.new("sha256")
+    h.update(key.encode())
+    repo_dir = tmp_path / ".esphome" / domain / h.hexdigest()[:8]
+
+    # Create the git repo directory structure
+    repo_dir.mkdir(parents=True)
+    git_dir = repo_dir / ".git"
+    git_dir.mkdir()
+
+    # Create FETCH_HEAD file with recent timestamp (1 hour ago)
+    fetch_head = git_dir / "FETCH_HEAD"
+    fetch_head.write_text("test")
+    recent_time = datetime.now() - timedelta(hours=1)
+    fetch_head.touch()  # Create the file
+    # Set modification time to 1 hour ago
+    os.utime(fetch_head, (recent_time.timestamp(), recent_time.timestamp()))
+
+    # Call with refresh=1d (1 day)
+    refresh = TimePeriodSeconds(days=1)
+    result_dir, revert = git.clone_or_update(
+        url=url,
+        ref=ref,
+        refresh=refresh,
+        domain=domain,
+    )
+
+    # Should NOT call git fetch since repo is fresh
+    mock_run_git_command.assert_not_called()
+    assert result_dir == repo_dir
+    assert revert is None
+
+
+def test_clone_or_update_clones_missing_repo(
+    tmp_path: Path, mock_run_git_command: Mock
+) -> None:
+    """Test that missing repos are cloned regardless of refresh setting."""
+    # Set up CORE.config_path so data_dir uses tmp_path
+    CORE.config_path = tmp_path / "test.yaml"
+
+    # Compute the expected repo directory path
+    url = "https://github.com/test/repo"
+    ref = None
+    key = f"{url}@{ref}"
+    domain = "test"
+
+    # Compute hash-based directory name (matching _compute_destination_path logic)
+    h = hashlib.new("sha256")
+    h.update(key.encode())
+    repo_dir = tmp_path / ".esphome" / domain / h.hexdigest()[:8]
+
+    # Create base directory but NOT the repo itself
+    base_dir = tmp_path / ".esphome" / domain
+    base_dir.mkdir(parents=True)
+    # repo_dir should NOT exist
+    assert not repo_dir.exists()
+
+    # Test with NEVER_REFRESH - should still clone since repo doesn't exist
+    result_dir, revert = git.clone_or_update(
+        url=url,
+        ref=ref,
+        refresh=git.NEVER_REFRESH,
+        domain=domain,
+    )
+
+    # Should call git clone
+    assert mock_run_git_command.called
+    clone_calls = [
+        call
+        for call in mock_run_git_command.call_args_list
+        if len(call[0]) > 0 and "clone" in call[0][0]
+    ]
+    assert len(clone_calls) > 0
+
+
+def test_clone_or_update_with_none_refresh_always_updates(
+    tmp_path: Path, mock_run_git_command: Mock
+) -> None:
+    """Test that refresh=None always updates existing repos."""
+    # Set up CORE.config_path so data_dir uses tmp_path
+    CORE.config_path = tmp_path / "test.yaml"
+
+    # Compute the expected repo directory path
+    url = "https://github.com/test/repo"
+    ref = None
+    key = f"{url}@{ref}"
+    domain = "test"
+
+    # Compute hash-based directory name (matching _compute_destination_path logic)
+    h = hashlib.new("sha256")
+    h.update(key.encode())
+    repo_dir = tmp_path / ".esphome" / domain / h.hexdigest()[:8]
+
+    # Create the git repo directory structure
+    repo_dir.mkdir(parents=True)
+    git_dir = repo_dir / ".git"
+    git_dir.mkdir()
+
+    # Create FETCH_HEAD file with very recent timestamp (1 second ago)
+    fetch_head = git_dir / "FETCH_HEAD"
+    fetch_head.write_text("test")
+    recent_time = datetime.now() - timedelta(seconds=1)
+    fetch_head.touch()  # Create the file
+    # Set modification time to 1 second ago
+    os.utime(fetch_head, (recent_time.timestamp(), recent_time.timestamp()))
+
+    # Mock git command responses
+    mock_run_git_command.return_value = "abc123"  # SHA for rev-parse
+
+    # Call with refresh=None (default behavior)
+    result_dir, revert = git.clone_or_update(
+        url=url,
+        ref=ref,
+        refresh=None,
+        domain=domain,
+    )
+
+    # Should call git fetch and update commands since refresh=None means always update
+    assert mock_run_git_command.called
+    # Check for fetch command
+    fetch_calls = [
+        call
+        for call in mock_run_git_command.call_args_list
+        if len(call[0]) > 0 and "fetch" in call[0][0]
+    ]
+    assert len(fetch_calls) > 0

tests/unit_tests/test_main.py

@@ -5,16 +5,19 @@ from __future__ import annotations
 from collections.abc import Generator
 from dataclasses import dataclass
 from pathlib import Path
+import re
 from typing import Any
 from unittest.mock import MagicMock, Mock, patch
 
 import pytest
 from pytest import CaptureFixture
 
+from esphome import platformio_api
 from esphome.__main__ import (
     Purpose,
     choose_upload_log_host,
     command_rename,
+    command_update_all,
     command_wizard,
     get_port_type,
     has_ip_address,
@@ -26,7 +29,9 @@ from esphome.__main__ import (
     mqtt_get_ip,
     show_logs,
     upload_program,
+    upload_using_esptool,
 )
+from esphome.components.esp32.const import KEY_ESP32, KEY_VARIANT, VARIANT_ESP32
 from esphome.const import (
     CONF_API,
     CONF_BROKER,
@@ -55,6 +60,17 @@ from esphome.const import (
 from esphome.core import CORE, EsphomeError
 
 
+def strip_ansi_codes(text: str) -> str:
+    """Remove ANSI escape codes from text.
+
+    This helps make test assertions cleaner by removing color codes and other
+    terminal formatting that can make tests brittle.
+    """
+    # Pattern to match ANSI escape sequences
+    ansi_escape = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")
+    return ansi_escape.sub("", text)
+
+
 @dataclass
 class MockSerialPort:
     """Mock serial port for testing.
@@ -207,6 +223,14 @@ def mock_run_external_process() -> Generator[Mock]:
         yield mock
 
 
+@pytest.fixture
+def mock_run_external_command() -> Generator[Mock]:
+    """Mock run_external_command for testing."""
+    with patch("esphome.__main__.run_external_command") as mock:
+        mock.return_value = 0  # Default to success
+        yield mock
+
+
 def test_choose_upload_log_host_with_string_default() -> None:
     """Test with a single string default device."""
     setup_core()
@@ -805,6 +829,122 @@ def test_upload_program_serial_esp8266_with_file(
     )
 
 
+def test_upload_using_esptool_path_conversion(
+    tmp_path: Path,
+    mock_run_external_command: Mock,
+    mock_get_idedata: Mock,
+) -> None:
+    """Test upload_using_esptool properly converts Path objects to strings for esptool.
+
+    This test ensures that img.path (Path object) is converted to string before
+    passing to esptool, preventing AttributeError.
+    """
+    setup_core(platform=PLATFORM_ESP32, tmp_path=tmp_path, name="test")
+
+    # Set up ESP32-specific data required by get_esp32_variant()
+    CORE.data[KEY_ESP32] = {KEY_VARIANT: VARIANT_ESP32}
+
+    # Create mock IDEData with Path objects
+    mock_idedata = MagicMock(spec=platformio_api.IDEData)
+    mock_idedata.firmware_bin_path = tmp_path / "firmware.bin"
+    mock_idedata.extra_flash_images = [
+        platformio_api.FlashImage(path=tmp_path / "bootloader.bin", offset="0x1000"),
+        platformio_api.FlashImage(path=tmp_path / "partitions.bin", offset="0x8000"),
+    ]
+
+    mock_get_idedata.return_value = mock_idedata
+
+    # Create the actual firmware files so they exist
+    (tmp_path / "firmware.bin").touch()
+    (tmp_path / "bootloader.bin").touch()
+    (tmp_path / "partitions.bin").touch()
+
+    config = {CONF_ESPHOME: {"platformio_options": {}}}
+
+    # Call upload_using_esptool without custom file argument
+    result = upload_using_esptool(config, "/dev/ttyUSB0", None, None)
+
+    assert result == 0
+
+    # Verify that run_external_command was called
+    assert mock_run_external_command.call_count == 1
+
+    # Get the actual call arguments
+    call_args = mock_run_external_command.call_args[0]
+
+    # The first argument should be esptool.main function,
+    # followed by the command arguments
+    assert len(call_args) > 1
+
+    # Find the indices of the flash image arguments
+    # They should come after "write-flash" and "-z"
+    cmd_list = list(call_args[1:])  # Skip the esptool.main function
+
+    # Verify all paths are strings, not Path objects
+    # The firmware and flash images should be at specific positions
+    write_flash_idx = cmd_list.index("write-flash")
+
+    # After write-flash we have: -z, --flash-size, detect, then offset/path pairs
+    # Check firmware at offset 0x10000 (ESP32)
+    firmware_offset_idx = write_flash_idx + 4
+    assert cmd_list[firmware_offset_idx] == "0x10000"
+    firmware_path = cmd_list[firmware_offset_idx + 1]
+    assert isinstance(firmware_path, str)
+    assert firmware_path.endswith("firmware.bin")
+
+    # Check bootloader
+    bootloader_offset_idx = firmware_offset_idx + 2
+    assert cmd_list[bootloader_offset_idx] == "0x1000"
+    bootloader_path = cmd_list[bootloader_offset_idx + 1]
+    assert isinstance(bootloader_path, str)
+    assert bootloader_path.endswith("bootloader.bin")
+
+    # Check partitions
+    partitions_offset_idx = bootloader_offset_idx + 2
+    assert cmd_list[partitions_offset_idx] == "0x8000"
+    partitions_path = cmd_list[partitions_offset_idx + 1]
+    assert isinstance(partitions_path, str)
+    assert partitions_path.endswith("partitions.bin")
+
+
+def test_upload_using_esptool_with_file_path(
+    tmp_path: Path,
+    mock_run_external_command: Mock,
+) -> None:
+    """Test upload_using_esptool with a custom file that's a Path object."""
+    setup_core(platform=PLATFORM_ESP8266, tmp_path=tmp_path, name="test")
+
+    # Create a test firmware file
+    firmware_file = tmp_path / "custom_firmware.bin"
+    firmware_file.touch()
+
+    config = {CONF_ESPHOME: {"platformio_options": {}}}
+
+    # Call with a Path object as the file argument (though usually it's a string)
+    result = upload_using_esptool(config, "/dev/ttyUSB0", str(firmware_file), None)
+
+    assert result == 0
+
+    # Verify that run_external_command was called
+    mock_run_external_command.assert_called_once()
+
+    # Get the actual call arguments
+    call_args = mock_run_external_command.call_args[0]
+    cmd_list = list(call_args[1:])  # Skip the esptool.main function
+
+    # Find the firmware path in the command
+    write_flash_idx = cmd_list.index("write-flash")
+
+    # For custom file, it should be at offset 0x0
+    firmware_offset_idx = write_flash_idx + 4
+    assert cmd_list[firmware_offset_idx] == "0x0"
+    firmware_path = cmd_list[firmware_offset_idx + 1]
+
+    # Verify it's a string, not a Path object
+    assert isinstance(firmware_path, str)
+    assert firmware_path.endswith("custom_firmware.bin")
+
+
 @pytest.mark.parametrize(
     "platform,device",
     [
@@ -1545,3 +1685,171 @@ esp32:
 
     captured = capfd.readouterr()
     assert "Rename failed" in captured.out
+
+
+def test_command_update_all_path_string_conversion(
+    tmp_path: Path,
+    mock_run_external_process: Mock,
+    capfd: CaptureFixture[str],
+) -> None:
+    """Test that command_update_all properly converts Path objects to strings in output."""
+    yaml1 = tmp_path / "device1.yaml"
+    yaml1.write_text("""
+esphome:
+  name: device1
+
+esp32:
+  board: nodemcu-32s
+""")
+
+    yaml2 = tmp_path / "device2.yaml"
+    yaml2.write_text("""
+esphome:
+  name: device2
+
+esp8266:
+  board: nodemcuv2
+""")
+
+    setup_core(tmp_path=tmp_path)
+    mock_run_external_process.return_value = 0
+
+    assert command_update_all(MockArgs(configuration=[str(tmp_path)])) == 0
+
+    captured = capfd.readouterr()
+    clean_output = strip_ansi_codes(captured.out)
+
+    # Check that Path objects were properly converted to strings
+    # The output should contain file paths without causing TypeError
+    assert "device1.yaml" in clean_output
+    assert "device2.yaml" in clean_output
+    assert "SUCCESS" in clean_output
+    assert "SUMMARY" in clean_output
+
+    # Verify run_external_process was called for each file
+    assert mock_run_external_process.call_count == 2
+
+
+def test_command_update_all_with_failures(
+    tmp_path: Path,
+    mock_run_external_process: Mock,
+    capfd: CaptureFixture[str],
+) -> None:
+    """Test command_update_all handles mixed success/failure cases properly."""
+    yaml1 = tmp_path / "success_device.yaml"
+    yaml1.write_text("""
+esphome:
+  name: success_device
+
+esp32:
+  board: nodemcu-32s
+""")
+
+    yaml2 = tmp_path / "failed_device.yaml"
+    yaml2.write_text("""
+esphome:
+  name: failed_device
+
+esp8266:
+  board: nodemcuv2
+""")
+
+    setup_core(tmp_path=tmp_path)
+
+    # Mock mixed results - first succeeds, second fails
+    mock_run_external_process.side_effect = [0, 1]
+
+    # Should return 1 (failure) since one device failed
+    assert command_update_all(MockArgs(configuration=[str(tmp_path)])) == 1
+
+    captured = capfd.readouterr()
+    clean_output = strip_ansi_codes(captured.out)
+
+    # Check that both success and failure are properly displayed
+    assert "SUCCESS" in clean_output
+    assert "ERROR" in clean_output or "FAILED" in clean_output
+    assert "SUMMARY" in clean_output
+
+    # Files are processed in alphabetical order, so we need to check which one succeeded/failed
+    # The mock_run_external_process.side_effect = [0, 1] applies to files in alphabetical order
+    # So "failed_device.yaml" gets 0 (success) and "success_device.yaml" gets 1 (failure)
+    assert "failed_device.yaml: SUCCESS" in clean_output
+    assert "success_device.yaml: FAILED" in clean_output
+
+
+def test_command_update_all_empty_directory(
+    tmp_path: Path,
+    mock_run_external_process: Mock,
+    capfd: CaptureFixture[str],
+) -> None:
+    """Test command_update_all with an empty directory (no YAML files)."""
+    setup_core(tmp_path=tmp_path)
+
+    assert command_update_all(MockArgs(configuration=[str(tmp_path)])) == 0
+    mock_run_external_process.assert_not_called()
+
+    captured = capfd.readouterr()
+    clean_output = strip_ansi_codes(captured.out)
+
+    assert "SUMMARY" in clean_output
+
+
+def test_command_update_all_single_file(
+    tmp_path: Path,
+    mock_run_external_process: Mock,
+    capfd: CaptureFixture[str],
+) -> None:
+    """Test command_update_all with a single YAML file specified."""
+    yaml_file = tmp_path / "single_device.yaml"
+    yaml_file.write_text("""
+esphome:
+  name: single_device
+
+esp32:
+  board: nodemcu-32s
+""")
+
+    setup_core(tmp_path=tmp_path)
+    mock_run_external_process.return_value = 0
+
+    assert command_update_all(MockArgs(configuration=[str(yaml_file)])) == 0
+
+    captured = capfd.readouterr()
+    clean_output = strip_ansi_codes(captured.out)
+
+    assert "single_device.yaml" in clean_output
+    assert "SUCCESS" in clean_output
+    mock_run_external_process.assert_called_once()
+
+
+def test_command_update_all_path_formatting_in_color_calls(
+    tmp_path: Path,
+    mock_run_external_process: Mock,
+    capfd: CaptureFixture[str],
+) -> None:
+    """Test that Path objects are properly converted when passed to color() function."""
+    yaml_file = tmp_path / "test-device_123.yaml"
+    yaml_file.write_text("""
+esphome:
+  name: test-device_123
+
+esp32:
+  board: nodemcu-32s
+""")
+
+    setup_core(tmp_path=tmp_path)
+    mock_run_external_process.return_value = 0
+
+    assert command_update_all(MockArgs(configuration=[str(tmp_path)])) == 0
+
+    captured = capfd.readouterr()
+    clean_output = strip_ansi_codes(captured.out)
+
+    assert "test-device_123.yaml" in clean_output
+    assert "Updating" in clean_output
+    assert "SUCCESS" in clean_output
+    assert "SUMMARY" in clean_output
+
+    # Should not have any Python error messages
+    assert "TypeError" not in clean_output
+    assert "can only concatenate str" not in clean_output