Fix several randomized test failures with the new git status implementation

Mikayla Maki created

Change summary

crates/collab/src/tests/randomized_integration_tests.rs | 30 +++++++---
crates/fs/src/fs.rs                                     | 18 +++++-
crates/project/src/worktree.rs                          | 16 ++++-
crates/sum_tree/src/sum_tree.rs                         |  5 +
4 files changed, 52 insertions(+), 17 deletions(-)

Detailed changes

crates/collab/src/tests/randomized_integration_tests.rs 🔗

@@ -422,7 +422,7 @@ async fn apply_client_operation(
             );
 
             ensure_project_shared(&project, client, cx).await;
-            if !client.fs.paths().contains(&new_root_path) {
+            if !client.fs.paths(false).contains(&new_root_path) {
                 client.fs.create_dir(&new_root_path).await.unwrap();
             }
             project
@@ -743,7 +743,7 @@ async fn apply_client_operation(
         } => {
             if !client
                 .fs
-                .directories()
+                .directories(false)
                 .contains(&path.parent().unwrap().to_owned())
             {
                 return Err(TestError::Inapplicable);
@@ -770,10 +770,16 @@ async fn apply_client_operation(
                 repo_path,
                 contents,
             } => {
-                if !client.fs.directories().contains(&repo_path) {
+                if !client.fs.directories(false).contains(&repo_path) {
                     return Err(TestError::Inapplicable);
                 }
 
+                for (path, _) in contents.iter() {
+                    if !client.fs.files().contains(&repo_path.join(path)) {
+                        return Err(TestError::Inapplicable);
+                    }
+                }
+
                 log::info!(
                     "{}: writing git index for repo {:?}: {:?}",
                     client.username,
@@ -795,7 +801,7 @@ async fn apply_client_operation(
                 repo_path,
                 new_branch,
             } => {
-                if !client.fs.directories().contains(&repo_path) {
+                if !client.fs.directories(false).contains(&repo_path) {
                     return Err(TestError::Inapplicable);
                 }
 
@@ -817,9 +823,14 @@ async fn apply_client_operation(
                 statuses,
                 git_operation,
             } => {
-                if !client.fs.directories().contains(&repo_path) {
+                if !client.fs.directories(false).contains(&repo_path) {
                     return Err(TestError::Inapplicable);
                 }
+                for (path, _) in statuses.iter() {
+                    if !client.fs.files().contains(&repo_path.join(path)) {
+                        return Err(TestError::Inapplicable);
+                    }
+                }
 
                 log::info!(
                     "{}: writing git statuses for repo {:?}: {:?}",
@@ -920,9 +931,10 @@ fn check_consistency_between_clients(clients: &[(Rc<TestClient>, TestAppContext)
                             assert_eq!(
                                 guest_snapshot.entries(false).collect::<Vec<_>>(),
                                 host_snapshot.entries(false).collect::<Vec<_>>(),
-                                "{} has different snapshot than the host for worktree {:?} and project {:?}",
+                                "{} has different snapshot than the host for worktree {:?} ({:?}) and project {:?}",
                                 client.username,
                                 host_snapshot.abs_path(),
+                                id,
                                 guest_project.remote_id(),
                             );
                             assert_eq!(guest_snapshot.repositories().collect::<Vec<_>>(), host_snapshot.repositories().collect::<Vec<_>>(),
@@ -1583,7 +1595,7 @@ impl TestPlan {
                                 .choose(&mut self.rng)
                                 .cloned() else { continue };
                             let project_root_name = root_name_for_project(&project, cx);
-                            let mut paths = client.fs.paths();
+                            let mut paths = client.fs.paths(false);
                             paths.remove(0);
                             let new_root_path = if paths.is_empty() || self.rng.gen() {
                                 Path::new("/").join(&self.next_root_dir_name(user_id))
@@ -1763,7 +1775,7 @@ impl TestPlan {
                     let is_dir = self.rng.gen::<bool>();
                     let content;
                     let mut path;
-                    let dir_paths = client.fs.directories();
+                    let dir_paths = client.fs.directories(false);
 
                     if is_dir {
                         content = String::new();
@@ -1817,7 +1829,7 @@ impl TestPlan {
 
         let repo_path = client
             .fs
-            .directories()
+            .directories(false)
             .choose(&mut self.rng)
             .unwrap()
             .clone();

crates/fs/src/fs.rs 🔗

@@ -12,6 +12,7 @@ use rope::Rope;
 use smol::io::{AsyncReadExt, AsyncWriteExt};
 use std::borrow::Cow;
 use std::cmp;
+use std::ffi::OsStr;
 use std::io::Write;
 use std::sync::Arc;
 use std::{
@@ -501,6 +502,11 @@ impl FakeFsState {
     }
 }
 
+#[cfg(any(test, feature = "test-support"))]
+lazy_static! {
+    pub static ref FS_DOT_GIT: &'static OsStr = OsStr::new(".git");
+}
+
 #[cfg(any(test, feature = "test-support"))]
 impl FakeFs {
     pub fn new(executor: Arc<gpui::executor::Background>) -> Arc<Self> {
@@ -693,7 +699,7 @@ impl FakeFs {
         });
     }
 
-    pub fn paths(&self) -> Vec<PathBuf> {
+    pub fn paths(&self, include_dot_git: bool) -> Vec<PathBuf> {
         let mut result = Vec::new();
         let mut queue = collections::VecDeque::new();
         queue.push_back((PathBuf::from("/"), self.state.lock().root.clone()));
@@ -703,12 +709,14 @@ impl FakeFs {
                     queue.push_back((path.join(name), entry.clone()));
                 }
             }
-            result.push(path);
+            if include_dot_git || !path.components().any(|component| component.as_os_str() == *FS_DOT_GIT) {
+                result.push(path);
+            }
         }
         result
     }
 
-    pub fn directories(&self) -> Vec<PathBuf> {
+    pub fn directories(&self, include_dot_git: bool) -> Vec<PathBuf> {
         let mut result = Vec::new();
         let mut queue = collections::VecDeque::new();
         queue.push_back((PathBuf::from("/"), self.state.lock().root.clone()));
@@ -717,7 +725,9 @@ impl FakeFs {
                 for (name, entry) in entries {
                     queue.push_back((path.join(name), entry.clone()));
                 }
-                result.push(path);
+                if include_dot_git || !path.components().any(|component| component.as_os_str() == *FS_DOT_GIT) {
+                    result.push(path);
+                }
             }
         }
         result

crates/project/src/worktree.rs 🔗

@@ -2541,7 +2541,7 @@ impl Entry {
     }
 
     pub fn git_status(&self) -> Option<GitFileStatus> {
-        self.git_status /*.status() */
+        self.git_status
     }
 }
 
@@ -3149,12 +3149,14 @@ impl BackgroundScanner {
         // refreshed. Do this before adding any new entries, so that renames can be
         // detected regardless of the order of the paths.
         let mut event_paths = Vec::<Arc<Path>>::with_capacity(abs_paths.len());
+        let mut event_metadata = Vec::<_>::with_capacity(abs_paths.len());
         for (abs_path, metadata) in abs_paths.iter().zip(metadata.iter()) {
             if let Ok(path) = abs_path.strip_prefix(&root_canonical_path) {
                 if matches!(metadata, Ok(None)) || doing_recursive_update {
                     state.remove_path(path);
                 }
                 event_paths.push(path.into());
+                event_metadata.push(metadata);
             } else {
                 log::error!(
                     "unexpected event {:?} for root path {:?}",
@@ -3164,7 +3166,7 @@ impl BackgroundScanner {
             }
         }
 
-        for (path, metadata) in event_paths.iter().cloned().zip(metadata.into_iter()) {
+        for (path, metadata) in event_paths.iter().cloned().zip(event_metadata.into_iter()) {
             let abs_path: Arc<Path> = root_abs_path.join(&path).into();
 
             match metadata {
@@ -3487,7 +3489,6 @@ impl BackgroundScanner {
             if new_paths.item().map_or(false, |e| e.path < path.0) {
                 new_paths.seek_forward(&path, Bias::Left, &());
             }
-
             loop {
                 match (old_paths.item(), new_paths.item()) {
                     (Some(old_entry), Some(new_entry)) => {
@@ -3506,6 +3507,13 @@ impl BackgroundScanner {
                             }
                             Ordering::Equal => {
                                 if self.phase == EventsReceivedDuringInitialScan {
+                                    if old_entry.id != new_entry.id {
+                                        changes.push((
+                                            old_entry.path.clone(),
+                                            old_entry.id,
+                                            Removed,
+                                        ));
+                                    }
                                     // If the worktree was not fully initialized when this event was generated,
                                     // we can't know whether this entry was added during the scan or whether
                                     // it was merely updated.
@@ -4685,7 +4693,7 @@ mod tests {
         log::info!("mutating fs");
         let mut files = Vec::new();
         let mut dirs = Vec::new();
-        for path in fs.as_fake().paths() {
+        for path in fs.as_fake().paths(false) {
             if path.starts_with(root_path) {
                 if fs.is_file(&path).await {
                     files.push(path);

crates/sum_tree/src/sum_tree.rs 🔗

@@ -480,6 +480,11 @@ impl<T: Item> SumTree<T> {
             } => child_trees.last().unwrap().rightmost_leaf(),
         }
     }
+
+    #[cfg(debug_assertions)]
+    pub fn _debug_entries(&self) -> Vec<&T> {
+        self.iter().collect::<Vec<_>>()
+    }
 }
 
 impl<T: Item + PartialEq> PartialEq for SumTree<T> {