worktree: Use public-api-only tests (#47152)

- **settings: Do not leak type serialization into downstream crates**
- **worktree: Separate integration tests into separate target**

Release Notes:

- N/A

Cargo.lock

@@ -20437,6 +20437,7 @@ dependencies = [
  "text",
  "tracing",
  "util",
+ "worktree",
  "zlog",
  "ztracing",
 ]

crates/settings/src/settings_store.rs

@@ -437,12 +437,15 @@ impl SettingsStore {
     ) {
         let mut content = self.user_settings.clone().unwrap_or_default().content;
         update(&mut content);
-        let new_text = serde_json::to_string(&UserSettingsContent {
-            content,
-            ..Default::default()
-        })
-        .unwrap();
-        _ = self.set_user_settings(&new_text, cx);
+        fn trail(this: &mut SettingsStore, content: Box<SettingsContent>, cx: &mut App) {
+            let new_text = serde_json::to_string(&UserSettingsContent {
+                content,
+                ..Default::default()
+            })
+            .unwrap();
+            _ = this.set_user_settings(&new_text, cx);
+        }
+        trail(self, content, cx);
     }
 
     pub async fn load_settings(fs: &Arc<dyn Fs>) -> Result<String> {

crates/worktree/Cargo.toml

@@ -8,6 +8,12 @@ license = "GPL-3.0-or-later"
 [lib]
 path = "src/worktree.rs"
 doctest = false
+test = false
+
+[[test]]
+name = "integration"
+required-features = ["test-support"]
+path = "tests/integration/main.rs"
 
 [lints]
 workspace = true
@@ -17,6 +23,7 @@ test-support = [
     "gpui/test-support",
     "http_client/test-support",
     "language/test-support",
+    "pretty_assertions",
     "settings/test-support",
     "text/test-support",
     "util/test-support",
@@ -40,6 +47,7 @@ log.workspace = true
 parking_lot.workspace = true
 paths.workspace = true
 postage.workspace = true
+pretty_assertions = { workspace = true, optional = true }
 rpc = { workspace = true, features = ["gpui"] }
 serde.workspace = true
 serde_json.workspace = true
@@ -59,11 +67,11 @@ git2.workspace = true
 gpui = { workspace = true, features = ["test-support"] }
 http_client.workspace = true
 paths = { workspace = true, features = ["test-support"] }
-pretty_assertions.workspace = true
 rand.workspace = true
 rpc = { workspace = true, features = ["test-support"] }
 settings = { workspace = true, features = ["test-support"] }
 util = { workspace = true, features = ["test-support"] }
+worktree = { workspace = true, features = ["test-support"] }
 zlog.workspace = true
 
 [package.metadata.cargo-machete]

crates/worktree/src/worktree.rs

@@ -1,7 +1,5 @@
 mod ignore;
 mod worktree_settings;
-#[cfg(test)]
-mod worktree_tests;
 
 use ::ignore::gitignore::{Gitignore, GitignoreBuilder};
 use anyhow::{Context as _, Result, anyhow};
@@ -714,7 +712,7 @@ impl Worktree {
         }
     }
 
-    #[cfg(any(test, feature = "test-support"))]
+    #[cfg(feature = "test-support")]
     pub fn has_update_observer(&self) -> bool {
         match self {
             Worktree::Local(this) => this.update_observer.is_some(),
@@ -1416,7 +1414,7 @@ impl LocalWorktree {
         lowest_ancestor.unwrap_or_else(|| RelPath::empty().into())
     }
 
-    fn create_entry(
+    pub fn create_entry(
         &self,
         path: Arc<RelPath>,
         is_dir: bool,
@@ -1477,7 +1475,7 @@ impl LocalWorktree {
         })
     }
 
-    fn write_file(
+    pub fn write_file(
         &self,
         path: Arc<RelPath>,
         text: Rope,
@@ -1593,7 +1591,7 @@ impl LocalWorktree {
         })
     }
 
-    fn delete_entry(
+    pub fn delete_entry(
         &self,
         entry_id: ProjectEntryId,
         trash: bool,
@@ -1816,7 +1814,7 @@ impl LocalWorktree {
         })
     }
 
-    fn observe_updates<F, Fut>(&mut self, project_id: u64, cx: &Context<Worktree>, callback: F)
+    pub fn observe_updates<F, Fut>(&mut self, project_id: u64, cx: &Context<Worktree>, callback: F)
     where
         F: 'static + Send + Fn(proto::UpdateWorktree) -> Fut,
         Fut: 'static + Send + Future<Output = bool>,
@@ -1891,6 +1889,13 @@ impl LocalWorktree {
         self.snapshot.update_abs_path(new_path, root_name);
         self.restart_background_scanners(cx);
     }
+    #[cfg(feature = "test-support")]
+    pub fn repositories(&self) -> Vec<Arc<Path>> {
+        self.git_repositories
+            .values()
+            .map(|entry| entry.work_directory_abs_path.clone())
+            .collect::<Vec<_>>()
+    }
 }
 
 impl RemoteWorktree {
@@ -2279,7 +2284,7 @@ impl Snapshot {
         }
     }
 
-    fn apply_remote_update(
+    pub fn apply_remote_update(
         &mut self,
         update: proto::UpdateWorktree,
         always_included_paths: &PathMatcher,
@@ -2680,14 +2685,14 @@ impl LocalSnapshot {
         ignore_stack
     }
 
-    #[cfg(test)]
-    fn expanded_entries(&self) -> impl Iterator<Item = &Entry> {
+    #[cfg(feature = "test-support")]
+    pub fn expanded_entries(&self) -> impl Iterator<Item = &Entry> {
         self.entries_by_path
             .cursor::<()>(())
             .filter(|entry| entry.kind == EntryKind::Dir && (entry.is_external || entry.is_ignored))
     }
 
-    #[cfg(test)]
+    #[cfg(feature = "test-support")]
     pub fn check_invariants(&self, git_state: bool) {
         use pretty_assertions::assert_eq;
 
@@ -2767,7 +2772,7 @@ impl LocalSnapshot {
         }
     }
 
-    #[cfg(test)]
+    #[cfg(feature = "test-support")]
     pub fn entries_without_ids(&self, include_ignored: bool) -> Vec<(&RelPath, u64, bool)> {
         let mut paths = Vec::new();
         for entry in self.entries_by_path.cursor::<()>(()) {
@@ -2875,7 +2880,7 @@ impl BackgroundScannerState {
                 .await;
         }
 
-        #[cfg(test)]
+        #[cfg(feature = "test-support")]
         self.snapshot.check_invariants(false);
 
         entry
@@ -2943,7 +2948,7 @@ impl BackgroundScannerState {
             self.changed_paths.insert(ix, parent_path.clone());
         }
 
-        #[cfg(test)]
+        #[cfg(feature = "test-support")]
         self.snapshot.check_invariants(false);
     }
 
@@ -2999,7 +3004,7 @@ impl BackgroundScannerState {
             .git_repositories
             .retain(|id, _| removed_ids.binary_search(id).is_err());
 
-        #[cfg(test)]
+        #[cfg(feature = "test-support")]
         self.snapshot.check_invariants(false);
     }
 
@@ -5130,7 +5135,7 @@ impl BackgroundScanner {
             return futures::future::pending().await;
         }
 
-        #[cfg(any(test, feature = "test-support"))]
+        #[cfg(feature = "test-support")]
         if self.fs.is_fake() {
             return self.executor.simulate_random_delay().await;
         }
@@ -5361,13 +5366,13 @@ struct UpdateIgnoreStatusJob {
 }
 
 pub trait WorktreeModelHandle {
-    #[cfg(any(test, feature = "test-support"))]
+    #[cfg(feature = "test-support")]
     fn flush_fs_events<'a>(
         &self,
         cx: &'a mut gpui::TestAppContext,
     ) -> futures::future::LocalBoxFuture<'a, ()>;
 
-    #[cfg(any(test, feature = "test-support"))]
+    #[cfg(feature = "test-support")]
     fn flush_fs_events_in_root_git_repository<'a>(
         &self,
         cx: &'a mut gpui::TestAppContext,
@@ -5381,7 +5386,7 @@ impl WorktreeModelHandle for Entity<Worktree> {
     //
     // This function mutates the worktree's directory and waits for those mutations to be picked up,
     // to ensure that all redundant FS events have already been processed.
-    #[cfg(any(test, feature = "test-support"))]
+    #[cfg(feature = "test-support")]
     fn flush_fs_events<'a>(
         &self,
         cx: &'a mut gpui::TestAppContext,
@@ -5451,7 +5456,7 @@ impl WorktreeModelHandle for Entity<Worktree> {
     // worktree and thus its FS events might go through a different path.
     // In order to flush those, we need to create artificial events in the .git folder and wait
     // for the repository to be reloaded.
-    #[cfg(any(test, feature = "test-support"))]
+    #[cfg(feature = "test-support")]
     fn flush_fs_events_in_root_git_repository<'a>(
         &self,
         cx: &'a mut gpui::TestAppContext,
@@ -5874,7 +5879,7 @@ impl ProjectEntryId {
     }
 }
 
-#[cfg(any(test, feature = "test-support"))]
+#[cfg(feature = "test-support")]
 impl CreatedEntry {
     pub fn into_included(self) -> Option<Entry> {
         match self {

crates/worktree/src/worktree_settings.rs

@@ -106,126 +106,3 @@ fn path_matchers(mut values: Vec<String>, context: &'static str) -> anyhow::Resu
     PathMatcher::new(values, PathStyle::local())
         .with_context(|| format!("Failed to parse globs from {}", context))
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use std::path::Path;
-
-    fn make_settings_with_read_only(patterns: &[&str]) -> WorktreeSettings {
-        WorktreeSettings {
-            project_name: None,
-            prevent_sharing_in_public_channels: false,
-            file_scan_exclusions: PathMatcher::default(),
-            file_scan_inclusions: PathMatcher::default(),
-            parent_dir_scan_inclusions: PathMatcher::default(),
-            private_files: PathMatcher::default(),
-            hidden_files: PathMatcher::default(),
-            read_only_files: PathMatcher::new(
-                patterns.iter().map(|s| s.to_string()),
-                PathStyle::local(),
-            )
-            .unwrap(),
-        }
-    }
-
-    #[test]
-    fn test_is_path_read_only_with_glob_patterns() {
-        let settings = make_settings_with_read_only(&["**/generated/**", "**/*.gen.rs"]);
-
-        let generated_file =
-            RelPath::new(Path::new("src/generated/schema.rs"), PathStyle::local()).unwrap();
-        assert!(
-            settings.is_path_read_only(&generated_file),
-            "Files in generated directory should be read-only"
-        );
-
-        let gen_rs_file = RelPath::new(Path::new("src/types.gen.rs"), PathStyle::local()).unwrap();
-        assert!(
-            settings.is_path_read_only(&gen_rs_file),
-            "Files with .gen.rs extension should be read-only"
-        );
-
-        let regular_file = RelPath::new(Path::new("src/main.rs"), PathStyle::local()).unwrap();
-        assert!(
-            !settings.is_path_read_only(&regular_file),
-            "Regular files should not be read-only"
-        );
-
-        let similar_name = RelPath::new(Path::new("src/generator.rs"), PathStyle::local()).unwrap();
-        assert!(
-            !settings.is_path_read_only(&similar_name),
-            "Files with 'generator' in name but not in generated dir should not be read-only"
-        );
-    }
-
-    #[test]
-    fn test_is_path_read_only_with_specific_paths() {
-        let settings = make_settings_with_read_only(&["vendor/**", "node_modules/**"]);
-
-        let vendor_file =
-            RelPath::new(Path::new("vendor/lib/package.js"), PathStyle::local()).unwrap();
-        assert!(
-            settings.is_path_read_only(&vendor_file),
-            "Files in vendor directory should be read-only"
-        );
-
-        let node_modules_file = RelPath::new(
-            Path::new("node_modules/lodash/index.js"),
-            PathStyle::local(),
-        )
-        .unwrap();
-        assert!(
-            settings.is_path_read_only(&node_modules_file),
-            "Files in node_modules should be read-only"
-        );
-
-        let src_file = RelPath::new(Path::new("src/app.js"), PathStyle::local()).unwrap();
-        assert!(
-            !settings.is_path_read_only(&src_file),
-            "Files in src should not be read-only"
-        );
-    }
-
-    #[test]
-    fn test_is_path_read_only_empty_patterns() {
-        let settings = make_settings_with_read_only(&[]);
-
-        let any_file = RelPath::new(Path::new("src/main.rs"), PathStyle::local()).unwrap();
-        assert!(
-            !settings.is_path_read_only(&any_file),
-            "No files should be read-only when patterns are empty"
-        );
-    }
-
-    #[test]
-    fn test_is_path_read_only_with_extension_pattern() {
-        let settings = make_settings_with_read_only(&["**/*.lock", "**/*.min.js"]);
-
-        let lock_file = RelPath::new(Path::new("Cargo.lock"), PathStyle::local()).unwrap();
-        assert!(
-            settings.is_path_read_only(&lock_file),
-            "Lock files should be read-only"
-        );
-
-        let nested_lock =
-            RelPath::new(Path::new("packages/app/yarn.lock"), PathStyle::local()).unwrap();
-        assert!(
-            settings.is_path_read_only(&nested_lock),
-            "Nested lock files should be read-only"
-        );
-
-        let minified_js =
-            RelPath::new(Path::new("dist/bundle.min.js"), PathStyle::local()).unwrap();
-        assert!(
-            settings.is_path_read_only(&minified_js),
-            "Minified JS files should be read-only"
-        );
-
-        let regular_js = RelPath::new(Path::new("src/app.js"), PathStyle::local()).unwrap();
-        assert!(
-            !settings.is_path_read_only(&regular_js),
-            "Regular JS files should not be read-only"
-        );
-    }
-}

crates/worktree/tests/integration/main.rs

@@ -1,4 +1,5 @@
-use crate::{Entry, EntryKind, Event, PathChange, Worktree, WorktreeModelHandle};
+mod worktree_settings;
+
 use anyhow::Result;
 use encoding_rs;
 use fs::{FakeFs, Fs, RealFs, RemoveOptions};
@@ -8,6 +9,7 @@ use parking_lot::Mutex;
 use postage::stream::Stream;
 use pretty_assertions::assert_eq;
 use rand::prelude::*;
+use worktree::{Entry, EntryKind, Event, PathChange, Worktree, WorktreeModelHandle};
 
 use serde_json::json;
 use settings::SettingsStore;
@@ -2022,7 +2024,7 @@ fn randomly_mutate_worktree(
 
     match rng.random_range(0_u32..100) {
         0..=33 if entry.path.as_ref() != RelPath::empty() => {
-            log::info!("deleting entry {:?} ({})", entry.path, entry.id.0);
+            log::info!("deleting entry {:?} ({})", entry.path, entry.id.to_usize());
             worktree.delete_entry(entry.id, false, cx).unwrap()
         }
         _ => {
@@ -2040,7 +2042,11 @@ fn randomly_mutate_worktree(
                     Ok(())
                 })
             } else {
-                log::info!("overwriting file {:?} ({})", &entry.path, entry.id.0);
+                log::info!(
+                    "overwriting file {:?} ({})",
+                    &entry.path,
+                    entry.id.to_usize()
+                );
                 let task = worktree.write_file(
                     entry.path.clone(),
                     "".into(),
@@ -2282,13 +2288,7 @@ async fn test_repository_above_root(executor: BackgroundExecutor, cx: &mut TestA
         .await;
     cx.run_until_parked();
     let repos = worktree.update(cx, |worktree, _| {
-        worktree
-            .as_local()
-            .unwrap()
-            .git_repositories
-            .values()
-            .map(|entry| entry.work_directory_abs_path.clone())
-            .collect::<Vec<_>>()
+        worktree.as_local().unwrap().repositories()
     });
     pretty_assertions::assert_eq!(repos, [Path::new(path!("/root")).into()]);
 
@@ -2301,13 +2301,7 @@ async fn test_repository_above_root(executor: BackgroundExecutor, cx: &mut TestA
     cx.run_until_parked();
 
     let repos = worktree.update(cx, |worktree, _| {
-        worktree
-            .as_local()
-            .unwrap()
-            .git_repositories
-            .values()
-            .map(|entry| entry.work_directory_abs_path.clone())
-            .collect::<Vec<_>>()
+        worktree.as_local().unwrap().repositories()
     });
     pretty_assertions::assert_eq!(repos, [Path::new(path!("/root")).into()]);
 }

crates/worktree/tests/integration/worktree_settings.rs

@@ -0,0 +1,121 @@
+use std::path::Path;
+use util::{
+    paths::{PathMatcher, PathStyle},
+    rel_path::RelPath,
+};
+use worktree::*;
+
+fn make_settings_with_read_only(patterns: &[&str]) -> WorktreeSettings {
+    WorktreeSettings {
+        project_name: None,
+        prevent_sharing_in_public_channels: false,
+        file_scan_exclusions: PathMatcher::default(),
+        file_scan_inclusions: PathMatcher::default(),
+        parent_dir_scan_inclusions: PathMatcher::default(),
+        private_files: PathMatcher::default(),
+        hidden_files: PathMatcher::default(),
+        read_only_files: PathMatcher::new(
+            patterns.iter().map(|s| s.to_string()),
+            PathStyle::local(),
+        )
+        .unwrap(),
+    }
+}
+
+#[test]
+fn test_is_path_read_only_with_glob_patterns() {
+    let settings = make_settings_with_read_only(&["**/generated/**", "**/*.gen.rs"]);
+
+    let generated_file =
+        RelPath::new(Path::new("src/generated/schema.rs"), PathStyle::local()).unwrap();
+    assert!(
+        settings.is_path_read_only(&generated_file),
+        "Files in generated directory should be read-only"
+    );
+
+    let gen_rs_file = RelPath::new(Path::new("src/types.gen.rs"), PathStyle::local()).unwrap();
+    assert!(
+        settings.is_path_read_only(&gen_rs_file),
+        "Files with .gen.rs extension should be read-only"
+    );
+
+    let regular_file = RelPath::new(Path::new("src/main.rs"), PathStyle::local()).unwrap();
+    assert!(
+        !settings.is_path_read_only(&regular_file),
+        "Regular files should not be read-only"
+    );
+
+    let similar_name = RelPath::new(Path::new("src/generator.rs"), PathStyle::local()).unwrap();
+    assert!(
+        !settings.is_path_read_only(&similar_name),
+        "Files with 'generator' in name but not in generated dir should not be read-only"
+    );
+}
+
+#[test]
+fn test_is_path_read_only_with_specific_paths() {
+    let settings = make_settings_with_read_only(&["vendor/**", "node_modules/**"]);
+
+    let vendor_file = RelPath::new(Path::new("vendor/lib/package.js"), PathStyle::local()).unwrap();
+    assert!(
+        settings.is_path_read_only(&vendor_file),
+        "Files in vendor directory should be read-only"
+    );
+
+    let node_modules_file = RelPath::new(
+        Path::new("node_modules/lodash/index.js"),
+        PathStyle::local(),
+    )
+    .unwrap();
+    assert!(
+        settings.is_path_read_only(&node_modules_file),
+        "Files in node_modules should be read-only"
+    );
+
+    let src_file = RelPath::new(Path::new("src/app.js"), PathStyle::local()).unwrap();
+    assert!(
+        !settings.is_path_read_only(&src_file),
+        "Files in src should not be read-only"
+    );
+}
+
+#[test]
+fn test_is_path_read_only_empty_patterns() {
+    let settings = make_settings_with_read_only(&[]);
+
+    let any_file = RelPath::new(Path::new("src/main.rs"), PathStyle::local()).unwrap();
+    assert!(
+        !settings.is_path_read_only(&any_file),
+        "No files should be read-only when patterns are empty"
+    );
+}
+
+#[test]
+fn test_is_path_read_only_with_extension_pattern() {
+    let settings = make_settings_with_read_only(&["**/*.lock", "**/*.min.js"]);
+
+    let lock_file = RelPath::new(Path::new("Cargo.lock"), PathStyle::local()).unwrap();
+    assert!(
+        settings.is_path_read_only(&lock_file),
+        "Lock files should be read-only"
+    );
+
+    let nested_lock =
+        RelPath::new(Path::new("packages/app/yarn.lock"), PathStyle::local()).unwrap();
+    assert!(
+        settings.is_path_read_only(&nested_lock),
+        "Nested lock files should be read-only"
+    );
+
+    let minified_js = RelPath::new(Path::new("dist/bundle.min.js"), PathStyle::local()).unwrap();
+    assert!(
+        settings.is_path_read_only(&minified_js),
+        "Minified JS files should be read-only"
+    );
+
+    let regular_js = RelPath::new(Path::new("src/app.js"), PathStyle::local()).unwrap();
+    assert!(
+        !settings.is_path_read_only(&regular_js),
+        "Regular JS files should not be read-only"
+    );
+}