Tests for the watched paths store

.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"],
   },
   {

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 }

ROADMAP.md

@@ -9,17 +9,25 @@
 - [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
 - [ ] 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
 
@@ -32,9 +40,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 <https://github.com/johnthagen/min-sized-rust>
 
 ## Documentation

lsp/Cargo.toml

@@ -13,11 +13,20 @@ 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" }
 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"] }
+test_support = { path = "../test_support" }
+mockall = { workspace = true }
+assert_fs = { workspace = true }
+ctor = { workspace = true }
+paste = "1.0.15"

lsp/src/app_state.rs

@@ -3,18 +3,18 @@ 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<Store>,
+    pub watched_paths: PathStore,
 }
 
 impl AppState {
     pub fn new(gist_id: String, github_token: String) -> Result<Self> {
         let sync_client = Arc::new(GithubClient::new(gist_id, github_token)?);
-        let watcher_store = Arc::new(Store::new(sync_client)?);
+        let watched_paths = PathStore::new(sync_client)?;
 
-        Ok(Self { watcher_store })
+        Ok(Self { watched_paths })
     }
 }

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<AppState>>,
+    // 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<Mutex<AppState>>,
 }
 
 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(&params.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(&params.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);
                 }
             }

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();

lsp/src/watching.rs

@@ -1,7 +1,9 @@
+mod path_store;
 mod path_watcher;
-mod store;
+mod watched_set;
 mod zed_config;
 
+pub use path_store::*;
 pub use path_watcher::*;
-pub use store::*;
+pub use watched_set::*;
 pub use zed_config::*;