fix display map tests

Piotr Osiewicz created

These tests failed due to an indefinite hang in buffer.condition in the following code:
\`\`\`rust
    let buffer = cx
        .add_model(|cx| Buffer::new(0, cx.model_id() as u64, text).with_language(language, cx));
    buffer.condition(cx, |buf, _| !buf.is_parsing()).await;
`\`\`
In both gpui1 and gpui2 \`.with_language\` spawns a task that notifies the context once it's done. The \`condition\` waits for notifications to be raised. The gist of the problem was that in gpui2, the spawned task was scheduled straight away, so we never really saw the notification with \`condition\`, causing us to wait indefinitely. This is probably a difference in test between schedulers in gpui1 and gpui2, but I kind of sidestepped the issue by spawning a condition before firing off a parsing task with \`set_language\`.

Change summary

crates/editor2/src/display_map.rs    | 24 ++++++++++++--------
crates/gpui2/src/app/test_context.rs | 35 ++++++++++++++++-------------
crates/project2/src/worktree.rs      | 14 +++++++----
3 files changed, 42 insertions(+), 31 deletions(-)

Detailed changes

crates/editor2/src/display_map.rs 🔗

@@ -1467,10 +1467,12 @@ pub mod tests {
 
         cx.update(|cx| init_test(cx, |s| s.defaults.tab_size = Some(2.try_into().unwrap())));
 
-        let buffer = cx.build_model(|cx| {
-            Buffer::new(0, cx.entity_id().as_u64(), text).with_language(language, cx)
+        let buffer = cx.build_model(|cx| Buffer::new(0, cx.entity_id().as_u64(), text));
+        let condition = cx.condition(&buffer, |buf, _| !buf.is_parsing());
+        cx.update_model(&buffer, |this, cx| {
+            this.set_language(Some(language), cx);
         });
-        cx.condition(&buffer, |buf, _| !buf.is_parsing()).await;
+        condition.await;
         let buffer = cx.build_model(|cx| MultiBuffer::singleton(buffer, cx));
 
         let font_size = px(14.0);
@@ -1554,10 +1556,12 @@ pub mod tests {
 
         cx.update(|cx| init_test(cx, |_| {}));
 
-        let buffer = cx.build_model(|cx| {
-            Buffer::new(0, cx.entity_id().as_u64(), text).with_language(language, cx)
+        let buffer = cx.build_model(|cx| Buffer::new(0, cx.entity_id().as_u64(), text));
+        let condition = cx.condition(&buffer, |buf, _| !buf.is_parsing());
+        buffer.update(cx, |this, cx| {
+            this.set_language(Some(language), cx);
         });
-        cx.condition(&buffer, |buf, _| !buf.is_parsing()).await;
+        condition.await;
         let buffer = cx.build_model(|cx| MultiBuffer::singleton(buffer, cx));
 
         let font_size = px(16.0);
@@ -1621,10 +1625,10 @@ pub mod tests {
 
         let (text, highlighted_ranges) = marked_text_ranges(r#"constˇ «a»: B = "c «d»""#, false);
 
-        let buffer = cx.build_model(|cx| {
-            Buffer::new(0, cx.entity_id().as_u64(), text).with_language(language, cx)
-        });
-        cx.condition(&buffer, |buf, _| !buf.is_parsing()).await;
+        let buffer = cx.build_model(|cx| Buffer::new(0, cx.entity_id().as_u64(), text));
+        let condition = cx.condition(&buffer, |buf, _| !buf.is_parsing());
+        buffer.update(cx, |this, cx| this.set_language(Some(language), cx));
+        condition.await;
 
         let buffer = cx.build_model(|cx| MultiBuffer::singleton(buffer, cx));
         let buffer_snapshot = buffer.read_with(cx, |buffer, cx| buffer.snapshot(cx));

crates/gpui2/src/app/test_context.rs 🔗

@@ -8,6 +8,7 @@ use crate::{
 use anyhow::{anyhow, bail};
 use futures::{Stream, StreamExt};
 use std::{future::Future, ops::Deref, rc::Rc, sync::Arc, time::Duration};
+use util::ResultExt;
 
 #[derive(Clone)]
 pub struct TestAppContext {
@@ -297,7 +298,7 @@ impl TestAppContext {
             .unwrap()
     }
 
-    pub fn notifications<T: 'static>(&mut self, entity: &impl Entity<T>) -> impl Stream<Item = ()> {
+    pub fn notifications<T: 'static>(&self, entity: &impl Entity<T>) -> impl Stream<Item = ()> {
         let (tx, rx) = futures::channel::mpsc::unbounded();
         self.update(|cx| {
             cx.observe(entity, {
@@ -307,7 +308,7 @@ impl TestAppContext {
                 }
             })
             .detach();
-            cx.observe_release(entity, move |_, _| tx.close_channel())
+            cx.observe_release(entity, move |_, _| dbg!(tx.close_channel()))
                 .detach()
         });
         rx
@@ -331,28 +332,30 @@ impl TestAppContext {
         rx
     }
 
-    pub async fn condition<T: 'static>(
-        &mut self,
+    pub fn condition<T: 'static>(
+        &self,
         model: &Model<T>,
-        mut predicate: impl FnMut(&mut T, &mut ModelContext<T>) -> bool,
-    ) {
+        mut predicate: impl FnMut(&mut T, &mut ModelContext<T>) -> bool + Send + 'static,
+    ) -> Task<()> {
         let timer = self.executor().timer(Duration::from_secs(3));
         let mut notifications = self.notifications(model);
 
         use futures::FutureExt as _;
         use smol::future::FutureExt as _;
-
-        async {
-            while notifications.next().await.is_some() {
-                if model.update(self, &mut predicate) {
-                    return Ok(());
+        let model = model.clone();
+        self.spawn(move |mut cx| async move {
+            async move {
+                while notifications.next().await.is_some() {
+                    if model.update(&mut cx, &mut predicate).log_err().unwrap() {
+                        return Ok(());
+                    }
                 }
+                bail!("model dropped")
             }
-            bail!("model dropped")
-        }
-        .race(timer.map(|_| Err(anyhow!("condition timed out"))))
-        .await
-        .unwrap();
+            .race(timer.map(|_| Err(anyhow!("condition timed out"))))
+            .await
+            .unwrap()
+        })
     }
 }
 

crates/project2/src/worktree.rs 🔗

@@ -4187,7 +4187,7 @@ impl WorktreeModelHandle for Model<Worktree> {
         &self,
         cx: &'a mut gpui::TestAppContext,
     ) -> futures::future::LocalBoxFuture<'a, ()> {
-        let file_name = "fs-event-sentinel";
+        let file_name: &'static str = "fs-event-sentinel";
 
         let tree = self.clone();
         let (fs, root_path) = self.update(cx, |tree, _| {
@@ -4200,14 +4200,18 @@ impl WorktreeModelHandle for Model<Worktree> {
                 .await
                 .unwrap();
 
-            cx.condition(&tree, |tree, _| tree.entry_for_path(file_name).is_some())
-                .await;
+            cx.condition(&tree, move |tree, _| {
+                tree.entry_for_path(file_name).is_some()
+            })
+            .await;
 
             fs.remove_file(&root_path.join(file_name), Default::default())
                 .await
                 .unwrap();
-            cx.condition(&tree, |tree, _| tree.entry_for_path(file_name).is_none())
-                .await;
+            cx.condition(&tree, move |tree, _| {
+                tree.entry_for_path(file_name).is_none()
+            })
+            .await;
 
             cx.update(|cx| tree.read(cx).as_local().unwrap().scan_complete())
                 .await;