Repository navigation
chore: run tests with ASAN in CI - #360
cartercanedy wants to merge 4 commits into
Conversation
09f43d2 to
103ce7c
Compare
|
Stacked PRs might not work (yet) with forked repositories. Marking this as Draft until #356 has been merged. |
a6c712d to
f7095e4
Compare
390a82f to
72ddf19
Compare
|
I tried to merge |
|
A first local test on Linux/Fedora failed. Update: Probably, because |
|
Run on CI succeeded: https://github.com/uklotzde/open62541/actions/runs/39746865588 |
There was a problem hiding this comment.
🟡 Changes recommended
The ASAN CI matrix can test an unintended target, and unpinned LLVM/nightly versions can cause required-job failures.
1 open issue
What changed in this PR
Adds AddressSanitizer test coverage for Rust tests and integrates native Linux/macOS ASAN jobs into CI.
Changes:
- Adds nightly ASAN test recipes with leak detection.
- Adds Linux and macOS ASAN CI matrix jobs.
- Updates the array ownership test to transfer raw allocation ownership safely.
| File | Description | Review findings |
|---|---|---|
src/ua/array.rs |
Updates the raw-array test to use move-based ownership transfer. | No unresolved findings. |
.justfile |
Defines reusable nightly ASAN test and command recipes. | No unresolved findings. |
.github/workflows/test.yaml |
Adds Linux/macOS ASAN jobs and macOS LLVM configuration. | The matrix target is not enforced by the test recipe; the LLVM/nightly pairing is not pinned or verified; and required-check configuration may still need repository-level updates. |
Review effort: Lite
| steps: | ||
| - name: Install Rust toolchain | ||
| # Sanitizers are only available on nightly Rust. | ||
| uses: dtolnay/rust-toolchain@nightly |
There was a problem hiding this comment.
I feel like this is going to be a complete noop practically. If the target triple is set to something else other than the system's target triple, it's either going to run fine (e.g. x86_64-linux-unknown-musl vs -gnu) or it's going to have an incompatible ABI and running it is going to fail

Closes #358
Summary
NOTE: stacked on top of #356, needs to be rebased before merge