Do not repeat the actual value inside Option::Some, Result::Ok, or Result::Err in test assertion failure messages.

hovinenb · copybara-github · commit 4c086f8595b3 · 2023-01-31T05:10:38.000-08:00
Previously, the test assertion failure message for the matchers some, ok, and err would contain the actual value. This is redundant since the value was already printed right above. This changes the implementation of explain_match in each of those matchers to just make reference to the explanation of the inner matcher along with a really brief description of the variant found ("has a value", "is a success with value", "is an error with value"). This makes the test assertion failure messages shorter and less jarring.

PiperOrigin-RevId: 505970192
diff --git a/googletest/src/matchers/err_matcher.rs b/googletest/src/matchers/err_matcher.rs
@@ -42,7 +42,7 @@ impl<T: Debug, E: Debug, InnerMatcherT: Matcher<E>> Matcher<Result<T, E>>
     fn explain_match(&self, actual: &Result<T, E>) -> MatchExplanation {
         match actual {
             Err(e) => MatchExplanation::create(format!(
-                "which is an error containing {e:#?}, {}",
+                "which is an error {}",
                 self.inner.explain_match(e)
             )),
             Ok(_) => MatchExplanation::create("which is a success".to_string()),
@@ -52,14 +52,11 @@ impl<T: Debug, E: Debug, InnerMatcherT: Matcher<E>> Matcher<Result<T, E>>
     fn describe(&self, matcher_result: MatcherResult) -> String {
         match matcher_result {
             MatcherResult::Matches => {
-                format!(
-                    "is an error containing a value, which {}",
-                    self.inner.describe(MatcherResult::Matches)
-                )
+                format!("is an error which {}", self.inner.describe(MatcherResult::Matches))
             }
             MatcherResult::DoesNotMatch => {
                 format!(
-                    "is a success or is an error containing a value, which {}",
+                    "is a success or is an error containing a value which {}",
                     self.inner.describe(MatcherResult::DoesNotMatch)
                 )
             }
@@ -116,10 +113,10 @@ mod tests {
             err(displays_as(contains_substring(
                 "\
 Value of: Err::<i32, i32>(1)
-Expected: is an error containing a value, which is equal to 2
+Expected: is an error which is equal to 2
 Actual: Err(
     1,
-), which is an error containing 1, which isn't equal to 2
+), which is an error which isn't equal to 2
 "
             )))
         )
@@ -129,7 +126,7 @@ Actual: Err(
     fn err_describe_matches() -> Result<()> {
         verify_that!(
             err::<i32, i32>(eq(1)).describe(MatcherResult::Matches),
-            eq("is an error containing a value, which is equal to 1")
+            eq("is an error which is equal to 1")
         )
     }
 }
diff --git a/googletest/src/matchers/ok_matcher.rs b/googletest/src/matchers/ok_matcher.rs
@@ -42,7 +42,7 @@ impl<T: Debug, E: Debug, InnerMatcherT: Matcher<T>> Matcher<Result<T, E>>
     fn explain_match(&self, actual: &Result<T, E>) -> MatchExplanation {
         match actual {
             Ok(o) => MatchExplanation::create(format!(
-                "which is a success containing {o:#?}, {}",
+                "which is a success {}",
                 self.inner.explain_match(o)
             )),
             Err(_) => MatchExplanation::create("which is an error".to_string()),
@@ -119,7 +119,7 @@ Value of: Ok::<i32, i32>(1)
 Expected: is a success containing a value, which is equal to 2
 Actual: Ok(
     1,
-), which is a success containing 1, which isn't equal to 2
+), which is a success which isn't equal to 2
 "
             )))
         )
diff --git a/googletest/src/matchers/some_matcher.rs b/googletest/src/matchers/some_matcher.rs
@@ -40,21 +40,21 @@ impl<T: Debug, InnerMatcherT: Matcher<T>> Matcher<Option<T>> for SomeMatcher<Inn
     fn explain_match(&self, actual: &Option<T>) -> MatchExplanation {
         match (self.matches(actual), actual) {
             (_, Some(t)) => MatchExplanation::create(format!(
-                "which contains {t:#?}, {}",
+                "which has a value {}",
                 self.inner.explain_match(t)
             )),
-            (_, None) => MatchExplanation::create("which is none".to_string()),
+            (_, None) => MatchExplanation::create("which is None".to_string()),
         }
     }
 
     fn describe(&self, matcher_result: MatcherResult) -> String {
         match matcher_result {
             MatcherResult::Matches => {
-                format!("is some(x), where x {}", self.inner.describe(MatcherResult::Matches))
+                format!("has a value which {}", self.inner.describe(MatcherResult::Matches))
             }
             MatcherResult::DoesNotMatch => {
                 format!(
-                    "is none or some(x), where x {}",
+                    "is None or has a value which {}",
                     self.inner.describe(MatcherResult::DoesNotMatch)
                 )
             }
@@ -107,10 +107,10 @@ mod tests {
             err(displays_as(contains_substring(
                 "\
 Value of: Some(2)
-Expected: is some(x), where x is equal to 1
+Expected: has a value which is equal to 1
 Actual: Some(
     2,
-), which contains 2, which isn't equal to 1
+), which has a value which isn't equal to 1
 "
             )))
         )
@@ -120,36 +120,36 @@ Actual: Some(
     fn some_describe_matches() -> Result<()> {
         verify_that!(
             some(eq(1)).describe(MatcherResult::Matches),
-            eq("is some(x), where x is equal to 1")
+            eq("has a value which is equal to 1")
         )
     }
 
     #[google_test]
     fn some_describe_does_not_match() -> Result<()> {
         verify_that!(
             some(eq(1)).describe(MatcherResult::DoesNotMatch),
-            eq("is none or some(x), where x isn't equal to 1")
+            eq("is None or has a value which isn't equal to 1")
         )
     }
 
     #[google_test]
     fn some_explain_match_with_none() -> Result<()> {
-        verify_that!(some(eq(1)).explain_match(&None), displays_as(eq("which is none")))
+        verify_that!(some(eq(1)).explain_match(&None), displays_as(eq("which is None")))
     }
 
     #[google_test]
     fn some_explain_match_with_some_success() -> Result<()> {
         verify_that!(
             some(eq(1)).explain_match(&Some(1)),
-            displays_as(eq("which contains 1, which is equal to 1"))
+            displays_as(eq("which has a value which is equal to 1"))
         )
     }
 
     #[google_test]
     fn some_explain_match_with_some_fail() -> Result<()> {
         verify_that!(
             some(eq(1)).explain_match(&Some(2)),
-            displays_as(eq("which contains 2, which isn't equal to 1"))
+            displays_as(eq("which has a value which isn't equal to 1"))
         )
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -40,21 +40,21 @@ impl<T: Debug, InnerMatcherT: Matcher<T>> Matcher<Option<T>> for SomeMatcher<Inn`
`40`	`40`	`fn explain_match(&self, actual: &Option<T>) -> MatchExplanation {`
`41`	`41`	`match (self.matches(actual), actual) {`
`42`	`42`	`(_, Some(t)) => MatchExplanation::create(format!(`
`43`		`- "which contains {t:#?}, {}",`
	`43`	`+ "which has a value {}",`
`44`	`44`	`self.inner.explain_match(t)`
`45`	`45`	`)),`
`46`		`- (_, None) => MatchExplanation::create("which is none".to_string()),`
	`46`	`+ (_, None) => MatchExplanation::create("which is None".to_string()),`
`47`	`47`	`}`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`fn describe(&self, matcher_result: MatcherResult) -> String {`
`51`	`51`	`match matcher_result {`
`52`	`52`	`MatcherResult::Matches => {`
`53`		`- format!("is some(x), where x {}", self.inner.describe(MatcherResult::Matches))`
	`53`	`+ format!("has a value which {}", self.inner.describe(MatcherResult::Matches))`
`54`	`54`	`}`
`55`	`55`	`MatcherResult::DoesNotMatch => {`
`56`	`56`	`format!(`
`57`		`- "is none or some(x), where x {}",`
	`57`	`+ "is None or has a value which {}",`
`58`	`58`	`self.inner.describe(MatcherResult::DoesNotMatch)`
`59`	`59`	`)`
`60`	`60`	`}`
`@@ -107,10 +107,10 @@ mod tests {`
`107`	`107`	`err(displays_as(contains_substring(`
`108`	`108`	`"\`
`109`	`109`	`Value of: Some(2)`
`110`		`-Expected: is some(x), where x is equal to 1`
	`110`	`+Expected: has a value which is equal to 1`
`111`	`111`	`Actual: Some(`
`112`	`112`	`2,`
`113`		`-), which contains 2, which isn't equal to 1`
	`113`	`+), which has a value which isn't equal to 1`
`114`	`114`	`"`
`115`	`115`	`)))`
`116`	`116`	`)`
`@@ -120,36 +120,36 @@ Actual: Some(`
`120`	`120`	`fn some_describe_matches() -> Result<()> {`
`121`	`121`	`verify_that!(`
`122`	`122`	`some(eq(1)).describe(MatcherResult::Matches),`
`123`		`- eq("is some(x), where x is equal to 1")`
	`123`	`+ eq("has a value which is equal to 1")`
`124`	`124`	`)`
`125`	`125`	`}`
`126`	`126`
`127`	`127`	`#[google_test]`
`128`	`128`	`fn some_describe_does_not_match() -> Result<()> {`
`129`	`129`	`verify_that!(`
`130`	`130`	`some(eq(1)).describe(MatcherResult::DoesNotMatch),`
`131`		`- eq("is none or some(x), where x isn't equal to 1")`
	`131`	`+ eq("is None or has a value which isn't equal to 1")`
`132`	`132`	`)`
`133`	`133`	`}`
`134`	`134`
`135`	`135`	`#[google_test]`
`136`	`136`	`fn some_explain_match_with_none() -> Result<()> {`
`137`		`- verify_that!(some(eq(1)).explain_match(&None), displays_as(eq("which is none")))`
	`137`	`+ verify_that!(some(eq(1)).explain_match(&None), displays_as(eq("which is None")))`
`138`	`138`	`}`
`139`	`139`
`140`	`140`	`#[google_test]`
`141`	`141`	`fn some_explain_match_with_some_success() -> Result<()> {`
`142`	`142`	`verify_that!(`
`143`	`143`	`some(eq(1)).explain_match(&Some(1)),`
`144`		`- displays_as(eq("which contains 1, which is equal to 1"))`
	`144`	`+ displays_as(eq("which has a value which is equal to 1"))`
`145`	`145`	`)`
`146`	`146`	`}`
`147`	`147`
`148`	`148`	`#[google_test]`
`149`	`149`	`fn some_explain_match_with_some_fail() -> Result<()> {`
`150`	`150`	`verify_that!(`
`151`	`151`	`some(eq(1)).explain_match(&Some(2)),`
`152`		`- displays_as(eq("which contains 2, which isn't equal to 1"))`
	`152`	`+ displays_as(eq("which has a value which isn't equal to 1"))`
`153`	`153`	`)`
`154`	`154`	`}`
`155`	`155`	`}`