[ci] Replace clang-tidy hash with direct config-file diff check (#17019)

This commit is contained in:
Jonathan Swoboda
2026-06-17 21:12:39 -04:00
committed by GitHub
parent 34844da668
commit 7cb6cf2f2a
9 changed files with 124 additions and 674 deletions

View File

@@ -1 +0,0 @@
6765760d573967b853b1f790f0f5478135d12f2b15ffa8bee9b0314090b582ee

View File

@@ -1,76 +0,0 @@
name: Clang-tidy Hash CI
on:
pull_request:
paths:
- ".clang-tidy"
- "platformio.ini"
- "requirements_dev.txt"
- "sdkconfig.defaults"
- ".clang-tidy.hash"
- "script/clang_tidy_hash.py"
- ".github/workflows/ci-clang-tidy-hash.yml"
permissions:
contents: read # actions/checkout for the PR head
pull-requests: write # pulls.createReview / listReviews / dismissReview when the clang-tidy hash is out of date
jobs:
verify-hash:
name: Verify clang-tidy hash
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
- name: Set up Python
uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0
with:
python-version: "3.11"
- name: Verify hash
run: |
python script/clang_tidy_hash.py --verify
- if: failure()
name: Show hash details
run: |
python script/clang_tidy_hash.py
echo "## Job Failed" | tee -a $GITHUB_STEP_SUMMARY
echo "You have modified clang-tidy configuration but have not updated the hash." | tee -a $GITHUB_STEP_SUMMARY
echo "Please run 'script/clang_tidy_hash.py --update' and commit the changes." | tee -a $GITHUB_STEP_SUMMARY
- if: failure() && github.event.pull_request.head.repo.full_name == github.repository
name: Request changes
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
await github.rest.pulls.createReview({
pull_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
event: 'REQUEST_CHANGES',
body: 'You have modified clang-tidy configuration but have not updated the hash.\nPlease run `script/clang_tidy_hash.py --update` and commit the changes.'
})
- if: success() && github.event.pull_request.head.repo.full_name == github.repository
name: Dismiss review
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
let reviews = await github.rest.pulls.listReviews({
pull_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo
});
for (let review of reviews.data) {
if (review.user.login === 'github-actions[bot]' && review.state === 'CHANGES_REQUESTED') {
await github.rest.pulls.dismissReview({
pull_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
review_id: review.id,
message: 'Clang-tidy hash now matches configuration.'
});
}
}

View File

@@ -537,15 +537,12 @@ jobs:
id: check_full_scan id: check_full_scan
run: | run: |
. venv/bin/activate . venv/bin/activate
# determine-jobs.clang-tidy-full-scan is true when core C++ changed # determine-jobs.clang-tidy-full-scan is true when core C++ or a
# OR the ci-run-all label forced --force-all. Independent of the # clang-tidy-relevant config file changed, or the ci-run-all label
# hash check, both must produce a full scan in the job itself. # forced --force-all.
if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then
echo "full_scan=true" >> $GITHUB_OUTPUT echo "full_scan=true" >> $GITHUB_OUTPUT
echo "reason=determine_jobs" >> $GITHUB_OUTPUT echo "reason=determine_jobs" >> $GITHUB_OUTPUT
elif python script/clang_tidy_hash.py --check; then
echo "full_scan=true" >> $GITHUB_OUTPUT
echo "reason=hash_changed" >> $GITHUB_OUTPUT
else else
echo "full_scan=false" >> $GITHUB_OUTPUT echo "full_scan=false" >> $GITHUB_OUTPUT
echo "reason=normal" >> $GITHUB_OUTPUT echo "reason=normal" >> $GITHUB_OUTPUT
@@ -607,15 +604,12 @@ jobs:
id: check_full_scan id: check_full_scan
run: | run: |
. venv/bin/activate . venv/bin/activate
# determine-jobs.clang-tidy-full-scan is true when core C++ changed # determine-jobs.clang-tidy-full-scan is true when core C++ or a
# OR the ci-run-all label forced --force-all. Independent of the # clang-tidy-relevant config file changed, or the ci-run-all label
# hash check, both must produce a full scan in the job itself. # forced --force-all.
if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then
echo "full_scan=true" >> $GITHUB_OUTPUT echo "full_scan=true" >> $GITHUB_OUTPUT
echo "reason=determine_jobs" >> $GITHUB_OUTPUT echo "reason=determine_jobs" >> $GITHUB_OUTPUT
elif python script/clang_tidy_hash.py --check; then
echo "full_scan=true" >> $GITHUB_OUTPUT
echo "reason=hash_changed" >> $GITHUB_OUTPUT
else else
echo "full_scan=false" >> $GITHUB_OUTPUT echo "full_scan=false" >> $GITHUB_OUTPUT
echo "reason=normal" >> $GITHUB_OUTPUT echo "reason=normal" >> $GITHUB_OUTPUT
@@ -691,15 +685,12 @@ jobs:
id: check_full_scan id: check_full_scan
run: | run: |
. venv/bin/activate . venv/bin/activate
# determine-jobs.clang-tidy-full-scan is true when core C++ changed # determine-jobs.clang-tidy-full-scan is true when core C++ or a
# OR the ci-run-all label forced --force-all. Independent of the # clang-tidy-relevant config file changed, or the ci-run-all label
# hash check, both must produce a full scan in the job itself. # forced --force-all.
if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then
echo "full_scan=true" >> $GITHUB_OUTPUT echo "full_scan=true" >> $GITHUB_OUTPUT
echo "reason=determine_jobs" >> $GITHUB_OUTPUT echo "reason=determine_jobs" >> $GITHUB_OUTPUT
elif python script/clang_tidy_hash.py --check; then
echo "full_scan=true" >> $GITHUB_OUTPUT
echo "reason=hash_changed" >> $GITHUB_OUTPUT
else else
echo "full_scan=false" >> $GITHUB_OUTPUT echo "full_scan=false" >> $GITHUB_OUTPUT
echo "reason=normal" >> $GITHUB_OUTPUT echo "reason=normal" >> $GITHUB_OUTPUT
@@ -779,15 +770,12 @@ jobs:
id: check_full_scan id: check_full_scan
run: | run: |
. venv/bin/activate . venv/bin/activate
# determine-jobs.clang-tidy-full-scan is true when core C++ changed # determine-jobs.clang-tidy-full-scan is true when core C++ or a
# OR the ci-run-all label forced --force-all. Independent of the # clang-tidy-relevant config file changed, or the ci-run-all label
# hash check, both must produce a full scan in the job itself. # forced --force-all.
if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then
echo "full_scan=true" >> $GITHUB_OUTPUT echo "full_scan=true" >> $GITHUB_OUTPUT
echo "reason=determine_jobs" >> $GITHUB_OUTPUT echo "reason=determine_jobs" >> $GITHUB_OUTPUT
elif python script/clang_tidy_hash.py --check; then
echo "full_scan=true" >> $GITHUB_OUTPUT
echo "reason=hash_changed" >> $GITHUB_OUTPUT
else else
echo "full_scan=false" >> $GITHUB_OUTPUT echo "full_scan=false" >> $GITHUB_OUTPUT
echo "reason=normal" >> $GITHUB_OUTPUT echo "reason=normal" >> $GITHUB_OUTPUT
@@ -1049,7 +1037,7 @@ jobs:
cache-key: ${{ needs.common.outputs.cache-key }} cache-key: ${{ needs.common.outputs.cache-key }}
- uses: esphome/pre-commit-action@43cd1109c09c544d97196f7730ee5b2e0cc6d81e # v3.0.1 fork with pinned actions/cache - uses: esphome/pre-commit-action@43cd1109c09c544d97196f7730ee5b2e0cc6d81e # v3.0.1 fork with pinned actions/cache
env: env:
SKIP: pylint,clang-tidy-hash,ci-custom SKIP: pylint,ci-custom
- uses: pre-commit-ci/lite-action@5d6cc0eb514c891a40562a58a8e71576c5c7fb43 # v1.1.0 - uses: pre-commit-ci/lite-action@5d6cc0eb514c891a40562a58a8e71576c5c7fb43 # v1.1.0
if: always() if: always()

View File

@@ -6,7 +6,7 @@ ci:
autoupdate_commit_msg: 'pre-commit: autoupdate' autoupdate_commit_msg: 'pre-commit: autoupdate'
autoupdate_schedule: off # Disabled until ruff versions are synced between deps and pre-commit autoupdate_schedule: off # Disabled until ruff versions are synced between deps and pre-commit
# Skip hooks that have issues in pre-commit CI environment # Skip hooks that have issues in pre-commit CI environment
skip: [pylint, clang-tidy-hash] skip: [pylint]
repos: repos:
- repo: https://github.com/astral-sh/ruff-pre-commit - repo: https://github.com/astral-sh/ruff-pre-commit
@@ -59,13 +59,6 @@ repos:
language: system language: system
types: [python] types: [python]
files: ^esphome/.+\.py$ files: ^esphome/.+\.py$
- id: clang-tidy-hash
name: Update clang-tidy hash
entry: python script/clang_tidy_hash.py --update-if-changed
language: python
files: ^(\.clang-tidy|platformio\.ini|requirements_dev\.txt|sdkconfig\.defaults|esphome/idf_component\.yml)$
pass_filenames: false
additional_dependencies: []
- id: ci-custom - id: ci-custom
name: ci-custom name: ci-custom
entry: python script/run-in-env.py script/ci-custom.py entry: python script/run-in-env.py script/ci-custom.py

View File

@@ -276,7 +276,7 @@ def lint_newline(fname, line, col, content):
return "File contains Windows newline. Please set your editor to Unix newline mode." return "File contains Windows newline. Please set your editor to Unix newline mode."
@lint_content_check(exclude=["*.svg", ".clang-tidy.hash"]) @lint_content_check(exclude=["*.svg"])
def lint_end_newline(fname, content): def lint_end_newline(fname, content):
if content and not content.endswith("\n"): if content and not content.endswith("\n"):
return "File does not end with a newline, please add an empty line at the end of the file." return "File does not end with a newline, please add an empty line at the end of the file."

208
script/clang_tidy_hash.py Executable file → Normal file
View File

@@ -1,66 +1,32 @@
#!/usr/bin/env python3 """Files that affect clang-tidy results, and a content hash over them.
"""Calculate and manage hash for clang-tidy configuration."""
``CLANG_TIDY_GLOBAL_FILES`` (plus ``SDKCONFIG_DEFAULTS_PREFIX``) is the single
source of truth for which files influence clang-tidy output. A change to any of
them can surface warnings in source files a PR didn't touch, so:
* ``script/determine-jobs.py`` runs a full clang-tidy scan when one changes, and
* ``calculate_clang_tidy_hash()`` folds them into the idedata cache key used by
``script/helpers.py`` (a content hash, unlike an mtime check, stays correct
across git checkouts).
"""
from __future__ import annotations from __future__ import annotations
import argparse
import hashlib import hashlib
from pathlib import Path from pathlib import Path
import re
import sys
# Add the script directory to path to import helpers # Root-relative paths whose contents affect clang-tidy results.
script_dir = Path(__file__).parent CLANG_TIDY_GLOBAL_FILES = (
sys.path.insert(0, str(script_dir)) ".clang-tidy",
"platformio.ini",
"requirements_dev.txt",
"esphome/idf_component.yml",
)
# sdkconfig.defaults and per-target sdkconfig.defaults.<target> files flip the
def read_file_lines(path: Path) -> list[str]: # CONFIG flags that decide which variant code paths clang-tidy sees. Matched by
"""Read lines from a file.""" # this prefix at the repo root.
with path.open() as f: SDKCONFIG_DEFAULTS_PREFIX = "sdkconfig.defaults"
return f.readlines()
def parse_requirement_line(line: str) -> tuple[str, str] | None:
"""Parse a requirement line and return (package, original_line) or None.
Handles formats like:
- package==1.2.3
- package==1.2.3 # comment
- package>=1.2.3,<2.0.0
"""
original_line = line.strip()
# Extract the part before any comment for parsing
parse_line = line
if "#" in parse_line:
parse_line = parse_line[: parse_line.index("#")]
parse_line = parse_line.strip()
if not parse_line:
return None
# Use regex to extract package name
# This matches package names followed by version operators
match = re.match(r"^([a-zA-Z0-9_-]+)(==|>=|<=|>|<|!=|~=)(.+)$", parse_line)
if match:
return (match.group(1), original_line) # Return package name and original line
return None
def get_clang_tidy_version_from_requirements(repo_root: Path | None = None) -> str:
"""Get clang-tidy version from requirements_dev.txt"""
repo_root = _ensure_repo_root(repo_root)
requirements_path = repo_root / "requirements_dev.txt"
lines = read_file_lines(requirements_path)
for line in lines:
parsed = parse_requirement_line(line)
if parsed and parsed[0] == "clang-tidy":
# Return the original line (preserves comments)
return parsed[1]
return "clang-tidy version not found"
def read_file_bytes(path: Path) -> bytes: def read_file_bytes(path: Path) -> bytes:
@@ -80,130 +46,20 @@ def _ensure_repo_root(repo_root: Path | None) -> Path:
def calculate_clang_tidy_hash(repo_root: Path | None = None) -> str: def calculate_clang_tidy_hash(repo_root: Path | None = None) -> str:
"""Calculate hash of clang-tidy configuration and version""" """Calculate a hash of the files that affect clang-tidy results."""
repo_root = _ensure_repo_root(repo_root) repo_root = _ensure_repo_root(repo_root)
hasher = hashlib.sha256() hasher = hashlib.sha256()
# Hash .clang-tidy file for name in CLANG_TIDY_GLOBAL_FILES:
clang_tidy_path = repo_root / ".clang-tidy" path = repo_root / name
content = read_file_bytes(clang_tidy_path) if path.exists():
hasher.update(content) hasher.update(read_file_bytes(path))
# Hash clang-tidy version from requirements_dev.txt # Hash each sdkconfig.defaults* file. Include the filename so adding or
version = get_clang_tidy_version_from_requirements(repo_root) # renaming a per-target variant is detected, not just content edits.
hasher.update(version.encode()) for path in sorted(repo_root.glob(f"{SDKCONFIG_DEFAULTS_PREFIX}*")):
hasher.update(path.name.encode())
# Hash the entire platformio.ini file hasher.update(read_file_bytes(path))
platformio_path = repo_root / "platformio.ini"
platformio_content = read_file_bytes(platformio_path)
hasher.update(platformio_content)
# Hash sdkconfig.defaults and any per-target sdkconfig.defaults.<target>:
# the per-target files flip CONFIG flags that change which variant code
# paths clang-tidy sees. Include the filename so a rename is detected.
for sdkconfig_path in sorted(repo_root.glob("sdkconfig.defaults*")):
hasher.update(sdkconfig_path.name.encode())
hasher.update(read_file_bytes(sdkconfig_path))
# Hash esphome/idf_component.yml: its managed deps drive the ESP-IDF
# build's include set, which clang-tidy analyzes.
idf_component_path = repo_root / "esphome" / "idf_component.yml"
if idf_component_path.exists():
hasher.update(read_file_bytes(idf_component_path))
return hasher.hexdigest() return hasher.hexdigest()
def read_stored_hash(repo_root: Path | None = None) -> str | None:
"""Read the stored hash from file"""
repo_root = _ensure_repo_root(repo_root)
hash_file = repo_root / ".clang-tidy.hash"
if hash_file.exists():
lines = read_file_lines(hash_file)
return lines[0].strip() if lines else None
return None
def write_file_content(path: Path, content: str) -> None:
"""Write content to a file."""
with path.open("w") as f:
f.write(content)
def write_hash(hash_value: str, repo_root: Path | None = None) -> None:
"""Write hash to file"""
repo_root = _ensure_repo_root(repo_root)
hash_file = repo_root / ".clang-tidy.hash"
# Strip any trailing newlines to ensure consistent formatting
write_file_content(hash_file, hash_value.strip() + "\n")
def main() -> None:
parser = argparse.ArgumentParser(description="Manage clang-tidy configuration hash")
parser.add_argument(
"--check",
action="store_true",
help="Check if full scan needed (exit 0 if needed)",
)
parser.add_argument("--update", action="store_true", help="Update the hash file")
parser.add_argument(
"--update-if-changed",
action="store_true",
help="Update hash only if configuration changed (for pre-commit)",
)
parser.add_argument(
"--verify", action="store_true", help="Verify hash matches (for CI)"
)
args = parser.parse_args()
current_hash = calculate_clang_tidy_hash()
stored_hash = read_stored_hash()
if args.check:
# Check if hash changed OR if .clang-tidy.hash was updated in this PR
# This is used in CI to determine if a full clang-tidy scan is needed
hash_changed = current_hash != stored_hash
# Lazy import to avoid requiring dependencies that aren't needed for other modes
from helpers import changed_files # noqa: E402
hash_file_updated = ".clang-tidy.hash" in changed_files()
# Exit 0 if full scan needed
sys.exit(0 if (hash_changed or hash_file_updated) else 1)
elif args.verify:
# Verify that hash file is up to date with current configuration
# This is used in pre-commit and CI checks to ensure hash was updated
if current_hash != stored_hash:
print("ERROR: Clang-tidy configuration has changed but hash not updated!")
print(f"Expected: {current_hash}")
print(f"Found: {stored_hash}")
print("\nPlease run: script/clang_tidy_hash.py --update")
sys.exit(1)
print("Hash verification passed")
elif args.update:
write_hash(current_hash)
print(f"Hash updated: {current_hash}")
elif args.update_if_changed:
if current_hash != stored_hash:
write_hash(current_hash)
print(f"Clang-tidy hash updated: {current_hash}")
# Exit 0 so pre-commit can stage the file
sys.exit(0)
else:
print("Clang-tidy hash unchanged")
sys.exit(0)
else:
print(f"Current hash: {current_hash}")
print(f"Stored hash: {stored_hash}")
print(f"Match: {current_hash == stored_hash}")
if __name__ == "__main__":
main()

View File

@@ -55,10 +55,10 @@ from functools import cache
import json import json
import os import os
from pathlib import Path from pathlib import Path
import subprocess
import sys import sys
from typing import Any from typing import Any
from clang_tidy_hash import CLANG_TIDY_GLOBAL_FILES, SDKCONFIG_DEFAULTS_PREFIX
from helpers import ( from helpers import (
CPP_FILE_EXTENSIONS, CPP_FILE_EXTENSIONS,
ESPHOME_TESTS_COMPONENTS_PATH, ESPHOME_TESTS_COMPONENTS_PATH,
@@ -280,23 +280,22 @@ def determine_integration_tests(branch: str | None = None) -> tuple[bool, list[s
@cache @cache
def _is_clang_tidy_full_scan() -> bool: def _is_clang_tidy_full_scan(branch: str | None = None) -> bool:
"""Check if clang-tidy configuration changed (requires full scan). """Check if a clang-tidy-relevant config file changed (requires full scan).
A change to a file that affects clang-tidy globally can surface warnings in
source files the PR didn't touch, so the entire codebase must be re-scanned.
Returns: Returns:
True if full scan is needed (hash changed), False otherwise. True if full scan is needed, False otherwise.
""" """
try: for file in changed_files(branch):
result = subprocess.run( if file in CLANG_TIDY_GLOBAL_FILES:
[str(Path(root_path) / "script" / "clang_tidy_hash.py"), "--check"], return True
capture_output=True, # Root-level sdkconfig.defaults and per-target sdkconfig.defaults.<target>
check=False, if "/" not in file and file.startswith(SDKCONFIG_DEFAULTS_PREFIX):
) return True
# Exit 0 means hash changed (full scan needed) return False
return result.returncode == 0
except Exception: # noqa: BLE001
# If hash check fails, run full scan to be safe
return True
def should_run_clang_tidy(branch: str | None = None) -> bool: def should_run_clang_tidy(branch: str | None = None) -> bool:
@@ -307,13 +306,12 @@ def should_run_clang_tidy(branch: str | None = None) -> bool:
Clang-tidy will run when ANY of the following conditions are met: Clang-tidy will run when ANY of the following conditions are met:
1. Clang-tidy configuration changed 1. A clang-tidy-relevant config file changed (full scan needed)
- The hash of .clang-tidy configuration file has changed - Any file in CLANG_TIDY_GLOBAL_FILES (.clang-tidy, platformio.ini,
- The hash includes the .clang-tidy file, clang-tidy version from requirements_dev.txt, requirements_dev.txt, esphome/idf_component.yml) or a root-level
and relevant platformio.ini sections sdkconfig.defaults* file
- When configuration changes, a full scan is needed to ensure all code complies - These affect clang-tidy results globally, so all code must be re-checked
with the new rules to ensure it still complies
- Detected by script/clang_tidy_hash.py --check returning exit code 0
2. Any C++ source files changed 2. Any C++ source files changed
- Any file with C++ extensions: .cpp, .h, .hpp, .cc, .cxx, .c, .tcc - Any file with C++ extensions: .cpp, .h, .hpp, .cc, .cxx, .c, .tcc
@@ -321,27 +319,14 @@ def should_run_clang_tidy(branch: str | None = None) -> bool:
- This ensures all C++ code is checked, including tests, examples, etc. - This ensures all C++ code is checked, including tests, examples, etc.
- Examples: esphome/core/component.cpp, tests/custom/my_component.h - Examples: esphome/core/component.cpp, tests/custom/my_component.h
3. The .clang-tidy.hash file itself changed
- This indicates the configuration has been updated and clang-tidy should run
- Ensures that PRs updating the clang-tidy configuration are properly validated
If the hash check fails for any reason, clang-tidy runs as a safety measure to ensure
code quality is maintained.
Args: Args:
branch: Branch to compare against. If None, uses default. branch: Branch to compare against. If None, uses default.
Returns: Returns:
True if clang-tidy should run, False otherwise. True if clang-tidy should run, False otherwise.
""" """
# First check if clang-tidy configuration changed (full scan needed) # First check if a clang-tidy-relevant config file changed (full scan needed)
if _is_clang_tidy_full_scan(): if _is_clang_tidy_full_scan(branch):
return True
# Check if .clang-tidy.hash file itself was changed
# This handles the case where the hash was properly updated in the PR
files = changed_files(branch)
if ".clang-tidy.hash" in files:
return True return True
return _any_changed_file_endswith(branch, CPP_FILE_EXTENSIONS) return _any_changed_file_endswith(branch, CPP_FILE_EXTENSIONS)
@@ -1276,9 +1261,9 @@ def main() -> None:
# Determine clang-tidy mode based on actual files that will be checked # Determine clang-tidy mode based on actual files that will be checked
is_full_scan = False is_full_scan = False
if run_clang_tidy: if run_clang_tidy:
# Full scan needed if: hash changed OR core files changed # Full scan needed if: a clang-tidy-relevant config file changed OR
# (is_core_change is forced True under --force-all) # core files changed (is_core_change is forced True under --force-all)
is_full_scan = _is_clang_tidy_full_scan() or is_core_change is_full_scan = _is_clang_tidy_full_scan(args.branch) or is_core_change
if is_full_scan: if is_full_scan:
# Full scan checks all files - always use split mode for efficiency # Full scan checks all files - always use split mode for efficiency

View File

@@ -1,9 +1,7 @@
"""Unit tests for script/clang_tidy_hash.py module.""" """Unit tests for script/clang_tidy_hash.py module."""
import hashlib
from pathlib import Path from pathlib import Path
import sys import sys
from unittest.mock import Mock, patch
import pytest import pytest
@@ -11,76 +9,45 @@ import pytest
sys.path.insert(0, str(Path(__file__).parent.parent.parent / "script")) sys.path.insert(0, str(Path(__file__).parent.parent.parent / "script"))
import clang_tidy_hash # noqa: E402 import clang_tidy_hash # noqa: E402
from clang_tidy_hash import CLANG_TIDY_GLOBAL_FILES # noqa: E402
@pytest.mark.parametrize( def _populate(repo_root: Path) -> None:
("file_content", "expected"), """Create every clang-tidy global file plus a base sdkconfig.defaults."""
[ for name in CLANG_TIDY_GLOBAL_FILES:
( path = repo_root / name
"clang-tidy==18.1.5 # via -r requirements_dev.in\n", path.parent.mkdir(parents=True, exist_ok=True)
"clang-tidy==18.1.5 # via -r requirements_dev.in", path.write_text(f"contents of {name}\n")
), (repo_root / "sdkconfig.defaults").write_text("CONFIG_BASE=y\n")
(
"other-package==1.0\nclang-tidy==17.0.0\nmore-packages==2.0\n",
"clang-tidy==17.0.0", def test_calculate_clang_tidy_hash_is_deterministic(tmp_path: Path) -> None:
), """Same inputs must produce the same hash."""
( _populate(tmp_path)
"# comment\nclang-tidy==16.0.0 # some comment\n", assert clang_tidy_hash.calculate_clang_tidy_hash(
"clang-tidy==16.0.0 # some comment", repo_root=tmp_path
), ) == clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path)
("no-clang-tidy-here==1.0\n", "clang-tidy version not found"),
],
) @pytest.mark.parametrize("filename", CLANG_TIDY_GLOBAL_FILES)
def test_get_clang_tidy_version_from_requirements( def test_calculate_clang_tidy_hash_changes_with_each_global_file(
file_content: str, expected: str tmp_path: Path, filename: str
) -> None: ) -> None:
"""Test extracting clang-tidy version from various file formats.""" """Editing any global file must change the hash."""
# Mock read_file_lines to return our test content _populate(tmp_path)
with patch("clang_tidy_hash.read_file_lines") as mock_read: before = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path)
mock_read.return_value = file_content.splitlines(keepends=True)
result = clang_tidy_hash.get_clang_tidy_version_from_requirements() (tmp_path / filename).write_text("changed\n")
after = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path)
assert result == expected assert after != before
def test_calculate_clang_tidy_hash_with_sdkconfig(tmp_path: Path) -> None:
"""Test calculating hash from all configuration sources including sdkconfig.defaults."""
clang_tidy_content = b"Checks: '-*,readability-*'\n"
requirements_version = "clang-tidy==18.1.5"
platformio_content = b"[env:esp32]\nplatform = espressif32\n"
sdkconfig_content = b""
requirements_content = "clang-tidy==18.1.5\n"
# Create temporary files
(tmp_path / ".clang-tidy").write_bytes(clang_tidy_content)
(tmp_path / "platformio.ini").write_bytes(platformio_content)
(tmp_path / "sdkconfig.defaults").write_bytes(sdkconfig_content)
(tmp_path / "requirements_dev.txt").write_text(requirements_content)
# Expected hash calculation
expected_hasher = hashlib.sha256()
expected_hasher.update(clang_tidy_content)
expected_hasher.update(requirements_version.encode())
expected_hasher.update(platformio_content)
expected_hasher.update(b"sdkconfig.defaults")
expected_hasher.update(sdkconfig_content)
expected_hash = expected_hasher.hexdigest()
result = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path)
assert result == expected_hash
def test_calculate_clang_tidy_hash_includes_per_target_sdkconfig( def test_calculate_clang_tidy_hash_includes_per_target_sdkconfig(
tmp_path: Path, tmp_path: Path,
) -> None: ) -> None:
"""Per-target sdkconfig.defaults.<target> files must be part of the hash.""" """Per-target sdkconfig.defaults.<target> files must be part of the hash."""
(tmp_path / ".clang-tidy").write_bytes(b"Checks: '-*'\n") _populate(tmp_path)
(tmp_path / "platformio.ini").write_bytes(b"[env:esp32]\n")
(tmp_path / "requirements_dev.txt").write_text("clang-tidy==18.1.5\n")
(tmp_path / "sdkconfig.defaults").write_bytes(b"CONFIG_BASE=y\n")
before = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path) before = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path)
# Adding a per-target file must change the hash. # Adding a per-target file must change the hash.
@@ -95,230 +62,14 @@ def test_calculate_clang_tidy_hash_includes_per_target_sdkconfig(
assert after_edit != after_add assert after_edit != after_add
def test_calculate_clang_tidy_hash_without_sdkconfig(tmp_path: Path) -> None: def test_calculate_clang_tidy_hash_handles_missing_optional_files(
"""Test calculating hash without sdkconfig.defaults file.""" tmp_path: Path,
clang_tidy_content = b"Checks: '-*,readability-*'\n" ) -> None:
requirements_version = "clang-tidy==18.1.5" """Hash calculation must not fail when files are absent."""
platformio_content = b"[env:esp32]\nplatform = espressif32\n" # Only .clang-tidy present; everything else missing.
requirements_content = "clang-tidy==18.1.5\n" (tmp_path / ".clang-tidy").write_text("Checks: '-*'\n")
# Create temporary files (without sdkconfig.defaults)
(tmp_path / ".clang-tidy").write_bytes(clang_tidy_content)
(tmp_path / "platformio.ini").write_bytes(platformio_content)
(tmp_path / "requirements_dev.txt").write_text(requirements_content)
# Expected hash calculation (no sdkconfig)
expected_hasher = hashlib.sha256()
expected_hasher.update(clang_tidy_content)
expected_hasher.update(requirements_version.encode())
expected_hasher.update(platformio_content)
expected_hash = expected_hasher.hexdigest()
result = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path) result = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path)
assert len(result) == 64 # sha256 hexdigest length
assert result == expected_hash
def test_read_stored_hash_exists(tmp_path: Path) -> None:
"""Test reading hash when file exists."""
stored_hash = "abc123def456"
hash_file = tmp_path / ".clang-tidy.hash"
hash_file.write_text(f"{stored_hash}\n")
result = clang_tidy_hash.read_stored_hash(repo_root=tmp_path)
assert result == stored_hash
def test_read_stored_hash_not_exists(tmp_path: Path) -> None:
"""Test reading hash when file doesn't exist."""
result = clang_tidy_hash.read_stored_hash(repo_root=tmp_path)
assert result is None
def test_write_hash(tmp_path: Path) -> None:
"""Test writing hash to file."""
hash_value = "abc123def456"
hash_file = tmp_path / ".clang-tidy.hash"
clang_tidy_hash.write_hash(hash_value, repo_root=tmp_path)
assert hash_file.exists()
assert hash_file.read_text() == hash_value.strip() + "\n"
@pytest.mark.parametrize(
("args", "current_hash", "stored_hash", "hash_file_in_changed", "expected_exit"),
[
(["--check"], "abc123", "abc123", False, 1), # Hashes match, no scan needed
(["--check"], "abc123", "def456", False, 0), # Hashes differ, scan needed
(["--check"], "abc123", None, False, 0), # No stored hash, scan needed
(
["--check"],
"abc123",
"abc123",
True,
0,
), # Hash file updated in PR, scan needed
],
)
def test_main_check_mode(
args: list[str],
current_hash: str,
stored_hash: str | None,
hash_file_in_changed: bool,
expected_exit: int,
) -> None:
"""Test main function in check mode."""
changed = [".clang-tidy.hash"] if hash_file_in_changed else []
# Create a mock module that can be imported
mock_helpers = Mock()
mock_helpers.changed_files = Mock(return_value=changed)
with (
patch("sys.argv", ["clang_tidy_hash.py"] + args),
patch("clang_tidy_hash.calculate_clang_tidy_hash", return_value=current_hash),
patch("clang_tidy_hash.read_stored_hash", return_value=stored_hash),
patch.dict("sys.modules", {"helpers": mock_helpers}),
pytest.raises(SystemExit) as exc_info,
):
clang_tidy_hash.main()
assert exc_info.value.code == expected_exit
def test_main_update_mode(capsys: pytest.CaptureFixture[str]) -> None:
"""Test main function in update mode."""
current_hash = "abc123"
with (
patch("sys.argv", ["clang_tidy_hash.py", "--update"]),
patch("clang_tidy_hash.calculate_clang_tidy_hash", return_value=current_hash),
patch("clang_tidy_hash.write_hash") as mock_write,
):
clang_tidy_hash.main()
mock_write.assert_called_once_with(current_hash)
captured = capsys.readouterr()
assert f"Hash updated: {current_hash}" in captured.out
@pytest.mark.parametrize(
("current_hash", "stored_hash"),
[
("abc123", "def456"), # Hash changed, should update
("abc123", None), # No stored hash, should update
],
)
def test_main_update_if_changed_mode_update(
current_hash: str, stored_hash: str | None, capsys: pytest.CaptureFixture[str]
) -> None:
"""Test main function in update-if-changed mode when update is needed."""
with (
patch("sys.argv", ["clang_tidy_hash.py", "--update-if-changed"]),
patch("clang_tidy_hash.calculate_clang_tidy_hash", return_value=current_hash),
patch("clang_tidy_hash.read_stored_hash", return_value=stored_hash),
patch("clang_tidy_hash.write_hash") as mock_write,
pytest.raises(SystemExit) as exc_info,
):
clang_tidy_hash.main()
assert exc_info.value.code == 0
mock_write.assert_called_once_with(current_hash)
captured = capsys.readouterr()
assert "Clang-tidy hash updated" in captured.out
def test_main_update_if_changed_mode_no_update(
capsys: pytest.CaptureFixture[str],
) -> None:
"""Test main function in update-if-changed mode when no update is needed."""
current_hash = "abc123"
stored_hash = "abc123"
with (
patch("sys.argv", ["clang_tidy_hash.py", "--update-if-changed"]),
patch("clang_tidy_hash.calculate_clang_tidy_hash", return_value=current_hash),
patch("clang_tidy_hash.read_stored_hash", return_value=stored_hash),
patch("clang_tidy_hash.write_hash") as mock_write,
pytest.raises(SystemExit) as exc_info,
):
clang_tidy_hash.main()
assert exc_info.value.code == 0
mock_write.assert_not_called()
captured = capsys.readouterr()
assert "Clang-tidy hash unchanged" in captured.out
def test_main_verify_mode_success(capsys: pytest.CaptureFixture[str]) -> None:
"""Test main function in verify mode when verification passes."""
current_hash = "abc123"
stored_hash = "abc123"
with (
patch("sys.argv", ["clang_tidy_hash.py", "--verify"]),
patch("clang_tidy_hash.calculate_clang_tidy_hash", return_value=current_hash),
patch("clang_tidy_hash.read_stored_hash", return_value=stored_hash),
):
clang_tidy_hash.main()
captured = capsys.readouterr()
assert "Hash verification passed" in captured.out
@pytest.mark.parametrize(
("current_hash", "stored_hash"),
[
("abc123", "def456"), # Hashes differ, verification fails
("abc123", None), # No stored hash, verification fails
],
)
def test_main_verify_mode_failure(
current_hash: str, stored_hash: str | None, capsys: pytest.CaptureFixture[str]
) -> None:
"""Test main function in verify mode when verification fails."""
with (
patch("sys.argv", ["clang_tidy_hash.py", "--verify"]),
patch("clang_tidy_hash.calculate_clang_tidy_hash", return_value=current_hash),
patch("clang_tidy_hash.read_stored_hash", return_value=stored_hash),
pytest.raises(SystemExit) as exc_info,
):
clang_tidy_hash.main()
assert exc_info.value.code == 1
captured = capsys.readouterr()
assert "ERROR: Clang-tidy configuration has changed" in captured.out
def test_main_default_mode(capsys: pytest.CaptureFixture[str]) -> None:
"""Test main function in default mode (no arguments)."""
current_hash = "abc123"
stored_hash = "def456"
with (
patch("sys.argv", ["clang_tidy_hash.py"]),
patch("clang_tidy_hash.calculate_clang_tidy_hash", return_value=current_hash),
patch("clang_tidy_hash.read_stored_hash", return_value=stored_hash),
):
clang_tidy_hash.main()
captured = capsys.readouterr()
assert f"Current hash: {current_hash}" in captured.out
assert f"Stored hash: {stored_hash}" in captured.out
assert "Match: False" in captured.out
def test_read_file_lines(tmp_path: Path) -> None:
"""Test read_file_lines helper function."""
test_file = tmp_path / "test.txt"
test_content = "line1\nline2\nline3\n"
test_file.write_text(test_content)
result = clang_tidy_hash.read_file_lines(test_file)
assert result == ["line1\n", "line2\n", "line3\n"]
def test_read_file_bytes(tmp_path: Path) -> None: def test_read_file_bytes(tmp_path: Path) -> None:
@@ -330,35 +81,3 @@ def test_read_file_bytes(tmp_path: Path) -> None:
result = clang_tidy_hash.read_file_bytes(test_file) result = clang_tidy_hash.read_file_bytes(test_file)
assert result == test_content assert result == test_content
def test_write_file_content(tmp_path: Path) -> None:
"""Test write_file_content helper function."""
test_file = tmp_path / "test.txt"
test_content = "test content"
clang_tidy_hash.write_file_content(test_file, test_content)
assert test_file.read_text() == test_content
@pytest.mark.parametrize(
("line", "expected"),
[
("clang-tidy==18.1.5", ("clang-tidy", "clang-tidy==18.1.5")),
(
"clang-tidy==18.1.5 # comment",
("clang-tidy", "clang-tidy==18.1.5 # comment"),
),
("some-package>=1.0,<2.0", ("some-package", "some-package>=1.0,<2.0")),
("pkg_with-dashes==1.0", ("pkg_with-dashes", "pkg_with-dashes==1.0")),
("# just a comment", None),
("", None),
(" ", None),
("invalid line without version", None),
],
)
def test_parse_requirement_line(line: str, expected: tuple[str, str] | None) -> None:
"""Test parsing individual requirement lines."""
result = clang_tidy_hash.parse_requirement_line(line)
assert result == expected

View File

@@ -5,7 +5,7 @@ import importlib.util
import json import json
from pathlib import Path from pathlib import Path
import sys import sys
from unittest.mock import Mock, call, patch from unittest.mock import Mock, patch
import pytest import pytest
@@ -653,52 +653,38 @@ def test_determine_integration_tests_non_yaml_fixture_runs_all() -> None:
@pytest.mark.parametrize( @pytest.mark.parametrize(
("check_returncode", "changed_files", "expected_result"), ("changed_files", "expected_result"),
[ [
(0, [], True), # Hash changed - need full scan ([], False), # Nothing changed
(1, ["esphome/core.cpp"], True), # C++ file changed (["esphome/core.cpp"], True), # C++ file changed
(1, ["README.md"], False), # No C++ files changed (["README.md"], False), # No C++ files changed
(1, [".clang-tidy.hash"], True), # Hash file itself changed ([".clang-tidy"], True), # clang-tidy config changed - full scan
(1, ["platformio.ini", ".clang-tidy.hash"], True), # Config + hash changed (["platformio.ini"], True), # build config changed - full scan
(["requirements_dev.txt"], True), # clang-tidy version source changed
(["sdkconfig.defaults"], True), # sdkconfig changed - full scan
(["sdkconfig.defaults.esp32c6"], True), # per-target sdkconfig changed
(["esphome/idf_component.yml"], True), # idf managed deps changed
(["platformio.ini", "README.md"], True), # config + non-C++
], ],
) )
def test_should_run_clang_tidy( def test_should_run_clang_tidy(
check_returncode: int,
changed_files: list[str], changed_files: list[str],
expected_result: bool, expected_result: bool,
) -> None: ) -> None:
"""Test should_run_clang_tidy function.""" """Test should_run_clang_tidy function."""
with ( with patch.object(determine_jobs, "changed_files", return_value=changed_files):
patch.object(determine_jobs, "changed_files", return_value=changed_files),
patch("subprocess.run") as mock_run,
):
# Test with hash check returning specific code
mock_run.return_value = Mock(returncode=check_returncode)
result = determine_jobs.should_run_clang_tidy() result = determine_jobs.should_run_clang_tidy()
assert result == expected_result assert result == expected_result
def test_should_run_clang_tidy_hash_check_exception() -> None:
"""Test should_run_clang_tidy when hash check fails with exception."""
# When hash check fails, clang-tidy should run as a safety measure
with (
patch.object(determine_jobs, "changed_files", return_value=["README.md"]),
patch("subprocess.run", side_effect=Exception("Hash check 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: def test_should_run_clang_tidy_with_branch() -> None:
"""Test should_run_clang_tidy with branch argument.""" """Test should_run_clang_tidy with branch argument."""
with patch.object(determine_jobs, "changed_files") as mock_changed: with patch.object(determine_jobs, "changed_files") as mock_changed:
mock_changed.return_value = [] mock_changed.return_value = []
with patch("subprocess.run") as mock_run: determine_jobs.should_run_clang_tidy("release")
mock_run.return_value = Mock(returncode=1) # Hash unchanged # changed_files is queried against the given branch by both the
determine_jobs.should_run_clang_tidy("release") # config-file full-scan check and the C++ extension check.
# Changed files is called twice now - once for hash check, once for .clang-tidy.hash check mock_changed.assert_called_with("release")
assert mock_changed.call_count == 2
mock_changed.assert_has_calls([call("release"), call("release")])
@pytest.mark.parametrize( @pytest.mark.parametrize(