From ea1cb4c2fafb360949d7f2cf7db7215ef2b2e338 Mon Sep 17 00:00:00 2001
From: David Sherret <dsherret@users.noreply.github.com>
Date: Tue, 28 Feb 2023 15:10:12 -0400
Subject: [PATCH] chore(test): remove TestCommandOutput macros (#17975)

---
 Cargo.lock                           | 57 +++++++++++++++-
 cli/tests/integration/bench_tests.rs | 10 ++-
 cli/tests/integration/cert_tests.rs  | 28 ++++----
 cli/tests/integration/mod.rs         | 12 ++--
 cli/tests/integration/test_tests.rs  |  8 ++-
 test_util/Cargo.toml                 |  1 +
 test_util/src/assertions.rs          | 60 -----------------
 test_util/src/builders.rs            | 97 ++++++++++++++++++++++++++--
 8 files changed, 175 insertions(+), 98 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 10234b6de1..063eddee64 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -12,6 +12,15 @@ dependencies = [
  "regex",
 ]
 
+[[package]]
+name = "addr2line"
+version = "0.19.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a76fd60b23679b7d19bd066031410fb7e458ccc5e958eb5c325888ce4baedc97"
+dependencies = [
+ "gimli",
+]
+
 [[package]]
 name = "adler"
 version = "1.0.2"
@@ -219,6 +228,21 @@ version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa"
 
+[[package]]
+name = "backtrace"
+version = "0.3.67"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "233d376d6d185f2a3093e58f283f60f880315b6c60075b01f36b3b85154564ca"
+dependencies = [
+ "addr2line",
+ "cc",
+ "cfg-if",
+ "libc",
+ "miniz_oxide 0.6.2",
+ "object",
+ "rustc-demangle",
+]
+
 [[package]]
 name = "base16ct"
 version = "0.1.1"
@@ -1816,7 +1840,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f82b0f4c27ad9f8bfd1f3208d882da2b09c301bc1c828fd3a00d0216d2fbbff6"
 dependencies = [
  "crc32fast",
- "miniz_oxide",
+ "miniz_oxide 0.5.4",
 ]
 
 [[package]]
@@ -2061,6 +2085,12 @@ dependencies = [
  "polyval",
 ]
 
+[[package]]
+name = "gimli"
+version = "0.27.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ad0a93d233ebf96623465aad4046a8d3aa4da22d4f4beba5388838c8a434bbb4"
+
 [[package]]
 name = "glibc_version"
 version = "0.1.2"
@@ -2877,6 +2907,15 @@ dependencies = [
  "adler",
 ]
 
+[[package]]
+name = "miniz_oxide"
+version = "0.6.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b275950c28b37e794e8c55d88aeb5e139d0ce23fdbbeda68f8d7174abdf9e8fa"
+dependencies = [
+ "adler",
+]
+
 [[package]]
 name = "mio"
 version = "0.8.5"
@@ -3113,6 +3152,15 @@ dependencies = [
  "cc",
 ]
 
+[[package]]
+name = "object"
+version = "0.30.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ea86265d3d3dcb6a27fc51bd29a4bf387fae9d2986b823079d4986af253eb439"
+dependencies = [
+ "memchr",
+]
+
 [[package]]
 name = "once_cell"
 version = "1.16.0"
@@ -3802,6 +3850,12 @@ dependencies = [
  "smallvec",
 ]
 
+[[package]]
+name = "rustc-demangle"
+version = "0.1.21"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7ef03e0a2b150c7a90d01faf6254c9c48a41e95fb2a8c2ac1c6f0d2b9aefc342"
+
 [[package]]
 name = "rustc-hash"
 version = "1.1.0"
@@ -4865,6 +4919,7 @@ dependencies = [
  "anyhow",
  "async-stream",
  "atty",
+ "backtrace",
  "base64 0.13.1",
  "flate2",
  "futures",
diff --git a/cli/tests/integration/bench_tests.rs b/cli/tests/integration/bench_tests.rs
index 4e94463e58..6da30857f3 100644
--- a/cli/tests/integration/bench_tests.rs
+++ b/cli/tests/integration/bench_tests.rs
@@ -3,8 +3,6 @@
 use deno_core::url::Url;
 use test_util as util;
 use util::assert_contains;
-use util::assert_exit_code;
-use util::assert_output_file;
 use util::env_vars_for_npm_tests;
 use util::TestContext;
 
@@ -196,7 +194,7 @@ fn recursive_permissions_pledge() {
     .new_command()
     .args("bench bench/recursive_permissions_pledge.js")
     .run();
-  assert_exit_code!(output, 1);
+  output.assert_exit_code(1);
   assert_contains!(
     output.text(),
     "pledge test permissions called before restoring previous pledge"
@@ -210,11 +208,11 @@ fn file_protocol() {
       .unwrap()
       .to_string();
   let context = TestContext::default();
-  let output = context
+  context
     .new_command()
     .args(format!("bench bench/file_protocol.ts {file_url}"))
-    .run();
-  assert_output_file!(output, "bench/file_protocol.out");
+    .run()
+    .assert_matches_file("bench/file_protocol.out");
 }
 
 itest!(package_json_basic {
diff --git a/cli/tests/integration/cert_tests.rs b/cli/tests/integration/cert_tests.rs
index 8fa439d78e..0031367589 100644
--- a/cli/tests/integration/cert_tests.rs
+++ b/cli/tests/integration/cert_tests.rs
@@ -12,8 +12,6 @@ use std::sync::Arc;
 use test_util as util;
 use test_util::TempDir;
 use tokio::task::LocalSet;
-use util::assert_exit_code;
-use util::assert_output_text;
 use util::TestContext;
 
 itest_flaky!(cafile_url_imports {
@@ -81,14 +79,14 @@ fn cafile_env_fetch() {
     Url::parse("https://localhost:5545/cert/cafile_url_imports.ts").unwrap();
   let context = TestContext::with_http_server();
   let cafile = context.testdata_path().join("tls/RootCA.pem");
-  let output = context
+
+  context
     .new_command()
     .args(format!("cache {module_url}"))
     .env("DENO_CERT", cafile.to_string_lossy())
-    .run();
-
-  assert_exit_code!(output, 0);
-  output.skip_output_check();
+    .run()
+    .assert_exit_code(0)
+    .skip_output_check();
 }
 
 #[flaky_test::flaky_test]
@@ -97,17 +95,16 @@ fn cafile_fetch() {
     Url::parse("http://localhost:4545/cert/cafile_url_imports.ts").unwrap();
   let context = TestContext::with_http_server();
   let cafile = context.testdata_path().join("tls/RootCA.pem");
-  let output = context
+  context
     .new_command()
     .args(format!(
       "cache --quiet --cert {} {}",
       cafile.to_string_lossy(),
       module_url,
     ))
-    .run();
-
-  assert_exit_code!(output, 0);
-  assert_output_text!(output, "");
+    .run()
+    .assert_exit_code(0)
+    .assert_matches_text("");
 }
 
 #[test]
@@ -124,12 +121,11 @@ fn cafile_compile() {
     .run();
   output.skip_output_check();
 
-  let exe_output = context
+  context
     .new_command()
     .command_name(output_exe.to_string_lossy())
-    .run();
-
-  assert_output_text!(exe_output, "[WILDCARD]\nHello\n");
+    .run()
+    .assert_matches_text("[WILDCARD]\nHello\n");
 }
 
 #[flaky_test::flaky_test]
diff --git a/cli/tests/integration/mod.rs b/cli/tests/integration/mod.rs
index 6d1daf7d77..9c865f2ad3 100644
--- a/cli/tests/integration/mod.rs
+++ b/cli/tests/integration/mod.rs
@@ -12,12 +12,12 @@ macro_rules! itest(
       .. Default::default()
     };
     let output = test.output();
-    test_util::assert_exit_code!(output, test.exit_code);
+    output.assert_exit_code(test.exit_code);
     if !test.output.is_empty() {
       assert!(test.output_str.is_none());
-      test_util::assert_output_file!(output, test.output);
+      output.assert_matches_file(test.output);
     } else {
-      test_util::assert_output_text!(output, test.output_str.unwrap_or(""));
+      output.assert_matches_text(test.output_str.unwrap_or(""));
     }
   }
 }
@@ -35,12 +35,12 @@ macro_rules! itest_flaky(
       .. Default::default()
     };
     let output = test.output();
-    test_util::assert_exit_code!(output, test.exit_code);
+    output.assert_exit_code(test.exit_code);
     if !test.output.is_empty() {
       assert!(test.output_str.is_none());
-      test_util::assert_output_file!(output, test.output);
+      output.assert_matches_file(test.output);
     } else {
-      test_util::assert_output_text!(output, test.output_str.unwrap_or(""));
+      output.assert_matches_text(test.output_str.unwrap_or(""));
     }
   }
 }
diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs
index 0ff09e69db..b9166c5106 100644
--- a/cli/tests/integration/test_tests.rs
+++ b/cli/tests/integration/test_tests.rs
@@ -2,7 +2,6 @@
 
 use deno_core::url::Url;
 use test_util as util;
-use util::assert_output_file;
 use util::env_vars_for_npm_tests;
 use util::TestContext;
 
@@ -417,8 +416,11 @@ fn file_protocol() {
       .to_string();
 
   let context = TestContext::default();
-  let output = context.new_command().args(format!("test {file_url}")).run();
-  assert_output_file!(output, "test/file_protocol.out");
+  context
+    .new_command()
+    .args(format!("test {file_url}"))
+    .run()
+    .assert_matches_file("test/file_protocol.out");
 }
 
 itest!(uncaught_errors {
diff --git a/test_util/Cargo.toml b/test_util/Cargo.toml
index 3a16cdca6f..93bad8594b 100644
--- a/test_util/Cargo.toml
+++ b/test_util/Cargo.toml
@@ -17,6 +17,7 @@ path = "src/test_server.rs"
 anyhow.workspace = true
 async-stream = "0.3.3"
 atty.workspace = true
+backtrace = "0.3.67"
 base64.workspace = true
 flate2.workspace = true
 futures.workspace = true
diff --git a/test_util/src/assertions.rs b/test_util/src/assertions.rs
index 9949265667..a004530b6e 100644
--- a/test_util/src/assertions.rs
+++ b/test_util/src/assertions.rs
@@ -39,63 +39,3 @@ macro_rules! assert_not_contains {
     }
   }
 }
-
-#[macro_export]
-macro_rules! assert_output_text {
-  ($output:expr, $expected:expr) => {
-    let expected_text = $expected;
-    let actual = $output.text();
-
-    if !expected_text.contains("[WILDCARD]") {
-      assert_eq!(actual, expected_text)
-    } else if !test_util::wildcard_match(&expected_text, actual) {
-      println!("OUTPUT START\n{}\nOUTPUT END", actual);
-      println!("EXPECTED START\n{expected_text}\nEXPECTED END");
-      panic!("pattern match failed");
-    }
-  };
-}
-
-#[macro_export]
-macro_rules! assert_output_file {
-  ($output:expr, $file_path:expr) => {
-    let output = &$output;
-    let output_path = output.testdata_dir().join($file_path);
-    println!("output path {}", output_path.display());
-    let expected_text =
-      std::fs::read_to_string(&output_path).unwrap_or_else(|err| {
-        panic!("failed loading {}\n\n{err:#}", output_path.display())
-      });
-    test_util::assert_output_text!(output, expected_text);
-  };
-}
-
-#[macro_export]
-macro_rules! assert_exit_code {
-  ($output:expr, $exit_code:expr) => {
-    let output = &$output;
-    let actual = output.text();
-    let expected_exit_code = $exit_code;
-    let actual_exit_code = output.exit_code();
-
-    if let Some(exit_code) = &actual_exit_code {
-      if *exit_code != expected_exit_code {
-        println!("OUTPUT\n{actual}\nOUTPUT");
-        panic!(
-          "bad exit code, expected: {:?}, actual: {:?}",
-          expected_exit_code, exit_code
-        );
-      }
-    } else {
-      println!("OUTPUT\n{actual}\nOUTPUT");
-      if let Some(signal) = output.signal() {
-        panic!(
-          "process terminated by signal, expected exit code: {:?}, actual signal: {:?}",
-          actual_exit_code, signal,
-        );
-      } else {
-        panic!("process terminated without status code on non unix platform, expected exit code: {:?}", actual_exit_code);
-      }
-    }
-  };
-}
diff --git a/test_util/src/builders.rs b/test_util/src/builders.rs
index bbd045a106..d48170bbfd 100644
--- a/test_util/src/builders.rs
+++ b/test_util/src/builders.rs
@@ -4,12 +4,15 @@ use std::cell::RefCell;
 use std::collections::HashMap;
 use std::io::Read;
 use std::io::Write;
+use std::path::Path;
 use std::path::PathBuf;
 use std::process::Command;
 use std::process::Stdio;
 use std::rc::Rc;
 
+use backtrace::Backtrace;
 use os_pipe::pipe;
+use pretty_assertions::assert_eq;
 
 use crate::copy_dir_recursive;
 use crate::deno_exe_path;
@@ -17,6 +20,7 @@ use crate::http_server;
 use crate::new_deno_dir;
 use crate::strip_ansi_codes;
 use crate::testdata_path;
+use crate::wildcard_match;
 use crate::HttpServerGuard;
 use crate::TempDir;
 
@@ -326,16 +330,20 @@ impl Drop for TestCommandOutput {
     // force the caller to assert these
     if !*self.asserted_exit_code.borrow() && self.exit_code != Some(0) {
       panic!(
-        "The non-zero exit code of the command was not asserted: {:?}.",
-        self.exit_code
+        "The non-zero exit code of the command was not asserted: {:?} at {}.",
+        self.exit_code,
+        failed_position(),
       )
     }
     if !*self.asserted_text.borrow() && !self.text.is_empty() {
       println!("OUTPUT\n{}\nOUTPUT", self.text);
-      panic!(concat!(
-        "The non-empty text of the command was not asserted. ",
-        "Call `output.skip_output_check()` to skip if necessary.",
-      ));
+      panic!(
+        concat!(
+          "The non-empty text of the command was not asserted. ",
+          "Call `output.skip_output_check()` to skip if necessary at {}.",
+        ),
+        failed_position()
+      );
     }
   }
 }
@@ -366,4 +374,81 @@ impl TestCommandOutput {
     self.skip_output_check();
     &self.text
   }
+
+  pub fn assert_exit_code(&self, expected_exit_code: i32) -> &Self {
+    let actual_exit_code = self.exit_code();
+
+    if let Some(exit_code) = &actual_exit_code {
+      if *exit_code != expected_exit_code {
+        println!("OUTPUT\n{}\nOUTPUT", self.text());
+        panic!(
+          "bad exit code, expected: {:?}, actual: {:?} at {}",
+          expected_exit_code,
+          exit_code,
+          failed_position(),
+        );
+      }
+    } else {
+      println!("OUTPUT\n{}\nOUTPUT", self.text());
+      if let Some(signal) = self.signal() {
+        panic!(
+          "process terminated by signal, expected exit code: {:?}, actual signal: {:?} at {}",
+          actual_exit_code,
+          signal,
+          failed_position(),
+        );
+      } else {
+        panic!(
+          "process terminated without status code on non unix platform, expected exit code: {:?} at {}",
+          actual_exit_code,
+          failed_position(),
+        );
+      }
+    }
+
+    self
+  }
+
+  pub fn assert_matches_text(&self, expected_text: impl AsRef<str>) -> &Self {
+    let expected_text = expected_text.as_ref();
+    let actual = self.text();
+
+    if !expected_text.contains("[WILDCARD]") {
+      assert_eq!(actual, expected_text, "at {}", failed_position());
+    } else if !wildcard_match(expected_text, actual) {
+      println!("OUTPUT START\n{actual}\nOUTPUT END");
+      println!("EXPECTED START\n{expected_text}\nEXPECTED END");
+      panic!("pattern match failed at {}", failed_position());
+    }
+
+    self
+  }
+
+  pub fn assert_matches_file(&self, file_path: impl AsRef<Path>) -> &Self {
+    let output_path = self.testdata_dir().join(file_path);
+    println!("output path {}", output_path.display());
+    let expected_text =
+      std::fs::read_to_string(&output_path).unwrap_or_else(|err| {
+        panic!("failed loading {}\n\n{err:#}", output_path.display())
+      });
+    self.assert_matches_text(expected_text)
+  }
+}
+
+fn failed_position() -> String {
+  let backtrace = Backtrace::new();
+
+  for frame in backtrace.frames() {
+    for symbol in frame.symbols() {
+      if let Some(filename) = symbol.filename() {
+        if !filename.to_string_lossy().ends_with("builders.rs") {
+          let line_num = symbol.lineno().unwrap_or(0);
+          let line_col = symbol.colno().unwrap_or(0);
+          return format!("{}:{}:{}", filename.display(), line_num, line_col);
+        }
+      }
+    }
+  }
+
+  "<unknown>".to_string()
 }