-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add a loom implementation for event-listener #126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised concurrent-queue
Loom tests are failing, but I guess that's due to loom
not being checked in CI there.
I'm unsure why this wasn't added to begin with. This adds Loom testing to the CI with a low number of max pre-emptions, in order to avoid making the test take forever. cc smol-rs/event-listener#126 (comment) Signed-off-by: John Nunley <dev@notgull.net>
I added Loom to the CI in smol-rs/concurrent-queue#62. It looks like it works. Maybe it's the optional feature flag that's gumming it up. |
I'm unsure why this wasn't added to begin with. This adds Loom testing to the CI with a low number of max pre-emptions, in order to avoid making the test take forever. cc smol-rs/event-listener#126 (comment) Signed-off-by: John Nunley <dev@notgull.net>
I think and here: |
To be clear, it's not that the concurrent-queue loom tests are failing, or even that concurrent-queue can't build under loom. This PR for event listener correctly builds concurrent-queue under loom and runs tests against concurrent-queue loom. The issue is this specific scenario for concurrent-queue, which may or may not even matter: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Mirrors #126 Signed-off-by: John Nunley <dev@notgull.net>
The loom tests aren't particularly useful yet, but the crate compiles and runs under loom now.
Note: This crate does not currently compile under loom without std because concurrent-queue does not compile with that combination (can't find spin_loop).
closes #23