[#1178] Add loom test for spsc index queue#1267
Conversation
mox692
force-pushed
the
iox2-1178-add_loom_test_spsc_index_queue
branch
5 times, most recently
from
8c11f2a to
ef0b0e6
Compare
|
Update: it's fixed! |
mox692
force-pushed
the
iox2-1178-add_loom_test_spsc_index_queue
branch
from
ef0b0e6 to
c6cd8c5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1267 +/- ##
==========================================
- Coverage 77.84% 77.83% -0.02%
==========================================
Files 405 404 -1
Lines 38680 38687 +7
Branches 1256 1256
==========================================
Hits 30111 30111
- Misses 7536 7543 +7
Partials 1033 1033
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
mox692
force-pushed
the
iox2-1178-add_loom_test_spsc_index_queue
branch
from
33c317a to
aa0746a
Compare
mox692
force-pushed
the
iox2-1178-add_loom_test_spsc_index_queue
branch
from
aa0746a to
417ba5b
Compare
|
I think this is ready for review. In addition to adding the test, I made a few updates to ensure the code compiles. Let me know if I should squash these two commits. |
| @@ -0,0 +1,66 @@ | |||
| // Copyright (c) 2026 Contributors to the Eclipse Foundation | |||
There was a problem hiding this comment.
Can you please give us a short description of what this is, what problem it is supposed to solve and where it will be used.
| use loom_oneshot::channel; | ||
|
|
||
| #[test] | ||
| fn spsc_index_queue_loom_tests() { |
There was a problem hiding this comment.
For a first test, this is completely sufficient, but I think we also need a test where we verify the same setup by pushing increasing numbers into the queue. push might then fail since the queue is full, but this is acceptable, good behavior and should not lead to a test failure.
On the consumer side, we should then verify that increasing numbers are received without losing anything. This would require a loop in both threads, maybe from 1...100.000. A good reference would be that the test does not take longer than 10secs.
| } | ||
| } | ||
|
|
||
| #[cfg(not(all(test, loom, feature = "std")))] |
There was a problem hiding this comment.
Why do we need those functions? The same functionality is already implemented with as_ptr(). If possible, I would remove them.
|
|
||
| #[cfg(all(test, loom, feature = "std"))] | ||
| use loom::cell::UnsafeCell; | ||
| pub(crate) type UnsafeCell<T> = loom::cell::UnsafeCell<T>; |
There was a problem hiding this comment.
Shouldn't this be pub type UnsafeCell<T> = loom::cell::UnsafeCell<T>;?
|
@mox692 I would really love to merge you contribution. Do you have by any chance some time to continue on this PR? |
3 participants
Notes for Reviewer
Ref: #1178
This PR adds loom test for
spsc_index_queue. TheUnsafeCelland other atomic types that are used in the index queue will use loom types during the loom test (#1194).Pre-Review Checklist for the PR Author
Convert to draft)iox2-123-introduce-posix-ipc-example)[#123] Add posix ipc example)task-list-completed)PR Reviewer Reminders