refactor: allocate IDs for tests (#14729)

1 files changed, 167 insertions, 220 deletions

diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs
index d5c0f6278..7c0552a0a 100644
--- a/cli/lsp/testing/execution.rs
+++ b/cli/lsp/testing/execution.rs
@@ -25,13 +25,13 @@ use deno_core::futures::future;
 use deno_core::futures::stream;
 use deno_core::futures::StreamExt;
 use deno_core::parking_lot::Mutex;
-use deno_core::serde_json::json;
-use deno_core::serde_json::Value;
+use deno_core::parking_lot::RwLock;
 use deno_core::ModuleSpecifier;
 use deno_runtime::ops::io::Stdio;
 use deno_runtime::ops::io::StdioPipe;
 use deno_runtime::permissions::Permissions;
 use deno_runtime::tokio_util::run_local;
+use indexmap::IndexMap;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::sync::Arc;
@@ -48,10 +48,10 @@ fn as_queue_and_filters(
   tests: &HashMap<ModuleSpecifier, TestDefinitions>,
 ) -> (
   HashSet<ModuleSpecifier>,
-  HashMap<ModuleSpecifier, TestFilter>,
+  HashMap<ModuleSpecifier, LspTestFilter>,
 ) {
   let mut queue: HashSet<ModuleSpecifier> = HashSet::new();
-  let mut filters: HashMap<ModuleSpecifier, TestFilter> = HashMap::new();
+  let mut filters: HashMap<ModuleSpecifier, LspTestFilter> = HashMap::new();
 
   if let Some(include) = &params.include {
     for item in include {
@@ -61,12 +61,12 @@ fn as_queue_and_filters(
           if let Some(test) = test_definitions.get_by_id(id) {
             let filter =
               filters.entry(item.text_document.uri.clone()).or_default();
-            if let Some(include) = filter.maybe_include.as_mut() {
+            if let Some(include) = filter.include.as_mut() {
               include.insert(test.id.clone(), test.clone());
             } else {
               let mut include = HashMap::new();
               include.insert(test.id.clone(), test.clone());
-              filter.maybe_include = Some(include);
+              filter.include = Some(include);
             }
           }
         }
@@ -80,29 +80,20 @@ fn as_queue_and_filters(
     queue.extend(tests.keys().cloned());
   }
 
-  if let Some(exclude) = &params.exclude {
-    for item in exclude {
-      if let Some(test_definitions) = tests.get(&item.text_document.uri) {
-        if let Some(id) = &item.id {
-          // there is currently no way to filter out a specific test, so we have
-          // to ignore the exclusion
-          if item.step_id.is_none() {
-            if let Some(test) = test_definitions.get_by_id(id) {
-              let filter =
-                filters.entry(item.text_document.uri.clone()).or_default();
-              if let Some(exclude) = filter.maybe_exclude.as_mut() {
-                exclude.insert(test.id.clone(), test.clone());
-              } else {
-                let mut exclude = HashMap::new();
-                exclude.insert(test.id.clone(), test.clone());
-                filter.maybe_exclude = Some(exclude);
-              }
-            }
+  for item in &params.exclude {
+    if let Some(test_definitions) = tests.get(&item.text_document.uri) {
+      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) {
+            let filter =
+              filters.entry(item.text_document.uri.clone()).or_default();
+            filter.exclude.insert(test.id.clone(), test.clone());
           }
-        } else {
-          // the entire test module is excluded
-          queue.remove(&item.text_document.uri);
         }
+      } else {
+        // the entire test module is excluded
+        queue.remove(&item.text_document.uri);
       }
     }
   }
@@ -131,14 +122,14 @@ fn as_test_messages<S: AsRef<str>>(
 }
 
 #[derive(Debug, Clone, Default, PartialEq)]
-struct TestFilter {
-  maybe_include: Option<HashMap<String, TestDefinition>>,
-  maybe_exclude: Option<HashMap<String, TestDefinition>>,
+struct LspTestFilter {
+  include: Option<HashMap<String, TestDefinition>>,
+  exclude: HashMap<String, TestDefinition>,
 }
 
-impl TestFilter {
+impl LspTestFilter {
   fn as_ids(&self, test_definitions: &TestDefinitions) -> Vec<String> {
-    let ids: Vec<String> = if let Some(include) = &self.maybe_include {
+    let ids: Vec<String> = if let Some(include) = &self.include {
       include.keys().cloned().collect()
     } else {
       test_definitions
@@ -147,33 +138,10 @@ impl TestFilter {
         .map(|td| td.id.clone())
         .collect()
     };
-    if let Some(exclude) = &self.maybe_exclude {
-      ids
-        .into_iter()
-        .filter(|id| !exclude.contains_key(id))
-        .collect()
-    } else {
-      ids
-    }
-  }
-
-  /// return the filter as a JSON value, suitable for sending as a filter to the
-  /// test runner.
-  fn as_test_options(&self) -> Value {
-    let maybe_include: Option<Vec<String>> = self
-      .maybe_include
-      .as_ref()
-      .map(|inc| inc.iter().map(|(_, td)| td.name.clone()).collect());
-    let maybe_exclude: Option<Vec<String>> = self
-      .maybe_exclude
-      .as_ref()
-      .map(|ex| ex.iter().map(|(_, td)| td.name.clone()).collect());
-    json!({
-      "filter": {
-        "include": maybe_include,
-        "exclude": maybe_exclude,
-      }
-    })
+    ids
+      .into_iter()
+      .filter(|id| !self.exclude.contains_key(id))
+      .collect()
   }
 }
 
@@ -184,14 +152,14 @@ async fn test_specifier(
   mode: test::TestMode,
   sender: &TestEventSender,
   token: CancellationToken,
-  options: Option<Value>,
+  filter: test::TestFilter,
 ) -> Result<(), AnyError> {
   if !token.is_cancelled() {
     let mut worker = create_main_worker(
       &ps,
       specifier.clone(),
       permissions,
-      vec![ops::testing::init(sender.clone())],
+      vec![ops::testing::init(sender.clone(), filter)],
       Stdio {
         stdin: StdioPipe::Inherit,
         stdout: StdioPipe::File(sender.stdout()),
@@ -217,10 +185,9 @@ async fn test_specifier(
 
     worker.dispatch_load_event(&located_script_name!())?;
 
-    let options = options.unwrap_or_else(|| json!({}));
     let test_result = worker.js_runtime.execute_script(
       &located_script_name!(),
-      &format!(r#"Deno[Deno.internal].runTests({})"#, json!(options)),
+      r#"Deno[Deno.internal].runTests()"#,
     )?;
 
     worker.js_runtime.resolve_value(test_result).await?;
@@ -241,7 +208,7 @@ async fn test_specifier(
 pub struct TestRun {
   id: u32,
   kind: lsp_custom::TestRunKind,
-  filters: HashMap<ModuleSpecifier, TestFilter>,
+  filters: HashMap<ModuleSpecifier, LspTestFilter>,
   queue: HashSet<ModuleSpecifier>,
   tests: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
   token: CancellationToken,
@@ -343,13 +310,31 @@ impl TestRun {
     let mut queue = self.queue.iter().collect::<Vec<&ModuleSpecifier>>();
     queue.sort();
 
+    let tests: Arc<RwLock<IndexMap<usize, test::TestDescription>>> =
+      Arc::new(RwLock::new(IndexMap::new()));
+    let mut test_steps = IndexMap::new();
+
+    let tests_ = tests.clone();
     let join_handles = queue.into_iter().map(move |specifier| {
       let specifier = specifier.clone();
       let ps = ps.clone();
       let permissions = permissions.clone();
       let mut sender = sender.clone();
-      let options = self.filters.get(&specifier).map(|f| f.as_test_options());
+      let lsp_filter = self.filters.get(&specifier);
+      let filter = test::TestFilter {
+        substring: None,
+        regex: None,
+        include: lsp_filter.and_then(|f| {
+          f.include
+            .as_ref()
+            .map(|i| i.values().map(|t| t.name.clone()).collect())
+        }),
+        exclude: lsp_filter
+          .map(|f| f.exclude.values().map(|t| t.name.clone()).collect())
+          .unwrap_or_default(),
+      };
       let token = self.token.clone();
+      let tests = tests_.clone();
 
       tokio::task::spawn_blocking(move || {
         let origin = specifier.to_string();
@@ -360,14 +345,23 @@ impl TestRun {
           test::TestMode::Executable,
           &sender,
           token,
-          options,
+          filter,
         ));
         if let Err(error) = file_result {
           if error.is::<JsError>() {
             sender.send(test::TestEvent::UncaughtError(
-              origin,
+              origin.clone(),
               Box::new(error.downcast::<JsError>().unwrap()),
             ))?;
+            for desc in tests.read().values() {
+              if desc.origin == origin {
+                sender.send(test::TestEvent::Result(
+                  desc.id,
+                  test::TestResult::Cancelled,
+                  0,
+                ))?
+              }
+            }
           } else {
             return Err(error);
           }
@@ -396,6 +390,10 @@ impl TestRun {
 
         while let Some(event) = receiver.recv().await {
           match event {
+            test::TestEvent::Register(description) => {
+              reporter.report_register(&description);
+              tests.write().insert(description.id, description);
+            }
             test::TestEvent::Plan(plan) => {
               summary.total += plan.total;
               summary.filtered_out += plan.filtered_out;
@@ -406,13 +404,14 @@ impl TestRun {
 
               reporter.report_plan(&plan);
             }
-            test::TestEvent::Wait(description) => {
-              reporter.report_wait(&description);
+            test::TestEvent::Wait(id) => {
+              reporter.report_wait(tests.read().get(&id).unwrap());
             }
             test::TestEvent::Output(output) => {
               reporter.report_output(&output);
             }
-            test::TestEvent::Result(description, result, elapsed) => {
+            test::TestEvent::Result(id, result, elapsed) => {
+              let description = tests.read().get(&id).unwrap().clone();
               match &result {
                 test::TestResult::Ok => summary.passed += 1,
                 test::TestResult::Ignored => summary.ignored += 1,
@@ -420,6 +419,9 @@ impl TestRun {
                   summary.failed += 1;
                   summary.failures.push((description.clone(), error.clone()));
                 }
+                test::TestResult::Cancelled => {
+                  summary.failed += 1;
+                }
               }
 
               reporter.report_result(&description, &result, elapsed);
@@ -429,10 +431,14 @@ impl TestRun {
               summary.failed += 1;
               summary.uncaught_errors.push((origin, error));
             }
-            test::TestEvent::StepWait(description) => {
-              reporter.report_step_wait(&description);
+            test::TestEvent::StepRegister(description) => {
+              reporter.report_step_register(&description);
+              test_steps.insert(description.id, description);
             }
-            test::TestEvent::StepResult(description, result, duration) => {
+            test::TestEvent::StepWait(id) => {
+              reporter.report_step_wait(test_steps.get(&id).unwrap());
+            }
+            test::TestEvent::StepResult(id, result, duration) => {
               match &result {
                 test::TestStepResult::Ok => {
                   summary.passed_steps += 1;
@@ -447,7 +453,11 @@ impl TestRun {
                   summary.pending_steps += 1;
                 }
               }
-              reporter.report_step_result(&description, &result, duration);
+              reporter.report_step_result(
+                test_steps.get(&id).unwrap(),
+                &result,
+                duration,
+              );
             }
           }
 
@@ -562,10 +572,8 @@ impl From<&TestOrTestStepDescription> for lsp_custom::TestData {
 
 impl From<&test::TestDescription> for lsp_custom::TestData {
   fn from(desc: &test::TestDescription) -> Self {
-    let id = checksum::gen(&[desc.origin.as_bytes(), desc.name.as_bytes()]);
-
     Self {
-      id,
+      id: desc.static_id(),
       label: desc.name.clone(),
       steps: Default::default(),
       range: None,
@@ -576,14 +584,9 @@ impl From<&test::TestDescription> for lsp_custom::TestData {
 impl From<&test::TestDescription> for lsp_custom::TestIdentifier {
   fn from(desc: &test::TestDescription) -> Self {
     let uri = ModuleSpecifier::parse(&desc.origin).unwrap();
-    let id = Some(checksum::gen(&[
-      desc.origin.as_bytes(),
-      desc.name.as_bytes(),
-    ]));
-
     Self {
       text_document: lsp::TextDocumentIdentifier { uri },
-      id,
+      id: Some(desc.static_id()),
       step_id: None,
     }
   }
@@ -591,14 +594,8 @@ impl From<&test::TestDescription> for lsp_custom::TestIdentifier {
 
 impl From<&test::TestStepDescription> for lsp_custom::TestData {
   fn from(desc: &test::TestStepDescription) -> Self {
-    let id = checksum::gen(&[
-      desc.test.origin.as_bytes(),
-      &desc.level.to_be_bytes(),
-      desc.name.as_bytes(),
-    ]);
-
     Self {
-      id,
+      id: desc.static_id(),
       label: desc.name.clone(),
       steps: Default::default(),
       range: None,
@@ -608,21 +605,14 @@ impl From<&test::TestStepDescription> for lsp_custom::TestData {
 
 impl From<&test::TestStepDescription> for lsp_custom::TestIdentifier {
   fn from(desc: &test::TestStepDescription) -> Self {
-    let uri = ModuleSpecifier::parse(&desc.test.origin).unwrap();
-    let id = Some(checksum::gen(&[
-      desc.test.origin.as_bytes(),
-      desc.test.name.as_bytes(),
-    ]));
-    let step_id = Some(checksum::gen(&[
-      desc.test.origin.as_bytes(),
-      &desc.level.to_be_bytes(),
-      desc.name.as_bytes(),
-    ]));
-
+    let uri = ModuleSpecifier::parse(&desc.origin).unwrap();
     Self {
       text_document: lsp::TextDocumentIdentifier { uri },
-      id,
-      step_id,
+      id: Some(checksum::gen(&[
+        desc.origin.as_bytes(),
+        desc.root_name.as_bytes(),
+      ])),
+      step_id: Some(desc.static_id()),
     }
   }
 }
@@ -653,61 +643,28 @@ impl LspTestReporter {
     }
   }
 
-  fn add_step(&self, desc: &test::TestStepDescription) {
-    if let Ok(specifier) = ModuleSpecifier::parse(&desc.test.origin) {
-      let mut tests = self.tests.lock();
-      let entry =
-        tests
-          .entry(specifier.clone())
-          .or_insert_with(|| TestDefinitions {
-            discovered: Default::default(),
-            injected: Default::default(),
-            script_version: "1".to_string(),
-          });
-      let mut prev: lsp_custom::TestData = desc.into();
-      if let Some(stack) = self.stack.get(&desc.test.origin) {
-        for item in stack.iter().rev() {
-          let mut data: lsp_custom::TestData = item.into();
-          data.steps = Some(vec![prev]);
-          prev = data;
-        }
-        entry.injected.push(prev.clone());
-        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(|| "<unknown>".to_string())
-        };
-        self
-          .client
-          .send_test_notification(TestingNotification::Module(
-            lsp_custom::TestModuleNotificationParams {
-              text_document: lsp::TextDocumentIdentifier { uri: specifier },
-              kind: lsp_custom::TestModuleNotificationKind::Insert,
-              label,
-              tests: vec![prev],
-            },
-          ));
-      }
-    }
+  fn progress(&self, message: lsp_custom::TestRunProgressMessage) {
+    self
+      .client
+      .send_test_notification(TestingNotification::Progress(
+        lsp_custom::TestRunProgressParams {
+          id: self.id,
+          message,
+        },
+      ));
   }
+}
 
-  /// Add a test which is being reported from the test runner but was not
-  /// statically identified
-  fn add_test(&self, desc: &test::TestDescription) {
-    if let Ok(specifier) = ModuleSpecifier::parse(&desc.origin) {
-      let mut tests = self.tests.lock();
-      let entry =
-        tests
-          .entry(specifier.clone())
-          .or_insert_with(|| TestDefinitions {
-            discovered: Default::default(),
-            injected: Default::default(),
-            script_version: "1".to_string(),
-          });
-      entry.injected.push(desc.into());
+impl test::TestReporter for LspTestReporter {
+  fn report_plan(&mut self, _plan: &test::TestPlan) {}
+
+  fn report_register(&mut self, desc: &test::TestDescription) {
+    let mut tests = self.tests.lock();
+    let tds = tests
+      .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 {
@@ -729,49 +686,7 @@ impl LspTestReporter {
     }
   }
 
-  fn progress(&self, message: lsp_custom::TestRunProgressMessage) {
-    self
-      .client
-      .send_test_notification(TestingNotification::Progress(
-        lsp_custom::TestRunProgressParams {
-          id: self.id,
-          message,
-        },
-      ));
-  }
-
-  fn includes_step(&self, desc: &test::TestStepDescription) -> bool {
-    if let Ok(specifier) = ModuleSpecifier::parse(&desc.test.origin) {
-      let tests = self.tests.lock();
-      if let Some(test_definitions) = tests.get(&specifier) {
-        return test_definitions
-          .get_step_by_name(&desc.test.name, desc.level, &desc.name)
-          .is_some();
-      }
-    }
-    false
-  }
-
-  fn includes_test(&self, desc: &test::TestDescription) -> bool {
-    if let Ok(specifier) = ModuleSpecifier::parse(&desc.origin) {
-      let tests = self.tests.lock();
-      if let Some(test_definitions) = tests.get(&specifier) {
-        return test_definitions.get_by_name(&desc.name).is_some();
-      }
-    }
-    false
-  }
-}
-
-impl test::TestReporter for LspTestReporter {
-  fn report_plan(&mut self, _plan: &test::TestPlan) {
-    // there is nothing to do on report_plan
-  }
-
   fn report_wait(&mut self, desc: &test::TestDescription) {
-    if !self.includes_test(desc) {
-      self.add_test(desc);
-    }
     self.current_origin = Some(desc.origin.clone());
     let test: lsp_custom::TestIdentifier = desc.into();
     let stack = self.stack.entry(desc.origin.clone()).or_default();
@@ -827,6 +742,13 @@ impl test::TestReporter for LspTestReporter {
           duration: Some(elapsed as u32),
         })
       }
+      test::TestResult::Cancelled => {
+        self.progress(lsp_custom::TestRunProgressMessage::Failed {
+          test: desc.into(),
+          messages: vec![],
+          duration: Some(elapsed as u32),
+        })
+      }
     }
   }
 
@@ -861,13 +783,46 @@ impl test::TestReporter for LspTestReporter {
     }
   }
 
-  fn report_step_wait(&mut self, desc: &test::TestStepDescription) {
-    if !self.includes_step(desc) {
-      self.add_step(desc);
+  fn report_step_register(&mut self, desc: &test::TestStepDescription) {
+    let mut tests = self.tests.lock();
+    let tds = tests
+      .entry(ModuleSpecifier::parse(&desc.location.file_name).unwrap())
+      .or_default();
+    if tds.inject(desc.into()) {
+      let specifier = ModuleSpecifier::parse(&desc.origin).unwrap();
+      let mut prev: lsp_custom::TestData = desc.into();
+      if let Some(stack) = self.stack.get(&desc.origin) {
+        for item in stack.iter().rev() {
+          let mut data: lsp_custom::TestData = item.into();
+          data.steps = vec![prev];
+          prev = data;
+        }
+        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(|| "<unknown>".to_string())
+        };
+        self
+          .client
+          .send_test_notification(TestingNotification::Module(
+            lsp_custom::TestModuleNotificationParams {
+              text_document: lsp::TextDocumentIdentifier { uri: specifier },
+              kind: lsp_custom::TestModuleNotificationKind::Insert,
+              label,
+              tests: vec![prev],
+            },
+          ));
+      }
     }
+  }
+
+  fn report_step_wait(&mut self, desc: &test::TestStepDescription) {
     let test: lsp_custom::TestIdentifier = desc.into();
-    let stack = self.stack.entry(desc.test.origin.clone()).or_default();
-    self.current_origin = Some(desc.test.origin.clone());
+    let stack = self.stack.entry(desc.origin.clone()).or_default();
+    self.current_origin = Some(desc.origin.clone());
     assert!(!stack.is_empty());
     stack.push(desc.into());
     self.progress(lsp_custom::TestRunProgressMessage::Started { test });
@@ -879,7 +834,7 @@ impl test::TestReporter for LspTestReporter {
     result: &test::TestStepResult,
     elapsed: u64,
   ) {
-    let stack = self.stack.entry(desc.test.origin.clone()).or_default();
+    let stack = self.stack.entry(desc.origin.clone()).or_default();
     assert_eq!(stack.pop(), Some(desc.into()));
     match result {
       test::TestStepResult::Ok => {
@@ -927,6 +882,7 @@ impl test::TestReporter for LspTestReporter {
 mod tests {
   use super::*;
   use crate::lsp::testing::collectors::tests::new_range;
+  use deno_core::serde_json::json;
 
   #[test]
   fn test_as_queue_and_filters() {
@@ -941,7 +897,7 @@ mod tests {
         id: None,
         step_id: None,
       }]),
-      exclude: Some(vec![lsp_custom::TestIdentifier {
+      exclude: vec![lsp_custom::TestIdentifier {
         text_document: lsp::TextDocumentIdentifier {
           uri: specifier.clone(),
         },
@@ -950,7 +906,7 @@ mod tests {
             .to_string(),
         ),
         step_id: None,
-      }]),
+      }],
     };
     let mut tests = HashMap::new();
     let test_def_a = TestDefinition {
@@ -959,7 +915,7 @@ mod tests {
       level: 0,
       name: "test a".to_string(),
       range: new_range(420, 424),
-      steps: None,
+      steps: vec![],
     };
     let test_def_b = TestDefinition {
       id: "69d9fe87f64f5b66cb8b631d4fd2064e8224b8715a049be54276c42189ff8f9f"
@@ -967,7 +923,7 @@ mod tests {
       level: 0,
       name: "test b".to_string(),
       range: new_range(480, 481),
-      steps: None,
+      steps: vec![],
     };
     let test_definitions = TestDefinitions {
       discovered: vec![test_def_a, test_def_b.clone()],
@@ -988,9 +944,9 @@ mod tests {
     let filter = maybe_filter.unwrap();
     assert_eq!(
       filter,
-      &TestFilter {
-        maybe_include: None,
-        maybe_exclude: Some(exclude),
+      &LspTestFilter {
+        include: None,
+        exclude,
       }
     );
     assert_eq!(
@@ -1000,14 +956,5 @@ mod tests {
           .to_string()
       ]
     );
-    assert_eq!(
-      filter.as_test_options(),
-      json!({
-        "filter": {
-          "include": null,
-          "exclude": vec!["test b"],
-        }
-      })
-    );
   }
 }