Revert "refactor(cli): use new sanitizer for resources (#22125)" (#22153)

9 files changed, 204 insertions, 239 deletions

diff --git a/cli/js/40_test.js b/cli/js/40_test.js
index 146163fdd..b6f966f20 100644
--- a/cli/js/40_test.js
+++ b/cli/js/40_test.js
@@ -15,8 +15,10 @@ const {
   MapPrototypeGet,
   MapPrototypeHas,
   MapPrototypeSet,
+  ObjectKeys,
   Promise,
   SafeArrayIterator,
+  Set,
   SymbolToStringTag,
   TypeError,
 } = primordials;
@@ -340,6 +342,182 @@ function assertOps(fn) {
   };
 }
 
+function prettyResourceNames(name) {
+  switch (name) {
+    case "fsFile":
+      return ["A file", "opened", "closed"];
+    case "fetchRequest":
+      return ["A fetch request", "started", "finished"];
+    case "fetchRequestBody":
+      return ["A fetch request body", "created", "closed"];
+    case "fetchResponse":
+      return ["A fetch response body", "created", "consumed"];
+    case "httpClient":
+      return ["An HTTP client", "created", "closed"];
+    case "dynamicLibrary":
+      return ["A dynamic library", "loaded", "unloaded"];
+    case "httpConn":
+      return ["An inbound HTTP connection", "accepted", "closed"];
+    case "httpStream":
+      return ["An inbound HTTP request", "accepted", "closed"];
+    case "tcpStream":
+      return ["A TCP connection", "opened/accepted", "closed"];
+    case "unixStream":
+      return ["A Unix connection", "opened/accepted", "closed"];
+    case "tlsStream":
+      return ["A TLS connection", "opened/accepted", "closed"];
+    case "tlsListener":
+      return ["A TLS listener", "opened", "closed"];
+    case "unixListener":
+      return ["A Unix listener", "opened", "closed"];
+    case "unixDatagram":
+      return ["A Unix datagram", "opened", "closed"];
+    case "tcpListener":
+      return ["A TCP listener", "opened", "closed"];
+    case "udpSocket":
+      return ["A UDP socket", "opened", "closed"];
+    case "timer":
+      return ["A timer", "started", "fired/cleared"];
+    case "textDecoder":
+      return ["A text decoder", "created", "finished"];
+    case "messagePort":
+      return ["A message port", "created", "closed"];
+    case "webSocketStream":
+      return ["A WebSocket", "opened", "closed"];
+    case "fsEvents":
+      return ["A file system watcher", "created", "closed"];
+    case "childStdin":
+      return ["A child process stdin", "opened", "closed"];
+    case "childStdout":
+      return ["A child process stdout", "opened", "closed"];
+    case "childStderr":
+      return ["A child process stderr", "opened", "closed"];
+    case "child":
+      return ["A child process", "started", "closed"];
+    case "signal":
+      return ["A signal listener", "created", "fired/cleared"];
+    case "stdin":
+      return ["The stdin pipe", "opened", "closed"];
+    case "stdout":
+      return ["The stdout pipe", "opened", "closed"];
+    case "stderr":
+      return ["The stderr pipe", "opened", "closed"];
+    case "compression":
+      return ["A CompressionStream", "created", "closed"];
+    default:
+      return [`A "${name}" resource`, "created", "cleaned up"];
+  }
+}
+
+function resourceCloseHint(name) {
+  switch (name) {
+    case "fsFile":
+      return "Close the file handle by calling `file.close()`.";
+    case "fetchRequest":
+      return "Await the promise returned from `fetch()` or abort the fetch with an abort signal.";
+    case "fetchRequestBody":
+      return "Terminate the request body `ReadableStream` by closing or erroring it.";
+    case "fetchResponse":
+      return "Consume or close the response body `ReadableStream`, e.g `await resp.text()` or `await resp.body.cancel()`.";
+    case "httpClient":
+      return "Close the HTTP client by calling `httpClient.close()`.";
+    case "dynamicLibrary":
+      return "Unload the dynamic library by calling `dynamicLibrary.close()`.";
+    case "httpConn":
+      return "Close the inbound HTTP connection by calling `httpConn.close()`.";
+    case "httpStream":
+      return "Close the inbound HTTP request by responding with `e.respondWith()` or closing the HTTP connection.";
+    case "tcpStream":
+      return "Close the TCP connection by calling `tcpConn.close()`.";
+    case "unixStream":
+      return "Close the Unix socket connection by calling `unixConn.close()`.";
+    case "tlsStream":
+      return "Close the TLS connection by calling `tlsConn.close()`.";
+    case "tlsListener":
+      return "Close the TLS listener by calling `tlsListener.close()`.";
+    case "unixListener":
+      return "Close the Unix socket listener by calling `unixListener.close()`.";
+    case "unixDatagram":
+      return "Close the Unix datagram socket by calling `unixDatagram.close()`.";
+    case "tcpListener":
+      return "Close the TCP listener by calling `tcpListener.close()`.";
+    case "udpSocket":
+      return "Close the UDP socket by calling `udpSocket.close()`.";
+    case "timer":
+      return "Clear the timer by calling `clearInterval` or `clearTimeout`.";
+    case "textDecoder":
+      return "Close the text decoder by calling `textDecoder.decode('')` or `await textDecoderStream.readable.cancel()`.";
+    case "messagePort":
+      return "Close the message port by calling `messagePort.close()`.";
+    case "webSocketStream":
+      return "Close the WebSocket by calling `webSocket.close()`.";
+    case "fsEvents":
+      return "Close the file system watcher by calling `watcher.close()`.";
+    case "childStdin":
+      return "Close the child process stdin by calling `proc.stdin.close()`.";
+    case "childStdout":
+      return "Close the child process stdout by calling `proc.stdout.close()` or `await child.stdout.cancel()`.";
+    case "childStderr":
+      return "Close the child process stderr by calling `proc.stderr.close()` or `await child.stderr.cancel()`.";
+    case "child":
+      return "Close the child process by calling `proc.kill()` or `proc.close()`.";
+    case "signal":
+      return "Clear the signal listener by calling `Deno.removeSignalListener`.";
+    case "stdin":
+      return "Close the stdin pipe by calling `Deno.stdin.close()`.";
+    case "stdout":
+      return "Close the stdout pipe by calling `Deno.stdout.close()`.";
+    case "stderr":
+      return "Close the stderr pipe by calling `Deno.stderr.close()`.";
+    case "compression":
+      return "Close the compression stream by calling `await stream.writable.close()`.";
+    default:
+      return "Close the resource before the end of the test.";
+  }
+}
+
+// Wrap test function in additional assertion that makes sure
+// the test case does not "leak" resources - ie. resource table after
+// the test has exactly the same contents as before the test.
+function assertResources(fn) {
+  /** @param desc {TestDescription | TestStepDescription} */
+  return async function resourceSanitizer(desc) {
+    const pre = core.resources();
+    const innerResult = await fn(desc);
+    if (innerResult) return innerResult;
+    const post = core.resources();
+
+    const allResources = new Set([
+      ...new SafeArrayIterator(ObjectKeys(pre)),
+      ...new SafeArrayIterator(ObjectKeys(post)),
+    ]);
+
+    const details = [];
+    for (const resource of allResources) {
+      const preResource = pre[resource];
+      const postResource = post[resource];
+      if (preResource === postResource) continue;
+
+      if (preResource === undefined) {
+        const [name, action1, action2] = prettyResourceNames(postResource);
+        const hint = resourceCloseHint(postResource);
+        const detail =
+          `${name} (rid ${resource}) was ${action1} during the test, but not ${action2} during the test. ${hint}`;
+        ArrayPrototypePush(details, detail);
+      } else {
+        const [name, action1, action2] = prettyResourceNames(preResource);
+        const detail =
+          `${name} (rid ${resource}) was ${action1} before the test started, but was ${action2} during the test. Do not close resources in a test that were not created during that test.`;
+        ArrayPrototypePush(details, detail);
+      }
+    }
+    if (details.length == 0) {
+      return null;
+    }
+    return { failed: { leakedResources: details } };
+  };
+}
+
 // Wrap test function in additional assertion that makes sure
 // that the test case does not accidentally exit prematurely.
 function assertExit(fn, isTest) {
@@ -542,8 +720,6 @@ function testInner(
     testDesc.name,
     testDesc.ignore,
     testDesc.only,
-    false, /*testDesc.sanitizeOps*/
-    testDesc.sanitizeResources,
     testDesc.location.fileName,
     testDesc.location.lineNumber,
     testDesc.location.columnNumber,
@@ -734,6 +910,9 @@ function wrapTest(desc) {
   if (desc.sanitizeOps) {
     testFn = assertOps(testFn);
   }
+  if (desc.sanitizeResources) {
+    testFn = assertResources(testFn);
+  }
   if (desc.sanitizeExit) {
     testFn = assertExit(testFn, true);
   }
diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs
index 11882e6af..69f218be8 100644
--- a/cli/lsp/testing/execution.rs
+++ b/cli/lsp/testing/execution.rs
@@ -363,9 +363,7 @@ impl TestRun {
                   test::TestResult::Ignored => summary.ignored += 1,
                   test::TestResult::Failed(error) => {
                     summary.failed += 1;
-                    summary
-                      .failures
-                      .push(((&description).into(), error.clone()));
+                    summary.failures.push((description.clone(), error.clone()));
                   }
                   test::TestResult::Cancelled => {
                     summary.failed += 1;
diff --git a/cli/ops/testing.rs b/cli/ops/testing.rs
index 4a2f48181..28c4145ea 100644
--- a/cli/ops/testing.rs
+++ b/cli/ops/testing.rs
@@ -123,8 +123,6 @@ fn op_register_test(
   #[string] name: String,
   ignore: bool,
   only: bool,
-  sanitize_ops: bool,
-  sanitize_resources: bool,
   #[string] file_name: String,
   #[smi] line_number: u32,
   #[smi] column_number: u32,
@@ -143,8 +141,6 @@ fn op_register_test(
     name,
     ignore,
     only,
-    sanitize_ops,
-    sanitize_resources,
     origin: origin.clone(),
     location: TestLocation {
       file_name,
diff --git a/cli/tests/testdata/test/resource_sanitizer.out b/cli/tests/testdata/test/resource_sanitizer.out
index 655dd2a17..a0e916654 100644
--- a/cli/tests/testdata/test/resource_sanitizer.out
+++ b/cli/tests/testdata/test/resource_sanitizer.out
@@ -6,10 +6,8 @@ leak ... FAILED ([WILDCARD])
 
 leak => ./test/resource_sanitizer.ts:[WILDCARD]
 error: Leaking resources:
-[UNORDERED_START]
-  - The stdin pipe was opened before the test started, but was closed during the test. Do not close resources in a test that were not created during that test.
-  - A file was opened during the test, but not closed during the test. Close the file handle by calling `file.close()`.
-[UNORDERED_END]
+  - The stdin pipe (rid 0) was opened before the test started, but was closed during the test. Do not close resources in a test that were not created during that test.
+  - A file (rid 3) was opened during the test, but not closed during the test. Close the file handle by calling `file.close()`.
 
  FAILURES 
 
diff --git a/cli/tools/test/fmt.rs b/cli/tools/test/fmt.rs
index 68adcbef1..2b6defeac 100644
--- a/cli/tools/test/fmt.rs
+++ b/cli/tools/test/fmt.rs
@@ -1,11 +1,5 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-use deno_core::stats::RuntimeActivity;
-use deno_core::stats::RuntimeActivityDiff;
-use deno_core::stats::RuntimeActivityType;
-use std::borrow::Cow;
-use std::ops::AddAssign;
-
 use super::*;
 
 pub fn to_relative_path_or_remote_url(cwd: &Url, path_or_url: &str) -> String {
@@ -80,136 +74,3 @@ pub fn format_test_error(js_error: &JsError) -> String {
     .to_string();
   format_js_error(&js_error)
 }
-
-pub fn format_sanitizer_diff(diff: RuntimeActivityDiff) -> Vec<String> {
-  let mut output = format_sanitizer_accum(diff.appeared, true);
-  output.extend(format_sanitizer_accum(diff.disappeared, false));
-  output.sort();
-  output
-}
-
-fn format_sanitizer_accum(
-  activities: Vec<RuntimeActivity>,
-  appeared: bool,
-) -> Vec<String> {
-  let mut accum = HashMap::new();
-  for activity in activities {
-    let item = format_sanitizer_accum_item(activity);
-    accum.entry(item).or_insert(0).add_assign(1);
-  }
-
-  let mut output = vec![];
-  for ((item_type, item_name), count) in accum.into_iter() {
-    if item_type == RuntimeActivityType::Resource {
-      // TODO(mmastrac): until we implement the new timers and op sanitization, these must be ignored in this path
-      if item_name == "timer" {
-        continue;
-      }
-      let (name, action1, action2) = pretty_resource_name(&item_name);
-      let hint = resource_close_hint(&item_name);
-
-      if appeared {
-        output.push(format!("{name} was {action1} during the test, but not {action2} during the test. {hint}"));
-      } else {
-        output.push(format!("{name} was {action1} before the test started, but was {action2} during the test. \
-          Do not close resources in a test that were not created during that test."));
-      }
-    } else {
-      // TODO(mmastrac): this will be done in a later PR
-      unimplemented!(
-        "Unhandled diff: {appeared} {} {:?} {}",
-        count,
-        item_type,
-        item_name
-      );
-    }
-  }
-  output
-}
-
-fn format_sanitizer_accum_item(
-  activity: RuntimeActivity,
-) -> (RuntimeActivityType, Cow<'static, str>) {
-  let activity_type = activity.activity();
-  match activity {
-    RuntimeActivity::AsyncOp(_, name) => (activity_type, name.into()),
-    RuntimeActivity::Interval(_) => (activity_type, "".into()),
-    RuntimeActivity::Resource(_, name) => (activity_type, name.into()),
-    RuntimeActivity::Timer(_) => (activity_type, "".into()),
-  }
-}
-
-fn pretty_resource_name(
-  name: &str,
-) -> (Cow<'static, str>, &'static str, &'static str) {
-  let (name, action1, action2) = match name {
-    "fsFile" => ("A file", "opened", "closed"),
-    "fetchRequest" => ("A fetch request", "started", "finished"),
-    "fetchRequestBody" => ("A fetch request body", "created", "closed"),
-    "fetchResponse" => ("A fetch response body", "created", "consumed"),
-    "httpClient" => ("An HTTP client", "created", "closed"),
-    "dynamicLibrary" => ("A dynamic library", "loaded", "unloaded"),
-    "httpConn" => ("An inbound HTTP connection", "accepted", "closed"),
-    "httpStream" => ("An inbound HTTP request", "accepted", "closed"),
-    "tcpStream" => ("A TCP connection", "opened/accepted", "closed"),
-    "unixStream" => ("A Unix connection", "opened/accepted", "closed"),
-    "tlsStream" => ("A TLS connection", "opened/accepted", "closed"),
-    "tlsListener" => ("A TLS listener", "opened", "closed"),
-    "unixListener" => ("A Unix listener", "opened", "closed"),
-    "unixDatagram" => ("A Unix datagram", "opened", "closed"),
-    "tcpListener" => ("A TCP listener", "opened", "closed"),
-    "udpSocket" => ("A UDP socket", "opened", "closed"),
-    "timer" => ("A timer", "started", "fired/cleared"),
-    "textDecoder" => ("A text decoder", "created", "finished"),
-    "messagePort" => ("A message port", "created", "closed"),
-    "webSocketStream" => ("A WebSocket", "opened", "closed"),
-    "fsEvents" => ("A file system watcher", "created", "closed"),
-    "childStdin" => ("A child process stdin", "opened", "closed"),
-    "childStdout" => ("A child process stdout", "opened", "closed"),
-    "childStderr" => ("A child process stderr", "opened", "closed"),
-    "child" => ("A child process", "started", "closed"),
-    "signal" => ("A signal listener", "created", "fired/cleared"),
-    "stdin" => ("The stdin pipe", "opened", "closed"),
-    "stdout" => ("The stdout pipe", "opened", "closed"),
-    "stderr" => ("The stderr pipe", "opened", "closed"),
-    "compression" => ("A CompressionStream", "created", "closed"),
-    _ => return (format!("\"{name}\"").into(), "created", "cleaned up"),
-  };
-  (name.into(), action1, action2)
-}
-
-fn resource_close_hint(name: &str) -> &'static str {
-  match name {
-    "fsFile" => "Close the file handle by calling `file.close()`.",
-    "fetchRequest" => "Await the promise returned from `fetch()` or abort the fetch with an abort signal.",
-    "fetchRequestBody" => "Terminate the request body `ReadableStream` by closing or erroring it.",
-    "fetchResponse" => "Consume or close the response body `ReadableStream`, e.g `await resp.text()` or `await resp.body.cancel()`.",
-    "httpClient" => "Close the HTTP client by calling `httpClient.close()`.",
-    "dynamicLibrary" => "Unload the dynamic library by calling `dynamicLibrary.close()`.",
-    "httpConn" => "Close the inbound HTTP connection by calling `httpConn.close()`.",
-    "httpStream" => "Close the inbound HTTP request by responding with `e.respondWith()` or closing the HTTP connection.",
-    "tcpStream" => "Close the TCP connection by calling `tcpConn.close()`.",
-    "unixStream" => "Close the Unix socket connection by calling `unixConn.close()`.",
-    "tlsStream" => "Close the TLS connection by calling `tlsConn.close()`.",
-    "tlsListener" => "Close the TLS listener by calling `tlsListener.close()`.",
-    "unixListener" => "Close the Unix socket listener by calling `unixListener.close()`.",
-    "unixDatagram" => "Close the Unix datagram socket by calling `unixDatagram.close()`.",
-    "tcpListener" => "Close the TCP listener by calling `tcpListener.close()`.",
-    "udpSocket" => "Close the UDP socket by calling `udpSocket.close()`.",
-    "timer" => "Clear the timer by calling `clearInterval` or `clearTimeout`.",
-    "textDecoder" => "Close the text decoder by calling `textDecoder.decode('')` or `await textDecoderStream.readable.cancel()`.",
-    "messagePort" => "Close the message port by calling `messagePort.close()`.",
-    "webSocketStream" => "Close the WebSocket by calling `webSocket.close()`.",
-    "fsEvents" => "Close the file system watcher by calling `watcher.close()`.",
-    "childStdin" => "Close the child process stdin by calling `proc.stdin.close()`.",
-    "childStdout" => "Close the child process stdout by calling `proc.stdout.close()` or `await child.stdout.cancel()`.",
-    "childStderr" => "Close the child process stderr by calling `proc.stderr.close()` or `await child.stderr.cancel()`.",
-    "child" => "Close the child process by calling `proc.kill()` or `proc.close()`.",
-    "signal" => "Clear the signal listener by calling `Deno.removeSignalListener`.",
-    "stdin" => "Close the stdin pipe by calling `Deno.stdin.close()`.",
-    "stdout" => "Close the stdout pipe by calling `Deno.stdout.close()`.",
-    "stderr" => "Close the stderr pipe by calling `Deno.stderr.close()`.",
-    "compression" => "Close the compression stream by calling `await stream.writable.close()`.",
-    _ => "Close the resource before the end of the test.",
-  }
-}
diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs
index e62582af3..d1dc76028 100644
--- a/cli/tools/test/mod.rs
+++ b/cli/tools/test/mod.rs
@@ -40,8 +40,6 @@ use deno_core::futures::StreamExt;
 use deno_core::located_script_name;
 use deno_core::parking_lot::Mutex;
 use deno_core::serde_v8;
-use deno_core::stats::RuntimeActivityStats;
-use deno_core::stats::RuntimeActivityStatsFilter;
 use deno_core::unsync::spawn;
 use deno_core::unsync::spawn_blocking;
 use deno_core::url::Url;
@@ -89,7 +87,6 @@ use tokio::sync::mpsc::WeakUnboundedSender;
 pub mod fmt;
 pub mod reporters;
 
-use fmt::format_sanitizer_diff;
 pub use fmt::format_test_error;
 use reporters::CompoundTestReporter;
 use reporters::DotTestReporter;
@@ -178,29 +175,6 @@ pub struct TestDescription {
   pub only: bool,
   pub origin: String,
   pub location: TestLocation,
-  pub sanitize_ops: bool,
-  pub sanitize_resources: bool,
-}
-
-/// May represent a failure of a test or test step.
-#[derive(Debug, Clone, PartialEq, Deserialize, Eq, Hash)]
-#[serde(rename_all = "camelCase")]
-pub struct TestFailureDescription {
-  pub id: usize,
-  pub name: String,
-  pub origin: String,
-  pub location: TestLocation,
-}
-
-impl From<&TestDescription> for TestFailureDescription {
-  fn from(value: &TestDescription) -> Self {
-    Self {
-      id: value.id,
-      name: value.name.clone(),
-      origin: value.origin.clone(),
-      location: value.location.clone(),
-    }
-  }
 }
 
 #[allow(clippy::derive_partial_eq_without_eq)]
@@ -358,7 +332,7 @@ pub struct TestSummary {
   pub ignored_steps: usize,
   pub filtered_out: usize,
   pub measured: usize,
-  pub failures: Vec<(TestFailureDescription, TestFailure)>,
+  pub failures: Vec<(TestDescription, TestFailure)>,
   pub uncaught_errors: Vec<(String, Box<JsError>)>,
 }
 
@@ -573,17 +547,6 @@ pub async fn run_tests_for_worker(
     used_only,
   }))?;
   let mut had_uncaught_error = false;
-  let stats = worker.js_runtime.runtime_activity_stats_factory();
-  let ops = worker.js_runtime.op_names();
-  let op_id_host_recv_message = ops
-    .iter()
-    .position(|op| *op == "op_host_recv_message")
-    .unwrap();
-  let op_id_host_recv_ctrl = ops
-    .iter()
-    .position(|op| *op == "op_host_recv_ctrl")
-    .unwrap();
-
   for (desc, function) in tests {
     if fail_fast_tracker.should_stop() {
       break;
@@ -598,11 +561,15 @@ pub async fn run_tests_for_worker(
     }
     sender.send(TestEvent::Wait(desc.id))?;
 
-    // Poll event loop once, to allow all ops that are already resolved, but haven't
-    // responded to settle.
-    // TODO(mmastrac): we should provide an API to poll the event loop until no futher
-    // progress is made.
+    // TODO(bartlomieju): this is a nasty (beautiful) hack, that was required
+    // when switching `JsRuntime` from `FuturesUnordered` to `JoinSet`. With
+    // `JoinSet` all pending ops are immediately polled and that caused a problem
+    // when some async ops were fired and canceled before running tests (giving
+    // false positives in the ops sanitizer). We should probably rewrite sanitizers
+    // to be done in Rust instead of in JS (40_testing.js).
     {
+      // Poll event loop once, this will allow all ops that are already resolved,
+      // but haven't responded to settle.
       let waker = noop_waker();
       let mut cx = Context::from_waker(&waker);
       let _ = worker
@@ -610,22 +577,6 @@ pub async fn run_tests_for_worker(
         .poll_event_loop(&mut cx, PollEventLoopOptions::default());
     }
 
-    let mut filter = RuntimeActivityStatsFilter::default();
-    if desc.sanitize_ops {
-      filter = filter.with_ops().with_timers();
-      filter = filter.omit_op(op_id_host_recv_ctrl as _);
-      filter = filter.omit_op(op_id_host_recv_message as _);
-    }
-    if desc.sanitize_resources {
-      filter = filter.with_resources();
-    }
-
-    let before = if !filter.is_empty() {
-      Some(stats.clone().capture(&filter))
-    } else {
-      None
-    };
-
     let earlier = SystemTime::now();
     let call = worker.js_runtime.call(&function);
     let result = match worker
@@ -649,22 +600,6 @@ pub async fn run_tests_for_worker(
         }
       }
     };
-    if let Some(before) = before {
-      let after = stats.clone().capture(&filter);
-      let diff = RuntimeActivityStats::diff(&before, &after);
-      let formatted = format_sanitizer_diff(diff);
-      if !formatted.is_empty() {
-        let failure = TestFailure::LeakedResources(formatted);
-        let elapsed = SystemTime::now().duration_since(earlier)?.as_millis();
-        sender.send(TestEvent::Result(
-          desc.id,
-          TestResult::Failed(failure),
-          elapsed as u64,
-        ))?;
-        continue;
-      }
-    }
-
     let scope = &mut worker.js_runtime.handle_scope();
     let result = v8::Local::new(scope, result);
     let result = serde_v8::from_v8::<TestResult>(scope, result)?;
diff --git a/cli/tools/test/reporters/common.rs b/cli/tools/test/reporters/common.rs
index 1dc879667..3d9cdba46 100644
--- a/cli/tools/test/reporters/common.rs
+++ b/cli/tools/test/reporters/common.rs
@@ -33,10 +33,7 @@ pub(super) fn format_test_step_ancestry(
   result
 }
 
-pub fn format_test_for_summary(
-  cwd: &Url,
-  desc: &TestFailureDescription,
-) -> String {
+pub fn format_test_for_summary(cwd: &Url, desc: &TestDescription) -> String {
   format!(
     "{} {}",
     &desc.name,
@@ -81,7 +78,7 @@ pub(super) fn report_sigint(
   let mut formatted_pending = BTreeSet::new();
   for id in tests_pending {
     if let Some(desc) = tests.get(id) {
-      formatted_pending.insert(format_test_for_summary(cwd, &desc.into()));
+      formatted_pending.insert(format_test_for_summary(cwd, desc));
     }
     if let Some(desc) = test_steps.get(id) {
       formatted_pending
@@ -110,10 +107,7 @@ pub(super) fn report_summary(
     #[allow(clippy::type_complexity)] // Type alias doesn't look better here
     let mut failures_by_origin: BTreeMap<
       String,
-      (
-        Vec<(&TestFailureDescription, &TestFailure)>,
-        Option<&JsError>,
-      ),
+      (Vec<(&TestDescription, &TestFailure)>, Option<&JsError>),
     > = BTreeMap::default();
     let mut failure_titles = vec![];
     for (description, failure) in &summary.failures {
diff --git a/cli/tools/test/reporters/dot.rs b/cli/tools/test/reporters/dot.rs
index 39a4e44d9..0df000dad 100644
--- a/cli/tools/test/reporters/dot.rs
+++ b/cli/tools/test/reporters/dot.rs
@@ -113,7 +113,7 @@ impl TestReporter for DotTestReporter {
         self
           .summary
           .failures
-          .push((description.into(), failure.clone()));
+          .push((description.clone(), failure.clone()));
       }
       TestResult::Cancelled => {
         self.summary.failed += 1;
@@ -162,9 +162,11 @@ impl TestReporter for DotTestReporter {
       TestStepResult::Failed(failure) => {
         self.summary.failed_steps += 1;
         self.summary.failures.push((
-          TestFailureDescription {
+          TestDescription {
             id: desc.id,
             name: common::format_test_step_ancestry(desc, tests, test_steps),
+            ignore: false,
+            only: false,
             origin: desc.origin.clone(),
             location: desc.location.clone(),
           },
diff --git a/cli/tools/test/reporters/pretty.rs b/cli/tools/test/reporters/pretty.rs
index c49081dd6..4a96192e6 100644
--- a/cli/tools/test/reporters/pretty.rs
+++ b/cli/tools/test/reporters/pretty.rs
@@ -233,7 +233,7 @@ impl TestReporter for PrettyTestReporter {
         self
           .summary
           .failures
-          .push((description.into(), failure.clone()));
+          .push((description.clone(), failure.clone()));
       }
       TestResult::Cancelled => {
         self.summary.failed += 1;
@@ -318,9 +318,11 @@ impl TestReporter for PrettyTestReporter {
       TestStepResult::Failed(failure) => {
         self.summary.failed_steps += 1;
         self.summary.failures.push((
-          TestFailureDescription {
+          TestDescription {
             id: desc.id,
             name: common::format_test_step_ancestry(desc, tests, test_steps),
+            ignore: false,
+            only: false,
             origin: desc.origin.clone(),
             location: desc.location.clone(),
           },