diff --git a/lsp/src/watching/path_store.rs b/lsp/src/watching/path_store.rs index 07d56d9..9299037 100644 --- a/lsp/src/watching/path_store.rs +++ b/lsp/src/watching/path_store.rs @@ -101,7 +101,7 @@ mod tests { use assert_fs::{NamedTempFile, TempDir, prelude::*}; use common::sync::{Error, FileError, MockGithubClient}; use mockall::predicate; - use notify::event::{AccessKind, DataChange}; + use notify::event::{AccessKind, AccessMode, CreateKind, DataChange, RemoveKind}; use paste::paste; use tokio::runtime::Runtime; @@ -271,11 +271,20 @@ mod tests { fn test_non_modify_event_handling() -> Result<()> { let ctx = MockWatchedSet::new_context(); ctx.expect().returning(move |event_handler: EventHandler| { - let event = Event::new(EventKind::Access(AccessKind::Read)); + let non_modify_events = [ + Event::new(EventKind::Access(AccessKind::Read)), + Event::new(EventKind::Access(AccessKind::Open(AccessMode::Any))), + Event::new(EventKind::Create(CreateKind::File)), + Event::new(EventKind::Create(CreateKind::Folder)), + Event::new(EventKind::Remove(RemoveKind::File)), + Event::new(EventKind::Remove(RemoveKind::Folder)), + ]; let rt = Runtime::new()?; rt.block_on(async { - event_handler(event).await; + for event in non_modify_events { + event_handler(event).await; + } }); let mock_watched_set = MockWatchedSet::default(); @@ -359,7 +368,7 @@ mod tests { } #[test] - fn test_non_modify_event_handling_with_sync_success() -> Result<()> { + fn test_modify_event_handling_with_sync_success() -> Result<()> { let temp_file = NamedTempFile::new("settings.json")?; temp_file.write_str(r#"{ "hello": "kitty" }"#)?; let temp_file_path = temp_file.path().to_path_buf(); @@ -395,7 +404,7 @@ mod tests { } #[test] - fn test_non_modify_event_handling_with_sync_failure() -> Result<()> { + fn test_modify_event_handling_with_sync_failure() -> Result<()> { let temp_file = NamedTempFile::new("settings.json")?; temp_file.write_str(r#"{ "hello": "kitty" }"#)?; let temp_file_path = temp_file.path().to_path_buf(); diff --git a/lsp/src/watching/path_watcher.rs b/lsp/src/watching/path_watcher.rs index ec3501e..f330fff 100644 --- a/lsp/src/watching/path_watcher.rs +++ b/lsp/src/watching/path_watcher.rs @@ -32,6 +32,7 @@ impl PathWatcher { // so need to enter tokio async context explicitly handle.block_on(async { if tx.send(res).await.is_err() { + // TODO: propagate this error to the path store level so it can be displayed to a Zed user error!("Path watcher receiver dropped or closed"); } }); @@ -63,13 +64,17 @@ impl PathWatcher { while let Some(res) = rx.recv().await { match res { Ok(event) => (event_handler)(event).await, - Err(e) => error!("Path watcher error: {}", e), + Err(e) => { + // TODO: propagate this error to the path store level so it can be displayed to a Zed user + error!("Path watcher error: {}", e); + } } } }); } pub fn watch(&self, path: &Path) -> Result<()> { + // println!("Watcher is running: {}", self.watcher.lock().unwrap()) self.watcher .lock() .map_err(|_| anyhow!("Path watcher mutex is poisoned"))? @@ -88,35 +93,130 @@ impl PathWatcher { } } +#[cfg(test)] mod tests { + use std::{ + sync::atomic::{AtomicBool, Ordering}, + time::Duration, + }; + + use anyhow::Result; + use assert_fs::{TempDir, prelude::*}; + + use crate::watching::{EventHandler, PathWatcher}; + + macro_rules! init_event_handler { + ($var:ident) => { + static EVENT_HANDLER_CALLED: AtomicBool = AtomicBool::new(false); + let $var: EventHandler = Box::new(|_| { + Box::pin(async { + set_event_handler_called!(); + }) + }); + }; + } + + macro_rules! clear_event_handler_called { + () => { + EVENT_HANDLER_CALLED.store(false, Ordering::Relaxed); + }; + } + + macro_rules! set_event_handler_called { + () => { + EVENT_HANDLER_CALLED.store(true, Ordering::Relaxed); + }; + } + + async fn assert_event_handler_called_with( + event_handler_called: &AtomicBool, + value: bool, + ) -> bool { + let mut duration = 100; + while duration <= 2000 { + if event_handler_called.load(Ordering::Relaxed) == value { + return true; + } + tokio::time::sleep(Duration::from_millis(duration)).await; + duration *= 2; + } + + false + } + + macro_rules! assert_event_handler_called { + () => { + assert_event_handler_called_with(&EVENT_HANDLER_CALLED, true).await; + }; + } + + macro_rules! assert_event_handler_not_called { + () => { + assert_event_handler_called_with(&EVENT_HANDLER_CALLED, false).await; + }; + } + + #[tokio::test] + async fn test_file_event_inside_watched_path_is_caught() -> Result<()> { + init_event_handler!(event_handler); + + let mut path_watcher = PathWatcher::new(event_handler)?; + let dir_watched = TempDir::new()?; + + path_watcher.start(); + path_watcher.watch(dir_watched.path())?; + dir_watched.child("file.txt").write_str("Hello, world!\n")?; + + assert_event_handler_called!(); - /* - Tests TODO - - events handling - - test create file does not trigger event handler - - create a store with the MockGithubClient passed - - start watcher - - add a new path to watch (assert_fs::TempDir), maybe with an already existing file - - create a new file in that dir - - ensure event was not triggered (MockGithubClient) - - test delete file does not trigger event handler - - create a store with the MockGithubClient passed - - start watcher - - add a new path to watch (assert_fs::TempDir), with an already existing file - - delete the file - - ensure event was not triggered (MockGithubClient) - - test modify file data triggers event handler - - create a store with the MockGithubClient passed - - start watcher - - add a new path to watch (assert_fs::TempDir), with an already existing file - - modify the file data - - ensure event was triggered (MockGithubClient) - - test modify file data outside of watched paths does not trigger event handler - - create a store with the MockGithubClient passed - - start watcher - - add a new path to watch (assert_fs::TempDir) - - create another assert_fs::TempDir with an existing file - - modify that file data - - ensure event was not triggered (MockGithubClient) - */ + Ok(()) + } + + #[tokio::test] + async fn test_file_event_outside_of_watched_path_is_ignored() -> Result<()> { + init_event_handler!(event_handler); + + let mut path_watcher = PathWatcher::new(event_handler)?; + + let root_dir = TempDir::new()?; + let child_dir_watched = root_dir.child("with_changes"); + child_dir_watched.create_dir_all()?; + let child_dir_ignored = root_dir.child("ignored"); + child_dir_ignored.create_dir_all()?; + + path_watcher.start(); + path_watcher.watch(child_dir_watched.path())?; + child_dir_ignored + .child("file.txt") + .write_str("Hello, world!")?; + + assert_event_handler_not_called!(); + + Ok(()) + } + + #[tokio::test] + async fn test_unwatch_successful() -> Result<()> { + init_event_handler!(event_handler); + + let mut path_watcher = PathWatcher::new(event_handler)?; + let dir_watched = TempDir::new()?; + + path_watcher.start(); + path_watcher.watch(dir_watched.path())?; + dir_watched.child("file.txt").write_str("Hello, world!\n")?; + + assert_event_handler_called!(); + + clear_event_handler_called!(); + + path_watcher.unwatch(dir_watched.path())?; + dir_watched + .child("another_file.txt") + .write_str("Hello, kitty!\n")?; + + assert_event_handler_not_called!(); + + Ok(()) + } } diff --git a/lsp/src/watching/watched_set.rs b/lsp/src/watching/watched_set.rs index d463c24..d0bd4cc 100644 --- a/lsp/src/watching/watched_set.rs +++ b/lsp/src/watching/watched_set.rs @@ -73,7 +73,10 @@ impl WatchedSet { mod tests { #![allow(clippy::unwrap_used, clippy::expect_used)] - use std::{path::PathBuf, sync::OnceLock}; + use std::{ + path::PathBuf, + sync::atomic::{AtomicBool, Ordering}, + }; use anyhow::Result; use notify::{Event, EventKind}; @@ -87,13 +90,11 @@ mod tests { #[test] fn test_new_successful() -> Result<()> { - static EVENT_HANDLER_CALLED: OnceLock = OnceLock::new(); + static EVENT_HANDLER_CALLED: AtomicBool = AtomicBool::new(false); let event_handler: EventHandler = Box::new(|_| { Box::pin(async { - EVENT_HANDLER_CALLED - .set(true) - .expect("Flag was already set"); + EVENT_HANDLER_CALLED.store(true, Ordering::Relaxed); }) }); @@ -108,7 +109,7 @@ mod tests { let set = WatchedSet::new(event_handler)?; assert!(set.paths.is_empty()); - assert!(EVENT_HANDLER_CALLED.get().expect("Flag was not set")); // testing that WatchedSet passes the event handler to PathWatcher + assert!(EVENT_HANDLER_CALLED.load(Ordering::Relaxed)); // testing that WatchedSet passes the event handler to PathWatcher Ok(()) }