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<TestDefinition>,
}

pub struct TestDefinitions {
  pub discovered: Vec<TestDefinition>,
  pub injected: Vec<lsp_custom::TestData>,
  pub script_version: String,
}
```
Now:
```rust
pub struct TestDefinition {
  pub id: String,
  pub name: String,
  pub range: Option<Range>,
  pub is_dynamic: bool, // True for 'injected' module, not statically detected but added at runtime.
  pub parent_id: Option<String>,
  pub step_ids: HashSet<String>,
}

pub struct TestModule {
  pub specifier: ModuleSpecifier,
  pub script_version: String,
  pub defs: HashMap<String, TestDefinition>,
}
```

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:
61f08d5a71/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/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 range: Option<Range>,
-  pub steps: Vec<TestDefinition>,
+  pub is_dynamic: bool,
+  pub parent_id: Option<String>,
+  pub step_ids: HashSet<String>,
 }
 
-impl TestDefinition {
+#[derive(Debug, Clone, PartialEq)]
-  pub fn new(
+pub struct TestModule {
-    specifier: &ModuleSpecifier,
+  pub specifier: ModuleSpecifier,
-    name: String,
-    range: SourceRange,
-    mut steps: Vec<TestDefinition>,
-  ) -> 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<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,
-    steps: Vec<TestDefinition>,
-  ) -> 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<S: AsRef<str>>(&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<TestDefinition>,
-  /// Tests and steps which the test runner notified us of, which were
-  /// dynamically added
-  pub injected: Vec<lsp_custom::TestData>,
   /// The version of the document that the discovered tests relate to.
   pub script_version: String,
+  pub defs: HashMap<String, TestDefinition>,
 }
 
-impl Default for TestDefinitions {
+impl TestModule {
-  fn default() -> Self {
+  pub fn new(specifier: ModuleSpecifier, script_version: String) -> Self {
-    TestDefinitions {
+    Self {
-      script_version: "1".to_string(),
+      specifier,
-      discovered: vec![],
+      script_version,
-      injected: vec![],
+      defs: Default::default(),
     }
   }
-}
 
-impl TestDefinitions {
+  /// Returns `(id, is_newly_registered)`.
+  pub fn register(
+    &mut self,
+    name: String,
+    range: Option<Range>,
+    is_dynamic: bool,
+    parent_id: Option<String>,
+  ) -> (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("<unknown>".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_uri: Option<&ModuleSpecifier>,
-    maybe_root: Option<&ModuleSpecifier>,
-    source_text_info: &SourceTextInfo,
   ) -> TestingNotification {
-    let label = if let Some(root) = maybe_root {
+    let label = self.label(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())
-    };
-    let mut tests_map: HashMap<String, lsp_custom::TestData> = 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)
-    }));
     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
+  pub fn label(&self, maybe_root_uri: Option<&ModuleSpecifier>) -> String {
-  /// same static id was already registered statically or dynamically. Otherwise
+    if let Some(root) = maybe_root_uri {
-  /// returns true.
+      self.specifier.as_str().replace(root.as_str(), "")
-  pub fn inject(&mut self, data: lsp_custom::TestData) -> bool {
+    } else {
-    if self.discovered.iter().any(|td| td.contains_id(&data.id))
+      self
-      || self.injected.iter().any(|td| td.id == data.id)
+        .specifier
-    {
+        .path_segments()
-      return false;
+        .and_then(|s| s.last().map(|s| s.to_string()))
+        .unwrap_or_else(|| "<unknown>".to_string())
     }
-    self.injected.push(data);
-    true
-  }
-
-  /// Return a test definition identified by the test ID.
-  pub fn get_by_id<S: AsRef<str>>(&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()
   }
 }

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<ModuleSpecifier, TestDefinitions>,
+  tests: &HashMap<ModuleSpecifier, TestModule>,
 ) -> (
   HashSet<ModuleSpecifier>,
   HashMap<ModuleSpecifier, LspTestFilter>,
@@ -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<String> {
+  fn as_ids(&self, test_module: &TestModule) -> Vec<String> {
     let ids: Vec<String> = if let Some(include) = &self.include {
       include.keys().cloned().collect()
     } else {
-      test_definitions
+      test_module
-        .discovered
+        .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<ModuleSpecifier, LspTestFilter>,
   queue: HashSet<ModuleSpecifier>,
-  tests: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
+  tests: Arc<Mutex<HashMap<ModuleSpecifier, TestModule>>>,
   token: CancellationToken,
   workspace_settings: config::WorkspaceSettings,
 }
@@ -156,7 +156,7 @@ pub struct TestRun {
 impl TestRun {
   pub fn new(
     params: &lsp_custom::TestRunRequestParams,
-    tests: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
+    tests: Arc<Mutex<HashMap<ModuleSpecifier, TestModule>>>,
     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
+            LspTestFilter::default().as_ids(test_module)
-              .discovered
-              .iter()
-              .map(|test| test.id.clone())
-              .collect()
           }
         } else {
           Vec::new()
@@ -479,117 +475,60 @@ impl TestRun {
 
 #[derive(Debug, PartialEq)]
 enum LspTestDescription {
-  TestDescription(test::TestDescription),
+  /// `(desc, static_id)`
-  /// `(desc, static_id, root_static_id)`
+  TestDescription(test::TestDescription, String),
-  TestStepDescription(test::TestStepDescription, String, String),
+  /// `(desc, static_id)`
+  TestStepDescription(test::TestStepDescription, String),
 }
 
 impl LspTestDescription {
   fn origin(&self) -> &str {
     match self {
-      LspTestDescription::TestDescription(d) => d.origin.as_str(),
+      LspTestDescription::TestDescription(d, _) => d.origin.as_str(),
-      LspTestDescription::TestStepDescription(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::TestDescription(d, _) => &d.location,
-      LspTestDescription::TestStepDescription(d, _, _) => &d.location,
+      LspTestDescription::TestStepDescription(d, _) => &d.location,
     }
   }
 
   fn parent_id(&self) -> Option<usize> {
     match self {
-      LspTestDescription::TestDescription(_) => None,
+      LspTestDescription::TestDescription(_, _) => None,
-      LspTestDescription::TestStepDescription(d, _, _) => Some(d.parent_id),
+      LspTestDescription::TestStepDescription(d, _) => Some(d.parent_id),
     }
   }
 
-  fn from_step_description(
+  fn static_id(&self) -> &str {
-    desc: &test::TestStepDescription,
+    match self {
+      LspTestDescription::TestDescription(_, i) => i,
+      LspTestDescription::TestStepDescription(_, i) => i,
+    }
+  }
+
+  fn as_test_identifier(
+    &self,
     tests: &IndexMap<usize, LspTestDescription>,
-  ) -> Self {
+  ) -> lsp_custom::TestIdentifier {
-    let mut id_components = Vec::with_capacity(7);
+    let uri = ModuleSpecifier::parse(&self.location().file_name).unwrap();
-    let mut root_static_id = None;
+    let static_id = self.static_id();
-    let mut step_desc = Some(desc);
+    let mut root_desc = self;
-    while let Some(desc) = step_desc {
+    while let Some(parent_id) = root_desc.parent_id() {
-      id_components.push(desc.name.as_bytes());
+      root_desc = tests.get(&parent_id).unwrap();
-      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());
+    let root_static_id = root_desc.static_id();
-    id_components.reverse();
+    lsp_custom::TestIdentifier {
-    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 {
       text_document: lsp::TextDocumentIdentifier { uri },
-      id: Some(desc.static_id()),
+      id: Some(root_static_id.to_string()),
-      step_id: None,
+      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<ModuleSpecifier>,
-  files: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
+  files: Arc<Mutex<HashMap<ModuleSpecifier, TestModule>>>,
   tests: IndexMap<usize, LspTestDescription>,
   current_test: Option<usize>,
 }
@@ -608,7 +547,7 @@ impl LspTestReporter {
     run: &TestRun,
     client: Client,
     maybe_root_uri: Option<&ModuleSpecifier>,
-    files: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
+    files: Arc<Mutex<HashMap<ModuleSpecifier, TestModule>>>,
   ) -> 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
+    let specifier = ModuleSpecifier::parse(&desc.location.file_name).unwrap();
-      .entry(ModuleSpecifier::parse(&desc.location.file_name).unwrap())
+    let test_module = files
-      .or_default();
+      .entry(specifier.clone())
-    if tds.inject(desc.into()) {
+      .or_insert_with(|| TestModule::new(specifier, "1".to_string()));
-      let specifier = ModuleSpecifier::parse(&desc.origin).unwrap();
+    let (static_id, is_new) = test_module.register_dynamic(desc);
-      let label = if let Some(root) = &self.maybe_root_uri {
+    self.tests.insert(
-        specifier.as_str().replace(root.as_str(), "")
+      desc.id,
-      } else {
+      LspTestDescription::TestDescription(desc.clone(), static_id.clone()),
-        specifier
+    );
-          .path_segments()
+    if is_new {
-          .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 },
+            text_document: lsp::TextDocumentIdentifier {
+              uri: test_module.specifier.clone(),
+            },
             kind: lsp_custom::TestModuleNotificationKind::Insert,
-            label,
+            label: test_module.label(self.maybe_root_uri.as_ref()),
-            tests: vec![desc.into()],
+            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();
+    if is_new {
-    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(|| "<unknown>".to_string())
-      };
       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,
+            label: test_module.label(self.maybe_root_uri.as_ref()),
-            tests: vec![data],
+            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 desc = self.tests.get(&desc.id).unwrap();
-    let test: lsp_custom::TestIdentifier = desc.into();
+    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),
+      range: Some(new_range(1, 5, 1, 9)),
-      steps: vec![],
+      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),
+      range: Some(new_range(2, 5, 2, 9)),
-      steps: vec![],
+      is_dynamic: false,
+      parent_id: None,
+      step_ids: Default::default(),
     };
-    let test_definitions = TestDefinitions {
+    let test_module = TestModule {
-      discovered: vec![test_def_a, test_def_b.clone()],
+      specifier: specifier.clone(),
-      injected: vec![],
       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(specifier.clone(), test_module.clone());
-    tests.insert(non_test_specifier, Default::default());
+    tests.insert(
+      non_test_specifier.clone(),
+      TestModule::new(non_test_specifier, "1".to_string()),
+    );
     let (queue, filters) = as_queue_and_filters(&params, &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()

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<Mutex<HashMap<u32, TestRun>>>,
   /// Tests that are discovered from a versioned document
-  tests: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>>,
+  tests: Arc<Mutex<HashMap<ModuleSpecifier, TestModule>>>,
   /// A channel for requesting that changes to documents be statically analyzed
   /// for tests
   update_channel: mpsc::UnboundedSender<Arc<StateSnapshot>>,
@@ -59,7 +59,7 @@ impl TestServer {
     performance: Arc<Performance>,
     maybe_root_uri: Option<ModuleSpecifier>,
   ) -> Self {
-    let tests: Arc<Mutex<HashMap<ModuleSpecifier, TestDefinitions>>> =
+    let tests: Arc<Mutex<HashMap<ModuleSpecifier, TestModule>>> =
       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 {
+                    let test_module = collector.take();
-                      discovered: collector.take(),
+                    if !test_module.is_empty() {
-                      injected: Default::default(),
-                      script_version,
-                    };
-                    if !test_definitions.discovered.is_empty() {
                       client.send_test_notification(
-                        test_definitions.as_notification(
+                        test_module.as_replace_notification(mru.as_ref()),
-                          specifier,
-                          mru.as_ref(),
-                          parsed_source.text_info(),
-                        ),
                       );
-                    } 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);
                   }
                 }
               }

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 {