mirror of
https://github.com/esphome/esphome.git
synced 2025-11-01 15:41:52 +00:00
[ci] Optimize clang-tidy for small PRs by avoiding unnecessary job splitting
This commit is contained in:
23
.github/workflows/ci.yml
vendored
23
.github/workflows/ci.yml
vendored
@@ -175,6 +175,7 @@ jobs:
|
|||||||
changed-components-with-tests: ${{ steps.determine.outputs.changed-components-with-tests }}
|
changed-components-with-tests: ${{ steps.determine.outputs.changed-components-with-tests }}
|
||||||
directly-changed-components-with-tests: ${{ steps.determine.outputs.directly-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 }}
|
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 }}
|
memory_impact: ${{ steps.determine.outputs.memory-impact }}
|
||||||
steps:
|
steps:
|
||||||
- name: Check out code from GitHub
|
- name: Check out code from GitHub
|
||||||
@@ -205,6 +206,7 @@ jobs:
|
|||||||
echo "changed-components-with-tests=$(echo "$output" | jq -c '.changed_components_with_tests')" >> $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 "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 "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
|
echo "memory-impact=$(echo "$output" | jq -c '.memory_impact')" >> $GITHUB_OUTPUT
|
||||||
|
|
||||||
integration-tests:
|
integration-tests:
|
||||||
@@ -249,7 +251,15 @@ jobs:
|
|||||||
needs:
|
needs:
|
||||||
- common
|
- common
|
||||||
- determine-jobs
|
- determine-jobs
|
||||||
if: needs.determine-jobs.outputs.clang-tidy == 'true'
|
# Skip split jobs for small PRs (< 30 files) - run single job instead for faster completion
|
||||||
|
# Skip unsplit job for large PRs (>= 30 files) - run split jobs instead for parallelization
|
||||||
|
# Jobs without split attribute (ESP8266, IDF, ZEPHYR) always run when clang-tidy is enabled
|
||||||
|
if: |
|
||||||
|
needs.determine-jobs.outputs.clang-tidy == 'true' &&
|
||||||
|
(matrix.split == null || (
|
||||||
|
!(matrix.split == true && fromJSON(needs.determine-jobs.outputs.changed-cpp-file-count) < 30) &&
|
||||||
|
!(matrix.split == false && fromJSON(needs.determine-jobs.outputs.changed-cpp-file-count) >= 30)
|
||||||
|
))
|
||||||
env:
|
env:
|
||||||
GH_TOKEN: ${{ github.token }}
|
GH_TOKEN: ${{ github.token }}
|
||||||
strategy:
|
strategy:
|
||||||
@@ -261,22 +271,33 @@ jobs:
|
|||||||
name: Run script/clang-tidy for ESP8266
|
name: Run script/clang-tidy for ESP8266
|
||||||
options: --environment esp8266-arduino-tidy --grep USE_ESP8266
|
options: --environment esp8266-arduino-tidy --grep USE_ESP8266
|
||||||
pio_cache_key: tidyesp8266
|
pio_cache_key: tidyesp8266
|
||||||
|
# For small PRs (< 30 files), run ESP32 Arduino as a single job for faster completion
|
||||||
|
# For larger PRs, split into 4 parallel jobs to distribute the load
|
||||||
|
- id: clang-tidy
|
||||||
|
name: Run script/clang-tidy for ESP32 Arduino
|
||||||
|
options: --environment esp32-arduino-tidy
|
||||||
|
pio_cache_key: tidyesp32
|
||||||
|
split: false
|
||||||
- id: clang-tidy
|
- id: clang-tidy
|
||||||
name: Run script/clang-tidy for ESP32 Arduino 1/4
|
name: Run script/clang-tidy for ESP32 Arduino 1/4
|
||||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 1
|
options: --environment esp32-arduino-tidy --split-num 4 --split-at 1
|
||||||
pio_cache_key: tidyesp32
|
pio_cache_key: tidyesp32
|
||||||
|
split: true
|
||||||
- id: clang-tidy
|
- id: clang-tidy
|
||||||
name: Run script/clang-tidy for ESP32 Arduino 2/4
|
name: Run script/clang-tidy for ESP32 Arduino 2/4
|
||||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 2
|
options: --environment esp32-arduino-tidy --split-num 4 --split-at 2
|
||||||
pio_cache_key: tidyesp32
|
pio_cache_key: tidyesp32
|
||||||
|
split: true
|
||||||
- id: clang-tidy
|
- id: clang-tidy
|
||||||
name: Run script/clang-tidy for ESP32 Arduino 3/4
|
name: Run script/clang-tidy for ESP32 Arduino 3/4
|
||||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 3
|
options: --environment esp32-arduino-tidy --split-num 4 --split-at 3
|
||||||
pio_cache_key: tidyesp32
|
pio_cache_key: tidyesp32
|
||||||
|
split: true
|
||||||
- id: clang-tidy
|
- id: clang-tidy
|
||||||
name: Run script/clang-tidy for ESP32 Arduino 4/4
|
name: Run script/clang-tidy for ESP32 Arduino 4/4
|
||||||
options: --environment esp32-arduino-tidy --split-num 4 --split-at 4
|
options: --environment esp32-arduino-tidy --split-num 4 --split-at 4
|
||||||
pio_cache_key: tidyesp32
|
pio_cache_key: tidyesp32
|
||||||
|
split: true
|
||||||
- id: clang-tidy
|
- id: clang-tidy
|
||||||
name: Run script/clang-tidy for ESP32 IDF
|
name: Run script/clang-tidy for ESP32 IDF
|
||||||
options: --environment esp32-idf-tidy --grep USE_ESP_IDF
|
options: --environment esp32-idf-tidy --grep USE_ESP_IDF
|
||||||
|
|||||||
@@ -210,6 +210,22 @@ def should_run_clang_tidy(branch: str | None = None) -> bool:
|
|||||||
return _any_changed_file_endswith(branch, CPP_FILE_EXTENSIONS)
|
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 < 30 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:
|
def should_run_clang_format(branch: str | None = None) -> bool:
|
||||||
"""Determine if clang-format should run based on changed files.
|
"""Determine if clang-format should run based on changed files.
|
||||||
|
|
||||||
@@ -408,6 +424,7 @@ def main() -> None:
|
|||||||
run_clang_tidy = should_run_clang_tidy(args.branch)
|
run_clang_tidy = should_run_clang_tidy(args.branch)
|
||||||
run_clang_format = should_run_clang_format(args.branch)
|
run_clang_format = should_run_clang_format(args.branch)
|
||||||
run_python_linters = should_run_python_linters(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
|
# Get both directly changed and all changed components (with dependencies) in one call
|
||||||
script_path = Path(__file__).parent / "list-components.py"
|
script_path = Path(__file__).parent / "list-components.py"
|
||||||
@@ -458,6 +475,7 @@ def main() -> None:
|
|||||||
"component_test_count": len(changed_components_with_tests),
|
"component_test_count": len(changed_components_with_tests),
|
||||||
"directly_changed_count": len(directly_changed_with_tests),
|
"directly_changed_count": len(directly_changed_with_tests),
|
||||||
"dependency_only_count": len(dependency_only_components),
|
"dependency_only_count": len(dependency_only_components),
|
||||||
|
"changed_cpp_file_count": changed_cpp_file_count,
|
||||||
"memory_impact": memory_impact,
|
"memory_impact": memory_impact,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -117,6 +117,9 @@ def test_main_all_tests_should_run(
|
|||||||
assert output["component_test_count"] == len(
|
assert output["component_test_count"] == len(
|
||||||
output["changed_components_with_tests"]
|
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
|
# memory_impact should be present
|
||||||
assert "memory_impact" in output
|
assert "memory_impact" in output
|
||||||
assert output["memory_impact"]["should_run"] == "false" # No files changed
|
assert output["memory_impact"]["should_run"] == "false" # No files changed
|
||||||
@@ -161,6 +164,8 @@ def test_main_no_tests_should_run(
|
|||||||
assert output["changed_components"] == []
|
assert output["changed_components"] == []
|
||||||
assert output["changed_components_with_tests"] == []
|
assert output["changed_components_with_tests"] == []
|
||||||
assert output["component_test_count"] == 0
|
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
|
# memory_impact should be present
|
||||||
assert "memory_impact" in output
|
assert "memory_impact" in output
|
||||||
assert output["memory_impact"]["should_run"] == "false"
|
assert output["memory_impact"]["should_run"] == "false"
|
||||||
@@ -249,6 +254,9 @@ def test_main_with_branch_argument(
|
|||||||
assert output["component_test_count"] == len(
|
assert output["component_test_count"] == len(
|
||||||
output["changed_components_with_tests"]
|
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
|
# memory_impact should be present
|
||||||
assert "memory_impact" in output
|
assert "memory_impact" in output
|
||||||
assert output["memory_impact"]["should_run"] == "false"
|
assert output["memory_impact"]["should_run"] == "false"
|
||||||
@@ -433,6 +441,40 @@ def test_should_run_clang_format_with_branch() -> None:
|
|||||||
mock_changed.assert_called_once_with("release")
|
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(
|
def test_main_filters_components_without_tests(
|
||||||
mock_should_run_integration_tests: Mock,
|
mock_should_run_integration_tests: Mock,
|
||||||
mock_should_run_clang_tidy: Mock,
|
mock_should_run_clang_tidy: Mock,
|
||||||
@@ -501,6 +543,9 @@ def test_main_filters_components_without_tests(
|
|||||||
assert set(output["changed_components_with_tests"]) == {"wifi", "sensor"}
|
assert set(output["changed_components_with_tests"]) == {"wifi", "sensor"}
|
||||||
# component_test_count should be based on components with tests
|
# component_test_count should be based on components with tests
|
||||||
assert output["component_test_count"] == 2
|
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
|
# memory_impact should be present
|
||||||
assert "memory_impact" in output
|
assert "memory_impact" in output
|
||||||
assert output["memory_impact"]["should_run"] == "false"
|
assert output["memory_impact"]["should_run"] == "false"
|
||||||
|
|||||||
Reference in New Issue
Block a user