Fix clang-tidy triggering full scan on Python-only core file changes (#9412)

This commit is contained in:
J. Nick Koston 2025-07-09 14:41:59 -10:00 committed by GitHub
parent ff836a8434
commit e9c7596e00
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 54 additions and 12 deletions

View File

@ -168,21 +168,26 @@ def get_changed_components() -> list[str] | None:
"""Get list of changed components using list-components.py script. """Get list of changed components using list-components.py script.
This function: This function:
1. First checks if any core files (esphome/core/*) changed - if so, returns None 1. First checks if any core C++/header files (esphome/core/*.{cpp,h,hpp,cc,cxx,c}) changed - if so, returns None
2. Otherwise delegates to ./script/list-components.py --changed which: 2. Otherwise delegates to ./script/list-components.py --changed which:
- Analyzes all changed files - Analyzes all changed files
- Determines which components are affected (including dependencies) - Determines which components are affected (including dependencies)
- Returns a list of component names that need to be checked - Returns a list of component names that need to be checked
Returns: Returns:
- None: Core files changed, need full scan - None: Core C++/header files changed, need full scan
- Empty list: No components changed (only non-component files changed) - Empty list: No components changed (only non-component files changed)
- List of strings: Names of components that need checking (e.g., ["wifi", "mqtt"]) - List of strings: Names of components that need checking (e.g., ["wifi", "mqtt"])
""" """
# Check if any core files changed first # Check if any core C++ or header files changed first
changed = changed_files() changed = changed_files()
if any(f.startswith("esphome/core/") for f in changed): core_cpp_changed = any(
print("Core files changed - will run full clang-tidy scan") f.startswith("esphome/core/")
and f.endswith((".cpp", ".h", ".hpp", ".cc", ".cxx", ".c"))
for f in changed
)
if core_cpp_changed:
print("Core C++/header files changed - will run full clang-tidy scan")
return None return None
# Use list-components.py to get changed components # Use list-components.py to get changed components
@ -207,10 +212,10 @@ def _filter_changed_ci(files: list[str]) -> list[str]:
This function implements intelligent filtering to reduce CI runtime by only This function implements intelligent filtering to reduce CI runtime by only
checking files that could be affected by the changes. It handles three scenarios: checking files that could be affected by the changes. It handles three scenarios:
1. Core files changed (returns None from get_changed_components): 1. Core C++/header files changed (returns None from get_changed_components):
- Triggered when any file in esphome/core/ is modified - Triggered when any C++/header file in esphome/core/ is modified
- Action: Check ALL files (full scan) - Action: Check ALL files (full scan)
- Reason: Core files are used throughout the codebase - Reason: Core C++/header files are used throughout the codebase
2. No components changed (returns empty list from get_changed_components): 2. No components changed (returns empty list from get_changed_components):
- Triggered when only non-component files changed (e.g., scripts, configs) - Triggered when only non-component files changed (e.g., scripts, configs)

View File

@ -394,10 +394,10 @@ def test_get_changed_files_from_command_relative_paths() -> None:
["esphome/core/application.h", "esphome/core/defines.h"], ["esphome/core/application.h", "esphome/core/defines.h"],
], ],
) )
def test_get_changed_components_core_files_trigger_full_scan( def test_get_changed_components_core_cpp_files_trigger_full_scan(
changed_files_list: list[str], changed_files_list: list[str],
) -> None: ) -> None:
"""Test that core file changes trigger full scan without calling subprocess.""" """Test that core C++/header file changes trigger full scan without calling subprocess."""
with patch("helpers.changed_files") as mock_changed: with patch("helpers.changed_files") as mock_changed:
mock_changed.return_value = changed_files_list mock_changed.return_value = changed_files_list
@ -406,6 +406,43 @@ def test_get_changed_components_core_files_trigger_full_scan(
assert result is None assert result is None
def test_get_changed_components_core_python_files_no_full_scan() -> None:
"""Test that core Python file changes do NOT trigger full scan."""
changed_files_list = [
"esphome/core/__init__.py",
"esphome/core/config.py",
"esphome/components/wifi/wifi.cpp",
]
with patch("helpers.changed_files") as mock_changed:
mock_changed.return_value = changed_files_list
mock_result = Mock()
mock_result.stdout = "wifi\n"
with patch("subprocess.run", return_value=mock_result):
result = get_changed_components()
# Should NOT return None - should call list-components.py
assert result == ["wifi"]
def test_get_changed_components_mixed_core_files_with_cpp() -> None:
"""Test that mixed Python and C++ core files still trigger full scan due to C++ file."""
changed_files_list = [
"esphome/core/__init__.py",
"esphome/core/config.py",
"esphome/core/helpers.cpp", # This C++ file should trigger full scan
"esphome/components/wifi/wifi.cpp",
]
with patch("helpers.changed_files") as mock_changed:
mock_changed.return_value = changed_files_list
# Should return None without calling subprocess due to helpers.cpp
result = get_changed_components()
assert result is None
@pytest.mark.parametrize( @pytest.mark.parametrize(
("changed_files_list", "expected"), ("changed_files_list", "expected"),
[ [
@ -451,7 +488,7 @@ def test_get_changed_components_script_failure() -> None:
@pytest.mark.parametrize( @pytest.mark.parametrize(
("components", "all_files", "expected_files"), ("components", "all_files", "expected_files"),
[ [
# Core files changed (full scan) # Core C++/header files changed (full scan)
( (
None, None,
["esphome/components/wifi/wifi.cpp", "esphome/core/helpers.cpp"], ["esphome/components/wifi/wifi.cpp", "esphome/core/helpers.cpp"],
@ -591,7 +628,7 @@ def test_filter_changed_empty_file_handling(
def test_filter_changed_ci_full_scan() -> None: def test_filter_changed_ci_full_scan() -> None:
"""Test _filter_changed_ci when core files changed (full scan).""" """Test _filter_changed_ci when core C++/header files changed (full scan)."""
all_files = ["esphome/components/wifi/wifi.cpp", "esphome/core/helpers.cpp"] all_files = ["esphome/components/wifi/wifi.cpp", "esphome/core/helpers.cpp"]
with patch("helpers.get_changed_components", return_value=None): with patch("helpers.get_changed_components", return_value=None):