Ensure that the assertion failure message of a fatal test failure is output even if there are preceding non-fatal errors in the test.

There was a bug in which, if a test had a mix of fatal and non-fatal assertions, and a non-fatal assertion failed, then the test assertion failure message of a subsequent fatal assertion failure would not be output. This did not affect the overall test result but would be confusing and misleading by not including all failure messages. This change fixes the bug and introduces an integration test to protect against it.

Author: hovinen

googletest/src/internal/test_outcome.rs

@@ -62,23 +62,23 @@ impl TestOutcome {
     ///
     /// **For internal use only. API stablility is not guaranteed!**
     #[doc(hidden)]
-    pub fn close_current_test_outcome<E: Display>(result: Result<(), E>) -> Result<(), ()> {
+    pub fn close_current_test_outcome<E: Display>(inner_result: Result<(), E>) -> Result<(), ()> {
         TestOutcome::with_current_test_outcome(|mut outcome| {
-            let result = match &*outcome {
-                Some(TestOutcome::Success) => match result {
+            let outer_result = match &*outcome {
+                Some(TestOutcome::Success) => match inner_result {
                     Ok(()) => Ok(()),
-                    Err(f) => {
-                        print!("{}", f);
-                        Err(())
-                    }
+                    Err(_) => Err(()),
                 },
                 Some(TestOutcome::Failure) => Err(()),
                 None => {
                     panic!("No test context found. This indicates a bug in GoogleTest.")
                 }
             };
+            if let Err(fatal_assertion_failure) = inner_result {
+                println!("{fatal_assertion_failure}");
+            }
             *outcome = None;
-            result
+            outer_result
         })
     }
 

integration_tests/Cargo.toml

@@ -94,6 +94,11 @@ name = "failure_due_to_returned_error"
 path = "src/failure_due_to_returned_error.rs"
 test = false
 
+[[bin]]
+name = "fatal_and_non_fatal_failure"
+path = "src/fatal_and_non_fatal_failure.rs"
+test = false
+
 [[bin]]
 name = "first_failure_aborts"
 path = "src/first_failure_aborts.rs"

integration_tests/src/fatal_and_non_fatal_failure.rs

@@ -0,0 +1,28 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+fn main() {}
+
+#[cfg(test)]
+mod tests {
+    use googletest::prelude::*;
+
+    #[googletest::test]
+    fn fatal_and_non_fatal_failure() -> Result<()> {
+        let value = 2;
+        verify_that!(value, eq(3)).and_log_failure();
+        verify_that!(value, eq(4))?;
+        Ok(())
+    }
+}

integration_tests/src/integration_tests.rs

@@ -126,8 +126,8 @@ mod tests {
     }
 
     #[test]
-    fn should_output_second_failure_message_on_second_assertion_failure_with_expect_that()
-    -> Result<()> {
+    fn should_output_second_failure_message_on_second_assertion_failure_with_expect_that(
+    ) -> Result<()> {
         let output = run_external_process_in_tests_directory("two_expect_that_failures")?;
 
         verify_that!(
@@ -220,6 +220,27 @@ mod tests {
         )
     }
 
+    #[test]
+    fn should_log_fatal_and_non_fatal_errors_to_stdout() -> Result<()> {
+        let output = run_external_process_in_tests_directory("fatal_and_non_fatal_failure")?;
+
+        verify_that!(
+            output,
+            all!(
+                contains_substring(indoc! {"
+                    Expected: is equal to 3
+                    Actual: 2,
+                      which isn't equal to 3
+                    "}),
+                contains_substring(indoc! {"
+                    Expected: is equal to 4
+                    Actual: 2,
+                      which isn't equal to 4
+                    "})
+            )
+        )
+    }
+
     #[test]
     fn should_abort_after_first_failure() -> Result<()> {
         let output = run_external_process_in_tests_directory("first_failure_aborts")?;

run_integration_tests.sh

@@ -35,6 +35,7 @@ INTEGRATION_TEST_BINARIES=(
   "failure_due_to_fail_macro_with_empty_message"
   "failure_due_to_fail_macro_with_format_arguments"
   "failure_due_to_returned_error"
+  "fatal_and_non_fatal_failure"
   "first_failure_aborts"
   "google_test_with_rstest"
   "non_fatal_failure_in_subroutine"