From 8953e53a04a282fb3261e3be3da95aaebdee0c59 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 10 Jul 2025 23:54:57 -1000 Subject: [PATCH] CI: Centralize test determination logic to reduce unnecessary job runners (#9432) --- .github/workflows/ci.yml | 105 +++++---- script/determine-jobs.py | 245 +++++++++++++++++++ script/helpers.py | 126 +++++++++- script/list-components.py | 27 ++- tests/script/test_determine_jobs.py | 352 ++++++++++++++++++++++++++++ tests/script/test_helpers.py | 165 +++++++++++++ 6 files changed, 963 insertions(+), 57 deletions(-) create mode 100755 script/determine-jobs.py create mode 100644 tests/script/test_determine_jobs.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 08d64243b9..503a50c5c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,6 +66,8 @@ jobs: runs-on: ubuntu-24.04 needs: - common + - determine-jobs + if: needs.determine-jobs.outputs.python-linters == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@v4.2.2 @@ -87,6 +89,8 @@ jobs: runs-on: ubuntu-24.04 needs: - common + - determine-jobs + if: needs.determine-jobs.outputs.python-linters == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@v4.2.2 @@ -108,6 +112,8 @@ jobs: runs-on: ubuntu-24.04 needs: - common + - determine-jobs + if: needs.determine-jobs.outputs.python-linters == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@v4.2.2 @@ -129,6 +135,8 @@ jobs: runs-on: ubuntu-24.04 needs: - common + - determine-jobs + if: needs.determine-jobs.outputs.python-linters == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@v4.2.2 @@ -232,11 +240,54 @@ jobs: path: venv key: ${{ runner.os }}-${{ steps.restore-python.outputs.python-version }}-venv-${{ needs.common.outputs.cache-key }} + determine-jobs: + name: Determine which jobs to run + runs-on: ubuntu-24.04 + needs: + - common + outputs: + integration-tests: ${{ steps.determine.outputs.integration-tests }} + clang-tidy: ${{ steps.determine.outputs.clang-tidy }} + clang-format: ${{ steps.determine.outputs.clang-format }} + python-linters: ${{ steps.determine.outputs.python-linters }} + changed-components: ${{ steps.determine.outputs.changed-components }} + component-test-count: ${{ steps.determine.outputs.component-test-count }} + steps: + - name: Check out code from GitHub + uses: actions/checkout@v4.2.2 + with: + # Fetch enough history to find the merge base + 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: Determine which tests to run + id: determine + env: + GH_TOKEN: ${{ github.token }} + run: | + . venv/bin/activate + output=$(python script/determine-jobs.py) + echo "Test determination output:" + echo "$output" | jq + + # 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-format=$(echo "$output" | jq -r '.clang_format')" >> $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 "component-test-count=$(echo "$output" | jq -r '.component_test_count')" >> $GITHUB_OUTPUT + integration-tests: name: Run integration tests runs-on: ubuntu-latest needs: - common + - determine-jobs + if: needs.determine-jobs.outputs.integration-tests == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@v4.2.2 @@ -271,6 +322,8 @@ jobs: runs-on: ubuntu-24.04 needs: - common + - determine-jobs + if: needs.determine-jobs.outputs.clang-format == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@v4.2.2 @@ -304,6 +357,8 @@ jobs: - pylint - pytest - pyupgrade + - determine-jobs + if: needs.determine-jobs.outputs.clang-tidy == 'true' env: GH_TOKEN: ${{ github.token }} strategy: @@ -411,50 +466,18 @@ jobs: # yamllint disable-line rule:line-length if: always() - list-components: - runs-on: ubuntu-24.04 - needs: - - common - if: github.event_name == 'pull_request' - env: - GH_TOKEN: ${{ github.token }} - outputs: - components: ${{ steps.list-components.outputs.components }} - count: ${{ steps.list-components.outputs.count }} - steps: - - name: Check out code from GitHub - uses: actions/checkout@v4.2.2 - - name: Restore Python - uses: ./.github/actions/restore-python - with: - python-version: ${{ env.DEFAULT_PYTHON }} - cache-key: ${{ needs.common.outputs.cache-key }} - - name: Find changed components - id: list-components - run: | - . venv/bin/activate - components=$(script/list-components.py --changed) - output_components=$(echo "$components" | jq -R -s -c 'split("\n")[:-1] | map(select(length > 0))') - count=$(echo "$output_components" | jq length) - - echo "components=$output_components" >> $GITHUB_OUTPUT - echo "count=$count" >> $GITHUB_OUTPUT - - echo "$count Components:" - echo "$output_components" | jq - test-build-components: name: Component test ${{ matrix.file }} runs-on: ubuntu-24.04 needs: - common - - list-components - if: github.event_name == 'pull_request' && fromJSON(needs.list-components.outputs.count) > 0 && fromJSON(needs.list-components.outputs.count) < 100 + - determine-jobs + if: github.event_name == 'pull_request' && fromJSON(needs.determine-jobs.outputs.component-test-count) > 0 && fromJSON(needs.determine-jobs.outputs.component-test-count) < 100 strategy: fail-fast: false max-parallel: 2 matrix: - file: ${{ fromJson(needs.list-components.outputs.components) }} + file: ${{ fromJson(needs.determine-jobs.outputs.changed-components) }} steps: - name: Install dependencies run: | @@ -482,8 +505,8 @@ jobs: runs-on: ubuntu-24.04 needs: - common - - list-components - if: github.event_name == 'pull_request' && fromJSON(needs.list-components.outputs.count) >= 100 + - determine-jobs + if: github.event_name == 'pull_request' && fromJSON(needs.determine-jobs.outputs.component-test-count) >= 100 outputs: matrix: ${{ steps.split.outputs.components }} steps: @@ -492,7 +515,7 @@ jobs: - name: Split components into 20 groups id: split run: | - components=$(echo '${{ needs.list-components.outputs.components }}' | jq -c '.[]' | shuf | jq -s -c '[_nwise(20) | join(" ")]') + components=$(echo '${{ needs.determine-jobs.outputs.changed-components }}' | jq -c '.[]' | shuf | jq -s -c '[_nwise(20) | join(" ")]') echo "components=$components" >> $GITHUB_OUTPUT test-build-components-split: @@ -500,9 +523,9 @@ jobs: runs-on: ubuntu-24.04 needs: - common - - list-components + - determine-jobs - test-build-components-splitter - if: github.event_name == 'pull_request' && fromJSON(needs.list-components.outputs.count) >= 100 + if: github.event_name == 'pull_request' && fromJSON(needs.determine-jobs.outputs.component-test-count) >= 100 strategy: fail-fast: false max-parallel: 4 @@ -553,7 +576,7 @@ jobs: - integration-tests - pyupgrade - clang-tidy - - list-components + - determine-jobs - test-build-components - test-build-components-splitter - test-build-components-split diff --git a/script/determine-jobs.py b/script/determine-jobs.py new file mode 100755 index 0000000000..fc5c397c65 --- /dev/null +++ b/script/determine-jobs.py @@ -0,0 +1,245 @@ +#!/usr/bin/env python3 +"""Determine which CI jobs should run based on changed files. + +This script is a centralized way to determine which CI jobs need to run based on +what files have changed. It outputs JSON with the following structure: + +{ + "integration_tests": true/false, + "clang_tidy": true/false, + "clang_format": true/false, + "python_linters": true/false, + "changed_components": ["component1", "component2", ...], + "component_test_count": 5 +} + +The CI workflow uses this information to: +- Skip or run integration tests +- Skip or run clang-tidy (and whether to do a full scan) +- Skip or run clang-format +- Skip or run Python linters (ruff, flake8, pylint, pyupgrade) +- Determine which components to test individually +- Decide how to split component tests (if there are many) + +Usage: + python script/determine-jobs.py [-b BRANCH] + +Options: + -b, --branch BRANCH Branch to compare against (default: dev) +""" + +from __future__ import annotations + +import argparse +import json +import os +from pathlib import Path +import subprocess +import sys +from typing import Any + +from helpers import ( + CPP_FILE_EXTENSIONS, + ESPHOME_COMPONENTS_PATH, + PYTHON_FILE_EXTENSIONS, + changed_files, + get_all_dependencies, + get_components_from_integration_fixtures, + parse_list_components_output, + root_path, +) + + +def should_run_integration_tests(branch: str | None = None) -> bool: + """Determine if integration tests should run based on changed files. + + This function is used by the CI workflow to intelligently skip integration tests when they're + not needed, saving significant CI time and resources. + + Integration tests will run when ANY of the following conditions are met: + + 1. Core C++ files changed (esphome/core/*) + - Any .cpp, .h, .tcc files in the core directory + - These files contain fundamental functionality used throughout ESPHome + - Examples: esphome/core/component.cpp, esphome/core/application.h + + 2. Core Python files changed (esphome/core/*.py) + - Only .py files in the esphome/core/ directory + - These are core Python files that affect the entire system + - Examples: esphome/core/config.py, esphome/core/__init__.py + - NOT included: esphome/*.py, esphome/dashboard/*.py, esphome/components/*/*.py + + 3. Integration test files changed + - Any file in tests/integration/ directory + - This includes test files themselves and fixture YAML files + - Examples: tests/integration/test_api.py, tests/integration/fixtures/api.yaml + + 4. Components used by integration tests (or their dependencies) changed + - The function parses all YAML files in tests/integration/fixtures/ + - Extracts which components are used in integration tests + - Recursively finds all dependencies of those components + - If any of these components have changes, tests must run + - Example: If api.yaml uses 'sensor' and 'api' components, and 'api' depends on 'socket', + then changes to sensor/, api/, or socket/ components trigger tests + + Args: + branch: Branch to compare against. If None, uses default. + + Returns: + True if integration tests should run, False otherwise. + """ + files = changed_files(branch) + + # Check if any core files changed (esphome/core/*) + for file in files: + if file.startswith("esphome/core/"): + return True + + # Check if any integration test files changed + if any("tests/integration" in file for file in files): + return True + + # Get all components used in integration tests and their dependencies + fixture_components = get_components_from_integration_fixtures() + all_required_components = get_all_dependencies(fixture_components) + + # Check if any required components changed + for file in files: + if file.startswith(ESPHOME_COMPONENTS_PATH): + parts = file.split("/") + if len(parts) >= 3: + component = parts[2] + if component in all_required_components: + return True + + return False + + +def should_run_clang_tidy(branch: str | None = None) -> bool: + """Determine if clang-tidy should run based on changed files. + + This function is used by the CI workflow to intelligently skip clang-tidy checks when they're + not needed, saving significant CI time and resources. + + Clang-tidy will run when ANY of the following conditions are met: + + 1. Clang-tidy configuration changed + - The hash of .clang-tidy configuration file has changed + - The hash includes the .clang-tidy file, clang-tidy version from requirements_dev.txt, + and relevant platformio.ini sections + - When configuration changes, a full scan is needed to ensure all code complies + with the new rules + - Detected by script/clang_tidy_hash.py --check returning exit code 0 + + 2. Any C++ source files changed + - Any file with C++ extensions: .cpp, .h, .hpp, .cc, .cxx, .c, .tcc + - Includes files anywhere in the repository, not just in esphome/ + - This ensures all C++ code is checked, including tests, examples, etc. + - Examples: esphome/core/component.cpp, tests/custom/my_component.h + + If the hash check fails for any reason, clang-tidy runs as a safety measure to ensure + code quality is maintained. + + Args: + branch: Branch to compare against. If None, uses default. + + Returns: + 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 + return True + + return _any_changed_file_endswith(branch, CPP_FILE_EXTENSIONS) + + +def should_run_clang_format(branch: str | None = None) -> bool: + """Determine if clang-format should run based on changed files. + + This function is used by the CI workflow to skip clang-format checks when no C++ files + have changed, saving CI time and resources. + + Clang-format will run when any C++ source files have changed. + + Args: + branch: Branch to compare against. If None, uses default. + + Returns: + True if clang-format should run, False otherwise. + """ + return _any_changed_file_endswith(branch, CPP_FILE_EXTENSIONS) + + +def should_run_python_linters(branch: str | None = None) -> bool: + """Determine if Python linters (ruff, flake8, pylint, pyupgrade) should run based on changed files. + + This function is used by the CI workflow to skip Python linting checks when no Python files + have changed, saving CI time and resources. + + Python linters will run when any Python source files have changed. + + Args: + branch: Branch to compare against. If None, uses default. + + Returns: + True if Python linters should run, False otherwise. + """ + return _any_changed_file_endswith(branch, PYTHON_FILE_EXTENSIONS) + + +def _any_changed_file_endswith(branch: str | None, extensions: tuple[str, ...]) -> bool: + """Check if a changed file ends with any of the specified extensions.""" + return any(file.endswith(extensions) for file in changed_files(branch)) + + +def main() -> None: + """Main function that determines which CI jobs to run.""" + parser = argparse.ArgumentParser( + description="Determine which CI jobs should run based on changed files" + ) + parser.add_argument( + "-b", "--branch", help="Branch to compare changed files against" + ) + args = parser.parse_args() + + # Determine what should run + run_integration = should_run_integration_tests(args.branch) + 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) + + # Get changed components using list-components.py for exact compatibility + script_path = Path(__file__).parent / "list-components.py" + cmd = [sys.executable, str(script_path), "--changed"] + if args.branch: + cmd.extend(["-b", args.branch]) + + result = subprocess.run(cmd, capture_output=True, text=True, check=True) + changed_components = parse_list_components_output(result.stdout) + + # Build output + output: dict[str, Any] = { + "integration_tests": run_integration, + "clang_tidy": run_clang_tidy, + "clang_format": run_clang_format, + "python_linters": run_python_linters, + "changed_components": changed_components, + "component_test_count": len(changed_components), + } + + # Output as JSON + print(json.dumps(output)) + + +if __name__ == "__main__": + main() diff --git a/script/helpers.py b/script/helpers.py index 5dbc7a32cc..ff63bbc5b6 100644 --- a/script/helpers.py +++ b/script/helpers.py @@ -1,5 +1,6 @@ from __future__ import annotations +from functools import cache import json import os import os.path @@ -7,6 +8,7 @@ from pathlib import Path import re import subprocess import time +from typing import Any import colorama @@ -15,6 +17,34 @@ basepath = os.path.join(root_path, "esphome") temp_folder = os.path.join(root_path, ".temp") temp_header_file = os.path.join(temp_folder, "all-include.cpp") +# C++ file extensions used for clang-tidy and clang-format checks +CPP_FILE_EXTENSIONS = (".cpp", ".h", ".hpp", ".cc", ".cxx", ".c", ".tcc") + +# Python file extensions +PYTHON_FILE_EXTENSIONS = (".py", ".pyi") + +# YAML file extensions +YAML_FILE_EXTENSIONS = (".yaml", ".yml") + +# Component path prefix +ESPHOME_COMPONENTS_PATH = "esphome/components/" + + +def parse_list_components_output(output: str) -> list[str]: + """Parse the output from list-components.py script. + + The script outputs one component name per line. + + Args: + output: The stdout from list-components.py + + Returns: + List of component names, or empty list if no output + """ + if not output or not output.strip(): + return [] + return [c.strip() for c in output.strip().split("\n") if c.strip()] + def styled(color: str | tuple[str, ...], msg: str, reset: bool = True) -> str: prefix = "".join(color) if isinstance(color, tuple) else color @@ -96,6 +126,7 @@ def _get_pr_number_from_github_env() -> str | None: return None +@cache def _get_changed_files_github_actions() -> list[str] | None: """Get changed files in GitHub Actions environment. @@ -135,7 +166,7 @@ def changed_files(branch: str | None = None) -> list[str]: return github_files # Original implementation for local development - if branch is None: + if not branch: # Treat None and empty string the same branch = "dev" check_remotes = ["upstream", "origin"] check_remotes.extend(splitlines_no_ends(get_output("git", "remote"))) @@ -183,7 +214,7 @@ def get_changed_components() -> list[str] | None: changed = changed_files() core_cpp_changed = any( f.startswith("esphome/core/") - and f.endswith((".cpp", ".h", ".hpp", ".cc", ".cxx", ".c")) + and f.endswith(CPP_FILE_EXTENSIONS[:-1]) # Exclude .tcc for core files for f in changed ) if core_cpp_changed: @@ -198,8 +229,7 @@ def get_changed_components() -> list[str] | None: result = subprocess.run( cmd, capture_output=True, text=True, check=True, close_fds=False ) - components = [c.strip() for c in result.stdout.strip().split("\n") if c.strip()] - return components + 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") @@ -249,7 +279,9 @@ def _filter_changed_ci(files: list[str]) -> list[str]: # Action: Check only the specific non-component files that changed changed = changed_files() files = [ - f for f in files if f in changed and not f.startswith("esphome/components/") + f + for f in files + if f in changed and not f.startswith(ESPHOME_COMPONENTS_PATH) ] if not files: print("No files changed") @@ -267,7 +299,7 @@ def _filter_changed_ci(files: list[str]) -> list[str]: # because changes in one file can affect other files in the same component. filtered_files = [] for f in files: - if f.startswith("esphome/components/"): + if f.startswith(ESPHOME_COMPONENTS_PATH): # Check if file belongs to any of the changed components parts = f.split("/") if len(parts) >= 3 and parts[2] in component_set: @@ -326,7 +358,7 @@ def git_ls_files(patterns: list[str] | None = None) -> dict[str, int]: return {s[3].strip(): int(s[0]) for s in lines} -def load_idedata(environment): +def load_idedata(environment: str) -> dict[str, Any]: start_time = time.time() print(f"Loading IDE data for environment '{environment}'...") @@ -442,3 +474,83 @@ def get_usable_cpu_count() -> int: return ( os.process_cpu_count() if hasattr(os, "process_cpu_count") else os.cpu_count() ) + + +def get_all_dependencies(component_names: set[str]) -> set[str]: + """Get all dependencies for a set of components. + + Args: + component_names: Set of component names to get dependencies for + + Returns: + Set of all components including dependencies and auto-loaded components + """ + from esphome.const import KEY_CORE + from esphome.core import CORE + from esphome.loader import get_component + + all_components: set[str] = set(component_names) + + # Reset CORE to ensure clean state + CORE.reset() + + # Set up fake config path for component loading + root = Path(__file__).parent.parent + CORE.config_path = str(root) + CORE.data[KEY_CORE] = {} + + # Keep finding dependencies until no new ones are found + while True: + new_components: set[str] = set() + + for comp_name in all_components: + comp = get_component(comp_name) + if not comp: + continue + + # Add dependencies (extract component name before '.') + new_components.update(dep.split(".")[0] for dep in comp.dependencies) + + # Add auto_load components + new_components.update(comp.auto_load) + + # Check if we found any new components + new_components -= all_components + if not new_components: + break + + all_components.update(new_components) + + return all_components + + +def get_components_from_integration_fixtures() -> set[str]: + """Extract all components used in integration test fixtures. + + Returns: + Set of component names used in integration test fixtures + """ + import yaml + + components: set[str] = set() + fixtures_dir = Path(__file__).parent.parent / "tests" / "integration" / "fixtures" + + for yaml_file in fixtures_dir.glob("*.yaml"): + with open(yaml_file) as f: + config: dict[str, any] | None = yaml.safe_load(f) + if not config: + continue + + # Add all top-level component keys + components.update(config.keys()) + + # Add platform components (e.g., output.template) + for value in config.values(): + if not isinstance(value, list): + continue + + for item in value: + if isinstance(item, dict) and "platform" in item: + components.add(item["platform"]) + + return components diff --git a/script/list-components.py b/script/list-components.py index 0afcaa0f9d..66212f44e7 100755 --- a/script/list-components.py +++ b/script/list-components.py @@ -20,6 +20,12 @@ def filter_component_files(str): return str.startswith("esphome/components/") | str.startswith("tests/components/") +def get_all_component_files() -> list[str]: + """Get all component files from git.""" + files = git_ls_files() + return list(filter(filter_component_files, files)) + + def extract_component_names_array_from_files_array(files): components = [] for file in files: @@ -165,17 +171,20 @@ def main(): if args.branch and not args.changed: parser.error("--branch requires --changed") - files = git_ls_files() - files = filter(filter_component_files, files) - if args.changed: - if args.branch: - changed = changed_files(args.branch) - else: - changed = changed_files() + # When --changed is passed, only get the changed files + changed = changed_files(args.branch) + # If any base test file(s) changed, there's no need to filter out components - if not any("tests/test_build_components" in file for file in changed): - files = [f for f in files if f in changed] + if any("tests/test_build_components" in file for file in changed): + # Need to get all component files + files = get_all_component_files() + else: + # Only look at changed component files + files = [f for f in changed if filter_component_files(f)] + else: + # Get all component files + files = get_all_component_files() for c in get_components(files, args.changed): print(c) diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py new file mode 100644 index 0000000000..4aaaadd80a --- /dev/null +++ b/tests/script/test_determine_jobs.py @@ -0,0 +1,352 @@ +"""Unit tests for script/determine-jobs.py module.""" + +from collections.abc import Generator +import importlib.util +import json +import os +import subprocess +import sys +from unittest.mock import Mock, patch + +import pytest + +# Add the script directory to Python path so we can import the module +script_dir = os.path.abspath( + os.path.join(os.path.dirname(__file__), "..", "..", "script") +) +sys.path.insert(0, script_dir) + +spec = importlib.util.spec_from_file_location( + "determine_jobs", os.path.join(script_dir, "determine-jobs.py") +) +determine_jobs = importlib.util.module_from_spec(spec) +spec.loader.exec_module(determine_jobs) + + +@pytest.fixture +def mock_should_run_integration_tests() -> Generator[Mock, None, None]: + """Mock should_run_integration_tests from helpers.""" + with patch.object(determine_jobs, "should_run_integration_tests") as mock: + yield mock + + +@pytest.fixture +def mock_should_run_clang_tidy() -> Generator[Mock, None, None]: + """Mock should_run_clang_tidy from helpers.""" + with patch.object(determine_jobs, "should_run_clang_tidy") as mock: + yield mock + + +@pytest.fixture +def mock_should_run_clang_format() -> Generator[Mock, None, None]: + """Mock should_run_clang_format from helpers.""" + with patch.object(determine_jobs, "should_run_clang_format") as mock: + yield mock + + +@pytest.fixture +def mock_should_run_python_linters() -> Generator[Mock, None, None]: + """Mock should_run_python_linters from helpers.""" + with patch.object(determine_jobs, "should_run_python_linters") as mock: + yield mock + + +@pytest.fixture +def mock_subprocess_run() -> Generator[Mock, None, None]: + """Mock subprocess.run for list-components.py calls.""" + with patch.object(determine_jobs.subprocess, "run") as mock: + yield mock + + +def test_main_all_tests_should_run( + 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, + capsys: pytest.CaptureFixture[str], +) -> None: + """Test when all tests should run.""" + mock_should_run_integration_tests.return_value = True + mock_should_run_clang_tidy.return_value = True + mock_should_run_clang_format.return_value = True + mock_should_run_python_linters.return_value = True + + # Mock list-components.py output + mock_result = Mock() + mock_result.stdout = "wifi\napi\nsensor\n" + mock_subprocess_run.return_value = mock_result + + # Run main function with mocked argv + with patch("sys.argv", ["determine-jobs.py"]): + determine_jobs.main() + + # Check output + captured = capsys.readouterr() + output = json.loads(captured.out) + + assert output["integration_tests"] is True + assert output["clang_tidy"] is True + assert output["clang_format"] is True + assert output["python_linters"] is True + assert output["changed_components"] == ["wifi", "api", "sensor"] + assert output["component_test_count"] == 3 + + +def test_main_no_tests_should_run( + 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, + capsys: pytest.CaptureFixture[str], +) -> None: + """Test when no tests should run.""" + mock_should_run_integration_tests.return_value = False + mock_should_run_clang_tidy.return_value = False + 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 = "" + mock_subprocess_run.return_value = mock_result + + # Run main function with mocked argv + with patch("sys.argv", ["determine-jobs.py"]): + determine_jobs.main() + + # Check output + captured = capsys.readouterr() + output = json.loads(captured.out) + + assert output["integration_tests"] is False + assert output["clang_tidy"] is False + assert output["clang_format"] is False + assert output["python_linters"] is False + assert output["changed_components"] == [] + assert output["component_test_count"] == 0 + + +def test_main_list_components_fails( + 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, + capsys: pytest.CaptureFixture[str], +) -> None: + """Test when list-components.py fails.""" + mock_should_run_integration_tests.return_value = True + mock_should_run_clang_tidy.return_value = True + mock_should_run_clang_format.return_value = True + mock_should_run_python_linters.return_value = True + + # Mock list-components.py failure + mock_subprocess_run.side_effect = subprocess.CalledProcessError(1, "cmd") + + # Run main function with mocked argv - should raise + with patch("sys.argv", ["determine-jobs.py"]): + with pytest.raises(subprocess.CalledProcessError): + determine_jobs.main() + + +def test_main_with_branch_argument( + 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, + capsys: pytest.CaptureFixture[str], +) -> None: + """Test with branch argument.""" + 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 = True + + # Mock list-components.py output + mock_result = Mock() + mock_result.stdout = "mqtt\n" + mock_subprocess_run.return_value = mock_result + + with patch("sys.argv", ["script.py", "-b", "main"]): + determine_jobs.main() + + # Check that functions were called with branch + mock_should_run_integration_tests.assert_called_once_with("main") + mock_should_run_clang_tidy.assert_called_once_with("main") + 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" in call_args + assert "-b" in call_args + assert "main" in call_args + + # Check output + captured = capsys.readouterr() + output = json.loads(captured.out) + + assert output["integration_tests"] is False + assert output["clang_tidy"] is True + assert output["clang_format"] is False + assert output["python_linters"] is True + assert output["changed_components"] == ["mqtt"] + assert output["component_test_count"] == 1 + + +def test_should_run_integration_tests( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test should_run_integration_tests function.""" + # Core C++ files trigger tests + with patch.object( + determine_jobs, "changed_files", return_value=["esphome/core/component.cpp"] + ): + result = determine_jobs.should_run_integration_tests() + assert result is True + + # Core Python files trigger tests + with patch.object( + determine_jobs, "changed_files", return_value=["esphome/core/config.py"] + ): + result = determine_jobs.should_run_integration_tests() + assert result is True + + # Python files directly in esphome/ do NOT trigger tests + with patch.object( + determine_jobs, "changed_files", return_value=["esphome/config.py"] + ): + result = determine_jobs.should_run_integration_tests() + assert result is False + + # Python files in subdirectories (not core) do NOT trigger tests + with patch.object( + determine_jobs, + "changed_files", + return_value=["esphome/dashboard/web_server.py"], + ): + result = determine_jobs.should_run_integration_tests() + assert result is False + + +def test_should_run_integration_tests_with_branch() -> None: + """Test should_run_integration_tests with branch argument.""" + with patch.object(determine_jobs, "changed_files") as mock_changed: + mock_changed.return_value = [] + determine_jobs.should_run_integration_tests("release") + mock_changed.assert_called_once_with("release") + + +def test_should_run_integration_tests_component_dependency() -> None: + """Test that integration tests run when components used in fixtures change.""" + with patch.object( + determine_jobs, "changed_files", return_value=["esphome/components/api/api.cpp"] + ): + with patch.object( + determine_jobs, "get_components_from_integration_fixtures" + ) as mock_fixtures: + mock_fixtures.return_value = {"api", "sensor"} + with patch.object(determine_jobs, "get_all_dependencies") as mock_deps: + mock_deps.return_value = {"api", "sensor", "network"} + result = determine_jobs.should_run_integration_tests() + assert result is True + + +@pytest.mark.parametrize( + ("check_returncode", "changed_files", "expected_result"), + [ + (0, [], True), # Hash changed - need full scan + (1, ["esphome/core.cpp"], True), # C++ file changed + (1, ["README.md"], False), # No C++ files changed + ], +) +def test_should_run_clang_tidy( + check_returncode: int, + changed_files: list[str], + expected_result: bool, +) -> None: + """Test should_run_clang_tidy function.""" + with patch.object(determine_jobs, "changed_files", return_value=changed_files): + # Test with hash check returning specific code + with patch("subprocess.run") as mock_run: + mock_run.return_value = Mock(returncode=check_returncode) + result = determine_jobs.should_run_clang_tidy() + assert result == expected_result + + # Test with hash check failing (exception) + if check_returncode != 0: + with patch("subprocess.run", side_effect=Exception("Failed")): + result = determine_jobs.should_run_clang_tidy() + assert result is True # Fail safe - run clang-tidy + + +def test_should_run_clang_tidy_with_branch() -> None: + """Test should_run_clang_tidy with branch argument.""" + with patch.object(determine_jobs, "changed_files") as mock_changed: + mock_changed.return_value = [] + with patch("subprocess.run") as mock_run: + mock_run.return_value = Mock(returncode=1) # Hash unchanged + determine_jobs.should_run_clang_tidy("release") + mock_changed.assert_called_once_with("release") + + +@pytest.mark.parametrize( + ("changed_files", "expected_result"), + [ + (["esphome/core.py"], True), + (["script/test.py"], True), + (["esphome/test.pyi"], True), # .pyi files should trigger + (["README.md"], False), + ([], False), + ], +) +def test_should_run_python_linters( + changed_files: list[str], expected_result: bool +) -> None: + """Test should_run_python_linters function.""" + with patch.object(determine_jobs, "changed_files", return_value=changed_files): + result = determine_jobs.should_run_python_linters() + assert result == expected_result + + +def test_should_run_python_linters_with_branch() -> None: + """Test should_run_python_linters with branch argument.""" + with patch.object(determine_jobs, "changed_files") as mock_changed: + mock_changed.return_value = [] + determine_jobs.should_run_python_linters("release") + mock_changed.assert_called_once_with("release") + + +@pytest.mark.parametrize( + ("changed_files", "expected_result"), + [ + (["esphome/core.cpp"], True), + (["esphome/core.h"], True), + (["test.hpp"], True), + (["test.cc"], True), + (["test.cxx"], True), + (["test.c"], True), + (["test.tcc"], True), + (["README.md"], False), + ([], False), + ], +) +def test_should_run_clang_format( + changed_files: list[str], expected_result: bool +) -> None: + """Test should_run_clang_format function.""" + with patch.object(determine_jobs, "changed_files", return_value=changed_files): + result = determine_jobs.should_run_clang_format() + assert result == expected_result + + +def test_should_run_clang_format_with_branch() -> None: + """Test should_run_clang_format with branch argument.""" + with patch.object(determine_jobs, "changed_files") as mock_changed: + mock_changed.return_value = [] + determine_jobs.should_run_clang_format("release") + mock_changed.assert_called_once_with("release") diff --git a/tests/script/test_helpers.py b/tests/script/test_helpers.py index bbebdd79c8..d0db08e6f7 100644 --- a/tests/script/test_helpers.py +++ b/tests/script/test_helpers.py @@ -27,6 +27,7 @@ _filter_changed_ci = helpers._filter_changed_ci _filter_changed_local = helpers._filter_changed_local build_all_include = helpers.build_all_include print_file_list = helpers.print_file_list +get_all_dependencies = helpers.get_all_dependencies @pytest.mark.parametrize( @@ -154,6 +155,14 @@ def test_github_actions_push_event(monkeypatch: MonkeyPatch) -> None: assert result == expected_files +@pytest.fixture(autouse=True) +def clear_caches(): + """Clear function caches before each test.""" + # Clear the cache for _get_changed_files_github_actions + _get_changed_files_github_actions.cache_clear() + yield + + def test_get_changed_files_github_actions_pull_request( monkeypatch: MonkeyPatch, ) -> None: @@ -847,3 +856,159 @@ def test_print_file_list_default_title(capsys: pytest.CaptureFixture[str]) -> No assert "Files:" in captured.out assert " test.cpp" in captured.out + + +@pytest.mark.parametrize( + ("component_configs", "initial_components", "expected_components"), + [ + # No dependencies + ( + {"sensor": ([], [])}, # (dependencies, auto_load) + {"sensor"}, + {"sensor"}, + ), + # Simple dependencies + ( + { + "sensor": (["esp32"], []), + "esp32": ([], []), + }, + {"sensor"}, + {"sensor", "esp32"}, + ), + # Auto-load components + ( + { + "light": ([], ["output", "power_supply"]), + "output": ([], []), + "power_supply": ([], []), + }, + {"light"}, + {"light", "output", "power_supply"}, + ), + # Transitive dependencies + ( + { + "comp_a": (["comp_b"], []), + "comp_b": (["comp_c"], []), + "comp_c": ([], []), + }, + {"comp_a"}, + {"comp_a", "comp_b", "comp_c"}, + ), + # Dependencies with dots (sensor.base) + ( + { + "my_comp": (["sensor.base", "binary_sensor.base"], []), + "sensor": ([], []), + "binary_sensor": ([], []), + }, + {"my_comp"}, + {"my_comp", "sensor", "binary_sensor"}, + ), + # Circular dependencies (should not cause infinite loop) + ( + { + "comp_a": (["comp_b"], []), + "comp_b": (["comp_a"], []), + }, + {"comp_a"}, + {"comp_a", "comp_b"}, + ), + ], +) +def test_get_all_dependencies( + component_configs: dict[str, tuple[list[str], list[str]]], + initial_components: set[str], + expected_components: set[str], +) -> None: + """Test dependency resolution for components.""" + with patch("esphome.loader.get_component") as mock_get_component: + + def get_component_side_effect(name: str): + if name in component_configs: + deps, auto_load = component_configs[name] + comp = Mock() + comp.dependencies = deps + comp.auto_load = auto_load + return comp + return None + + mock_get_component.side_effect = get_component_side_effect + + result = helpers.get_all_dependencies(initial_components) + + assert result == expected_components + + +def test_get_all_dependencies_handles_missing_components() -> None: + """Test handling of components that can't be loaded.""" + with patch("esphome.loader.get_component") as mock_get_component: + # First component exists, its dependency doesn't + comp = Mock() + comp.dependencies = ["missing_comp"] + comp.auto_load = [] + + mock_get_component.side_effect = ( + lambda name: comp if name == "existing" else None + ) + + result = helpers.get_all_dependencies({"existing", "nonexistent"}) + + # Should still include all components, even if some can't be loaded + assert result == {"existing", "nonexistent", "missing_comp"} + + +def test_get_all_dependencies_empty_set() -> None: + """Test with empty initial component set.""" + result = helpers.get_all_dependencies(set()) + assert result == set() + + +def test_get_components_from_integration_fixtures() -> None: + """Test extraction of components from fixture YAML files.""" + yaml_content = { + "sensor": [{"platform": "template", "name": "test"}], + "binary_sensor": [{"platform": "gpio", "pin": 5}], + "esphome": {"name": "test"}, + "api": {}, + } + expected_components = { + "sensor", + "binary_sensor", + "esphome", + "api", + "template", + "gpio", + } + + mock_yaml_file = Mock() + + with ( + patch("pathlib.Path.glob") as mock_glob, + patch("builtins.open", create=True), + patch("yaml.safe_load", return_value=yaml_content), + ): + mock_glob.return_value = [mock_yaml_file] + + components = helpers.get_components_from_integration_fixtures() + + assert components == expected_components + + +@pytest.mark.parametrize( + "output,expected", + [ + ("wifi\napi\nsensor\n", ["wifi", "api", "sensor"]), + ("wifi\n", ["wifi"]), + ("", []), + (" \n \n", []), + ("\n\n", []), + (" wifi \n api \n", ["wifi", "api"]), + ("wifi\n\napi\n\nsensor", ["wifi", "api", "sensor"]), + ], +) +def test_parse_list_components_output(output: str, expected: list[str]) -> None: + """Test parse_list_components_output function.""" + result = helpers.parse_list_components_output(output) + assert result == expected