From b0de4d76000c53664ed17a69559531ef1ab57dd9 Mon Sep 17 00:00:00 2001 From: Bradford Hovinen Date: Fri, 4 Aug 2023 10:15:01 +0200 Subject: [PATCH] Support test functions returning () with `#[googletest::test]`. A test which only uses the `expect_*` family of assertions (thus requiring `#[googletest::test]`) will only ever return `Ok(())`. There is therefore no reason to require the function itself to return a `Result`. That requirement just creates unnecessary boilerplate. This change modifies the `#[googletest::test]` implementation to remove the requirement that the return type be present. If it is present, then it must still be a `Result`. The generated compiler error can no longer occur and is removed. It would be ideal to check whether the given return type is in fact a `Result` and return a compiler error if it is not, but that is beyond the scope of this change. --- README.md | 16 +++++- googletest/crate_docs.md | 17 +++++- googletest_macro/src/lib.rs | 66 +++++++++++----------- integration_tests/src/integration_tests.rs | 6 ++ 4 files changed, 69 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 67afb619..5a82bea5 100644 --- a/README.md +++ b/README.md @@ -181,7 +181,21 @@ This is analogous to the `EXPECT_*` family of macros in GoogleTest. To make a non-fatal assertion, use the macro [`expect_that!`]. The test must also be marked with [`googletest::test`] instead of the Rust-standard `#[test]`. -It must return [`Result<()>`]. + +```rust +use googletest::prelude::*; + +#[googletest::test] +fn three_non_fatal_assertions() { + let value = 2; + expect_that!(value, eq(2)); // Passes; test still considered passing. + expect_that!(value, eq(3)); // Fails; logs failure and marks the test failed. + expect_that!(value, eq(4)); // A second failure, also logged. +} +``` + +This can be used in the same tests as `verify_that!`, in which case the test +function must also return [`Result<()>`]: ```rust use googletest::prelude::*; diff --git a/googletest/crate_docs.md b/googletest/crate_docs.md index 1d32fe3f..9b3a5394 100644 --- a/googletest/crate_docs.md +++ b/googletest/crate_docs.md @@ -291,7 +291,22 @@ aborts. To make a non-fatal assertion, use the macro [`expect_that!`]. The test must also be marked with [`googletest::test`][test] instead of the Rust-standard -`#[test]`. It must return [`Result<()>`]. +`#[test]`. + +```no_run +use googletest::prelude::*; + +#[googletest::test] +fn three_non_fatal_assertions() { + let value = 2; + expect_that!(value, eq(2)); // Passes; test still considered passing. + expect_that!(value, eq(3)); // Fails; logs failure and marks the test failed. + expect_that!(value, eq(4)); // A second failure, also logged. +} +``` + +This can be used in the same tests as `verify_that!`, in which case the test +function must also return [`Result<()>`]: ```no_run use googletest::prelude::*; diff --git a/googletest_macro/src/lib.rs b/googletest_macro/src/lib.rs index 4fa0514e..87628bac 100644 --- a/googletest_macro/src/lib.rs +++ b/googletest_macro/src/lib.rs @@ -21,31 +21,22 @@ use syn::{parse_macro_input, Attribute, ItemFn, ReturnType}; /// /// ```ignore /// #[googletest::test] -/// fn should_work() -> googletest::Result { +/// fn should_work() { /// ... -/// Ok(()) /// } /// ``` /// -/// The test function should return [`googletest::Result`] so that one can use -/// `verify_that!` with the question mark operator to abort execution. The last -/// line of the test should return `Ok(())`. -/// -/// Any function your test invokes which contains a `verify_that!` call should -/// be invoked with the `?` operator so that a failure in the subroutine aborts -/// the rest of the test execution: +/// The test function is not required to have a return type. If it does have a +/// return type, that type must be [`googletest::Result`]. One may do this if +/// one wishes to use both fatal and non-fatal assertions in the same test. For +/// example: /// /// ```ignore /// #[googletest::test] -/// fn should_work() -> googletest::Result { -/// ... -/// assert_that_everything_is_okay()?; -/// do_some_more_stuff(); // Will not be executed if assert failed. -/// Ok(()) -/// } -/// -/// fn assert_that_everything_is_okay() -> googletest::Result { -/// verify_that!(...) +/// fn should_work() -> googletest::Result<()> { +/// let value = 2; +/// expect_that!(value, gt(0)); +/// verify_that!(value, eq(2)) /// } /// ``` /// @@ -59,14 +50,8 @@ pub fn test( let attrs = parsed_fn.attrs.drain(..).collect::>(); let (mut sig, block) = (parsed_fn.sig, parsed_fn.block); let output_type = match sig.output.clone() { - ReturnType::Type(_, output_type) => output_type, - _ => { - return quote! { - compile_error!( - "Test function with the #[googletest::test] attribute must return googletest::Result<()>" - ); - }.into() - } + ReturnType::Type(_, output_type) => Some(output_type), + ReturnType::Default => None, }; sig.output = ReturnType::Default; let (maybe_closure, invocation) = if sig.asyncness.is_some() { @@ -93,14 +78,27 @@ pub fn test( }, ) }; - let function = quote! { - #(#attrs)* - #sig -> std::result::Result<(), googletest::internal::test_outcome::TestFailure> { - #maybe_closure - use googletest::internal::test_outcome::TestOutcome; - TestOutcome::init_current_test_outcome(); - let result: #output_type = #invocation; - TestOutcome::close_current_test_outcome(result) + let function = if let Some(output_type) = output_type { + quote! { + #(#attrs)* + #sig -> std::result::Result<(), googletest::internal::test_outcome::TestFailure> { + #maybe_closure + use googletest::internal::test_outcome::TestOutcome; + TestOutcome::init_current_test_outcome(); + let result: #output_type = #invocation; + TestOutcome::close_current_test_outcome(result) + } + } + } else { + quote! { + #(#attrs)* + #sig -> std::result::Result<(), googletest::internal::test_outcome::TestFailure> { + #maybe_closure + use googletest::internal::test_outcome::TestOutcome; + TestOutcome::init_current_test_outcome(); + #invocation; + TestOutcome::close_current_test_outcome(googletest::Result::Ok(())) + } } }; let output = if attrs.iter().any(is_test_attribute) { diff --git a/integration_tests/src/integration_tests.rs b/integration_tests/src/integration_tests.rs index b2dd0f96..ae8cedcd 100644 --- a/integration_tests/src/integration_tests.rs +++ b/integration_tests/src/integration_tests.rs @@ -50,6 +50,12 @@ mod tests { Ok(()) } + #[googletest::test] + fn should_pass_with_expect_that_returning_unit() { + let value = 2; + expect_that!(value, eq(2)); + } + #[test] fn should_fail_on_assertion_failure() -> Result<()> { let status = run_external_process("simple_assertion_failure").status()?;