test_sharding: Emit panics when skipping should_panic tests in a sharding context.

Googler · copybara-github · commit cb8eaf9126bc · 2025-02-04T10:47:15.000-08:00
When a test uses `#[should_panic]` and the test is being skipped since it's not running in its assigned shard, if we just return `Ok(())`, that will break the test.
 To fix this, we simulate the expected panic to make sure the test skips successfully.

PiperOrigin-RevId: 723147758
diff --git a/googletest_macro/src/lib.rs b/googletest_macro/src/lib.rs
@@ -14,8 +14,8 @@
 
 use quote::quote;
 use syn::{
-    parse_macro_input, punctuated::Punctuated, spanned::Spanned, Attribute, DeriveInput, FnArg,
-    ItemFn, PatType, ReturnType, Signature, Type,
+    parse_macro_input, punctuated::Punctuated, spanned::Spanned, Attribute, DeriveInput, Expr,
+    ExprLit, FnArg, ItemFn, Lit, MetaNameValue, PatType, ReturnType, Signature, Type,
 };
 
 /// Marks a test to be run by the Google Rust test runner.
@@ -88,17 +88,26 @@ pub fn gtest(
         h.finish()
     };
 
-    let (outer_return_type, trailer) = if attrs
+    let (skipped_test_result, outer_return_type, trailer) = attrs
         .iter()
-        .any(|attr| attr.path().is_ident("should_panic"))
-    {
-        (quote! { () }, quote! { .unwrap(); })
-    } else {
+        .find(|attr| attr.path().is_ident("should_panic"))
+        .map(|attr| {
+            let error_message = extract_should_panic_expected(attr).unwrap_or("".to_string());
+            (
+                quote! {
+                    {
+                        panic!("{}", #error_message);
+                    }
+                },
+                quote! { () },
+                quote! { .unwrap(); }
+            )})
+        .unwrap_or_else(||
         (
+            quote! {Ok(())},
             quote! { ::std::result::Result<(), googletest::internal::test_outcome::TestFailure> },
             quote! {},
-        )
-    };
+        ));
 
     let is_rstest_enabled = is_rstest_enabled(&attrs);
     let outer_sig = {
@@ -186,7 +195,7 @@ pub fn gtest(
                 let result: #invocation_result_type = #invocation;
                 TestOutcome::close_current_test_outcome(#result)
             } else {
-                Ok(())
+                #skipped_test_result
             }
             #trailer
         }
@@ -204,6 +213,22 @@ pub fn gtest(
     output.into()
 }
 
+/// Extract the optional "expected" string literal from a `should_panic`
+/// attribute.
+fn extract_should_panic_expected(attr: &Attribute) -> Option<String> {
+    let Ok(name_value) = attr.parse_args::<MetaNameValue>() else {
+        return None;
+    };
+    match name_value.value {
+        Expr::Lit(ExprLit { lit: Lit::Str(expected), .. })
+            if name_value.path.is_ident("expected") =>
+        {
+            Some(expected.value())
+        }
+        _ => None,
+    }
+}
+
 /// Alias for [`googletest::gtest`].
 ///
 /// Generally, prefer using `#[gtest]` to mark googletest-based tests.
diff --git a/integration_tests/Cargo.toml b/integration_tests/Cargo.toml
@@ -46,6 +46,21 @@ name = "always_fails"
 path = "src/always_fails.rs"
 test = false
 
+[[bin]]
+name = "always_panics"
+path = "src/always_panics.rs"
+test = false
+
+[[bin]]
+name = "expect_panic"
+path = "src/expect_panic.rs"
+test = false
+
+[[bin]]
+name = "expect_panic_with_expected"
+path = "src/expect_panic_with_expected.rs"
+test = false
+
 [[bin]]
 name = "assert_predicate_with_failure"
 path = "src/assert_predicate_with_failure.rs"
diff --git a/integration_tests/src/always_panics.rs b/integration_tests/src/always_panics.rs
@@ -0,0 +1,25 @@
+// Copyright 2025 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::*;
+
+    #[gtest]
+    fn always_panics() -> Result<()> {
+        panic!("panic");
+    }
+}
diff --git a/integration_tests/src/expect_panic.rs b/integration_tests/src/expect_panic.rs
@@ -0,0 +1,26 @@
+// Copyright 2025 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::*;
+
+    #[gtest]
+    #[should_panic]
+    fn should_panic() -> Result<()> {
+        panic!("panic");
+    }
+}
diff --git a/integration_tests/src/expect_panic_with_expected.rs b/integration_tests/src/expect_panic_with_expected.rs
@@ -0,0 +1,31 @@
+// Copyright 2025 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::*;
+
+    #[gtest]
+    #[should_panic(expected = "<*)))‑{")]
+    // This emoticon is meant to validate that we're not accidentally
+    // macro-expanding the `expected` to a spot that is in format
+    // string position.  If we do, then the `{` character should cause
+    // a format-string parsing error.
+
+    fn should_panic() -> Result<()> {
+        panic!("{}", "This is a school of fish: <*)))‑{   <><");
+    }
+}
diff --git a/integration_tests/src/integration_tests.rs b/integration_tests/src/integration_tests.rs
@@ -752,24 +752,35 @@ mod tests {
 
     #[gtest]
     fn always_fails_test_respects_sharding() -> Result<()> {
-        fn execute_test(shard: u64, total_shards: u64) -> Result<bool> {
-            let status_file = tempfile::tempdir()?.path().join("shard_status_file");
-            let gtest_total_shards = format!("{total_shards}");
-            let gtest_shard_index = format!("{shard}");
-
-            let success = run_external_process("always_fails")
-                .env("GTEST_TOTAL_SHARDS", gtest_total_shards)
-                .env("GTEST_SHARD_INDEX", gtest_shard_index)
-                .env("GTEST_SHARD_STATUS_FILE", &status_file)
-                .status()?
-                .success();
-
-            verify_that!(status_file.exists(), eq(true))?;
-            Ok(success)
-        }
+        // The test case should only run and fail in one shard.
+        let results = [
+            execute_sharded_test("always_fails", 0, 3)?,
+            execute_sharded_test("always_fails", 1, 3)?,
+            execute_sharded_test("always_fails", 2, 3)?,
+        ];
+        let successes = results.iter().filter(|b| **b).count();
+        let failures = results.iter().filter(|b| !**b).count();
 
-        // The test case should only run in one shard.
-        let results = [execute_test(0, 3)?, execute_test(1, 3)?, execute_test(2, 3)?];
+        expect_that!(successes, eq(2));
+        expect_that!(failures, eq(1));
+        Ok(())
+    }
+
+    #[gtest]
+    fn always_panics_test_always_fails() -> Result<()> {
+        let status = run_external_process("always_panics").status()?;
+
+        verify_that!(status.success(), eq(false))
+    }
+
+    #[gtest]
+    fn always_panics_test_respects_sharding() -> Result<()> {
+        // The test case should only run and fail in one shard.
+        let results = [
+            execute_sharded_test("always_panics", 0, 3)?,
+            execute_sharded_test("always_panics", 1, 3)?,
+            execute_sharded_test("always_panics", 2, 3)?,
+        ];
         let successes = results.iter().filter(|b| **b).count();
         let failures = results.iter().filter(|b| !**b).count();
 
@@ -778,6 +789,41 @@ mod tests {
         Ok(())
     }
 
+    #[gtest]
+    fn expect_panic_test_always_succeeds() -> Result<()> {
+        let status = run_external_process("expect_panic").status()?;
+
+        verify_that!(status.success(), is_true())
+    }
+
+    #[gtest]
+    fn expect_panic_test_respects_sharding() -> Result<()> {
+        let results = [
+            execute_sharded_test("expect_panic", 0, 3)?,
+            execute_sharded_test("expect_panic", 1, 3)?,
+            execute_sharded_test("expect_panic", 2, 3)?,
+        ];
+        verify_that!(results.iter().all(|b| *b), is_true())
+    }
+
+    #[gtest]
+    fn expect_panic_with_expected_test_always_succeeds() -> Result<()> {
+        let status = run_external_process("expect_panic_with_expected").status()?;
+
+        verify_that!(status.success(), is_true())
+    }
+
+    #[gtest]
+    fn expect_panic_with_expected_test_respects_sharding() -> Result<()> {
+        let results = [
+            execute_sharded_test("expect_panic_with_expected", 0, 3)?,
+            execute_sharded_test("expect_panic_with_expected", 1, 3)?,
+            execute_sharded_test("expect_panic_with_expected", 2, 3)?,
+        ];
+
+        verify_that!(results.iter().all(|b| *b), is_true())
+    }
+
     #[gtest]
     fn verify_true_when_true_returns_ok() {
         assert!(verify_true!("test" == "test").is_ok())
@@ -1968,4 +2014,20 @@ mod tests {
         );
         Command::new(command_path)
     }
+
+    fn execute_sharded_test(process: &'static str, shard: u64, total_shards: u64) -> Result<bool> {
+        let status_file = tempfile::tempdir()?.path().join("shard_status_file");
+        let gtest_total_shards = format!("{total_shards}");
+        let gtest_shard_index = format!("{shard}");
+
+        let success = run_external_process(process)
+            .env("GTEST_TOTAL_SHARDS", gtest_total_shards)
+            .env("GTEST_SHARD_INDEX", gtest_shard_index)
+            .env("GTEST_SHARD_STATUS_FILE", &status_file)
+            .status()?
+            .success();
+
+        verify_that!(status_file.exists(), eq(true))?;
+        Ok(success)
+    }
 }
diff --git a/run_integration_tests.sh b/run_integration_tests.sh
@@ -25,6 +25,7 @@ set -e
 INTEGRATION_TEST_BINARIES=(
   "integration_tests"
   "always_fails"
+  "always_panics"
   "assert_predicate_with_failure"
   "assertion_failure_in_subroutine"
   "assertion_failures_with_short_structured_actual_values"
@@ -98,6 +99,8 @@ INTEGRATION_TEST_BINARIES=(
   "verify_predicate_with_failure"
   "verify_predicate_with_failure_as_method_in_submodule"
   "macro_hygiene"
+  "expect_panic"
+  "expect_panic_with_expected"
 )
 
 cargo build
