CR Comments

quickwit-oss · Oct 5, 2022 · e4e85c1 · e4e85c1
1 parent df175b8
commit e4e85c1
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 29 deletions.
diff --git a/quickwit/quickwit-actors/Cargo.toml b/quickwit/quickwit-actors/Cargo.toml
@@ -18,11 +18,12 @@ futures = { workspace = true }
 thiserror = { workspace = true }
 tokio = { workspace = true }
 tracing = { workspace = true }
-quickwit-common = { workspace = true }
-quickwit-proto = { workspace = true }
 serde = { workspace = true }
 serde_json = { workspace = true }
 
+quickwit-common = { workspace = true }
+quickwit-proto = { workspace = true }
+
 [features]
 testsuite = []
 

diff --git a/quickwit/quickwit-actors/src/lib.rs b/quickwit/quickwit-actors/src/lib.rs
@@ -46,7 +46,7 @@ mod spawn_builder;
 mod supervisor;
 
 #[cfg(test)]
-mod tests;
+pub(crate) mod tests;
 mod universe;
 
 pub use actor::{Actor, ActorExitStatus, Handler};
@@ -93,13 +93,3 @@ impl<E: fmt::Debug + ServiceError> ServiceError for AskError<E> {
         }
     }
 }
-
-struct TestActor;
-
-impl Actor for TestActor {
-    type ObservableState = usize;
-
-    fn observable_state(&self) -> Self::ObservableState {
-        0
-    }
-}
diff --git a/quickwit/quickwit-actors/src/mailbox.rs b/quickwit/quickwit-actors/src/mailbox.rs
@@ -304,18 +304,18 @@ impl<A: Actor> WeakMailbox<A> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::TestActor;
+    use crate::tests::PingReceiverActor;
 
     #[test]
     fn test_weak_mailbox_downgrade_upgrade() {
-        let (mailbox, _inbox) = create_test_mailbox::<TestActor>();
+        let (mailbox, _inbox) = create_test_mailbox::<PingReceiverActor>();
         let weak_mailbox = mailbox.downgrade();
         assert!(weak_mailbox.upgrade().is_some());
     }
 
     #[test]
     fn test_weak_mailbox_failing_upgrade() {
-        let (mailbox, _inbox) = create_test_mailbox::<TestActor>();
+        let (mailbox, _inbox) = create_test_mailbox::<PingReceiverActor>();
         let weak_mailbox = mailbox.downgrade();
         drop(mailbox);
         assert!(weak_mailbox.upgrade().is_none());

diff --git a/quickwit/quickwit-actors/src/registry.rs b/quickwit/quickwit-actors/src/registry.rs
@@ -124,13 +124,10 @@ impl ActorRegistry {
             }));
     }
 
-    // As far as I can tell this is a false alarm from clippy.
-    #[allow(clippy::await_holding_lock)]
     pub async fn observe(&self, timeout: Duration) -> Vec<ActorObservation> {
         self.gc();
-        let rlock = self.actors.read().unwrap();
         let mut obs_futures = Vec::new();
-        for registry_for_type in rlock.values() {
+        for registry_for_type in self.actors.read().unwrap().values() {
             for obs in &registry_for_type.observables {
                 if obs.is_disconnected() {
                     continue;
@@ -150,7 +147,6 @@ impl ActorRegistry {
                 });
             }
         }
-        drop(rlock);
         future::join_all(obs_futures.into_iter()).await
     }
 
@@ -193,38 +189,39 @@ fn get_iter<A: Actor>(
 mod tests {
     use std::time::Duration;
 
-    use crate::{TestActor, Universe};
+    use crate::tests::PingReceiverActor;
+    use crate::Universe;
 
     #[tokio::test]
     async fn test_registry() {
-        let test_actor = TestActor;
+        let test_actor = PingReceiverActor::default();
         let universe = Universe::new();
         let (_mailbox, _handle) = universe.spawn_builder().spawn(test_actor);
-        let _actor_mailbox = universe.get_one::<TestActor>().unwrap();
+        let _actor_mailbox = universe.get_one::<PingReceiverActor>().unwrap();
     }
 
     #[tokio::test]
     async fn test_registry_killed_actor() {
-        let test_actor = TestActor;
+        let test_actor = PingReceiverActor::default();
         let universe = Universe::new();
         let (_mailbox, handle) = universe.spawn_builder().spawn(test_actor);
         handle.kill().await;
-        assert!(universe.get_one::<TestActor>().is_none());
+        assert!(universe.get_one::<PingReceiverActor>().is_none());
     }
 
     #[tokio::test]
     async fn test_registry_last_mailbox_dropped_actor() {
-        let test_actor = TestActor;
+        let test_actor = PingReceiverActor::default();
         let universe = Universe::new();
         let (mailbox, handle) = universe.spawn_builder().spawn(test_actor);
         drop(mailbox);
         handle.join().await;
-        assert!(universe.get_one::<TestActor>().is_none());
+        assert!(universe.get_one::<PingReceiverActor>().is_none());
     }
 
     #[tokio::test]
     async fn test_get_actor_states() {
-        let test_actor = TestActor;
+        let test_actor = PingReceiverActor::default();
         let universe = Universe::new();
         let (_mailbox, _handle) = universe.spawn_builder().spawn(test_actor);
         let obs = universe.observe(Duration::from_millis(1000)).await;