Fix the use of the #[google_test] attribute macro in combination with other test attributes such as #[rstest].

Previously, there were some problems when using `#[google_test]` together with `#[rstest]`:

 * Placing `#[rstest]` after `#[google_test]` would result in a compiler warning.
 * Placing `#[rstest]` before `#[google_test]` would result in the test being executed twice.
 * Parameterised tests would only work if `#[rstest]` appeared first; otherwise they would result in a compiler error.

This makes two changes:

 * First, it changes the way the test method produced by `#[google_test]` is generated. Previously, it used a nested function using the same signature as the original test function. Now, it makes the *signature* of the *outer* test function that of the the *original* test function and places the *body* of the original test function in a `move` closure, which is then invoked in the body of the new test function. This should allow parameters being passed to tests to work transparently.

 * Second, it make the googletest procedural macro detect whether there are any other test attributes present and emit the `#[test]` attribute only if there aren't.

   Previously, it would emit `#[test]` in the generated code unconditionally, which sometimes resulted in multiple tests being generated or in annotated tests being generated on inner functions. Now, it avoids emitting `#[test]` when it sees that another attribute is present which would emit `#[test]`, so the test is generated correctly.

Unfortunately, these changes are insufficient to fix the problem that the test runs twice if `#[rstest]` appears *before* `#[google_test]`. That does not appear to be feasible within GoogleTest Rust alone, since the `#[google_test]` macro cannot see in that case that it was run with `#[rstest]`. For that reason, this also updates the documentation.

Fixes #77

PiperOrigin-RevId: 517924751

Author: hovinenb

README.md

@@ -206,23 +206,27 @@ as [rstest](https://crates.io/crates/rstest). Just apply both attribute macros
 to the test:
 
 ```rust
+#[google_test]
 #[rstest]
 #[case(1)]
 #[case(2)]
 #[case(3)]
-#[google_test]
 fn rstest_works_with_google_test(#[case] value: u32) -> Result<()> {
    verify_that!(value, gt(0))
 }
 ```
 
+Make sure to put `#[google_test]` *before* `#[rstest]`. Otherwise the annotated
+test will run twice, since both macros will attempt to register a test with the
+Rust test harness.
+
 The macro also works together with
 [async tests with Tokio](https://docs.rs/tokio/latest/tokio/attr.test.html) in
 the same way:
 
 ```rust
-#[tokio::test]
 #[google_test]
+#[tokio::test]
 async fn should_work_with_tokio() -> Result<()> {
     verify_that!(3, gt(0))
 }

googletest/Cargo.toml

@@ -38,6 +38,7 @@ googletest_macro = { path = "../googletest_macro", version = "0.4.1" }
 num-traits = "0.2.15"
 regex = "1.6.0"
 indoc = { version = "2", optional = true }
+rstest = { version = "0.16.0", optional = true }
 
 [dev-dependencies]
 indoc = "1"
@@ -98,6 +99,12 @@ name = "first_failure_aborts"
 path = "integration_tests/first_failure_aborts.rs"
 test = false
 
+[[bin]]
+name = "google_test_with_rstest"
+path = "integration_tests/google_test_with_rstest.rs"
+test = false
+required-feature = ["rstest"]
+
 [[bin]]
 name = "non_fatal_failure_in_subroutine"
 path = "integration_tests/non_fatal_failure_in_subroutine.rs"

googletest/integration_tests/google_test_with_rstest.rs

@@ -0,0 +1,65 @@
+// Copyright 2023 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() {}
+
+// Mixing rstest and google_test should not result in any compiler warnings. The
+// fact that this successfully compiles is part of the test.
+#[deny(warnings)]
+#[cfg(test)]
+mod tests {
+    #[cfg(not(google3))]
+    use googletest::matchers;
+    use googletest::{google_test, verify_that, Result};
+    use matchers::eq;
+    use rstest::rstest;
+
+    #[rstest]
+    #[google_test]
+    fn test_should_work_with_rstest_first() -> Result<()> {
+        verify_that!(1, eq(1))
+    }
+
+    #[rstest::rstest]
+    #[google_test]
+    fn test_should_work_with_qualified_rstest_first() -> Result<()> {
+        verify_that!(1, eq(1))
+    }
+
+    #[google_test]
+    #[rstest]
+    fn test_should_work_with_rstest_second() -> Result<()> {
+        verify_that!(1, eq(1))
+    }
+
+    #[google_test]
+    #[rstest::rstest]
+    fn test_should_work_with_qualified_rstest_second() -> Result<()> {
+        verify_that!(1, eq(1))
+    }
+
+    #[rstest]
+    #[case(1)]
+    #[google_test]
+    fn paramterised_test_should_work_with_rstest_first(#[case] value: u32) -> Result<()> {
+        verify_that!(value, eq(value))
+    }
+
+    #[google_test]
+    #[rstest]
+    #[case(1)]
+    fn paramterised_test_should_work_with_rstest_second(#[case] value: u32) -> Result<()> {
+        verify_that!(value, eq(value))
+    }
+}

googletest/integration_tests/integration_tests.rs

@@ -17,7 +17,7 @@ fn main() {}
 #[cfg(test)]
 mod tests {
     #[cfg(not(google3))]
-    use googletest::{all, matchers};
+    use googletest::{all, matchers, matchers::str_matcher};
     use googletest::{
         assert_that, expect_pred, expect_that, google_test, verify_pred, verify_that,
         GoogleTestSupport, Result,
@@ -27,6 +27,7 @@ mod tests {
     use matchers::all;
     use matchers::{anything, contains_regex, contains_substring, displays_as, eq, err, not};
     use std::process::Command;
+    use str_matcher::StrMatcherConfigurator;
 
     #[google_test]
     fn should_pass() -> Result<()> {
@@ -457,6 +458,20 @@ mod tests {
         )
     }
 
+    #[google_test]
+    fn test_with_google_test_and_rstest_runs_only_once() -> Result<()> {
+        let output = run_external_process_in_tests_directory("google_test_with_rstest")?;
+
+        expect_that!(
+            output,
+            contains_substring("tests::test_should_work_with_rstest_second").times(eq(1))
+        );
+        verify_that!(
+            output,
+            contains_substring("tests::test_should_work_with_qualified_rstest_second").times(eq(1))
+        )
+    }
+
     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()?;

googletest_macro/src/lib.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use quote::quote;
-use syn::{parse_macro_input, ItemFn};
+use syn::{parse_macro_input, Attribute, ItemFn, ReturnType};
 
 /// Marks a test to be run by the Google Rust test runner.
 ///
@@ -53,18 +53,46 @@ pub fn google_test(
     _args: proc_macro::TokenStream,
     input: proc_macro::TokenStream,
 ) -> proc_macro::TokenStream {
-    let parsed_fn = parse_macro_input!(input as ItemFn);
-    let fn_name = parsed_fn.sig.ident.clone();
-    let output = quote! {
-        #[test]
-        fn #fn_name() -> std::result::Result<(), ()> {
-            #parsed_fn
-
+    let mut parsed_fn = parse_macro_input!(input as ItemFn);
+    let attrs = parsed_fn.attrs.drain(..).collect::<Vec<_>>();
+    let (mut sig, block) = (parsed_fn.sig, parsed_fn.block);
+    let ReturnType::Type(_, output_type) = sig.output.clone() else {
+        return quote! {
+            compile_error!(
+                "Test function with the #[google_test] attribute must return googletest::Result<()>"
+            );
+        }.into();
+    };
+    sig.output = ReturnType::Default;
+    let function = quote! {
+        #(#attrs)*
+        #sig -> std::result::Result<(), ()> {
+            let test = move || #block;
             use googletest::internal::test_outcome::TestOutcome;
             TestOutcome::init_current_test_outcome();
-            let result = #fn_name();
+            let result: #output_type = test();
             TestOutcome::close_current_test_outcome(result)
         }
     };
+    let output = if attrs.iter().any(is_test_attribute) {
+        function
+    } else {
+        quote! {
+            #[test]
+            #function
+        }
+    };
     output.into()
 }
+
+fn is_test_attribute(attr: &Attribute) -> bool {
+    let Some(first_segment) = attr.path.segments.first() else {
+        return false;
+    };
+    let Some(last_segment) = attr.path.segments.last() else {
+        return false;
+    };
+    first_segment.ident == "rstest"
+        && last_segment.ident == "rstest"
+        && attr.path.segments.len() <= 2
+}

run_integration_tests.sh

@@ -34,6 +34,7 @@ INTEGRATION_TEST_BINARIES=(
   "failure_due_to_fail_macro_with_format_arguments"
   "failure_due_to_returned_error"
   "first_failure_aborts"
+  "google_test_with_rstest"
   "non_fatal_failure_in_subroutine"
   "simple_assertion_failure"
   "simple_assertion_failure_with_assert_that"
@@ -46,6 +47,6 @@ INTEGRATION_TEST_BINARIES=(
 
 cargo build
 for binary in ${INTEGRATION_TEST_BINARIES[@]}; do
-  cargo rustc -p googletest --bin $binary --features indoc -- --test
+  cargo rustc -p googletest --bin $binary --features indoc,rstest -- --test
 done
 ./target/debug/integration_tests