Fix clang-tidy-check: distinguish tooling failures from issue detection#357
Merged
greenc-FNAL merged 5 commits intomaintenance/ensure-clang-tidy-info-retrievalfrom Feb 24, 2026
Merged
Conversation
Copilot
AI
changed the title
[WIP] Address feedback on clang-tidy-check log visibility
Fix clang-tidy-check: distinguish tooling failures from issue detection
Feb 24, 2026
594da03 to
8ee4851
Compare
cmake exits non-zero both when clang-tidy finds issues (fixes YAML populated) and when a tooling failure occurs (fixes YAML absent/empty). The previous `|| true` discarded all non-zero exit codes, masking real failures. Now we capture the exit code and fail the job only when cmake is non-zero AND no fixes YAML was produced. Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
46e3703 to
4e5501f
Compare
The previous logic incorrectly used the fixes YAML file to distinguish tooling failures from issue detection. This fails when clang-tidy finds issues without automatic fixes (many checks don't provide auto-fixes). Now checks for diagnostic output in the log to determine if clang-tidy ran successfully. Only fails CI when exit code is non-zero AND no diagnostics are present, indicating a true tooling failure.
Contributor
|
@phlexbot format |
Contributor
|
No automatic cmake-format fixes were necessary. |
Contributor
|
No automatic markdownlint fixes were necessary. |
Contributor
|
No automatic jsonnetfmt fixes were necessary. |
Contributor
|
No automatic clang-format fixes were necessary. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the clang-tidy GitHub Actions workflow to correctly differentiate “clang-tidy found issues” from “clang-tidy infrastructure/tooling failed,” so real tooling failures no longer get silently masked.
Changes:
- Capture the
cmake --build ... clang-tidy-checkexit code and analyze the log to decide whether to fail the job (tooling failure) or continue (issues found). - Switch issue detection from “non-empty fixes YAML” to “diagnostic output present in log.”
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
e177322
into
maintenance/ensure-clang-tidy-info-retrieval
41 checks passed
greenc-FNAL
added a commit
that referenced
this pull request
Feb 24, 2026
…355) * Resolve `clang-tidy-check` issues with log visibility and CI status * Fix clang-tidy-check: distinguish tooling failures from issue detection (#357) * Initial plan * Distinguish clang-tidy tooling failures from issue detection cmake exits non-zero both when clang-tidy finds issues (fixes YAML populated) and when a tooling failure occurs (fixes YAML absent/empty). The previous `|| true` discarded all non-zero exit codes, masking real failures. Now we capture the exit code and fail the job only when cmake is non-zero AND no fixes YAML was produced. Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> * Fix clang-tidy failure detection: check log content not fixes file The previous logic incorrectly used the fixes YAML file to distinguish tooling failures from issue detection. This fails when clang-tidy finds issues without automatic fixes (many checks don't provide auto-fixes). Now checks for diagnostic output in the log to determine if clang-tidy ran successfully. Only fails CI when exit code is non-zero AND no diagnostics are present, indicating a true tooling failure. * Bypass early exit due to `exit_on_error` Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove unwanted whitespace --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> Co-authored-by: Chris Green <greenc@fnal.gov> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
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.
cmake --build . --target clang-tidy-checkexits non-zero both when clang-tidy finds code issues and when a tooling failure occurs. The previous|| truemasked both cases, silently passing on real infrastructure failures.Changes
clang-tidy-check.yaml: Replace|| truewith explicit exit-code capture and log content analysis to distinguish the two non-zero cases:::error::+ fail jobThe heuristic checks for clang-tidy diagnostic output (file:line:col format) rather than relying on the fixes file, which is only populated when auto-fixes are available. Many clang-tidy checks do not provide automatic fixes.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.