[ci] Fix clang-tidy split decision to account for component dependencies (#11430)

script/determine-jobs.py

@@ -57,6 +57,7 @@ from helpers import (
     get_component_from_path,
     get_component_test_files,
     get_components_from_integration_fixtures,
+    git_ls_files,
     parse_test_filename,
     root_path,
 )
@@ -162,6 +163,26 @@ def should_run_integration_tests(branch: str | None = None) -> bool:
     return False
 
 
+@cache
+def _is_clang_tidy_full_scan() -> bool:
+    """Check if clang-tidy configuration changed (requires full scan).
+
+    Returns:
+        True if full scan is needed (hash changed), False otherwise.
+    """
+    try:
+        result = subprocess.run(
+            [os.path.join(root_path, "script", "clang_tidy_hash.py"), "--check"],
+            capture_output=True,
+            check=False,
+        )
+        # Exit 0 means hash changed (full scan needed)
+        return result.returncode == 0
+    except Exception:
+        # If hash check fails, run full scan to be safe
+        return True
+
+
 def should_run_clang_tidy(branch: str | None = None) -> bool:
     """Determine if clang-tidy should run based on changed files.
 
@@ -198,17 +219,7 @@ def should_run_clang_tidy(branch: str | None = None) -> bool:
         True if clang-tidy should run, False otherwise.
     """
     # First check if clang-tidy configuration changed (full scan needed)
-    try:
-        result = subprocess.run(
-            [os.path.join(root_path, "script", "clang_tidy_hash.py"), "--check"],
-            capture_output=True,
-            check=False,
-        )
-        # Exit 0 means hash changed (full scan needed)
-        if result.returncode == 0:
-            return True
-    except Exception:
-        # If hash check fails, run clang-tidy to be safe
+    if _is_clang_tidy_full_scan():
         return True
 
     # Check if .clang-tidy.hash file itself was changed
@@ -586,13 +597,37 @@ def main() -> None:
     # Detect components for memory impact analysis (merged config)
     memory_impact = detect_memory_impact_config(args.branch)
 
+    # Determine clang-tidy mode based on actual files that will be checked
     if run_clang_tidy:
-        if changed_cpp_file_count < CLANG_TIDY_SPLIT_THRESHOLD:
-            clang_tidy_mode = "nosplit"
-        else:
+        is_full_scan = _is_clang_tidy_full_scan()
+
+        if is_full_scan:
+            # Full scan checks all files - always use split mode for efficiency
             clang_tidy_mode = "split"
+            files_to_check_count = -1  # Sentinel value for "all files"
+        else:
+            # Targeted scan - calculate actual files that will be checked
+            # This accounts for component dependencies, not just directly changed files
+            if changed_components:
+                # Count C++ files in all changed components (including dependencies)
+                all_cpp_files = list(git_ls_files(["*.cpp"]).keys())
+                component_set = set(changed_components)
+                files_to_check_count = sum(
+                    1
+                    for f in all_cpp_files
+                    if get_component_from_path(f) in component_set
+                )
+            else:
+                # If no components changed, use the simple count of changed C++ files
+                files_to_check_count = changed_cpp_file_count
+
+            if files_to_check_count < CLANG_TIDY_SPLIT_THRESHOLD:
+                clang_tidy_mode = "nosplit"
+            else:
+                clang_tidy_mode = "split"
     else:
         clang_tidy_mode = "disabled"
+        files_to_check_count = 0
 
     # Build output
     output: dict[str, Any] = {

tests/script/test_determine_jobs.py

@@ -71,6 +71,12 @@ def mock_changed_files() -> Generator[Mock, None, None]:
         yield mock
 
 
+@pytest.fixture(autouse=True)
+def clear_clang_tidy_cache() -> None:
+    """Clear the clang-tidy full scan cache before each test."""
+    determine_jobs._is_clang_tidy_full_scan.cache_clear()
+
+
 def test_main_all_tests_should_run(
     mock_should_run_integration_tests: Mock,
     mock_should_run_clang_tidy: Mock,
@@ -98,7 +104,10 @@ def test_main_all_tests_should_run(
     mock_subprocess_run.return_value = mock_result
 
     # Run main function with mocked argv
-    with patch("sys.argv", ["determine-jobs.py"]):
+    with (
+        patch("sys.argv", ["determine-jobs.py"]),
+        patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=False),
+    ):
         determine_jobs.main()
 
     # Check output
@@ -224,7 +233,10 @@ def test_main_with_branch_argument(
     )
     mock_subprocess_run.return_value = mock_result
 
-    with patch("sys.argv", ["script.py", "-b", "main"]):
+    with (
+        patch("sys.argv", ["script.py", "-b", "main"]),
+        patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=False),
+    ):
         determine_jobs.main()
 
     # Check that functions were called with branch
@@ -363,16 +375,6 @@ def test_should_run_clang_tidy_hash_check_exception() -> None:
         result = determine_jobs.should_run_clang_tidy()
         assert result is True  # Fail safe - run clang-tidy
 
-    # Even with C++ files, exception should trigger clang-tidy
-    with (
-        patch.object(
-            determine_jobs, "changed_files", return_value=["esphome/core.cpp"]
-        ),
-        patch("subprocess.run", side_effect=Exception("Hash check failed")),
-    ):
-        result = determine_jobs.should_run_clang_tidy()
-        assert result is True
-
 
 def test_should_run_clang_tidy_with_branch() -> None:
     """Test should_run_clang_tidy with branch argument."""
@@ -763,3 +765,120 @@ def test_detect_memory_impact_config_skips_base_bus_components(tmp_path: Path) -
     assert result["should_run"] == "true"
     assert result["components"] == ["wifi"]
     assert "i2c" not in result["components"]
+
+
+# Tests for clang-tidy split mode logic
+
+
+def test_clang_tidy_mode_full_scan(
+    mock_should_run_integration_tests: Mock,
+    mock_should_run_clang_tidy: Mock,
+    mock_should_run_clang_format: Mock,
+    mock_should_run_python_linters: Mock,
+    mock_subprocess_run: Mock,
+    mock_changed_files: Mock,
+    capsys: pytest.CaptureFixture[str],
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Test that full scan (hash changed) always uses split mode."""
+    monkeypatch.delenv("GITHUB_ACTIONS", raising=False)
+
+    mock_should_run_integration_tests.return_value = False
+    mock_should_run_clang_tidy.return_value = True
+    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 full scan (hash changed)
+    with (
+        patch("sys.argv", ["determine-jobs.py"]),
+        patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=True),
+    ):
+        determine_jobs.main()
+
+    captured = capsys.readouterr()
+    output = json.loads(captured.out)
+
+    # Full scan should always use split mode
+    assert output["clang_tidy_mode"] == "split"
+
+
+@pytest.mark.parametrize(
+    ("component_count", "files_per_component", "expected_mode"),
+    [
+        # Small PR: 5 files in 1 component -> nosplit
+        (1, 5, "nosplit"),
+        # Medium PR: 30 files in 2 components -> nosplit
+        (2, 15, "nosplit"),
+        # Medium PR: 64 files total -> nosplit (just under threshold)
+        (2, 32, "nosplit"),
+        # Large PR: 65 files total -> split (at threshold)
+        (2, 33, "split"),  # 2 * 33 = 66 files
+        # Large PR: 100 files in 10 components -> split
+        (10, 10, "split"),
+    ],
+    ids=[
+        "1_comp_5_files_nosplit",
+        "2_comp_30_files_nosplit",
+        "2_comp_64_files_nosplit_under_threshold",
+        "2_comp_66_files_split_at_threshold",
+        "10_comp_100_files_split",
+    ],
+)
+def test_clang_tidy_mode_targeted_scan(
+    component_count: int,
+    files_per_component: int,
+    expected_mode: str,
+    mock_should_run_integration_tests: Mock,
+    mock_should_run_clang_tidy: Mock,
+    mock_should_run_clang_format: Mock,
+    mock_should_run_python_linters: Mock,
+    mock_subprocess_run: Mock,
+    mock_changed_files: Mock,
+    capsys: pytest.CaptureFixture[str],
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Test clang-tidy mode selection based on files_to_check count."""
+    monkeypatch.delenv("GITHUB_ACTIONS", raising=False)
+
+    mock_should_run_integration_tests.return_value = False
+    mock_should_run_clang_tidy.return_value = True
+    mock_should_run_clang_format.return_value = False
+    mock_should_run_python_linters.return_value = False
+
+    # 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 git_ls_files to return files for each component
+    cpp_files = {
+        f"esphome/components/{comp}/file{i}.cpp": 0
+        for comp in components
+        for i in range(files_per_component)
+    }
+
+    # Create a mock that returns the cpp_files dict for any call
+    def mock_git_ls_files(patterns=None):
+        return cpp_files
+
+    with (
+        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),
+    ):
+        determine_jobs.main()
+
+    captured = capsys.readouterr()
+    output = json.loads(captured.out)
+
+    assert output["clang_tidy_mode"] == expected_mode