review fixes

Author: andrewjstone

trust-quorum/src/established_conn.rs

@@ -190,7 +190,7 @@ impl EstablishedConn {
             debug!(self.log, "Received {msg:?}");
             match msg {
                 WireMsg::Tq(msg) => {
-                    if let Err(_) = self.main_tx.try_send(ConnToMainMsg {
+                    if let Err(e) = self.main_tx.try_send(ConnToMainMsg {
                         task_id: self.task_id,
                         msg: ConnToMainMsgInner::Received {
                             from: self.peer_id.clone(),
@@ -201,7 +201,7 @@ impl EstablishedConn {
                             self.log,
                             "Failed to send received fsm msg to main task"
                         );
-                        panic!("Connection to main task channel full");
+                        panic!("Connection to main task channel error: {e:#?}");
                     }
                 }
                 WireMsg::Ping => {
@@ -210,7 +210,7 @@ impl EstablishedConn {
                 }
                 WireMsg::NetworkConfig(config) => {
                     let generation = config.generation;
-                    if let Err(_) = self.main_tx.try_send(ConnToMainMsg {
+                    if let Err(e) = self.main_tx.try_send(ConnToMainMsg {
                         task_id: self.task_id,
                         msg: ConnToMainMsgInner::ReceivedNetworkConfig {
                             from: self.peer_id.clone(),
@@ -222,11 +222,11 @@ impl EstablishedConn {
                             "Failed to send received NetworkConfig with
                              generation {generation} to main task"
                         );
-                        panic!("Connection to main task channnel full");
+                        panic!("Connection to main task channel error: {e:#?}");
                     }
                 }
                 WireMsg::ProxyRequest(req) => {
-                    if let Err(_) = self.main_tx.try_send(ConnToMainMsg {
+                    if let Err(e) = self.main_tx.try_send(ConnToMainMsg {
                         task_id: self.task_id,
                         msg: ConnToMainMsgInner::ProxyRequestReceived {
                             from: self.peer_id.clone(),
@@ -235,13 +235,13 @@ impl EstablishedConn {
                     }) {
                         error!(
                             self.log,
-                            "Failed to send received proxy msg to the main task"
+                            "Failed to send received proxy request to the main task"
                         );
-                        panic!("Connection to main task channel full");
+                        panic!("Connection to main task channel error: {e:#?}");
                     }
                 }
                 WireMsg::ProxyResponse(rsp) => {
-                    if let Err(_) = self.main_tx.try_send(ConnToMainMsg {
+                    if let Err(e) = self.main_tx.try_send(ConnToMainMsg {
                         task_id: self.task_id,
                         msg: ConnToMainMsgInner::ProxyResponseReceived {
                             from: self.peer_id.clone(),
@@ -250,9 +250,9 @@ impl EstablishedConn {
                     }) {
                         error!(
                             self.log,
-                            "Failed to send received proxy msg to the main task"
+                            "Failed to send received proxy resposne to the main task"
                         );
-                        panic!("Connection to main task channel full");
+                        panic!("Connection to main task channel error: {e:#?}");
                     }
                 }
             }

trust-quorum/src/proxy.rs

@@ -374,6 +374,10 @@ mod tests {
         let proxy = Proxy::new(tx.clone());
         let mut tracker = Tracker::new();
 
+        // All spawned tasks will increment this value when processing a
+        // request. The result is polled with `wait_for_condition` at the end
+        // of the test, and therefore there is no reason to join on any of the
+        // spawned test tasks.
         let requests_completed = Arc::new(AtomicUsize::new(0));
 
         // This is the first "user" task that will issue proxy operations
@@ -474,15 +478,17 @@ mod tests {
         // connections here, so just use an ID of an arbitrary task.
         let task_id = task::spawn(async {}).id();
 
+        // All spawned tasks will increment this value when processing a
+        // request. The result is polled with `wait_for_condition` at the end
+        // of the test, and therefore there is no reason to join on any of the
+        // spawned test tasks.
         let requests_completed = Arc::new(AtomicUsize::new(0));
 
         // This is the first "user" task that will issue proxy operations
         let count = requests_completed.clone();
         let dest = destination.clone();
         let _ = spawn(async move {
             let s = proxy.commit(dest, rack_id, Epoch(1)).await.unwrap_err();
-
-            // The first attempt should succeed
             assert_matches!(s, ProxyError::InvalidResponse(_));
             let _ = count.fetch_add(1, Ordering::Relaxed);
         });
@@ -546,15 +552,17 @@ mod tests {
         let proxy = Proxy::new(tx.clone());
         let mut tracker = Tracker::new();
 
+        // All spawned tasks will increment this value when processing a
+        // request. The result is polled with `wait_for_condition` at the end
+        // of the test, and therefore there is no reason to join on any of the
+        // spawned test tasks.
         let requests_completed = Arc::new(AtomicUsize::new(0));
 
         // This is the first "user" task that will issue proxy operations
         let count = requests_completed.clone();
         let dest = destination.clone();
         let _ = spawn(async move {
             let s = proxy.commit(dest, rack_id, Epoch(1)).await.unwrap_err();
-
-            // The first attempt should succeed
             assert_eq!(s, ProxyError::Disconnected);
             let _ = count.fetch_add(1, Ordering::Relaxed);
         });

trust-quorum/src/task.rs

@@ -1391,7 +1391,7 @@ mod tests {
     /// at the first 3 nodes. Then we go and issue a `PrepareAndCommit` to the last
     /// node and ensure it commits.
     #[tokio::test]
-    pub async fn tq_initial_config_prepare_and_commit() {
+    async fn tq_initial_config_prepare_and_commit() {
         let num_nodes = 4;
         let setup = TestSetup::spawn_nodes(
             "tq_initial_config_prepare_and_commit",
@@ -1526,7 +1526,7 @@ mod tests {
     /// the configuration for the prior epoch. This should result in commit
     /// advancing to the latest epoch.
     #[tokio::test]
-    pub async fn tq_reconfig_with_commit_advance() {
+    async fn tq_reconfig_with_commit_advance() {
         let num_nodes = 4;
         let setup = TestSetup::spawn_nodes(
             "tq_recofnig_with_commit_advance",
@@ -1744,7 +1744,7 @@ mod tests {
     }
 
     #[tokio::test]
-    pub async fn tq_upgrade_from_lrtq() {
+    async fn tq_upgrade_from_lrtq() {
         let num_nodes = 4;
         let (setup, rack_id) = TestSetup::spawn_nodes_with_lrtq_shares(
             "tq_upgrade_from_lrtq",
@@ -1831,7 +1831,7 @@ mod tests {
 
     /// Ensure state is persisted as we expect
     #[tokio::test]
-    pub async fn tq_persistent_state() {
+    async fn tq_persistent_state() {
         let num_nodes = 4;
         let mut setup =
             TestSetup::spawn_nodes("tq_initial_config", num_nodes).await;
@@ -2117,7 +2117,7 @@ mod tests {
 
     /// Proxy API requests to other nodes
     #[tokio::test]
-    pub async fn tq_proxy() {
+    async fn tq_proxy() {
         let num_nodes = 4;
         let mut setup = TestSetup::spawn_nodes("tq_proxy", num_nodes).await;
         let rack_id = RackUuid::new_v4();