Ramon Roche 1cf7d75525 fix(ci): lint test files on PRs without breaking push-to-main
The pr-review-poster was flagging `gtest/gtest.h file not found` on any
PR that added or modified a test file, because clang-tidy-diff-18.py
ran against files that weren't in the compilation database. PR #27004
and PR #26233 both hit this. The root cause is that test TUs only
enter compile_commands.json when BUILD_TESTING is ON, which the
historical clang-tidy build does not enable.

This PR fixes both halves of the problem:

1. Add a second make target `px4_sitl_default-clang-test` that configures
   a separate build dir with -DCMAKE_TESTING=ON. Test TUs land in its
   compile_commands.json with resolved gtest/fuzztest include paths.

2. Add an umbrella `clang-ci` target that depends on both
   `px4_sitl_default-clang` and `px4_sitl_default-clang-test` so the PR
   job prepares both build dirs with one make invocation.

3. On PR events the workflow uses `make clang-ci`, installs
   libclang-rt-18-dev (needed so fuzztest's FUZZTEST_FUZZING_MODE flags
   do not fail the abseil try_compile with a misleading "pthreads not
   found" error), and routes the clang-tidy-diff producer at the
   test-enabled build dir.

4. Push-to-main is left entirely alone: same single build dir, same
   `make px4_sitl_default-clang`, same `make clang-tidy`. Test files
   are not in that DB so run-clang-tidy.py keeps ignoring them exactly
   as before. This preserves green main while ~189 pre-existing
   clang-tidy issues in test files remain untouched; fixing those is
   out of scope for this change.

5. Replace the fragile `:!*/test/*` pathspec filter (which missed flat
   `*Test.cpp` files in module roots) with
   `Tools/ci/clang-tidy-diff-filter.py`, which reads the compilation
   database and drops any changed source file that is not a TU.
   Headers always pass through. Production code that happens to use
   test-like names (src/systemcmds/actuator_test, src/drivers/test_ppm,
   etc.) stays analyzed because those are real px4_add_module targets.

Verified in the ghcr.io/px4/px4-dev:v1.17.0-rc2 container and on the
real CI runner:
- cmake configure with CMAKE_TESTING=ON succeeds after installing
  libclang-rt-18-dev (Found Threads: TRUE)
- compile_commands.json grows from 1333 to 1521 TUs
- Modifying HysteresisTest.cpp with a new `const char *p = NULL`
  correctly flags hicpp-use-nullptr and
  clang-diagnostic-unused-variable on the new line, while pre-existing
  issues on other lines of the same file stay suppressed by
  clang-tidy-diff-18.py's line filter ("Suppressed ... 1 due to line
  filter")
- No gtest/gtest.h false positives
- Push-to-main path unchanged, still green

Signed-off-by: Ramon Roche <mrpollo@gmail.com>
2026-04-11 10:03:51 -06:00

162 lines
5.5 KiB
YAML

name: Static Analysis
on:
push:
branches:
- 'main'
paths-ignore:
- 'docs/**'
pull_request:
branches:
- '**'
paths-ignore:
- 'docs/**'
permissions:
contents: read
jobs:
# Push-to-main: unchanged historical behavior. Single clang build dir
# with BUILD_TESTING=OFF. `make clang-tidy` builds and analyzes every
# TU in compile_commands.json. Test files are not in the DB and
# therefore never analyzed.
clang_tidy_push:
name: Clang-Tidy
if: github.event_name != 'pull_request'
runs-on: [runs-on, runner=16cpu-linux-x64, "run-id=${{ github.run_id }}", "extras=s3-cache"]
container:
image: ghcr.io/px4/px4-dev:v1.17.0-rc2
permissions:
contents: read
steps:
- uses: runs-on/action@v2
- uses: actions/checkout@v6
with:
fetch-depth: 0
fetch-tags: true
- name: Configure Git Safe Directory
run: git config --system --add safe.directory '*'
- uses: ./.github/actions/setup-ccache
id: ccache
with:
cache-key-prefix: ccache-clang-tidy
max-size: 150M
- name: Build and Analyze - Clang-Tidy
run: make -j$(nproc) clang-tidy
- uses: ./.github/actions/save-ccache
if: always()
with:
cache-primary-key: ${{ steps.ccache.outputs.cache-primary-key }}
# Pull request: diff-based analysis with a second BUILD_TESTING=ON
# build dir so test files in the PR diff can be linted by
# clang-tidy-diff-18.py with resolved gtest/fuzztest includes.
# Results are uploaded as a `pr-review` artifact for the PR Review
# Poster workflow to post as inline comments.
clang_tidy_pr:
name: Clang-Tidy
if: github.event_name == 'pull_request'
runs-on: [runs-on, runner=8cpu-linux-x64, "run-id=${{ github.run_id }}", "extras=s3-cache"]
container:
image: ghcr.io/px4/px4-dev:v1.17.0-rc2
permissions:
contents: read
pull-requests: read
steps:
- uses: runs-on/action@v2
- uses: actions/checkout@v6
with:
fetch-depth: 0
fetch-tags: true
- name: Configure Git Safe Directory
run: git config --system --add safe.directory '*'
- uses: ./.github/actions/setup-ccache
id: ccache
with:
cache-key-prefix: ccache-clang-tidy
max-size: 150M
# fuzztest (enabled via BUILD_TESTING in the -test build dir) pulls
# in abseil via FetchContent, and abseil runs a try_compile with
# fuzztest's -fsanitize=address flags. The px4-dev container ships
# clang but not the clang compiler-rt runtime, so that link fails
# and the configure reports a misleading "pthreads not found".
# libclang-rt-18-dev provides libclang_rt.asan and friends.
- name: Install clang compiler-rt
run: |
apt-get update
apt-get install -y --no-install-recommends libclang-rt-18-dev
# `make clang-ci` prepares both clang build directories:
# - build/px4_sitl_default-clang: full build, BUILD_TESTING=OFF
# (used by run-clang-tidy-pr.py for whole-file analysis of
# changed production code)
# - build/px4_sitl_default-clang-test: configure-only, BUILD_TESTING=ON
# (used by clang-tidy-diff-18.py so test files are in the
# compilation database with resolved gtest/fuzztest includes)
- name: Build clang SITL
run: make -j$(nproc) clang-ci
- name: Run Clang-Tidy Analysis
run: python3 Tools/ci/run-clang-tidy-pr.py origin/${{ github.base_ref }}
# Produce a `pr-review` artifact for the PR Review Poster workflow
# to consume. clang-tidy-diff-18 emits a unified fixes.yml that
# the producer script translates into line-anchored review comments.
- name: Export clang-tidy fixes for PR review
if: always()
run: |
mkdir -p pr-review
# Drop changed C/C++ source files that are not in
# compile_commands.json for the test-enabled build. Files not
# in the DB are platform-specific sources, vendored code, or
# submodule code we don't own. Feeding them to clang-tidy-diff
# produces false positives from unresolved headers.
python3 Tools/ci/clang-tidy-diff-filter.py \
--build-dir build/px4_sitl_default-clang-test \
--base-ref origin/${{ github.base_ref }} \
--out pr-review/diff.patch
if [ -s pr-review/diff.patch ]; then
clang-tidy-diff-18.py -p1 \
-path build/px4_sitl_default-clang-test \
-export-fixes pr-review/fixes.yml \
-j0 < pr-review/diff.patch || true
else
echo "No analyzable files in diff; skipping clang-tidy-diff"
fi
- name: Build pr-review artifact
if: always()
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
python3 Tools/ci/clang-tidy-fixes-to-review.py \
--fixes pr-review/fixes.yml \
--repo-root "$GITHUB_WORKSPACE" \
--repo "$GITHUB_REPOSITORY" \
--pr-number "${{ github.event.pull_request.number }}" \
--commit-sha "${{ github.event.pull_request.head.sha }}" \
--out-dir pr-review \
--event COMMENT
- name: Upload pr-review artifact
if: always()
uses: actions/upload-artifact@v7
with:
name: pr-review
path: |
pr-review/manifest.json
pr-review/comments.json
retention-days: 1
- uses: ./.github/actions/save-ccache
if: always()
with:
cache-primary-key: ${{ steps.ccache.outputs.cache-primary-key }}