From aecad7f3531c76306274d86afb458fcbc08edca2 Mon Sep 17 00:00:00 2001
From: Matt Mastracci <matthew@mastracci.com>
Date: Mon, 5 Feb 2024 10:27:17 -0700
Subject: [PATCH] refactor(cli): Add TestFailureDescription (#22267)

Extract zero-risk changes from #22226
---
 cli/lsp/testing/execution.rs       |  4 +++-
 cli/tools/test/mod.rs              | 23 ++++++++++++++++++++++-
 cli/tools/test/reporters/common.rs | 12 +++++++++---
 cli/tools/test/reporters/dot.rs    |  6 ++----
 cli/tools/test/reporters/pretty.rs |  6 ++----
 5 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs
index 69f218be85..11882e6af3 100644
--- a/cli/lsp/testing/execution.rs
+++ b/cli/lsp/testing/execution.rs
@@ -363,7 +363,9 @@ impl TestRun {
                   test::TestResult::Ignored => summary.ignored += 1,
                   test::TestResult::Failed(error) => {
                     summary.failed += 1;
-                    summary.failures.push((description.clone(), error.clone()));
+                    summary
+                      .failures
+                      .push(((&description).into(), error.clone()));
                   }
                   test::TestResult::Cancelled => {
                     summary.failed += 1;
diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs
index 7a5d633ec7..85c00bb0f6 100644
--- a/cli/tools/test/mod.rs
+++ b/cli/tools/test/mod.rs
@@ -177,6 +177,27 @@ pub struct TestDescription {
   pub location: TestLocation,
 }
 
+/// 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)]
 #[derive(Debug, Clone, PartialEq, Deserialize)]
 #[serde(rename_all = "camelCase")]
@@ -332,7 +353,7 @@ pub struct TestSummary {
   pub ignored_steps: usize,
   pub filtered_out: usize,
   pub measured: usize,
-  pub failures: Vec<(TestDescription, TestFailure)>,
+  pub failures: Vec<(TestFailureDescription, TestFailure)>,
   pub uncaught_errors: Vec<(String, Box<JsError>)>,
 }
 
diff --git a/cli/tools/test/reporters/common.rs b/cli/tools/test/reporters/common.rs
index 3d9cdba468..1dc8796670 100644
--- a/cli/tools/test/reporters/common.rs
+++ b/cli/tools/test/reporters/common.rs
@@ -33,7 +33,10 @@ pub(super) fn format_test_step_ancestry(
   result
 }
 
-pub fn format_test_for_summary(cwd: &Url, desc: &TestDescription) -> String {
+pub fn format_test_for_summary(
+  cwd: &Url,
+  desc: &TestFailureDescription,
+) -> String {
   format!(
     "{} {}",
     &desc.name,
@@ -78,7 +81,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));
+      formatted_pending.insert(format_test_for_summary(cwd, &desc.into()));
     }
     if let Some(desc) = test_steps.get(id) {
       formatted_pending
@@ -107,7 +110,10 @@ pub(super) fn report_summary(
     #[allow(clippy::type_complexity)] // Type alias doesn't look better here
     let mut failures_by_origin: BTreeMap<
       String,
-      (Vec<(&TestDescription, &TestFailure)>, Option<&JsError>),
+      (
+        Vec<(&TestFailureDescription, &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 0df000dad2..39a4e44d98 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.clone(), failure.clone()));
+          .push((description.into(), failure.clone()));
       }
       TestResult::Cancelled => {
         self.summary.failed += 1;
@@ -162,11 +162,9 @@ impl TestReporter for DotTestReporter {
       TestStepResult::Failed(failure) => {
         self.summary.failed_steps += 1;
         self.summary.failures.push((
-          TestDescription {
+          TestFailureDescription {
             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 4a96192e6d..c49081dd66 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.clone(), failure.clone()));
+          .push((description.into(), failure.clone()));
       }
       TestResult::Cancelled => {
         self.summary.failed += 1;
@@ -318,11 +318,9 @@ impl TestReporter for PrettyTestReporter {
       TestStepResult::Failed(failure) => {
         self.summary.failed_steps += 1;
         self.summary.failures.push((
-          TestDescription {
+          TestFailureDescription {
             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(),
           },