mirror of
https://github.com/esphome/esphome.git
synced 2025-10-23 20:23:50 +01:00
[ci] Optimize clang-tidy for small PRs by avoiding unnecessary job spitting (#11402)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
186
.github/workflows/ci.yml
vendored
186
.github/workflows/ci.yml
vendored
@@ -170,11 +170,13 @@ jobs:
|
||||
outputs:
|
||||
integration-tests: ${{ steps.determine.outputs.integration-tests }}
|
||||
clang-tidy: ${{ steps.determine.outputs.clang-tidy }}
|
||||
clang-tidy-mode: ${{ steps.determine.outputs.clang-tidy-mode }}
|
||||
python-linters: ${{ steps.determine.outputs.python-linters }}
|
||||
changed-components: ${{ steps.determine.outputs.changed-components }}
|
||||
changed-components-with-tests: ${{ steps.determine.outputs.changed-components-with-tests }}
|
||||
directly-changed-components-with-tests: ${{ steps.determine.outputs.directly-changed-components-with-tests }}
|
||||
component-test-count: ${{ steps.determine.outputs.component-test-count }}
|
||||
changed-cpp-file-count: ${{ steps.determine.outputs.changed-cpp-file-count }}
|
||||
memory_impact: ${{ steps.determine.outputs.memory-impact }}
|
||||
steps:
|
||||
- name: Check out code from GitHub
|
||||
@@ -200,11 +202,13 @@ jobs:
|
||||
# Extract individual fields
|
||||
echo "integration-tests=$(echo "$output" | jq -r '.integration_tests')" >> $GITHUB_OUTPUT
|
||||
echo "clang-tidy=$(echo "$output" | jq -r '.clang_tidy')" >> $GITHUB_OUTPUT
|
||||
echo "clang-tidy-mode=$(echo "$output" | jq -r '.clang_tidy_mode')" >> $GITHUB_OUTPUT
|
||||
echo "python-linters=$(echo "$output" | jq -r '.python_linters')" >> $GITHUB_OUTPUT
|
||||
echo "changed-components=$(echo "$output" | jq -c '.changed_components')" >> $GITHUB_OUTPUT
|
||||
echo "changed-components-with-tests=$(echo "$output" | jq -c '.changed_components_with_tests')" >> $GITHUB_OUTPUT
|
||||
echo "directly-changed-components-with-tests=$(echo "$output" | jq -c '.directly_changed_components_with_tests')" >> $GITHUB_OUTPUT
|
||||
echo "component-test-count=$(echo "$output" | jq -r '.component_test_count')" >> $GITHUB_OUTPUT
|
||||
echo "changed-cpp-file-count=$(echo "$output" | jq -r '.changed_cpp_file_count')" >> $GITHUB_OUTPUT
|
||||
echo "memory-impact=$(echo "$output" | jq -c '.memory_impact')" >> $GITHUB_OUTPUT
|
||||
|
||||
integration-tests:
|
||||
@@ -243,7 +247,7 @@ jobs:
|
||||
. venv/bin/activate
|
||||
pytest -vv --no-cov --tb=native -n auto tests/integration/
|
||||
|
||||
clang-tidy:
|
||||
clang-tidy-single:
|
||||
name: ${{ matrix.name }}
|
||||
runs-on: ubuntu-24.04
|
||||
needs:
|
||||
@@ -261,22 +265,6 @@ jobs:
|
||||
name: Run script/clang-tidy for ESP8266
|
||||
options: --environment esp8266-arduino-tidy --grep USE_ESP8266
|
||||
pio_cache_key: tidyesp8266
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 Arduino 1/4
|
||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 1
|
||||
pio_cache_key: tidyesp32
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 Arduino 2/4
|
||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 2
|
||||
pio_cache_key: tidyesp32
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 Arduino 3/4
|
||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 3
|
||||
pio_cache_key: tidyesp32
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 Arduino 4/4
|
||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 4
|
||||
pio_cache_key: tidyesp32
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 IDF
|
||||
options: --environment esp32-idf-tidy --grep USE_ESP_IDF
|
||||
@@ -357,6 +345,166 @@ jobs:
|
||||
# yamllint disable-line rule:line-length
|
||||
if: always()
|
||||
|
||||
clang-tidy-nosplit:
|
||||
name: Run script/clang-tidy for ESP32 Arduino
|
||||
runs-on: ubuntu-24.04
|
||||
needs:
|
||||
- common
|
||||
- determine-jobs
|
||||
if: needs.determine-jobs.outputs.clang-tidy-mode == 'nosplit'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
steps:
|
||||
- name: Check out code from GitHub
|
||||
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
|
||||
with:
|
||||
# Need history for HEAD~1 to work for checking changed files
|
||||
fetch-depth: 2
|
||||
|
||||
- name: Restore Python
|
||||
uses: ./.github/actions/restore-python
|
||||
with:
|
||||
python-version: ${{ env.DEFAULT_PYTHON }}
|
||||
cache-key: ${{ needs.common.outputs.cache-key }}
|
||||
|
||||
- name: Cache platformio
|
||||
if: github.ref == 'refs/heads/dev'
|
||||
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
|
||||
with:
|
||||
path: ~/.platformio
|
||||
key: platformio-tidyesp32-${{ hashFiles('platformio.ini') }}
|
||||
|
||||
- name: Cache platformio
|
||||
if: github.ref != 'refs/heads/dev'
|
||||
uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
|
||||
with:
|
||||
path: ~/.platformio
|
||||
key: platformio-tidyesp32-${{ hashFiles('platformio.ini') }}
|
||||
|
||||
- name: Register problem matchers
|
||||
run: |
|
||||
echo "::add-matcher::.github/workflows/matchers/gcc.json"
|
||||
echo "::add-matcher::.github/workflows/matchers/clang-tidy.json"
|
||||
|
||||
- name: Check if full clang-tidy scan needed
|
||||
id: check_full_scan
|
||||
run: |
|
||||
. venv/bin/activate
|
||||
if python script/clang_tidy_hash.py --check; then
|
||||
echo "full_scan=true" >> $GITHUB_OUTPUT
|
||||
echo "reason=hash_changed" >> $GITHUB_OUTPUT
|
||||
else
|
||||
echo "full_scan=false" >> $GITHUB_OUTPUT
|
||||
echo "reason=normal" >> $GITHUB_OUTPUT
|
||||
fi
|
||||
|
||||
- name: Run clang-tidy
|
||||
run: |
|
||||
. venv/bin/activate
|
||||
if [ "${{ steps.check_full_scan.outputs.full_scan }}" = "true" ]; then
|
||||
echo "Running FULL clang-tidy scan (hash changed)"
|
||||
script/clang-tidy --all-headers --fix --environment esp32-arduino-tidy
|
||||
else
|
||||
echo "Running clang-tidy on changed files only"
|
||||
script/clang-tidy --all-headers --fix --changed --environment esp32-arduino-tidy
|
||||
fi
|
||||
env:
|
||||
# Also cache libdeps, store them in a ~/.platformio subfolder
|
||||
PLATFORMIO_LIBDEPS_DIR: ~/.platformio/libdeps
|
||||
|
||||
- name: Suggested changes
|
||||
run: script/ci-suggest-changes
|
||||
if: always()
|
||||
|
||||
clang-tidy-split:
|
||||
name: ${{ matrix.name }}
|
||||
runs-on: ubuntu-24.04
|
||||
needs:
|
||||
- common
|
||||
- determine-jobs
|
||||
if: needs.determine-jobs.outputs.clang-tidy-mode == 'split'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
strategy:
|
||||
fail-fast: false
|
||||
max-parallel: 1
|
||||
matrix:
|
||||
include:
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 Arduino 1/4
|
||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 1
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 Arduino 2/4
|
||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 2
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 Arduino 3/4
|
||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 3
|
||||
- id: clang-tidy
|
||||
name: Run script/clang-tidy for ESP32 Arduino 4/4
|
||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 4
|
||||
|
||||
steps:
|
||||
- name: Check out code from GitHub
|
||||
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
|
||||
with:
|
||||
# Need history for HEAD~1 to work for checking changed files
|
||||
fetch-depth: 2
|
||||
|
||||
- name: Restore Python
|
||||
uses: ./.github/actions/restore-python
|
||||
with:
|
||||
python-version: ${{ env.DEFAULT_PYTHON }}
|
||||
cache-key: ${{ needs.common.outputs.cache-key }}
|
||||
|
||||
- name: Cache platformio
|
||||
if: github.ref == 'refs/heads/dev'
|
||||
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
|
||||
with:
|
||||
path: ~/.platformio
|
||||
key: platformio-tidyesp32-${{ hashFiles('platformio.ini') }}
|
||||
|
||||
- name: Cache platformio
|
||||
if: github.ref != 'refs/heads/dev'
|
||||
uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
|
||||
with:
|
||||
path: ~/.platformio
|
||||
key: platformio-tidyesp32-${{ hashFiles('platformio.ini') }}
|
||||
|
||||
- name: Register problem matchers
|
||||
run: |
|
||||
echo "::add-matcher::.github/workflows/matchers/gcc.json"
|
||||
echo "::add-matcher::.github/workflows/matchers/clang-tidy.json"
|
||||
|
||||
- name: Check if full clang-tidy scan needed
|
||||
id: check_full_scan
|
||||
run: |
|
||||
. venv/bin/activate
|
||||
if python script/clang_tidy_hash.py --check; then
|
||||
echo "full_scan=true" >> $GITHUB_OUTPUT
|
||||
echo "reason=hash_changed" >> $GITHUB_OUTPUT
|
||||
else
|
||||
echo "full_scan=false" >> $GITHUB_OUTPUT
|
||||
echo "reason=normal" >> $GITHUB_OUTPUT
|
||||
fi
|
||||
|
||||
- name: Run clang-tidy
|
||||
run: |
|
||||
. venv/bin/activate
|
||||
if [ "${{ steps.check_full_scan.outputs.full_scan }}" = "true" ]; then
|
||||
echo "Running FULL clang-tidy scan (hash changed)"
|
||||
script/clang-tidy --all-headers --fix ${{ matrix.options }}
|
||||
else
|
||||
echo "Running clang-tidy on changed files only"
|
||||
script/clang-tidy --all-headers --fix --changed ${{ matrix.options }}
|
||||
fi
|
||||
env:
|
||||
# Also cache libdeps, store them in a ~/.platformio subfolder
|
||||
PLATFORMIO_LIBDEPS_DIR: ~/.platformio/libdeps
|
||||
|
||||
- name: Suggested changes
|
||||
run: script/ci-suggest-changes
|
||||
if: always()
|
||||
|
||||
test-build-components-splitter:
|
||||
name: Split components for intelligent grouping (40 weighted per batch)
|
||||
runs-on: ubuntu-24.04
|
||||
@@ -797,7 +945,9 @@ jobs:
|
||||
- pylint
|
||||
- pytest
|
||||
- integration-tests
|
||||
- clang-tidy
|
||||
- clang-tidy-single
|
||||
- clang-tidy-nosplit
|
||||
- clang-tidy-split
|
||||
- determine-jobs
|
||||
- test-build-components-splitter
|
||||
- test-build-components-split
|
||||
|
@@ -61,6 +61,11 @@ from helpers import (
|
||||
root_path,
|
||||
)
|
||||
|
||||
# Threshold for splitting clang-tidy jobs
|
||||
# For small PRs (< 65 files), use nosplit for faster CI
|
||||
# For large PRs (>= 65 files), use split for better parallelization
|
||||
CLANG_TIDY_SPLIT_THRESHOLD = 65
|
||||
|
||||
|
||||
class Platform(StrEnum):
|
||||
"""Platform identifiers for memory impact analysis."""
|
||||
@@ -210,6 +215,22 @@ def should_run_clang_tidy(branch: str | None = None) -> bool:
|
||||
return _any_changed_file_endswith(branch, CPP_FILE_EXTENSIONS)
|
||||
|
||||
|
||||
def count_changed_cpp_files(branch: str | None = None) -> int:
|
||||
"""Count the number of changed C++ files.
|
||||
|
||||
This is used to determine whether to split clang-tidy jobs or run them as a single job.
|
||||
For PRs with < 65 changed C++ files, running a single job is faster than splitting.
|
||||
|
||||
Args:
|
||||
branch: Branch to compare against. If None, uses default.
|
||||
|
||||
Returns:
|
||||
Number of changed C++ files.
|
||||
"""
|
||||
files = changed_files(branch)
|
||||
return sum(1 for file in files if file.endswith(CPP_FILE_EXTENSIONS))
|
||||
|
||||
|
||||
def should_run_clang_format(branch: str | None = None) -> bool:
|
||||
"""Determine if clang-format should run based on changed files.
|
||||
|
||||
@@ -412,6 +433,7 @@ def main() -> None:
|
||||
run_clang_tidy = should_run_clang_tidy(args.branch)
|
||||
run_clang_format = should_run_clang_format(args.branch)
|
||||
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"
|
||||
@@ -449,10 +471,19 @@ def main() -> None:
|
||||
# Detect components for memory impact analysis (merged config)
|
||||
memory_impact = detect_memory_impact_config(args.branch)
|
||||
|
||||
if run_clang_tidy:
|
||||
if changed_cpp_file_count < CLANG_TIDY_SPLIT_THRESHOLD:
|
||||
clang_tidy_mode = "nosplit"
|
||||
else:
|
||||
clang_tidy_mode = "split"
|
||||
else:
|
||||
clang_tidy_mode = "disabled"
|
||||
|
||||
# Build output
|
||||
output: dict[str, Any] = {
|
||||
"integration_tests": run_integration,
|
||||
"clang_tidy": run_clang_tidy,
|
||||
"clang_tidy_mode": clang_tidy_mode,
|
||||
"clang_format": run_clang_format,
|
||||
"python_linters": run_python_linters,
|
||||
"changed_components": changed_components,
|
||||
@@ -462,6 +493,7 @@ def main() -> None:
|
||||
"component_test_count": len(changed_components_with_tests),
|
||||
"directly_changed_count": len(directly_changed_with_tests),
|
||||
"dependency_only_count": len(dependency_only_components),
|
||||
"changed_cpp_file_count": changed_cpp_file_count,
|
||||
"memory_impact": memory_impact,
|
||||
}
|
||||
|
||||
|
@@ -107,6 +107,7 @@ def test_main_all_tests_should_run(
|
||||
|
||||
assert output["integration_tests"] is True
|
||||
assert output["clang_tidy"] is True
|
||||
assert output["clang_tidy_mode"] in ["nosplit", "split"]
|
||||
assert output["clang_format"] is True
|
||||
assert output["python_linters"] is True
|
||||
assert output["changed_components"] == ["wifi", "api", "sensor"]
|
||||
@@ -117,6 +118,9 @@ def test_main_all_tests_should_run(
|
||||
assert output["component_test_count"] == len(
|
||||
output["changed_components_with_tests"]
|
||||
)
|
||||
# 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
|
||||
assert "memory_impact" in output
|
||||
assert output["memory_impact"]["should_run"] == "false" # No files changed
|
||||
@@ -156,11 +160,14 @@ def test_main_no_tests_should_run(
|
||||
|
||||
assert output["integration_tests"] is False
|
||||
assert output["clang_tidy"] is False
|
||||
assert output["clang_tidy_mode"] == "disabled"
|
||||
assert output["clang_format"] is False
|
||||
assert output["python_linters"] is False
|
||||
assert output["changed_components"] == []
|
||||
assert output["changed_components_with_tests"] == []
|
||||
assert output["component_test_count"] == 0
|
||||
# changed_cpp_file_count should be 0
|
||||
assert output["changed_cpp_file_count"] == 0
|
||||
# memory_impact should be present
|
||||
assert "memory_impact" in output
|
||||
assert output["memory_impact"]["should_run"] == "false"
|
||||
@@ -239,6 +246,7 @@ def test_main_with_branch_argument(
|
||||
|
||||
assert output["integration_tests"] is False
|
||||
assert output["clang_tidy"] is True
|
||||
assert output["clang_tidy_mode"] in ["nosplit", "split"]
|
||||
assert output["clang_format"] is False
|
||||
assert output["python_linters"] is True
|
||||
assert output["changed_components"] == ["mqtt"]
|
||||
@@ -249,6 +257,9 @@ def test_main_with_branch_argument(
|
||||
assert output["component_test_count"] == len(
|
||||
output["changed_components_with_tests"]
|
||||
)
|
||||
# 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
|
||||
assert "memory_impact" in output
|
||||
assert output["memory_impact"]["should_run"] == "false"
|
||||
@@ -433,6 +444,40 @@ def test_should_run_clang_format_with_branch() -> None:
|
||||
mock_changed.assert_called_once_with("release")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("changed_files", "expected_count"),
|
||||
[
|
||||
(["esphome/core.cpp"], 1),
|
||||
(["esphome/core.h"], 1),
|
||||
(["test.hpp"], 1),
|
||||
(["test.cc"], 1),
|
||||
(["test.cxx"], 1),
|
||||
(["test.c"], 1),
|
||||
(["test.tcc"], 1),
|
||||
(["esphome/core.cpp", "esphome/core.h"], 2),
|
||||
(["esphome/core.cpp", "esphome/core.h", "test.cc"], 3),
|
||||
(["README.md"], 0),
|
||||
(["esphome/config.py"], 0),
|
||||
(["README.md", "esphome/config.py"], 0),
|
||||
(["esphome/core.cpp", "README.md", "esphome/config.py"], 1),
|
||||
([], 0),
|
||||
],
|
||||
)
|
||||
def test_count_changed_cpp_files(changed_files: list[str], expected_count: int) -> None:
|
||||
"""Test count_changed_cpp_files function."""
|
||||
with patch.object(determine_jobs, "changed_files", return_value=changed_files):
|
||||
result = determine_jobs.count_changed_cpp_files()
|
||||
assert result == expected_count
|
||||
|
||||
|
||||
def test_count_changed_cpp_files_with_branch() -> None:
|
||||
"""Test count_changed_cpp_files with branch argument."""
|
||||
with patch.object(determine_jobs, "changed_files") as mock_changed:
|
||||
mock_changed.return_value = []
|
||||
determine_jobs.count_changed_cpp_files("release")
|
||||
mock_changed.assert_called_once_with("release")
|
||||
|
||||
|
||||
def test_main_filters_components_without_tests(
|
||||
mock_should_run_integration_tests: Mock,
|
||||
mock_should_run_clang_tidy: Mock,
|
||||
@@ -501,6 +546,9 @@ def test_main_filters_components_without_tests(
|
||||
assert set(output["changed_components_with_tests"]) == {"wifi", "sensor"}
|
||||
# component_test_count should be based on components with tests
|
||||
assert output["component_test_count"] == 2
|
||||
# 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
|
||||
assert "memory_impact" in output
|
||||
assert output["memory_impact"]["should_run"] == "false"
|
||||
|
Reference in New Issue
Block a user