Make arbitrary types implementing std::error::Error work transparently with the ? operator in tests.

Previously, this worked only by manually calling err_to_test_failure() on the test result. This isn't necessary, since the ? operator automatically invokes Into::into() as long as that's available. This change introduces the required blanket trait implementation and eliminates err_to_test_failure() altogether.

This is a breaking API change. To port existing code, just remove the call to err_to_test_failure(). If the output of that is returned directly, then replace it with a ? operator usage:

OLD: return do_something().err_to_test_failure();
NEW: return Ok(do_something()?);
PiperOrigin-RevId: 505056332

Author: hovinenb

googletest/integration_tests/integration_tests.rs

@@ -20,7 +20,7 @@ mod tests {
     use googletest::{all, matchers};
     use googletest::{
         assert_that, expect_pred, expect_that, google_test, verify_pred, verify_that,
-        GoogleTestSupport, MapErrorToTestFailure, Result,
+        GoogleTestSupport, Result,
     };
     #[cfg(google3)]
     use matchers::all;
@@ -49,24 +49,21 @@ mod tests {
 
     #[google_test]
     fn should_fail_on_assertion_failure() -> Result<()> {
-        let status =
-            run_external_process("simple_assertion_failure").status().err_to_test_failure()?;
+        let status = run_external_process("simple_assertion_failure").status()?;
 
         verify_that!(status.success(), eq(false))
     }
 
     #[google_test]
     fn should_fail_on_assertion_failure_with_assert_that() -> Result<()> {
-        let status = run_external_process("simple_assertion_failure_with_assert_that")
-            .status()
-            .err_to_test_failure()?;
+        let status = run_external_process("simple_assertion_failure_with_assert_that").status()?;
 
         verify_that!(status.success(), eq(false))
     }
 
     #[google_test]
     fn should_fail_on_assertion_failure_with_expect_that() -> Result<()> {
-        let status = run_external_process("expect_that_failure").status().err_to_test_failure()?;
+        let status = run_external_process("expect_that_failure").status()?;
 
         verify_that!(status.success(), eq(false))
     }
@@ -143,25 +140,21 @@ Actual: 2, which isn't equal to 4
 
     #[google_test]
     fn should_fail_due_to_assertion_failure_in_subroutine() -> Result<()> {
-        let status =
-            run_external_process("simple_assertion_failure").status().err_to_test_failure()?;
+        let status = run_external_process("simple_assertion_failure").status()?;
 
         verify_that!(status.success(), eq(false))
     }
 
     #[google_test]
     fn should_fail_due_to_returned_error_in_subroutine() -> Result<()> {
-        let status =
-            run_external_process("failure_due_to_returned_error").status().err_to_test_failure()?;
+        let status = run_external_process("failure_due_to_returned_error").status()?;
 
         verify_that!(status.success(), eq(false))
     }
 
     #[google_test]
     fn should_fail_test_on_and_log_failure() -> Result<()> {
-        let status = run_external_process("non_fatal_failure_in_subroutine")
-            .status()
-            .err_to_test_failure()?;
+        let status = run_external_process("non_fatal_failure_in_subroutine").status()?;
 
         verify_that!(status.success(), eq(false))
     }
@@ -257,8 +250,7 @@ Actual: 2, which isn't equal to 3
 
     #[google_test]
     fn verify_pred_should_fail_test_on_failure() -> Result<()> {
-        let status =
-            run_external_process("verify_predicate_with_failure").status().err_to_test_failure()?;
+        let status = run_external_process("verify_predicate_with_failure").status()?;
 
         verify_that!(status.success(), eq(false))
     }
@@ -281,15 +273,14 @@ eq_predicate(a, b) was false with
 
     #[google_test]
     fn assert_pred_should_fail_test_on_failure() -> Result<()> {
-        let status =
-            run_external_process("assert_predicate_with_failure").status().err_to_test_failure()?;
+        let status = run_external_process("assert_predicate_with_failure").status()?;
 
         verify_that!(status.success(), eq(false))
     }
 
     #[google_test]
     fn expect_pred_should_fail_test_on_failure() -> Result<()> {
-        let status = run_external_process("expect_pred_failure").status().err_to_test_failure()?;
+        let status = run_external_process("expect_pred_failure").status()?;
 
         verify_that!(status.success(), eq(false))
     }
@@ -414,8 +405,7 @@ a_submodule :: A_STRUCT_IN_SUBMODULE.eq_predicate_as_method(a, b) was false with
 
     #[google_test]
     fn fail_macro_causes_test_failure() -> Result<()> {
-        let status =
-            run_external_process("failure_due_to_fail_macro").status().err_to_test_failure()?;
+        let status = run_external_process("failure_due_to_fail_macro").status()?;
 
         verify_that!(status.success(), eq(false))
     }
@@ -478,8 +468,8 @@ Actual: 1, which isn't equal to 2
 
     fn run_external_process_in_tests_directory(name: &'static str) -> Result<String> {
         let mut command = run_external_process(name);
-        let std::process::Output { stdout, .. } = command.output().err_to_test_failure()?;
-        String::from_utf8(stdout).err_to_test_failure()
+        let std::process::Output { stdout, .. } = command.output()?;
+        Ok(String::from_utf8(stdout)?)
     }
 
     fn run_external_process(name: &'static str) -> Command {

googletest/src/internal/test_outcome.rs

@@ -114,10 +114,6 @@ impl TestAssertionFailure {
         TestOutcome::fail_current_test();
         print!("{}", self);
     }
-
-    pub(crate) fn from_error<T: std::fmt::Display>(err: T) -> TestAssertionFailure {
-        TestAssertionFailure::create(format!("{}", err))
-    }
 }
 
 impl Display for TestAssertionFailure {
@@ -138,3 +134,9 @@ impl Debug for TestAssertionFailure {
         Display::fmt(self, f)
     }
 }
+
+impl<T: std::error::Error> From<T> for TestAssertionFailure {
+    fn from(value: T) -> Self {
+        TestAssertionFailure::create(format!("{value}"))
+    }
+}

googletest/src/lib.rs

@@ -133,31 +133,3 @@ impl<T> GoogleTestSupport for std::result::Result<T, TestAssertionFailure> {
         self
     }
 }
-
-/// Adds to `Result` support for ergonomically mapping the `Err` variant to a
-/// [`TestAssertionFailure`].
-pub trait MapErrorToTestFailure<T> {
-    /// Converts the `Err` variant of a `Result` to a [`TestAssertionFailure`].
-    ///
-    /// This allows the use of the `?` operator with any supported `Result`
-    /// inside a test or other function returning [`Result`]. For
-    /// example:
-    ///
-    /// ```rust
-    /// fn test_something() -> Result<()> {
-    ///     let output =
-    ///         std::process::Command("do_something").output()
-    ///             .err_to_test_failure()?;
-    /// }
-    /// ```
-    ///
-    /// The current implementation requires that the `Err` variant of the
-    /// `Result` implement [`std::fmt::Display`].
-    fn err_to_test_failure(self) -> Result<T>;
-}
-
-impl<T, E: std::fmt::Display> MapErrorToTestFailure<T> for std::result::Result<T, E> {
-    fn err_to_test_failure(self) -> Result<T> {
-        self.map_err(TestAssertionFailure::from_error)
-    }
-}