From 3afbc6b04766a836f8d599f334325023a1d1662a Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Thu, 8 Jan 2026 19:34:35 +0200 Subject: [PATCH 1/9] Rename Store to PathStore --- lsp/src/app_state.rs | 6 +++--- lsp/src/watching.rs | 4 ++-- lsp/src/watching/{store.rs => path_store.rs} | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 15 deletions(-) rename lsp/src/watching/{store.rs => path_store.rs} (98%) diff --git a/lsp/src/app_state.rs b/lsp/src/app_state.rs index 9c36dd9..7311f01 100644 --- a/lsp/src/app_state.rs +++ b/lsp/src/app_state.rs @@ -3,17 +3,17 @@ use std::sync::Arc; use anyhow::Result; use common::sync::GithubClient; -use crate::watching::Store; +use crate::watching::PathStore; #[derive(Debug)] pub struct AppState { - pub watcher_store: Arc, + pub watcher_store: Arc, } impl AppState { pub fn new(gist_id: String, github_token: String) -> Result { let sync_client = Arc::new(GithubClient::new(gist_id, github_token)?); - let watcher_store = Arc::new(Store::new(sync_client)?); + let watcher_store = Arc::new(PathStore::new(sync_client)?); Ok(Self { watcher_store }) } diff --git a/lsp/src/watching.rs b/lsp/src/watching.rs index facd183..b763e9e 100644 --- a/lsp/src/watching.rs +++ b/lsp/src/watching.rs @@ -1,7 +1,7 @@ +mod path_store; mod path_watcher; -mod store; mod zed_config; +pub use path_store::*; pub use path_watcher::*; -pub use store::*; pub use zed_config::*; diff --git a/lsp/src/watching/store.rs b/lsp/src/watching/path_store.rs similarity index 98% rename from lsp/src/watching/store.rs rename to lsp/src/watching/path_store.rs index 16ab0c2..a2caf3a 100644 --- a/lsp/src/watching/store.rs +++ b/lsp/src/watching/path_store.rs @@ -27,12 +27,12 @@ impl WatchedSet { } #[derive(Debug)] -pub struct Store { +pub struct PathStore { // behind mutex to control the simultaneous change of paths set and path watcher watched_set: Mutex, } -impl Store { +impl PathStore { pub fn new(client: Arc) -> Result { let event_handler = Box::new(move |event| { let client_clone = Arc::clone(&client); @@ -57,6 +57,14 @@ impl Store { }) } + pub async fn start_watcher(&self) { + let mut watched_set = self.watched_set.lock().await; + + watched_set.watcher.start(); + } + + // no need to stop watcher or clear the store because it will be stopped (when dropped) on the store drop + pub async fn watch(&self, file_path: PathBuf) -> anyhow::Result<()> { { let mut watched_set = self.watched_set.lock().await; @@ -89,14 +97,6 @@ impl Store { Ok(()) } - - pub async fn start_watcher(&self) { - let mut watched_set = self.watched_set.lock().await; - - watched_set.watcher.start(); - } - - // no need to stop watcher or clear the store because it will be stopped (when dropped) on the store drop } fn process_event(event: &Event) -> Result> { From f80669ee47f450444d1c25a09ef41f1ab26b7cf2 Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Thu, 8 Jan 2026 19:34:48 +0200 Subject: [PATCH 2/9] Update ROADMAP.md --- ROADMAP.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index fa1c9cb..9798531 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -9,11 +9,18 @@ - [x] Write a test a script to locally build the LSP binary and copy it to the Zed's folder for the extension (see Zed Discord Presence LSP server info for the full path, also check out the Zed sources, namely paths.rs) - [x] Forbid the usage of unwrap and expect for Option, Result - [x] Use serde_json from zed_extension_api, not directly +- [x] To support multiple sync providers, implement a Client trait and use it for all sync operations. Move it to the "common" crate. - [~] **Add unit and integration tests** - [ ] Test all error-returning code paths and ensure that all error conditions are either properly logged and/or reported back to Zed in form or a JSON-RPC error response - [ ] Revamp README before publishing the extension to make it easier to consume and start using the extension immediately +- [ ] Add secrecy crate and use it for all usages of the Github token + +## LSP server + - [ ] Manually save a settings file on its open (before adding to the watch list) to handle the case when the LSP server is restarted after the initialization_options are changes in settings.json file. - [ ] Ensure that restarting or shutting down the LSP server doesn't prevent the last coming updates from getting synced; otherwise, mitigate that +- [ ] Report a sync error in a visible way (crash the server? know how to report an LSP error inside Zed?) +- [ ] Create a true integration test when a server is spawned, it's fed with JSON-RPC messages, and Github API URL is mocked via env var to point to a local mock server as another process - [ ] After implementing naive changes persistence (sync files as FS events come), seek the ways to improve it (e.g. queuing events) – [ ] (experimental) Rewrite the LSP server to use structured async concurrency, thread-per-code async runtime, and get rid of Arc's around every data structure - [ ] Add secrecy crate and use it for all usages of the Github token @@ -32,9 +39,9 @@ ## CI - [ ] Enable ["cancel-in-progress"](https://share.google/Vk4zJKCbkerc5BAfC) for CI builds -- [ ] Add matrix to compile for Windows on ARM64 -- [ ] Speed up the build if possible (caching, Docker images, etc.) -- [ ] Extract "compile" as a separate local Github action +- [x] Add matrix to compile for Windows on ARM64 +- [x] Speed up the build if possible (caching, Docker images, etc.) +- [ ] ~~Extract "compile" as a separate local Github action~~ No need for that, because "compile" is used only for release. - [ ] Optimize binaries size ## Documentation From 8dd018e2cea04dd383cd5175c0d0aeede99d14ce Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Thu, 8 Jan 2026 19:41:42 +0200 Subject: [PATCH 3/9] Add unit tests blueprint for PathStore --- lsp/src/watching/path_store.rs | 53 ++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/lsp/src/watching/path_store.rs b/lsp/src/watching/path_store.rs index a2caf3a..7ccc798 100644 --- a/lsp/src/watching/path_store.rs +++ b/lsp/src/watching/path_store.rs @@ -115,3 +115,56 @@ fn process_event(event: &Event) -> Result> { Ok(Some(LocalFileData::new(path, body)?)) } + +#[cfg(test)] +mod tests { + /* + - new + - test successful creation + - create a store with the MockGithubClient passed + - test unsuccessful creation is watched set creation failed + + - start watcher + - test successful start watcher + - watcher started (mock for PathWatcher) + + - watch + - test successful watch new path + - new path passed to path watcher for watch (mock for PathWatcher) + - new path added to watched set (mock for WatchedSet) + - test failure watch path already watched + + - unwatch + - test successful unwatch + - test failure unwatch path not watched + - path passed to path watcher for unwatch (mock for PathWatcher) + - path removed from watched set (mock for WatchedSet) + + - 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) + */ +} From aa5648414670d180a65e365b20280759f6f59def Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Thu, 8 Jan 2026 20:09:53 +0200 Subject: [PATCH 4/9] Improve and fix cargo nextest tasks --- .zed/tasks.json | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/.zed/tasks.json b/.zed/tasks.json index 463f529..0f630b0 100644 --- a/.zed/tasks.json +++ b/.zed/tasks.json @@ -11,23 +11,14 @@ // "NEXTEST_SUCCESS_OUTPUT": "immediate" }, }, - { - "label": "cargo nextest tests::$ZED_CUSTOM_RUST_TEST_NAME", - "command": "cargo nextest run tests::$ZED_CUSTOM_RUST_TEST_NAME", - "tags": ["rust-test"], - "env": { - // "NEXTEST_FAILURE_OUTPUT": "immediate", - // "NEXTEST_SUCCESS_OUTPUT": "immediate" - }, - }, { "label": "cargo nextest $ZED_STEM (package: $ZED_CUSTOM_RUST_PACKAGE)", "command": "cargo nextest run -p $ZED_CUSTOM_RUST_PACKAGE $ZED_STEM::$ZED_SYMBOL", "tags": ["rust-mod-test"], }, { - "label": "cargo nextest tests", - "command": "cargo nextest run tests", + "label": "cargo nextest tests (crate root)", + "command": "cargo nextest run -E 'test(/^tests/)'", "tags": ["rust-mod-test"], }, { From b94a13d7b7588f85bc7689e9121ed1e20e00e9f2 Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Thu, 8 Jan 2026 20:10:23 +0200 Subject: [PATCH 5/9] Add test for PathStore successful creation --- lsp/Cargo.toml | 3 +++ lsp/src/watching/path_store.rs | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/lsp/Cargo.toml b/lsp/Cargo.toml index 490f6dc..3d3b1c8 100644 --- a/lsp/Cargo.toml +++ b/lsp/Cargo.toml @@ -21,3 +21,6 @@ debug-ignore = "1.0.5" notify = "8.2.0" tower-lsp = "0.20.0" # TODO: upgrade tracing-subscriber = { version = "0.3.20", features = ["env-filter", "json", "chrono"] } + +[dev-dependencies] +common = { path = "../common", features = ["test-support"] } diff --git a/lsp/src/watching/path_store.rs b/lsp/src/watching/path_store.rs index 7ccc798..809b9c3 100644 --- a/lsp/src/watching/path_store.rs +++ b/lsp/src/watching/path_store.rs @@ -118,6 +118,15 @@ fn process_event(event: &Event) -> Result> { #[cfg(test)] mod tests { + use common::sync::MockGithubClient; + + use super::*; + + #[tokio::test] + async fn test_successful_creation() { + assert!(PathStore::new(Arc::new(MockGithubClient::default())).is_ok()); + } + /* - new - test successful creation From 95756b396317dcaa73d9307a1f348e149944a931 Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Fri, 9 Jan 2026 16:49:44 +0200 Subject: [PATCH 6/9] Extract WatchedSet, refactor and simplify async and locking --- Cargo.lock | 1 + Cargo.toml | 1 + lsp/Cargo.toml | 1 + lsp/src/app_state.rs | 6 +-- lsp/src/backend.rs | 48 +++++++++++----------- lsp/src/watching.rs | 2 + lsp/src/watching/path_store.rs | 70 ++++++--------------------------- lsp/src/watching/watched_set.rs | 66 +++++++++++++++++++++++++++++++ 8 files changed, 113 insertions(+), 82 deletions(-) create mode 100644 lsp/src/watching/watched_set.rs diff --git a/Cargo.lock b/Cargo.lock index dcc0f19..8b1ae23 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4160,6 +4160,7 @@ dependencies = [ "common", "debug-ignore", "jsonc-parser", + "mockall_double", "notify", "octocrab", "paths", diff --git a/Cargo.toml b/Cargo.toml index 5a793b0..d681748 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ mockall = { version = "0.14.0" } assert_fs = "1.1.3" ctor = { version = "0.6.3", default-features = false, features = ["proc_macro"] } paths = { git = "https://github.com/zed-industries/zed.git" } +mockall_double = "0.3.1" [dependencies] zed_extension_api = { workspace = true } diff --git a/lsp/Cargo.toml b/lsp/Cargo.toml index 3d3b1c8..c4cd12d 100644 --- a/lsp/Cargo.toml +++ b/lsp/Cargo.toml @@ -21,6 +21,7 @@ debug-ignore = "1.0.5" notify = "8.2.0" tower-lsp = "0.20.0" # TODO: upgrade tracing-subscriber = { version = "0.3.20", features = ["env-filter", "json", "chrono"] } +mockall_double = { workspace = true } [dev-dependencies] common = { path = "../common", features = ["test-support"] } diff --git a/lsp/src/app_state.rs b/lsp/src/app_state.rs index 7311f01..3045d5e 100644 --- a/lsp/src/app_state.rs +++ b/lsp/src/app_state.rs @@ -7,14 +7,14 @@ use crate::watching::PathStore; #[derive(Debug)] pub struct AppState { - pub watcher_store: Arc, + pub watched_paths: PathStore, } impl AppState { pub fn new(gist_id: String, github_token: String) -> Result { let sync_client = Arc::new(GithubClient::new(gist_id, github_token)?); - let watcher_store = Arc::new(PathStore::new(sync_client)?); + let watched_paths = PathStore::new(sync_client)?; - Ok(Self { watcher_store }) + Ok(Self { watched_paths }) } } diff --git a/lsp/src/backend.rs b/lsp/src/backend.rs index c4b6c54..595f869 100644 --- a/lsp/src/backend.rs +++ b/lsp/src/backend.rs @@ -1,5 +1,5 @@ use std::path::PathBuf; -use std::sync::{Arc, OnceLock}; +use std::sync::{Mutex, OnceLock}; use anyhow::Result; use common::config::Config; @@ -23,45 +23,47 @@ const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); #[derive(Debug)] pub struct Backend { - // Arc is needed for cross-thread ownership (Tokio), OnceLock - for delayed initialization; - // both of them - because of how the LanguageServer trait is defined by tower-lsp: - // its methods don't mutate self. - app_state: Arc>, + // OnceLock is needed for cross-thread sync (Tokio) and for delayed initialization. + // Mutex is needed for interior mutability over a shared reference + // because LanguageServer trait methods accept &self (not &mut self). + app_state: OnceLock>, } impl Backend { pub fn new(_client: Client) -> Self { Self { - app_state: Arc::new(OnceLock::new()), + app_state: OnceLock::new(), } } - async fn watch_path(&self, path: PathBuf) -> Result<()> { + fn watch_path(&self, path: PathBuf) -> Result<()> { let info_msg = format!("Watching path: {}", path.display()); #[allow(clippy::expect_used)] self.app_state .get() - .expect("App state must be already initialized") - .watcher_store - .watch(path) - .await?; + .expect("App state must already be initialized") + .lock() + .expect("Watched paths store mutex is poisoned") + .watched_paths + .watch(path)?; info!("{}", info_msg); Ok(()) } - async fn unwatch_path(&self, path: PathBuf) -> Result<()> { + fn unwatch_path(&self, path: &PathBuf) -> Result<()> { let info_msg = format!("Unwatching path: {}", path.display()); #[allow(clippy::expect_used)] self.app_state .get() .expect("App state must be already initialized") - .watcher_store - .unwatch(path) - .await?; + .lock() + .expect("Watched paths store mutex is poisoned") + .watched_paths + .unwatch(path)?; info!("{}", info_msg); @@ -91,16 +93,17 @@ impl LanguageServer for Backend { #[allow(clippy::expect_used)] self.app_state - .set(app_state) + .set(Mutex::new(app_state)) .expect("AppState should not yet be initialized"); #[allow(clippy::expect_used)] self.app_state .get() .expect("App state should have been already initialized") - .watcher_store - .start_watcher() - .await; + .lock() + .expect("Watched paths store mutex is poisoned") + .watched_paths + .start_watcher(); Ok(InitializeResult { server_info: Some(ServerInfo { @@ -141,8 +144,9 @@ impl LanguageServer for Backend { match ZedConfigFilePath::from_file_uri(¶ms.text_document.uri) { Ok(path) => { let path_to_watch = path.to_watched_path_buf(); - // TODO: expose sync_client in app state and sync file explicitly after opening (quick'n'dirty way to fight losing last changes on LSP restart on settings update) - if let Err(err) = self.watch_path(path_to_watch).await { + // TODO: expose sync_client in app state and sync file explicitly after opening + // (quick'n'dirty way to fight losing last changes on LSP restart on settings update) + if let Err(err) = self.watch_path(path_to_watch) { error!("Failed to start watching path: {}", err); } } @@ -165,7 +169,7 @@ impl LanguageServer for Backend { match ZedConfigFilePath::from_file_uri(¶ms.text_document.uri) { Ok(path) => { let path_to_watch = path.to_watched_path_buf(); - if let Err(err) = self.unwatch_path(path_to_watch).await { + if let Err(err) = self.unwatch_path(&path_to_watch) { error!("Failed to stop watching path: {}", err); } } diff --git a/lsp/src/watching.rs b/lsp/src/watching.rs index b763e9e..3291f61 100644 --- a/lsp/src/watching.rs +++ b/lsp/src/watching.rs @@ -1,7 +1,9 @@ mod path_store; mod path_watcher; +mod watched_set; mod zed_config; pub use path_store::*; pub use path_watcher::*; +pub use watched_set::*; pub use zed_config::*; diff --git a/lsp/src/watching/path_store.rs b/lsp/src/watching/path_store.rs index 809b9c3..b9a1c58 100644 --- a/lsp/src/watching/path_store.rs +++ b/lsp/src/watching/path_store.rs @@ -1,35 +1,18 @@ -use std::{collections::HashSet, fs, path::PathBuf, pin::Pin, sync::Arc}; +use std::{fs, path::PathBuf, pin::Pin, sync::Arc}; use anyhow::Result; -use anyhow::{Context, anyhow, bail}; +use anyhow::{Context, anyhow}; use common::sync::{Client, LocalFileData}; +use mockall_double::double; use notify::{Event, EventKind, event::ModifyKind}; -use tokio::sync::Mutex; use tracing::{debug, error}; -use crate::watching::{EventHandler, PathWatcher}; - -#[derive(Debug)] -struct WatchedSet { - paths: HashSet, - watcher: PathWatcher, -} - -impl WatchedSet { - fn new(event_handler: EventHandler) -> Result { - let watcher = PathWatcher::new(event_handler)?; - - Ok(Self { - paths: HashSet::new(), - watcher, - }) - } -} +#[double] +use crate::watching::WatchedSet; #[derive(Debug)] pub struct PathStore { - // behind mutex to control the simultaneous change of paths set and path watcher - watched_set: Mutex, + watched_set: WatchedSet, } impl PathStore { @@ -53,49 +36,22 @@ impl PathStore { }); Ok(Self { - watched_set: Mutex::new(WatchedSet::new(event_handler)?), + watched_set: WatchedSet::new(event_handler)?, }) } - pub async fn start_watcher(&self) { - let mut watched_set = self.watched_set.lock().await; - - watched_set.watcher.start(); + pub fn start_watcher(&mut self) { + self.watched_set.start_watcher(); } // no need to stop watcher or clear the store because it will be stopped (when dropped) on the store drop - pub async fn watch(&self, file_path: PathBuf) -> anyhow::Result<()> { - { - let mut watched_set = self.watched_set.lock().await; - - if watched_set.paths.contains(&file_path) { - bail!("Path is already being watched: {}", file_path.display()); - } - - watched_set.watcher.watch(&file_path)?; - watched_set.paths.insert(file_path); - } - - Ok(()) + pub fn watch(&mut self, file_path: PathBuf) -> anyhow::Result<()> { + self.watched_set.watch(file_path) } - pub async fn unwatch(&self, file_path: PathBuf) -> anyhow::Result<()> { - { - let mut watched_set = self.watched_set.lock().await; - - if !watched_set.paths.contains(&file_path) { - bail!( - "Path is not being watched, failed to unwatch: {}", - file_path.display() - ); - } - - watched_set.watcher.unwatch(&file_path)?; - watched_set.paths.remove(&file_path); - } - - Ok(()) + pub fn unwatch(&mut self, file_path: &PathBuf) -> anyhow::Result<()> { + self.watched_set.unwatch(file_path) } } diff --git a/lsp/src/watching/watched_set.rs b/lsp/src/watching/watched_set.rs new file mode 100644 index 0000000..07346e4 --- /dev/null +++ b/lsp/src/watching/watched_set.rs @@ -0,0 +1,66 @@ +use std::{collections::HashSet, path::PathBuf, sync::Mutex}; + +use anyhow::{Result, bail}; + +use crate::watching::{EventHandler, PathWatcher}; + +#[derive(Debug)] +pub struct WatchedSet { + paths: HashSet, + watcher: PathWatcher, + mx: Mutex<()>, +} + +#[cfg_attr(test, mockall::automock)] +impl WatchedSet { + pub fn new(event_handler: EventHandler) -> Result { + let watcher = PathWatcher::new(event_handler)?; + + Ok(Self { + paths: HashSet::new(), + watcher, + mx: Mutex::new(()), + }) + } + + pub fn start_watcher(&mut self) { + #[allow(clippy::expect_used)] + let _lock = self.mx.lock().expect("Watched set mutex is poisoned"); + + self.watcher.start(); + } + + pub fn watch(&mut self, path: PathBuf) -> Result<()> { + #[allow(clippy::expect_used)] + let _lock = self.mx.lock().expect("Watched set mutex is poisoned"); + + if self.paths.contains(&path) { + bail!("Path is already being watched: {}", path.display()); + } + + self.watcher.watch(&path)?; + self.paths.insert(path); + + Ok(()) + } + + pub fn unwatch(&mut self, path: &PathBuf) -> Result<()> { + #[allow(clippy::expect_used)] + let _lock = self.mx.lock().expect("Watched set mutex is poisoned"); + + if !self.paths.contains(path) { + bail!( + "Path is not being watched, failed to unwatch: {}", + path.display() + ); + } + + self.watcher.unwatch(path)?; + self.paths.remove(path); + + Ok(()) + } +} + +#[cfg(test)] +mod tests {} From 0bdc79bf5a00b808c7e0f735a71d3ad4040f1f40 Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Wed, 14 Jan 2026 22:17:06 +0200 Subject: [PATCH 7/9] Add note on watched set type state to roadmap --- ROADMAP.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ROADMAP.md b/ROADMAP.md index 9798531..53dafa0 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -27,6 +27,7 @@ - [ ] To support multiple sync providers, implement a SyncClient trait and use it for all sync operations. Move it to the "common" crate. - [ ] Report a sync error in a visible way (crash the server? know how to report an LSP error inside Zed?) - [ ] Backup installed themes to the gist automatically +- [ ] Use type-state pattern to guarantee that one cannot watch/unwatch a path using non-started WatcherSet or PathStore ### CLI tool From 2f6fe1f3de8829cd18d25a97530034ae79f6ca94 Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Tue, 13 Jan 2026 12:43:35 +0200 Subject: [PATCH 8/9] Add unit tests for PathStore --- Cargo.lock | 11 +++ lsp/Cargo.toml | 5 ++ lsp/src/main.rs | 3 + lsp/src/watching/path_store.rs | 155 ++++++++++++++++++++++++++++----- 4 files changed, 154 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8b1ae23..e05cc30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2061,6 +2061,12 @@ dependencies = [ "windows-link", ] +[[package]] +name = "paste" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" + [[package]] name = "paths" version = "0.1.0" @@ -4157,14 +4163,19 @@ name = "zed-settings-sync-lsp" version = "0.0.1-alpha.5" dependencies = [ "anyhow", + "assert_fs", "common", + "ctor", "debug-ignore", "jsonc-parser", + "mockall", "mockall_double", "notify", "octocrab", + "paste", "paths", "serde", + "test_support", "tokio", "tower-lsp", "tracing", diff --git a/lsp/Cargo.toml b/lsp/Cargo.toml index c4cd12d..33e160c 100644 --- a/lsp/Cargo.toml +++ b/lsp/Cargo.toml @@ -25,3 +25,8 @@ mockall_double = { workspace = true } [dev-dependencies] common = { path = "../common", features = ["test-support"] } +test_support = { path = "../test_support" } +mockall = { workspace = true } +assert_fs = { workspace = true } +ctor = { workspace = true } +paste = "1.0.15" diff --git a/lsp/src/main.rs b/lsp/src/main.rs index 2b2f859..9bd5804 100644 --- a/lsp/src/main.rs +++ b/lsp/src/main.rs @@ -8,6 +8,9 @@ mod backend; mod logger; mod watching; +#[cfg(test)] +test_support::nextest_only!(); + #[tokio::main] async fn main() { logger::init_logger(); diff --git a/lsp/src/watching/path_store.rs b/lsp/src/watching/path_store.rs index b9a1c58..201ab0c 100644 --- a/lsp/src/watching/path_store.rs +++ b/lsp/src/watching/path_store.rs @@ -74,36 +74,151 @@ fn process_event(event: &Event) -> Result> { #[cfg(test)] mod tests { + #![allow(clippy::unwrap_used)] + + use assert_fs::{TempDir, prelude::*}; use common::sync::MockGithubClient; + use mockall::{Sequence, predicate}; + use paste::paste; use super::*; + use crate::watching::MockWatchedSet; #[tokio::test] async fn test_successful_creation() { + let ctx = MockWatchedSet::new_context(); + ctx.expect().returning(|_| Ok(MockWatchedSet::default())); + assert!(PathStore::new(Arc::new(MockGithubClient::default())).is_ok()); } + #[tokio::test] + async fn test_unsuccessful_creation_when_watched_set_creation_failed() { + let ctx = MockWatchedSet::new_context(); + ctx.expect() + .returning(|_| Err(anyhow!("Failed to create watched set"))); + + assert!(PathStore::new(Arc::new(MockGithubClient::default())).is_err()); + } + + macro_rules! setup_watched_set_mock { + ($method:ident, $path:expr) => { + paste! { + let ctx = MockWatchedSet::new_context(); + ctx.expect().returning(move |_| { + let mut seq = Sequence::new(); + let mut mock_watched_set = MockWatchedSet::default(); + mock_watched_set + .expect_start_watcher() + .in_sequence(&mut seq) + .returning(|| ()); + mock_watched_set + .[]() + .with(predicate::eq($path)) + .in_sequence(&mut seq) + .returning(|_| Ok(())); + + Ok(mock_watched_set) + }); + } + }; + ($method:ident, $path:expr, $err_msg:expr) => { + paste! { + let ctx = MockWatchedSet::new_context(); + ctx.expect().returning(move |_| { + let mut seq = Sequence::new(); + let mut mock_watched_set = MockWatchedSet::default(); + mock_watched_set + .expect_start_watcher() + .in_sequence(&mut seq) + .returning(|| ()); + mock_watched_set + .[]() + .with(predicate::eq($path)) + .in_sequence(&mut seq) + .returning(|_| Err(anyhow!($err_msg))); + + Ok(mock_watched_set) + }); + } + }; + } + + #[tokio::test] + async fn test_successful_watch_path() -> Result<()> { + let dir = TempDir::new()?; + dir.child("foobar").touch()?; + let path = dir.path().to_path_buf(); + let path_clone = path.clone(); + + setup_watched_set_mock!(watch, path.clone()); + + let mut store = PathStore::new(Arc::new(MockGithubClient::default()))?; + store.start_watcher(); + store.watch(path_clone)?; + + Ok(()) + } + + #[tokio::test] + async fn test_unsuccessful_watch_path() -> Result<()> { + let dir = TempDir::new()?; + dir.child("foobar").touch()?; + let path = dir.path().to_path_buf(); + let path_clone = path.clone(); + + setup_watched_set_mock!(watch, path.clone(), "Path already being watched"); + + let mut store = PathStore::new(Arc::new(MockGithubClient::default()))?; + store.start_watcher(); + + assert_eq!( + store.watch(path_clone).unwrap_err().to_string(), + "Path already being watched" + ); + + Ok(()) + } + + #[tokio::test] + async fn test_successful_unwatch_path() -> Result<()> { + let dir = TempDir::new()?; + dir.child("foobar").touch()?; + let path = dir.path().to_path_buf(); + let path_clone = path.clone(); + + setup_watched_set_mock!(unwatch, path.clone()); + + let mut store = PathStore::new(Arc::new(MockGithubClient::default()))?; + store.start_watcher(); + store.unwatch(&path_clone)?; + + Ok(()) + } + + #[tokio::test] + async fn test_unsuccessful_unwatch_path() -> Result<()> { + let dir = TempDir::new()?; + dir.child("foobar").touch()?; + let path = dir.path().to_path_buf(); + let path_clone = path.clone(); + + setup_watched_set_mock!(unwatch, path.clone(), "Path was not watched"); + + let mut store = PathStore::new(Arc::new(MockGithubClient::default()))?; + store.start_watcher(); + + assert_eq!( + store.unwatch(&path_clone).unwrap_err().to_string(), + "Path was not watched" + ); + + Ok(()) + } + /* - - new - - test successful creation - - create a store with the MockGithubClient passed - - test unsuccessful creation is watched set creation failed - - - start watcher - - test successful start watcher - - watcher started (mock for PathWatcher) - - - watch - - test successful watch new path - - new path passed to path watcher for watch (mock for PathWatcher) - - new path added to watched set (mock for WatchedSet) - - test failure watch path already watched - - - unwatch - - test successful unwatch - - test failure unwatch path not watched - - path passed to path watcher for unwatch (mock for PathWatcher) - - path removed from watched set (mock for WatchedSet) + + Integration tests, here or just for WatchedSet (TODO) - events handling - test create file does not trigger event handler From 12f36c83d3895dee64e10c7434d5b23c71baee0b Mon Sep 17 00:00:00 2001 From: Viktor Zahorodnii Date: Sat, 17 Jan 2026 17:12:52 +0200 Subject: [PATCH 9/9] Made process_event an async function (read file asynchronously) --- lsp/Cargo.toml | 2 +- lsp/src/watching/path_store.rs | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lsp/Cargo.toml b/lsp/Cargo.toml index 33e160c..9e0d479 100644 --- a/lsp/Cargo.toml +++ b/lsp/Cargo.toml @@ -13,7 +13,7 @@ jsonc-parser = { workspace = true } octocrab = { workspace = true } zed_extension_api = { workspace = true } serde = { workspace = true } -tokio = { workspace = true, features = ["io-std"] } +tokio = { workspace = true, features = ["io-std", "fs"] } tracing = { workspace = true } paths = { workspace = true } common = { path = "../common" } diff --git a/lsp/src/watching/path_store.rs b/lsp/src/watching/path_store.rs index 201ab0c..ce8ff42 100644 --- a/lsp/src/watching/path_store.rs +++ b/lsp/src/watching/path_store.rs @@ -1,10 +1,11 @@ -use std::{fs, path::PathBuf, pin::Pin, sync::Arc}; +use std::{path::PathBuf, pin::Pin, sync::Arc}; use anyhow::Result; use anyhow::{Context, anyhow}; use common::sync::{Client, LocalFileData}; use mockall_double::double; use notify::{Event, EventKind, event::ModifyKind}; +use tokio::fs; use tracing::{debug, error}; #[double] @@ -20,7 +21,7 @@ impl PathStore { let event_handler = Box::new(move |event| { let client_clone = Arc::clone(&client); Box::pin(async move { - match process_event(&event) { + match process_event(&event).await { Ok(data) => { let Some(data) = data else { return; @@ -55,7 +56,7 @@ impl PathStore { } } -fn process_event(event: &Event) -> Result> { +async fn process_event(event: &Event) -> Result> { debug!("Processing file watcher event: {event:?}"); let EventKind::Modify(ModifyKind::Data(_)) = event.kind else { @@ -67,7 +68,9 @@ fn process_event(event: &Event) -> Result> { "event did not provide the path of the modified file" ))?; - let body = fs::read_to_string(&path).with_context(|| "Could not read the modified file")?; + let body = fs::read_to_string(&path) + .await + .with_context(|| format!("Could not read the modified file: {}", path.display()))?; Ok(Some(LocalFileData::new(path, body)?)) }