diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0bfbfde527..f085aedcc0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 diff --git a/script/determine-jobs.py b/script/determine-jobs.py index a0e04a256e..c9aebd2cb7 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -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, } diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 7587dbee69..02aaad2e3a 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -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"