diff --git a/script/determine-jobs.py b/script/determine-jobs.py index 8039aff83f9..e4d002975c6 100755 --- a/script/determine-jobs.py +++ b/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 diff --git a/script/helpers.py b/script/helpers.py index 6ba093b413a..7cc001d92f5 100644 --- a/script/helpers.py +++ b/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. diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index a05b683a5ff..80f572d9fe6 100644 --- a/tests/script/test_determine_jobs.py +++ b/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. diff --git a/tests/script/test_helpers.py b/tests/script/test_helpers.py index 43c4445dcfc..077b6ef23e5 100644 --- a/tests/script/test_helpers.py +++ b/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