From 8636a7f198cdfb0537d807b733a0a29091062d86 Mon Sep 17 00:00:00 2001 From: Josh Thomas Date: Tue, 10 Dec 2024 20:27:21 -0600 Subject: [PATCH] add integration tests for retries, fix bugs that surfaced (#18) --- crates/djls-ipc/src/client.rs | 42 +++++++++--------- crates/djls-ipc/src/server.rs | 9 ++-- crates/djls-ipc/tests/integration.rs | 66 ++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 26 deletions(-) diff --git a/crates/djls-ipc/src/client.rs b/crates/djls-ipc/src/client.rs index 9b1bcf1..b2f7dd4 100644 --- a/crates/djls-ipc/src/client.rs +++ b/crates/djls-ipc/src/client.rs @@ -1,10 +1,13 @@ use anyhow::{Context, Result}; use async_trait::async_trait; use serde::{Deserialize, Serialize}; -use std::{path::Path, time::Duration}; +use std::{ + path::{Path, PathBuf}, + time::Duration, +}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] pub(crate) struct ConnectionConfig { max_retries: u32, initial_delay_ms: u64, @@ -103,6 +106,7 @@ pub struct Message { pub struct Client { connection: Box, message_id: u64, + socket_path: PathBuf, } impl Client { @@ -111,6 +115,7 @@ impl Client { Ok(Self { connection, message_id: 0, + socket_path: path.to_owned(), }) } @@ -378,16 +383,21 @@ mod client_tests { value: String, } + fn create_test_client(mock_conn: MockConnection) -> Client { + Client { + connection: Box::new(mock_conn), + message_id: 0, + socket_path: PathBuf::from("/test/socket"), + } + } + #[tokio::test] async fn test_successful_message_exchange() -> Result<()> { let mock_conn = MockConnection::new(vec![Ok( r#"{"id":1,"content":{"value":"response"}}"#.to_string() )]); - let mut client = Client { - connection: Box::new(mock_conn), - message_id: 0, - }; + let mut client = create_test_client(mock_conn); let request = TestMessage { value: "test".to_string(), @@ -403,10 +413,7 @@ mod client_tests { async fn test_connection_error() { let mock_conn = MockConnection::new(vec![Err(anyhow::anyhow!("Connection error"))]); - let mut client = Client { - connection: Box::new(mock_conn), - message_id: 0, - }; + let mut client = create_test_client(mock_conn); let request = TestMessage { value: "test".to_string(), @@ -422,10 +429,7 @@ mod client_tests { r#"{"id":2,"content":{"value":"response"}}"#.to_string() )]); - let mut client = Client { - connection: Box::new(mock_conn), - message_id: 0, - }; + let mut client = create_test_client(mock_conn); let request = TestMessage { value: "test".to_string(), @@ -443,10 +447,7 @@ mod client_tests { async fn test_invalid_json_response() { let mock_conn = MockConnection::new(vec![Ok("invalid json".to_string())]); - let mut client = Client { - connection: Box::new(mock_conn), - message_id: 0, - }; + let mut client = create_test_client(mock_conn); let request = TestMessage { value: "test".to_string(), @@ -462,10 +463,7 @@ mod client_tests { Ok(r#"{"id":2,"content":{"value":"response2"}}"#.to_string()), ]); - let mut client = Client { - connection: Box::new(mock_conn), - message_id: 0, - }; + let mut client = create_test_client(mock_conn); let request1 = TestMessage { value: "test1".to_string(), diff --git a/crates/djls-ipc/src/server.rs b/crates/djls-ipc/src/server.rs index 4656841..346e73d 100644 --- a/crates/djls-ipc/src/server.rs +++ b/crates/djls-ipc/src/server.rs @@ -24,10 +24,11 @@ impl Server { fn start_with_options(python_path: &str, args: &[&str], use_module: bool) -> Result { let temp_dir = tempdir()?; - let path = { - let socket_path = temp_dir.path().join("ipc.sock"); - socket_path - }; + let path = args + .windows(2) + .find(|w| w[0] == "--ipc-path") + .map(|w| PathBuf::from(w[1])) + .unwrap_or_else(|| temp_dir.path().join("ipc.sock")); let mut command = Command::new("python"); if use_module { diff --git a/crates/djls-ipc/tests/integration.rs b/crates/djls-ipc/tests/integration.rs index 0ea146a..09aa290 100644 --- a/crates/djls-ipc/tests/integration.rs +++ b/crates/djls-ipc/tests/integration.rs @@ -1,3 +1,8 @@ +use std::{ + path::Path, + time::{Duration, Instant}, +}; + use anyhow::Result; use djls_ipc::{Client, Server}; use serde::{Deserialize, Serialize}; @@ -139,3 +144,64 @@ async fn test_rapid_messages() -> Result<()> { Ok(()) } + +#[tokio::test] +async fn test_connect_with_delayed_server() -> Result<()> { + let path = format!("{}/echo_server.py", FIXTURES_PATH); + let server_path = Path::new(&path).to_owned(); + + // Start server with a shorter delay + let server_handle = tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(50)).await; + Server::start_script(server_path.to_str().unwrap(), &[]) + }); + + // Wait for server to start + let server = server_handle.await??; + let mut client = Client::connect(server.get_path()).await?; + + // Test the connection works + let msg = "test".to_string(); + let response: String = client.send(msg.clone()).await?; + assert_eq!(response, msg); + + Ok(()) +} + +#[tokio::test] +async fn test_connect_with_server_restart() -> Result<()> { + let temp_dir = tempfile::tempdir()?; + let socket_path = temp_dir.path().join("ipc.sock"); + + // Start first server + let path = format!("{}/echo_server.py", FIXTURES_PATH); + let server = Server::start_script(&path, &["--ipc-path", socket_path.to_str().unwrap()])?; + + let mut client = Client::connect(&socket_path).await?; + + let msg = "test".to_string(); + let response: String = client.send(msg.clone()).await?; + assert_eq!(response, msg); + + // Drop old server and client + drop(server); + drop(client); + tokio::time::sleep(Duration::from_millis(50)).await; + + // Start new server + let new_server = Server::start_script(&path, &["--ipc-path", socket_path.to_str().unwrap()])?; + println!( + "Second server started, socket path: {:?}", + new_server.get_path() + ); + + // Create new client + let mut new_client = Client::connect(&socket_path).await?; + + // Try to send a message + let msg = "test after restart".to_string(); + let response: String = new_client.send(msg.clone()).await?; + assert_eq!(response, msg); + + Ok(()) +}