From 7042ff0d4b057690d4207fcaaa2aa25666ebad8d Mon Sep 17 00:00:00 2001 From: Bradford Hovinen Date: Fri, 7 Jul 2023 17:38:51 +0200 Subject: [PATCH] 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. --- googletest/src/internal/test_outcome.rs | 16 +++++------ integration_tests/Cargo.toml | 5 ++++ .../src/fatal_and_non_fatal_failure.rs | 28 +++++++++++++++++++ integration_tests/src/integration_tests.rs | 25 +++++++++++++++-- run_integration_tests.sh | 1 + 5 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 integration_tests/src/fatal_and_non_fatal_failure.rs diff --git a/googletest/src/internal/test_outcome.rs b/googletest/src/internal/test_outcome.rs index f21f2f8f..da526763 100644 --- a/googletest/src/internal/test_outcome.rs +++ b/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(result: Result<(), E>) -> Result<(), ()> { + pub fn close_current_test_outcome(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 }) } diff --git a/integration_tests/Cargo.toml b/integration_tests/Cargo.toml index 5d67582c..12e141b6 100644 --- a/integration_tests/Cargo.toml +++ b/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" diff --git a/integration_tests/src/fatal_and_non_fatal_failure.rs b/integration_tests/src/fatal_and_non_fatal_failure.rs new file mode 100644 index 00000000..ed95d9cd --- /dev/null +++ b/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(()) + } +} diff --git a/integration_tests/src/integration_tests.rs b/integration_tests/src/integration_tests.rs index 513c4b47..2295b275 100644 --- a/integration_tests/src/integration_tests.rs +++ b/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")?; diff --git a/run_integration_tests.sh b/run_integration_tests.sh index ceaa09af..0fddc556 100644 --- a/run_integration_tests.sh +++ b/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"