From 399451b676afd319469434d89d3fe040716676d0 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 5 Apr 2023 10:26:43 +0200 Subject: [PATCH] Capture copilot behavior in a editor unit test --- crates/copilot/Cargo.toml | 12 ++ crates/copilot/src/copilot.rs | 39 ++-- crates/copilot/src/request.rs | 2 +- crates/editor/Cargo.toml | 2 + crates/editor/src/editor.rs | 3 +- crates/editor/src/editor_tests.rs | 334 +++++++++++++++++++++++------- 6 files changed, 299 insertions(+), 93 deletions(-) diff --git a/crates/copilot/Cargo.toml b/crates/copilot/Cargo.toml index 52b9c223bd..1c37b6e298 100644 --- a/crates/copilot/Cargo.toml +++ b/crates/copilot/Cargo.toml @@ -8,6 +8,17 @@ publish = false path = "src/copilot.rs" doctest = false +[features] +test-support = [ + "client/test-support", + "collections/test-support", + "gpui/test-support", + "language/test-support", + "lsp/test-support", + "settings/test-support", + "util/test-support", +] + [dependencies] collections = { path = "../collections" } context_menu = { path = "../context_menu" } @@ -30,6 +41,7 @@ smol = "1.2.5" futures = "0.3" [dev-dependencies] +collections = { path = "../collections", features = ["test-support"] } gpui = { path = "../gpui", features = ["test-support"] } language = { path = "../language", features = ["test-support"] } settings = { path = "../settings", features = ["test-support"] } diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index bcf3188482..c0c5afc068 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -1,4 +1,4 @@ -mod request; +pub mod request; mod sign_in; use anyhow::{anyhow, Context, Result}; @@ -125,12 +125,8 @@ enum CopilotServer { #[derive(Clone, Debug)] enum SignInStatus { - Authorized { - _user: String, - }, - Unauthorized { - _user: String, - }, + Authorized, + Unauthorized, SigningIn { prompt: Option, task: Shared>>>, @@ -238,6 +234,23 @@ impl Copilot { } } + #[cfg(any(test, feature = "test-support"))] + pub fn fake(cx: &mut gpui::TestAppContext) -> (ModelHandle, lsp::FakeLanguageServer) { + let (server, fake_server) = + LanguageServer::fake("copilot".into(), Default::default(), cx.to_async()); + let http = util::http::FakeHttpClient::create(|_| async { unreachable!() }); + let this = cx.add_model(|cx| Self { + http: http.clone(), + node_runtime: NodeRuntime::new(http, cx.background().clone()), + server: CopilotServer::Started { + server: Arc::new(server), + status: SignInStatus::Authorized, + subscriptions_by_buffer_id: Default::default(), + }, + }); + (this, fake_server) + } + fn start_language_server( http: Arc, node_runtime: Arc, @@ -617,14 +630,10 @@ impl Copilot { ) { if let CopilotServer::Started { status, .. } = &mut self.server { *status = match lsp_status { - request::SignInStatus::Ok { user } - | request::SignInStatus::MaybeOk { user } - | request::SignInStatus::AlreadySignedIn { user } => { - SignInStatus::Authorized { _user: user } - } - request::SignInStatus::NotAuthorized { user } => { - SignInStatus::Unauthorized { _user: user } - } + request::SignInStatus::Ok { .. } + | request::SignInStatus::MaybeOk { .. } + | request::SignInStatus::AlreadySignedIn { .. } => SignInStatus::Authorized, + request::SignInStatus::NotAuthorized { .. } => SignInStatus::Unauthorized, request::SignInStatus::NotSignedIn => SignInStatus::SignedOut, }; cx.notify(); diff --git a/crates/copilot/src/request.rs b/crates/copilot/src/request.rs index e946c8f5cc..415f160ea3 100644 --- a/crates/copilot/src/request.rs +++ b/crates/copilot/src/request.rs @@ -117,7 +117,7 @@ pub struct GetCompletionsResult { pub completions: Vec, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Completion { pub text: String, diff --git a/crates/editor/Cargo.toml b/crates/editor/Cargo.toml index ef2489d7ec..82b7082576 100644 --- a/crates/editor/Cargo.toml +++ b/crates/editor/Cargo.toml @@ -11,6 +11,7 @@ doctest = false [features] test-support = [ "rand", + "copilot/test-support", "text/test-support", "language/test-support", "gpui/test-support", @@ -65,6 +66,7 @@ tree-sitter-javascript = { version = "*", optional = true } tree-sitter-typescript = { git = "https://github.com/tree-sitter/tree-sitter-typescript", rev = "5d20856f34315b068c41edaee2ac8a100081d259", optional = true } [dev-dependencies] +copilot = { path = "../copilot", features = ["test-support"] } text = { path = "../text", features = ["test-support"] } language = { path = "../language", features = ["test-support"] } lsp = { path = "../lsp", features = ["test-support"] } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 84d18bafe8..dd32d7420c 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -95,6 +95,7 @@ const CURSOR_BLINK_INTERVAL: Duration = Duration::from_millis(500); const MAX_LINE_LEN: usize = 1024; const MIN_NAVIGATION_HISTORY_ROW_DELTA: i64 = 10; const MAX_SELECTION_HISTORY_LEN: usize = 1024; +const COPILOT_DEBOUNCE_TIMEOUT: Duration = Duration::from_millis(75); pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); @@ -2799,7 +2800,7 @@ impl Editor { let (buffer, buffer_position) = self.buffer.read(cx).text_anchor_for_position(cursor, cx)?; self.copilot_state.pending_refresh = cx.spawn_weak(|this, mut cx| async move { - cx.background().timer(Duration::from_millis(75)).await; + cx.background().timer(COPILOT_DEBOUNCE_TIMEOUT).await; let (completion, completions_cycling) = copilot.update(&mut cx, |copilot, cx| { ( copilot.completions(&buffer, buffer_position, cx), diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 292c24f79a..7f7f7ec7ed 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -16,7 +16,7 @@ use language::{BracketPairConfig, FakeLspAdapter, LanguageConfig, LanguageRegist use parking_lot::Mutex; use project::FakeFs; use settings::EditorSettings; -use std::{cell::RefCell, rc::Rc, time::Instant}; +use std::{cell::RefCell, future::Future, rc::Rc, time::Instant}; use unindent::Unindent; use util::{ assert_set_eq, @@ -4585,81 +4585,6 @@ async fn test_completion(cx: &mut gpui::TestAppContext) { cx.assert_editor_state("editor.closeˇ"); handle_resolve_completion_request(&mut cx, None).await; apply_additional_edits.await.unwrap(); - - // Handle completion request passing a marked string specifying where the completion - // should be triggered from using '|' character, what range should be replaced, and what completions - // should be returned using '<' and '>' to delimit the range - async fn handle_completion_request<'a>( - cx: &mut EditorLspTestContext<'a>, - marked_string: &str, - completions: Vec<&'static str>, - ) { - let complete_from_marker: TextRangeMarker = '|'.into(); - let replace_range_marker: TextRangeMarker = ('<', '>').into(); - let (_, mut marked_ranges) = marked_text_ranges_by( - marked_string, - vec![complete_from_marker.clone(), replace_range_marker.clone()], - ); - - let complete_from_position = - cx.to_lsp(marked_ranges.remove(&complete_from_marker).unwrap()[0].start); - let replace_range = - cx.to_lsp_range(marked_ranges.remove(&replace_range_marker).unwrap()[0].clone()); - - cx.handle_request::(move |url, params, _| { - let completions = completions.clone(); - async move { - assert_eq!(params.text_document_position.text_document.uri, url.clone()); - assert_eq!( - params.text_document_position.position, - complete_from_position - ); - Ok(Some(lsp::CompletionResponse::Array( - completions - .iter() - .map(|completion_text| lsp::CompletionItem { - label: completion_text.to_string(), - text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { - range: replace_range, - new_text: completion_text.to_string(), - })), - ..Default::default() - }) - .collect(), - ))) - } - }) - .next() - .await; - } - - async fn handle_resolve_completion_request<'a>( - cx: &mut EditorLspTestContext<'a>, - edits: Option>, - ) { - let edits = edits.map(|edits| { - edits - .iter() - .map(|(marked_string, new_text)| { - let (_, marked_ranges) = marked_text_ranges(marked_string, false); - let replace_range = cx.to_lsp_range(marked_ranges[0].clone()); - lsp::TextEdit::new(replace_range, new_text.to_string()) - }) - .collect::>() - }); - - cx.handle_request::(move |_, _, _| { - let edits = edits.clone(); - async move { - Ok(lsp::CompletionItem { - additional_text_edits: edits, - ..Default::default() - }) - } - }) - .next() - .await; - } } #[gpui::test] @@ -5956,6 +5881,160 @@ async fn test_move_to_enclosing_bracket(cx: &mut gpui::TestAppContext) { ); } +#[gpui::test] +async fn test_copilot(deterministic: Arc, cx: &mut gpui::TestAppContext) { + let (copilot, copilot_lsp) = Copilot::fake(cx); + cx.update(|cx| cx.set_global(copilot)); + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string(), ":".to_string()]), + ..Default::default() + }), + ..Default::default() + }, + cx, + ) + .await; + + cx.set_state(indoc! {" + oneˇ + two + three + "}); + + // When inserting, ensure autocompletion is favored over Copilot suggestions. + cx.simulate_keystroke("."); + let _ = handle_completion_request( + &mut cx, + indoc! {" + one.|<> + two + three + "}, + vec!["completion_a", "completion_b"], + ); + handle_copilot_completion_request( + &copilot_lsp, + vec![copilot::request::Completion { + text: "copilot1".into(), + position: lsp::Position::new(0, 5), + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 5)), + ..Default::default() + }], + vec![], + ); + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(editor.context_menu_visible()); + assert!(!editor.has_active_copilot_suggestion(cx)); + + // Confirming a completion inserts it and hides the context menu, without showing + // the copilot suggestion afterwards. + editor + .confirm_completion(&Default::default(), cx) + .unwrap() + .detach(); + assert!(!editor.context_menu_visible()); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.text(cx), "one.completion_a\ntwo\nthree\n"); + assert_eq!(editor.display_text(cx), "one.completion_a\ntwo\nthree\n"); + }); + + cx.set_state(indoc! {" + oneˇ + two + three + "}); + + // When inserting, ensure autocompletion is favored over Copilot suggestions. + cx.simulate_keystroke("."); + let _ = handle_completion_request( + &mut cx, + indoc! {" + one.|<> + two + three + "}, + vec!["completion_a", "completion_b"], + ); + handle_copilot_completion_request( + &copilot_lsp, + vec![copilot::request::Completion { + text: "one.copilot1".into(), + position: lsp::Position::new(0, 4), + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 4)), + ..Default::default() + }], + vec![], + ); + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(editor.context_menu_visible()); + assert!(!editor.has_active_copilot_suggestion(cx)); + + // When hiding the context menu, the Copilot suggestion becomes visible. + editor.hide_context_menu(cx); + assert!(!editor.context_menu_visible()); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot1\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.\ntwo\nthree\n"); + }); + + // Ensure existing completion is interpolated when inserting again. + cx.simulate_keystroke("c"); + deterministic.run_until_parked(); + cx.update_editor(|editor, cx| { + assert!(!editor.context_menu_visible()); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot1\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + }); + + // After debouncing, new Copilot completions should be requested. + handle_copilot_completion_request( + &copilot_lsp, + vec![copilot::request::Completion { + text: "one.copilot2".into(), + position: lsp::Position::new(0, 5), + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 5)), + ..Default::default() + }], + vec![], + ); + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + cx.update_editor(|editor, cx| { + assert!(!editor.context_menu_visible()); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + + // Canceling should remove the active Copilot suggestion. + editor.cancel(&Default::default(), cx); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.c\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + + // After canceling, tabbing shouldn't insert the previously shown suggestion. + editor.tab(&Default::default(), cx); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.c \ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c \ntwo\nthree\n"); + + // When undoing the previously active suggestion is shown again. + editor.undo(&Default::default(), cx); + assert!(editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); + + // Tabbing when there is an active suggestion inserts it. + editor.tab(&Default::default(), cx); + assert!(!editor.has_active_copilot_suggestion(cx)); + assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); + assert_eq!(editor.text(cx), "one.copilot2\ntwo\nthree\n"); + }); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(row as u32, column as u32); point..point @@ -5971,3 +6050,106 @@ fn assert_selection_ranges(marked_text: &str, view: &mut Editor, cx: &mut ViewCo marked_text ); } + +/// Handle completion request passing a marked string specifying where the completion +/// should be triggered from using '|' character, what range should be replaced, and what completions +/// should be returned using '<' and '>' to delimit the range +fn handle_completion_request<'a>( + cx: &mut EditorLspTestContext<'a>, + marked_string: &str, + completions: Vec<&'static str>, +) -> impl Future { + let complete_from_marker: TextRangeMarker = '|'.into(); + let replace_range_marker: TextRangeMarker = ('<', '>').into(); + let (_, mut marked_ranges) = marked_text_ranges_by( + marked_string, + vec![complete_from_marker.clone(), replace_range_marker.clone()], + ); + + let complete_from_position = + cx.to_lsp(marked_ranges.remove(&complete_from_marker).unwrap()[0].start); + let replace_range = + cx.to_lsp_range(marked_ranges.remove(&replace_range_marker).unwrap()[0].clone()); + + let mut request = cx.handle_request::(move |url, params, _| { + let completions = completions.clone(); + async move { + assert_eq!(params.text_document_position.text_document.uri, url.clone()); + assert_eq!( + params.text_document_position.position, + complete_from_position + ); + Ok(Some(lsp::CompletionResponse::Array( + completions + .iter() + .map(|completion_text| lsp::CompletionItem { + label: completion_text.to_string(), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: replace_range, + new_text: completion_text.to_string(), + })), + ..Default::default() + }) + .collect(), + ))) + } + }); + + async move { + request.next().await; + } +} + +fn handle_resolve_completion_request<'a>( + cx: &mut EditorLspTestContext<'a>, + edits: Option>, +) -> impl Future { + let edits = edits.map(|edits| { + edits + .iter() + .map(|(marked_string, new_text)| { + let (_, marked_ranges) = marked_text_ranges(marked_string, false); + let replace_range = cx.to_lsp_range(marked_ranges[0].clone()); + lsp::TextEdit::new(replace_range, new_text.to_string()) + }) + .collect::>() + }); + + let mut request = + cx.handle_request::(move |_, _, _| { + let edits = edits.clone(); + async move { + Ok(lsp::CompletionItem { + additional_text_edits: edits, + ..Default::default() + }) + } + }); + + async move { + request.next().await; + } +} + +fn handle_copilot_completion_request( + lsp: &lsp::FakeLanguageServer, + completions: Vec, + completions_cycling: Vec, +) { + lsp.handle_request::(move |_params, _cx| { + let completions = completions.clone(); + async move { + Ok(copilot::request::GetCompletionsResult { + completions: completions.clone(), + }) + } + }); + lsp.handle_request::(move |_params, _cx| { + let completions_cycling = completions_cycling.clone(); + async move { + Ok(copilot::request::GetCompletionsResult { + completions: completions_cycling.clone(), + }) + } + }); +}