Refactor tests and junk server.

src/network.rs

@@ -311,7 +311,10 @@ mod tests {
     use std::sync::MutexGuard;
 
     use crate::test_utils::{
-        junk_server::make_server, make_aborting_rpc, make_echo_rpc,
+        junk_server::{
+            make_test_server, NON_CLIENT, NON_SERVER, TEST_CLIENT, TEST_SERVER,
+        },
+        make_aborting_rpc, make_echo_rpc,
     };
     use crate::{ReplyMessage, Result};
 
@@ -373,7 +376,9 @@ mod tests {
             network
                 .clients
                 .insert(client.into(), (enabled, server.into()));
-            network.servers.insert("test-server".into(), make_server());
+            network
+                .servers
+                .insert(TEST_SERVER.into(), make_test_server());
             network.request_bus.clone()
         };
 
@@ -395,8 +400,8 @@ mod tests {
     #[test]
     fn test_proxy_rpc() -> Result<()> {
         let (rpc, rx) =
-            make_echo_rpc("test-client", "test-server", &[0x09u8, 0x00u8]);
-        let reply = send_rpc(rpc, rx, "test-client", "test-server", true);
+            make_echo_rpc(TEST_CLIENT, TEST_SERVER, &[0x09u8, 0x00u8]);
+        let reply = send_rpc(rpc, rx, TEST_CLIENT, TEST_SERVER, true);
         match reply {
             Ok(reply) => assert_eq!(reply.as_ref(), &[0x00u8, 0x09u8]),
             Err(e) => panic!("Expecting echo message, got {}", e),
@@ -407,8 +412,8 @@ mod tests {
 
     #[test]
     fn test_proxy_rpc_server_error() -> Result<()> {
-        let (rpc, rx) = make_aborting_rpc("test-client", "test-server");
-        let reply = send_rpc(rpc, rx, "test-client", "test-server", true);
+        let (rpc, rx) = make_aborting_rpc(TEST_CLIENT, TEST_SERVER);
+        let reply = send_rpc(rpc, rx, TEST_CLIENT, TEST_SERVER, true);
         let err = reply.expect_err("Network should proxy server errors");
         assert_eq!(std::io::ErrorKind::ConnectionReset, err.kind());
 
@@ -417,8 +422,8 @@ mod tests {
 
     #[test]
     fn test_proxy_rpc_server_not_found() -> Result<()> {
-        let (rpc, rx) = make_aborting_rpc("test-client", "non-test-server");
-        let reply = send_rpc(rpc, rx, "test-client", "non-test-server", true);
+        let (rpc, rx) = make_aborting_rpc(TEST_CLIENT, NON_SERVER);
+        let reply = send_rpc(rpc, rx, TEST_CLIENT, NON_SERVER, true);
         let err = reply.expect_err("Network should check server in memory");
         assert_eq!(std::io::ErrorKind::NotFound, err.kind());
 
@@ -427,8 +432,8 @@ mod tests {
 
     #[test]
     fn test_proxy_rpc_client_disabled() -> Result<()> {
-        let (rpc, rx) = make_aborting_rpc("test-client", "test-server");
-        let reply = send_rpc(rpc, rx, "test-client", "test-server", false);
+        let (rpc, rx) = make_aborting_rpc(TEST_CLIENT, TEST_SERVER);
+        let reply = send_rpc(rpc, rx, TEST_CLIENT, TEST_SERVER, false);
         let err =
             reply.expect_err("Network should check if client is disabled");
         assert_eq!(std::io::ErrorKind::BrokenPipe, err.kind());
@@ -438,8 +443,8 @@ mod tests {
 
     #[test]
     fn test_proxy_rpc_no_such_client() -> Result<()> {
-        let (rpc, rx) = make_aborting_rpc("non-test-client", "test-server");
-        let reply = send_rpc(rpc, rx, "test-client", "test-server", true);
+        let (rpc, rx) = make_aborting_rpc(NON_CLIENT, TEST_SERVER);
+        let reply = send_rpc(rpc, rx, TEST_CLIENT, TEST_SERVER, true);
         let err = reply.expect_err("Network should check client names");
         assert_eq!(std::io::ErrorKind::NotConnected, err.kind());
 

src/server.rs

@@ -112,7 +112,10 @@ impl Server {
 
 #[cfg(test)]
 mod tests {
-    use crate::test_utils::junk_server::{make_server, EchoRpcHandler};
+    use crate::test_utils::junk_server::{
+        make_test_server, EchoRpcHandler,
+        JunkRpcs::{Aborting, Echo},
+    };
 
     use super::*;
 
@@ -127,7 +130,7 @@ mod tests {
 
     #[test]
     fn test_register_rpc_handler() -> Result<()> {
-        let server = make_server();
+        let server = make_test_server();
 
         assert_eq!(2, rpc_handlers_len(server.as_ref()));
         Ok(())
@@ -135,7 +138,7 @@ mod tests {
 
     #[test]
     fn test_register_rpc_handler_failure() -> Result<()> {
-        let mut server = make_server();
+        let mut server = make_test_server();
         let server = std::sync::Arc::get_mut(&mut server)
             .expect("Server should only be held by the current thread");
 
@@ -151,7 +154,7 @@ mod tests {
 
     #[test]
     fn test_serve_rpc() -> Result<()> {
-        let server = make_server();
+        let server = make_test_server();
 
         let reply = server.dispatch(
             "echo".to_string(),
@@ -165,7 +168,7 @@ mod tests {
 
     #[test]
     fn test_rpc_not_found() -> Result<()> {
-        let server = make_server();
+        let server = make_test_server();
 
         let reply = server.dispatch("acorn".to_string(), RequestMessage::new());
         match futures::executor::block_on(reply) {
@@ -177,10 +180,10 @@ mod tests {
 
     #[test]
     fn test_rpc_error() -> Result<()> {
-        let server = make_server();
+        let server = make_test_server();
 
         let reply = futures::executor::block_on(
-            server.dispatch("aborting".to_string(), RequestMessage::new()),
+            server.dispatch(Aborting.name(), RequestMessage::new()),
         );
 
         assert_eq!(
@@ -196,21 +199,18 @@ mod tests {
 
     #[test]
     fn test_server_survives_3_rpc_errors() -> Result<()> {
-        let server = make_server();
+        let server = make_test_server();
 
         // TODO(ditsing): server hangs after the 4th RPC error.
         for _ in 0..3 {
             let server_clone = server.clone();
             let _ = futures::executor::block_on(
-                server_clone
-                    .dispatch("aborting".to_string(), RequestMessage::new()),
+                server_clone.dispatch(Aborting.name(), RequestMessage::new()),
             );
         }
 
-        let reply = server.dispatch(
-            "echo".to_string(),
-            RequestMessage::from_static(&[0x08, 0x07]),
-        );
+        let reply = server
+            .dispatch(Echo.name(), RequestMessage::from_static(&[0x08, 0x07]));
         let result = futures::executor::block_on(reply)?;
 
         assert_eq!(ReplyMessage::from_static(&[0x07, 0x08]), result);

src/test_utils/junk_server.rs

@@ -2,6 +2,29 @@ use std::sync::Arc;
 
 use crate::{server::RpcHandler, ReplyMessage, RequestMessage, Server};
 
+pub const TEST_SERVER: &str = &"test-server";
+pub const NON_SERVER: &str = &"non-server";
+
+pub const TEST_CLIENT: &str = &"test-client";
+pub const NON_CLIENT: &str = &"non-client";
+
+pub enum JunkRpcs {
+    Echo,
+    Aborting,
+    Woods,
+}
+
+impl JunkRpcs {
+    pub fn name(&self) -> String {
+        match *self {
+            Self::Echo => "echo",
+            Self::Aborting => "aborting",
+            Self::Woods => "woods",
+        }
+        .into()
+    }
+}
+
 pub struct EchoRpcHandler {}
 
 impl RpcHandler for EchoRpcHandler {
@@ -20,14 +43,17 @@ impl RpcHandler for AbortingRpcHandler {
     }
 }
 
-pub fn make_server() -> Arc<Server> {
-    let mut server = Server::make_server("test-server".to_string());
+pub fn make_test_server() -> Arc<Server> {
+    let mut server = Server::make_server(TEST_SERVER.into());
     server
-        .register_rpc_handler("echo".to_string(), Box::new(EchoRpcHandler {}))
+        .register_rpc_handler(
+            JunkRpcs::Echo.name(),
+            Box::new(EchoRpcHandler {}),
+        )
         .expect("Registering the first RPC handler should not fail");
     server
         .register_rpc_handler(
-            "aborting".to_string(),
+            JunkRpcs::Aborting.name(),
             Box::new(AbortingRpcHandler {}),
         )
         .expect("Registering the second RPC handler should not fail");

src/test_utils/mod.rs

@@ -8,7 +8,7 @@ pub(crate) fn make_aborting_rpc<C: Into<String>, S: Into<String>>(
     client: C,
     server: S,
 ) -> (RpcOnWire, Receiver<Result<ReplyMessage>>) {
-    make_rpc(client, server, "aborting", &[])
+    make_rpc(client, server, junk_server::JunkRpcs::Aborting, &[])
 }
 
 pub(crate) fn make_echo_rpc<C: Into<String>, S: Into<String>>(
@@ -16,13 +16,13 @@ pub(crate) fn make_echo_rpc<C: Into<String>, S: Into<String>>(
     server: S,
     data: &[u8],
 ) -> (RpcOnWire, Receiver<Result<ReplyMessage>>) {
-    make_rpc(client, server, "echo", data)
+    make_rpc(client, server, junk_server::JunkRpcs::Echo, data)
 }
 
-pub(crate) fn make_rpc<C: Into<String>, S: Into<String>, M: Into<String>>(
+pub(crate) fn make_rpc<C: Into<String>, S: Into<String>>(
     client: C,
     server: S,
-    service_method: M,
+    service_method: junk_server::JunkRpcs,
     data: &[u8],
 ) -> (RpcOnWire, Receiver<Result<ReplyMessage>>) {
     let (tx, rx) = futures::channel::oneshot::channel();
@@ -30,7 +30,7 @@ pub(crate) fn make_rpc<C: Into<String>, S: Into<String>, M: Into<String>>(
         RpcOnWire {
             client: client.into(),
             server: server.into(),
-            service_method: service_method.into(),
+            service_method: service_method.name(),
             request: RequestMessage::copy_from_slice(data),
             reply_channel: tx,
         },