fix(ci): lint test files on PRs without breaking push-to-main#27059
Merged
fix(ci): lint test files on PRs without breaking push-to-main#27059
Conversation
mrpollo
changed the title
Contributor
Author
|
CI verification on 97b68d4 (Clang-Tidy pass in 1m59s):
The test-file-lint path itself (BUILD_TESTING=ON → test TUs in compile_commands.json → clang-tidy-diff on a modified test file) was verified in the px4-dev container locally: modifying |
mrpollo
added a commit
that referenced
this pull request
Apr 11, 2026
Replace the inline `if [ "${{ github.event_name }}" = "pull_request" ]`
branches with separate workflow steps gated on `github.event_name`.
The push-to-main path is now two plainly named steps
(`Build - px4_sitl_default (Clang)` + `Run Clang-Tidy Analysis (push)`)
that exactly reproduce the pre-PR-#27059 commands, and the pull request
path is its own pair with `clang-ci` + run-clang-tidy-pr.py. Makes it
visually obvious from the workflow file that the push path is
unchanged.
Signed-off-by: Ramon Roche <mrpollo@gmail.com>
mrpollo
force-pushed
the
mrpollo/clang-tidy-diff-compile-db-filter
branch
from
183d27f to
e9a60be
Compare
mrpollo
changed the title
mrpollo
force-pushed
the
mrpollo/clang-tidy-diff-compile-db-filter
branch
2 times, most recently
from
7e76c5a to
924c2b7
Compare
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>
mrpollo
force-pushed
the
mrpollo/clang-tidy-diff-compile-db-filter
branch
from
924c2b7 to
ff3e85f
Compare
mrpollo
force-pushed
the
mrpollo/clang-tidy-diff-compile-db-filter
branch
from
096bf1a to
ff3e85f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #27055. clang-tidy-diff was flagging
gtest/gtest.h file not foundon PRs that touched test files because test TUs are not in the compilation database unlessBUILD_TESTING=ON.On PR events the workflow now calls
make clang-ci, a new umbrella target that also configures a second build dir (build/px4_sitl_default-clang-test) with-DCMAKE_TESTING=ON.clang-tidy-diff-18.pyruns against that dir so test files have resolved gtest includes. The line filter keeps pre-existing issues invisible.Push-to-main is untouched: same single build, same
make clang-tidy, same behavior.Needs
libclang-rt-18-devinstalled in the container so fuzztest's ASAN flags don't break the abseil configure. Also replaces the fragile:!*/test/*pathspec withTools/ci/clang-tidy-diff-filter.py, which drops diff entries that aren't incompile_commands.json.