feat: add quality.yml CI workflow with sanitizers and decoupled clang-tidy by bburda · Pull Request #348 · selfpatch/ros2_medkit

bburda · 2026-04-03T10:34:53Z

Pull Request

Summary

New quality.yml workflow with 4 parallel Jazzy-only jobs: format-lint, incremental clang-tidy (with ctcache), ASan+UBSan, and TSan
New ROS2MedkitSanitizers.cmake module for -DSANITIZER=asan,ubsan / -DSANITIZER=tsan
TSan suppression file for known DDS/rclcpp false positives
Simplified ci.yml: clang-tidy removed from jazzy-build, jazzy-lint job deleted

Issue

closes Add quality.yml CI workflow: sanitizers + decoupled clang-tidy #346

Type

Bug fix
New feature or tests
Breaking change
Documentation only

Testing

Local build passes without sanitizers (regression check, 2413 tests pass)
Local build + unit tests pass under ASan+UBSan (2413 tests, 0 failures)
Sanitizer cmake module verified: -fsanitize=address,undefined flags confirmed in build output
CI will validate all 4 quality jobs on this PR
TSan suppression file covers known DDS/rclcpp false positives
ASan options include new_delete_type_mismatch=0 to suppress known ROS 2/DDS allocator mismatches

Checklist

Breaking changes are clearly described (and announced in docs / changelog if needed)
Tests were added or updated if needed
Docs were updated if behavior or public API changed

Clang-tidy and format linters move to quality.yml workflow.

Copilot

Pull request overview

Adds a dedicated “Quality” CI workflow to run Jazzy-only formatting/linting, incremental clang-tidy, and sanitizer (ASan/UBSan + TSan) unit-test runs, while simplifying the existing CI workflow by removing Jazzy clang-tidy/lint steps.

Changes:

Introduces .github/workflows/quality.yml with parallel jobs for format-lint, clang-tidy (with ctcache), ASan+UBSan, and TSan.
Adds ROS2MedkitSanitizers.cmake and wires it into several packages to enable -DSANITIZER=... builds.
Removes Jazzy clang-tidy/lint coupling from .github/workflows/ci.yml and adds a TSan suppressions file.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`tsan_suppressions.txt`	Adds CI-referenced TSan suppressions for known ROS 2/DDS false positives.
`src/ros2_medkit_serialization/CMakeLists.txt`	Includes new sanitizer module for per-package sanitizer flag injection.
`src/ros2_medkit_gateway/CMakeLists.txt`	Includes new sanitizer module for gateway builds/tests.
`src/ros2_medkit_fault_reporter/CMakeLists.txt`	Includes new sanitizer module for sanitizer-enabled builds/tests.
`src/ros2_medkit_fault_manager/CMakeLists.txt`	Includes new sanitizer module for sanitizer-enabled builds/tests.
`src/ros2_medkit_discovery_plugins/ros2_medkit_linux_introspection/CMakeLists.txt`	Includes new sanitizer module for sanitizer-enabled builds/tests.
`src/ros2_medkit_diagnostic_bridge/CMakeLists.txt`	Includes new sanitizer module for sanitizer-enabled builds/tests.
`src/ros2_medkit_cmake/CMakeLists.txt`	Installs the new sanitizer CMake module.
`src/ros2_medkit_cmake/cmake/ROS2MedkitSanitizers.cmake`	Implements `-DSANITIZER=...` parsing/validation and applies sanitizer flags.
`src/ros2_medkit_cmake/cmake/ros2_medkit_cmake-extras.cmake`	Updates documentation comment to mention the sanitizer module.
`.github/workflows/quality.yml`	New quality workflow running format-lint, clang-tidy, ASan+UBSan, and TSan jobs.
`.github/workflows/ci.yml`	Removes clang-tidy from Jazzy build and deletes the Jazzy lint job; updates gating dependencies.

.github/workflows/quality.yml

src/ros2_medkit_cmake/cmake/ROS2MedkitSanitizers.cmake

…containers - PyPI package is 'ctcache', not 'clang-tidy-cache' - Use shell expansion for TSAN_OPTIONS suppressions path to resolve correctly inside GitHub Actions containers (github.workspace gives host path, not container path)

- ctcache is not on PyPI, install from git+https://github.com/... - Add httplib::* to TSan suppressions for known races in Server::stop() from system-packaged libcpp-httplib.so

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

.github/workflows/quality.yml

- Install ctcache from GitHub (not on PyPI) - Add git safe.directory for clang-tidy container job - Fix ResourceChangeNotifier destructor race: add active caller counter so shutdown() waits for in-flight notify() calls to drain before destroying members (seq_cst ordering prevents TOCTOU) - Add TSan suppressions for third-party code only: cpp-httplib Server::stop() race, rclcpp mutex/deadlock during test teardown

- Replace active_notify_count_ spin-wait (caused TSan timeout) with lock-based approach: check shutdown flag under queue_mutex_ in notify() so the flag check and queue push are atomic - Create ctcache dir before run-clang-tidy (cache miss leaves it empty)

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

tsan_suppressions.txt

.github/workflows/quality.yml

src/ros2_medkit_gateway/src/resource_change_notifier.cpp

.github/workflows/quality.yml

src/ros2_medkit_cmake/cmake/ROS2MedkitSanitizers.cmake

tsan_suppressions.txt

- ctcache expects real clang-tidy path as argv[1], but run-clang-tidy passes args directly. Add ctcache-wrapper shell script that prepends /usr/bin/clang-tidy to the clang-tidy-cache invocation. - Replace memory_order_relaxed with seq_cst (default) for shutdown_flag_ loads in worker_loop(). Under TSan, relaxed loads may not observe cross-thread stores promptly, causing worker to miss shutdown signal.

Growing test count pushes some jobs close to their limits. Set all quality.yml jobs to 45m, ci.yml jazzy-test and coverage to 45m. Step-level 15m timeouts on test execution remain as safety nets.

- Fix command injection: use env var for changed files list instead of direct GitHub expression interpolation in bash - Add concurrency group to cancel stale quality workflow runs - Add ccache to format-lint job (was building from scratch every run) - Add permissions: contents: read - Pin ctcache to commit SHA for reproducibility - Remove dead compdb install, fix merge comment - Document ASan options, -O1 override, and TSan suppressions - Add include(ROS2MedkitSanitizers) to 4 missing packages: graph_provider, beacon_common, param_beacon, topic_beacon

colcon test does not pass --timeout to ctest (confirmed after trying --ctest-args, space-prefix, and CMAKE_CTEST_ARGUMENTS). Run ctest directly per package in sanitizer jobs with --timeout 180. Remove non-functional CMAKE_CTEST_ARGUMENTS from Sanitizers.cmake.

ament_add_gtest sets TIMEOUT 60 per-test property which ctest --timeout cannot override. Sed CTestTestfile.cmake after build to extend to 180s.

bburda added 4 commits April 3, 2026 11:21

feat: add ROS2MedkitSanitizers.cmake module for ASan/TSan/UBSan support

3391bb0

feat: add TSan suppression file for DDS/rclcpp false positives

01d576e

refactor: remove clang-tidy from jazzy-build, drop jazzy-lint job

0ae5eb3

Clang-tidy and format linters move to quality.yml workflow.

feat: add quality.yml workflow with format-lint, clang-tidy, ASan, TSan

2130036

Copilot AI review requested due to automatic review settings April 3, 2026 10:34

Copilot started reviewing on behalf of bburda April 3, 2026 10:35 View session

bburda self-assigned this Apr 3, 2026

bburda requested a review from mfaferek93 April 3, 2026 10:36

Copilot AI reviewed Apr 3, 2026

View reviewed changes

.github/workflows/quality.yml Show resolved Hide resolved

src/ros2_medkit_cmake/cmake/ROS2MedkitSanitizers.cmake Show resolved Hide resolved

src/ros2_medkit_cmake/cmake/ROS2MedkitSanitizers.cmake Show resolved Hide resolved

bburda added 2 commits April 3, 2026 13:24

fix: install ctcache from GitHub, add cpp-httplib TSan suppression

6919077

- ctcache is not on PyPI, install from git+https://github.com/... - Add httplib::* to TSan suppressions for known races in Server::stop() from system-packaged libcpp-httplib.so

bburda requested a review from Copilot April 3, 2026 12:26

Copilot started reviewing on behalf of bburda April 3, 2026 12:27 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

.github/workflows/quality.yml Outdated Show resolved Hide resolved

.github/workflows/quality.yml Show resolved Hide resolved

.github/workflows/quality.yml Show resolved Hide resolved

bburda added 2 commits April 3, 2026 15:02

bburda requested a review from Copilot April 3, 2026 14:05

Copilot AI reviewed Apr 3, 2026

View reviewed changes

bburda force-pushed the feat/346-quality-ci branch 4 times, most recently from 04f9d5b to e101e27 Compare April 3, 2026 17:23

mfaferek93 reviewed Apr 3, 2026

View reviewed changes

bburda force-pushed the feat/346-quality-ci branch 2 times, most recently from ecdab29 to 19dcb24 Compare April 3, 2026 18:50

bburda force-pushed the feat/346-quality-ci branch from 19dcb24 to 9560087 Compare April 3, 2026 19:33

bburda added 3 commits April 3, 2026 21:56

chore: increase CI job timeouts to 45m minimum

13f1162

Growing test count pushes some jobs close to their limits. Set all quality.yml jobs to 45m, ci.yml jazzy-test and coverage to 45m. Step-level 15m timeouts on test execution remain as safety nets.

fix: override ament 60s test timeout for sanitizer jobs

1997645

ament_add_gtest sets TIMEOUT 60 per-test property which ctest --timeout cannot override. Sed CTestTestfile.cmake after build to extend to 180s.

mfaferek93 self-requested a review April 4, 2026 09:22

mfaferek93 approved these changes Apr 4, 2026

View reviewed changes

mfaferek93 merged commit 4bb10e1 into main Apr 4, 2026
14 checks passed

Conversation

bburda commented Apr 3, 2026

Pull Request

Summary

Issue

Type

Testing

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants