From 5ee87739c7d07f4722017f2b74e4d32f559afe06 Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 13:03:18 +0100 Subject: [PATCH 01/44] test: added scoped fixtures and e2e tests Co-authored-by: zaaarf Co-authored-by: frelodev --- Cargo.toml | 2 + src/lib.rs | 3 + src/tests/e2e.rs | 417 +++++++++++++++++++++++++++++++++++++++++++++++ src/tests/mod.rs | 2 + 4 files changed, 424 insertions(+) create mode 100644 src/tests/e2e.rs create mode 100644 src/tests/mod.rs diff --git a/Cargo.toml b/Cargo.toml index 4ec0fa1..f5a1f49 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,6 +68,8 @@ default = [] # extra async-trait = ["dep:async-trait"] serialize = ["dep:serde", "uuid/serde"] +# special tests which require more setup +test-e2e = [] # ffi java = ["lazy_static", "jni", "tracing-subscriber", "jni-toolbox"] js = ["napi-build", "tracing-subscriber", "napi", "napi-derive"] diff --git a/src/lib.rs b/src/lib.rs index 3179b7f..7177d8f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -125,6 +125,9 @@ pub mod ext; /// language-specific ffi "glue" pub mod ffi; +#[cfg(any(feature = "test-e2e", test))] +pub mod tests; + /// internal network services and interceptors pub(crate) mod network; diff --git a/src/tests/e2e.rs b/src/tests/e2e.rs new file mode 100644 index 0000000..e2ba7d2 --- /dev/null +++ b/src/tests/e2e.rs @@ -0,0 +1,417 @@ +use std::{error::Error, fmt::Display, future::Future}; + +use crate::api::{AsyncReceiver, AsyncSender}; + +#[derive(Debug)] +pub struct AssertionError(String); +impl AssertionError { + fn new(msg: &str) -> Self { + Self(msg.to_string()) + } +} +impl Display for AssertionError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} +impl std::error::Error for AssertionError {} + +#[allow(async_fn_in_trait)] +pub trait ScopedFixture { + async fn setup(&mut self) -> Result>; + + async fn cleanup(&mut self, resource: Option) { + drop(resource) + } + + async fn inner_with(mut self, cb: impl FnOnce(&mut T) -> F) -> Result<(), Box> + where + Self: Sized, + F: Future>>, + { + match self.setup().await { + Ok(mut t) => { + let res = cb(&mut t).await; + self.cleanup(Some(t)).await; + res + } + Err(e) => { + self.cleanup(None).await; + Err(e) + } + } + } + + async fn with(self, cb: impl FnOnce(&mut T) -> F) + where + Self: Sized, + F: Future>>, + { + if let Err(e) = self.inner_with(cb).await { + panic!("{e}"); + } + } +} + +macro_rules! assert_or_err { + ($s:expr) => { + if !$s { + return Err(AssertionError::new(&format!( + "assertion failed at line {}: {}", + std::line!(), + stringify!($s) + )) + .into()); + } + }; + ($s:expr, $msg:literal) => { + if !$s { + return Err(AssertionError::new($msg).into()); + } + }; +} + +struct ClientFixture { + name: String, + username: Option, + password: Option, +} +impl ClientFixture { + fn of(name: &str) -> Self { + Self { + name: name.to_string(), + username: None, + password: None, + } + } +} + +impl ScopedFixture for ClientFixture { + async fn setup(&mut self) -> Result> { + let upper = self.name.to_uppercase(); + let username = self.username.clone().unwrap_or_else(|| { + std::env::var(format!("CODEMP_TEST_USERNAME_{upper}")).unwrap_or_default() + }); + let password = self.password.clone().unwrap_or_else(|| { + std::env::var(format!("CODEMP_TEST_PASSWORD_{upper}")).unwrap_or_default() + }); + let client = crate::Client::connect(crate::api::Config { + username, + password, + tls: Some(false), + ..Default::default() + }) + .await?; + + Ok(client) + } +} + +struct WorkspaceFixture { + user: String, + invite: Option, + workspace: String, +} +impl WorkspaceFixture { + #[allow(unused)] + fn of(user: &str, invite: &str, workspace: &str) -> Self { + Self { + user: user.to_string(), + invite: Some(invite.to_string()), + workspace: workspace.to_string(), + } + } + + fn any(user: &str) -> Self { + Self { + user: user.to_string(), + invite: None, + workspace: uuid::Uuid::new_v4().to_string(), + } + } + + fn two(user: &str, invite: &str) -> Self { + Self { + user: user.to_string(), + invite: Some(invite.to_string()), + workspace: uuid::Uuid::new_v4().to_string(), + } + } +} + +impl ScopedFixture<(crate::Client, crate::Workspace)> for WorkspaceFixture { + async fn setup(&mut self) -> Result<(crate::Client, crate::Workspace), Box> { + let client = ClientFixture::of(&self.user).setup().await?; + client.create_workspace(&self.workspace).await?; + let workspace = client.attach_workspace(&self.workspace).await?; + Ok((client, workspace)) + } + + async fn cleanup(&mut self, resource: Option<(crate::Client, crate::Workspace)>) { + if let Some((client, _workspace)) = resource { + client.leave_workspace(&self.workspace); + if let Err(e) = client.delete_workspace(&self.workspace).await { + eprintln!("could not delete workspace: {e}"); + } + } + } +} + +impl ScopedFixture<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))> for WorkspaceFixture { + async fn setup(&mut self) -> Result<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace)), Box> { + let client = ClientFixture::of(&self.user).setup().await?; + let invite_client = ClientFixture::of( + &self + .invite + .clone() + .unwrap_or(uuid::Uuid::new_v4().to_string()), + ) + .setup() + .await?; + client.create_workspace(&self.workspace).await?; + client + .invite_to_workspace(&self.workspace, invite_client.current_user().name.clone()) + .await?; + let workspace = client.attach_workspace(&self.workspace).await?; + let invite = invite_client.attach_workspace(&self.workspace).await?; + Ok(((client, workspace), (invite_client, invite))) + } + + async fn cleanup(&mut self, resource: Option<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))>) { + if let Some(((client, _workspace), (_, _))) = resource { + client.leave_workspace(&self.workspace); + if let Err(e) = client.delete_workspace(&self.workspace).await { + eprintln!("could not delete workspace: {e}"); + } + } + } +} + +#[tokio::test] +async fn test_workspace_interactions() { + if let Err(e) = async { + let client_alice = ClientFixture::of("alice").setup().await?; + let client_bob = ClientFixture::of("bob").setup().await?; + let workspace_name = uuid::Uuid::new_v4().to_string(); + + client_alice.create_workspace(&workspace_name).await?; + let owned_workspaces = client_alice.fetch_owned_workspaces().await?; + assert_or_err!(owned_workspaces.contains(&workspace_name)); + client_alice.attach_workspace(&workspace_name).await?; + assert_or_err!(vec![workspace_name.clone()] == client_alice.active_workspaces()); + + client_alice + .invite_to_workspace(&workspace_name, &client_bob.current_user().name) + .await?; + client_bob.attach_workspace(&workspace_name).await?; + assert_or_err!(client_bob + .fetch_joined_workspaces() + .await? + .contains(&workspace_name)); + + assert_or_err!(client_bob.leave_workspace(&workspace_name)); + assert_or_err!(client_alice.leave_workspace(&workspace_name)); + + client_alice.delete_workspace(&workspace_name).await?; + + Ok::<(), Box>(()) + } + .await + { + panic!("{e}"); + } +} + +// ======= + +#[tokio::test] +async fn cannot_delete_others_workspaces() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, ws_alice), (client_bob, _))| { + let ws_alice = ws_alice.clone(); + let client_bob = client_bob.clone(); + async move { + assert_or_err!( + client_bob.delete_workspace(&ws_alice.id()).await.is_err(), + "bob was allowed to delete a workspace he didn't own!" + ); + Ok(()) + } + + }) + .await +} + +#[tokio::test] +async fn test_cant_create_buffer_twice() { + WorkspaceFixture::any("alice") + .with(|(_, ws): &mut (crate::Client, crate::Workspace)| { + let ws = ws.clone(); + async move { + ws.create_buffer("cacca").await?; + assert!( + ws.create_buffer("cacca").await.is_err(), + "alice could create again the same buffer" + ); + Ok(()) + } + }) + .await; +} + +#[tokio::test] +async fn test_buffer_search() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, workspace_alice), (_, workspace_bob))| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + let workspace_bob = workspace_bob.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!(vec![buffer_name.clone()] == workspace_alice.fetch_buffers().await?); + assert_or_err!(vec![buffer_name.clone()] == workspace_bob.fetch_buffers().await?); + + assert_or_err!(!workspace_alice + .search_buffers(Some(&buffer_name[0..4])) + .is_empty()); + assert_or_err!(workspace_alice.search_buffers(Some("_")).is_empty()); + + workspace_alice.delete_buffer(&buffer_name).await?; + + Ok(()) + } + }) + .await; +} + +#[tokio::test] +async fn cannot_delete_others_buffers() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, workspace_alice), (_, workspace_bob))| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + let workspace_bob = workspace_bob.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!(workspace_bob.delete_buffer(&buffer_name).await.is_err()); + Ok(()) + } + }) + .await; +} + +// ===== + +#[tokio::test] +async fn test_send_operation() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, workspace_alice), (_, workspace_bob))| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + let workspace_bob = workspace_bob.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + let alice = workspace_alice.attach_buffer(&buffer_name).await?; + let bob = workspace_bob.attach_buffer(&buffer_name).await?; + + alice.send(crate::api::TextChange { + start_idx: 0, + end_idx: 0, + content: "hello world".to_string(), + })?; + + let result = bob.recv().await?; + assert_or_err!(result.change.start_idx == 0); + assert_or_err!(result.change.end_idx == 0); + assert_or_err!(result.change.content == "hello world"); + + Ok(()) + } + }) + .await; +} + +#[tokio::test] +async fn test_content_converges() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, workspace_alice), (_, workspace_bob))| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + let workspace_bob = workspace_bob.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + let alice = workspace_alice.attach_buffer(&buffer_name).await?; + let bob = workspace_bob.attach_buffer(&buffer_name).await?; + + let mut join_set = tokio::task::JoinSet::new(); + + let _alice = alice.clone(); + join_set.spawn(async move { + for i in 0..10 { + _alice.content().await?; + _alice.send(crate::api::TextChange { + start_idx: 7 * i, + end_idx: 7 * i, + content: format!("alice{i} "), // TODO generate a random string instead!! + })?; + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + Ok::<(), crate::errors::ControllerError>(()) + }); + + let _bob = bob.clone(); + join_set.spawn(async move { + for i in 0..10 { + _bob.content().await?; + _bob.send(crate::api::TextChange { + start_idx: 5 * i, + end_idx: 5 * i, + content: format!("bob{i} "), // TODO generate a random string instead!! + })?; + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + Ok::<(), crate::errors::ControllerError>(()) + }); + + while let Some(x) = join_set.join_next().await { + x??; + } + + // TODO is there a nicer way to make sure we received all changes? + + for i in 0..100 { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + match bob.try_recv().await? { + Some(change) => bob.ack(change.version), + None => break, + } + eprintln!("bob more to recv at attempt #{i}"); + } + + for i in 0..100 { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + match alice.try_recv().await? { + Some(change) => alice.ack(change.version), + None => break, + } + eprintln!("alice more to recv at attempt #{i}"); + } + + let alice_content = alice.content().await?; + let bob_content = bob.content().await?; + + eprintln!("alice: {alice_content}"); + eprintln!("bob : {bob_content}"); + + assert_or_err!(alice_content == bob_content); + assert_or_err!(false); + + Ok(()) + } + }) + .await; +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs new file mode 100644 index 0000000..f65a2ca --- /dev/null +++ b/src/tests/mod.rs @@ -0,0 +1,2 @@ +#[cfg(feature = "test-e2e")] +pub mod e2e; From 26bbd190ddb0f8a21ba25075a568cc10ea6e9387 Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 13:21:09 +0100 Subject: [PATCH 02/44] test: split down test stuff a bit --- Cargo.toml | 2 +- src/ffi/js/workspace.rs | 2 +- src/tests/client.rs | 134 +++++++++++++ src/tests/e2e.rs | 417 ---------------------------------------- src/tests/fixtures.rs | 155 +++++++++++++++ src/tests/mod.rs | 47 ++++- src/tests/server.rs | 106 ++++++++++ 7 files changed, 442 insertions(+), 421 deletions(-) create mode 100644 src/tests/client.rs delete mode 100644 src/tests/e2e.rs create mode 100644 src/tests/fixtures.rs create mode 100644 src/tests/server.rs diff --git a/Cargo.toml b/Cargo.toml index f5a1f49..e5e1d44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,7 +64,7 @@ napi-build = { version = "2.1", optional = true } pyo3-build-config = { version = "0.22", optional = true } [features] -default = [] +default = ["test-e2e"] # extra async-trait = ["dep:async-trait"] serialize = ["dep:serde", "uuid/serde"] diff --git a/src/ffi/js/workspace.rs b/src/ffi/js/workspace.rs index e862322..d1ba78d 100644 --- a/src/ffi/js/workspace.rs +++ b/src/ffi/js/workspace.rs @@ -121,7 +121,7 @@ impl Workspace { })?; self.callback(move |controller: Workspace| { tsfn.call(controller.clone(), ThreadsafeFunctionCallMode::Blocking); //check this with tracing also we could use Ok(event) to get the error - // If it blocks the main thread too many time we have to change this + // If it blocks the main thread too many time we have to change this }); Ok(()) diff --git a/src/tests/client.rs b/src/tests/client.rs new file mode 100644 index 0000000..0e015ab --- /dev/null +++ b/src/tests/client.rs @@ -0,0 +1,134 @@ +use crate::api::{AsyncReceiver, AsyncSender}; +use super::{assert_or_err, fixtures::{ScopedFixture, WorkspaceFixture}}; + +#[tokio::test] +async fn test_buffer_search() { + WorkspaceFixture::one("alice") + .with(|(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!(!workspace_alice + .search_buffers(Some(&buffer_name[0..4])) + .is_empty()); + assert_or_err!(workspace_alice.search_buffers(Some("_")).is_empty()); + workspace_alice.delete_buffer(&buffer_name).await?; + Ok(()) + } + }) + .await; +} + +#[tokio::test] +async fn test_send_operation() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, workspace_alice), (_, workspace_bob))| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + let workspace_bob = workspace_bob.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + let alice = workspace_alice.attach_buffer(&buffer_name).await?; + let bob = workspace_bob.attach_buffer(&buffer_name).await?; + + alice.send(crate::api::TextChange { + start_idx: 0, + end_idx: 0, + content: "hello world".to_string(), + })?; + + let result = bob.recv().await?; + assert_or_err!(result.change.start_idx == 0); + assert_or_err!(result.change.end_idx == 0); + assert_or_err!(result.change.content == "hello world"); + + Ok(()) + } + }) + .await; +} + +#[tokio::test] +async fn test_content_converges() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, workspace_alice), (_, workspace_bob))| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + let workspace_bob = workspace_bob.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + let alice = workspace_alice.attach_buffer(&buffer_name).await?; + let bob = workspace_bob.attach_buffer(&buffer_name).await?; + + let mut join_set = tokio::task::JoinSet::new(); + + let _alice = alice.clone(); + join_set.spawn(async move { + for i in 0..10 { + _alice.content().await?; + _alice.send(crate::api::TextChange { + start_idx: 7 * i, + end_idx: 7 * i, + content: format!("alice{i} "), // TODO generate a random string instead!! + })?; + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + Ok::<(), crate::errors::ControllerError>(()) + }); + + let _bob = bob.clone(); + join_set.spawn(async move { + for i in 0..10 { + _bob.content().await?; + _bob.send(crate::api::TextChange { + start_idx: 5 * i, + end_idx: 5 * i, + content: format!("bob{i} "), // TODO generate a random string instead!! + })?; + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + Ok::<(), crate::errors::ControllerError>(()) + }); + + while let Some(x) = join_set.join_next().await { + x??; + } + + // TODO is there a nicer way to make sure we received all changes? + + for i in 0..100 { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + match bob.try_recv().await? { + Some(change) => bob.ack(change.version), + None => break, + } + eprintln!("bob more to recv at attempt #{i}"); + } + + for i in 0..100 { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + match alice.try_recv().await? { + Some(change) => alice.ack(change.version), + None => break, + } + eprintln!("alice more to recv at attempt #{i}"); + } + + let alice_content = alice.content().await?; + let bob_content = bob.content().await?; + + eprintln!("alice: {alice_content}"); + eprintln!("bob : {bob_content}"); + + assert_or_err!(alice_content == bob_content); + assert_or_err!(false); + + Ok(()) + } + }) + .await; +} diff --git a/src/tests/e2e.rs b/src/tests/e2e.rs deleted file mode 100644 index e2ba7d2..0000000 --- a/src/tests/e2e.rs +++ /dev/null @@ -1,417 +0,0 @@ -use std::{error::Error, fmt::Display, future::Future}; - -use crate::api::{AsyncReceiver, AsyncSender}; - -#[derive(Debug)] -pub struct AssertionError(String); -impl AssertionError { - fn new(msg: &str) -> Self { - Self(msg.to_string()) - } -} -impl Display for AssertionError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} -impl std::error::Error for AssertionError {} - -#[allow(async_fn_in_trait)] -pub trait ScopedFixture { - async fn setup(&mut self) -> Result>; - - async fn cleanup(&mut self, resource: Option) { - drop(resource) - } - - async fn inner_with(mut self, cb: impl FnOnce(&mut T) -> F) -> Result<(), Box> - where - Self: Sized, - F: Future>>, - { - match self.setup().await { - Ok(mut t) => { - let res = cb(&mut t).await; - self.cleanup(Some(t)).await; - res - } - Err(e) => { - self.cleanup(None).await; - Err(e) - } - } - } - - async fn with(self, cb: impl FnOnce(&mut T) -> F) - where - Self: Sized, - F: Future>>, - { - if let Err(e) = self.inner_with(cb).await { - panic!("{e}"); - } - } -} - -macro_rules! assert_or_err { - ($s:expr) => { - if !$s { - return Err(AssertionError::new(&format!( - "assertion failed at line {}: {}", - std::line!(), - stringify!($s) - )) - .into()); - } - }; - ($s:expr, $msg:literal) => { - if !$s { - return Err(AssertionError::new($msg).into()); - } - }; -} - -struct ClientFixture { - name: String, - username: Option, - password: Option, -} -impl ClientFixture { - fn of(name: &str) -> Self { - Self { - name: name.to_string(), - username: None, - password: None, - } - } -} - -impl ScopedFixture for ClientFixture { - async fn setup(&mut self) -> Result> { - let upper = self.name.to_uppercase(); - let username = self.username.clone().unwrap_or_else(|| { - std::env::var(format!("CODEMP_TEST_USERNAME_{upper}")).unwrap_or_default() - }); - let password = self.password.clone().unwrap_or_else(|| { - std::env::var(format!("CODEMP_TEST_PASSWORD_{upper}")).unwrap_or_default() - }); - let client = crate::Client::connect(crate::api::Config { - username, - password, - tls: Some(false), - ..Default::default() - }) - .await?; - - Ok(client) - } -} - -struct WorkspaceFixture { - user: String, - invite: Option, - workspace: String, -} -impl WorkspaceFixture { - #[allow(unused)] - fn of(user: &str, invite: &str, workspace: &str) -> Self { - Self { - user: user.to_string(), - invite: Some(invite.to_string()), - workspace: workspace.to_string(), - } - } - - fn any(user: &str) -> Self { - Self { - user: user.to_string(), - invite: None, - workspace: uuid::Uuid::new_v4().to_string(), - } - } - - fn two(user: &str, invite: &str) -> Self { - Self { - user: user.to_string(), - invite: Some(invite.to_string()), - workspace: uuid::Uuid::new_v4().to_string(), - } - } -} - -impl ScopedFixture<(crate::Client, crate::Workspace)> for WorkspaceFixture { - async fn setup(&mut self) -> Result<(crate::Client, crate::Workspace), Box> { - let client = ClientFixture::of(&self.user).setup().await?; - client.create_workspace(&self.workspace).await?; - let workspace = client.attach_workspace(&self.workspace).await?; - Ok((client, workspace)) - } - - async fn cleanup(&mut self, resource: Option<(crate::Client, crate::Workspace)>) { - if let Some((client, _workspace)) = resource { - client.leave_workspace(&self.workspace); - if let Err(e) = client.delete_workspace(&self.workspace).await { - eprintln!("could not delete workspace: {e}"); - } - } - } -} - -impl ScopedFixture<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))> for WorkspaceFixture { - async fn setup(&mut self) -> Result<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace)), Box> { - let client = ClientFixture::of(&self.user).setup().await?; - let invite_client = ClientFixture::of( - &self - .invite - .clone() - .unwrap_or(uuid::Uuid::new_v4().to_string()), - ) - .setup() - .await?; - client.create_workspace(&self.workspace).await?; - client - .invite_to_workspace(&self.workspace, invite_client.current_user().name.clone()) - .await?; - let workspace = client.attach_workspace(&self.workspace).await?; - let invite = invite_client.attach_workspace(&self.workspace).await?; - Ok(((client, workspace), (invite_client, invite))) - } - - async fn cleanup(&mut self, resource: Option<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))>) { - if let Some(((client, _workspace), (_, _))) = resource { - client.leave_workspace(&self.workspace); - if let Err(e) = client.delete_workspace(&self.workspace).await { - eprintln!("could not delete workspace: {e}"); - } - } - } -} - -#[tokio::test] -async fn test_workspace_interactions() { - if let Err(e) = async { - let client_alice = ClientFixture::of("alice").setup().await?; - let client_bob = ClientFixture::of("bob").setup().await?; - let workspace_name = uuid::Uuid::new_v4().to_string(); - - client_alice.create_workspace(&workspace_name).await?; - let owned_workspaces = client_alice.fetch_owned_workspaces().await?; - assert_or_err!(owned_workspaces.contains(&workspace_name)); - client_alice.attach_workspace(&workspace_name).await?; - assert_or_err!(vec![workspace_name.clone()] == client_alice.active_workspaces()); - - client_alice - .invite_to_workspace(&workspace_name, &client_bob.current_user().name) - .await?; - client_bob.attach_workspace(&workspace_name).await?; - assert_or_err!(client_bob - .fetch_joined_workspaces() - .await? - .contains(&workspace_name)); - - assert_or_err!(client_bob.leave_workspace(&workspace_name)); - assert_or_err!(client_alice.leave_workspace(&workspace_name)); - - client_alice.delete_workspace(&workspace_name).await?; - - Ok::<(), Box>(()) - } - .await - { - panic!("{e}"); - } -} - -// ======= - -#[tokio::test] -async fn cannot_delete_others_workspaces() { - WorkspaceFixture::two("alice", "bob") - .with(|((_, ws_alice), (client_bob, _))| { - let ws_alice = ws_alice.clone(); - let client_bob = client_bob.clone(); - async move { - assert_or_err!( - client_bob.delete_workspace(&ws_alice.id()).await.is_err(), - "bob was allowed to delete a workspace he didn't own!" - ); - Ok(()) - } - - }) - .await -} - -#[tokio::test] -async fn test_cant_create_buffer_twice() { - WorkspaceFixture::any("alice") - .with(|(_, ws): &mut (crate::Client, crate::Workspace)| { - let ws = ws.clone(); - async move { - ws.create_buffer("cacca").await?; - assert!( - ws.create_buffer("cacca").await.is_err(), - "alice could create again the same buffer" - ); - Ok(()) - } - }) - .await; -} - -#[tokio::test] -async fn test_buffer_search() { - WorkspaceFixture::two("alice", "bob") - .with(|((_, workspace_alice), (_, workspace_bob))| { - let buffer_name = uuid::Uuid::new_v4().to_string(); - let workspace_alice = workspace_alice.clone(); - let workspace_bob = workspace_bob.clone(); - - async move { - workspace_alice.create_buffer(&buffer_name).await?; - assert_or_err!(vec![buffer_name.clone()] == workspace_alice.fetch_buffers().await?); - assert_or_err!(vec![buffer_name.clone()] == workspace_bob.fetch_buffers().await?); - - assert_or_err!(!workspace_alice - .search_buffers(Some(&buffer_name[0..4])) - .is_empty()); - assert_or_err!(workspace_alice.search_buffers(Some("_")).is_empty()); - - workspace_alice.delete_buffer(&buffer_name).await?; - - Ok(()) - } - }) - .await; -} - -#[tokio::test] -async fn cannot_delete_others_buffers() { - WorkspaceFixture::two("alice", "bob") - .with(|((_, workspace_alice), (_, workspace_bob))| { - let buffer_name = uuid::Uuid::new_v4().to_string(); - let workspace_alice = workspace_alice.clone(); - let workspace_bob = workspace_bob.clone(); - - async move { - workspace_alice.create_buffer(&buffer_name).await?; - assert_or_err!(workspace_bob.delete_buffer(&buffer_name).await.is_err()); - Ok(()) - } - }) - .await; -} - -// ===== - -#[tokio::test] -async fn test_send_operation() { - WorkspaceFixture::two("alice", "bob") - .with(|((_, workspace_alice), (_, workspace_bob))| { - let buffer_name = uuid::Uuid::new_v4().to_string(); - let workspace_alice = workspace_alice.clone(); - let workspace_bob = workspace_bob.clone(); - - async move { - workspace_alice.create_buffer(&buffer_name).await?; - let alice = workspace_alice.attach_buffer(&buffer_name).await?; - let bob = workspace_bob.attach_buffer(&buffer_name).await?; - - alice.send(crate::api::TextChange { - start_idx: 0, - end_idx: 0, - content: "hello world".to_string(), - })?; - - let result = bob.recv().await?; - assert_or_err!(result.change.start_idx == 0); - assert_or_err!(result.change.end_idx == 0); - assert_or_err!(result.change.content == "hello world"); - - Ok(()) - } - }) - .await; -} - -#[tokio::test] -async fn test_content_converges() { - WorkspaceFixture::two("alice", "bob") - .with(|((_, workspace_alice), (_, workspace_bob))| { - let buffer_name = uuid::Uuid::new_v4().to_string(); - let workspace_alice = workspace_alice.clone(); - let workspace_bob = workspace_bob.clone(); - - async move { - workspace_alice.create_buffer(&buffer_name).await?; - let alice = workspace_alice.attach_buffer(&buffer_name).await?; - let bob = workspace_bob.attach_buffer(&buffer_name).await?; - - let mut join_set = tokio::task::JoinSet::new(); - - let _alice = alice.clone(); - join_set.spawn(async move { - for i in 0..10 { - _alice.content().await?; - _alice.send(crate::api::TextChange { - start_idx: 7 * i, - end_idx: 7 * i, - content: format!("alice{i} "), // TODO generate a random string instead!! - })?; - tokio::time::sleep(std::time::Duration::from_millis(100)).await; - } - Ok::<(), crate::errors::ControllerError>(()) - }); - - let _bob = bob.clone(); - join_set.spawn(async move { - for i in 0..10 { - _bob.content().await?; - _bob.send(crate::api::TextChange { - start_idx: 5 * i, - end_idx: 5 * i, - content: format!("bob{i} "), // TODO generate a random string instead!! - })?; - tokio::time::sleep(std::time::Duration::from_millis(100)).await; - } - Ok::<(), crate::errors::ControllerError>(()) - }); - - while let Some(x) = join_set.join_next().await { - x??; - } - - // TODO is there a nicer way to make sure we received all changes? - - for i in 0..100 { - tokio::time::sleep(std::time::Duration::from_millis(50)).await; - match bob.try_recv().await? { - Some(change) => bob.ack(change.version), - None => break, - } - eprintln!("bob more to recv at attempt #{i}"); - } - - for i in 0..100 { - tokio::time::sleep(std::time::Duration::from_millis(50)).await; - match alice.try_recv().await? { - Some(change) => alice.ack(change.version), - None => break, - } - eprintln!("alice more to recv at attempt #{i}"); - } - - let alice_content = alice.content().await?; - let bob_content = bob.content().await?; - - eprintln!("alice: {alice_content}"); - eprintln!("bob : {bob_content}"); - - assert_or_err!(alice_content == bob_content); - assert_or_err!(false); - - Ok(()) - } - }) - .await; -} diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs new file mode 100644 index 0000000..9f2aebc --- /dev/null +++ b/src/tests/fixtures.rs @@ -0,0 +1,155 @@ +use std::{error::Error, future::Future}; + +// TODO create a BufferFixture too + +#[allow(async_fn_in_trait)] +pub trait ScopedFixture { + async fn setup(&mut self) -> Result>; + + async fn cleanup(&mut self, resource: Option) { + drop(resource) + } + + async fn inner_with(mut self, cb: impl FnOnce(&mut T) -> F) -> Result<(), Box> + where + Self: Sized, + F: Future>>, + { + match self.setup().await { + Ok(mut t) => { + let res = cb(&mut t).await; + self.cleanup(Some(t)).await; + res + } + Err(e) => { + self.cleanup(None).await; + Err(e) + } + } + } + + async fn with(self, cb: impl FnOnce(&mut T) -> F) + where + Self: Sized, + F: Future>>, + { + if let Err(e) = self.inner_with(cb).await { + panic!("{e}"); + } + } +} + +pub struct ClientFixture { + name: String, + username: Option, + password: Option, +} +impl ClientFixture { + pub fn of(name: &str) -> Self { + Self { + name: name.to_string(), + username: None, + password: None, + } + } +} + +impl ScopedFixture for ClientFixture { + async fn setup(&mut self) -> Result> { + let upper = self.name.to_uppercase(); + let username = self.username.clone().unwrap_or_else(|| { + std::env::var(format!("CODEMP_TEST_USERNAME_{upper}")).unwrap_or_default() + }); + let password = self.password.clone().unwrap_or_else(|| { + std::env::var(format!("CODEMP_TEST_PASSWORD_{upper}")).unwrap_or_default() + }); + let client = crate::Client::connect(crate::api::Config { + username, + password, + tls: Some(false), + ..Default::default() + }) + .await?; + + Ok(client) + } +} + +pub struct WorkspaceFixture { + user: String, + invite: Option, + workspace: String, +} +impl WorkspaceFixture { + pub fn of(user: &str, invite: &str, workspace: &str) -> Self { + Self { + user: user.to_string(), + invite: Some(invite.to_string()), + workspace: workspace.to_string(), + } + } + + pub fn one(user: &str) -> Self { + Self { + user: user.to_string(), + invite: None, + workspace: uuid::Uuid::new_v4().to_string(), + } + } + + pub fn two(user: &str, invite: &str) -> Self { + Self { + user: user.to_string(), + invite: Some(invite.to_string()), + workspace: uuid::Uuid::new_v4().to_string(), + } + } +} + +impl ScopedFixture<(crate::Client, crate::Workspace)> for WorkspaceFixture { + async fn setup(&mut self) -> Result<(crate::Client, crate::Workspace), Box> { + let client = ClientFixture::of(&self.user).setup().await?; + client.create_workspace(&self.workspace).await?; + let workspace = client.attach_workspace(&self.workspace).await?; + Ok((client, workspace)) + } + + async fn cleanup(&mut self, resource: Option<(crate::Client, crate::Workspace)>) { + if let Some((client, _workspace)) = resource { + client.leave_workspace(&self.workspace); + if let Err(e) = client.delete_workspace(&self.workspace).await { + eprintln!("could not delete workspace: {e}"); + } + } + } +} + +impl ScopedFixture<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))> for WorkspaceFixture { + async fn setup(&mut self) -> Result<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace)), Box> { + let client = ClientFixture::of(&self.user).setup().await?; + let invite_client = ClientFixture::of( + &self + .invite + .clone() + .unwrap_or(uuid::Uuid::new_v4().to_string()), + ) + .setup() + .await?; + client.create_workspace(&self.workspace).await?; + client + .invite_to_workspace(&self.workspace, invite_client.current_user().name.clone()) + .await?; + let workspace = client.attach_workspace(&self.workspace).await?; + let invite = invite_client.attach_workspace(&self.workspace).await?; + Ok(((client, workspace), (invite_client, invite))) + } + + async fn cleanup(&mut self, resource: Option<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))>) { + if let Some(((client, _workspace), (_, _))) = resource { + client.leave_workspace(&self.workspace); + if let Err(e) = client.delete_workspace(&self.workspace).await { + eprintln!("could not delete workspace: {e}"); + } + } + } +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index f65a2ca..a1907ca 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,2 +1,45 @@ -#[cfg(feature = "test-e2e")] -pub mod e2e; +#[cfg(all(test, feature = "test-e2e"))] +mod client; + +#[cfg(all(test, feature = "test-e2e"))] +mod server; + +pub mod fixtures; + +#[derive(Debug)] +pub struct AssertionError(String); + +impl AssertionError { + pub fn new(msg: &str) -> Self { + Self(msg.to_string()) + } +} + +impl std::fmt::Display for AssertionError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl std::error::Error for AssertionError {} + +#[macro_export] +macro_rules! assert_or_err { + ($s:expr) => { + if !$s { + return Err($crate::tests::AssertionError::new(&format!( + "assertion failed at line {}: {}", + std::line!(), + stringify!($s) + )) + .into()); + } + }; + ($s:expr, $msg:literal) => { + if !$s { + return Err($crate::tests::AssertionError::new($msg).into()); + } + }; +} + +pub use assert_or_err; diff --git a/src/tests/server.rs b/src/tests/server.rs new file mode 100644 index 0000000..070db87 --- /dev/null +++ b/src/tests/server.rs @@ -0,0 +1,106 @@ +use super::{assert_or_err, fixtures::{ClientFixture, ScopedFixture, WorkspaceFixture}}; + +#[tokio::test] +async fn cannot_delete_others_workspaces() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, ws_alice), (client_bob, _))| { + let ws_alice = ws_alice.clone(); + let client_bob = client_bob.clone(); + async move { + assert_or_err!( + client_bob.delete_workspace(&ws_alice.id()).await.is_err(), + "bob was allowed to delete a workspace he didn't own!" + ); + Ok(()) + } + + }) + .await +} + +#[tokio::test] +async fn test_buffer_create() { + WorkspaceFixture::one("alice") + .with(|(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!(vec![buffer_name.clone()] == workspace_alice.fetch_buffers().await?); + workspace_alice.delete_buffer(&buffer_name).await?; + + Ok(()) + } + }) + .await; +} + +#[tokio::test] +async fn test_cant_create_buffer_twice() { + WorkspaceFixture::one("alice") + .with(|(_, ws): &mut (crate::Client, crate::Workspace)| { + let ws = ws.clone(); + async move { + ws.create_buffer("cacca").await?; + assert!( + ws.create_buffer("cacca").await.is_err(), + "alice could create again the same buffer" + ); + Ok(()) + } + }) + .await; +} + +#[tokio::test] +async fn cannot_delete_others_buffers() { + WorkspaceFixture::two("alice", "bob") + .with(|((_, workspace_alice), (_, workspace_bob))| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + let workspace_bob = workspace_bob.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!(workspace_bob.delete_buffer(&buffer_name).await.is_err()); + Ok(()) + } + }) + .await; +} + +#[tokio::test] // TODO split down this test in smaller checks +async fn test_workspace_interactions() { + if let Err(e) = async { + let client_alice = ClientFixture::of("alice").setup().await?; + let client_bob = ClientFixture::of("bob").setup().await?; + let workspace_name = uuid::Uuid::new_v4().to_string(); + + client_alice.create_workspace(&workspace_name).await?; + let owned_workspaces = client_alice.fetch_owned_workspaces().await?; + assert_or_err!(owned_workspaces.contains(&workspace_name)); + client_alice.attach_workspace(&workspace_name).await?; + assert_or_err!(vec![workspace_name.clone()] == client_alice.active_workspaces()); + + client_alice + .invite_to_workspace(&workspace_name, &client_bob.current_user().name) + .await?; + client_bob.attach_workspace(&workspace_name).await?; + assert_or_err!(client_bob + .fetch_joined_workspaces() + .await? + .contains(&workspace_name)); + + assert_or_err!(client_bob.leave_workspace(&workspace_name)); + assert_or_err!(client_alice.leave_workspace(&workspace_name)); + + client_alice.delete_workspace(&workspace_name).await?; + + Ok::<(), Box>(()) + } + .await + { + panic!("{e}"); + } +} From adaaf86e326319096f8fd25246a1cb6c22ea39ae Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 13:27:25 +0100 Subject: [PATCH 03/44] ci: run functional tests too --- .github/workflows/test.yml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cbe5aee..227c11a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -40,7 +40,7 @@ jobs: toolchain: ${{ matrix.toolchain }} - run: cargo build --release --verbose --features=${{ matrix.features }} - test: + test-unit: runs-on: ubuntu-latest strategy: fail-fast: false @@ -57,3 +57,20 @@ jobs: with: toolchain: ${{ matrix.toolchain }} - run: cargo test --verbose + + test-functional: + runs-on: ubuntu-latest + steps: + - uses: arduino/setup-protoc@v3 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ matrix.toolchain }} + - run: cargo test --verbose --features=test-e2e + env: + CODEMP_TEST_USERNAME_ALICE: ${{ secrets.CODEMP_TEST_USERNAME_ALICE }} + CODEMP_TEST_PASSWORD_ALICE: ${{ secrets.CODEMP_TEST_PASSWORD_ALICE }} + CODEMP_TEST_USERNAME_BOB: ${{ secrets.CODEMP_TEST_USERNAME_BOB }} + CODEMP_TEST_PASSWORD_BOB: ${{ secrets.CODEMP_TEST_PASSWORD_BOB }} From 60b4c3dee9dce06929c28e2f55b78d526fa9b9e2 Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 13:28:21 +0100 Subject: [PATCH 04/44] fix(tests): test-e2e should not be on by default --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index e5e1d44..f5a1f49 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,7 +64,7 @@ napi-build = { version = "2.1", optional = true } pyo3-build-config = { version = "0.22", optional = true } [features] -default = ["test-e2e"] +default = [] # extra async-trait = ["dep:async-trait"] serialize = ["dep:serde", "uuid/serde"] From e0c913b46ddd903015348002e627ece49cb128ca Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 13:31:42 +0100 Subject: [PATCH 05/44] ci(test): only run fn/build tests after unit --- .github/workflows/test.yml | 40 ++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 227c11a..cd2fba8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,7 +12,26 @@ permissions: contents: read jobs: - build: + test-unit: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + toolchain: + - stable + - beta + steps: + - uses: arduino/setup-protoc@v3 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ matrix.toolchain }} + - run: cargo test --verbose + + test-build: + needs: [test-unit] runs-on: ${{ matrix.runner }} strategy: fail-fast: false @@ -40,25 +59,8 @@ jobs: toolchain: ${{ matrix.toolchain }} - run: cargo build --release --verbose --features=${{ matrix.features }} - test-unit: - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - toolchain: - - stable - - beta - steps: - - uses: arduino/setup-protoc@v3 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@master - with: - toolchain: ${{ matrix.toolchain }} - - run: cargo test --verbose - test-functional: + needs: [test-unit] runs-on: ubuntu-latest steps: - uses: arduino/setup-protoc@v3 From cb6c95e1aadae0bce5e07e7c50ae4ef0b0bcffbb Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 13:32:38 +0100 Subject: [PATCH 06/44] ci(test): no toolchain matrix for fn tests --- .github/workflows/test.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cd2fba8..952175b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -68,8 +68,6 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@master - with: - toolchain: ${{ matrix.toolchain }} - run: cargo test --verbose --features=test-e2e env: CODEMP_TEST_USERNAME_ALICE: ${{ secrets.CODEMP_TEST_USERNAME_ALICE }} From 4772af495964deb9db7286868d52a8fa305fb65d Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 13:45:39 +0100 Subject: [PATCH 07/44] ci: more fine-grained ci, less jobs --- .github/workflows/test.yml | 53 +++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 952175b..5f5bde2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,12 +14,23 @@ permissions: jobs: test-unit: runs-on: ubuntu-latest + steps: + - uses: arduino/setup-protoc@v3 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - run: cargo test --verbose + + test-beta: + runs-on: ubuntu-latest + needs: [test-unit] strategy: fail-fast: false matrix: toolchain: - - stable - beta + - nightly steps: - uses: arduino/setup-protoc@v3 with: @@ -30,6 +41,22 @@ jobs: toolchain: ${{ matrix.toolchain }} - run: cargo test --verbose + test-functional: + needs: [test-unit] + runs-on: ubuntu-latest + steps: + - uses: arduino/setup-protoc@v3 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - run: cargo test --verbose --features=test-e2e + env: + CODEMP_TEST_USERNAME_ALICE: ${{ secrets.CODEMP_TEST_USERNAME_ALICE }} + CODEMP_TEST_PASSWORD_ALICE: ${{ secrets.CODEMP_TEST_PASSWORD_ALICE }} + CODEMP_TEST_USERNAME_BOB: ${{ secrets.CODEMP_TEST_USERNAME_BOB }} + CODEMP_TEST_PASSWORD_BOB: ${{ secrets.CODEMP_TEST_PASSWORD_BOB }} + test-build: needs: [test-unit] runs-on: ${{ matrix.runner }} @@ -45,32 +72,10 @@ jobs: - js - py - luajit - - lua54 - toolchain: - - stable - - beta steps: - uses: arduino/setup-protoc@v3 with: repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@master - with: - toolchain: ${{ matrix.toolchain }} + - uses: dtolnay/rust-toolchain@stable - run: cargo build --release --verbose --features=${{ matrix.features }} - - test-functional: - needs: [test-unit] - runs-on: ubuntu-latest - steps: - - uses: arduino/setup-protoc@v3 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@master - - run: cargo test --verbose --features=test-e2e - env: - CODEMP_TEST_USERNAME_ALICE: ${{ secrets.CODEMP_TEST_USERNAME_ALICE }} - CODEMP_TEST_PASSWORD_ALICE: ${{ secrets.CODEMP_TEST_PASSWORD_ALICE }} - CODEMP_TEST_USERNAME_BOB: ${{ secrets.CODEMP_TEST_USERNAME_BOB }} - CODEMP_TEST_PASSWORD_BOB: ${{ secrets.CODEMP_TEST_PASSWORD_BOB }} From 90568bba8d49f360cbfae513806cadc5014d810f Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 13:50:08 +0100 Subject: [PATCH 08/44] test: ignored a test, fixed another leftover assert!(false) for debugging, test_cant_delete_other_buffers will not work since server doesn't track buffer ownership yet --- src/tests/client.rs | 1 - src/tests/server.rs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index 0e015ab..08a8d91 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -125,7 +125,6 @@ async fn test_content_converges() { eprintln!("bob : {bob_content}"); assert_or_err!(alice_content == bob_content); - assert_or_err!(false); Ok(()) } diff --git a/src/tests/server.rs b/src/tests/server.rs index 070db87..28c127f 100644 --- a/src/tests/server.rs +++ b/src/tests/server.rs @@ -54,6 +54,7 @@ async fn test_cant_create_buffer_twice() { } #[tokio::test] +#[ignore] // TODO server has no concept of buffer ownership! async fn cannot_delete_others_buffers() { WorkspaceFixture::two("alice", "bob") .with(|((_, workspace_alice), (_, workspace_bob))| { From e0b919bae81c19c2fc5bb241b523c8683d02ace3 Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 14:22:51 +0100 Subject: [PATCH 09/44] fix: more time in tryRecv to compensate longer RTT --- src/tests/client.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index 08a8d91..db78a1c 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -100,8 +100,8 @@ async fn test_content_converges() { // TODO is there a nicer way to make sure we received all changes? - for i in 0..100 { - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + for i in 0..20 { + tokio::time::sleep(std::time::Duration::from_millis(200)).await; match bob.try_recv().await? { Some(change) => bob.ack(change.version), None => break, @@ -109,8 +109,8 @@ async fn test_content_converges() { eprintln!("bob more to recv at attempt #{i}"); } - for i in 0..100 { - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + for i in 0..20 { + tokio::time::sleep(std::time::Duration::from_millis(200)).await; match alice.try_recv().await? { Some(change) => alice.ack(change.version), None => break, From 752a682efc7df09b37ffeed4e1e36e2c10c5af48 Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 16:58:21 +0100 Subject: [PATCH 10/44] ci(test): only run build tests after functional so that functional tests run sooner --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5f5bde2..c34621b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -58,7 +58,7 @@ jobs: CODEMP_TEST_PASSWORD_BOB: ${{ secrets.CODEMP_TEST_PASSWORD_BOB }} test-build: - needs: [test-unit] + needs: [test-functional] runs-on: ${{ matrix.runner }} strategy: fail-fast: false From cd4e09c1cdef4dafd9166ad64823779a3ddb11ec Mon Sep 17 00:00:00 2001 From: alemi Date: Wed, 30 Oct 2024 17:33:51 +0100 Subject: [PATCH 11/44] test: show for which test workspace was basically fixtures leak workspaces when errors occur, so to better debug what is happening every test now names its workspace. this is tedious and should probably be removed eventually but for now it helps a ton --- src/tests/client.rs | 6 +++--- src/tests/fixtures.rs | 8 ++++---- src/tests/server.rs | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index db78a1c..f0ddd1a 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -3,7 +3,7 @@ use super::{assert_or_err, fixtures::{ScopedFixture, WorkspaceFixture}}; #[tokio::test] async fn test_buffer_search() { - WorkspaceFixture::one("alice") + WorkspaceFixture::one("alice", "test-buffer-search") .with(|(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); @@ -23,7 +23,7 @@ async fn test_buffer_search() { #[tokio::test] async fn test_send_operation() { - WorkspaceFixture::two("alice", "bob") + WorkspaceFixture::two("alice", "bob", "test-send-operation") .with(|((_, workspace_alice), (_, workspace_bob))| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); @@ -53,7 +53,7 @@ async fn test_send_operation() { #[tokio::test] async fn test_content_converges() { - WorkspaceFixture::two("alice", "bob") + WorkspaceFixture::two("alice", "bob", "test-content-converges") .with(|((_, workspace_alice), (_, workspace_bob))| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs index 9f2aebc..e4ef086 100644 --- a/src/tests/fixtures.rs +++ b/src/tests/fixtures.rs @@ -89,19 +89,19 @@ impl WorkspaceFixture { } } - pub fn one(user: &str) -> Self { + pub fn one(user: &str, ws: &str) -> Self { Self { user: user.to_string(), invite: None, - workspace: uuid::Uuid::new_v4().to_string(), + workspace: format!("{ws}-{}", uuid::Uuid::new_v4()), } } - pub fn two(user: &str, invite: &str) -> Self { + pub fn two(user: &str, invite: &str, ws: &str) -> Self { Self { user: user.to_string(), invite: Some(invite.to_string()), - workspace: uuid::Uuid::new_v4().to_string(), + workspace: format!("{ws}-{}", uuid::Uuid::new_v4()), } } } diff --git a/src/tests/server.rs b/src/tests/server.rs index 28c127f..9ec5fb3 100644 --- a/src/tests/server.rs +++ b/src/tests/server.rs @@ -2,7 +2,7 @@ use super::{assert_or_err, fixtures::{ClientFixture, ScopedFixture, WorkspaceFix #[tokio::test] async fn cannot_delete_others_workspaces() { - WorkspaceFixture::two("alice", "bob") + WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-workspaces") .with(|((_, ws_alice), (client_bob, _))| { let ws_alice = ws_alice.clone(); let client_bob = client_bob.clone(); @@ -20,7 +20,7 @@ async fn cannot_delete_others_workspaces() { #[tokio::test] async fn test_buffer_create() { - WorkspaceFixture::one("alice") + WorkspaceFixture::one("alice", "test-buffer-create") .with(|(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); @@ -38,7 +38,7 @@ async fn test_buffer_create() { #[tokio::test] async fn test_cant_create_buffer_twice() { - WorkspaceFixture::one("alice") + WorkspaceFixture::one("alice", "test-cant-create-buffer-twice") .with(|(_, ws): &mut (crate::Client, crate::Workspace)| { let ws = ws.clone(); async move { @@ -56,7 +56,7 @@ async fn test_cant_create_buffer_twice() { #[tokio::test] #[ignore] // TODO server has no concept of buffer ownership! async fn cannot_delete_others_buffers() { - WorkspaceFixture::two("alice", "bob") + WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-buffers") .with(|((_, workspace_alice), (_, workspace_bob))| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); @@ -76,7 +76,7 @@ async fn test_workspace_interactions() { if let Err(e) = async { let client_alice = ClientFixture::of("alice").setup().await?; let client_bob = ClientFixture::of("bob").setup().await?; - let workspace_name = uuid::Uuid::new_v4().to_string(); + let workspace_name = format!("test-workspace-interactions-{}", uuid::Uuid::new_v4().to_string()); client_alice.create_workspace(&workspace_name).await?; let owned_workspaces = client_alice.fetch_owned_workspaces().await?; From 111020afd8de5c4f5b5f9366de167d4b86fe56ee Mon Sep 17 00:00:00 2001 From: zaaarf Date: Sat, 2 Nov 2024 23:26:42 +0100 Subject: [PATCH 12/44] test: buffer fixture --- src/tests/fixtures.rs | 99 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs index e4ef086..a181187 100644 --- a/src/tests/fixtures.rs +++ b/src/tests/fixtures.rs @@ -1,7 +1,5 @@ use std::{error::Error, future::Future}; -// TODO create a BufferFixture too - #[allow(async_fn_in_trait)] pub trait ScopedFixture { async fn setup(&mut self) -> Result>; @@ -44,6 +42,7 @@ pub struct ClientFixture { username: Option, password: Option, } + impl ClientFixture { pub fn of(name: &str) -> Self { Self { @@ -77,14 +76,15 @@ impl ScopedFixture for ClientFixture { pub struct WorkspaceFixture { user: String, - invite: Option, + invitee: Option, workspace: String, } + impl WorkspaceFixture { - pub fn of(user: &str, invite: &str, workspace: &str) -> Self { + pub fn of(user: &str, invitee: &str, workspace: &str) -> Self { Self { user: user.to_string(), - invite: Some(invite.to_string()), + invitee: Some(invitee.to_string()), workspace: workspace.to_string(), } } @@ -92,7 +92,7 @@ impl WorkspaceFixture { pub fn one(user: &str, ws: &str) -> Self { Self { user: user.to_string(), - invite: None, + invitee: None, workspace: format!("{ws}-{}", uuid::Uuid::new_v4()), } } @@ -100,7 +100,7 @@ impl WorkspaceFixture { pub fn two(user: &str, invite: &str, ws: &str) -> Self { Self { user: user.to_string(), - invite: Some(invite.to_string()), + invitee: Some(invite.to_string()), workspace: format!("{ws}-{}", uuid::Uuid::new_v4()), } } @@ -127,9 +127,9 @@ impl ScopedFixture<(crate::Client, crate::Workspace)> for WorkspaceFixture { impl ScopedFixture<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))> for WorkspaceFixture { async fn setup(&mut self) -> Result<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace)), Box> { let client = ClientFixture::of(&self.user).setup().await?; - let invite_client = ClientFixture::of( + let invitee_client = ClientFixture::of( &self - .invite + .invitee .clone() .unwrap_or(uuid::Uuid::new_v4().to_string()), ) @@ -137,15 +137,15 @@ impl ScopedFixture<((crate::Client, crate::Workspace), (crate::Client, crate::Wo .await?; client.create_workspace(&self.workspace).await?; client - .invite_to_workspace(&self.workspace, invite_client.current_user().name.clone()) + .invite_to_workspace(&self.workspace, invitee_client.current_user().name.clone()) .await?; let workspace = client.attach_workspace(&self.workspace).await?; - let invite = invite_client.attach_workspace(&self.workspace).await?; - Ok(((client, workspace), (invite_client, invite))) + let invitee_workspace = invitee_client.attach_workspace(&self.workspace).await?; + Ok(((client, workspace), (invitee_client, invitee_workspace))) } async fn cleanup(&mut self, resource: Option<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))>) { - if let Some(((client, _workspace), (_, _))) = resource { + if let Some(((client, _), (_, _))) = resource { client.leave_workspace(&self.workspace); if let Err(e) = client.delete_workspace(&self.workspace).await { eprintln!("could not delete workspace: {e}"); @@ -153,3 +153,76 @@ impl ScopedFixture<((crate::Client, crate::Workspace), (crate::Client, crate::Wo } } } + +pub struct BufferFixture { + user: String, + invitee: Option, + workspace: String, + buffer: String +} + +impl BufferFixture { + pub fn of(user: &str, invitee: &str, workspace: &str, buffer: &str) -> Self { + Self { + user: user.to_string(), + invitee: Some(invitee.to_string()), + workspace: workspace.to_string(), + buffer: buffer.to_string() + } + } + + pub fn one(user: &str, ws: &str, buf: &str) -> Self { + Self { + user: user.to_string(), + invitee: None, + workspace: format!("{ws}-{}", uuid::Uuid::new_v4()), + buffer: buf.to_string() + } + } + + pub fn two(user: &str, invite: &str, ws: &str, buf: &str) -> Self { + Self { + user: user.to_string(), + invitee: Some(invite.to_string()), + workspace: format!("{ws}-{}", uuid::Uuid::new_v4()), + buffer: buf.to_string() + } + } +} + +impl ScopedFixture<((crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller))> for BufferFixture { + async fn setup(&mut self) -> Result<((crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller)), Box> { + let client = ClientFixture::of(&self.user).setup().await?; + let invitee_client = ClientFixture::of( + &self + .invitee + .clone() + .unwrap_or(uuid::Uuid::new_v4().to_string()), + ) + .setup() + .await?; + client.create_workspace(&self.workspace).await?; + client + .invite_to_workspace(&self.workspace, invitee_client.current_user().name.clone()) + .await?; + + let workspace = client.attach_workspace(&self.workspace).await?; + workspace.create_buffer(&self.buffer).await?; + let buffer = workspace.attach_buffer(&self.buffer).await?; + + let invitee_workspace = invitee_client.attach_workspace(&self.workspace).await?; + let invitee_buffer = invitee_workspace.attach_buffer(&self.buffer).await?; + + Ok(((client, workspace, buffer), (invitee_client, invitee_workspace, invitee_buffer))) + } + + async fn cleanup(&mut self, resource: Option<((crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller))>) { + if let Some(((client, _, _), (_, _, _))) = resource { + // buffer deletion is implied in workspace deletion + client.leave_workspace(&self.workspace); + if let Err(e) = client.delete_workspace(&self.workspace).await { + eprintln!("could not delete workspace: {e}"); + } + } + } +} From 7a535d44928e24d4fbee3c9be14cb0fc1e2f1500 Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:18:24 +0100 Subject: [PATCH 13/44] test: workspace creation and lookup --- src/tests/client.rs | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index f0ddd1a..53025f0 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -1,20 +1,35 @@ +use super::{ + assert_or_err, + fixtures::{ClientFixture, ScopedFixture, WorkspaceFixture}, +}; use crate::api::{AsyncReceiver, AsyncSender}; use super::{assert_or_err, fixtures::{ScopedFixture, WorkspaceFixture}}; #[tokio::test] -async fn test_buffer_search() { - WorkspaceFixture::one("alice", "test-buffer-search") - .with(|(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { - let buffer_name = uuid::Uuid::new_v4().to_string(); - let workspace_alice = workspace_alice.clone(); +async fn test_workspace_creation_and_lookup() { + ClientFixture::of("alice") + .with(|client| { + let client = client.clone(); async move { - workspace_alice.create_buffer(&buffer_name).await?; - assert_or_err!(!workspace_alice - .search_buffers(Some(&buffer_name[0..4])) - .is_empty()); - assert_or_err!(workspace_alice.search_buffers(Some("_")).is_empty()); - workspace_alice.delete_buffer(&buffer_name).await?; + let workspace_name = uuid::Uuid::new_v4().to_string(); + let wrong_name = uuid::Uuid::new_v4().to_string(); + + client.create_workspace(&workspace_name).await?; + let ws = client.get_workspace(&workspace_name); + assert_or_err!(ws.is_some()); + assert_or_err!(ws.unwrap().id() == workspace_name); + + assert_or_err!(client.get_workspace(&wrong_name).is_none()); + + let wslist = client.fetch_owned_workspaces().await?; + assert_or_err!(wslist.len() == 1); + assert_or_err!(wslist.contains(&workspace_name)); + Ok(()) + } + }) + .await +} Ok(()) } }) From dc627dc6af0b88247e7e2b087fe5b26654ab0c28 Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:18:47 +0100 Subject: [PATCH 14/44] fmt: formatter shenanigans --- src/tests/fixtures.rs | 65 +++++++++++++++++++++++++++++++++-------- src/tests/server.rs | 67 +++++++++++++++++++++++++------------------ 2 files changed, 92 insertions(+), 40 deletions(-) diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs index a181187..b023077 100644 --- a/src/tests/fixtures.rs +++ b/src/tests/fixtures.rs @@ -124,8 +124,21 @@ impl ScopedFixture<(crate::Client, crate::Workspace)> for WorkspaceFixture { } } -impl ScopedFixture<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))> for WorkspaceFixture { - async fn setup(&mut self) -> Result<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace)), Box> { +impl + ScopedFixture<( + (crate::Client, crate::Workspace), + (crate::Client, crate::Workspace), + )> for WorkspaceFixture +{ + async fn setup( + &mut self, + ) -> Result< + ( + (crate::Client, crate::Workspace), + (crate::Client, crate::Workspace), + ), + Box, + > { let client = ClientFixture::of(&self.user).setup().await?; let invitee_client = ClientFixture::of( &self @@ -144,7 +157,13 @@ impl ScopedFixture<((crate::Client, crate::Workspace), (crate::Client, crate::Wo Ok(((client, workspace), (invitee_client, invitee_workspace))) } - async fn cleanup(&mut self, resource: Option<((crate::Client, crate::Workspace), (crate::Client, crate::Workspace))>) { + async fn cleanup( + &mut self, + resource: Option<( + (crate::Client, crate::Workspace), + (crate::Client, crate::Workspace), + )>, + ) { if let Some(((client, _), (_, _))) = resource { client.leave_workspace(&self.workspace); if let Err(e) = client.delete_workspace(&self.workspace).await { @@ -158,7 +177,7 @@ pub struct BufferFixture { user: String, invitee: Option, workspace: String, - buffer: String + buffer: String, } impl BufferFixture { @@ -167,7 +186,7 @@ impl BufferFixture { user: user.to_string(), invitee: Some(invitee.to_string()), workspace: workspace.to_string(), - buffer: buffer.to_string() + buffer: buffer.to_string(), } } @@ -176,7 +195,7 @@ impl BufferFixture { user: user.to_string(), invitee: None, workspace: format!("{ws}-{}", uuid::Uuid::new_v4()), - buffer: buf.to_string() + buffer: buf.to_string(), } } @@ -185,13 +204,26 @@ impl BufferFixture { user: user.to_string(), invitee: Some(invite.to_string()), workspace: format!("{ws}-{}", uuid::Uuid::new_v4()), - buffer: buf.to_string() + buffer: buf.to_string(), } } } -impl ScopedFixture<((crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller))> for BufferFixture { - async fn setup(&mut self) -> Result<((crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller)), Box> { +impl + ScopedFixture<( + (crate::Client, crate::Workspace, crate::buffer::Controller), + (crate::Client, crate::Workspace, crate::buffer::Controller), + )> for BufferFixture +{ + async fn setup( + &mut self, + ) -> Result< + ( + (crate::Client, crate::Workspace, crate::buffer::Controller), + (crate::Client, crate::Workspace, crate::buffer::Controller), + ), + Box, + > { let client = ClientFixture::of(&self.user).setup().await?; let invitee_client = ClientFixture::of( &self @@ -213,14 +245,23 @@ impl ScopedFixture<((crate::Client, crate::Workspace, crate::buffer::Controller) let invitee_workspace = invitee_client.attach_workspace(&self.workspace).await?; let invitee_buffer = invitee_workspace.attach_buffer(&self.buffer).await?; - Ok(((client, workspace, buffer), (invitee_client, invitee_workspace, invitee_buffer))) + Ok(( + (client, workspace, buffer), + (invitee_client, invitee_workspace, invitee_buffer), + )) } - async fn cleanup(&mut self, resource: Option<((crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller))>) { + async fn cleanup( + &mut self, + resource: Option<( + (crate::Client, crate::Workspace, crate::buffer::Controller), + (crate::Client, crate::Workspace, crate::buffer::Controller), + )>, + ) { if let Some(((client, _, _), (_, _, _))) = resource { // buffer deletion is implied in workspace deletion client.leave_workspace(&self.workspace); - if let Err(e) = client.delete_workspace(&self.workspace).await { + if let Err(e) = client.delete_workspace(&self.workspace).await { eprintln!("could not delete workspace: {e}"); } } diff --git a/src/tests/server.rs b/src/tests/server.rs index 9ec5fb3..0f3fda7 100644 --- a/src/tests/server.rs +++ b/src/tests/server.rs @@ -1,38 +1,46 @@ -use super::{assert_or_err, fixtures::{ClientFixture, ScopedFixture, WorkspaceFixture}}; +use super::{ + assert_or_err, + fixtures::{ClientFixture, ScopedFixture, WorkspaceFixture}, +}; -#[tokio::test] -async fn cannot_delete_others_workspaces() { - WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-workspaces") - .with(|((_, ws_alice), (client_bob, _))| { - let ws_alice = ws_alice.clone(); - let client_bob = client_bob.clone(); - async move { - assert_or_err!( - client_bob.delete_workspace(&ws_alice.id()).await.is_err(), - "bob was allowed to delete a workspace he didn't own!" - ); - Ok(()) - } +// Moved this in client for now. +// #[tokio::test] +// async fn cannot_delete_others_workspaces() { +// WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-workspaces") +// .with(|((_, ws_alice), (client_bob, _))| { +// let ws_alice = ws_alice.clone(); +// let client_bob = client_bob.clone(); +// async move { +// assert_or_err!( +// client_bob.delete_workspace(&ws_alice.id()).await.is_err(), +// "bob was allowed to delete a workspace he didn't own!" +// ); +// Ok(()) +// } - }) - .await -} +// }) +// .await +// } #[tokio::test] async fn test_buffer_create() { WorkspaceFixture::one("alice", "test-buffer-create") - .with(|(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { - let buffer_name = uuid::Uuid::new_v4().to_string(); - let workspace_alice = workspace_alice.clone(); + .with( + |(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); - async move { - workspace_alice.create_buffer(&buffer_name).await?; - assert_or_err!(vec![buffer_name.clone()] == workspace_alice.fetch_buffers().await?); - workspace_alice.delete_buffer(&buffer_name).await?; + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!( + vec![buffer_name.clone()] == workspace_alice.fetch_buffers().await? + ); + workspace_alice.delete_buffer(&buffer_name).await?; - Ok(()) - } - }) + Ok(()) + } + }, + ) .await; } @@ -76,7 +84,10 @@ async fn test_workspace_interactions() { if let Err(e) = async { let client_alice = ClientFixture::of("alice").setup().await?; let client_bob = ClientFixture::of("bob").setup().await?; - let workspace_name = format!("test-workspace-interactions-{}", uuid::Uuid::new_v4().to_string()); + let workspace_name = format!( + "test-workspace-interactions-{}", + uuid::Uuid::new_v4().to_string() + ); client_alice.create_workspace(&workspace_name).await?; let owned_workspaces = client_alice.fetch_owned_workspaces().await?; From 44e5e390ebec80eeac194905a10b1d48cc63fa41 Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:19:38 +0100 Subject: [PATCH 15/44] chore: implement from ConnectionError and from RemoteError for AssertionError --- src/tests/mod.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/tests/mod.rs b/src/tests/mod.rs index a1907ca..b699b4f 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -5,6 +5,7 @@ mod client; mod server; pub mod fixtures; +use crate::errors::{ConnectionError, RemoteError}; #[derive(Debug)] pub struct AssertionError(String); @@ -23,6 +24,27 @@ impl std::fmt::Display for AssertionError { impl std::error::Error for AssertionError {} +impl From for AssertionError { + fn from(value: ConnectionError) -> Self { + match value { + ConnectionError::Transport(error) => AssertionError::new(&format!( + "Connection::Transport error during setup of a test: {}", + error, + )), + ConnectionError::Remote(remote_error) => AssertionError::new(&format!( + "Connection::Remote error during setup of a test: {}", + remote_error, + )), + } + } +} + +impl From for AssertionError { + fn from(value: RemoteError) -> Self { + AssertionError::new(&format!("Remote error during setup of a test: {}", value,)) + } +} + #[macro_export] macro_rules! assert_or_err { ($s:expr) => { From 9ff9a47d86c94d377d30671e792f283d5d35b4a2 Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:20:09 +0100 Subject: [PATCH 16/44] test: can't create workspace more than once --- src/tests/client.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/tests/client.rs b/src/tests/client.rs index 53025f0..56a7680 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -30,6 +30,23 @@ async fn test_workspace_creation_and_lookup() { }) .await } + +#[tokio::test] +async fn test_cant_create_same_workspace_more_than_once() { + ClientFixture::of("alice") + .with(|client| { + let client = client.clone(); + + async move { + let workspace_name = uuid::Uuid::new_v4().to_string(); + + client.create_workspace(&workspace_name).await?; + assert_or_err!(client.create_workspace(workspace_name).await.is_err()); + Ok(()) + } + }) + .await +} Ok(()) } }) From ccb5406ccfe139d5cc8017469e526f17b71ffb3c Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:20:30 +0100 Subject: [PATCH 17/44] test: attaching to workspaces and making them active --- src/tests/client.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/tests/client.rs b/src/tests/client.rs index 56a7680..14c57f9 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -47,6 +47,26 @@ async fn test_cant_create_same_workspace_more_than_once() { }) .await } + +#[tokio::test] +async fn test_workspace_attach_and_active_workspaces() { + ClientFixture::of("alice") + .with(|client| { + let client = client.clone(); + + async move { + let workspace_name = uuid::Uuid::new_v4().to_string(); + + client.create_workspace(&workspace_name).await?; + let ws = client.attach_workspace(&workspace_name).await?; + + assert_or_err!(ws.id() == workspace_name); + assert_or_err!(client.active_workspaces().contains(&workspace_name)); + Ok(()) + } + }) + .await +} Ok(()) } }) From fdd272646def49cd60e7abe8dd9f441e4322ba38 Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:21:06 +0100 Subject: [PATCH 18/44] test: attaching to a non existing workspace is an error --- src/tests/client.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/tests/client.rs b/src/tests/client.rs index 14c57f9..5a5468d 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -67,6 +67,24 @@ async fn test_workspace_attach_and_active_workspaces() { }) .await } + +#[tokio::test] +async fn test_attaching_to_non_existing_is_error() { + ClientFixture::of("alice") + .with(|client| { + let client = client.clone(); + + async move { + let workspace_name = uuid::Uuid::new_v4().to_string(); + + // we don't create any workspace. + // client.create_workspace(workspace_name).await?; + assert_or_err!(client.attach_workspace(&workspace_name).await.is_err()); + Ok(()) + } + }) + .await +} Ok(()) } }) From 781fd9ba4266e2cc7c508438a5a1d24c1109ea82 Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:22:13 +0100 Subject: [PATCH 19/44] test: leaving workspaces, attaching after leaving --- src/tests/client.rs | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/tests/client.rs b/src/tests/client.rs index 5a5468d..16874b3 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -85,6 +85,49 @@ async fn test_attaching_to_non_existing_is_error() { }) .await } + +#[tokio::test] +async fn test_attach_and_leave_workspace_interactions() { + ClientFixture::of("alice") + .with(|client| { + let client = client.clone(); + + async move { + let workspace_name = uuid::Uuid::new_v4().to_string(); + + client.create_workspace(&workspace_name).await?; + // leaving a workspace you are not attached to, returns true + assert_or_err!( + client.leave_workspace(&workspace_name), + "leaving a workspace you are not attached to returned false, should return true." + ); + + // leaving a workspace you are attached to, returns true + // when there is only one reference to it. + client.attach_workspace(&workspace_name).await?; + assert_or_err!( + client.leave_workspace(&workspace_name), + "leaving a workspace with a single reference returned false." + ); + + // we are also implicitly testing repeated leaving and attaching + // to the same workspace. + let res = client.attach_workspace(&workspace_name).await; + assert_or_err!(res.is_ok(), "could not attach to the same workspace immediately after successfully leaving it."); + + // we should have an extra reference here, which should make the + // consume return false. + let ws = res.unwrap(); + assert_or_err!( + !client.leave_workspace(&workspace_name), + "leaving a workspace while some reference still exist returned true." + ); + + Ok(()) + } + }) + .await +} Ok(()) } }) From 9d0c961ac25fb1cdbeca60f36881b199776f812c Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:22:41 +0100 Subject: [PATCH 20/44] test: delete empty workspace --- src/tests/client.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/tests/client.rs b/src/tests/client.rs index 16874b3..a972628 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -128,9 +128,27 @@ async fn test_attach_and_leave_workspace_interactions() { }) .await } + +#[tokio::test] +async fn test_delete_empty_workspace() { + ClientFixture::of("alice") + .with(|client| { + let client = client.clone(); + + async move { + let workspace_name = uuid::Uuid::new_v4().to_string(); + + client.create_workspace(&workspace_name).await?; + client.delete_workspace(&workspace_name).await?; + + assert_or_err!(client.get_workspace(&workspace_name).is_none()); + assert_or_err!(client.fetch_owned_workspaces().await?.is_empty()); + Ok(()) } }) + .await +} .await; } From 35b9b12aaf7ad583cc1a693cdeaa8b56ccfab477 Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:23:00 +0100 Subject: [PATCH 21/44] test: deleting twice or non existing is an error --- src/tests/client.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/tests/client.rs b/src/tests/client.rs index a972628..c6a0df3 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -149,6 +149,25 @@ async fn test_delete_empty_workspace() { }) .await } + +#[tokio::test] +async fn test_deleting_twice_or_non_existing_is_an_error() { + ClientFixture::of("alice") + .with(|client| { + let client = client.clone(); + + async move { + let workspace_name = uuid::Uuid::new_v4().to_string(); + + client.create_workspace(&workspace_name).await?; + client.delete_workspace(&workspace_name).await?; + + assert_or_err!(client.delete_workspace(&workspace_name).await.is_err()); + Ok(()) + } + }) + .await +} .await; } From 84bfc45fc8aea873ee6d29971d3af2a834a9670e Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:24:08 +0100 Subject: [PATCH 22/44] test(WIP): added skeleton for testing of behaviour when deleting workspace with people attached. Need to know what is the expected behaviour that we want. --- src/tests/client.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/tests/client.rs b/src/tests/client.rs index c6a0df3..39dd182 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -168,6 +168,23 @@ async fn test_deleting_twice_or_non_existing_is_an_error() { }) .await } + +// Now we can begin using WorkspaceFixtures for with a single user. + +// #[tokio::test] +// async fn test_delete_workspace_with_users_attached() { +// WorkspaceFixture::one("bob", "to-be-deleted") +// .with(|(client, workspace): &mut (crate::Client, crate::Workspace)| { + +// async move { +// client.delete_workspace(workspace.id()).await?; + +// // TODO: I Don't know what should happen here. +// Ok(()) +// } +// }) +// .await +// } .await; } From 7f17d1fd6ec5781ab5aca8e56f05d7bec184c635 Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:24:47 +0100 Subject: [PATCH 23/44] test: inviting users to one's workspace --- src/tests/client.rs | 57 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/tests/client.rs b/src/tests/client.rs index 39dd182..bf698c7 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -185,6 +185,63 @@ async fn test_deleting_twice_or_non_existing_is_an_error() { // }) // .await // } + +#[tokio::test] +async fn test_invite_user_to_workspace_and_invited_lookup() { + WorkspaceFixture::one("bob", "workspace-di-bob") + .with( + |(client_bob, workspace_bob): &mut (crate::Client, crate::Workspace)| { + let client_bob = client_bob.clone(); + let workspace_bob = workspace_bob.clone(); + + async move { + let client_alice = ClientFixture::of("alice").setup().await?; + + let wrong_workspace_name = uuid::Uuid::new_v4().to_string(); + // inviting to a non existing workspace is an error + assert_or_err!(client_bob + .invite_to_workspace( + wrong_workspace_name, + client_alice.current_user().name.clone(), + ) + .await + .is_err()); + + client_bob + .invite_to_workspace( + workspace_bob.id(), + client_alice.current_user().name.clone(), + ) + .await?; + + // there are two users now in the workspace of bob + // alice is one of the users + // bob is one of the users + // the workspace appears in the joined workspaces for alice + // the workspace does not appear in the owned workspaces for alice + + let user_list = workspace_bob.fetch_users().await?; + assert_or_err!(user_list.len() == 2); + assert_or_err!(user_list + .iter() + .any(|u| u.name == client_alice.current_user().name)); + assert_or_err!(user_list + .iter() + .any(|u| u.name == client_bob.current_user().name)); + + let alice_owned_workspaces = client_alice.fetch_owned_workspaces().await?; + let alice_invited_workspaces = client_alice.fetch_joined_workspaces().await?; + + assert_or_err!(alice_owned_workspaces.is_empty()); + assert_or_err!(alice_invited_workspaces.contains(&workspace_bob.id())); + Ok(()) + } + }, + ) + .await +} + +// Now we can use workspace fixtures with invite. .await; } From 3773ebfd7dd1870355a2ddbf56593ba1c129dfba Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:26:14 +0100 Subject: [PATCH 24/44] test: moved around previously existing tests. and other formatter shenanigans --- src/tests/client.rs | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index bf698c7..ec4409f 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -3,7 +3,6 @@ use super::{ fixtures::{ClientFixture, ScopedFixture, WorkspaceFixture}, }; use crate::api::{AsyncReceiver, AsyncSender}; -use super::{assert_or_err, fixtures::{ScopedFixture, WorkspaceFixture}}; #[tokio::test] async fn test_workspace_creation_and_lookup() { @@ -242,6 +241,43 @@ async fn test_invite_user_to_workspace_and_invited_lookup() { } // Now we can use workspace fixtures with invite. + +#[tokio::test] +async fn cannot_delete_others_workspaces() { + WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-workspaces") + .with(|((_, ws_alice), (client_bob, _))| { + let ws_alice = ws_alice.clone(); + let client_bob = client_bob.clone(); + async move { + assert_or_err!( + client_bob.delete_workspace(&ws_alice.id()).await.is_err(), + "bob was allowed to delete a workspace he didn't own!" + ); + Ok(()) + } + }) + .await +} + +#[tokio::test] +async fn test_buffer_search() { + WorkspaceFixture::one("alice", "test-buffer-search") + .with( + |(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); + + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!(!workspace_alice + .search_buffers(Some(&buffer_name[0..4])) + .is_empty()); + assert_or_err!(workspace_alice.search_buffers(Some("_")).is_empty()); + workspace_alice.delete_buffer(&buffer_name).await?; + Ok(()) + } + }, + ) .await; } From c0bff459893d969f16b02b400097c7f53b51ca7a Mon Sep 17 00:00:00 2001 From: cschen Date: Sun, 3 Nov 2024 17:53:27 +0100 Subject: [PATCH 25/44] fix(test): add more clear explainations of what went wrong. --- src/tests/client.rs | 77 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 19 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index ec4409f..1dfdab1 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -16,14 +16,26 @@ async fn test_workspace_creation_and_lookup() { client.create_workspace(&workspace_name).await?; let ws = client.get_workspace(&workspace_name); - assert_or_err!(ws.is_some()); - assert_or_err!(ws.unwrap().id() == workspace_name); + assert_or_err!( + ws.is_some(), + "Failed to retrieve the workspace just created." + ); + assert_or_err!( + ws.unwrap().id() == workspace_name, + "The retreived workspace has a different name than the one created." + ); - assert_or_err!(client.get_workspace(&wrong_name).is_none()); + assert_or_err!( + client.get_workspace(&wrong_name).is_none(), + "Looking up a non existent workspace returned something." + ); let wslist = client.fetch_owned_workspaces().await?; - assert_or_err!(wslist.len() == 1); - assert_or_err!(wslist.contains(&workspace_name)); + assert_or_err!(wslist.len() == 1, "the amount of owned workspaces is not 1"); + assert_or_err!( + wslist.contains(&workspace_name), + "the newly create workspace is not present in the owned list." + ); Ok(()) } }) @@ -40,7 +52,10 @@ async fn test_cant_create_same_workspace_more_than_once() { let workspace_name = uuid::Uuid::new_v4().to_string(); client.create_workspace(&workspace_name).await?; - assert_or_err!(client.create_workspace(workspace_name).await.is_err()); + assert_or_err!( + client.create_workspace(workspace_name).await.is_err(), + "a workspace was created twice without error." + ); Ok(()) } }) @@ -59,8 +74,14 @@ async fn test_workspace_attach_and_active_workspaces() { client.create_workspace(&workspace_name).await?; let ws = client.attach_workspace(&workspace_name).await?; - assert_or_err!(ws.id() == workspace_name); - assert_or_err!(client.active_workspaces().contains(&workspace_name)); + assert_or_err!( + ws.id() == workspace_name, + "we attached to a workspace that has a different name than the one requested." + ); + assert_or_err!( + client.active_workspaces().contains(&workspace_name), + "the workspace is not present in the active workspaces list after attaching." + ); Ok(()) } }) @@ -78,7 +99,10 @@ async fn test_attaching_to_non_existing_is_error() { // we don't create any workspace. // client.create_workspace(workspace_name).await?; - assert_or_err!(client.attach_workspace(&workspace_name).await.is_err()); + assert_or_err!( + client.attach_workspace(&workspace_name).await.is_err(), + "we attached to a non existing workspace." + ); Ok(()) } }) @@ -116,7 +140,7 @@ async fn test_attach_and_leave_workspace_interactions() { // we should have an extra reference here, which should make the // consume return false. - let ws = res.unwrap(); + let _ws = res.unwrap(); assert_or_err!( !client.leave_workspace(&workspace_name), "leaving a workspace while some reference still exist returned true." @@ -140,8 +164,14 @@ async fn test_delete_empty_workspace() { client.create_workspace(&workspace_name).await?; client.delete_workspace(&workspace_name).await?; - assert_or_err!(client.get_workspace(&workspace_name).is_none()); - assert_or_err!(client.fetch_owned_workspaces().await?.is_empty()); + assert_or_err!( + client.get_workspace(&workspace_name).is_none(), + "a deleted workspaces was still available for lookup." + ); + assert_or_err!( + client.fetch_owned_workspaces().await?.is_empty(), + "a deleted workspace was still present in the owned workspaces list." + ); Ok(()) } @@ -161,7 +191,10 @@ async fn test_deleting_twice_or_non_existing_is_an_error() { client.create_workspace(&workspace_name).await?; client.delete_workspace(&workspace_name).await?; - assert_or_err!(client.delete_workspace(&workspace_name).await.is_err()); + assert_or_err!( + client.delete_workspace(&workspace_name).await.is_err(), + "we could delete a workspace twice, or a non existing one." + ); Ok(()) } }) @@ -204,7 +237,8 @@ async fn test_invite_user_to_workspace_and_invited_lookup() { client_alice.current_user().name.clone(), ) .await - .is_err()); + .is_err(), + "we invited a user to a non existing workspace."); client_bob .invite_to_workspace( @@ -220,19 +254,24 @@ async fn test_invite_user_to_workspace_and_invited_lookup() { // the workspace does not appear in the owned workspaces for alice let user_list = workspace_bob.fetch_users().await?; - assert_or_err!(user_list.len() == 2); + assert_or_err!(user_list.len() == 2, + "after inviting alice there should be only two users in a workspace."); assert_or_err!(user_list .iter() - .any(|u| u.name == client_alice.current_user().name)); + .any(|u| u.name == client_alice.current_user().name), + "alice was invited but is not present as one of the users."); assert_or_err!(user_list .iter() - .any(|u| u.name == client_bob.current_user().name)); + .any(|u| u.name == client_bob.current_user().name), + "bob owns the workspace but is not present in the workspace."); let alice_owned_workspaces = client_alice.fetch_owned_workspaces().await?; let alice_invited_workspaces = client_alice.fetch_joined_workspaces().await?; - assert_or_err!(alice_owned_workspaces.is_empty()); - assert_or_err!(alice_invited_workspaces.contains(&workspace_bob.id())); + assert_or_err!(alice_owned_workspaces.is_empty(), + "The workspace alice was invided to is listed as owned for her."); + assert_or_err!(alice_invited_workspaces.contains(&workspace_bob.id()), + "The list of workspaces to which alice was invited to does not contain the one bob invited her to."); Ok(()) } }, From 518143d5b9ec3ae3dd54482e4b356b25650f6d5e Mon Sep 17 00:00:00 2001 From: zaaarf Date: Sun, 3 Nov 2024 19:36:36 +0100 Subject: [PATCH 26/44] docs(java): specify the new behaviour of leaveWorkspace --- dist/java/src/mp/code/Client.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dist/java/src/mp/code/Client.java b/dist/java/src/mp/code/Client.java index 465d10a..113aa65 100644 --- a/dist/java/src/mp/code/Client.java +++ b/dist/java/src/mp/code/Client.java @@ -128,7 +128,8 @@ public final class Client { /** * Leaves a workspace. * @param workspaceId the id of the workspaces to leave - * @return true if it succeeded (usually fails if the workspace wasn't active) + * @return true if it succeeded or wasn't in the workspace; false if there are still + * leftover references around */ public boolean leaveWorkspace(String workspaceId) { return leave_workspace(this.ptr, workspaceId); From c08e209e064587a0ed1a1139a1b519455927316b Mon Sep 17 00:00:00 2001 From: zaaarf Date: Sun, 3 Nov 2024 19:58:09 +0100 Subject: [PATCH 27/44] tests: cleanup redundant code --- src/tests/fixtures.rs | 10 ++-------- src/tests/server.rs | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs index b023077..37fed29 100644 --- a/src/tests/fixtures.rs +++ b/src/tests/fixtures.rs @@ -253,17 +253,11 @@ impl async fn cleanup( &mut self, - resource: Option<( + _resource: Option<( (crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller), )>, ) { - if let Some(((client, _, _), (_, _, _))) = resource { - // buffer deletion is implied in workspace deletion - client.leave_workspace(&self.workspace); - if let Err(e) = client.delete_workspace(&self.workspace).await { - eprintln!("could not delete workspace: {e}"); - } - } + // buffer deletion is implied in workspace deletion } } diff --git a/src/tests/server.rs b/src/tests/server.rs index 0f3fda7..15676a1 100644 --- a/src/tests/server.rs +++ b/src/tests/server.rs @@ -86,7 +86,7 @@ async fn test_workspace_interactions() { let client_bob = ClientFixture::of("bob").setup().await?; let workspace_name = format!( "test-workspace-interactions-{}", - uuid::Uuid::new_v4().to_string() + uuid::Uuid::new_v4() ); client_alice.create_workspace(&workspace_name).await?; From c1c9dea0333141a8f6bae384f9099a27d0bd7537 Mon Sep 17 00:00:00 2001 From: zaaarf Date: Sun, 3 Nov 2024 20:11:21 +0100 Subject: [PATCH 28/44] tests: actually delete the buffer in the buffer fixture --- src/tests/fixtures.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs index 37fed29..9c2ea1d 100644 --- a/src/tests/fixtures.rs +++ b/src/tests/fixtures.rs @@ -253,11 +253,15 @@ impl async fn cleanup( &mut self, - _resource: Option<( + mut resource: Option<( (crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller), )>, ) { - // buffer deletion is implied in workspace deletion + if let Some(((_, ws, buf), (_, _, _))) = resource.take() { + if let Err(e) = ws.delete_buffer(buf.path()).await { + eprintln!("could not delete buffer: {e:?}"); + } + } } } From 888f7fd80cfd919867e24e529e2a0350f2ba27e8 Mon Sep 17 00:00:00 2001 From: zaaarf Date: Sun, 3 Nov 2024 20:11:36 +0100 Subject: [PATCH 29/44] docs: better wording in detach_buffer docs --- src/workspace.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/workspace.rs b/src/workspace.rs index f8054fc..787166a 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -185,10 +185,10 @@ impl Workspace { /// /// This will stop and drop its [`buffer::Controller`]. /// - /// Returns `true` if connectly dropped or wasn't present, `false` if dropped but wasn't last ref - /// - /// If this method returns `false` you have a dangling ref, maybe just waiting for garbage - /// collection or maybe preventing the controller from being dropped completely + /// Returns `true` if it was connectly dropped or wasn't present, `false` if it was dropped but + /// wasn't the last existing reference to it. If this method returns `false` it means you have + /// a dangling reference somewhere. It may just be waiting for garbage collection, but as long + /// as it exists, it will prevent the controller from being completely dropped. #[allow(clippy::redundant_pattern_matching)] // all cases are clearer this way pub fn detach_buffer(&self, path: &str) -> bool { match self.0.buffers.remove(path) { From ee2ced51ca333db2f50ea9c775eb2fba1fdbf0b2 Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 00:15:17 +0100 Subject: [PATCH 30/44] chore: removed need of adding type hints Co-authored-by: cschen --- src/tests/client.rs | 10 +++++----- src/tests/fixtures.rs | 26 +++++--------------------- src/tests/server.rs | 6 +++--- 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index 1dfdab1..4be6d4b 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -222,7 +222,7 @@ async fn test_deleting_twice_or_non_existing_is_an_error() { async fn test_invite_user_to_workspace_and_invited_lookup() { WorkspaceFixture::one("bob", "workspace-di-bob") .with( - |(client_bob, workspace_bob): &mut (crate::Client, crate::Workspace)| { + |(client_bob, workspace_bob)| { let client_bob = client_bob.clone(); let workspace_bob = workspace_bob.clone(); @@ -284,7 +284,7 @@ async fn test_invite_user_to_workspace_and_invited_lookup() { #[tokio::test] async fn cannot_delete_others_workspaces() { WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-workspaces") - .with(|((_, ws_alice), (client_bob, _))| { + .with(|(_, ws_alice, client_bob, _)| { let ws_alice = ws_alice.clone(); let client_bob = client_bob.clone(); async move { @@ -302,7 +302,7 @@ async fn cannot_delete_others_workspaces() { async fn test_buffer_search() { WorkspaceFixture::one("alice", "test-buffer-search") .with( - |(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { + |(_, workspace_alice)| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); @@ -323,7 +323,7 @@ async fn test_buffer_search() { #[tokio::test] async fn test_send_operation() { WorkspaceFixture::two("alice", "bob", "test-send-operation") - .with(|((_, workspace_alice), (_, workspace_bob))| { + .with(|(_, workspace_alice, _, workspace_bob)| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); let workspace_bob = workspace_bob.clone(); @@ -353,7 +353,7 @@ async fn test_send_operation() { #[tokio::test] async fn test_content_converges() { WorkspaceFixture::two("alice", "bob", "test-content-converges") - .with(|((_, workspace_alice), (_, workspace_bob))| { + .with(|(_, workspace_alice, _, workspace_bob)| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); let workspace_bob = workspace_bob.clone(); diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs index 9c2ea1d..6409935 100644 --- a/src/tests/fixtures.rs +++ b/src/tests/fixtures.rs @@ -124,21 +124,8 @@ impl ScopedFixture<(crate::Client, crate::Workspace)> for WorkspaceFixture { } } -impl - ScopedFixture<( - (crate::Client, crate::Workspace), - (crate::Client, crate::Workspace), - )> for WorkspaceFixture -{ - async fn setup( - &mut self, - ) -> Result< - ( - (crate::Client, crate::Workspace), - (crate::Client, crate::Workspace), - ), - Box, - > { +impl ScopedFixture<(crate::Client, crate::Workspace, crate::Client, crate::Workspace)> for WorkspaceFixture { + async fn setup(&mut self) -> Result<(crate::Client, crate::Workspace, crate::Client, crate::Workspace), Box> { let client = ClientFixture::of(&self.user).setup().await?; let invitee_client = ClientFixture::of( &self @@ -154,17 +141,14 @@ impl .await?; let workspace = client.attach_workspace(&self.workspace).await?; let invitee_workspace = invitee_client.attach_workspace(&self.workspace).await?; - Ok(((client, workspace), (invitee_client, invitee_workspace))) + Ok((client, workspace, invitee_client, invitee_workspace)) } async fn cleanup( &mut self, - resource: Option<( - (crate::Client, crate::Workspace), - (crate::Client, crate::Workspace), - )>, + resource: Option<(crate::Client, crate::Workspace, crate::Client, crate::Workspace)>, ) { - if let Some(((client, _), (_, _))) = resource { + if let Some((client, _, _, _)) = resource { client.leave_workspace(&self.workspace); if let Err(e) = client.delete_workspace(&self.workspace).await { eprintln!("could not delete workspace: {e}"); diff --git a/src/tests/server.rs b/src/tests/server.rs index 15676a1..09bcbe0 100644 --- a/src/tests/server.rs +++ b/src/tests/server.rs @@ -26,7 +26,7 @@ use super::{ async fn test_buffer_create() { WorkspaceFixture::one("alice", "test-buffer-create") .with( - |(_, workspace_alice): &mut (crate::Client, crate::Workspace)| { + |(_, workspace_alice)| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); @@ -47,7 +47,7 @@ async fn test_buffer_create() { #[tokio::test] async fn test_cant_create_buffer_twice() { WorkspaceFixture::one("alice", "test-cant-create-buffer-twice") - .with(|(_, ws): &mut (crate::Client, crate::Workspace)| { + .with(|(_, ws)| { let ws = ws.clone(); async move { ws.create_buffer("cacca").await?; @@ -65,7 +65,7 @@ async fn test_cant_create_buffer_twice() { #[ignore] // TODO server has no concept of buffer ownership! async fn cannot_delete_others_buffers() { WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-buffers") - .with(|((_, workspace_alice), (_, workspace_bob))| { + .with(|(_, workspace_alice, _, workspace_bob)| { let buffer_name = uuid::Uuid::new_v4().to_string(); let workspace_alice = workspace_alice.clone(); let workspace_bob = workspace_bob.clone(); From c42b091b63ed6f76a953178cc5f87887d8990827 Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 00:16:27 +0100 Subject: [PATCH 31/44] test: better assert_or_err messages Co-authored-by: cschen --- src/tests/mod.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/tests/mod.rs b/src/tests/mod.rs index b699b4f..802c364 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -48,6 +48,7 @@ impl From for AssertionError { #[macro_export] macro_rules! assert_or_err { ($s:expr) => { + #[allow(clippy::bool_comparison)] if !$s { return Err($crate::tests::AssertionError::new(&format!( "assertion failed at line {}: {}", @@ -58,6 +59,18 @@ macro_rules! assert_or_err { } }; ($s:expr, $msg:literal) => { + #[allow(clippy::bool_comparison)] + if !$s { + return Err($crate::tests::AssertionError::new(&format!( + "{} (line {})", + $msg, + std::line!(), + )) + .into()); + } + }; + ($s:expr, raw $msg:literal) => { + #[allow(clippy::bool_comparison)] if !$s { return Err($crate::tests::AssertionError::new($msg).into()); } From 300f6620c0ce4fe59d84dc012090b5b00a6be73a Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 00:18:06 +0100 Subject: [PATCH 32/44] fix: cleanup for buffer fixture idk who made this?? it was already fixed in dev env, uncommitted Co-authored-by: cschen Co-authored-by: zaaarf --- src/tests/fixtures.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs index 6409935..9a0ae60 100644 --- a/src/tests/fixtures.rs +++ b/src/tests/fixtures.rs @@ -237,14 +237,16 @@ impl async fn cleanup( &mut self, - mut resource: Option<( + resource: Option<( (crate::Client, crate::Workspace, crate::buffer::Controller), (crate::Client, crate::Workspace, crate::buffer::Controller), )>, ) { - if let Some(((_, ws, buf), (_, _, _))) = resource.take() { - if let Err(e) = ws.delete_buffer(buf.path()).await { - eprintln!("could not delete buffer: {e:?}"); + if let Some(((client, _, _), (_, _, _))) = resource { + // buffer deletion is implied in workspace deletion + client.leave_workspace(&self.workspace); + if let Err(e) = client.delete_workspace(&self.workspace).await { + eprintln!("could not delete workspace: {e}"); } } } From 3f0b04af6eba465c17ec4a9501a72df2171b2e64 Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 00:19:29 +0100 Subject: [PATCH 33/44] feat(test): added fixture macro --- src/tests/mod.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 802c364..6349652 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -78,3 +78,23 @@ macro_rules! assert_or_err { } pub use assert_or_err; + +#[macro_export] +macro_rules! fixture { + ($fixture:expr => | $($arg:ident),* | $body:expr) => { + #[allow(unused_parens)] + $fixture + .with(|($($arg),*)| { + $( + let $arg = $arg.clone(); + )* + + async move { + $body + } + }) + .await; + }; +} + +pub use fixture; From 2155f0d36c5ace79b21102a0f17014bf5ae2d8e0 Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 00:19:42 +0100 Subject: [PATCH 34/44] test: improved tests cleanup and logic Co-authored-by: cschen --- src/tests/client.rs | 321 ++++++++++++++++---------------------------- 1 file changed, 116 insertions(+), 205 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index 4be6d4b..cfdab4a 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -6,218 +6,135 @@ use crate::api::{AsyncReceiver, AsyncSender}; #[tokio::test] async fn test_workspace_creation_and_lookup() { - ClientFixture::of("alice") - .with(|client| { - let client = client.clone(); + super::fixture! { + ClientFixture::of("alice") => |client| { + let workspace_name = uuid::Uuid::new_v4().to_string(); + let wrong_name = uuid::Uuid::new_v4().to_string(); - async move { - let workspace_name = uuid::Uuid::new_v4().to_string(); - let wrong_name = uuid::Uuid::new_v4().to_string(); + client.create_workspace(&workspace_name).await?; + let wslist = client.fetch_owned_workspaces().await.unwrap_or_default(); + let ws = client.get_workspace(&workspace_name); + let ws_exists = ws.is_some(); + let ws_name_matches = ws.map_or(false, |x| x.id() == workspace_name); + let ws_wrong_name_doesnt_exist = client.get_workspace(&wrong_name).is_none(); - client.create_workspace(&workspace_name).await?; - let ws = client.get_workspace(&workspace_name); - assert_or_err!( - ws.is_some(), - "Failed to retrieve the workspace just created." - ); - assert_or_err!( - ws.unwrap().id() == workspace_name, - "The retreived workspace has a different name than the one created." - ); + let res = client.delete_workspace(&workspace_name).await; - assert_or_err!( - client.get_workspace(&wrong_name).is_none(), - "Looking up a non existent workspace returned something." - ); + assert_or_err!(ws_exists); + assert_or_err!(ws_name_matches); + assert_or_err!(ws_wrong_name_doesnt_exist); + assert_or_err!(res.is_ok()); + assert_or_err!(client.get_workspace(&workspace_name).is_none()); + assert_or_err!(wslist.len() == 1); + assert_or_err!(wslist.contains(&workspace_name)); - let wslist = client.fetch_owned_workspaces().await?; - assert_or_err!(wslist.len() == 1, "the amount of owned workspaces is not 1"); - assert_or_err!( - wslist.contains(&workspace_name), - "the newly create workspace is not present in the owned list." - ); - Ok(()) - } - }) - .await + Ok(()) + } + }; +} + +#[tokio::test] +async fn test_attach_and_leave_workspace() { + super::fixture! { + ClientFixture::of("alice") => |client| { + let workspace_name = uuid::Uuid::new_v4().to_string(); + + client.create_workspace(&workspace_name).await?; + + // leaving a workspace you are not attached to, returns true + let leave_workspace_before = client.leave_workspace(&workspace_name); + + let attach_workspace_that_exists = match client.attach_workspace(&workspace_name).await { + Ok(_) => true, + Err(e) => { + eprintln!("error attaching to workspace: {e}"); + false + }, + }; + + // leaving a workspace you are attached to, returns true + // when there is only one reference to it. + let leave_workspace_after = client.leave_workspace(&workspace_name); + + let _ = client.delete_workspace(&workspace_name).await; + + assert_or_err!(leave_workspace_before, "leaving a workspace you are not attached to returned false, should return true."); + assert_or_err!(attach_workspace_that_exists, "attaching a workspace that exists failed with error"); + assert_or_err!(leave_workspace_after, "leaving a workspace with a single reference returned false."); + + Ok(()) + } + } +} + +#[tokio::test] +async fn test_leave_workspace_with_dangling_ref() { + super::fixture! { + WorkspaceFixture::one("alice", "test-dangling-ref") => |client, workspace| { + assert_or_err!(client.leave_workspace(&workspace.id()) == false); + Ok(()) + } + } +} + +#[tokio::test] +async fn test_attach_after_leave() { + super::fixture! { + WorkspaceFixture::one("alice", "test-dangling-ref") => |client, workspace| { + client.leave_workspace(&workspace.id()); + assert_or_err!(client.attach_workspace(&workspace.id()).await.is_ok()); + Ok(()) + } + } +} + +#[tokio::test] +async fn test_active_workspaces() { + super::fixture! { + WorkspaceFixture::one("alice", "test-active-workspaces") => |client, workspace| { + assert_or_err!(client.active_workspaces().contains(&workspace.id())); + Ok(()) + } + } } #[tokio::test] async fn test_cant_create_same_workspace_more_than_once() { - ClientFixture::of("alice") - .with(|client| { - let client = client.clone(); - - async move { - let workspace_name = uuid::Uuid::new_v4().to_string(); - - client.create_workspace(&workspace_name).await?; - assert_or_err!( - client.create_workspace(workspace_name).await.is_err(), - "a workspace was created twice without error." - ); - Ok(()) - } - }) - .await -} - -#[tokio::test] -async fn test_workspace_attach_and_active_workspaces() { - ClientFixture::of("alice") - .with(|client| { - let client = client.clone(); - - async move { - let workspace_name = uuid::Uuid::new_v4().to_string(); - - client.create_workspace(&workspace_name).await?; - let ws = client.attach_workspace(&workspace_name).await?; - - assert_or_err!( - ws.id() == workspace_name, - "we attached to a workspace that has a different name than the one requested." - ); - assert_or_err!( - client.active_workspaces().contains(&workspace_name), - "the workspace is not present in the active workspaces list after attaching." - ); - Ok(()) - } - }) - .await + super::fixture! { + WorkspaceFixture::one("alice", "test-create-multiple-times") => |client, workspace| { + assert_or_err!(client.create_workspace(workspace.id()).await.is_err(), "created same workspace twice"); + Ok(()) + } + } } #[tokio::test] async fn test_attaching_to_non_existing_is_error() { - ClientFixture::of("alice") - .with(|client| { - let client = client.clone(); + super::fixture! { + ClientFixture::of("alice") => |client| { + let workspace_name = uuid::Uuid::new_v4().to_string(); - async move { - let workspace_name = uuid::Uuid::new_v4().to_string(); - - // we don't create any workspace. - // client.create_workspace(workspace_name).await?; - assert_or_err!( - client.attach_workspace(&workspace_name).await.is_err(), - "we attached to a non existing workspace." - ); - Ok(()) - } - }) - .await + // we don't create any workspace. + // client.create_workspace(workspace_name).await?; + assert_or_err!(client.attach_workspace(&workspace_name).await.is_err()); + Ok(()) + } + } } #[tokio::test] -async fn test_attach_and_leave_workspace_interactions() { - ClientFixture::of("alice") - .with(|client| { - let client = client.clone(); +async fn test_deleting_workspace_twice_is_an_error() { + super::fixture! { + WorkspaceFixture::one("alice", "test-delete-twice") => |client, workspace| { + let workspace_name = workspace.id(); - async move { - let workspace_name = uuid::Uuid::new_v4().to_string(); - - client.create_workspace(&workspace_name).await?; - // leaving a workspace you are not attached to, returns true - assert_or_err!( - client.leave_workspace(&workspace_name), - "leaving a workspace you are not attached to returned false, should return true." - ); - - // leaving a workspace you are attached to, returns true - // when there is only one reference to it. - client.attach_workspace(&workspace_name).await?; - assert_or_err!( - client.leave_workspace(&workspace_name), - "leaving a workspace with a single reference returned false." - ); - - // we are also implicitly testing repeated leaving and attaching - // to the same workspace. - let res = client.attach_workspace(&workspace_name).await; - assert_or_err!(res.is_ok(), "could not attach to the same workspace immediately after successfully leaving it."); - - // we should have an extra reference here, which should make the - // consume return false. - let _ws = res.unwrap(); - assert_or_err!( - !client.leave_workspace(&workspace_name), - "leaving a workspace while some reference still exist returned true." - ); - - Ok(()) - } - }) - .await + client.delete_workspace(&workspace_name).await?; + assert_or_err!(client.delete_workspace(&workspace_name).await.is_err()); + Ok(()) + } + } } -#[tokio::test] -async fn test_delete_empty_workspace() { - ClientFixture::of("alice") - .with(|client| { - let client = client.clone(); - - async move { - let workspace_name = uuid::Uuid::new_v4().to_string(); - - client.create_workspace(&workspace_name).await?; - client.delete_workspace(&workspace_name).await?; - - assert_or_err!( - client.get_workspace(&workspace_name).is_none(), - "a deleted workspaces was still available for lookup." - ); - assert_or_err!( - client.fetch_owned_workspaces().await?.is_empty(), - "a deleted workspace was still present in the owned workspaces list." - ); - - Ok(()) - } - }) - .await -} - -#[tokio::test] -async fn test_deleting_twice_or_non_existing_is_an_error() { - ClientFixture::of("alice") - .with(|client| { - let client = client.clone(); - - async move { - let workspace_name = uuid::Uuid::new_v4().to_string(); - - client.create_workspace(&workspace_name).await?; - client.delete_workspace(&workspace_name).await?; - - assert_or_err!( - client.delete_workspace(&workspace_name).await.is_err(), - "we could delete a workspace twice, or a non existing one." - ); - Ok(()) - } - }) - .await -} - -// Now we can begin using WorkspaceFixtures for with a single user. - -// #[tokio::test] -// async fn test_delete_workspace_with_users_attached() { -// WorkspaceFixture::one("bob", "to-be-deleted") -// .with(|(client, workspace): &mut (crate::Client, crate::Workspace)| { - -// async move { -// client.delete_workspace(workspace.id()).await?; - -// // TODO: I Don't know what should happen here. -// Ok(()) -// } -// }) -// .await -// } - #[tokio::test] async fn test_invite_user_to_workspace_and_invited_lookup() { WorkspaceFixture::one("bob", "workspace-di-bob") @@ -237,8 +154,7 @@ async fn test_invite_user_to_workspace_and_invited_lookup() { client_alice.current_user().name.clone(), ) .await - .is_err(), - "we invited a user to a non existing workspace."); + .is_err()); client_bob .invite_to_workspace( @@ -254,24 +170,19 @@ async fn test_invite_user_to_workspace_and_invited_lookup() { // the workspace does not appear in the owned workspaces for alice let user_list = workspace_bob.fetch_users().await?; - assert_or_err!(user_list.len() == 2, - "after inviting alice there should be only two users in a workspace."); + assert_or_err!(user_list.len() == 2); assert_or_err!(user_list .iter() - .any(|u| u.name == client_alice.current_user().name), - "alice was invited but is not present as one of the users."); + .any(|u| u.name == client_alice.current_user().name)); assert_or_err!(user_list .iter() - .any(|u| u.name == client_bob.current_user().name), - "bob owns the workspace but is not present in the workspace."); + .any(|u| u.name == client_bob.current_user().name)); let alice_owned_workspaces = client_alice.fetch_owned_workspaces().await?; let alice_invited_workspaces = client_alice.fetch_joined_workspaces().await?; - assert_or_err!(alice_owned_workspaces.is_empty(), - "The workspace alice was invided to is listed as owned for her."); - assert_or_err!(alice_invited_workspaces.contains(&workspace_bob.id()), - "The list of workspaces to which alice was invited to does not contain the one bob invited her to."); + assert_or_err!(alice_owned_workspaces.is_empty()); + assert_or_err!(alice_invited_workspaces.contains(&workspace_bob.id())); Ok(()) } }, From 59f9096fb6cd19c0f64b7cf75831503c30fd1bad Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 01:04:45 +0100 Subject: [PATCH 35/44] test: split lookup and create/delete tests --- src/tests/client.rs | 53 +++++++++++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index cfdab4a..ea1130e 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -5,28 +5,24 @@ use super::{ use crate::api::{AsyncReceiver, AsyncSender}; #[tokio::test] -async fn test_workspace_creation_and_lookup() { +async fn test_workspace_creation_and_deletion() { super::fixture! { ClientFixture::of("alice") => |client| { let workspace_name = uuid::Uuid::new_v4().to_string(); - let wrong_name = uuid::Uuid::new_v4().to_string(); client.create_workspace(&workspace_name).await?; - let wslist = client.fetch_owned_workspaces().await.unwrap_or_default(); - let ws = client.get_workspace(&workspace_name); - let ws_exists = ws.is_some(); - let ws_name_matches = ws.map_or(false, |x| x.id() == workspace_name); - let ws_wrong_name_doesnt_exist = client.get_workspace(&wrong_name).is_none(); + + // we can't error, so we return empty vec which will be interpreted as err + let workspace_list_before = client.fetch_owned_workspaces().await.unwrap_or_default(); let res = client.delete_workspace(&workspace_name).await; - assert_or_err!(ws_exists); - assert_or_err!(ws_name_matches); - assert_or_err!(ws_wrong_name_doesnt_exist); - assert_or_err!(res.is_ok()); - assert_or_err!(client.get_workspace(&workspace_name).is_none()); - assert_or_err!(wslist.len() == 1); - assert_or_err!(wslist.contains(&workspace_name)); + // we can and should err here, because empty vec will be counted as success! + let workspace_list_after = client.fetch_owned_workspaces().await?; + + assert_or_err!(workspace_list_before.contains(&workspace_name)); + assert_or_err!(res.is_ok(), "failed deleting workspace"); + assert_or_err!(workspace_list_after.contains(&workspace_name) == false); Ok(()) } @@ -67,6 +63,17 @@ async fn test_attach_and_leave_workspace() { } } +#[tokio::test] +async fn test_workspace_lookup() { + super::fixture! { + WorkspaceFixture::one("alice", "test-lookup") => |client, workspace| { + assert_or_err!(client.get_workspace(&workspace.id()).is_some()); + assert_or_err!(client.get_workspace(&uuid::Uuid::new_v4().to_string()).is_none()); + Ok(()) + } + } +} + #[tokio::test] async fn test_leave_workspace_with_dangling_ref() { super::fixture! { @@ -77,11 +84,25 @@ async fn test_leave_workspace_with_dangling_ref() { } } +#[tokio::test] +async fn test_lookup_after_leave() { + super::fixture! { + WorkspaceFixture::one("alice", "test-lookup-after-leave") => |client, workspace| { + client.leave_workspace(&workspace.id()); + assert_or_err!(client.get_workspace(&workspace.id()).is_none()); + Ok(()) + } + } +} + #[tokio::test] async fn test_attach_after_leave() { super::fixture! { - WorkspaceFixture::one("alice", "test-dangling-ref") => |client, workspace| { + WorkspaceFixture::one("alice", "test-attach-after-leave") => |client, workspace| { client.leave_workspace(&workspace.id()); + // TODO this is very server specific! disconnect may be instant or caught with next + // keepalive, let's arbitrarily say that after 20 seconds we should have been disconnected + tokio::time::sleep(std::time::Duration::from_secs(20)).await; assert_or_err!(client.attach_workspace(&workspace.id()).await.is_ok()); Ok(()) } @@ -137,7 +158,7 @@ async fn test_deleting_workspace_twice_is_an_error() { #[tokio::test] async fn test_invite_user_to_workspace_and_invited_lookup() { - WorkspaceFixture::one("bob", "workspace-di-bob") + WorkspaceFixture::one("alice", "test-invite") .with( |(client_bob, workspace_bob)| { let client_bob = client_bob.clone(); From f350bc0ea8bee2adcd33e0b4df63eff3a4862e24 Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 01:09:39 +0100 Subject: [PATCH 36/44] test: dont use is_ok(), propagate err --- src/tests/client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index ea1130e..3f65d8a 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -21,7 +21,7 @@ async fn test_workspace_creation_and_deletion() { let workspace_list_after = client.fetch_owned_workspaces().await?; assert_or_err!(workspace_list_before.contains(&workspace_name)); - assert_or_err!(res.is_ok(), "failed deleting workspace"); + res?; assert_or_err!(workspace_list_after.contains(&workspace_name) == false); Ok(()) @@ -103,7 +103,7 @@ async fn test_attach_after_leave() { // TODO this is very server specific! disconnect may be instant or caught with next // keepalive, let's arbitrarily say that after 20 seconds we should have been disconnected tokio::time::sleep(std::time::Duration::from_secs(20)).await; - assert_or_err!(client.attach_workspace(&workspace.id()).await.is_ok()); + client.attach_workspace(&workspace.id()).await?; Ok(()) } } From 2e05c3d75718484edf31bc0e995aceaf0c837d93 Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 01:24:46 +0100 Subject: [PATCH 37/44] test: split down invite_to_workspace_and_lookup --- src/tests/client.rs | 86 +++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 53 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index 3f65d8a..dfdbd84 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -63,6 +63,23 @@ async fn test_attach_and_leave_workspace() { } } +#[tokio::test] +async fn test_invite_user_to_workspace() { + let client_alice = ClientFixture::of("alice").setup().await.expect("failed setting up alice's client"); + let client_bob = ClientFixture::of("bob").setup().await.expect("failed setting up bob's client"); + let ws_name = uuid::Uuid::new_v4().to_string(); + + // after this we can't just fail anymore: we need to cleanup, so store errs + client_alice.create_workspace(&ws_name).await.expect("failed creating workspace"); + let could_invite = client_alice.invite_to_workspace(&ws_name, &client_bob.current_user().name).await; + let ws_list = client_bob.fetch_joined_workspaces().await.unwrap_or_default(); // can't fail, empty is err + let could_delete = client_alice.delete_workspace(&ws_name).await; + + could_invite.expect("could not invite bob"); + assert!(ws_list.contains(&ws_name)); + could_delete.expect("could not delete workspace"); +} + #[tokio::test] async fn test_workspace_lookup() { super::fixture! { @@ -157,61 +174,24 @@ async fn test_deleting_workspace_twice_is_an_error() { } #[tokio::test] -async fn test_invite_user_to_workspace_and_invited_lookup() { - WorkspaceFixture::one("alice", "test-invite") - .with( - |(client_bob, workspace_bob)| { - let client_bob = client_bob.clone(); - let workspace_bob = workspace_bob.clone(); - - async move { - let client_alice = ClientFixture::of("alice").setup().await?; - - let wrong_workspace_name = uuid::Uuid::new_v4().to_string(); - // inviting to a non existing workspace is an error - assert_or_err!(client_bob - .invite_to_workspace( - wrong_workspace_name, - client_alice.current_user().name.clone(), - ) - .await - .is_err()); - - client_bob - .invite_to_workspace( - workspace_bob.id(), - client_alice.current_user().name.clone(), - ) - .await?; - - // there are two users now in the workspace of bob - // alice is one of the users - // bob is one of the users - // the workspace appears in the joined workspaces for alice - // the workspace does not appear in the owned workspaces for alice - - let user_list = workspace_bob.fetch_users().await?; - assert_or_err!(user_list.len() == 2); - assert_or_err!(user_list - .iter() - .any(|u| u.name == client_alice.current_user().name)); - assert_or_err!(user_list - .iter() - .any(|u| u.name == client_bob.current_user().name)); - - let alice_owned_workspaces = client_alice.fetch_owned_workspaces().await?; - let alice_invited_workspaces = client_alice.fetch_joined_workspaces().await?; - - assert_or_err!(alice_owned_workspaces.is_empty()); - assert_or_err!(alice_invited_workspaces.contains(&workspace_bob.id())); - Ok(()) - } - }, - ) - .await +async fn test_cannot_invite_self() { + super::fixture! { + WorkspaceFixture::one("alice", "test-invite-self") => |client, workspace| { + assert_or_err!(client.invite_to_workspace(workspace.id(), &client.current_user().name).await.is_err()); + Ok(()) + } + } } -// Now we can use workspace fixtures with invite. +#[tokio::test] +async fn test_cannot_invite_to_nonexisting() { + super::fixture! { + WorkspaceFixture::two("alice", "bob", "test-invite-self") => |client, _ws, client_bob, _workspace_bob| { + assert_or_err!(client.invite_to_workspace(uuid::Uuid::new_v4().to_string(), &client_bob.current_user().name).await.is_err()); + Ok(()) + } + } +} #[tokio::test] async fn cannot_delete_others_workspaces() { From b58a11f06a057064497b36319799230841cdd49d Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 01:27:07 +0100 Subject: [PATCH 38/44] ci: just run on push --- .github/workflows/test.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c34621b..7a27779 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2,8 +2,6 @@ name: test on: push: - pull_request: - types: [synchronize, review_requested] env: CARGO_TERM_COLOR: always From b549f82ce55c5760c5bdd70ce5d64296ae693f9e Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 01:29:19 +0100 Subject: [PATCH 39/44] test: even more time to disconnect? --- src/tests/client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index dfdbd84..b5da984 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -118,8 +118,8 @@ async fn test_attach_after_leave() { WorkspaceFixture::one("alice", "test-attach-after-leave") => |client, workspace| { client.leave_workspace(&workspace.id()); // TODO this is very server specific! disconnect may be instant or caught with next - // keepalive, let's arbitrarily say that after 20 seconds we should have been disconnected - tokio::time::sleep(std::time::Duration::from_secs(20)).await; + // keepalive, let's arbitrarily say that after 30 seconds we should have been disconnected + tokio::time::sleep(std::time::Duration::from_secs(30)).await; client.attach_workspace(&workspace.id()).await?; Ok(()) } From ed0e05ffe249c84e933c9770c2e8386136101925 Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 01:30:49 +0100 Subject: [PATCH 40/44] test: drop workspace so that it disconnects --- src/tests/client.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index b5da984..fbdf648 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -116,11 +116,13 @@ async fn test_lookup_after_leave() { async fn test_attach_after_leave() { super::fixture! { WorkspaceFixture::one("alice", "test-attach-after-leave") => |client, workspace| { - client.leave_workspace(&workspace.id()); + let ws_name = workspace.id(); + drop(workspace); + client.leave_workspace(&ws_name); // TODO this is very server specific! disconnect may be instant or caught with next - // keepalive, let's arbitrarily say that after 30 seconds we should have been disconnected - tokio::time::sleep(std::time::Duration::from_secs(30)).await; - client.attach_workspace(&workspace.id()).await?; + // keepalive, let's arbitrarily say that after 20 seconds we should have been disconnected + tokio::time::sleep(std::time::Duration::from_secs(20)).await; + client.attach_workspace(&ws_name).await?; Ok(()) } } From bf9ea18d671bd2914faea778394b37369f08d743 Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 01:42:21 +0100 Subject: [PATCH 41/44] test: dont use ws fixture coz leave wont work --- src/tests/client.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index fbdf648..1b0d362 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -115,14 +115,23 @@ async fn test_lookup_after_leave() { #[tokio::test] async fn test_attach_after_leave() { super::fixture! { - WorkspaceFixture::one("alice", "test-attach-after-leave") => |client, workspace| { - let ws_name = workspace.id(); - drop(workspace); - client.leave_workspace(&ws_name); + ClientFixture::of("alice") => |client| { + let ws_name = uuid::Uuid::new_v4().to_string(); + client.create_workspace(&ws_name).await?; + + let could_attach = client.attach_workspace(&ws_name).await.is_ok(); + let clean_leave = client.leave_workspace(&ws_name); // TODO this is very server specific! disconnect may be instant or caught with next // keepalive, let's arbitrarily say that after 20 seconds we should have been disconnected tokio::time::sleep(std::time::Duration::from_secs(20)).await; - client.attach_workspace(&ws_name).await?; + let could_attach_again = client.attach_workspace(&ws_name).await; + let could_delete = client.delete_workspace(&ws_name).await; + + assert_or_err!(could_attach); + assert_or_err!(clean_leave); + could_attach_again?; + could_delete?; + Ok(()) } } From 97ee48629d9b1ca8f8dcd4907b12e827ad7ad95b Mon Sep 17 00:00:00 2001 From: alemi Date: Tue, 5 Nov 2024 01:48:45 +0100 Subject: [PATCH 42/44] test: wait 1s so it can catch up in case its slow --- src/tests/client.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tests/client.rs b/src/tests/client.rs index 1b0d362..d0ffa06 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -320,8 +320,10 @@ async fn test_content_converges() { x??; } - // TODO is there a nicer way to make sure we received all changes? + // test runners may be slow, give 1s to catch up, just in case + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + // TODO is there a nicer way to make sure we received all changes? for i in 0..20 { tokio::time::sleep(std::time::Duration::from_millis(200)).await; match bob.try_recv().await? { From fb6e1cdeea9231bbe922e2356b733ae42bed1d85 Mon Sep 17 00:00:00 2001 From: zaaarf Date: Tue, 5 Nov 2024 19:17:01 +0100 Subject: [PATCH 43/44] feat: use single tuple in buffer fixture too --- src/tests/fixtures.rs | 62 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 12 deletions(-) diff --git a/src/tests/fixtures.rs b/src/tests/fixtures.rs index 9a0ae60..b52712c 100644 --- a/src/tests/fixtures.rs +++ b/src/tests/fixtures.rs @@ -124,8 +124,25 @@ impl ScopedFixture<(crate::Client, crate::Workspace)> for WorkspaceFixture { } } -impl ScopedFixture<(crate::Client, crate::Workspace, crate::Client, crate::Workspace)> for WorkspaceFixture { - async fn setup(&mut self) -> Result<(crate::Client, crate::Workspace, crate::Client, crate::Workspace), Box> { +impl + ScopedFixture<( + crate::Client, + crate::Workspace, + crate::Client, + crate::Workspace, + )> for WorkspaceFixture +{ + async fn setup( + &mut self, + ) -> Result< + ( + crate::Client, + crate::Workspace, + crate::Client, + crate::Workspace, + ), + Box, + > { let client = ClientFixture::of(&self.user).setup().await?; let invitee_client = ClientFixture::of( &self @@ -146,7 +163,12 @@ impl ScopedFixture<(crate::Client, crate::Workspace, crate::Client, crate::Works async fn cleanup( &mut self, - resource: Option<(crate::Client, crate::Workspace, crate::Client, crate::Workspace)>, + resource: Option<( + crate::Client, + crate::Workspace, + crate::Client, + crate::Workspace, + )>, ) { if let Some((client, _, _, _)) = resource { client.leave_workspace(&self.workspace); @@ -195,16 +217,24 @@ impl BufferFixture { impl ScopedFixture<( - (crate::Client, crate::Workspace, crate::buffer::Controller), - (crate::Client, crate::Workspace, crate::buffer::Controller), + crate::Client, + crate::Workspace, + crate::buffer::Controller, + crate::Client, + crate::Workspace, + crate::buffer::Controller, )> for BufferFixture { async fn setup( &mut self, ) -> Result< ( - (crate::Client, crate::Workspace, crate::buffer::Controller), - (crate::Client, crate::Workspace, crate::buffer::Controller), + crate::Client, + crate::Workspace, + crate::buffer::Controller, + crate::Client, + crate::Workspace, + crate::buffer::Controller, ), Box, > { @@ -230,19 +260,27 @@ impl let invitee_buffer = invitee_workspace.attach_buffer(&self.buffer).await?; Ok(( - (client, workspace, buffer), - (invitee_client, invitee_workspace, invitee_buffer), + client, + workspace, + buffer, + invitee_client, + invitee_workspace, + invitee_buffer, )) } async fn cleanup( &mut self, resource: Option<( - (crate::Client, crate::Workspace, crate::buffer::Controller), - (crate::Client, crate::Workspace, crate::buffer::Controller), + crate::Client, + crate::Workspace, + crate::buffer::Controller, + crate::Client, + crate::Workspace, + crate::buffer::Controller, )>, ) { - if let Some(((client, _, _), (_, _, _))) = resource { + if let Some((client, _, _, _, _, _)) = resource { // buffer deletion is implied in workspace deletion client.leave_workspace(&self.workspace); if let Err(e) = client.delete_workspace(&self.workspace).await { From e25b82aefb48610dea2af04fda07444feb2f62d5 Mon Sep 17 00:00:00 2001 From: zaaarf Date: Tue, 5 Nov 2024 19:17:22 +0100 Subject: [PATCH 44/44] chore: cargo fmt --- src/ffi/js/workspace.rs | 2 +- src/tests/client.rs | 52 +++++++++++++++++++++++++---------------- src/tests/server.rs | 50 ++++++++++----------------------------- 3 files changed, 45 insertions(+), 59 deletions(-) diff --git a/src/ffi/js/workspace.rs b/src/ffi/js/workspace.rs index d1ba78d..e862322 100644 --- a/src/ffi/js/workspace.rs +++ b/src/ffi/js/workspace.rs @@ -121,7 +121,7 @@ impl Workspace { })?; self.callback(move |controller: Workspace| { tsfn.call(controller.clone(), ThreadsafeFunctionCallMode::Blocking); //check this with tracing also we could use Ok(event) to get the error - // If it blocks the main thread too many time we have to change this + // If it blocks the main thread too many time we have to change this }); Ok(()) diff --git a/src/tests/client.rs b/src/tests/client.rs index d0ffa06..dcd4dc0 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -65,14 +65,28 @@ async fn test_attach_and_leave_workspace() { #[tokio::test] async fn test_invite_user_to_workspace() { - let client_alice = ClientFixture::of("alice").setup().await.expect("failed setting up alice's client"); - let client_bob = ClientFixture::of("bob").setup().await.expect("failed setting up bob's client"); + let client_alice = ClientFixture::of("alice") + .setup() + .await + .expect("failed setting up alice's client"); + let client_bob = ClientFixture::of("bob") + .setup() + .await + .expect("failed setting up bob's client"); let ws_name = uuid::Uuid::new_v4().to_string(); // after this we can't just fail anymore: we need to cleanup, so store errs - client_alice.create_workspace(&ws_name).await.expect("failed creating workspace"); - let could_invite = client_alice.invite_to_workspace(&ws_name, &client_bob.current_user().name).await; - let ws_list = client_bob.fetch_joined_workspaces().await.unwrap_or_default(); // can't fail, empty is err + client_alice + .create_workspace(&ws_name) + .await + .expect("failed creating workspace"); + let could_invite = client_alice + .invite_to_workspace(&ws_name, &client_bob.current_user().name) + .await; + let ws_list = client_bob + .fetch_joined_workspaces() + .await + .unwrap_or_default(); // can't fail, empty is err let could_delete = client_alice.delete_workspace(&ws_name).await; could_invite.expect("could not invite bob"); @@ -224,22 +238,20 @@ async fn cannot_delete_others_workspaces() { #[tokio::test] async fn test_buffer_search() { WorkspaceFixture::one("alice", "test-buffer-search") - .with( - |(_, workspace_alice)| { - let buffer_name = uuid::Uuid::new_v4().to_string(); - let workspace_alice = workspace_alice.clone(); + .with(|(_, workspace_alice)| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); - async move { - workspace_alice.create_buffer(&buffer_name).await?; - assert_or_err!(!workspace_alice - .search_buffers(Some(&buffer_name[0..4])) - .is_empty()); - assert_or_err!(workspace_alice.search_buffers(Some("_")).is_empty()); - workspace_alice.delete_buffer(&buffer_name).await?; - Ok(()) - } - }, - ) + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!(!workspace_alice + .search_buffers(Some(&buffer_name[0..4])) + .is_empty()); + assert_or_err!(workspace_alice.search_buffers(Some("_")).is_empty()); + workspace_alice.delete_buffer(&buffer_name).await?; + Ok(()) + } + }) .await; } diff --git a/src/tests/server.rs b/src/tests/server.rs index 09bcbe0..ebcf4ce 100644 --- a/src/tests/server.rs +++ b/src/tests/server.rs @@ -3,44 +3,21 @@ use super::{ fixtures::{ClientFixture, ScopedFixture, WorkspaceFixture}, }; -// Moved this in client for now. -// #[tokio::test] -// async fn cannot_delete_others_workspaces() { -// WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-workspaces") -// .with(|((_, ws_alice), (client_bob, _))| { -// let ws_alice = ws_alice.clone(); -// let client_bob = client_bob.clone(); -// async move { -// assert_or_err!( -// client_bob.delete_workspace(&ws_alice.id()).await.is_err(), -// "bob was allowed to delete a workspace he didn't own!" -// ); -// Ok(()) -// } - -// }) -// .await -// } - #[tokio::test] async fn test_buffer_create() { WorkspaceFixture::one("alice", "test-buffer-create") - .with( - |(_, workspace_alice)| { - let buffer_name = uuid::Uuid::new_v4().to_string(); - let workspace_alice = workspace_alice.clone(); + .with(|(_, workspace_alice)| { + let buffer_name = uuid::Uuid::new_v4().to_string(); + let workspace_alice = workspace_alice.clone(); - async move { - workspace_alice.create_buffer(&buffer_name).await?; - assert_or_err!( - vec![buffer_name.clone()] == workspace_alice.fetch_buffers().await? - ); - workspace_alice.delete_buffer(&buffer_name).await?; + async move { + workspace_alice.create_buffer(&buffer_name).await?; + assert_or_err!(vec![buffer_name.clone()] == workspace_alice.fetch_buffers().await?); + workspace_alice.delete_buffer(&buffer_name).await?; - Ok(()) - } - }, - ) + Ok(()) + } + }) .await; } @@ -62,7 +39,7 @@ async fn test_cant_create_buffer_twice() { } #[tokio::test] -#[ignore] // TODO server has no concept of buffer ownership! +#[ignore] // TODO reference server has no concept of buffer ownership yet! async fn cannot_delete_others_buffers() { WorkspaceFixture::two("alice", "bob", "test-cannot-delete-others-buffers") .with(|(_, workspace_alice, _, workspace_bob)| { @@ -84,10 +61,7 @@ async fn test_workspace_interactions() { if let Err(e) = async { let client_alice = ClientFixture::of("alice").setup().await?; let client_bob = ClientFixture::of("bob").setup().await?; - let workspace_name = format!( - "test-workspace-interactions-{}", - uuid::Uuid::new_v4() - ); + let workspace_name = format!("test-workspace-interactions-{}", uuid::Uuid::new_v4()); client_alice.create_workspace(&workspace_name).await?; let owned_workspaces = client_alice.fetch_owned_workspaces().await?;