Address review: drop redundant comments and rely on existing CORE.reset() between tests

esphome/components/external_components/__init__.py

@@ -41,8 +41,6 @@ async def to_code(config: dict[str, Any]) -> None:
 
 
 def _process_git_config(config: dict[str, Any], refresh: TimePeriodSeconds) -> Path:
-    # CORE.skip_external_update (set by `esphome logs`) is honored inside
-    # git.clone_or_update; no need to translate to NEVER_REFRESH here.
     repo_dir, _ = git.clone_or_update(
         url=config[CONF_URL],
         ref=config.get(CONF_REF),

esphome/components/packages/__init__.py

@@ -214,9 +214,6 @@ def _process_remote_package(config: dict[str, Any]) -> dict[str, Any]:
 
     If loading fails after cloning, attempts a revert and retry in case
     a prior cached checkout is stale.
-
-    CORE.skip_external_update (set by `esphome logs`) is honored inside
-    git.clone_or_update; no need to translate to NEVER_REFRESH here.
     """
     repo_dir, revert = git.clone_or_update(
         url=config[CONF_URL],
@@ -560,9 +557,6 @@ def do_packages_pass(
 
     Returns the config with all packages loaded in-place (but not yet merged)
     and a consolidated ``substitutions:`` block restored at the front.
-
-    CORE.skip_external_update (set by `esphome logs`) is honored inside
-    git.clone_or_update via _process_remote_package.
     """
     if CONF_PACKAGES not in config:
         return config

esphome/git.py

@@ -150,9 +150,6 @@ def clone_or_update(
             raise
 
     else:
-        # Check refresh needed
-        # Skip refresh if NEVER_REFRESH is specified or external updates are
-        # globally disabled for this run (e.g. `esphome logs`).
         if refresh == NEVER_REFRESH or CORE.skip_external_update:
             _LOGGER.debug("Skipping update for %s (refresh disabled)", key)
             return repo_dir, None

tests/component_tests/external_components/test_init.py

@@ -4,8 +4,6 @@ from pathlib import Path
 from typing import Any
 from unittest.mock import MagicMock
 
-import pytest
-
 from esphome.components.external_components import do_external_components_pass
 from esphome.const import (
     CONF_EXTERNAL_COMPONENTS,
@@ -38,18 +36,10 @@ def _make_config(tmp_path: Path) -> dict[str, Any]:
     }
 
 
-@pytest.fixture
-def reset_skip_external_update() -> None:
-    """Ensure CORE.skip_external_update is restored between tests."""
-    yield
-    CORE.skip_external_update = False
-
-
 def test_external_components_skip_update_via_core_flag(
     tmp_path: Path,
     mock_clone_or_update: MagicMock,
     mock_install_meta_finder: MagicMock,
-    reset_skip_external_update: None,
 ) -> None:
     """When CORE.skip_external_update is True, refresh is still passed through;
     git.clone_or_update itself short-circuits the actual fetch."""
@@ -69,7 +59,6 @@ def test_external_components_normal_refresh(
     tmp_path: Path,
     mock_clone_or_update: MagicMock,
     mock_install_meta_finder: MagicMock,
-    reset_skip_external_update: None,
 ) -> None:
     """When CORE.skip_external_update is False, the configured refresh value is used."""
     mock_clone_or_update.return_value = (tmp_path, None)

tests/component_tests/packages/test_init.py

@@ -4,8 +4,6 @@ from pathlib import Path
 from typing import Any
 from unittest.mock import MagicMock
 
-import pytest
-
 from esphome.components.packages import do_packages_pass
 from esphome.const import CONF_FILES, CONF_PACKAGES, CONF_REFRESH, CONF_URL
 from esphome.core import CORE, TimePeriodSeconds
@@ -24,18 +22,10 @@ def _make_config() -> dict[str, Any]:
     }
 
 
-@pytest.fixture
-def reset_skip_external_update() -> None:
-    """Ensure CORE.skip_external_update is restored between tests."""
-    yield
-    CORE.skip_external_update = False
-
-
 def test_packages_skip_update_via_core_flag(
     tmp_path: Path,
     mock_clone_or_update: MagicMock,
     mock_load_yaml: MagicMock,
-    reset_skip_external_update: None,
 ) -> None:
     """When CORE.skip_external_update is True, refresh is still passed through;
     git.clone_or_update itself short-circuits the actual fetch."""
@@ -60,7 +50,6 @@ def test_packages_normal_refresh(
     tmp_path: Path,
     mock_clone_or_update: MagicMock,
     mock_load_yaml: MagicMock,
-    reset_skip_external_update: None,
 ) -> None:
     """When CORE.skip_external_update is False, the configured refresh value is used."""
     mock_clone_or_update.return_value = (tmp_path, None)

tests/unit_tests/test_external_files.py

@@ -241,7 +241,9 @@ def test_download_content_with_network_error_no_cache_fails(
 @patch("esphome.external_files.requests.get")
 @patch("esphome.external_files.has_remote_file_changed")
 def test_download_content_skip_external_update_uses_cache(
-    mock_has_changed: MagicMock, mock_get: MagicMock, setup_core: Path
+    mock_has_changed: MagicMock,
+    mock_get: MagicMock,
+    setup_core: Path,
 ) -> None:
     """Test download_content skips network checks when CORE.skip_external_update is set."""
     test_file = setup_core / "cached.txt"
@@ -249,11 +251,8 @@ def test_download_content_skip_external_update_uses_cache(
     test_file.write_bytes(cached_content)
 
     CORE.skip_external_update = True
-    try:
-        url = "https://example.com/file.txt"
-        result = external_files.download_content(url, test_file)
-    finally:
-        CORE.skip_external_update = False
+    url = "https://example.com/file.txt"
+    result = external_files.download_content(url, test_file)
 
     assert result == cached_content
     mock_has_changed.assert_not_called()
@@ -263,7 +262,9 @@ def test_download_content_skip_external_update_uses_cache(
 @patch("esphome.external_files.requests.get")
 @patch("esphome.external_files.has_remote_file_changed")
 def test_download_content_skip_external_update_downloads_when_missing(
-    mock_has_changed: MagicMock, mock_get: MagicMock, setup_core: Path
+    mock_has_changed: MagicMock,
+    mock_get: MagicMock,
+    setup_core: Path,
 ) -> None:
     """Test download_content still downloads when file is missing, even with skip_external_update."""
     test_file = setup_core / "missing.txt"
@@ -276,11 +277,8 @@ def test_download_content_skip_external_update_downloads_when_missing(
     mock_get.return_value = mock_response
 
     CORE.skip_external_update = True
-    try:
-        url = "https://example.com/file.txt"
-        result = external_files.download_content(url, test_file)
-    finally:
-        CORE.skip_external_update = False
+    url = "https://example.com/file.txt"
+    result = external_files.download_content(url, test_file)
 
     assert result == new_content
     assert test_file.read_bytes() == new_content

tests/unit_tests/test_git.py

@@ -253,15 +253,12 @@ def test_clone_or_update_skips_when_core_skip_external_update(
     (git_dir / "FETCH_HEAD").write_text("test")
 
     CORE.skip_external_update = True
-    try:
-        result_dir, revert = git.clone_or_update(
-            url=url,
-            ref=ref,
-            refresh=TimePeriodSeconds(days=1),
-            domain=domain,
-        )
-    finally:
-        CORE.skip_external_update = False
+    result_dir, revert = git.clone_or_update(
+        url=url,
+        ref=ref,
+        refresh=TimePeriodSeconds(days=1),
+        domain=domain,
+    )
 
     mock_run_git_command.assert_not_called()
     assert result_dir == repo_dir