Merge remote-tracking branch 'origin/benchmarks-core-python-gate' into dnm-codspeed-deepcopy-proof

esphome/__main__.py

@@ -2601,6 +2601,8 @@ def run_esphome(argv):
         config = read_config(
             command_line_substitutions,
             skip_external_update=skip_external,
+            # Snapshot only needed by `esphome config --no-defaults`.
+            snapshot_user_config=getattr(args, "no_defaults", False),
         )
         # Refresh the cache so the next upload/logs hits the fast path
         # instead of re-running read_config. Skip when the storage

esphome/config.py

@@ -1108,6 +1108,7 @@ def validate_config(
     config: dict[str, Any],
     command_line_substitutions: dict[str, Any] | None,
     skip_external_update: bool = False,
+    snapshot_user_config: bool = False,
 ) -> Config:
     result = Config()
 
@@ -1218,11 +1219,13 @@ def validate_config(
     # Snapshot the user's config before any schema validation defaults are
     # applied. preload_core_config and later validation steps rewrite entries
     # in-place with defaulted values; deep-copying here preserves the
-    # user-supplied keys for `esphome config --no-defaults`.
-    result.user_config = copy.deepcopy(config)
-    if substitutions is not None:
-        result.user_config[CONF_SUBSTITUTIONS] = copy.deepcopy(substitutions)
-        result.user_config.move_to_end(CONF_SUBSTITUTIONS, last=False)
+    # user-supplied keys for `esphome config --no-defaults`. The deep copy is
+    # expensive, so it is only taken when that command actually asked for it.
+    if snapshot_user_config:
+        result.user_config = copy.deepcopy(config)
+        if substitutions is not None:
+            result.user_config[CONF_SUBSTITUTIONS] = copy.deepcopy(substitutions)
+            result.user_config.move_to_end(CONF_SUBSTITUTIONS, last=False)
 
     # 2. Load partial core config
     import esphome.core.config as core_config
@@ -1335,7 +1338,9 @@ class InvalidYAMLError(EsphomeError):
 
 
 def _load_config(
-    command_line_substitutions: dict[str, Any], skip_external_update: bool = False
+    command_line_substitutions: dict[str, Any],
+    skip_external_update: bool = False,
+    snapshot_user_config: bool = False,
 ) -> Config:
     """Load the configuration file."""
     try:
@@ -1344,7 +1349,12 @@ def _load_config(
         raise InvalidYAMLError(e) from e
 
     try:
-        return validate_config(config, command_line_substitutions, skip_external_update)
+        return validate_config(
+            config,
+            command_line_substitutions,
+            skip_external_update=skip_external_update,
+            snapshot_user_config=snapshot_user_config,
+        )
     except EsphomeError:
         raise
     except Exception:
@@ -1353,10 +1363,16 @@ def _load_config(
 
 
 def load_config(
-    command_line_substitutions: dict[str, Any], skip_external_update: bool = False
+    command_line_substitutions: dict[str, Any],
+    skip_external_update: bool = False,
+    snapshot_user_config: bool = False,
 ) -> Config:
     try:
-        return _load_config(command_line_substitutions, skip_external_update)
+        return _load_config(
+            command_line_substitutions,
+            skip_external_update=skip_external_update,
+            snapshot_user_config=snapshot_user_config,
+        )
     except vol.Invalid as err:
         raise EsphomeError(f"Error while parsing config: {err}") from err
 
@@ -1497,11 +1513,17 @@ def strip_default_ids(config):
 
 
 def read_config(
-    command_line_substitutions: dict[str, Any], skip_external_update: bool = False
+    command_line_substitutions: dict[str, Any],
+    skip_external_update: bool = False,
+    snapshot_user_config: bool = False,
 ) -> Config | None:
     _LOGGER.info("Reading configuration %s...", CORE.config_path)
     try:
-        res = load_config(command_line_substitutions, skip_external_update)
+        res = load_config(
+            command_line_substitutions,
+            skip_external_update=skip_external_update,
+            snapshot_user_config=snapshot_user_config,
+        )
     except EsphomeError as err:
         _LOGGER.error("Error while reading config: %s", err)
         return None

script/determine-jobs.py

@@ -63,6 +63,7 @@ from helpers import (
     CPP_FILE_EXTENSIONS,
     ESPHOME_TESTS_COMPONENTS_PATH,
     PYTHON_FILE_EXTENSIONS,
+    base_python_changed,
     changed_files,
     core_changed,
     filter_component_and_test_cpp_files,
@@ -657,16 +658,20 @@ BENCHMARK_INFRASTRUCTURE_FILES = frozenset(
 
 
 def should_run_benchmarks(branch: str | None = None) -> bool:
-    """Determine if C++ benchmarks should run based on changed files.
+    """Determine if benchmarks (C++ and Python) should run based on changed files.
 
     Benchmarks run when any of the following conditions are met:
 
-    1. Core C++ files changed (esphome/core/*)
-    2. The host platform changed (esphome/components/host/*) — benchmarks
+    1. Core files changed (esphome/core/*, C++ or Python)
+    2. Top-level Python files changed (esphome/*.py and esphome/*.pyi) —
+       the Python benchmarks exercise config loading (config.py,
+       yaml_util.py, ...), so a slowdown there is invisible unless the
+       benchmarks job runs
+    3. The host platform changed (esphome/components/host/*) — benchmarks
        are built and run on the host platform, so its implementations of
        ``millis()``/``micros()``/etc. affect every benchmark
-    3. A directly changed component has benchmark files (no dependency expansion)
-    4. Benchmark infrastructure changed (tests/benchmarks/*, script/cpp_benchmark.py,
+    4. A directly changed component has benchmark files (no dependency expansion)
+    5. Benchmark infrastructure changed (tests/benchmarks/*, script/cpp_benchmark.py,
        script/build_helpers.py, script/setup_codspeed_lib.py)
 
     Unlike unit tests, benchmarks do NOT expand to dependent components.
@@ -683,6 +688,11 @@ def should_run_benchmarks(branch: str | None = None) -> bool:
     if core_changed(files):
         return True
 
+    # Top-level esphome/*.py modules are what the Python benchmarks in
+    # tests/benchmarks/python/ exercise
+    if base_python_changed(files):
+        return True
+
     # Host platform supplies the runtime that benchmarks execute on
     if any(f.startswith("esphome/components/host/") for f in files):
         return True

script/helpers.py

@@ -1380,6 +1380,27 @@ def core_changed(files: list[str]) -> bool:
     )
 
 
+def base_python_changed(files: list[str]) -> bool:
+    """Check if any Python file directly in esphome/ has changed.
+
+    Matches top-level modules and stubs (.py and .pyi) like esphome/config.py
+    and esphome/yaml_util.py but not files in subdirectories such as
+    esphome/components/ or esphome/dashboard/.
+
+    Args:
+        files: List of file paths to check
+
+    Returns:
+        True if any top-level esphome Python file has changed
+    """
+    return any(
+        f.startswith("esphome/")
+        and f.endswith(PYTHON_FILE_EXTENSIONS)
+        and "/" not in f.removeprefix("esphome/")
+        for f in files
+    )
+
+
 def get_cpp_changed_components(files: list[str]) -> list[str]:
     """Get components that have changed C++ files or tests.
 

tests/script/test_determine_jobs.py

@@ -2475,6 +2475,35 @@ def test_should_run_benchmarks_core_header_change() -> None:
         assert determine_jobs.should_run_benchmarks() is True
 
 
+def test_should_run_benchmarks_top_level_python_change() -> None:
+    """Test benchmarks trigger on top-level esphome Python module changes.
+
+    The Python benchmarks exercise config loading, so changes to modules
+    like config.py and yaml_util.py must run them; a regression in #16718
+    went unnoticed because these files matched no trigger.
+    """
+    for py_file in [
+        "esphome/config.py",
+        "esphome/yaml_util.py",
+        "esphome/__main__.py",
+        "esphome/helpers.py",
+    ]:
+        with patch.object(determine_jobs, "changed_files", return_value=[py_file]):
+            assert determine_jobs.should_run_benchmarks() is True, (
+                f"Expected benchmarks to run for {py_file}"
+            )
+
+
+def test_should_run_benchmarks_nested_python_change() -> None:
+    """Test benchmarks do NOT trigger for nested non-core Python changes."""
+    with patch.object(
+        determine_jobs,
+        "changed_files",
+        return_value=["esphome/dashboard/web_server.py"],
+    ):
+        assert determine_jobs.should_run_benchmarks() is False
+
+
 def test_should_run_benchmarks_host_platform_change() -> None:
     """Test benchmarks trigger on host platform changes.
 

tests/script/test_helpers.py

@@ -1851,3 +1851,24 @@ def test_get_component_test_files_component_without_tests(
 )
 def test_is_validate_only_file(filename: str, expected: bool, tmp_path: Path) -> None:
     assert helpers.is_validate_only_file(tmp_path / filename) is expected
+
+
+@pytest.mark.parametrize(
+    ("files", "expected"),
+    [
+        (["esphome/config.py"], True),
+        (["esphome/yaml_util.py"], True),
+        (["esphome/__main__.py"], True),
+        (["esphome/const.pyi"], True),
+        (["README.md", "esphome/helpers.py"], True),
+        (["esphome/core/config.py"], False),
+        (["esphome/components/sensor/__init__.py"], False),
+        (["esphome/dashboard/web_server.py"], False),
+        (["esphome/idf_component.yml"], False),
+        (["tests/unit_tests/test_config.py"], False),
+        ([], False),
+    ],
+)
+def test_base_python_changed(files: list[str], expected: bool) -> None:
+    """Only Python modules directly in esphome/ count as base Python changes."""
+    assert helpers.base_python_changed(files) is expected

tests/unit_tests/test_main.py

@@ -6362,6 +6362,26 @@ def test_run_esphome_skip_external_update_per_command(
     assert mock_read.call_args.kwargs["skip_external_update"] is expected_skip
 
 
+@pytest.mark.parametrize(
+    ("argv_extra", "expected"),
+    [(["--no-defaults"], True), ([], False)],
+)
+def test_run_esphome_snapshot_user_config_only_for_no_defaults(
+    tmp_path: Path, argv_extra: list[str], expected: bool
+) -> None:
+    """read_config is invoked with snapshot_user_config=True only when the
+    config command is run with --no-defaults; otherwise the expensive deep
+    copy is skipped."""
+    yaml_file = tmp_path / "device.yaml"
+    yaml_file.write_text("esphome:\n  name: test\n")
+
+    with patch("esphome.config.read_config", return_value=None) as mock_read:
+        run_esphome(["esphome", "config", str(yaml_file), *argv_extra])
+
+    mock_read.assert_called_once()
+    assert mock_read.call_args.kwargs["snapshot_user_config"] is expected
+
+
 def test_get_configured_xtal_freq_reads_sdkconfig(tmp_path: Path) -> None:
     """Test reading XTAL_FREQ from sdkconfig."""
     CORE.name = "test-device"

tests/unit_tests/test_substitutions.py

@@ -370,7 +370,7 @@ def test_validate_config_captures_user_config_snapshot(tmp_path: Path) -> None:
     """
     test_config = _get_test_minimal_valid_config(tmp_path)
 
-    result = config_module.validate_config(test_config, None)
+    result = config_module.validate_config(test_config, None, snapshot_user_config=True)
 
     # Snapshot is populated.
     assert result.user_config is not None
@@ -393,7 +393,7 @@ def test_validate_config_user_config_snapshot_is_deep_copy(tmp_path: Path) -> No
     """
     test_config = _get_test_minimal_valid_config(tmp_path)
 
-    result = config_module.validate_config(test_config, None)
+    result = config_module.validate_config(test_config, None, snapshot_user_config=True)
 
     assert result.user_config is not None
     # preload_core_config injected build_path onto the validated config.
@@ -404,6 +404,32 @@ def test_validate_config_user_config_snapshot_is_deep_copy(tmp_path: Path) -> No
     assert result["esphome"] is not result.user_config["esphome"]
 
 
+def test_validate_config_snapshot_without_substitutions(tmp_path: Path) -> None:
+    """The snapshot works for configs that have no substitutions block."""
+    test_config = _get_test_minimal_valid_config(tmp_path)
+    del test_config[CONF_SUBSTITUTIONS]
+
+    result = config_module.validate_config(test_config, None, snapshot_user_config=True)
+
+    assert result.user_config is not None
+    assert CONF_SUBSTITUTIONS not in result.user_config
+    assert result.user_config["esphome"] == {"name": "test_device"}
+
+
+def test_validate_config_skips_user_config_snapshot_by_default(
+    tmp_path: Path,
+) -> None:
+    """Without ``snapshot_user_config`` the deep copy is skipped entirely;
+    only ``esphome config --no-defaults`` needs the snapshot and the copy is
+    too expensive to take on every load.
+    """
+    test_config = _get_test_minimal_valid_config(tmp_path)
+
+    result = config_module.validate_config(test_config, None)
+
+    assert result.user_config is None
+
+
 def test_merge_config_preserves_ordered_dict() -> None:
     """Test that merge_config preserves OrderedDict type.