add test coverage for diagnostic popover

K Simmons created

Change summary

crates/editor/src/hover_popover.rs | 145 ++++++++++++++++++++++++-------
crates/editor/src/test.rs          |  81 +++++++++++++++--
crates/language/src/buffer.rs      |   6 
3 files changed, 185 insertions(+), 47 deletions(-)

Detailed changes

crates/editor/src/hover_popover.rs 🔗

@@ -419,6 +419,8 @@ mod tests {
     use futures::StreamExt;
     use indoc::indoc;
 
+    use language::{Diagnostic, DiagnosticSet};
+    use lsp::notification;
     use project::HoverBlock;
 
     use crate::test::EditorLspTestContext;
@@ -426,7 +428,7 @@ mod tests {
     use super::*;
 
     #[gpui::test]
-    async fn test_hover_popover(cx: &mut gpui::TestAppContext) {
+    async fn test_mouse_hover_info_popover(cx: &mut gpui::TestAppContext) {
         let mut cx = EditorLspTestContext::new_rust(
             lsp::ServerCapabilities {
                 hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
@@ -460,19 +462,18 @@ mod tests {
             fn test()
                 [println!]();"});
         let mut requests =
-            cx.lsp
-                .handle_request::<lsp::request::HoverRequest, _, _>(move |_, _| async move {
-                    Ok(Some(lsp::Hover {
-                        contents: lsp::HoverContents::Markup(lsp::MarkupContent {
-                            kind: lsp::MarkupKind::Markdown,
-                            value: indoc! {"
+            cx.handle_request::<lsp::request::HoverRequest, _, _>(move |_, _, _| async move {
+                Ok(Some(lsp::Hover {
+                    contents: lsp::HoverContents::Markup(lsp::MarkupContent {
+                        kind: lsp::MarkupKind::Markdown,
+                        value: indoc! {"
                                 # Some basic docs
                                 Some test documentation"}
-                            .to_string(),
-                        }),
-                        range: Some(symbol_range),
-                    }))
-                });
+                        .to_string(),
+                    }),
+                    range: Some(symbol_range),
+                }))
+            });
         cx.foreground()
             .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100));
         requests.next().await;
@@ -498,6 +499,9 @@ mod tests {
         let hover_point = cx.display_point(indoc! {"
             fn te|st()
                 println!();"});
+        let mut request = cx
+            .lsp
+            .handle_request::<lsp::request::HoverRequest, _, _>(|_, _| async move { Ok(None) });
         cx.update_editor(|editor, cx| {
             hover_at(
                 editor,
@@ -507,15 +511,24 @@ mod tests {
                 cx,
             )
         });
-        let mut request = cx
-            .lsp
-            .handle_request::<lsp::request::HoverRequest, _, _>(|_, _| async move { Ok(None) });
         cx.foreground()
             .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100));
         request.next().await;
         cx.editor(|editor, _| {
             assert!(!editor.hover_state.visible());
         });
+    }
+
+    #[gpui::test]
+    async fn test_keyboard_hover_info_popover(cx: &mut gpui::TestAppContext) {
+        let mut cx = EditorLspTestContext::new_rust(
+            lsp::ServerCapabilities {
+                hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
+                ..Default::default()
+            },
+            cx,
+        )
+        .await;
 
         // Hover with keyboard has no delay
         cx.set_state(indoc! {"
@@ -525,24 +538,23 @@ mod tests {
         let symbol_range = cx.lsp_range(indoc! {"
             [fn] test()
                 println!();"});
-        cx.lsp
-            .handle_request::<lsp::request::HoverRequest, _, _>(move |_, _| async move {
-                Ok(Some(lsp::Hover {
-                    contents: lsp::HoverContents::Markup(lsp::MarkupContent {
-                        kind: lsp::MarkupKind::Markdown,
-                        value: indoc! {"
-                            # Some other basic docs
-                            Some other test documentation"}
-                        .to_string(),
-                    }),
-                    range: Some(symbol_range),
-                }))
-            })
-            .next()
-            .await;
-        cx.foreground().run_until_parked();
+        cx.handle_request::<lsp::request::HoverRequest, _, _>(move |_, _, _| async move {
+            Ok(Some(lsp::Hover {
+                contents: lsp::HoverContents::Markup(lsp::MarkupContent {
+                    kind: lsp::MarkupKind::Markdown,
+                    value: indoc! {"
+                        # Some other basic docs
+                        Some other test documentation"}
+                    .to_string(),
+                }),
+                range: Some(symbol_range),
+            }))
+        })
+        .next()
+        .await;
+
+        cx.condition(|editor, _| editor.hover_state.visible()).await;
         cx.editor(|editor, _| {
-            assert!(editor.hover_state.visible());
             assert_eq!(
                 editor.hover_state.info_popover.clone().unwrap().contents,
                 vec![
@@ -558,4 +570,73 @@ mod tests {
             )
         });
     }
+
+    #[gpui::test]
+    async fn test_hover_diagnostic_and_info_popovers(cx: &mut gpui::TestAppContext) {
+        let mut cx = EditorLspTestContext::new_rust(
+            lsp::ServerCapabilities {
+                hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
+                ..Default::default()
+            },
+            cx,
+        )
+        .await;
+
+        // Hover with just diagnostic, pops DiagnosticPopover immediately and then
+        // info popover once request completes
+        cx.set_state(indoc! {"
+            fn te|st()
+                println!();"});
+
+        // Send diagnostic to client
+        let range = cx.text_anchor_range(indoc! {"
+            fn [test]()
+                println!();"});
+        cx.update_buffer(|buffer, cx| {
+            let snapshot = buffer.text_snapshot();
+            let set = DiagnosticSet::from_sorted_entries(
+                vec![DiagnosticEntry {
+                    range,
+                    diagnostic: Diagnostic {
+                        message: "A test diagnostic message.".to_string(),
+                        ..Default::default()
+                    },
+                }],
+                &snapshot,
+            );
+            buffer.update_diagnostics(set, cx);
+        });
+
+        // Hover pops diagnostic immediately
+        cx.update_editor(|editor, cx| hover(editor, &Hover, cx));
+        cx.condition(|Editor { hover_state, .. }, _| {
+            hover_state.diagnostic_popover.is_some() && hover_state.info_popover.is_none()
+        })
+        .await;
+
+        // Info Popover shows after request responded to
+        let range = cx.lsp_range(indoc! {"
+            fn [test]()
+                println!();"});
+        let mut requests =
+            cx.handle_request::<lsp::request::HoverRequest, _, _>(move |_, _, _| async move {
+                Ok(Some(lsp::Hover {
+                    contents: lsp::HoverContents::Markup(lsp::MarkupContent {
+                        kind: lsp::MarkupKind::Markdown,
+                        value: indoc! {"
+                        # Some other basic docs
+                        Some other test documentation"}
+                        .to_string(),
+                    }),
+                    range: Some(range),
+                }))
+            });
+        cx.foreground()
+            .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100));
+        requests.next().await;
+        cx.condition(|Editor { hover_state, .. }, _| {
+            hover_state.diagnostic_popover.is_some() && hover_state.info_task.is_some()
+        })
+        .await;
+    }
 }

crates/editor/src/test.rs 🔗

@@ -9,9 +9,13 @@ use futures::{Future, StreamExt};
 use indoc::indoc;
 
 use collections::BTreeMap;
-use gpui::{json, keymap::Keystroke, AppContext, ModelHandle, ViewContext, ViewHandle};
-use language::{point_to_lsp, FakeLspAdapter, Language, LanguageConfig, Selection};
-use lsp::request;
+use gpui::{
+    json, keymap::Keystroke, AppContext, ModelContext, ModelHandle, ViewContext, ViewHandle,
+};
+use language::{
+    point_to_lsp, Buffer, BufferSnapshot, FakeLspAdapter, Language, LanguageConfig, Selection,
+};
+use lsp::{notification, request};
 use project::Project;
 use settings::Settings;
 use util::{
@@ -23,7 +27,8 @@ use workspace::{pane, AppState, Workspace, WorkspaceHandle};
 use crate::{
     display_map::{DisplayMap, DisplaySnapshot, ToDisplayPoint},
     multi_buffer::ToPointUtf16,
-    AnchorRangeExt, Autoscroll, DisplayPoint, Editor, EditorMode, MultiBuffer, ToPoint,
+    AnchorRangeExt, Autoscroll, DisplayPoint, Editor, EditorMode, EditorSnapshot, MultiBuffer,
+    ToPoint,
 };
 
 #[cfg(test)]
@@ -119,7 +124,7 @@ impl<'a> EditorTestContext<'a> {
         self.editor.condition(self.cx, predicate)
     }
 
-    pub fn editor<F, T>(&mut self, read: F) -> T
+    pub fn editor<F, T>(&self, read: F) -> T
     where
         F: FnOnce(&Editor, &AppContext) -> T,
     {
@@ -133,12 +138,48 @@ impl<'a> EditorTestContext<'a> {
         self.editor.update(self.cx, update)
     }
 
-    pub fn buffer_text(&mut self) -> String {
-        self.editor.read_with(self.cx, |editor, cx| {
-            editor.buffer.read(cx).snapshot(cx).text()
+    pub fn multibuffer<F, T>(&self, read: F) -> T
+    where
+        F: FnOnce(&MultiBuffer, &AppContext) -> T,
+    {
+        self.editor(|editor, cx| read(editor.buffer().read(cx), cx))
+    }
+
+    pub fn update_multibuffer<F, T>(&mut self, update: F) -> T
+    where
+        F: FnOnce(&mut MultiBuffer, &mut ModelContext<MultiBuffer>) -> T,
+    {
+        self.update_editor(|editor, cx| editor.buffer().update(cx, update))
+    }
+
+    pub fn buffer_text(&self) -> String {
+        self.multibuffer(|buffer, cx| buffer.snapshot(cx).text())
+    }
+
+    pub fn buffer<F, T>(&self, read: F) -> T
+    where
+        F: FnOnce(&Buffer, &AppContext) -> T,
+    {
+        self.multibuffer(|multibuffer, cx| {
+            let buffer = multibuffer.as_singleton().unwrap().read(cx);
+            read(buffer, cx)
         })
     }
 
+    pub fn update_buffer<F, T>(&mut self, update: F) -> T
+    where
+        F: FnOnce(&mut Buffer, &mut ModelContext<Buffer>) -> T,
+    {
+        self.update_multibuffer(|multibuffer, cx| {
+            let buffer = multibuffer.as_singleton().unwrap();
+            buffer.update(cx, update)
+        })
+    }
+
+    pub fn buffer_snapshot(&self) -> BufferSnapshot {
+        self.buffer(|buffer, _| buffer.snapshot())
+    }
+
     pub fn simulate_keystroke(&mut self, keystroke_text: &str) {
         let keystroke = Keystroke::parse(keystroke_text).unwrap();
         let input = if keystroke.modified() {
@@ -164,6 +205,18 @@ impl<'a> EditorTestContext<'a> {
         locations[0].to_display_point(&snapshot.display_snapshot)
     }
 
+    // Returns anchors for the current buffer using `[`..`]`
+    pub fn text_anchor_range(&self, marked_text: &str) -> Range<language::Anchor> {
+        let range_marker: TextRangeMarker = ('[', ']').into();
+        let (unmarked_text, mut ranges) =
+            marked_text_ranges_by(&marked_text, vec![range_marker.clone()]);
+        assert_eq!(self.buffer_text(), unmarked_text);
+        let offset_range = ranges.remove(&range_marker).unwrap()[0].clone();
+        let snapshot = self.buffer_snapshot();
+
+        snapshot.anchor_before(offset_range.start)..snapshot.anchor_after(offset_range.end)
+    }
+
     // Sets the editor state via a marked string.
     // `|` characters represent empty selections
     // `[` to `}` represents a non empty selection with the head at `}`
@@ -433,7 +486,7 @@ pub struct EditorLspTestContext<'a> {
     pub cx: EditorTestContext<'a>,
     pub lsp: lsp::FakeLanguageServer,
     pub workspace: ViewHandle<Workspace>,
-    pub editor_lsp_url: lsp::Url,
+    pub buffer_lsp_url: lsp::Url,
 }
 
 impl<'a> EditorLspTestContext<'a> {
@@ -507,7 +560,7 @@ impl<'a> EditorLspTestContext<'a> {
             },
             lsp,
             workspace,
-            editor_lsp_url: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
+            buffer_lsp_url: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
         }
     }
 
@@ -530,7 +583,7 @@ impl<'a> EditorLspTestContext<'a> {
     // Constructs lsp range using a marked string with '[', ']' range delimiters
     pub fn lsp_range(&mut self, marked_text: &str) -> lsp::Range {
         let (unmarked, mut ranges) = marked_text_ranges_by(marked_text, vec![('[', ']').into()]);
-        assert_eq!(unmarked, self.cx.buffer_text());
+        assert_eq!(unmarked, self.buffer_text());
         let offset_range = ranges.remove(&('[', ']').into()).unwrap()[0].clone();
         self.to_lsp_range(offset_range)
     }
@@ -594,12 +647,16 @@ impl<'a> EditorLspTestContext<'a> {
         F: 'static + Send + FnMut(lsp::Url, T::Params, gpui::AsyncAppContext) -> Fut,
         Fut: 'static + Send + Future<Output = Result<T::Result>>,
     {
-        let url = self.editor_lsp_url.clone();
+        let url = self.buffer_lsp_url.clone();
         self.lsp.handle_request::<T, _, _>(move |params, cx| {
             let url = url.clone();
             handler(url, params, cx)
         })
     }
+
+    pub fn notify<T: notification::Notification>(&self, params: T::Params) {
+        self.lsp.notify::<T>(params);
+    }
 }
 
 impl<'a> Deref for EditorLspTestContext<'a> {

crates/language/src/buffer.rs 🔗

@@ -2462,11 +2462,11 @@ impl operation_queue::Operation for Operation {
 impl Default for Diagnostic {
     fn default() -> Self {
         Self {
-            code: Default::default(),
+            code: None,
             severity: DiagnosticSeverity::ERROR,
             message: Default::default(),
-            group_id: Default::default(),
-            is_primary: Default::default(),
+            group_id: 0,
+            is_primary: false,
             is_valid: true,
             is_disk_based: false,
             is_unnecessary: false,