Better TestWindow support (#3876)

Conrad Irwin created

Adding guest roles led us down a rabbit hole where we'd have liked to
rely on a
side-effect of activating a window in tests; but the test window didn't
implement that.

Looking into that, I realized our TestWindow wasn't doing a great job of
emulating the MacWindow, so this makes the two more similar.

Change summary

crates/diagnostics/src/diagnostics.rs         |   1 
crates/editor/src/link_go_to_definition.rs    |   5 
crates/gpui/src/app/test_context.rs           | 135 +++----------------
crates/gpui/src/platform/test/platform.rs     |  38 ++++
crates/gpui/src/platform/test/window.rs       | 143 ++++++++++++++++----
crates/project_symbols/src/project_symbols.rs |   1 
crates/workspace/src/workspace.rs             |   9 
7 files changed, 176 insertions(+), 156 deletions(-)

Detailed changes

@@ -930,10 +930,7 @@ mod tests {
                 fn do_work() { Β«testΒ»(); }
             "});
 
-        // Deactivating the window dismisses the highlight
-        cx.update_workspace(|workspace, cx| {
-            workspace.on_window_activation_changed(cx);
-        });
+        cx.cx.cx.deactivate_window();
         cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
                 fn test() { do_work(); }
                 fn do_work() { test(); }

crates/gpui/src/app/test_context.rs πŸ”—

@@ -1,14 +1,13 @@
 use crate::{
     div, Action, AnyView, AnyWindowHandle, AppCell, AppContext, AsyncAppContext,
-    BackgroundExecutor, Bounds, ClipboardItem, Context, Entity, EventEmitter, ForegroundExecutor,
-    InputEvent, IntoElement, KeyDownEvent, Keystroke, Model, ModelContext, Pixels, Platform,
-    PlatformWindow, Point, Render, Result, Size, Task, TestDispatcher, TestPlatform, TestWindow,
-    TestWindowHandlers, TextSystem, View, ViewContext, VisualContext, WindowBounds, WindowContext,
-    WindowHandle, WindowOptions,
+    BackgroundExecutor, ClipboardItem, Context, Entity, EventEmitter, ForegroundExecutor,
+    IntoElement, Keystroke, Model, ModelContext, Pixels, Platform, Render, Result, Size, Task,
+    TestDispatcher, TestPlatform, TestWindow, TextSystem, View, ViewContext, VisualContext,
+    WindowContext, WindowHandle, WindowOptions,
 };
 use anyhow::{anyhow, bail};
 use futures::{Stream, StreamExt};
-use std::{future::Future, mem, ops::Deref, rc::Rc, sync::Arc, time::Duration};
+use std::{future::Future, ops::Deref, rc::Rc, sync::Arc, time::Duration};
 
 #[derive(Clone)]
 pub struct TestAppContext {
@@ -185,42 +184,7 @@ impl TestAppContext {
     }
 
     pub fn simulate_window_resize(&self, window_handle: AnyWindowHandle, size: Size<Pixels>) {
-        let (mut handlers, scale_factor) = self
-            .app
-            .borrow_mut()
-            .update_window(window_handle, |_, cx| {
-                let platform_window = cx.window.platform_window.as_test().unwrap();
-                let scale_factor = platform_window.scale_factor();
-                match &mut platform_window.bounds {
-                    WindowBounds::Fullscreen | WindowBounds::Maximized => {
-                        platform_window.bounds = WindowBounds::Fixed(Bounds {
-                            origin: Point::default(),
-                            size: size.map(|pixels| f64::from(pixels).into()),
-                        });
-                    }
-                    WindowBounds::Fixed(bounds) => {
-                        bounds.size = size.map(|pixels| f64::from(pixels).into());
-                    }
-                }
-
-                (
-                    mem::take(&mut platform_window.handlers.lock().resize),
-                    scale_factor,
-                )
-            })
-            .unwrap();
-
-        for handler in &mut handlers {
-            handler(size, scale_factor);
-        }
-
-        self.app
-            .borrow_mut()
-            .update_window(window_handle, |_, cx| {
-                let platform_window = cx.window.platform_window.as_test().unwrap();
-                platform_window.handlers.lock().resize = handlers;
-            })
-            .unwrap();
+        self.test_window(window_handle).simulate_resize(size);
     }
 
     pub fn spawn<Fut, R>(&self, f: impl FnOnce(AsyncAppContext) -> Fut) -> Task<R>
@@ -313,41 +277,22 @@ impl TestAppContext {
         keystroke: Keystroke,
         is_held: bool,
     ) {
-        let keystroke2 = keystroke.clone();
-        let handled = window
-            .update(self, |_, cx| {
-                cx.dispatch_event(InputEvent::KeyDown(KeyDownEvent { keystroke, is_held }))
-            })
-            .is_ok_and(|handled| handled);
-        if handled {
-            return;
-        }
-
-        let input_handler = self.update_test_window(window, |window| window.input_handler.clone());
-        let Some(input_handler) = input_handler else {
-            panic!(
-                "dispatch_keystroke {:?} failed to dispatch action or input",
-                &keystroke2
-            );
-        };
-        let text = keystroke2.ime_key.unwrap_or(keystroke2.key);
-        input_handler.lock().replace_text_in_range(None, &text);
+        self.test_window(window)
+            .simulate_keystroke(keystroke, is_held)
     }
 
-    pub fn update_test_window<R>(
-        &mut self,
-        window: AnyWindowHandle,
-        f: impl FnOnce(&mut TestWindow) -> R,
-    ) -> R {
-        window
-            .update(self, |_, cx| {
-                f(cx.window
-                    .platform_window
-                    .as_any_mut()
-                    .downcast_mut::<TestWindow>()
-                    .unwrap())
-            })
+    pub fn test_window(&self, window: AnyWindowHandle) -> TestWindow {
+        self.app
+            .borrow_mut()
+            .windows
+            .get_mut(window.id)
+            .unwrap()
+            .as_mut()
             .unwrap()
+            .platform_window
+            .as_test()
+            .unwrap()
+            .clone()
     }
 
     pub fn notifications<T: 'static>(&mut self, entity: &impl Entity<T>) -> impl Stream<Item = ()> {
@@ -563,11 +508,7 @@ impl<'a> VisualTestContext<'a> {
     }
 
     pub fn window_title(&mut self) -> Option<String> {
-        self.cx
-            .update_window(self.window, |_, cx| {
-                cx.window.platform_window.as_test().unwrap().title.clone()
-            })
-            .unwrap()
+        self.cx.test_window(self.window).0.lock().title.clone()
     }
 
     pub fn simulate_keystrokes(&mut self, keystrokes: &str) {
@@ -578,37 +519,11 @@ impl<'a> VisualTestContext<'a> {
         self.cx.simulate_input(self.window, input)
     }
 
-    pub fn simulate_activation(&mut self) {
-        self.simulate_window_events(&mut |handlers| {
-            handlers
-                .active_status_change
-                .iter_mut()
-                .for_each(|f| f(true));
-        })
-    }
-
-    pub fn simulate_deactivation(&mut self) {
-        self.simulate_window_events(&mut |handlers| {
-            handlers
-                .active_status_change
-                .iter_mut()
-                .for_each(|f| f(false));
-        })
-    }
-
-    fn simulate_window_events(&mut self, f: &mut dyn FnMut(&mut TestWindowHandlers)) {
-        let handlers = self
-            .cx
-            .update_window(self.window, |_, cx| {
-                cx.window
-                    .platform_window
-                    .as_test()
-                    .unwrap()
-                    .handlers
-                    .clone()
-            })
-            .unwrap();
-        f(&mut *handlers.lock());
+    pub fn deactivate_window(&mut self) {
+        if Some(self.window) == self.test_platform.active_window() {
+            self.test_platform.set_active_window(None)
+        }
+        self.background_executor.run_until_parked();
     }
 }
 

crates/gpui/src/platform/test/platform.rs πŸ”—

@@ -19,7 +19,7 @@ pub struct TestPlatform {
     background_executor: BackgroundExecutor,
     foreground_executor: ForegroundExecutor,
 
-    active_window: Arc<Mutex<Option<AnyWindowHandle>>>,
+    pub(crate) active_window: RefCell<Option<TestWindow>>,
     active_display: Rc<dyn PlatformDisplay>,
     active_cursor: Mutex<CursorStyle>,
     current_clipboard_item: Mutex<Option<ClipboardItem>>,
@@ -79,6 +79,28 @@ impl TestPlatform {
         self.prompts.borrow_mut().multiple_choice.push_back(tx);
         rx
     }
+
+    pub(crate) fn set_active_window(&self, window: Option<TestWindow>) {
+        let executor = self.foreground_executor().clone();
+        let previous_window = self.active_window.borrow_mut().take();
+        *self.active_window.borrow_mut() = window.clone();
+
+        executor
+            .spawn(async move {
+                if let Some(previous_window) = previous_window {
+                    if let Some(window) = window.as_ref() {
+                        if Arc::ptr_eq(&previous_window.0, &window.0) {
+                            return;
+                        }
+                    }
+                    previous_window.simulate_active_status_change(false);
+                }
+                if let Some(window) = window {
+                    window.simulate_active_status_change(true);
+                }
+            })
+            .detach();
+    }
 }
 
 // todo!("implement out what our tests needed in GPUI 1")
@@ -106,7 +128,7 @@ impl Platform for TestPlatform {
     }
 
     fn activate(&self, _ignoring_other_apps: bool) {
-        unimplemented!()
+        //
     }
 
     fn hide(&self) {
@@ -130,7 +152,10 @@ impl Platform for TestPlatform {
     }
 
     fn active_window(&self) -> Option<crate::AnyWindowHandle> {
-        self.active_window.lock().clone()
+        self.active_window
+            .borrow()
+            .as_ref()
+            .map(|window| window.0.lock().handle)
     }
 
     fn open_window(
@@ -139,12 +164,13 @@ impl Platform for TestPlatform {
         options: WindowOptions,
         _draw: Box<dyn FnMut() -> Result<Scene>>,
     ) -> Box<dyn crate::PlatformWindow> {
-        *self.active_window.lock() = Some(handle);
-        Box::new(TestWindow::new(
+        let window = TestWindow::new(
             options,
+            handle,
             self.weak.clone(),
             self.active_display.clone(),
-        ))
+        );
+        Box::new(window)
     }
 
     fn set_display_link_output_callback(

crates/gpui/src/platform/test/window.rs πŸ”—

@@ -1,7 +1,7 @@
 use crate::{
-    px, AtlasKey, AtlasTextureId, AtlasTile, Pixels, PlatformAtlas, PlatformDisplay,
-    PlatformInputHandler, PlatformWindow, Point, Size, TestPlatform, TileId, WindowAppearance,
-    WindowBounds, WindowOptions,
+    px, AnyWindowHandle, AtlasKey, AtlasTextureId, AtlasTile, Bounds, InputEvent, KeyDownEvent,
+    Keystroke, Pixels, PlatformAtlas, PlatformDisplay, PlatformInputHandler, PlatformWindow, Point,
+    Size, TestPlatform, TileId, WindowAppearance, WindowBounds, WindowOptions,
 };
 use collections::HashMap;
 use parking_lot::Mutex;
@@ -10,51 +10,122 @@ use std::{
     sync::{self, Arc},
 };
 
-#[derive(Default)]
-pub(crate) struct TestWindowHandlers {
-    pub(crate) active_status_change: Vec<Box<dyn FnMut(bool)>>,
-    pub(crate) input: Vec<Box<dyn FnMut(crate::InputEvent) -> bool>>,
-    pub(crate) moved: Vec<Box<dyn FnMut()>>,
-    pub(crate) resize: Vec<Box<dyn FnMut(Size<Pixels>, f32)>>,
-}
-
-pub struct TestWindow {
+pub struct TestWindowState {
     pub(crate) bounds: WindowBounds,
+    pub(crate) handle: AnyWindowHandle,
     display: Rc<dyn PlatformDisplay>,
     pub(crate) title: Option<String>,
     pub(crate) edited: bool,
-    pub(crate) input_handler: Option<Arc<Mutex<Box<dyn PlatformInputHandler>>>>,
-    pub(crate) handlers: Arc<Mutex<TestWindowHandlers>>,
     platform: Weak<TestPlatform>,
     sprite_atlas: Arc<dyn PlatformAtlas>,
+
+    input_callback: Option<Box<dyn FnMut(InputEvent) -> bool>>,
+    active_status_change_callback: Option<Box<dyn FnMut(bool)>>,
+    resize_callback: Option<Box<dyn FnMut(Size<Pixels>, f32)>>,
+    moved_callback: Option<Box<dyn FnMut()>>,
+    input_handler: Option<Box<dyn PlatformInputHandler>>,
 }
 
+#[derive(Clone)]
+pub struct TestWindow(pub(crate) Arc<Mutex<TestWindowState>>);
+
 impl TestWindow {
     pub fn new(
         options: WindowOptions,
+        handle: AnyWindowHandle,
         platform: Weak<TestPlatform>,
         display: Rc<dyn PlatformDisplay>,
     ) -> Self {
-        Self {
+        Self(Arc::new(Mutex::new(TestWindowState {
             bounds: options.bounds,
             display,
             platform,
-            input_handler: None,
+            handle,
             sprite_atlas: Arc::new(TestAtlas::new()),
-            handlers: Default::default(),
             title: Default::default(),
             edited: false,
+
+            input_callback: None,
+            active_status_change_callback: None,
+            resize_callback: None,
+            moved_callback: None,
+            input_handler: None,
+        })))
+    }
+
+    pub fn simulate_resize(&mut self, size: Size<Pixels>) {
+        let scale_factor = self.scale_factor();
+        let mut lock = self.0.lock();
+        let Some(mut callback) = lock.resize_callback.take() else {
+            return;
+        };
+        match &mut lock.bounds {
+            WindowBounds::Fullscreen | WindowBounds::Maximized => {
+                lock.bounds = WindowBounds::Fixed(Bounds {
+                    origin: Point::default(),
+                    size: size.map(|pixels| f64::from(pixels).into()),
+                });
+            }
+            WindowBounds::Fixed(bounds) => {
+                bounds.size = size.map(|pixels| f64::from(pixels).into());
+            }
         }
+        drop(lock);
+        callback(size, scale_factor);
+        self.0.lock().resize_callback = Some(callback);
+    }
+
+    pub(crate) fn simulate_active_status_change(&self, active: bool) {
+        let mut lock = self.0.lock();
+        let Some(mut callback) = lock.active_status_change_callback.take() else {
+            return;
+        };
+        drop(lock);
+        callback(active);
+        self.0.lock().active_status_change_callback = Some(callback);
+    }
+
+    pub fn simulate_input(&mut self, event: InputEvent) -> bool {
+        let mut lock = self.0.lock();
+        let Some(mut callback) = lock.input_callback.take() else {
+            return false;
+        };
+        drop(lock);
+        let result = callback(event);
+        self.0.lock().input_callback = Some(callback);
+        result
+    }
+
+    pub fn simulate_keystroke(&mut self, keystroke: Keystroke, is_held: bool) {
+        if self.simulate_input(InputEvent::KeyDown(KeyDownEvent {
+            keystroke: keystroke.clone(),
+            is_held,
+        })) {
+            return;
+        }
+
+        let mut lock = self.0.lock();
+        let Some(mut input_handler) = lock.input_handler.take() else {
+            panic!(
+                "simulate_keystroke {:?} input event was not handled and there was no active input",
+                &keystroke
+            );
+        };
+        drop(lock);
+        let text = keystroke.ime_key.unwrap_or(keystroke.key);
+        input_handler.replace_text_in_range(None, &text);
+
+        self.0.lock().input_handler = Some(input_handler);
     }
 }
 
 impl PlatformWindow for TestWindow {
     fn bounds(&self) -> WindowBounds {
-        self.bounds
+        self.0.lock().bounds
     }
 
     fn content_size(&self) -> Size<Pixels> {
-        let bounds = match self.bounds {
+        let bounds = match self.bounds() {
             WindowBounds::Fixed(bounds) => bounds,
             WindowBounds::Maximized | WindowBounds::Fullscreen => self.display().bounds(),
         };
@@ -74,7 +145,7 @@ impl PlatformWindow for TestWindow {
     }
 
     fn display(&self) -> std::rc::Rc<dyn crate::PlatformDisplay> {
-        self.display.clone()
+        self.0.lock().display.clone()
     }
 
     fn mouse_position(&self) -> Point<Pixels> {
@@ -90,11 +161,11 @@ impl PlatformWindow for TestWindow {
     }
 
     fn set_input_handler(&mut self, input_handler: Box<dyn crate::PlatformInputHandler>) {
-        self.input_handler = Some(Arc::new(Mutex::new(input_handler)));
+        self.0.lock().input_handler = Some(input_handler);
     }
 
     fn clear_input_handler(&mut self) {
-        self.input_handler = None;
+        self.0.lock().input_handler = None;
     }
 
     fn prompt(
@@ -103,19 +174,29 @@ impl PlatformWindow for TestWindow {
         _msg: &str,
         _answers: &[&str],
     ) -> futures::channel::oneshot::Receiver<usize> {
-        self.platform.upgrade().expect("platform dropped").prompt()
+        self.0
+            .lock()
+            .platform
+            .upgrade()
+            .expect("platform dropped")
+            .prompt()
     }
 
     fn activate(&self) {
-        unimplemented!()
+        self.0
+            .lock()
+            .platform
+            .upgrade()
+            .unwrap()
+            .set_active_window(Some(self.clone()))
     }
 
     fn set_title(&mut self, title: &str) {
-        self.title = Some(title.to_owned());
+        self.0.lock().title = Some(title.to_owned());
     }
 
     fn set_edited(&mut self, edited: bool) {
-        self.edited = edited;
+        self.0.lock().edited = edited;
     }
 
     fn show_character_palette(&self) {
@@ -135,15 +216,15 @@ impl PlatformWindow for TestWindow {
     }
 
     fn on_input(&self, callback: Box<dyn FnMut(crate::InputEvent) -> bool>) {
-        self.handlers.lock().input.push(callback)
+        self.0.lock().input_callback = Some(callback)
     }
 
     fn on_active_status_change(&self, callback: Box<dyn FnMut(bool)>) {
-        self.handlers.lock().active_status_change.push(callback)
+        self.0.lock().active_status_change_callback = Some(callback)
     }
 
     fn on_resize(&self, callback: Box<dyn FnMut(Size<Pixels>, f32)>) {
-        self.handlers.lock().resize.push(callback)
+        self.0.lock().resize_callback = Some(callback)
     }
 
     fn on_fullscreen(&self, _callback: Box<dyn FnMut(bool)>) {
@@ -151,7 +232,7 @@ impl PlatformWindow for TestWindow {
     }
 
     fn on_moved(&self, callback: Box<dyn FnMut()>) {
-        self.handlers.lock().moved.push(callback)
+        self.0.lock().moved_callback = Some(callback)
     }
 
     fn on_should_close(&self, _callback: Box<dyn FnMut() -> bool>) {
@@ -175,7 +256,7 @@ impl PlatformWindow for TestWindow {
     }
 
     fn sprite_atlas(&self) -> sync::Arc<dyn crate::PlatformAtlas> {
-        self.sprite_atlas.clone()
+        self.0.lock().sprite_atlas.clone()
     }
 
     fn as_test(&mut self) -> Option<&mut TestWindow> {

crates/workspace/src/workspace.rs πŸ”—

@@ -3266,6 +3266,7 @@ impl Workspace {
         let user_store = project.read(cx).user_store();
 
         let workspace_store = cx.new_model(|cx| WorkspaceStore::new(client.clone(), cx));
+        cx.activate_window();
         let app_state = Arc::new(AppState {
             languages: project.read(cx).languages().clone(),
             workspace_store,
@@ -4764,8 +4765,7 @@ mod tests {
         });
 
         // Deactivating the window saves the file.
-        cx.simulate_deactivation();
-        cx.executor().run_until_parked();
+        cx.deactivate_window();
         item.update(cx, |item, _| assert_eq!(item.save_count, 1));
 
         // Autosave on focus change.
@@ -4785,14 +4785,13 @@ mod tests {
         item.update(cx, |item, _| assert_eq!(item.save_count, 2));
 
         // Deactivating the window still saves the file.
-        cx.simulate_activation();
+        cx.update(|cx| cx.activate_window());
         item.update(cx, |item, cx| {
             cx.focus_self();
             item.is_dirty = true;
         });
-        cx.simulate_deactivation();
+        cx.deactivate_window();
 
-        cx.executor().run_until_parked();
         item.update(cx, |item, _| assert_eq!(item.save_count, 3));
 
         // Autosave after delay.