1
0
mirror of https://github.com/esphome/esphome.git synced 2025-10-23 04:03:52 +01:00

[ci] Fix clang-tidy split mode for core file changes (#11434)

This commit is contained in:
J. Nick Koston
2025-10-20 20:39:50 -10:00
committed by GitHub
parent 0b2f5fcd7e
commit 0ae9009e41
4 changed files with 358 additions and 236 deletions

View File

@@ -43,7 +43,6 @@ from enum import StrEnum
from functools import cache
import json
import os
from pathlib import Path
import subprocess
import sys
from typing import Any
@@ -53,10 +52,13 @@ from helpers import (
CPP_FILE_EXTENSIONS,
PYTHON_FILE_EXTENSIONS,
changed_files,
filter_component_files,
get_all_dependencies,
get_changed_components,
get_component_from_path,
get_component_test_files,
get_components_from_integration_fixtures,
get_components_with_dependencies,
git_ls_files,
parse_test_filename,
root_path,
@@ -561,16 +563,29 @@ def main() -> None:
run_python_linters = should_run_python_linters(args.branch)
changed_cpp_file_count = count_changed_cpp_files(args.branch)
# Get both directly changed and all changed components (with dependencies) in one call
script_path = Path(__file__).parent / "list-components.py"
cmd = [sys.executable, str(script_path), "--changed-with-deps"]
if args.branch:
cmd.extend(["-b", args.branch])
# Get changed components
# get_changed_components() returns:
# None: Core files changed (need full scan)
# []: No components changed
# [list]: Changed components (already includes dependencies)
changed_components_result = get_changed_components()
result = subprocess.run(cmd, capture_output=True, text=True, check=True)
component_data = json.loads(result.stdout)
directly_changed_components = component_data["directly_changed"]
changed_components = component_data["all_changed"]
if changed_components_result is None:
# Core files changed - will trigger full clang-tidy scan
# No specific components to test
changed_components = []
directly_changed_components = []
is_core_change = True
else:
# Get both directly changed and all changed (with dependencies)
changed = changed_files(args.branch)
component_files = [f for f in changed if filter_component_files(f)]
directly_changed_components = get_components_with_dependencies(
component_files, False
)
changed_components = get_components_with_dependencies(component_files, True)
is_core_change = False
# Filter to only components that have test files
# Components without tests shouldn't generate CI test jobs
@@ -581,11 +596,11 @@ def main() -> None:
# Get directly changed components with tests (for isolated testing)
# These will be tested WITHOUT --testing-mode in CI to enable full validation
# (pin conflicts, etc.) since they contain the actual changes being reviewed
directly_changed_with_tests = [
directly_changed_with_tests = {
component
for component in directly_changed_components
if _component_has_tests(component)
]
}
# Get dependency-only components (for grouped testing)
dependency_only_components = [
@@ -599,7 +614,8 @@ def main() -> None:
# Determine clang-tidy mode based on actual files that will be checked
if run_clang_tidy:
is_full_scan = _is_clang_tidy_full_scan()
# Full scan needed if: hash changed OR core files changed
is_full_scan = _is_clang_tidy_full_scan() or is_core_change
if is_full_scan:
# Full scan checks all files - always use split mode for efficiency
@@ -638,7 +654,7 @@ def main() -> None:
"python_linters": run_python_linters,
"changed_components": changed_components,
"changed_components_with_tests": changed_components_with_tests,
"directly_changed_components_with_tests": directly_changed_with_tests,
"directly_changed_components_with_tests": list(directly_changed_with_tests),
"dependency_only_components_with_tests": dependency_only_components,
"component_test_count": len(changed_components_with_tests),
"directly_changed_count": len(directly_changed_with_tests),

View File

@@ -1,5 +1,6 @@
from __future__ import annotations
from collections.abc import Callable
from functools import cache
import json
import os
@@ -7,6 +8,7 @@ import os.path
from pathlib import Path
import re
import subprocess
import sys
import time
from typing import Any
@@ -304,7 +306,10 @@ def get_changed_components() -> list[str] | None:
for f in changed
)
if core_cpp_changed:
print("Core C++/header files changed - will run full clang-tidy scan")
print(
"Core C++/header files changed - will run full clang-tidy scan",
file=sys.stderr,
)
return None
# Use list-components.py to get changed components
@@ -318,7 +323,10 @@ def get_changed_components() -> list[str] | None:
return parse_list_components_output(result.stdout)
except subprocess.CalledProcessError:
# If the script fails, fall back to full scan
print("Could not determine changed components - will run full clang-tidy scan")
print(
"Could not determine changed components - will run full clang-tidy scan",
file=sys.stderr,
)
return None
@@ -370,14 +378,14 @@ def _filter_changed_ci(files: list[str]) -> list[str]:
if f in changed and not f.startswith(ESPHOME_COMPONENTS_PATH)
]
if not files:
print("No files changed")
print("No files changed", file=sys.stderr)
return files
# Scenario 3: Specific components changed
# Action: Check ALL files in each changed component
# Convert component list to set for O(1) lookups
component_set = set(components)
print(f"Changed components: {', '.join(sorted(components))}")
print(f"Changed components: {', '.join(sorted(components))}", file=sys.stderr)
# The 'files' parameter contains ALL files in the codebase that clang-tidy would check.
# We filter this down to only files in the changed components.
@@ -648,3 +656,220 @@ def get_components_from_integration_fixtures() -> set[str]:
components.add(item["platform"])
return components
def filter_component_files(file_path: str) -> bool:
"""Check if a file path is a component file.
Args:
file_path: Path to check
Returns:
True if the file is in a component directory
"""
return file_path.startswith("esphome/components/") or file_path.startswith(
"tests/components/"
)
def extract_component_names_from_files(files: list[str]) -> list[str]:
"""Extract unique component names from a list of file paths.
Args:
files: List of file paths
Returns:
List of unique component names (preserves order)
"""
return list(
dict.fromkeys(comp for file in files if (comp := get_component_from_path(file)))
)
def add_item_to_components_graph(
components_graph: dict[str, list[str]], parent: str, child: str
) -> None:
"""Add a dependency relationship to the components graph.
Args:
components_graph: Graph mapping parent components to their children
parent: Parent component name
child: Child component name (dependent)
"""
if not parent.startswith("__") and parent != child:
if parent not in components_graph:
components_graph[parent] = []
if child not in components_graph[parent]:
components_graph[parent].append(child)
def resolve_auto_load(
auto_load: list[str] | Callable[[], list[str]] | Callable[[dict | None], list[str]],
config: dict | None = None,
) -> list[str]:
"""Resolve AUTO_LOAD to a list, handling callables with or without config parameter.
Args:
auto_load: The AUTO_LOAD value (list or callable)
config: Optional config to pass to callable AUTO_LOAD functions
Returns:
List of component names to auto-load
"""
if not callable(auto_load):
return auto_load
import inspect
if inspect.signature(auto_load).parameters:
return auto_load(config)
return auto_load()
def create_components_graph() -> dict[str, list[str]]:
"""Create a graph of component dependencies.
Returns:
Dictionary mapping parent components to their children (dependencies)
"""
from pathlib import Path
from esphome import const
from esphome.core import CORE
from esphome.loader import ComponentManifest, get_component, get_platform
# The root directory of the repo
root = Path(__file__).parent.parent
components_dir = root / "esphome" / "components"
# Fake some directory so that get_component works
CORE.config_path = root
# Various configuration to capture different outcomes used by `AUTO_LOAD` function.
KEY_CORE = const.KEY_CORE
KEY_TARGET_FRAMEWORK = const.KEY_TARGET_FRAMEWORK
KEY_TARGET_PLATFORM = const.KEY_TARGET_PLATFORM
PLATFORM_ESP32 = const.PLATFORM_ESP32
PLATFORM_ESP8266 = const.PLATFORM_ESP8266
TARGET_CONFIGURATIONS = [
{KEY_TARGET_FRAMEWORK: None, KEY_TARGET_PLATFORM: None},
{KEY_TARGET_FRAMEWORK: "arduino", KEY_TARGET_PLATFORM: None},
{KEY_TARGET_FRAMEWORK: "esp-idf", KEY_TARGET_PLATFORM: None},
{KEY_TARGET_FRAMEWORK: None, KEY_TARGET_PLATFORM: PLATFORM_ESP32},
{KEY_TARGET_FRAMEWORK: None, KEY_TARGET_PLATFORM: PLATFORM_ESP8266},
]
CORE.data[KEY_CORE] = TARGET_CONFIGURATIONS[0]
components_graph = {}
platforms = []
components: list[tuple[ComponentManifest, str, Path]] = []
for path in components_dir.iterdir():
if not path.is_dir():
continue
if not (path / "__init__.py").is_file():
continue
name = path.name
comp = get_component(name)
if comp is None:
raise RuntimeError(
f"Cannot find component {name}. Make sure current path is pip installed ESPHome"
)
components.append((comp, name, path))
if comp.is_platform_component:
platforms.append(name)
platforms = set(platforms)
for comp, name, path in components:
for dependency in comp.dependencies:
add_item_to_components_graph(
components_graph, dependency.split(".")[0], name
)
for target_config in TARGET_CONFIGURATIONS:
CORE.data[KEY_CORE] = target_config
for item in resolve_auto_load(comp.auto_load, config=None):
add_item_to_components_graph(components_graph, item, name)
# restore config
CORE.data[KEY_CORE] = TARGET_CONFIGURATIONS[0]
for platform_path in path.iterdir():
platform_name = platform_path.stem
if platform_name == name or platform_name not in platforms:
continue
platform = get_platform(platform_name, name)
if platform is None:
continue
add_item_to_components_graph(components_graph, platform_name, name)
for dependency in platform.dependencies:
add_item_to_components_graph(
components_graph, dependency.split(".")[0], name
)
for target_config in TARGET_CONFIGURATIONS:
CORE.data[KEY_CORE] = target_config
for item in resolve_auto_load(platform.auto_load, config={}):
add_item_to_components_graph(components_graph, item, name)
# restore config
CORE.data[KEY_CORE] = TARGET_CONFIGURATIONS[0]
return components_graph
def find_children_of_component(
components_graph: dict[str, list[str]], component_name: str, depth: int = 0
) -> list[str]:
"""Find all components that depend on the given component (recursively).
Args:
components_graph: Graph mapping parent components to their children
component_name: Component name to find children for
depth: Current recursion depth (max 10)
Returns:
List of all dependent component names (may contain duplicates removed at end)
"""
if component_name not in components_graph:
return []
children = []
for child in components_graph[component_name]:
children.append(child)
if depth < 10:
children.extend(
find_children_of_component(components_graph, child, depth + 1)
)
# Remove duplicate values
return list(set(children))
def get_components_with_dependencies(
files: list[str], get_dependencies: bool = False
) -> list[str]:
"""Get component names from files, optionally including their dependencies.
Args:
files: List of file paths
get_dependencies: If True, include all dependent components
Returns:
Sorted list of component names
"""
components = extract_component_names_from_files(files)
if get_dependencies:
components_graph = create_components_graph()
all_components = components.copy()
for c in components:
all_components.extend(find_children_of_component(components_graph, c))
# Remove duplicate values
all_changed_components = list(set(all_components))
return sorted(all_changed_components)
return sorted(components)

View File

@@ -1,24 +1,12 @@
#!/usr/bin/env python3
import argparse
from collections.abc import Callable
from pathlib import Path
import sys
from helpers import changed_files, get_component_from_path, git_ls_files
from esphome.const import (
KEY_CORE,
KEY_TARGET_FRAMEWORK,
KEY_TARGET_PLATFORM,
PLATFORM_ESP32,
PLATFORM_ESP8266,
from helpers import (
changed_files,
filter_component_files,
get_components_with_dependencies,
git_ls_files,
)
from esphome.core import CORE
from esphome.loader import ComponentManifest, get_component, get_platform
def filter_component_files(str):
return str.startswith("esphome/components/") | str.startswith("tests/components/")
def get_all_component_files() -> list[str]:
@@ -27,156 +15,6 @@ def get_all_component_files() -> list[str]:
return list(filter(filter_component_files, files))
def extract_component_names_array_from_files_array(files):
components = []
for file in files:
component_name = get_component_from_path(file)
if component_name and component_name not in components:
components.append(component_name)
return components
def add_item_to_components_graph(components_graph, parent, child):
if not parent.startswith("__") and parent != child:
if parent not in components_graph:
components_graph[parent] = []
if child not in components_graph[parent]:
components_graph[parent].append(child)
def resolve_auto_load(
auto_load: list[str] | Callable[[], list[str]] | Callable[[dict | None], list[str]],
config: dict | None = None,
) -> list[str]:
"""Resolve AUTO_LOAD to a list, handling callables with or without config parameter.
Args:
auto_load: The AUTO_LOAD value (list or callable)
config: Optional config to pass to callable AUTO_LOAD functions
Returns:
List of component names to auto-load
"""
if not callable(auto_load):
return auto_load
import inspect
if inspect.signature(auto_load).parameters:
return auto_load(config)
return auto_load()
def create_components_graph():
# The root directory of the repo
root = Path(__file__).parent.parent
components_dir = root / "esphome" / "components"
# Fake some directory so that get_component works
CORE.config_path = root
# Various configuration to capture different outcomes used by `AUTO_LOAD` function.
TARGET_CONFIGURATIONS = [
{KEY_TARGET_FRAMEWORK: None, KEY_TARGET_PLATFORM: None},
{KEY_TARGET_FRAMEWORK: "arduino", KEY_TARGET_PLATFORM: None},
{KEY_TARGET_FRAMEWORK: "esp-idf", KEY_TARGET_PLATFORM: None},
{KEY_TARGET_FRAMEWORK: None, KEY_TARGET_PLATFORM: PLATFORM_ESP32},
{KEY_TARGET_FRAMEWORK: None, KEY_TARGET_PLATFORM: PLATFORM_ESP8266},
]
CORE.data[KEY_CORE] = TARGET_CONFIGURATIONS[0]
components_graph = {}
platforms = []
components: list[tuple[ComponentManifest, str, Path]] = []
for path in components_dir.iterdir():
if not path.is_dir():
continue
if not (path / "__init__.py").is_file():
continue
name = path.name
comp = get_component(name)
if comp is None:
print(
f"Cannot find component {name}. Make sure current path is pip installed ESPHome"
)
sys.exit(1)
components.append((comp, name, path))
if comp.is_platform_component:
platforms.append(name)
platforms = set(platforms)
for comp, name, path in components:
for dependency in comp.dependencies:
add_item_to_components_graph(
components_graph, dependency.split(".")[0], name
)
for target_config in TARGET_CONFIGURATIONS:
CORE.data[KEY_CORE] = target_config
for item in resolve_auto_load(comp.auto_load, config=None):
add_item_to_components_graph(components_graph, item, name)
# restore config
CORE.data[KEY_CORE] = TARGET_CONFIGURATIONS[0]
for platform_path in path.iterdir():
platform_name = platform_path.stem
if platform_name == name or platform_name not in platforms:
continue
platform = get_platform(platform_name, name)
if platform is None:
continue
add_item_to_components_graph(components_graph, platform_name, name)
for dependency in platform.dependencies:
add_item_to_components_graph(
components_graph, dependency.split(".")[0], name
)
for target_config in TARGET_CONFIGURATIONS:
CORE.data[KEY_CORE] = target_config
for item in resolve_auto_load(platform.auto_load, config={}):
add_item_to_components_graph(components_graph, item, name)
# restore config
CORE.data[KEY_CORE] = TARGET_CONFIGURATIONS[0]
return components_graph
def find_children_of_component(components_graph, component_name, depth=0):
if component_name not in components_graph:
return []
children = []
for child in components_graph[component_name]:
children.append(child)
if depth < 10:
children.extend(
find_children_of_component(components_graph, child, depth + 1)
)
# Remove duplicate values
return list(set(children))
def get_components(files: list[str], get_dependencies: bool = False):
components = extract_component_names_array_from_files_array(files)
if get_dependencies:
components_graph = create_components_graph()
all_components = components.copy()
for c in components:
all_components.extend(find_children_of_component(components_graph, c))
# Remove duplicate values
all_changed_components = list(set(all_components))
return sorted(all_changed_components)
return sorted(components)
def main():
parser = argparse.ArgumentParser()
parser.add_argument(
@@ -251,8 +89,8 @@ def main():
# Return JSON with both directly changed and all changed components
import json
directly_changed = get_components(files, False)
all_changed = get_components(files, True)
directly_changed = get_components_with_dependencies(files, False)
all_changed = get_components_with_dependencies(files, True)
output = {
"directly_changed": directly_changed,
"all_changed": all_changed,
@@ -260,11 +98,11 @@ def main():
print(json.dumps(output))
elif args.changed_direct:
# Return only directly changed components (without dependencies)
for c in get_components(files, False):
for c in get_components_with_dependencies(files, False):
print(c)
else:
# Return all changed components (with dependencies) - default behavior
for c in get_components(files, args.changed):
for c in get_components_with_dependencies(files, args.changed):
print(c)

View File

@@ -96,17 +96,34 @@ def test_main_all_tests_should_run(
mock_should_run_clang_format.return_value = True
mock_should_run_python_linters.return_value = True
# Mock list-components.py output (now returns JSON with --changed-with-deps)
mock_result = Mock()
mock_result.stdout = json.dumps(
{"directly_changed": ["wifi", "api"], "all_changed": ["wifi", "api", "sensor"]}
)
mock_subprocess_run.return_value = mock_result
# Mock changed_files to return non-component files (to avoid memory impact)
# Memory impact only runs when component C++ files change
mock_changed_files.return_value = [
"esphome/config.py",
"esphome/helpers.py",
]
# Run main function with mocked argv
with (
patch("sys.argv", ["determine-jobs.py"]),
patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=False),
patch.object(
determine_jobs,
"get_changed_components",
return_value=["wifi", "api", "sensor"],
),
patch.object(
determine_jobs,
"filter_component_files",
side_effect=lambda f: f.startswith("esphome/components/"),
),
patch.object(
determine_jobs,
"get_components_with_dependencies",
side_effect=lambda files, deps: ["wifi", "api"]
if not deps
else ["wifi", "api", "sensor"],
),
):
determine_jobs.main()
@@ -130,9 +147,9 @@ def test_main_all_tests_should_run(
# changed_cpp_file_count should be present
assert "changed_cpp_file_count" in output
assert isinstance(output["changed_cpp_file_count"], int)
# memory_impact should be present
# memory_impact should be false (no component C++ files changed)
assert "memory_impact" in output
assert output["memory_impact"]["should_run"] == "false" # No files changed
assert output["memory_impact"]["should_run"] == "false"
def test_main_no_tests_should_run(
@@ -154,13 +171,18 @@ def test_main_no_tests_should_run(
mock_should_run_clang_format.return_value = False
mock_should_run_python_linters.return_value = False
# Mock empty list-components.py output
mock_result = Mock()
mock_result.stdout = json.dumps({"directly_changed": [], "all_changed": []})
mock_subprocess_run.return_value = mock_result
# Mock changed_files to return no component files
mock_changed_files.return_value = []
# Run main function with mocked argv
with patch("sys.argv", ["determine-jobs.py"]):
with (
patch("sys.argv", ["determine-jobs.py"]),
patch.object(determine_jobs, "get_changed_components", return_value=[]),
patch.object(determine_jobs, "filter_component_files", return_value=False),
patch.object(
determine_jobs, "get_components_with_dependencies", return_value=[]
),
):
determine_jobs.main()
# Check output
@@ -226,16 +248,22 @@ def test_main_with_branch_argument(
mock_should_run_clang_format.return_value = False
mock_should_run_python_linters.return_value = True
# Mock list-components.py output
mock_result = Mock()
mock_result.stdout = json.dumps(
{"directly_changed": ["mqtt"], "all_changed": ["mqtt"]}
)
mock_subprocess_run.return_value = mock_result
# Mock changed_files to return non-component files (to avoid memory impact)
# Memory impact only runs when component C++ files change
mock_changed_files.return_value = ["esphome/config.py"]
with (
patch("sys.argv", ["script.py", "-b", "main"]),
patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=False),
patch.object(determine_jobs, "get_changed_components", return_value=["mqtt"]),
patch.object(
determine_jobs,
"filter_component_files",
side_effect=lambda f: f.startswith("esphome/components/"),
),
patch.object(
determine_jobs, "get_components_with_dependencies", return_value=["mqtt"]
),
):
determine_jobs.main()
@@ -245,13 +273,6 @@ def test_main_with_branch_argument(
mock_should_run_clang_format.assert_called_once_with("main")
mock_should_run_python_linters.assert_called_once_with("main")
# Check that list-components.py was called with branch
mock_subprocess_run.assert_called_once()
call_args = mock_subprocess_run.call_args[0][0]
assert "--changed-with-deps" in call_args
assert "-b" in call_args
assert "main" in call_args
# Check output
captured = capsys.readouterr()
output = json.loads(captured.out)
@@ -272,7 +293,7 @@ def test_main_with_branch_argument(
# changed_cpp_file_count should be present
assert "changed_cpp_file_count" in output
assert isinstance(output["changed_cpp_file_count"], int)
# memory_impact should be present
# memory_impact should be false (no component C++ files changed)
assert "memory_impact" in output
assert output["memory_impact"]["should_run"] == "false"
@@ -500,16 +521,11 @@ def test_main_filters_components_without_tests(
mock_should_run_clang_format.return_value = False
mock_should_run_python_linters.return_value = False
# Mock list-components.py output with 3 components
# wifi: has tests, sensor: has tests, airthings_ble: no tests
mock_result = Mock()
mock_result.stdout = json.dumps(
{
"directly_changed": ["wifi", "sensor"],
"all_changed": ["wifi", "sensor", "airthings_ble"],
}
)
mock_subprocess_run.return_value = mock_result
# Mock changed_files to return component files
mock_changed_files.return_value = [
"esphome/components/wifi/wifi.cpp",
"esphome/components/sensor/sensor.h",
]
# Create test directory structure
tests_dir = tmp_path / "tests" / "components"
@@ -533,6 +549,23 @@ def test_main_filters_components_without_tests(
patch.object(determine_jobs, "root_path", str(tmp_path)),
patch.object(helpers, "root_path", str(tmp_path)),
patch("sys.argv", ["determine-jobs.py"]),
patch.object(
determine_jobs,
"get_changed_components",
return_value=["wifi", "sensor", "airthings_ble"],
),
patch.object(
determine_jobs,
"filter_component_files",
side_effect=lambda f: f.startswith("esphome/components/"),
),
patch.object(
determine_jobs,
"get_components_with_dependencies",
side_effect=lambda files, deps: ["wifi", "sensor"]
if not deps
else ["wifi", "sensor", "airthings_ble"],
),
):
# Clear the cache since we're mocking root_path
determine_jobs._component_has_tests.cache_clear()
@@ -788,15 +821,18 @@ def test_clang_tidy_mode_full_scan(
mock_should_run_clang_format.return_value = False
mock_should_run_python_linters.return_value = False
# Mock list-components.py output
mock_result = Mock()
mock_result.stdout = json.dumps({"directly_changed": [], "all_changed": []})
mock_subprocess_run.return_value = mock_result
# Mock changed_files to return no component files
mock_changed_files.return_value = []
# Mock full scan (hash changed)
with (
patch("sys.argv", ["determine-jobs.py"]),
patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=True),
patch.object(determine_jobs, "get_changed_components", return_value=[]),
patch.object(determine_jobs, "filter_component_files", return_value=False),
patch.object(
determine_jobs, "get_components_with_dependencies", return_value=[]
),
):
determine_jobs.main()
@@ -853,12 +889,10 @@ def test_clang_tidy_mode_targeted_scan(
# Create component names
components = [f"comp{i}" for i in range(component_count)]
# Mock list-components.py output
mock_result = Mock()
mock_result.stdout = json.dumps(
{"directly_changed": components, "all_changed": components}
)
mock_subprocess_run.return_value = mock_result
# Mock changed_files to return component files
mock_changed_files.return_value = [
f"esphome/components/{comp}/file.cpp" for comp in components
]
# Mock git_ls_files to return files for each component
cpp_files = {
@@ -875,6 +909,15 @@ def test_clang_tidy_mode_targeted_scan(
patch("sys.argv", ["determine-jobs.py"]),
patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=False),
patch.object(determine_jobs, "git_ls_files", side_effect=mock_git_ls_files),
patch.object(determine_jobs, "get_changed_components", return_value=components),
patch.object(
determine_jobs,
"filter_component_files",
side_effect=lambda f: f.startswith("esphome/components/"),
),
patch.object(
determine_jobs, "get_components_with_dependencies", return_value=components
),
):
determine_jobs.main()