Resolve tests per file instead of per crate in test explorer

This commit is contained in:
hkalbasi 2024-03-29 05:34:43 +03:30
parent ad51a17c62
commit beec6914c8
8 changed files with 143 additions and 41 deletions

View File

@ -353,6 +353,10 @@ pub fn discover_tests_in_crate(&self, crate_id: CrateId) -> Cancellable<Vec<Test
self.with_db(|db| test_explorer::discover_tests_in_crate(db, crate_id)) self.with_db(|db| test_explorer::discover_tests_in_crate(db, crate_id))
} }
pub fn discover_tests_in_file(&self, file_id: FileId) -> Cancellable<Vec<TestItem>> {
self.with_db(|db| test_explorer::discover_tests_in_file(db, file_id))
}
/// Renders the crate graph to GraphViz "dot" syntax. /// Renders the crate graph to GraphViz "dot" syntax.
pub fn view_crate_graph(&self, full: bool) -> Cancellable<Result<String, String>> { pub fn view_crate_graph(&self, full: bool) -> Cancellable<Result<String, String>> {
self.with_db(|db| view_crate_graph::view_crate_graph(db, full)) self.with_db(|db| view_crate_graph::view_crate_graph(db, full))

View File

@ -7,7 +7,7 @@
}; };
use syntax::TextRange; use syntax::TextRange;
use crate::{navigation_target::ToNav, runnables::runnable_fn, Runnable, TryToNav}; use crate::{runnables::runnable_fn, NavigationTarget, Runnable, TryToNav};
#[derive(Debug)] #[derive(Debug)]
pub enum TestItemKind { pub enum TestItemKind {
@ -56,7 +56,12 @@ fn find_crate_by_id(crate_graph: &CrateGraph, crate_id: &str) -> Option<CrateId>
}) })
} }
fn discover_tests_in_module(db: &RootDatabase, module: Module, prefix_id: String) -> Vec<TestItem> { fn discover_tests_in_module(
db: &RootDatabase,
module: Module,
prefix_id: String,
only_in_this_file: bool,
) -> Vec<TestItem> {
let sema = Semantics::new(db); let sema = Semantics::new(db);
let mut r = vec![]; let mut r = vec![];
@ -64,9 +69,9 @@ fn discover_tests_in_module(db: &RootDatabase, module: Module, prefix_id: String
let module_name = let module_name =
c.name(db).as_ref().and_then(|n| n.as_str()).unwrap_or("[mod without name]").to_owned(); c.name(db).as_ref().and_then(|n| n.as_str()).unwrap_or("[mod without name]").to_owned();
let module_id = format!("{prefix_id}::{module_name}"); let module_id = format!("{prefix_id}::{module_name}");
let module_children = discover_tests_in_module(db, c, module_id.clone()); let module_children = discover_tests_in_module(db, c, module_id.clone(), only_in_this_file);
if !module_children.is_empty() { if !module_children.is_empty() {
let nav = c.to_nav(db).call_site; let nav = NavigationTarget::from_module_to_decl(sema.db, c).call_site;
r.push(TestItem { r.push(TestItem {
id: module_id, id: module_id,
kind: TestItemKind::Module, kind: TestItemKind::Module,
@ -76,7 +81,9 @@ fn discover_tests_in_module(db: &RootDatabase, module: Module, prefix_id: String
text_range: Some(nav.focus_or_full_range()), text_range: Some(nav.focus_or_full_range()),
runnable: None, runnable: None,
}); });
r.extend(module_children); if !only_in_this_file || c.is_inline(db) {
r.extend(module_children);
}
} }
} }
for def in module.declarations(db) { for def in module.declarations(db) {
@ -112,6 +119,55 @@ pub(crate) fn discover_tests_in_crate_by_test_id(
discover_tests_in_crate(db, crate_id) discover_tests_in_crate(db, crate_id)
} }
pub(crate) fn discover_tests_in_file(db: &RootDatabase, file_id: FileId) -> Vec<TestItem> {
let sema = Semantics::new(db);
let Some(module) = sema.file_to_module_def(file_id) else { return vec![] };
let Some((mut tests, id)) = find_module_id_and_test_parents(&sema, module) else {
return vec![];
};
tests.extend(discover_tests_in_module(db, module, id, true));
tests
}
fn find_module_id_and_test_parents(
sema: &Semantics<'_, RootDatabase>,
module: Module,
) -> Option<(Vec<TestItem>, String)> {
let Some(parent) = module.parent(sema.db) else {
let name = module.krate().display_name(sema.db)?.to_string();
return Some((
vec![TestItem {
id: name.clone(),
kind: TestItemKind::Crate(module.krate().into()),
label: name.clone(),
parent: None,
file: None,
text_range: None,
runnable: None,
}],
name,
));
};
let (mut r, mut id) = find_module_id_and_test_parents(sema, parent)?;
let parent = Some(id.clone());
id += "::";
let module_name = &module.name(sema.db);
let module_name = module_name.as_ref().and_then(|n| n.as_str()).unwrap_or("[mod without name]");
id += module_name;
let nav = NavigationTarget::from_module_to_decl(sema.db, module).call_site;
r.push(TestItem {
id: id.clone(),
kind: TestItemKind::Module,
label: module_name.to_owned(),
parent,
file: Some(nav.file_id),
text_range: Some(nav.focus_or_full_range()),
runnable: None,
});
Some((r, id))
}
pub(crate) fn discover_tests_in_crate(db: &RootDatabase, crate_id: CrateId) -> Vec<TestItem> { pub(crate) fn discover_tests_in_crate(db: &RootDatabase, crate_id: CrateId) -> Vec<TestItem> {
let crate_graph = db.crate_graph(); let crate_graph = db.crate_graph();
if !crate_graph[crate_id].origin.is_local() { if !crate_graph[crate_id].origin.is_local() {
@ -133,6 +189,6 @@ pub(crate) fn discover_tests_in_crate(db: &RootDatabase, crate_id: CrateId) -> V
text_range: None, text_range: None,
runnable: None, runnable: None,
}]; }];
r.extend(discover_tests_in_module(db, module, crate_test_id)); r.extend(discover_tests_in_module(db, module, crate_test_id, false));
r r
} }

View File

@ -238,9 +238,12 @@ pub(crate) fn handle_discover_test(
let (tests, scope) = match params.test_id { let (tests, scope) = match params.test_id {
Some(id) => { Some(id) => {
let crate_id = id.split_once("::").map(|it| it.0).unwrap_or(&id); let crate_id = id.split_once("::").map(|it| it.0).unwrap_or(&id);
(snap.analysis.discover_tests_in_crate_by_test_id(crate_id)?, vec![crate_id.to_owned()]) (
snap.analysis.discover_tests_in_crate_by_test_id(crate_id)?,
Some(vec![crate_id.to_owned()]),
)
} }
None => (snap.analysis.discover_test_roots()?, vec![]), None => (snap.analysis.discover_test_roots()?, None),
}; };
for t in &tests { for t in &tests {
hack_recover_crate_name::insert_name(t.id.clone()); hack_recover_crate_name::insert_name(t.id.clone());
@ -254,6 +257,7 @@ pub(crate) fn handle_discover_test(
}) })
.collect(), .collect(),
scope, scope,
scope_file: None,
}) })
} }

View File

@ -194,7 +194,8 @@ pub struct TestItem {
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
pub struct DiscoverTestResults { pub struct DiscoverTestResults {
pub tests: Vec<TestItem>, pub tests: Vec<TestItem>,
pub scope: Vec<String>, pub scope: Option<Vec<String>>,
pub scope_file: Option<Vec<TextDocumentIdentifier>>,
} }
pub enum DiscoverTest {} pub enum DiscoverTest {}

View File

@ -9,9 +9,8 @@
use always_assert::always; use always_assert::always;
use crossbeam_channel::{never, select, Receiver}; use crossbeam_channel::{never, select, Receiver};
use ide_db::base_db::{SourceDatabase, SourceDatabaseExt, VfsPath}; use ide_db::base_db::{SourceDatabase, SourceDatabaseExt, VfsPath};
use itertools::Itertools;
use lsp_server::{Connection, Notification, Request}; use lsp_server::{Connection, Notification, Request};
use lsp_types::notification::Notification as _; use lsp_types::{notification::Notification as _, TextDocumentIdentifier};
use stdx::thread::ThreadIntent; use stdx::thread::ThreadIntent;
use vfs::FileId; use vfs::FileId;
@ -533,22 +532,14 @@ fn update_tests(&mut self) {
let snapshot = self.snapshot(); let snapshot = self.snapshot();
move || { move || {
let tests = subscriptions let tests = subscriptions
.into_iter() .iter()
.filter_map(|f| snapshot.analysis.crates_for(f).ok()) .copied()
.flatten() .filter_map(|f| snapshot.analysis.discover_tests_in_file(f).ok())
.unique()
.filter_map(|c| snapshot.analysis.discover_tests_in_crate(c).ok())
.flatten() .flatten()
.collect::<Vec<_>>(); .collect::<Vec<_>>();
for t in &tests { for t in &tests {
hack_recover_crate_name::insert_name(t.id.clone()); hack_recover_crate_name::insert_name(t.id.clone());
} }
let scope = tests
.iter()
.filter_map(|t| Some(t.id.split_once("::")?.0))
.unique()
.map(|it| it.to_owned())
.collect();
Task::DiscoverTest(lsp_ext::DiscoverTestResults { Task::DiscoverTest(lsp_ext::DiscoverTestResults {
tests: tests tests: tests
.into_iter() .into_iter()
@ -557,7 +548,13 @@ fn update_tests(&mut self) {
to_proto::test_item(&snapshot, t, line_index.as_ref()) to_proto::test_item(&snapshot, t, line_index.as_ref())
}) })
.collect(), .collect(),
scope, scope: None,
scope_file: Some(
subscriptions
.into_iter()
.map(|f| TextDocumentIdentifier { uri: to_proto::url(&snapshot, f) })
.collect(),
),
}) })
} }
}); });

View File

@ -1,5 +1,5 @@
<!--- <!---
lsp/ext.rs hash: d5febcbf63650753 lsp/ext.rs hash: 223f48a89a5126a0
If you need to change the above hash to make the test pass, please check if you If you need to change the above hash to make the test pass, please check if you
need to adjust this doc as well and ping this issue: need to adjust this doc as well and ping this issue:
@ -440,7 +440,11 @@ interface DiscoverTestResults {
// For each test which its id is in this list, the response // For each test which its id is in this list, the response
// contains all tests that are children of this test, and // contains all tests that are children of this test, and
// client should remove old tests not included in the response. // client should remove old tests not included in the response.
scope: string[]; scope: string[] | undefined;
// For each file which its uri is in this list, the response
// contains all tests that are located in this file, and
// client should remove old tests not included in the response.
scopeFile: lc.TextDocumentIdentifier[] | undefined;
} }
``` ```

View File

@ -83,7 +83,11 @@ export type TestItem = {
range?: lc.Range | undefined; range?: lc.Range | undefined;
runnable?: Runnable | undefined; runnable?: Runnable | undefined;
}; };
export type DiscoverTestResults = { tests: TestItem[]; scope: string[] }; export type DiscoverTestResults = {
tests: TestItem[];
scope: string[] | undefined;
scopeFile: lc.TextDocumentIdentifier[] | undefined;
};
export type TestState = export type TestState =
| { tag: "failed"; message: string } | { tag: "failed"; message: string }
| { tag: "passed" } | { tag: "passed" }

View File

@ -12,6 +12,7 @@ export const prepareTestExplorer = (
) => { ) => {
let currentTestRun: vscode.TestRun | undefined; let currentTestRun: vscode.TestRun | undefined;
let idToTestMap: Map<string, vscode.TestItem> = new Map(); let idToTestMap: Map<string, vscode.TestItem> = new Map();
const fileToTestMap: Map<string, vscode.TestItem[]> = new Map();
const idToRunnableMap: Map<string, ra.Runnable> = new Map(); const idToRunnableMap: Map<string, ra.Runnable> = new Map();
testController.createRunProfile( testController.createRunProfile(
@ -59,6 +60,18 @@ export const prepareTestExplorer = (
false, false,
); );
const deleteTest = (item: vscode.TestItem, parentList: vscode.TestItemCollection) => {
parentList.delete(item.id);
idToTestMap.delete(item.id);
idToRunnableMap.delete(item.id);
if (item.uri) {
fileToTestMap.set(
item.uri.toString(),
fileToTestMap.get(item.uri.toString())!.filter((t) => t.id !== item.id),
);
}
};
const addTest = (item: ra.TestItem) => { const addTest = (item: ra.TestItem) => {
const parentList = item.parent const parentList = item.parent
? idToTestMap.get(item.parent)!.children ? idToTestMap.get(item.parent)!.children
@ -76,7 +89,7 @@ export const prepareTestExplorer = (
oldTest.range = range; oldTest.range = range;
return; return;
} }
parentList.delete(item.id); deleteTest(oldTest, parentList);
} }
const iconToVscodeMap = { const iconToVscodeMap = {
package: "package", package: "package",
@ -91,6 +104,12 @@ export const prepareTestExplorer = (
test.range = range; test.range = range;
test.canResolveChildren = item.canResolveChildren; test.canResolveChildren = item.canResolveChildren;
idToTestMap.set(item.id, test); idToTestMap.set(item.id, test);
if (uri) {
if (!fileToTestMap.has(uri.toString())) {
fileToTestMap.set(uri.toString(), []);
}
fileToTestMap.get(uri.toString())!.push(test);
}
if (item.runnable) { if (item.runnable) {
idToRunnableMap.set(item.id, item.runnable); idToRunnableMap.set(item.id, item.runnable);
} }
@ -98,7 +117,7 @@ export const prepareTestExplorer = (
}; };
const addTestGroup = (testsAndScope: ra.DiscoverTestResults) => { const addTestGroup = (testsAndScope: ra.DiscoverTestResults) => {
const { tests, scope } = testsAndScope; const { tests, scope, scopeFile } = testsAndScope;
const testSet: Set<string> = new Set(); const testSet: Set<string> = new Set();
for (const test of tests) { for (const test of tests) {
addTest(test); addTest(test);
@ -107,25 +126,38 @@ export const prepareTestExplorer = (
// FIXME(hack_recover_crate_name): We eagerly resolve every test if we got a lazy top level response (detected // FIXME(hack_recover_crate_name): We eagerly resolve every test if we got a lazy top level response (detected
// by checking that `scope` is empty). This is not a good thing and wastes cpu and memory unnecessarily, so we // by checking that `scope` is empty). This is not a good thing and wastes cpu and memory unnecessarily, so we
// should remove it. // should remove it.
if (scope.length === 0) { if (!scope && !scopeFile) {
for (const test of tests) { for (const test of tests) {
void testController.resolveHandler!(idToTestMap.get(test.id)); void testController.resolveHandler!(idToTestMap.get(test.id));
} }
} }
if (!scope) { if (scope) {
return; const recursivelyRemove = (tests: vscode.TestItemCollection) => {
} for (const [_, test] of tests) {
const recursivelyRemove = (tests: vscode.TestItemCollection) => { if (!testSet.has(test.id)) {
for (const [testId, _] of tests) { deleteTest(test, tests);
if (!testSet.has(testId)) { } else {
tests.delete(testId); recursivelyRemove(test.children);
} else { }
recursivelyRemove(tests.get(testId)!.children);
} }
};
for (const root of scope) {
recursivelyRemove(idToTestMap.get(root)!.children);
}
}
if (scopeFile) {
const removeByFile = (file: vscode.Uri) => {
const testsToBeRemoved = (fileToTestMap.get(file.toString()) || []).filter(
(t) => !testSet.has(t.id),
);
for (const test of testsToBeRemoved) {
const parentList = test.parent?.children || testController.items;
deleteTest(test, parentList);
}
};
for (const file of scopeFile) {
removeByFile(vscode.Uri.parse(file.uri));
} }
};
for (const root of scope) {
recursivelyRemove(idToTestMap.get(root)!.children);
} }
}; };