[#1178] Add loom test setup #1194

mox692 · 2025-11-24T10:48:17Z

Notes for Reviewer

This PR adds the setup for loom tests. The actual loom tests will be added in follow-up PRs.

A couple of notes:

There are two ways to enable loom tests: via a Cargo feature or via a cfg flag. I chose cfg because I assume we don’t want to introduce a new Cargo feature across several crates just for this purpose. Using cfg avoids having to touch Cargo.toml in many places.
There are some places where we need to branch explicitly when using loom:
- I added some non-const helper functions for loom tests, since some loom primitives don’t support const initialization.
- For the same reason, I changed some static variables that would otherwise cause a runtime error when executed under loom.

Pre-Review Checklist for the PR Author

Checklist for the PR Reviewer

Commits are properly organized and messages are according to the guideline
Unit tests have been written for new behavior
Public API is documented
PR title describes the changes

Post-review Checklist for the PR Author

All open points are addressed and tracked via issues

References

Relates to #1178.

elfenpiff · 2025-11-24T10:54:12Z

@mox692 Thanks you for the pull requests and taking care of loom!

codecov · 2025-11-24T11:34:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.34%. Comparing base (704ea97) to head (e4871c6).
⚠️ Report is 133 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1194      +/-   ##
==========================================
+ Coverage   77.39%   78.34%   +0.94%     
==========================================
  Files         397      413      +16     
  Lines       39532    41033    +1501     
  Branches      916     1117     +201     
==========================================
+ Hits        30596    32147    +1551     
+ Misses       8190     8030     -160     
- Partials      746      856     +110

Flag	Coverage Δ
CPP	`72.39% <ø> (+7.77%)`	⬆️
Rust	`78.17% <100.00%> (+0.75%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
iceoryx2-bb/elementary/src/lazy_singleton.rs	`50.00% <ø> (ø)`
iceoryx2-bb/elementary/src/unique_id.rs	`100.00% <ø> (ø)`
iceoryx2-bb/lock-free/src/spsc/index_queue.rs	`87.26% <100.00%> (+0.49%)`	⬆️
iceoryx2-bb/log/src/lib.rs	`46.37% <ø> (ø)`
iceoryx2-bb/posix/src/signal.rs	`77.90% <ø> (ø)`
iceoryx2-bb/posix/src/unique_system_id.rs	`96.20% <100.00%> (+0.09%)`	⬆️
iceoryx2-pal/concurrency-sync/src/iox_atomic.rs	`96.19% <100.00%> (ø)`
iceoryx2-pal/concurrency-sync/src/mutex.rs	`94.54% <ø> (ø)`
iceoryx2-pal/concurrency-sync/src/rwlock.rs	`95.19% <ø> (-0.44%)`	⬇️
iceoryx2-pal/posix/src/linux/errno.rs	`25.00% <ø> (ø)`

... and 134 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

elfenpiff

I reviewed it and you implemented it exactly how we intended to integrate loom into iceoryx2 - well done, thank you!

Since it is a crucial part in iceoryx2 I ask @elBoberido that he reviews the PR as well and approves it. Then we can merge it.

elBoberido

Looks good. Just some questions regarding no_std and documentation. Nothing blocking if no_std is still possible, which I assume.

iceoryx2-bb/elementary/src/lazy_singleton.rs

elBoberido · 2025-11-24T15:41:40Z

iceoryx2-bb/elementary/src/lazy_singleton.rs

+    #[cfg(all(test, loom))]
+    pub fn new() -> Self {
+        Self {
+            data: UnsafeCell::new(None),
+            is_initialized: IoxAtomicBool::new(false),
+            is_finalized: IoxAtomicBool::new(false),
+        }
+    }


Can we document that loom cannot handle const functions somewhere? I mean in case we introduce a new function and loom breaks. A good option would be to add the error message and the fix to FAQ_ICEORYX_DEVS.md.

iceoryx2-bb/elementary/src/unique_id.rs

…basic

mox692 · 2025-11-30T05:43:40Z

Thank you for the review!

no_std

Yes good catch, I think this PR should probably be modified so that the loom tests are gated behind the std feature. At this moment, loom does not support no_std environment.

Would it be acceptable to enable loom support only for the targets that are built with std?

orecham · 2025-11-30T09:07:54Z

Would it be acceptable to enable loom support only for the targets that are built with std?

@mox692 Yup, makes sense to me :)

orecham · 2025-12-10T06:30:40Z

iceoryx2-bb/lock-free/Cargo.toml

 version = { workspace = true }

+[lints]
+workspace = true


@elfenpiff Do we want to have these kind of configurations defined at the crate level?

orecham · 2025-12-10T06:45:50Z

iceoryx2-bb/lock-free/src/spsc/index_queue.rs

 //! ```

-use core::{alloc::Layout, cell::UnsafeCell, fmt::Debug, sync::atomic::Ordering};
+#[cfg(all(test, loom, feature = "std"))]


If the type is not coming from std, perhaps the std feature gate could be removed. The check for the loom environment would be enough. It's probably functionally the same, but it seems semantically inaccurate.

Does the loom setup work if removing the std feature gate?

Does the loom setup work if removing the std feature gate?

Yes, it should work. Let me update the code so that the std feature gate is applied only to the parts that require std.

orecham · 2025-12-10T06:46:56Z

iceoryx2-bb/log/src/lib.rs

+
+#[cfg(not(all(test, loom, feature = "std")))]
 static LOG_LEVEL: IoxAtomicU8 = IoxAtomicU8::new(DEFAULT_LOG_LEVEL as u8);
+#[cfg(all(test, loom, feature = "std"))]


In instances like this, the std feature gate makes more sense to me.

orecham · 2025-12-10T06:48:29Z

iceoryx2-pal/posix/src/linux/errno.rs

    }
 }

+#[cfg(not(all(test, loom, feature = "std")))]


Curious, why is this removed for the loom build?

This function contains static variable IS_LOCKED, which loom can't emulate. Since this is top level function and not used in the internal code path, I just exclude that function from loom instead of doing this:

pub unsafe fn strerror_r() { ... #[cfg(all(test, loom, feature = "std"))] static IS_LOCKED: IoxAtomicBool = IoxAtomicBool::new(false); #[cfg(not(all(test, loom, feature = "std")))] static IS_LOCKED: std::sync::LazyLock<IoxAtomicU32> = std::sync::LazyLock::new(|| { unimplemented!("loom does not provide const-initialization for atomic variables.") }); ... }

... but this might be confusing, as all other places do this way. I may update this to unify the style.

[eclipse-iceoryx#1178] Add loom test setup

ada36e6

elfenpiff previously approved these changes Nov 24, 2025

View reviewed changes

elBoberido reviewed Nov 24, 2025

View reviewed changes

Merge remote-tracking branch 'upstream/main' into iox2-1178-add_loom_…

a57c1b4

…basic

address review: put loom test behind the std feature

e4871c6

mox692 dismissed elfenpiff’s stale review via e4871c6 December 9, 2025 06:43

orecham reviewed Dec 10, 2025

View reviewed changes

[#1178] Add loom test setup #1194

Are you sure you want to change the base?

[#1178] Add loom test setup #1194

Uh oh!

Conversation

mox692 commented Nov 24, 2025

Notes for Reviewer

Pre-Review Checklist for the PR Author

Checklist for the PR Reviewer

Post-review Checklist for the PR Author

References

Uh oh!

elfenpiff commented Nov 24, 2025

Uh oh!

codecov bot commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

elfenpiff left a comment

Choose a reason for hiding this comment

Uh oh!

elBoberido left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elBoberido Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mox692 commented Nov 30, 2025

Uh oh!

orecham commented Nov 30, 2025

Uh oh!

orecham Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

orecham Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mox692 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

orecham Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

orecham Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

mox692 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Nov 24, 2025 •

edited

Loading

orecham Dec 10, 2025 •

edited

Loading