diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 3ae7e5fc58..81cde74cbb 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -5,11 +5,12 @@ use crate::{ TestClient, TestServer, }, }; +use anyhow::{anyhow, Result}; use call::{room, ActiveCall, ParticipantLocation, Room}; use client::{User, RECEIVE_TIMEOUT}; use collections::{HashMap, HashSet}; use fs::{repository::GitFileStatus, FakeFs, Fs as _, RemoveOptions}; -use futures::StreamExt as _; +use futures::{channel::mpsc, StreamExt as _}; use gpui::{ px, size, AppContext, BackgroundExecutor, BorrowAppContext, Model, Modifiers, MouseButton, MouseDownEvent, TestAppContext, @@ -21,6 +22,7 @@ use language::{ }; use live_kit_client::MacOSDisplay; use lsp::LanguageServerId; +use parking_lot::Mutex; use project::{ search::SearchQuery, DiagnosticSummary, FormatTrigger, HoverBlockKind, Project, ProjectPath, SearchResult, @@ -4662,6 +4664,7 @@ async fn test_references( let mut fake_language_servers = client_a.language_registry().register_fake_lsp_adapter( "Rust", FakeLspAdapter { + name: "my-fake-lsp-adapter", capabilities: lsp::ServerCapabilities { references_provider: Some(lsp::OneOf::Left(true)), ..Default::default() @@ -4698,12 +4701,40 @@ async fn test_references( // Request references to a symbol as the guest. let fake_language_server = fake_language_servers.next().await.unwrap(); - fake_language_server.handle_request::(|params, _| async move { + let (lsp_response_tx, rx) = mpsc::unbounded::>>>(); + fake_language_server.handle_request::({ + let rx = Arc::new(Mutex::new(Some(rx))); + move |params, _| { + assert_eq!( + params.text_document_position.text_document.uri.as_str(), + "file:///root/dir-1/one.rs" + ); + let rx = rx.clone(); + async move { + let mut response_rx = rx.lock().take().unwrap(); + let result = response_rx.next().await.unwrap(); + *rx.lock() = Some(response_rx); + result + } + } + }); + + let references = project_b.update(cx_b, |p, cx| p.references(&buffer_b, 7, cx)); + + // User is informed that a request is pending. + executor.run_until_parked(); + project_b.read_with(cx_b, |project, _| { + let status = project.language_server_statuses().next().cloned().unwrap(); + assert_eq!(status.name, "my-fake-lsp-adapter"); assert_eq!( - params.text_document_position.text_document.uri.as_str(), - "file:///root/dir-1/one.rs" + status.pending_work.values().next().unwrap().message, + Some("Finding references...".into()) ); - Ok(Some(vec![ + }); + + // Cause the language server to respond. + lsp_response_tx + .unbounded_send(Ok(Some(vec![ lsp::Location { uri: lsp::Url::from_file_path("/root/dir-1/two.rs").unwrap(), range: lsp::Range::new(lsp::Position::new(0, 24), lsp::Position::new(0, 27)), @@ -4716,16 +4747,18 @@ async fn test_references( uri: lsp::Url::from_file_path("/root/dir-2/three.rs").unwrap(), range: lsp::Range::new(lsp::Position::new(0, 37), lsp::Position::new(0, 40)), }, - ])) - }); - - let references = project_b - .update(cx_b, |p, cx| p.references(&buffer_b, 7, cx)) - .await + ]))) .unwrap(); - cx_b.read(|cx| { + + let references = references.await.unwrap(); + executor.run_until_parked(); + project_b.read_with(cx_b, |project, cx| { + // User is informed that a request is no longer pending. + let status = project.language_server_statuses().next().unwrap(); + assert!(status.pending_work.is_empty()); + assert_eq!(references.len(), 3); - assert_eq!(project_b.read(cx).worktrees().count(), 2); + assert_eq!(project.worktrees().count(), 2); let two_buffer = references[0].buffer.read(cx); let three_buffer = references[2].buffer.read(cx); @@ -4743,6 +4776,32 @@ async fn test_references( assert_eq!(references[1].range.to_offset(two_buffer), 35..38); assert_eq!(references[2].range.to_offset(three_buffer), 37..40); }); + + let references = project_b.update(cx_b, |p, cx| p.references(&buffer_b, 7, cx)); + + // User is informed that a request is pending. + executor.run_until_parked(); + project_b.read_with(cx_b, |project, _| { + let status = project.language_server_statuses().next().cloned().unwrap(); + assert_eq!(status.name, "my-fake-lsp-adapter"); + assert_eq!( + status.pending_work.values().next().unwrap().message, + Some("Finding references...".into()) + ); + }); + + // Cause the LSP request to fail. + lsp_response_tx + .unbounded_send(Err(anyhow!("can't find references"))) + .unwrap(); + references.await.unwrap_err(); + + // User is informed that the request is no longer pending. + executor.run_until_parked(); + project_b.read_with(cx_b, |project, _| { + let status = project.language_server_statuses().next().unwrap(); + assert!(status.pending_work.is_empty()); + }); } #[gpui::test(iterations = 10)] diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index 59bd023d77..707bf91f75 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -463,7 +463,7 @@ impl TestAppContext { } } -impl Model { +impl Model { /// Block until the next event is emitted by the model, then return it. pub fn next_event(&self, cx: &mut TestAppContext) -> Evt where @@ -491,6 +491,31 @@ impl Model { } panic!("no event received") } + + /// Returns a future that resolves when the model notifies. + pub fn next_notification(&self, cx: &TestAppContext) -> impl Future { + use postage::prelude::{Sink as _, Stream as _}; + + let (mut tx, mut rx) = postage::mpsc::channel(1); + let mut cx = cx.app.app.borrow_mut(); + let subscription = cx.observe(self, move |_, _| { + tx.try_send(()).ok(); + }); + + let duration = if std::env::var("CI").is_ok() { + Duration::from_secs(5) + } else { + Duration::from_secs(1) + }; + + async move { + let notification = crate::util::timeout(duration, rx.recv()) + .await + .expect("next notification timed out"); + drop(subscription); + notification.expect("model dropped while test was waiting for its next notification") + } + } } impl View { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 75f329e824..58c1a6a093 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -347,7 +347,7 @@ pub enum LanguageServerState { }, } -#[derive(Serialize)] +#[derive(Clone, Debug, Serialize)] pub struct LanguageServerStatus { pub name: String, pub pending_work: BTreeMap, @@ -3924,19 +3924,6 @@ impl Project { }, cx, ); - self.enqueue_buffer_ordered_message( - BufferOrderedMessage::LanguageServerUpdate { - language_server_id, - message: proto::update_language_server::Variant::WorkStart( - proto::LspWorkStart { - token, - message: report.message, - percentage: report.percentage, - }, - ), - }, - ) - .ok(); } } lsp::WorkDoneProgress::Report(report) => { @@ -3984,15 +3971,6 @@ impl Project { .ok(); } else { self.on_lsp_work_end(language_server_id, token.clone(), cx); - self.enqueue_buffer_ordered_message( - BufferOrderedMessage::LanguageServerUpdate { - language_server_id, - message: proto::update_language_server::Variant::WorkEnd( - proto::LspWorkEnd { token }, - ), - }, - ) - .ok(); } } } @@ -4006,9 +3984,21 @@ impl Project { cx: &mut ModelContext, ) { if let Some(status) = self.language_server_statuses.get_mut(&language_server_id) { - status.pending_work.insert(token, progress); + status.pending_work.insert(token.clone(), progress.clone()); cx.notify(); } + + if self.is_local() { + self.enqueue_buffer_ordered_message(BufferOrderedMessage::LanguageServerUpdate { + language_server_id, + message: proto::update_language_server::Variant::WorkStart(proto::LspWorkStart { + token, + message: progress.message, + percentage: progress.percentage.map(|p| p as u32), + }), + }) + .ok(); + } } fn on_lsp_work_progress( @@ -4049,6 +4039,16 @@ impl Project { status.pending_work.remove(&token); cx.notify(); } + + if self.is_local() { + self.enqueue_buffer_ordered_message(BufferOrderedMessage::LanguageServerUpdate { + language_server_id, + message: proto::update_language_server::Variant::WorkEnd(proto::LspWorkEnd { + token, + }), + }) + .ok(); + } } fn on_lsp_did_change_watched_files( @@ -6721,7 +6721,7 @@ impl Project { let lsp_request = language_server.request::(lsp_params); let id = lsp_request.id(); - if status.is_some() { + let _cleanup = if status.is_some() { cx.update(|cx| { this.update(cx, |this, cx| { this.on_lsp_work_start( @@ -6737,22 +6737,35 @@ impl Project { }) }) .log_err(); - } + + Some(defer(|| { + cx.update(|cx| { + this.update(cx, |this, cx| { + this.on_lsp_work_end( + language_server.server_id(), + id.to_string(), + cx, + ); + }) + }) + .log_err(); + })) + } else { + None + }; let result = lsp_request.await; - let response = match result { - Ok(response) => response, - Err(err) => { - log::warn!( - "Generic lsp request to {} failed: {}", - language_server.name(), - err - ); - return Err(err); - } - }; - let result = request + let response = result.map_err(|err| { + log::warn!( + "Generic lsp request to {} failed: {}", + language_server.name(), + err + ); + err + })?; + + request .response_from_lsp( response, this.upgrade().ok_or_else(|| anyhow!("no app context"))?, @@ -6760,22 +6773,7 @@ impl Project { language_server.server_id(), cx.clone(), ) - .await; - - if status.is_some() { - cx.update(|cx| { - this.update(cx, |this, cx| { - this.on_lsp_work_end( - language_server.server_id(), - id.to_string(), - cx, - ); - }) - }) - .log_err(); - } - - result + .await }); } } else if let Some(project_id) = self.remote_id() {