From d592cf07d6e54d465002d1209d1b43ce2eb49d9e Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Wed, 30 Aug 2023 16:31:31 +0100 Subject: [PATCH] refactor(lsp): store test definitions in adjacency list (#20330) Previously: ```rust pub struct TestDefinition { pub id: String, pub name: String, pub range: SourceRange, pub steps: Vec, } pub struct TestDefinitions { pub discovered: Vec, pub injected: Vec, pub script_version: String, } ``` Now: ```rust pub struct TestDefinition { pub id: String, pub name: String, pub range: Option, pub is_dynamic: bool, // True for 'injected' module, not statically detected but added at runtime. pub parent_id: Option, pub step_ids: HashSet, } pub struct TestModule { pub specifier: ModuleSpecifier, pub script_version: String, pub defs: HashMap, } ``` Storing the test tree as a literal tree diminishes the value of IDs, even though vscode stores them that way. This makes all data easily accessible from `TestModule`. It unifies the interface between 'discovered' and 'injected' tests. This unblocks some enhancements wrt syncing tests between the LSP and extension, such as this TODO: https://github.com/denoland/vscode_deno/blob/61f08d5a71536a0a5f7dce965955b09e6bd957e1/client/src/testing.ts#L251-L259 and https://github.com/denoland/vscode_deno/issues/900. We should also get more flexibility overall. `TestCollector` is cleaned up, now stores a `&mut TestModule` directly and registers tests as it comes across them with `TestModule::register()`. This method ensures sanity in the redundant data from having both of `TestDefinition::{parent_id,step_ids}`. All of the messy conversions between `TestDescription`, `LspTestDescription`, `TestDefinition`, `TestData` and `TestIdentifier` are cleaned up. They shouldn't have been using `impl From` and now the full list of tests is available to their implementations. --- cli/lsp/testing/collectors.rs | 891 +++++++++++++++++++++------------ cli/lsp/testing/definitions.rs | 271 +++++----- cli/lsp/testing/execution.rs | 299 +++++------ cli/lsp/testing/server.rs | 30 +- cli/tools/test/mod.rs | 7 - 5 files changed, 836 insertions(+), 662 deletions(-) diff --git a/cli/lsp/testing/collectors.rs b/cli/lsp/testing/collectors.rs index 6976cffb95..f5f1ea0d1c 100644 --- a/cli/lsp/testing/collectors.rs +++ b/cli/lsp/testing/collectors.rs @@ -1,51 +1,59 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -use super::definitions::TestDefinition; +use crate::lsp::analysis::source_range_to_lsp_range; + +use super::definitions::TestModule; use deno_ast::swc::ast; use deno_ast::swc::visit::Visit; use deno_ast::swc::visit::VisitWith; -use deno_ast::SourceRange; use deno_ast::SourceRangedForSpanned; use deno_ast::SourceTextInfo; use deno_core::ModuleSpecifier; +use lsp::Range; use std::collections::HashMap; use std::collections::HashSet; +use tower_lsp::lsp_types as lsp; /// Parse an arrow expression for any test steps and return them. -fn arrow_to_steps( - parent: &str, +fn visit_arrow( arrow_expr: &ast::ArrowExpr, -) -> Vec { + parent_id: &str, + text_info: &SourceTextInfo, + test_module: &mut TestModule, +) { if let Some((maybe_test_context, maybe_step_var)) = parse_test_context_param(arrow_expr.params.get(0)) { let mut collector = TestStepCollector::new( - parent.to_string(), maybe_test_context, maybe_step_var, + parent_id, + text_info, + test_module, ); arrow_expr.body.visit_with(&mut collector); - collector.take() - } else { - vec![] } } /// Parse a function for any test steps and return them. -fn fn_to_steps(parent: &str, function: &ast::Function) -> Vec { +fn visit_fn( + function: &ast::Function, + parent_id: &str, + text_info: &SourceTextInfo, + test_module: &mut TestModule, +) { if let Some((maybe_test_context, maybe_step_var)) = parse_test_context_param(function.params.get(0).map(|p| &p.pat)) { let mut collector = TestStepCollector::new( - parent.to_string(), maybe_test_context, maybe_step_var, + parent_id, + text_info, + test_module, ); function.body.visit_with(&mut collector); - collector.take() - } else { - vec![] } } @@ -120,188 +128,223 @@ fn parse_test_context_param( /// Check a call expression of a test or test step to determine the name of the /// test or test step as well as any sub steps. -fn check_call_expr( - parent: &str, +fn visit_call_expr( node: &ast::CallExpr, fns: Option<&HashMap>, - text_info: Option<&SourceTextInfo>, -) -> Option<(String, Vec)> { + range: Range, + parent_id: Option<&str>, + text_info: &SourceTextInfo, + test_module: &mut TestModule, +) { if let Some(expr) = node.args.get(0).map(|es| es.expr.as_ref()) { match expr { ast::Expr::Object(obj_lit) => { let mut maybe_name = None; - let mut steps = vec![]; for prop in &obj_lit.props { - if let ast::PropOrSpread::Prop(prop) = prop { - match prop.as_ref() { - ast::Prop::KeyValue(key_value_prop) => { - if let ast::PropName::Ident(ast::Ident { sym, .. }) = - &key_value_prop.key - { - match sym.to_string().as_str() { - "name" => match key_value_prop.value.as_ref() { - // matches string literals (e.g. "test name" or - // 'test name') - ast::Expr::Lit(ast::Lit::Str(lit_str)) => { - maybe_name = Some(lit_str.value.to_string()); - } - // matches template literals with only a single quasis - // (e.g. `test name`) - ast::Expr::Tpl(tpl) => { - if tpl.quasis.len() == 1 { - maybe_name = Some(tpl.quasis[0].raw.to_string()); - } - } - _ => (), - }, - "fn" => match key_value_prop.value.as_ref() { - ast::Expr::Arrow(arrow_expr) => { - steps = arrow_to_steps(parent, arrow_expr); - } - ast::Expr::Fn(fn_expr) => { - steps = fn_to_steps(parent, &fn_expr.function); - } - _ => (), - }, - _ => (), - } + let ast::PropOrSpread::Prop(prop) = prop else { + continue; + }; + let ast::Prop::KeyValue(key_value_prop) = prop.as_ref() else { + continue; + }; + let ast::PropName::Ident(ast::Ident { sym, .. }) = + &key_value_prop.key + else { + continue; + }; + if sym == "name" { + match key_value_prop.value.as_ref() { + // matches string literals (e.g. "test name" or + // 'test name') + ast::Expr::Lit(ast::Lit::Str(lit_str)) => { + maybe_name = Some(lit_str.value.to_string()); + } + // matches template literals with only a single quasis + // (e.g. `test name`) + ast::Expr::Tpl(tpl) => { + if tpl.quasis.len() == 1 { + maybe_name = Some(tpl.quasis[0].raw.to_string()); } } - ast::Prop::Method(method_prop) => { - steps = fn_to_steps(parent, &method_prop.function); - } - _ => (), + _ => {} } + break; + } + } + let name = match maybe_name { + Some(n) => n, + None => return, + }; + let (id, _) = test_module.register( + name, + Some(range), + false, + parent_id.map(str::to_owned), + ); + for prop in &obj_lit.props { + let ast::PropOrSpread::Prop(prop) = prop else { + continue; + }; + match prop.as_ref() { + ast::Prop::KeyValue(key_value_prop) => { + let ast::PropName::Ident(ast::Ident { sym, .. }) = + &key_value_prop.key + else { + continue; + }; + if sym == "fn" { + match key_value_prop.value.as_ref() { + ast::Expr::Arrow(arrow_expr) => { + visit_arrow(arrow_expr, &id, text_info, test_module); + } + ast::Expr::Fn(fn_expr) => { + visit_fn(&fn_expr.function, &id, text_info, test_module); + } + _ => {} + } + break; + } + } + ast::Prop::Method(method_prop) => { + let ast::PropName::Ident(ast::Ident { sym, .. }) = + &method_prop.key + else { + continue; + }; + if sym == "fn" { + visit_fn(&method_prop.function, &id, text_info, test_module); + break; + } + } + _ => {} } } - maybe_name.map(|name| (name, steps)) } ast::Expr::Fn(fn_expr) => { if let Some(ast::Ident { sym, .. }) = fn_expr.ident.as_ref() { let name = sym.to_string(); - let steps = fn_to_steps(parent, &fn_expr.function); - Some((name, steps)) - } else { - None + let (id, _) = test_module.register( + name, + Some(range), + false, + parent_id.map(str::to_owned), + ); + visit_fn(&fn_expr.function, &id, text_info, test_module); } } ast::Expr::Lit(ast::Lit::Str(lit_str)) => { let name = lit_str.value.to_string(); - let mut steps = vec![]; + let (id, _) = test_module.register( + name, + Some(range), + false, + parent_id.map(str::to_owned), + ); match node.args.get(1).map(|es| es.expr.as_ref()) { Some(ast::Expr::Fn(fn_expr)) => { - steps = fn_to_steps(parent, &fn_expr.function); + visit_fn(&fn_expr.function, &id, text_info, test_module); } Some(ast::Expr::Arrow(arrow_expr)) => { - steps = arrow_to_steps(parent, arrow_expr); + visit_arrow(arrow_expr, &id, text_info, test_module); } - _ => (), + _ => {} } - Some((name, steps)) } ast::Expr::Tpl(tpl) => { if tpl.quasis.len() == 1 { - let mut steps = vec![]; + let name = tpl.quasis[0].raw.to_string(); + let (id, _) = test_module.register( + name, + Some(range), + false, + parent_id.map(str::to_owned), + ); match node.args.get(1).map(|es| es.expr.as_ref()) { Some(ast::Expr::Fn(fn_expr)) => { - steps = fn_to_steps(parent, &fn_expr.function); + visit_fn(&fn_expr.function, &id, text_info, test_module); } Some(ast::Expr::Arrow(arrow_expr)) => { - steps = arrow_to_steps(parent, arrow_expr); + visit_arrow(arrow_expr, &id, text_info, test_module); } - _ => (), + _ => {} } - - Some((tpl.quasis[0].raw.to_string(), steps)) - } else { - None } } ast::Expr::Ident(ident) => { let name = ident.sym.to_string(); - fns.and_then(|fns| { - fns - .get(&name) - .map(|fn_expr| (name, fn_to_steps(parent, fn_expr))) - }) + if let Some(fn_expr) = fns.and_then(|fns| fns.get(&name)) { + let (parent_id, _) = test_module.register( + name, + Some(range), + false, + parent_id.map(str::to_owned), + ); + visit_fn(fn_expr, &parent_id, text_info, test_module); + } } _ => { - if let Some(text_info) = text_info { - let range = node.range(); - let indexes = text_info.line_and_column_display(range.start); - Some(( + if parent_id.is_none() { + let node_range = node.range(); + let indexes = text_info.line_and_column_display(node_range.start); + test_module.register( format!("Test {}:{}", indexes.line_number, indexes.column_number), - vec![], - )) - } else { - None + Some(range), + false, + None, + ); } } } - } else { - None } } /// A structure which can be used to walk a branch of AST determining if the /// branch contains any testing steps. -struct TestStepCollector { - steps: Vec, - parent: String, +struct TestStepCollector<'a> { maybe_test_context: Option, vars: HashSet, + parent_id: &'a str, + text_info: &'a SourceTextInfo, + test_module: &'a mut TestModule, } -impl TestStepCollector { +impl<'a> TestStepCollector<'a> { fn new( - parent: String, maybe_test_context: Option, maybe_step_var: Option, + parent_id: &'a str, + text_info: &'a SourceTextInfo, + test_module: &'a mut TestModule, ) -> Self { let mut vars = HashSet::new(); if let Some(var) = maybe_step_var { vars.insert(var); } Self { - steps: Vec::default(), - parent, maybe_test_context, vars, + parent_id, + text_info, + test_module, } } - - fn add_step>( - &mut self, - name: N, - range: SourceRange, - steps: Vec, - ) { - let step = - TestDefinition::new_step(name.as_ref().to_string(), range, steps); - self.steps.push(step); - } - - fn check_call_expr(&mut self, node: &ast::CallExpr, range: SourceRange) { - if let Some((name, steps)) = check_call_expr(&self.parent, node, None, None) - { - self.add_step(name, range, steps); - } - } - - /// Move out the test definitions - pub fn take(self) -> Vec { - self.steps - } } -impl Visit for TestStepCollector { +impl Visit for TestStepCollector<'_> { fn visit_call_expr(&mut self, node: &ast::CallExpr) { if let ast::Callee::Expr(callee_expr) = &node.callee { match callee_expr.as_ref() { // Identify calls to identified variables ast::Expr::Ident(ident) => { if self.vars.contains(&ident.sym.to_string()) { - self.check_call_expr(node, ident.range()); + visit_call_expr( + node, + None, + source_range_to_lsp_range(&ident.range(), self.text_info), + Some(self.parent_id), + self.text_info, + self.test_module, + ); } } // Identify calls to `test.step()` @@ -311,7 +354,17 @@ impl Visit for TestStepCollector { if ns_prop_ident.sym.eq("step") { if let ast::Expr::Ident(ident) = member_expr.obj.as_ref() { if ident.sym == *test_context { - self.check_call_expr(node, ns_prop_ident.range()); + visit_call_expr( + node, + None, + source_range_to_lsp_range( + &ns_prop_ident.range(), + self.text_info, + ), + Some(self.parent_id), + self.text_info, + self.test_module, + ); } } } @@ -382,53 +435,29 @@ impl Visit for TestStepCollector { /// Walk an AST and determine if it contains any `Deno.test` tests. pub struct TestCollector { - definitions: Vec, - specifier: ModuleSpecifier, + test_module: TestModule, vars: HashSet, fns: HashMap, text_info: SourceTextInfo, } impl TestCollector { - pub fn new(specifier: ModuleSpecifier, text_info: SourceTextInfo) -> Self { + pub fn new( + specifier: ModuleSpecifier, + script_version: String, + text_info: SourceTextInfo, + ) -> Self { Self { - definitions: Vec::new(), - specifier, + test_module: TestModule::new(specifier, script_version), vars: HashSet::new(), fns: HashMap::new(), text_info, } } - fn add_definition>( - &mut self, - name: N, - range: SourceRange, - steps: Vec, - ) { - let definition = TestDefinition::new( - &self.specifier, - name.as_ref().to_string(), - range, - steps, - ); - self.definitions.push(definition); - } - - fn check_call_expr(&mut self, node: &ast::CallExpr, range: SourceRange) { - if let Some((name, steps)) = check_call_expr( - self.specifier.as_str(), - node, - Some(&self.fns), - Some(&self.text_info), - ) { - self.add_definition(name, range, steps); - } - } - /// Move out the test definitions - pub fn take(self) -> Vec { - self.definitions + pub fn take(self) -> TestModule { + self.test_module } } @@ -438,7 +467,14 @@ impl Visit for TestCollector { match callee_expr.as_ref() { ast::Expr::Ident(ident) => { if self.vars.contains(&ident.sym.to_string()) { - self.check_call_expr(node, ident.range()); + visit_call_expr( + node, + Some(&self.fns), + source_range_to_lsp_range(&ident.range(), &self.text_info), + None, + &self.text_info, + &mut self.test_module, + ); } } ast::Expr::Member(member_expr) => { @@ -446,7 +482,17 @@ impl Visit for TestCollector { if ns_prop_ident.sym.to_string() == "test" { if let ast::Expr::Ident(ident) = member_expr.obj.as_ref() { if ident.sym.to_string() == "Deno" { - self.check_call_expr(node, ns_prop_ident.range()); + visit_call_expr( + node, + Some(&self.fns), + source_range_to_lsp_range( + &ns_prop_ident.range(), + &self.text_info, + ), + None, + &self.text_info, + &mut self.test_module, + ); } } } @@ -519,18 +565,17 @@ impl Visit for TestCollector { #[cfg(test)] pub mod tests { - use super::*; - use deno_ast::StartSourcePos; - use deno_core::resolve_url; + use crate::lsp::testing::definitions::TestDefinition; - pub fn new_range(start: usize, end: usize) -> SourceRange { - SourceRange::new( - StartSourcePos::START_SOURCE_POS + start, - StartSourcePos::START_SOURCE_POS + end, - ) + use super::*; + use deno_core::resolve_url; + use lsp::Position; + + pub fn new_range(l1: u32, c1: u32, l2: u32, c2: u32) -> Range { + Range::new(Position::new(l1, c1), Position::new(l2, c2)) } - fn collect(source: &str) -> Vec { + fn collect(source: &str) -> TestModule { let specifier = resolve_url("file:///a/example.ts").unwrap(); let parsed_module = deno_ast::parse_module(deno_ast::ParseParams { @@ -543,54 +588,81 @@ pub mod tests { }) .unwrap(); let text_info = parsed_module.text_info().clone(); - let mut collector = TestCollector::new(specifier, text_info); + let mut collector = + TestCollector::new(specifier, "1".to_string(), text_info); parsed_module.module().visit_with(&mut collector); collector.take() } #[test] fn test_test_collector_test() { - let res = collect( + let test_module = collect( r#" Deno.test("test", () => {}); "#, ); assert_eq!( - res, - vec![TestDefinition { - id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" - .to_string(), - name: "test".to_string(), - range: new_range(12, 16), - steps: vec![], - },] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![( + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + TestDefinition { + id: + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + name: "test".to_string(), + range: Some(new_range(1, 11, 1, 15)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), + } + ),] + .into_iter() + .collect(), + } ); } #[test] fn test_test_collector_test_tpl() { - let res = collect( + let test_module = collect( r#" Deno.test(`test`, () => {}); "#, ); assert_eq!( - res, - vec![TestDefinition { - id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" - .to_string(), - name: "test".to_string(), - range: new_range(12, 16), - steps: vec![], - },] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![( + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + TestDefinition { + id: + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + name: "test".to_string(), + range: Some(new_range(1, 11, 1, 15)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), + } + ),] + .into_iter() + .collect(), + } ); } #[test] fn test_test_collector_a() { - let res = collect( + let test_module = collect( r#" Deno.test({ name: "test", @@ -607,34 +679,52 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" - .to_string(), - name: "test".to_string(), - range: new_range(12, 16), - steps: vec![TestDefinition { - id: - "704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c" - .to_string(), - name: "step".to_string(), - range: new_range(81, 85), - steps: vec![TestDefinition { - id: - "0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d" - .to_string(), - name: "sub step".to_string(), - range: new_range(128, 132), - steps: vec![], - }] - }], - },] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![ + ( + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string(), + TestDefinition { + id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string(), + name: "test".to_string(), + range: Some(new_range(1, 11, 1, 15)), + is_dynamic: false, + parent_id: None, + step_ids: vec!["704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string()].into_iter().collect(), + } + ), + ( + "704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string(), + TestDefinition { + id: "704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string(), + name: "step".to_string(), + range: Some(new_range(4, 18, 4, 22)), + is_dynamic: false, + parent_id: Some("4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string()), + step_ids: vec!["0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d".to_string()].into_iter().collect(), + } + ), + ( + "0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d".to_string(), + TestDefinition { + id: "0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d".to_string(), + name: "sub step".to_string(), + range: Some(new_range(5, 18, 5, 22)), + is_dynamic: false, + parent_id: Some("704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string()), + step_ids: Default::default(), + } + ), + ].into_iter().collect(), + } ); } #[test] fn test_test_collector_a_tpl() { - let res = collect( + let test_module = collect( r#" Deno.test({ name: `test`, @@ -651,34 +741,52 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" - .to_string(), - name: "test".to_string(), - range: new_range(12, 16), - steps: vec![TestDefinition { - id: - "704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c" - .to_string(), - name: "step".to_string(), - range: new_range(81, 85), - steps: vec![TestDefinition { - id: - "0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d" - .to_string(), - name: "sub step".to_string(), - range: new_range(128, 132), - steps: vec![], - }] - }], - },] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![ + ( + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string(), + TestDefinition { + id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string(), + name: "test".to_string(), + range: Some(new_range(1, 11, 1, 15)), + is_dynamic: false, + parent_id: None, + step_ids: vec!["704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string()].into_iter().collect(), + } + ), + ( + "704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string(), + TestDefinition { + id: "704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string(), + name: "step".to_string(), + range: Some(new_range(4, 18, 4, 22)), + is_dynamic: false, + parent_id: Some("4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string()), + step_ids: vec!["0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d".to_string()].into_iter().collect(), + } + ), + ( + "0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d".to_string(), + TestDefinition { + id: "0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d".to_string(), + name: "sub step".to_string(), + range: Some(new_range(5, 18, 5, 22)), + is_dynamic: false, + parent_id: Some("704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string()), + step_ids: Default::default(), + } + ), + ].into_iter().collect(), + } ); } #[test] fn test_test_collector_destructure() { - let res = collect( + let test_module = collect( r#" const { test } = Deno; test("test", () => {}); @@ -686,20 +794,33 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" - .to_string(), - name: "test".to_string(), - range: new_range(36, 40), - steps: vec![], - }] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![( + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + TestDefinition { + id: + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + name: "test".to_string(), + range: Some(new_range(2, 6, 2, 10)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), + } + ),] + .into_iter() + .collect(), + } ); } #[test] fn test_test_collector_destructure_rebind_step() { - let res = collect( + let test_module = collect( r#" Deno.test(async function useFnName({ step: s }) { await s("step", () => {}); @@ -708,27 +829,41 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "86b4c821900e38fc89f24bceb0e45193608ab3f9d2a6019c7b6a5aceff5d7df2" - .to_string(), - name: "useFnName".to_string(), - range: new_range(12, 16), - steps: vec![TestDefinition { - id: - "dac8a169b8f8c6babf11122557ea545de2733bfafed594d044b22bc6863a0856" - .to_string(), - name: "step".to_string(), - range: new_range(71, 72), - steps: vec![], - }], - }] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![ + ( + "86b4c821900e38fc89f24bceb0e45193608ab3f9d2a6019c7b6a5aceff5d7df2".to_string(), + TestDefinition { + id: "86b4c821900e38fc89f24bceb0e45193608ab3f9d2a6019c7b6a5aceff5d7df2".to_string(), + name: "useFnName".to_string(), + range: Some(new_range(1, 11, 1, 15)), + is_dynamic: false, + parent_id: None, + step_ids: vec!["dac8a169b8f8c6babf11122557ea545de2733bfafed594d044b22bc6863a0856".to_string()].into_iter().collect(), + } + ), + ( + "dac8a169b8f8c6babf11122557ea545de2733bfafed594d044b22bc6863a0856".to_string(), + TestDefinition { + id: "dac8a169b8f8c6babf11122557ea545de2733bfafed594d044b22bc6863a0856".to_string(), + name: "step".to_string(), + range: Some(new_range(2, 14, 2, 15)), + is_dynamic: false, + parent_id: Some("86b4c821900e38fc89f24bceb0e45193608ab3f9d2a6019c7b6a5aceff5d7df2".to_string()), + step_ids: Default::default(), + } + ), + ].into_iter().collect(), + } ); } #[test] fn test_test_collector_rebind() { - let res = collect( + let test_module = collect( r#" const t = Deno.test; t("test", () => {}); @@ -736,20 +871,33 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" - .to_string(), - name: "test".to_string(), - range: new_range(34, 35), - steps: vec![], - }] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![( + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + TestDefinition { + id: + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + name: "test".to_string(), + range: Some(new_range(2, 6, 2, 7)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), + } + ),] + .into_iter() + .collect(), + } ); } #[test] fn test_test_collector_separate_test_function_with_string_name() { - let res = collect( + let test_module = collect( r#" function someFunction() {} Deno.test("test", someFunction); @@ -757,40 +905,66 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" - .to_string(), - name: "test".to_string(), - range: new_range(45, 49), - steps: vec![], - }] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![( + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + TestDefinition { + id: + "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9" + .to_string(), + name: "test".to_string(), + range: Some(new_range(2, 11, 2, 15)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), + } + ),] + .into_iter() + .collect(), + } ); } #[test] fn test_test_collector_function_only() { - let res = collect( + let test_module = collect( r#" Deno.test(async function someFunction() {}); "#, ); assert_eq!( - res, - vec![TestDefinition { - id: "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568" - .to_string(), - name: "someFunction".to_string(), - range: new_range(12, 16), - steps: vec![] - }] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![( + "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568" + .to_string(), + TestDefinition { + id: + "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568" + .to_string(), + name: "someFunction".to_string(), + range: Some(new_range(1, 11, 1, 15)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), + } + ),] + .into_iter() + .collect(), + } ); } #[test] fn test_test_collector_separate_test_function() { - let res = collect( + let test_module = collect( r#" async function someFunction() {} Deno.test(someFunction); @@ -798,20 +972,33 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568" - .to_string(), - name: "someFunction".to_string(), - range: new_range(51, 55), - steps: vec![] - }] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![( + "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568" + .to_string(), + TestDefinition { + id: + "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568" + .to_string(), + name: "someFunction".to_string(), + range: Some(new_range(2, 11, 2, 15)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), + } + ),] + .into_iter() + .collect(), + } ); } #[test] fn test_test_collector_unknown_test() { - let res = collect( + let test_module = collect( r#" const someFunction = () => ({ name: "test", fn: () => {} }); Deno.test(someFunction()); @@ -819,21 +1006,34 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "6d05d6dc35548b86a1e70acaf24a5bc2dd35db686b35b685ad5931d201b4a918" - .to_string(), - name: "Test 3:7".to_string(), - range: new_range(79, 83), - steps: vec![] - }] + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![( + "6d05d6dc35548b86a1e70acaf24a5bc2dd35db686b35b685ad5931d201b4a918" + .to_string(), + TestDefinition { + id: + "6d05d6dc35548b86a1e70acaf24a5bc2dd35db686b35b685ad5931d201b4a918" + .to_string(), + name: "Test 3:7".to_string(), + range: Some(new_range(2, 11, 2, 15)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), + } + ),] + .into_iter() + .collect(), + } ); } // Regression test for https://github.com/denoland/vscode_deno/issues/656. #[test] fn test_test_collector_nested_steps_same_name_and_level() { - let res = collect( + let test_module = collect( r#" Deno.test("1", async (t) => { await t.step("step 1", async (t) => { @@ -847,36 +1047,71 @@ pub mod tests { ); assert_eq!( - res, - vec![TestDefinition { - id: "3799fc549a32532145ffc8532b0cd943e025bbc19a02e2cde9be94f87bceb829".to_string(), - name: "1".to_string(), - range: new_range(12, 16), - steps: vec![ - TestDefinition { - id: "e714fc695c0895327bf7148a934c3303ad515af029a14906be46f80340c6d7e3".to_string(), - name: "step 1".to_string(), - range: new_range(53, 57), - steps: vec![TestDefinition { + &test_module, + &TestModule { + specifier: test_module.specifier.clone(), + script_version: test_module.script_version.clone(), + defs: vec![ + ( + "3799fc549a32532145ffc8532b0cd943e025bbc19a02e2cde9be94f87bceb829".to_string(), + TestDefinition { + id: "3799fc549a32532145ffc8532b0cd943e025bbc19a02e2cde9be94f87bceb829".to_string(), + name: "1".to_string(), + range: Some(new_range(1, 11, 1, 15)), + is_dynamic: false, + parent_id: None, + step_ids: vec![ + "e714fc695c0895327bf7148a934c3303ad515af029a14906be46f80340c6d7e3".to_string(), + "ec6b03d3dd3dde78d2d11ed981d3386083aeca701510cc049189d74bd79f8587".to_string(), + ].into_iter().collect() + } + ), + ( + "e714fc695c0895327bf7148a934c3303ad515af029a14906be46f80340c6d7e3".to_string(), + TestDefinition { + id: "e714fc695c0895327bf7148a934c3303ad515af029a14906be46f80340c6d7e3".to_string(), + name: "step 1".to_string(), + range: Some(new_range(2, 16, 2, 20)), + is_dynamic: false, + parent_id: Some("3799fc549a32532145ffc8532b0cd943e025bbc19a02e2cde9be94f87bceb829".to_string()), + step_ids: vec!["d874949e18dfc297e15c52ff13f13b4e6ae911ec1818b2c761e3313bc018a3ab".to_string()].into_iter().collect() + } + ), + ( + "d874949e18dfc297e15c52ff13f13b4e6ae911ec1818b2c761e3313bc018a3ab".to_string(), + TestDefinition { id: "d874949e18dfc297e15c52ff13f13b4e6ae911ec1818b2c761e3313bc018a3ab".to_string(), name: "nested step".to_string(), - range: new_range(101, 105), - steps: vec![], - }], - }, - TestDefinition { - id: "ec6b03d3dd3dde78d2d11ed981d3386083aeca701510cc049189d74bd79f8587".to_string(), - name: "step 2".to_string(), - range: new_range(160, 164), - steps: vec![TestDefinition { + range: Some(new_range(3, 18, 3, 22)), + is_dynamic: false, + parent_id: Some("e714fc695c0895327bf7148a934c3303ad515af029a14906be46f80340c6d7e3".to_string()), + step_ids: Default::default(), + } + ), + ( + "ec6b03d3dd3dde78d2d11ed981d3386083aeca701510cc049189d74bd79f8587".to_string(), + TestDefinition { + id: "ec6b03d3dd3dde78d2d11ed981d3386083aeca701510cc049189d74bd79f8587".to_string(), + name: "step 2".to_string(), + range: Some(new_range(5, 16, 5, 20)), + is_dynamic: false, + parent_id: Some("3799fc549a32532145ffc8532b0cd943e025bbc19a02e2cde9be94f87bceb829".to_string()), + step_ids: vec!["96729f1f1608e50160b0bf11946719384b4021fd1d26b14eff7765034b3d2684".to_string()].into_iter().collect() + } + ), + ( + "96729f1f1608e50160b0bf11946719384b4021fd1d26b14eff7765034b3d2684".to_string(), + TestDefinition { id: "96729f1f1608e50160b0bf11946719384b4021fd1d26b14eff7765034b3d2684".to_string(), name: "nested step".to_string(), - range: new_range(208, 212), - steps: vec![], - }], - }, - ] - }] + range: Some(new_range(6, 18, 6, 22)), + is_dynamic: false, + parent_id: Some("ec6b03d3dd3dde78d2d11ed981d3386083aeca701510cc049189d74bd79f8587".to_string()), + step_ids: Default::default(), + } + ), + ].into_iter().collect(), + } ); } } diff --git a/cli/lsp/testing/definitions.rs b/cli/lsp/testing/definitions.rs index 6992c995de..30b0d3bb05 100644 --- a/cli/lsp/testing/definitions.rs +++ b/cli/lsp/testing/definitions.rs @@ -1,172 +1,181 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use super::lsp_custom; +use super::lsp_custom::TestData; -use crate::lsp::analysis::source_range_to_lsp_range; use crate::lsp::client::TestingNotification; +use crate::lsp::logging::lsp_warn; +use crate::tools::test::TestDescription; +use crate::tools::test::TestStepDescription; use crate::util::checksum; -use deno_ast::SourceRange; -use deno_ast::SourceTextInfo; use deno_core::ModuleSpecifier; +use lsp::Range; use std::collections::HashMap; +use std::collections::HashSet; use tower_lsp::lsp_types as lsp; #[derive(Debug, Clone, PartialEq)] pub struct TestDefinition { pub id: String, pub name: String, - pub range: SourceRange, - pub steps: Vec, + pub range: Option, + pub is_dynamic: bool, + pub parent_id: Option, + pub step_ids: HashSet, } -impl TestDefinition { - pub fn new( - specifier: &ModuleSpecifier, - name: String, - range: SourceRange, - mut steps: Vec, - ) -> Self { - let mut id_components = Vec::with_capacity(7); - id_components.push(specifier.as_str().as_bytes()); - id_components.push(name.as_bytes()); - let id = checksum::gen(&id_components); - Self::fix_ids(&mut steps, &mut id_components); - Self { - id, - name, - range, - steps, - } - } - - fn fix_ids<'a>( - steps: &'a mut Vec, - id_components: &mut Vec<&'a [u8]>, - ) { - for step in steps { - id_components.push(step.name.as_bytes()); - step.id = checksum::gen(id_components); - Self::fix_ids(&mut step.steps, id_components); - id_components.pop(); - } - } - - pub fn new_step( - name: String, - range: SourceRange, - steps: Vec, - ) -> Self { - Self { - // ID will be fixed later when the entire ancestry is available. - id: "".to_string(), - name, - range, - steps, - } - } - - fn as_test_data( - &self, - source_text_info: &SourceTextInfo, - ) -> lsp_custom::TestData { - lsp_custom::TestData { - id: self.id.clone(), - label: self.name.clone(), - steps: self - .steps - .iter() - .map(|step| step.as_test_data(source_text_info)) - .collect(), - range: Some(source_range_to_lsp_range(&self.range, source_text_info)), - } - } - - fn contains_id>(&self, id: S) -> bool { - let id = id.as_ref(); - self.id == id || self.steps.iter().any(|td| td.contains_id(id)) - } -} - -#[derive(Debug, Clone)] -pub struct TestDefinitions { - /// definitions of tests and their steps which were statically discovered from - /// the source document. - pub discovered: Vec, - /// Tests and steps which the test runner notified us of, which were - /// dynamically added - pub injected: Vec, +#[derive(Debug, Clone, PartialEq)] +pub struct TestModule { + pub specifier: ModuleSpecifier, /// The version of the document that the discovered tests relate to. pub script_version: String, + pub defs: HashMap, } -impl Default for TestDefinitions { - fn default() -> Self { - TestDefinitions { - script_version: "1".to_string(), - discovered: vec![], - injected: vec![], +impl TestModule { + pub fn new(specifier: ModuleSpecifier, script_version: String) -> Self { + Self { + specifier, + script_version, + defs: Default::default(), } } -} -impl TestDefinitions { + /// Returns `(id, is_newly_registered)`. + pub fn register( + &mut self, + name: String, + range: Option, + is_dynamic: bool, + parent_id: Option, + ) -> (String, bool) { + let mut id_components = Vec::with_capacity(7); + id_components.push(name.as_bytes()); + let mut current_parent_id = &parent_id; + while let Some(parent_id) = current_parent_id { + let parent = match self.defs.get(parent_id) { + Some(d) => d, + None => { + lsp_warn!("Internal Error: parent_id \"{}\" of test \"{}\" was not registered.", parent_id, &name); + id_components.push("".as_bytes()); + break; + } + }; + id_components.push(parent.name.as_bytes()); + current_parent_id = &parent.parent_id; + } + id_components.push(self.specifier.as_str().as_bytes()); + id_components.reverse(); + let id = checksum::gen(&id_components); + if self.defs.contains_key(&id) { + return (id, false); + } + if let Some(parent_id) = &parent_id { + let parent = self.defs.get_mut(parent_id).unwrap(); + parent.step_ids.insert(id.clone()); + } + self.defs.insert( + id.clone(), + TestDefinition { + id: id.clone(), + name, + range, + is_dynamic, + parent_id, + step_ids: Default::default(), + }, + ); + (id, true) + } + + /// Returns `(id, was_newly_registered)`. + pub fn register_dynamic(&mut self, desc: &TestDescription) -> (String, bool) { + self.register(desc.name.clone(), None, true, None) + } + + /// Returns `(id, was_newly_registered)`. + pub fn register_step_dynamic( + &mut self, + desc: &TestStepDescription, + parent_static_id: &str, + ) -> (String, bool) { + self.register( + desc.name.clone(), + None, + true, + Some(parent_static_id.to_string()), + ) + } + + pub fn get(&self, id: &str) -> Option<&TestDefinition> { + self.defs.get(id) + } + + pub fn get_test_data(&self, id: &str) -> TestData { + fn get_test_data_inner(tm: &TestModule, id: &str) -> TestData { + let def = tm.defs.get(id).unwrap(); + TestData { + id: def.id.clone(), + label: def.name.clone(), + steps: def + .step_ids + .iter() + .map(|id| get_test_data_inner(tm, id)) + .collect(), + range: def.range, + } + } + let def = self.defs.get(id).unwrap(); + let mut current_data = get_test_data_inner(self, &def.id); + let mut current_parent_id = &def.parent_id; + while let Some(parent_id) = current_parent_id { + let parent = self.defs.get(parent_id).unwrap(); + current_data = TestData { + id: parent.id.clone(), + label: parent.name.clone(), + steps: vec![current_data], + range: None, + }; + current_parent_id = &parent.parent_id; + } + current_data + } + /// Return the test definitions as a testing module notification. - pub fn as_notification( + pub fn as_replace_notification( &self, - specifier: &ModuleSpecifier, - maybe_root: Option<&ModuleSpecifier>, - source_text_info: &SourceTextInfo, + maybe_root_uri: Option<&ModuleSpecifier>, ) -> TestingNotification { - let label = if let Some(root) = maybe_root { - specifier.as_str().replace(root.as_str(), "") - } else { - specifier - .path_segments() - .and_then(|s| s.last().map(|s| s.to_string())) - .unwrap_or_else(|| "".to_string()) - }; - let mut tests_map: HashMap = self - .injected - .iter() - .map(|td| (td.id.clone(), td.clone())) - .collect(); - tests_map.extend(self.discovered.iter().map(|td| { - let test_data = td.as_test_data(source_text_info); - (test_data.id.clone(), test_data) - })); + let label = self.label(maybe_root_uri); TestingNotification::Module(lsp_custom::TestModuleNotificationParams { text_document: lsp::TextDocumentIdentifier { - uri: specifier.clone(), + uri: self.specifier.clone(), }, kind: lsp_custom::TestModuleNotificationKind::Replace, label, - tests: tests_map.into_values().collect(), + tests: self + .defs + .iter() + .filter(|(_, def)| def.parent_id.is_none()) + .map(|(id, _)| self.get_test_data(id)) + .collect(), }) } - /// Register a dynamically-detected test. Returns false if a test with the - /// same static id was already registered statically or dynamically. Otherwise - /// returns true. - pub fn inject(&mut self, data: lsp_custom::TestData) -> bool { - if self.discovered.iter().any(|td| td.contains_id(&data.id)) - || self.injected.iter().any(|td| td.id == data.id) - { - return false; + pub fn label(&self, maybe_root_uri: Option<&ModuleSpecifier>) -> String { + if let Some(root) = maybe_root_uri { + self.specifier.as_str().replace(root.as_str(), "") + } else { + self + .specifier + .path_segments() + .and_then(|s| s.last().map(|s| s.to_string())) + .unwrap_or_else(|| "".to_string()) } - self.injected.push(data); - true - } - - /// Return a test definition identified by the test ID. - pub fn get_by_id>(&self, id: S) -> Option<&TestDefinition> { - self - .discovered - .iter() - .find(|td| td.id.as_str() == id.as_ref()) } pub fn is_empty(&self) -> bool { - self.discovered.is_empty() && self.injected.is_empty() + self.defs.is_empty() } } diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index 54c5e69143..9c8c7c98fe 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -1,7 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use super::definitions::TestDefinition; -use super::definitions::TestDefinitions; +use super::definitions::TestModule; use super::lsp_custom; use crate::args::flags_from_vec; @@ -14,7 +14,6 @@ use crate::lsp::logging::lsp_log; use crate::tools::test; use crate::tools::test::FailFastTracker; use crate::tools::test::TestEventSender; -use crate::util::checksum; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; @@ -44,7 +43,7 @@ use tower_lsp::lsp_types as lsp; /// any filters to be applied to those tests fn as_queue_and_filters( params: &lsp_custom::TestRunRequestParams, - tests: &HashMap, + tests: &HashMap, ) -> ( HashSet, HashMap, @@ -57,7 +56,7 @@ fn as_queue_and_filters( if let Some(test_definitions) = tests.get(&item.text_document.uri) { queue.insert(item.text_document.uri.clone()); if let Some(id) = &item.id { - if let Some(test) = test_definitions.get_by_id(id) { + if let Some(test) = test_definitions.get(id) { let filter = filters.entry(item.text_document.uri.clone()).or_default(); if let Some(include) = filter.include.as_mut() { @@ -80,7 +79,7 @@ fn as_queue_and_filters( if let Some(id) = &item.id { // there is no way to exclude a test step if item.step_id.is_none() { - if let Some(test) = test_definitions.get_by_id(id) { + if let Some(test) = test_definitions.get(id) { let filter = filters.entry(item.text_document.uri.clone()).or_default(); filter.exclude.insert(test.id.clone(), test.clone()); @@ -125,14 +124,15 @@ struct LspTestFilter { } impl LspTestFilter { - fn as_ids(&self, test_definitions: &TestDefinitions) -> Vec { + fn as_ids(&self, test_module: &TestModule) -> Vec { let ids: Vec = if let Some(include) = &self.include { include.keys().cloned().collect() } else { - test_definitions - .discovered + test_module + .defs .iter() - .map(|td| td.id.clone()) + .filter(|(_, d)| d.parent_id.is_none()) + .map(|(k, _)| k.clone()) .collect() }; ids @@ -148,7 +148,7 @@ pub struct TestRun { kind: lsp_custom::TestRunKind, filters: HashMap, queue: HashSet, - tests: Arc>>, + tests: Arc>>, token: CancellationToken, workspace_settings: config::WorkspaceSettings, } @@ -156,7 +156,7 @@ pub struct TestRun { impl TestRun { pub fn new( params: &lsp_custom::TestRunRequestParams, - tests: Arc>>, + tests: Arc>>, workspace_settings: config::WorkspaceSettings, ) -> Self { let (queue, filters) = { @@ -183,15 +183,11 @@ impl TestRun { .queue .iter() .map(|s| { - let ids = if let Some(test_definitions) = tests.get(s) { + let ids = if let Some(test_module) = tests.get(s) { if let Some(filter) = self.filters.get(s) { - filter.as_ids(test_definitions) + filter.as_ids(test_module) } else { - test_definitions - .discovered - .iter() - .map(|test| test.id.clone()) - .collect() + LspTestFilter::default().as_ids(test_module) } } else { Vec::new() @@ -479,117 +475,60 @@ impl TestRun { #[derive(Debug, PartialEq)] enum LspTestDescription { - TestDescription(test::TestDescription), - /// `(desc, static_id, root_static_id)` - TestStepDescription(test::TestStepDescription, String, String), + /// `(desc, static_id)` + TestDescription(test::TestDescription, String), + /// `(desc, static_id)` + TestStepDescription(test::TestStepDescription, String), } impl LspTestDescription { fn origin(&self) -> &str { match self { - LspTestDescription::TestDescription(d) => d.origin.as_str(), - LspTestDescription::TestStepDescription(d, _, _) => d.origin.as_str(), + LspTestDescription::TestDescription(d, _) => d.origin.as_str(), + LspTestDescription::TestStepDescription(d, _) => d.origin.as_str(), } } fn location(&self) -> &test::TestLocation { match self { - LspTestDescription::TestDescription(d) => &d.location, - LspTestDescription::TestStepDescription(d, _, _) => &d.location, + LspTestDescription::TestDescription(d, _) => &d.location, + LspTestDescription::TestStepDescription(d, _) => &d.location, } } fn parent_id(&self) -> Option { match self { - LspTestDescription::TestDescription(_) => None, - LspTestDescription::TestStepDescription(d, _, _) => Some(d.parent_id), + LspTestDescription::TestDescription(_, _) => None, + LspTestDescription::TestStepDescription(d, _) => Some(d.parent_id), } } - fn from_step_description( - desc: &test::TestStepDescription, + fn static_id(&self) -> &str { + match self { + LspTestDescription::TestDescription(_, i) => i, + LspTestDescription::TestStepDescription(_, i) => i, + } + } + + fn as_test_identifier( + &self, tests: &IndexMap, - ) -> Self { - let mut id_components = Vec::with_capacity(7); - let mut root_static_id = None; - let mut step_desc = Some(desc); - while let Some(desc) = step_desc { - id_components.push(desc.name.as_bytes()); - let parent_desc = tests.get(&desc.parent_id).unwrap(); - step_desc = match parent_desc { - LspTestDescription::TestDescription(d) => { - id_components.push(d.name.as_bytes()); - root_static_id = Some(d.static_id()); - None - } - LspTestDescription::TestStepDescription(d, _, _) => Some(d), - }; + ) -> lsp_custom::TestIdentifier { + let uri = ModuleSpecifier::parse(&self.location().file_name).unwrap(); + let static_id = self.static_id(); + let mut root_desc = self; + while let Some(parent_id) = root_desc.parent_id() { + root_desc = tests.get(&parent_id).unwrap(); } - id_components.push(desc.origin.as_bytes()); - id_components.reverse(); - let static_id = checksum::gen(&id_components); - LspTestDescription::TestStepDescription( - desc.clone(), - static_id, - root_static_id.unwrap(), - ) - } -} - -impl From<&test::TestDescription> for LspTestDescription { - fn from(desc: &test::TestDescription) -> Self { - Self::TestDescription(desc.clone()) - } -} - -impl From<&LspTestDescription> for lsp_custom::TestIdentifier { - fn from(desc: &LspTestDescription) -> lsp_custom::TestIdentifier { - match desc { - LspTestDescription::TestDescription(d) => d.into(), - LspTestDescription::TestStepDescription(d, static_id, root_static_id) => { - let uri = ModuleSpecifier::parse(&d.origin).unwrap(); - Self { - text_document: lsp::TextDocumentIdentifier { uri }, - id: Some(root_static_id.clone()), - step_id: Some(static_id.clone()), - } - } - } - } -} - -impl From<&LspTestDescription> for lsp_custom::TestData { - fn from(desc: &LspTestDescription) -> Self { - match desc { - LspTestDescription::TestDescription(d) => d.into(), - LspTestDescription::TestStepDescription(d, static_id, _) => Self { - id: static_id.clone(), - label: d.name.clone(), - steps: Default::default(), - range: None, - }, - } - } -} - -impl From<&test::TestDescription> for lsp_custom::TestData { - fn from(desc: &test::TestDescription) -> Self { - Self { - id: desc.static_id(), - label: desc.name.clone(), - steps: Default::default(), - range: None, - } - } -} - -impl From<&test::TestDescription> for lsp_custom::TestIdentifier { - fn from(desc: &test::TestDescription) -> Self { - let uri = ModuleSpecifier::parse(&desc.origin).unwrap(); - Self { + let root_static_id = root_desc.static_id(); + lsp_custom::TestIdentifier { text_document: lsp::TextDocumentIdentifier { uri }, - id: Some(desc.static_id()), - step_id: None, + id: Some(root_static_id.to_string()), + step_id: if static_id == root_static_id { + None + } else { + Some(static_id.to_string()) + }, } } } @@ -598,7 +537,7 @@ struct LspTestReporter { client: Client, id: u32, maybe_root_uri: Option, - files: Arc>>, + files: Arc>>, tests: IndexMap, current_test: Option, } @@ -608,7 +547,7 @@ impl LspTestReporter { run: &TestRun, client: Client, maybe_root_uri: Option<&ModuleSpecifier>, - files: Arc>>, + files: Arc>>, ) -> Self { Self { client, @@ -634,29 +573,27 @@ impl LspTestReporter { fn report_plan(&mut self, _plan: &test::TestPlan) {} fn report_register(&mut self, desc: &test::TestDescription) { - self.tests.insert(desc.id, desc.into()); let mut files = self.files.lock(); - let tds = files - .entry(ModuleSpecifier::parse(&desc.location.file_name).unwrap()) - .or_default(); - if tds.inject(desc.into()) { - let specifier = ModuleSpecifier::parse(&desc.origin).unwrap(); - let label = if let Some(root) = &self.maybe_root_uri { - specifier.as_str().replace(root.as_str(), "") - } else { - specifier - .path_segments() - .and_then(|s| s.last().map(|s| s.to_string())) - .unwrap_or_else(|| "".to_string()) - }; + let specifier = ModuleSpecifier::parse(&desc.location.file_name).unwrap(); + let test_module = files + .entry(specifier.clone()) + .or_insert_with(|| TestModule::new(specifier, "1".to_string())); + let (static_id, is_new) = test_module.register_dynamic(desc); + self.tests.insert( + desc.id, + LspTestDescription::TestDescription(desc.clone(), static_id.clone()), + ); + if is_new { self .client .send_test_notification(TestingNotification::Module( lsp_custom::TestModuleNotificationParams { - text_document: lsp::TextDocumentIdentifier { uri: specifier }, + text_document: lsp::TextDocumentIdentifier { + uri: test_module.specifier.clone(), + }, kind: lsp_custom::TestModuleNotificationKind::Insert, - label, - tests: vec![desc.into()], + label: test_module.label(self.maybe_root_uri.as_ref()), + tests: vec![test_module.get_test_data(&static_id)], }, )); } @@ -664,7 +601,8 @@ impl LspTestReporter { fn report_wait(&mut self, desc: &test::TestDescription) { self.current_test = Some(desc.id); - let test = desc.into(); + let desc = self.tests.get(&desc.id).unwrap(); + let test = desc.as_test_identifier(&self.tests); self.progress(lsp_custom::TestRunProgressMessage::Started { test }); } @@ -672,7 +610,7 @@ impl LspTestReporter { let test = self .current_test .as_ref() - .map(|id| self.tests.get(id).unwrap().into()); + .map(|id| self.tests.get(id).unwrap().as_test_identifier(&self.tests)); let value = String::from_utf8_lossy(output).replace('\n', "\r\n"); self.progress(lsp_custom::TestRunProgressMessage::Output { value, @@ -691,26 +629,30 @@ impl LspTestReporter { self.current_test = None; match result { test::TestResult::Ok => { + let desc = self.tests.get(&desc.id).unwrap(); self.progress(lsp_custom::TestRunProgressMessage::Passed { - test: desc.into(), + test: desc.as_test_identifier(&self.tests), duration: Some(elapsed as u32), }) } test::TestResult::Ignored => { + let desc = self.tests.get(&desc.id).unwrap(); self.progress(lsp_custom::TestRunProgressMessage::Skipped { - test: desc.into(), + test: desc.as_test_identifier(&self.tests), }) } test::TestResult::Failed(failure) => { + let desc = self.tests.get(&desc.id).unwrap(); self.progress(lsp_custom::TestRunProgressMessage::Failed { - test: desc.into(), + test: desc.as_test_identifier(&self.tests), messages: as_test_messages(failure.to_string(), false), duration: Some(elapsed as u32), }) } test::TestResult::Cancelled => { + let desc = self.tests.get(&desc.id).unwrap(); self.progress(lsp_custom::TestRunProgressMessage::Failed { - test: desc.into(), + test: desc.as_test_identifier(&self.tests), messages: vec![], duration: Some(elapsed as u32), }) @@ -728,7 +670,7 @@ impl LspTestReporter { let messages = as_test_messages(err_string, false); for desc in self.tests.values().filter(|d| d.origin() == origin) { self.progress(lsp_custom::TestRunProgressMessage::Failed { - test: desc.into(), + test: desc.as_test_identifier(&self.tests), messages: messages.clone(), duration: None, }); @@ -736,43 +678,30 @@ impl LspTestReporter { } fn report_step_register(&mut self, desc: &test::TestStepDescription) { + let mut files = self.files.lock(); + let specifier = ModuleSpecifier::parse(&desc.location.file_name).unwrap(); + let test_module = files + .entry(specifier.clone()) + .or_insert_with(|| TestModule::new(specifier, "1".to_string())); + let (static_id, is_new) = test_module.register_step_dynamic( + desc, + self.tests.get(&desc.parent_id).unwrap().static_id(), + ); self.tests.insert( desc.id, - LspTestDescription::from_step_description(desc, &self.tests), + LspTestDescription::TestStepDescription(desc.clone(), static_id.clone()), ); - let desc = self.tests.get(&desc.id).unwrap(); - let mut files = self.files.lock(); - let tds = files - .entry(ModuleSpecifier::parse(&desc.location().file_name).unwrap()) - .or_default(); - let data: lsp_custom::TestData = desc.into(); - if tds.inject(data.clone()) { - let mut data = data; - let mut current_desc = desc; - while let Some(parent_id) = current_desc.parent_id() { - let parent_desc = self.tests.get(&parent_id).unwrap(); - let mut parent_data: lsp_custom::TestData = parent_desc.into(); - parent_data.steps = vec![data]; - data = parent_data; - current_desc = parent_desc; - } - let specifier = ModuleSpecifier::parse(desc.origin()).unwrap(); - let label = if let Some(root) = &self.maybe_root_uri { - specifier.as_str().replace(root.as_str(), "") - } else { - specifier - .path_segments() - .and_then(|s| s.last().map(|s| s.to_string())) - .unwrap_or_else(|| "".to_string()) - }; + if is_new { self .client .send_test_notification(TestingNotification::Module( lsp_custom::TestModuleNotificationParams { - text_document: lsp::TextDocumentIdentifier { uri: specifier }, + text_document: lsp::TextDocumentIdentifier { + uri: test_module.specifier.clone(), + }, kind: lsp_custom::TestModuleNotificationKind::Insert, - label, - tests: vec![data], + label: test_module.label(self.maybe_root_uri.as_ref()), + tests: vec![test_module.get_test_data(&static_id)], }, )); } @@ -782,8 +711,8 @@ impl LspTestReporter { if self.current_test == Some(desc.parent_id) { self.current_test = Some(desc.id); } - let desc = &LspTestDescription::from_step_description(desc, &self.tests); - let test: lsp_custom::TestIdentifier = desc.into(); + let desc = self.tests.get(&desc.id).unwrap(); + let test = desc.as_test_identifier(&self.tests); self.progress(lsp_custom::TestRunProgressMessage::Started { test }); } @@ -796,22 +725,22 @@ impl LspTestReporter { if self.current_test == Some(desc.id) { self.current_test = Some(desc.parent_id); } - let desc = &LspTestDescription::from_step_description(desc, &self.tests); + let desc = self.tests.get(&desc.id).unwrap(); match result { test::TestStepResult::Ok => { self.progress(lsp_custom::TestRunProgressMessage::Passed { - test: desc.into(), + test: desc.as_test_identifier(&self.tests), duration: Some(elapsed as u32), }) } test::TestStepResult::Ignored => { self.progress(lsp_custom::TestRunProgressMessage::Skipped { - test: desc.into(), + test: desc.as_test_identifier(&self.tests), }) } test::TestStepResult::Failed(failure) => { self.progress(lsp_custom::TestRunProgressMessage::Failed { - test: desc.into(), + test: desc.as_test_identifier(&self.tests), messages: as_test_messages(failure.to_string(), false), duration: Some(elapsed as u32), }) @@ -875,23 +804,35 @@ mod tests { id: "0b7c6bf3cd617018d33a1bf982a08fe088c5bb54fcd5eb9e802e7c137ec1af94" .to_string(), name: "test a".to_string(), - range: new_range(420, 424), - steps: vec![], + range: Some(new_range(1, 5, 1, 9)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), }; let test_def_b = TestDefinition { id: "69d9fe87f64f5b66cb8b631d4fd2064e8224b8715a049be54276c42189ff8f9f" .to_string(), name: "test b".to_string(), - range: new_range(480, 481), - steps: vec![], + range: Some(new_range(2, 5, 2, 9)), + is_dynamic: false, + parent_id: None, + step_ids: Default::default(), }; - let test_definitions = TestDefinitions { - discovered: vec![test_def_a, test_def_b.clone()], - injected: vec![], + let test_module = TestModule { + specifier: specifier.clone(), script_version: "1".to_string(), + defs: vec![ + (test_def_a.id.clone(), test_def_a.clone()), + (test_def_b.id.clone(), test_def_b.clone()), + ] + .into_iter() + .collect(), }; - tests.insert(specifier.clone(), test_definitions.clone()); - tests.insert(non_test_specifier, Default::default()); + tests.insert(specifier.clone(), test_module.clone()); + tests.insert( + non_test_specifier.clone(), + TestModule::new(non_test_specifier, "1".to_string()), + ); let (queue, filters) = as_queue_and_filters(¶ms, &tests); assert_eq!(json!(queue), json!([specifier])); let mut exclude = HashMap::new(); @@ -911,7 +852,7 @@ mod tests { } ); assert_eq!( - filter.as_ids(&test_definitions), + filter.as_ids(&test_module), vec![ "0b7c6bf3cd617018d33a1bf982a08fe088c5bb54fcd5eb9e802e7c137ec1af94" .to_string() diff --git a/cli/lsp/testing/server.rs b/cli/lsp/testing/server.rs index 65a72bed43..58ac7a8c49 100644 --- a/cli/lsp/testing/server.rs +++ b/cli/lsp/testing/server.rs @@ -1,7 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use super::collectors::TestCollector; -use super::definitions::TestDefinitions; +use super::definitions::TestModule; use super::execution::TestRun; use super::lsp_custom; @@ -47,7 +47,7 @@ pub struct TestServer { /// A map of run ids to test runs runs: Arc>>, /// Tests that are discovered from a versioned document - tests: Arc>>, + tests: Arc>>, /// A channel for requesting that changes to documents be statically analyzed /// for tests update_channel: mpsc::UnboundedSender>, @@ -59,7 +59,7 @@ impl TestServer { performance: Arc, maybe_root_uri: Option, ) -> Self { - let tests: Arc>> = + let tests: Arc>> = Arc::new(Mutex::new(HashMap::new())); let (update_channel, mut update_rx) = @@ -109,31 +109,27 @@ impl TestServer { if let Some(Ok(parsed_source)) = document.maybe_parsed_source() { - let old_tds = tests.remove(specifier).unwrap_or_default(); + let was_empty = tests + .remove(specifier) + .map(|tm| tm.is_empty()) + .unwrap_or(true); let mut collector = TestCollector::new( specifier.clone(), + script_version, parsed_source.text_info().clone(), ); parsed_source.module().visit_with(&mut collector); - let test_definitions = TestDefinitions { - discovered: collector.take(), - injected: Default::default(), - script_version, - }; - if !test_definitions.discovered.is_empty() { + let test_module = collector.take(); + if !test_module.is_empty() { client.send_test_notification( - test_definitions.as_notification( - specifier, - mru.as_ref(), - parsed_source.text_info(), - ), + test_module.as_replace_notification(mru.as_ref()), ); - } else if !old_tds.is_empty() { + } else if !was_empty { client.send_test_notification(as_delete_notification( specifier.clone(), )); } - tests.insert(specifier.clone(), test_definitions); + tests.insert(specifier.clone(), test_module); } } } diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 2a56f5506b..9f485411c1 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -15,7 +15,6 @@ use crate::graph_util::graph_valid_with_cli_options; use crate::graph_util::has_graph_root_local_dependent_changed; use crate::module_loader::ModuleLoadPreparer; use crate::ops; -use crate::util::checksum; use crate::util::file_watcher; use crate::util::fs::collect_specifiers; use crate::util::path::get_extension; @@ -173,12 +172,6 @@ pub struct TestDescription { pub location: TestLocation, } -impl TestDescription { - pub fn static_id(&self) -> String { - checksum::gen(&[self.location.file_name.as_bytes(), self.name.as_bytes()]) - } -} - #[derive(Debug, Clone, Eq, PartialEq, Deserialize)] #[serde(rename_all = "camelCase")] pub enum TestOutput {