foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-24 08:09:08 -05:00
Code Issues Packages 1 Wiki Activity

fix(lsp/testing): use full ancestry to compute static id of step (#20297)

Browse source 
Fixes https://github.com/denoland/vscode_deno/issues/656.

Test steps were ID'd by a checksum of `[origin, level, step_name]` which
is incorrect. Now it's `[origin, ...ancestor_names, step_name]`.
This commit is contained in:
Nayeem Rahman 2023-08-27 10:16:09 +01:00 committed by GitHub
parent 2080669943
commit e1fe31508c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 171 additions and 134 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

120
cli/lsp/testing/collectors.rs
View file

@ -15,7 +15,6 @@ use std::collections::HashSet;
/// Parse an arrow expression for any test steps and return them.
fn arrow_to_steps(
parent: &str,
level: usize,
arrow_expr: &ast::ArrowExpr,
) -> Vec<TestDefinition> {
if let Some((maybe_test_context, maybe_step_var)) =
@ -23,7 +22,6 @@ fn arrow_to_steps(
{
let mut collector = TestStepCollector::new(
parent.to_string(),
level,
maybe_test_context,
maybe_step_var,
);
@ -35,17 +33,12 @@ fn arrow_to_steps(
}
/// Parse a function for any test steps and return them.
fn fn_to_steps(
parent: &str,
level: usize,
function: &ast::Function,
) -> Vec<TestDefinition> {
fn fn_to_steps(parent: &str, function: &ast::Function) -> Vec<TestDefinition> {
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(),
level,
maybe_test_context,
maybe_step_var,
);
@ -130,7 +123,6 @@ fn parse_test_context_param(
fn check_call_expr(
parent: &str,
node: &ast::CallExpr,
level: usize,
fns: Option<&HashMap<String, ast::Function>>,
text_info: Option<&SourceTextInfo>,
) -> Option<(String, Vec<TestDefinition>)> {
@ -164,10 +156,10 @@ fn check_call_expr(
},
"fn" => match key_value_prop.value.as_ref() {
ast::Expr::Arrow(arrow_expr) => {
steps = arrow_to_steps(parent, level, arrow_expr);
steps = arrow_to_steps(parent, arrow_expr);
}
ast::Expr::Fn(fn_expr) => {
steps = fn_to_steps(parent, level, &fn_expr.function);
steps = fn_to_steps(parent, &fn_expr.function);
}
_ => (),
},
@ -176,7 +168,7 @@ fn check_call_expr(
}
}
ast::Prop::Method(method_prop) => {
steps = fn_to_steps(parent, level, &method_prop.function);
steps = fn_to_steps(parent, &method_prop.function);
}
_ => (),
}
@ -187,7 +179,7 @@ fn check_call_expr(
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, level, &fn_expr.function);
let steps = fn_to_steps(parent, &fn_expr.function);
Some((name, steps))
} else {
None
@ -198,10 +190,10 @@ fn check_call_expr(
let mut steps = vec![];
match node.args.get(1).map(|es| es.expr.as_ref()) {
Some(ast::Expr::Fn(fn_expr)) => {
steps = fn_to_steps(parent, level, &fn_expr.function);
steps = fn_to_steps(parent, &fn_expr.function);
}
Some(ast::Expr::Arrow(arrow_expr)) => {
steps = arrow_to_steps(parent, level, arrow_expr);
steps = arrow_to_steps(parent, arrow_expr);
}
_ => (),
}
@ -212,10 +204,10 @@ fn check_call_expr(
let mut steps = vec![];
match node.args.get(1).map(|es| es.expr.as_ref()) {
Some(ast::Expr::Fn(fn_expr)) => {
steps = fn_to_steps(parent, level, &fn_expr.function);
steps = fn_to_steps(parent, &fn_expr.function);
}
Some(ast::Expr::Arrow(arrow_expr)) => {
steps = arrow_to_steps(parent, level, arrow_expr);
steps = arrow_to_steps(parent, arrow_expr);
}
_ => (),
}
@ -230,7 +222,7 @@ fn check_call_expr(
fns.and_then(|fns| {
fns
.get(&name)
.map(|fn_expr| (name, fn_to_steps(parent, level, fn_expr)))
.map(|fn_expr| (name, fn_to_steps(parent, fn_expr)))
})
}
_ => {
@ -255,7 +247,6 @@ fn check_call_expr(
/// branch contains any testing steps.
struct TestStepCollector {
steps: Vec<TestDefinition>,
level: usize,
parent: String,
maybe_test_context: Option<String>,
vars: HashSet<String>,
@ -264,7 +255,6 @@ struct TestStepCollector {
impl TestStepCollector {
fn new(
parent: String,
level: usize,
maybe_test_context: Option<String>,
maybe_step_var: Option<String>,
) -> Self {
@ -274,7 +264,6 @@ impl TestStepCollector {
}
Self {
steps: Vec::default(),
level,
parent,
maybe_test_context,
vars,
@ -287,19 +276,13 @@ impl TestStepCollector {
range: SourceRange,
steps: Vec<TestDefinition>,
) {
let step = TestDefinition::new_step(
name.as_ref().to_string(),
range,
self.parent.clone(),
self.level,
steps,
);
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, self.level + 1, None, None)
if let Some((name, steps)) = check_call_expr(&self.parent, node, None, None)
{
self.add_step(name, range, steps);
}
@ -436,7 +419,6 @@ impl TestCollector {
if let Some((name, steps)) = check_call_expr(
self.specifier.as_str(),
node,
1,
Some(&self.fns),
Some(&self.text_info),
) {
@ -579,7 +561,6 @@ pub mod tests {
vec![TestDefinition {
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
.to_string(),
level: 0,
name: "test".to_string(),
range: new_range(12, 16),
steps: vec![],
@ -600,7 +581,6 @@ pub mod tests {
vec![TestDefinition {
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
.to_string(),
level: 0,
name: "test".to_string(),
range: new_range(12, 16),
steps: vec![],
@ -631,21 +611,18 @@ pub mod tests {
vec![TestDefinition {
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
.to_string(),
level: 0,
name: "test".to_string(),
range: new_range(12, 16),
steps: vec![TestDefinition {
id:
"b3b2daad49e5c3095fe26aba0a840131f3d8f32e105e95507f5fc5118642b059"
"704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c"
.to_string(),
level: 1,
name: "step".to_string(),
range: new_range(81, 85),
steps: vec![TestDefinition {
id:
"abf356f59139b77574089615f896a6f501c010985d95b8a93abeb0069ccb2201"
"0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d"
.to_string(),
level: 2,
name: "sub step".to_string(),
range: new_range(128, 132),
steps: vec![],
@ -678,21 +655,18 @@ pub mod tests {
vec![TestDefinition {
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
.to_string(),
level: 0,
name: "test".to_string(),
range: new_range(12, 16),
steps: vec![TestDefinition {
id:
"b3b2daad49e5c3095fe26aba0a840131f3d8f32e105e95507f5fc5118642b059"
"704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c"
.to_string(),
level: 1,
name: "step".to_string(),
range: new_range(81, 85),
steps: vec![TestDefinition {
id:
"abf356f59139b77574089615f896a6f501c010985d95b8a93abeb0069ccb2201"
"0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d"
.to_string(),
level: 2,
name: "sub step".to_string(),
range: new_range(128, 132),
steps: vec![],
@ -716,7 +690,6 @@ pub mod tests {
vec![TestDefinition {
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
.to_string(),
level: 0,
name: "test".to_string(),
range: new_range(36, 40),
steps: vec![],
@ -739,14 +712,12 @@ pub mod tests {
vec![TestDefinition {
id: "86b4c821900e38fc89f24bceb0e45193608ab3f9d2a6019c7b6a5aceff5d7df2"
.to_string(),
level: 0,
name: "useFnName".to_string(),
range: new_range(12, 16),
steps: vec![TestDefinition {
id:
"b3b2daad49e5c3095fe26aba0a840131f3d8f32e105e95507f5fc5118642b059"
"dac8a169b8f8c6babf11122557ea545de2733bfafed594d044b22bc6863a0856"
.to_string(),
level: 1,
name: "step".to_string(),
range: new_range(71, 72),
steps: vec![],
@ -769,7 +740,6 @@ pub mod tests {
vec![TestDefinition {
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
.to_string(),
level: 0,
name: "test".to_string(),
range: new_range(34, 35),
steps: vec![],
@ -791,7 +761,6 @@ pub mod tests {
vec![TestDefinition {
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
.to_string(),
level: 0,
name: "test".to_string(),
range: new_range(45, 49),
steps: vec![],
@ -812,7 +781,6 @@ pub mod tests {
vec![TestDefinition {
id: "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568"
.to_string(),
level: 0,
name: "someFunction".to_string(),
range: new_range(12, 16),
steps: vec![]
@ -834,7 +802,6 @@ pub mod tests {
vec![TestDefinition {
id: "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568"
.to_string(),
level: 0,
name: "someFunction".to_string(),
range: new_range(51, 55),
steps: vec![]
@ -856,11 +823,60 @@ pub mod tests {
vec![TestDefinition {
id: "6d05d6dc35548b86a1e70acaf24a5bc2dd35db686b35b685ad5931d201b4a918"
.to_string(),
level: 0,
name: "Test 3:7".to_string(),
range: new_range(79, 83),
steps: vec![]
}]
);
}
// 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(
r#"
Deno.test("1", async (t) => {
await t.step("step 1", async (t) => {
await t.step("nested step", () => {});
});
await t.step("step 2", async (t) => {
await t.step("nested step", () => {});
});
});
"#,
);
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 {
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 {
id: "96729f1f1608e50160b0bf11946719384b4021fd1d26b14eff7765034b3d2684".to_string(),
name: "nested step".to_string(),
range: new_range(208, 212),
steps: vec![],
}],
},
]
}]
);
}
}

33
cli/lsp/testing/definitions.rs
View file

@ -15,7 +15,6 @@ use tower_lsp::lsp_types as lsp;
#[derive(Debug, Clone, PartialEq)]
pub struct TestDefinition {
pub id: String,
pub level: usize,
pub name: String,
pub range: SourceRange,
pub steps: Vec<TestDefinition>,
@ -26,33 +25,41 @@ impl TestDefinition {
specifier: &ModuleSpecifier,
name: String,
range: SourceRange,
steps: Vec<TestDefinition>,
mut steps: Vec<TestDefinition>,
) -> Self {
let id = checksum::gen(&[specifier.as_str().as_bytes(), name.as_bytes()]);
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,
level: 0,
name,
range,
steps,
}
}
fn fix_ids<'a>(
steps: &'a mut Vec<TestDefinition>,
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,
parent: String,
level: usize,
steps: Vec<TestDefinition>,
) -> Self {
let id = checksum::gen(&[
parent.as_bytes(),
&level.to_be_bytes(),
name.as_bytes(),
]);
Self {
id,
level,
// ID will be fixed later when the entire ancestry is available.
id: "".to_string(),
name,
range,
steps,

144
cli/lsp/testing/execution.rs
View file

@ -478,48 +478,96 @@ impl TestRun {
}
#[derive(Debug, PartialEq)]
enum TestOrTestStepDescription {
enum LspTestDescription {
TestDescription(test::TestDescription),
TestStepDescription(test::TestStepDescription),
/// `(desc, static_id, root_static_id)`
TestStepDescription(test::TestStepDescription, String, String),
}
impl TestOrTestStepDescription {
impl LspTestDescription {
fn origin(&self) -> &str {
match self {
TestOrTestStepDescription::TestDescription(d) => d.origin.as_str(),
TestOrTestStepDescription::TestStepDescription(d) => d.origin.as_str(),
}
LspTestDescription::TestDescription(d) => d.origin.as_str(),
LspTestDescription::TestStepDescription(d, _, _) => d.origin.as_str(),
}
}
impl From<&test::TestDescription> for TestOrTestStepDescription {
fn location(&self) -> &test::TestLocation {
match self {
LspTestDescription::TestDescription(d) => &d.location,
LspTestDescription::TestStepDescription(d, _, _) => &d.location,
}
}
fn parent_id(&self) -> Option<usize> {
match self {
LspTestDescription::TestDescription(_) => None,
LspTestDescription::TestStepDescription(d, _, _) => Some(d.parent_id),
}
}
fn from_step_description(
desc: &test::TestStepDescription,
tests: &IndexMap<usize, LspTestDescription>,
) -> 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),
};
}
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<&test::TestStepDescription> for TestOrTestStepDescription {
fn from(desc: &test::TestStepDescription) -> Self {
Self::TestStepDescription(desc.clone())
}
}
impl From<&TestOrTestStepDescription> for lsp_custom::TestIdentifier {
fn from(desc: &TestOrTestStepDescription) -> lsp_custom::TestIdentifier {
impl From<&LspTestDescription> for lsp_custom::TestIdentifier {
fn from(desc: &LspTestDescription) -> lsp_custom::TestIdentifier {
match desc {
TestOrTestStepDescription::TestDescription(test_desc) => test_desc.into(),
TestOrTestStepDescription::TestStepDescription(test_step_desc) => {
test_step_desc.into()
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<&TestOrTestStepDescription> for lsp_custom::TestData {
fn from(desc: &TestOrTestStepDescription) -> Self {
impl From<&LspTestDescription> for lsp_custom::TestData {
fn from(desc: &LspTestDescription) -> Self {
match desc {
TestOrTestStepDescription::TestDescription(desc) => desc.into(),
TestOrTestStepDescription::TestStepDescription(desc) => desc.into(),
LspTestDescription::TestDescription(d) => d.into(),
LspTestDescription::TestStepDescription(d, static_id, _) => Self {
id: static_id.clone(),
label: d.name.clone(),
steps: Default::default(),
range: None,
},
}
}
}
@ -546,37 +594,12 @@ impl From<&test::TestDescription> for lsp_custom::TestIdentifier {
}
}
impl From<&test::TestStepDescription> for lsp_custom::TestData {
fn from(desc: &test::TestStepDescription) -> Self {
Self {
id: desc.static_id(),
label: desc.name.clone(),
steps: Default::default(),
range: None,
}
}
}
impl From<&test::TestStepDescription> for lsp_custom::TestIdentifier {
fn from(desc: &test::TestStepDescription) -> Self {
let uri = ModuleSpecifier::parse(&desc.origin).unwrap();
Self {
text_document: lsp::TextDocumentIdentifier { uri },
id: Some(checksum::gen(&[
desc.origin.as_bytes(),
desc.root_name.as_bytes(),
])),
step_id: Some(desc.static_id()),
}
}
}
struct LspTestReporter {
client: Client,
id: u32,
maybe_root_uri: Option<ModuleSpecifier>,
files: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
tests: IndexMap<usize, TestOrTestStepDescription>,
tests: IndexMap<usize, LspTestDescription>,
current_test: Option<usize>,
}
@ -713,26 +736,27 @@ impl LspTestReporter {
}
fn report_step_register(&mut self, desc: &test::TestStepDescription) {
self.tests.insert(desc.id, desc.into());
self.tests.insert(
desc.id,
LspTestDescription::from_step_description(desc, &self.tests),
);
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())
.entry(ModuleSpecifier::parse(&desc.location().file_name).unwrap())
.or_default();
let data: lsp_custom::TestData = desc.into();
if tds.inject(data.clone()) {
let mut step_desc = Some(desc);
let mut data = data;
while let Some(desc) = step_desc {
let parent_desc = self.tests.get(&desc.parent_id).unwrap();
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;
step_desc = match parent_desc {
TestOrTestStepDescription::TestDescription(_) => None,
TestOrTestStepDescription::TestStepDescription(d) => Some(d),
current_desc = parent_desc;
}
}
let specifier = ModuleSpecifier::parse(&desc.origin).unwrap();
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 {
@ -758,6 +782,7 @@ 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();
self.progress(lsp_custom::TestRunProgressMessage::Started { test });
}
@ -771,6 +796,7 @@ 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);
match result {
test::TestStepResult::Ok => {
self.progress(lsp_custom::TestRunProgressMessage::Passed {
@ -848,7 +874,6 @@ mod tests {
let test_def_a = TestDefinition {
id: "0b7c6bf3cd617018d33a1bf982a08fe088c5bb54fcd5eb9e802e7c137ec1af94"
.to_string(),
level: 0,
name: "test a".to_string(),
range: new_range(420, 424),
steps: vec![],
@ -856,7 +881,6 @@ mod tests {
let test_def_b = TestDefinition {
id: "69d9fe87f64f5b66cb8b631d4fd2064e8224b8715a049be54276c42189ff8f9f"
.to_string(),
level: 0,
name: "test b".to_string(),
range: new_range(480, 481),
steps: vec![],

10
cli/tools/test/mod.rs
View file

@ -297,16 +297,6 @@ pub struct TestStepDescription {
pub root_name: String,
}
impl TestStepDescription {
pub fn static_id(&self) -> String {
checksum::gen(&[
self.location.file_name.as_bytes(),
&self.level.to_be_bytes(),
self.name.as_bytes(),
])
}
}
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Debug, Clone, PartialEq, Deserialize)]
#[serde(rename_all = "camelCase")]