feat(cli/tools): add a subcommand `--hide-stacktraces` for test (#24095)

7 files changed, 115 insertions, 48 deletions

diff --git a/cli/tools/test/fmt.rs b/cli/tools/test/fmt.rs
index d66c72239..174155072 100644
--- a/cli/tools/test/fmt.rs
+++ b/cli/tools/test/fmt.rs
@@ -72,16 +72,24 @@ fn abbreviate_test_error(js_error: &JsError) -> JsError {
 // This function prettifies `JsError` and applies some changes specifically for
 // test runner purposes:
 //
+// - hide stack traces if `options.hide_stacktraces` is set to `true`
+//
 // - filter out stack frames:
 //   - if stack trace consists of mixed user and internal code, the frames
 //     below the first user code frame are filtered out
 //   - if stack trace consists only of internal code it is preserved as is
-pub fn format_test_error(js_error: &JsError) -> String {
+pub fn format_test_error(
+  js_error: &JsError,
+  options: &TestFailureFormatOptions,
+) -> String {
   let mut js_error = abbreviate_test_error(js_error);
   js_error.exception_message = js_error
     .exception_message
     .trim_start_matches("Uncaught ")
     .to_string();
+  if options.hide_stacktraces {
+    return js_error.exception_message;
+  }
   format_js_error(&js_error)
 }
 
diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs
index 4fbd0423e..0dc213350 100644
--- a/cli/tools/test/mod.rs
+++ b/cli/tools/test/mod.rs
@@ -288,6 +288,11 @@ impl From<&TestDescription> for TestFailureDescription {
   }
 }
 
+#[derive(Debug, Default, Clone, PartialEq)]
+pub struct TestFailureFormatOptions {
+  pub hide_stacktraces: bool,
+}
+
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Debug, Clone, PartialEq, Deserialize)]
 #[serde(rename_all = "camelCase")]
@@ -302,52 +307,55 @@ pub enum TestFailure {
   HasSanitizersAndOverlaps(IndexSet<String>), // Long names of overlapped tests
 }
 
-impl std::fmt::Display for TestFailure {
-  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl TestFailure {
+  pub fn format(
+    &self,
+    options: &TestFailureFormatOptions,
+  ) -> Cow<'static, str> {
     match self {
       TestFailure::JsError(js_error) => {
-        write!(f, "{}", format_test_error(js_error))
+        Cow::Owned(format_test_error(js_error, options))
+      }
+      TestFailure::FailedSteps(1) => Cow::Borrowed("1 test step failed."),
+      TestFailure::FailedSteps(n) => {
+        Cow::Owned(format!("{} test steps failed.", n))
       }
-      TestFailure::FailedSteps(1) => write!(f, "1 test step failed."),
-      TestFailure::FailedSteps(n) => write!(f, "{n} test steps failed."),
       TestFailure::IncompleteSteps => {
-        write!(f, "Completed while steps were still running. Ensure all steps are awaited with `await t.step(...)`.")
+        Cow::Borrowed("Completed while steps were still running. Ensure all steps are awaited with `await t.step(...)`.")
       }
       TestFailure::Incomplete => {
-        write!(
-          f,
-          "Didn't complete before parent. Await step with `await t.step(...)`."
-        )
+        Cow::Borrowed("Didn't complete before parent. Await step with `await t.step(...)`.")
       }
       TestFailure::Leaked(details, trailer_notes) => {
-        write!(f, "Leaks detected:")?;
+        let mut f = String::new();
+        write!(f, "Leaks detected:").unwrap();
         for detail in details {
-          write!(f, "\n  - {}", detail)?;
+          write!(f, "\n  - {}", detail).unwrap();
         }
         for trailer in trailer_notes {
-          write!(f, "\n{}", trailer)?;
+          write!(f, "\n{}", trailer).unwrap();
         }
-        Ok(())
+        Cow::Owned(f)
       }
       TestFailure::OverlapsWithSanitizers(long_names) => {
-        write!(f, "Started test step while another test step with sanitizers was running:")?;
+        let mut f = String::new();
+        write!(f, "Started test step while another test step with sanitizers was running:").unwrap();
         for long_name in long_names {
-          write!(f, "\n  * {}", long_name)?;
+          write!(f, "\n  * {}", long_name).unwrap();
         }
-        Ok(())
+        Cow::Owned(f)
       }
       TestFailure::HasSanitizersAndOverlaps(long_names) => {
-        write!(f, "Started test step with sanitizers while another test step was running:")?;
+        let mut f = String::new();
+        write!(f, "Started test step with sanitizers while another test step was running:").unwrap();
         for long_name in long_names {
-          write!(f, "\n  * {}", long_name)?;
+          write!(f, "\n  * {}", long_name).unwrap();
         }
-        Ok(())
+        Cow::Owned(f)
       }
     }
   }
-}
 
-impl TestFailure {
   pub fn overview(&self) -> String {
     match self {
       TestFailure::JsError(js_error) => js_error.exception_message.clone(),
@@ -369,10 +377,6 @@ impl TestFailure {
     }
   }
 
-  pub fn detail(&self) -> String {
-    self.to_string()
-  }
-
   fn format_label(&self) -> String {
     match self {
       TestFailure::Incomplete => colors::gray("INCOMPLETE").to_string(),
@@ -512,6 +516,7 @@ struct TestSpecifiersOptions {
   specifier: TestSpecifierOptions,
   reporter: TestReporterConfig,
   junit_path: Option<String>,
+  hide_stacktraces: bool,
 }
 
 #[derive(Debug, Default, Clone)]
@@ -545,23 +550,31 @@ impl TestSummary {
 
 fn get_test_reporter(options: &TestSpecifiersOptions) -> Box<dyn TestReporter> {
   let parallel = options.concurrent_jobs.get() > 1;
+  let failure_format_options = TestFailureFormatOptions {
+    hide_stacktraces: options.hide_stacktraces,
+  };
   let reporter: Box<dyn TestReporter> = match &options.reporter {
-    TestReporterConfig::Dot => {
-      Box::new(DotTestReporter::new(options.cwd.clone()))
-    }
+    TestReporterConfig::Dot => Box::new(DotTestReporter::new(
+      options.cwd.clone(),
+      failure_format_options,
+    )),
     TestReporterConfig::Pretty => Box::new(PrettyTestReporter::new(
       parallel,
       options.log_level != Some(Level::Error),
       options.filter,
       false,
       options.cwd.clone(),
+      failure_format_options,
+    )),
+    TestReporterConfig::Junit => Box::new(JunitTestReporter::new(
+      options.cwd.clone(),
+      "-".to_string(),
+      failure_format_options,
     )),
-    TestReporterConfig::Junit => {
-      Box::new(JunitTestReporter::new(options.cwd.clone(), "-".to_string()))
-    }
     TestReporterConfig::Tap => Box::new(TapTestReporter::new(
       options.cwd.clone(),
       options.concurrent_jobs > NonZeroUsize::new(1).unwrap(),
+      failure_format_options,
     )),
   };
 
@@ -569,6 +582,9 @@ fn get_test_reporter(options: &TestSpecifiersOptions) -> Box<dyn TestReporter> {
     let junit = Box::new(JunitTestReporter::new(
       options.cwd.clone(),
       junit_path.to_string(),
+      TestFailureFormatOptions {
+        hide_stacktraces: options.hide_stacktraces,
+      },
     ));
     return Box::new(CompoundTestReporter::new(vec![reporter, junit]));
   }
@@ -1807,6 +1823,7 @@ pub async fn run_tests(
       filter: workspace_test_options.filter.is_some(),
       reporter: workspace_test_options.reporter,
       junit_path: workspace_test_options.junit_path,
+      hide_stacktraces: workspace_test_options.hide_stacktraces,
       specifier: TestSpecifierOptions {
         filter: TestFilter::from_flag(&workspace_test_options.filter),
         shuffle: workspace_test_options.shuffle,
@@ -1973,6 +1990,7 @@ pub async fn run_tests_with_watch(
             filter: workspace_test_options.filter.is_some(),
             reporter: workspace_test_options.reporter,
             junit_path: workspace_test_options.junit_path,
+            hide_stacktraces: workspace_test_options.hide_stacktraces,
             specifier: TestSpecifierOptions {
               filter: TestFilter::from_flag(&workspace_test_options.filter),
               shuffle: workspace_test_options.shuffle,
diff --git a/cli/tools/test/reporters/common.rs b/cli/tools/test/reporters/common.rs
index e4d8d4247..7ca83db80 100644
--- a/cli/tools/test/reporters/common.rs
+++ b/cli/tools/test/reporters/common.rs
@@ -105,6 +105,7 @@ pub(super) fn report_summary(
   cwd: &Url,
   summary: &TestSummary,
   elapsed: &Duration,
+  options: &TestFailureFormatOptions,
 ) {
   if !summary.failures.is_empty() || !summary.uncaught_errors.is_empty() {
     #[allow(clippy::type_complexity)] // Type alias doesn't look better here
@@ -136,8 +137,13 @@ pub(super) fn report_summary(
         if !failure.hide_in_summary() {
           let failure_title = format_test_for_summary(cwd, description);
           writeln!(writer, "{}", &failure_title).unwrap();
-          writeln!(writer, "{}: {}", colors::red_bold("error"), failure)
-            .unwrap();
+          writeln!(
+            writer,
+            "{}: {}",
+            colors::red_bold("error"),
+            failure.format(options)
+          )
+          .unwrap();
           writeln!(writer).unwrap();
           failure_titles.push(failure_title);
         }
@@ -152,7 +158,7 @@ pub(super) fn report_summary(
           writer,
           "{}: {}",
           colors::red_bold("error"),
-          format_test_error(js_error)
+          format_test_error(js_error, options)
         )
         .unwrap();
         writeln!(writer, "This error was not caught from a test and caused the test runner to fail on the referenced module.").unwrap();
diff --git a/cli/tools/test/reporters/dot.rs b/cli/tools/test/reporters/dot.rs
index 8fbd59232..169c4b0e7 100644
--- a/cli/tools/test/reporters/dot.rs
+++ b/cli/tools/test/reporters/dot.rs
@@ -9,11 +9,15 @@ pub struct DotTestReporter {
   width: usize,
   cwd: Url,
   summary: TestSummary,
+  failure_format_options: TestFailureFormatOptions,
 }
 
 #[allow(clippy::print_stdout)]
 impl DotTestReporter {
-  pub fn new(cwd: Url) -> DotTestReporter {
+  pub fn new(
+    cwd: Url,
+    failure_format_options: TestFailureFormatOptions,
+  ) -> DotTestReporter {
     let console_width = if let Some(size) = crate::util::console::console_size()
     {
       size.cols as usize
@@ -26,6 +30,7 @@ impl DotTestReporter {
       width: console_width,
       cwd,
       summary: TestSummary::new(),
+      failure_format_options,
     }
   }
 
@@ -190,6 +195,7 @@ impl TestReporter for DotTestReporter {
       &self.cwd,
       &self.summary,
       elapsed,
+      &self.failure_format_options,
     );
     println!();
   }
diff --git a/cli/tools/test/reporters/junit.rs b/cli/tools/test/reporters/junit.rs
index 4b69218df..3998bee40 100644
--- a/cli/tools/test/reporters/junit.rs
+++ b/cli/tools/test/reporters/junit.rs
@@ -15,19 +15,28 @@ pub struct JunitTestReporter {
   // from child to parent to build the full test name that reflects the test
   // hierarchy.
   test_name_tree: TestNameTree,
+  failure_format_options: TestFailureFormatOptions,
 }
 
 impl JunitTestReporter {
-  pub fn new(cwd: Url, output_path: String) -> Self {
+  pub fn new(
+    cwd: Url,
+    output_path: String,
+    failure_format_options: TestFailureFormatOptions,
+  ) -> Self {
     Self {
       cwd,
       output_path,
       cases: IndexMap::new(),
       test_name_tree: TestNameTree::new(),
+      failure_format_options,
     }
   }
 
-  fn convert_status(status: &TestResult) -> quick_junit::TestCaseStatus {
+  fn convert_status(
+    status: &TestResult,
+    failure_format_options: &TestFailureFormatOptions,
+  ) -> quick_junit::TestCaseStatus {
     match status {
       TestResult::Ok => quick_junit::TestCaseStatus::success(),
       TestResult::Ignored => quick_junit::TestCaseStatus::skipped(),
@@ -35,7 +44,7 @@ impl JunitTestReporter {
         kind: quick_junit::NonSuccessKind::Failure,
         message: Some(failure.overview()),
         ty: None,
-        description: Some(failure.detail()),
+        description: Some(failure.format(failure_format_options).into_owned()),
         reruns: vec![],
       },
       TestResult::Cancelled => quick_junit::TestCaseStatus::NonSuccess {
@@ -50,6 +59,7 @@ impl JunitTestReporter {
 
   fn convert_step_status(
     status: &TestStepResult,
+    failure_format_options: &TestFailureFormatOptions,
   ) -> quick_junit::TestCaseStatus {
     match status {
       TestStepResult::Ok => quick_junit::TestCaseStatus::success(),
@@ -59,7 +69,9 @@ impl JunitTestReporter {
           kind: quick_junit::NonSuccessKind::Failure,
           message: Some(failure.overview()),
           ty: None,
-          description: Some(failure.detail()),
+          description: Some(
+            failure.format(failure_format_options).into_owned(),
+          ),
           reruns: vec![],
         }
       }
@@ -111,7 +123,7 @@ impl TestReporter for JunitTestReporter {
     elapsed: u64,
   ) {
     if let Some(case) = self.cases.get_mut(&description.id) {
-      case.status = Self::convert_status(result);
+      case.status = Self::convert_status(result, &self.failure_format_options);
       case.set_time(Duration::from_millis(elapsed));
     }
   }
@@ -153,7 +165,8 @@ impl TestReporter for JunitTestReporter {
     _test_steps: &IndexMap<usize, TestStepDescription>,
   ) {
     if let Some(case) = self.cases.get_mut(&description.id) {
-      case.status = Self::convert_step_status(result);
+      case.status =
+        Self::convert_step_status(result, &self.failure_format_options);
       case.set_time(Duration::from_millis(elapsed));
     }
   }
diff --git a/cli/tools/test/reporters/pretty.rs b/cli/tools/test/reporters/pretty.rs
index cb9f2c435..4120bbfb5 100644
--- a/cli/tools/test/reporters/pretty.rs
+++ b/cli/tools/test/reporters/pretty.rs
@@ -20,6 +20,7 @@ pub struct PrettyTestReporter {
     HashMap<usize, IndexMap<usize, (TestStepDescription, TestStepResult, u64)>>,
   summary: TestSummary,
   writer: Box<dyn std::io::Write>,
+  failure_format_options: TestFailureFormatOptions,
 }
 
 impl PrettyTestReporter {
@@ -29,6 +30,7 @@ impl PrettyTestReporter {
     filter: bool,
     repl: bool,
     cwd: Url,
+    failure_format_options: TestFailureFormatOptions,
   ) -> PrettyTestReporter {
     PrettyTestReporter {
       parallel,
@@ -45,6 +47,7 @@ impl PrettyTestReporter {
       child_results_buffer: Default::default(),
       summary: TestSummary::new(),
       writer: Box::new(std::io::stdout()),
+      failure_format_options,
     }
   }
 
@@ -395,7 +398,13 @@ impl TestReporter for PrettyTestReporter {
     _test_steps: &IndexMap<usize, TestStepDescription>,
   ) {
     self.write_output_end();
-    common::report_summary(&mut self.writer, &self.cwd, &self.summary, elapsed);
+    common::report_summary(
+      &mut self.writer,
+      &self.cwd,
+      &self.summary,
+      elapsed,
+      &self.failure_format_options,
+    );
     if !self.repl {
       writeln!(&mut self.writer).unwrap();
     }
diff --git a/cli/tools/test/reporters/tap.rs b/cli/tools/test/reporters/tap.rs
index 62cb58a83..ea68ddd43 100644
--- a/cli/tools/test/reporters/tap.rs
+++ b/cli/tools/test/reporters/tap.rs
@@ -20,11 +20,16 @@ pub struct TapTestReporter {
   n: usize,
   step_n: usize,
   step_results: HashMap<usize, Vec<(TestStepDescription, TestStepResult)>>,
+  failure_format_options: TestFailureFormatOptions,
 }
 
 #[allow(clippy::print_stdout)]
 impl TapTestReporter {
-  pub fn new(cwd: Url, is_concurrent: bool) -> TapTestReporter {
+  pub fn new(
+    cwd: Url,
+    is_concurrent: bool,
+    failure_format_options: TestFailureFormatOptions,
+  ) -> TapTestReporter {
     TapTestReporter {
       cwd,
       is_concurrent,
@@ -33,6 +38,7 @@ impl TapTestReporter {
       n: 0,
       step_n: 0,
       step_results: HashMap::new(),
+      failure_format_options,
     }
   }
 
@@ -45,6 +51,7 @@ impl TapTestReporter {
   }
 
   fn print_diagnostic(
+    &self,
     indent: usize,
     failure: &TestFailure,
     location: DiagnosticLocation,
@@ -56,7 +63,7 @@ impl TapTestReporter {
     // YAML is a superset of JSON, so we can avoid a YAML dependency here.
     // This makes the output less readable though.
     let diagnostic = serde_json::to_string(&json!({
-      "message": failure.to_string(),
+      "message": failure.format(&self.failure_format_options),
       "severity": "fail".to_string(),
       "at": location,
     }))
@@ -102,7 +109,7 @@ impl TapTestReporter {
     Self::print_line(4, status, self.step_n, &desc.name, directive);
 
     if let TestStepResult::Failed(failure) = result {
-      Self::print_diagnostic(
+      self.print_diagnostic(
         4,
         failure,
         DiagnosticLocation {
@@ -171,7 +178,7 @@ impl TestReporter for TapTestReporter {
     Self::print_line(0, status, self.n, &description.name, directive);
 
     if let TestResult::Failed(failure) = result {
-      Self::print_diagnostic(
+      self.print_diagnostic(
         0,
         failure,
         DiagnosticLocation {