Merge pull request #528 from asomers/alt-mutex

Change the recommendation for static methods

mockall/examples/synchronization.rs

@@ -35,27 +35,19 @@ fn main() {
 mod test {
     use crate::my_mock::MockThing;
     use lazy_static::lazy_static;
-    use std::sync::{Mutex, MutexGuard};
+    use std::sync::Mutex;
 
     lazy_static! {
         static ref MTX: Mutex<()> = Mutex::new(());
     }
 
-    // When a test panics, it will poison the Mutex. Since we don't actually
-    // care about the state of the data we ignore that it is poisoned and grab
-    // the lock regardless.  If you just do `let _m = &MTX.lock().unwrap()`, one
-    // test panicking will cause all other tests that try and acquire a lock on
-    // that Mutex to also panic.
-    fn get_lock(m: &'static Mutex<()>) -> MutexGuard<'static, ()> {
-        match m.lock() {
-            Ok(guard) => guard,
-            Err(poisoned) => poisoned.into_inner(),
-        }
-    }
-
     #[test]
     fn test_1() {
-        let _m = get_lock(&MTX);
+        // The mutex might be poisoned if another test fails.  But we don't
+        // care, because it doesn't hold any data.  So don't unwrap the Result
+        // object; whether it's poisoned or not, we'll still hold the
+        // MutexGuard.
+        let _m = MTX.lock();
 
         let ctx = MockThing::one_context();
         ctx.expect().returning(|| 1);
@@ -65,7 +57,7 @@ mod test {
 
     #[test]
     fn test_2() {
-        let _m = get_lock(&MTX);
+        let _m = MTX.lock();
 
         let ctx = MockThing::one_context();
         ctx.expect().returning(|| 2);

mockall/tests/automock_foreign_c.rs

@@ -8,10 +8,6 @@ use std::sync::Mutex;
 mod ffi {
     extern "C" {
         pub(super) fn foo(x: u32) -> i64;
-        // Every should_panic method needs to operate on a separate method so it
-        // doesn't poison other tests
-        #[allow(dead_code)]
-        pub(super) fn foo1(x: u32) -> i64;
     }
 }
 
@@ -21,24 +17,26 @@ lazy_static! {
 
 // Ensure we can still use the original mocked function
 pub fn normal_usage() {
+    let _m = FOO_MTX.lock();
     unsafe {
         ffi::foo(42);
     }
 }
 
 #[test]
-#[should_panic(expected = "mock_ffi::foo1(5): No matching expectation found")]
+#[should_panic(expected = "mock_ffi::foo(5): No matching expectation found")]
 fn with_no_matches() {
-    let ctx = mock_ffi::foo1_context();
+    let _m = FOO_MTX.lock();
+    let ctx = mock_ffi::foo_context();
     ctx.expect()
         .with(predicate::eq(4))
         .returning(i64::from);
-    unsafe{ mock_ffi::foo1(5) };
+    unsafe{ mock_ffi::foo(5) };
 }
 
 #[test]
 fn returning() {
-    let _m = FOO_MTX.lock().unwrap();
+    let _m = FOO_MTX.lock();
     let ctx = mock_ffi::foo_context();
     ctx.expect().returning(i64::from);
     assert_eq!(42, unsafe{mock_ffi::foo(42)});
@@ -48,7 +46,7 @@ fn returning() {
 /// function pointer.
 #[test]
 fn c_abi() {
-    let _m = FOO_MTX.lock().unwrap();
+    let _m = FOO_MTX.lock();
     let ctx = mock_ffi::foo_context();
     ctx.expect().returning(i64::from);
     let p: unsafe extern "C" fn(u32) -> i64 = mock_ffi::foo;

mockall/tests/automock_module.rs

@@ -2,34 +2,40 @@
 //! Mocking an entire module of functions
 #![deny(warnings)]
 
+
 pub mod m {
+    use std::sync::Mutex;
     use mockall::*;
     type T = u32;
 
+    lazy_static! {
+        static ref BAR_MTX: Mutex<()> = Mutex::new(());
+    }
+
     #[automock]
     pub mod foo {
         use super::T;
         pub fn bar(_x: T) -> i64 {unimplemented!()}
-        // We must have a separate method for every should_panic test
-        pub fn bar1(_x: T) -> i64 {unimplemented!()}
         // Module functions should be able to use impl Trait, too
         pub fn baz() -> impl std::fmt::Debug + Send { unimplemented!()}
         // Module functions can use mutable arguments
         pub fn bean(mut _x: u32) { unimplemented!() }
     }
 
     #[test]
-    #[should_panic(expected = "mock_foo::bar1(5): No matching expectation found")]
+    #[should_panic(expected = "mock_foo::bar(5): No matching expectation found")]
     fn with_no_matches() {
-        let ctx = mock_foo::bar1_context();
+        let _m = BAR_MTX.lock();
+        let ctx = mock_foo::bar_context();
         ctx.expect()
             .with(predicate::eq(4))
             .return_const(0);
-        mock_foo::bar1(5);
+        mock_foo::bar(5);
     }
 
     #[test]
     fn returning() {
+        let _m = BAR_MTX.lock();
         let ctx = mock_foo::bar_context();
         ctx.expect()
             .returning(|x| i64::from(x) + 1);

mockall/tests/mock_generic_struct_with_generic_static_method.rs

@@ -8,10 +8,6 @@ use std::sync::Mutex;
 mock! {
     Foo<T: 'static + std::fmt::Debug> {
         fn foo<Q: 'static + std::fmt::Debug>(t: T, q: Q) -> u64;
-        // We must use a different method for every should_panic test, so the
-        // shared mutex doesn't get poisoned.
-        fn foo2<Q: 'static + std::fmt::Debug>(t: T, q: Q) -> u64;
-        fn foo3<Q: 'static + std::fmt::Debug>(t: T, q: Q) -> u64;
     }
 }
 
@@ -22,7 +18,7 @@ lazy_static! {
 // Checkpointing the mock object should not checkpoint static methods too
 #[test]
 fn checkpoint() {
-    let _m = FOO_MTX.lock().unwrap();
+    let _m = FOO_MTX.lock();
 
     let mut mock = MockFoo::<u32>::new();
     let ctx = MockFoo::<u32>::foo_context();
@@ -36,9 +32,10 @@ fn checkpoint() {
 // It should also be possible to checkpoint just the context object
 #[test]
 #[should_panic(expected =
-    "MockFoo::foo2: Expectation(<anything>) called 0 time(s) which is fewer than expected 1")]
+    "MockFoo::foo: Expectation(<anything>) called 0 time(s) which is fewer than expected 1")]
 fn ctx_checkpoint() {
-    let ctx = MockFoo::<u32>::foo2_context();
+    let _m = FOO_MTX.lock();
+    let ctx = MockFoo::<u32>::foo_context();
     ctx.expect::<i16>()
         .returning(|_, _| 0)
         .times(1..3);
@@ -48,21 +45,22 @@ fn ctx_checkpoint() {
 
 // Expectations should be cleared when a context object drops
 #[test]
-#[should_panic(expected = "MockFoo::foo3(42, 69): No matching expectation found")]
+#[should_panic(expected = "MockFoo::foo(42, 69): No matching expectation found")]
 fn ctx_hygiene() {
+    let _m = FOO_MTX.lock();
     {
-        let ctx0 = MockFoo::<u32>::foo3_context();
+        let ctx0 = MockFoo::<u32>::foo_context();
         ctx0.expect::<i16>()
             .returning(|_, _| 0);
     }
-    MockFoo::foo3(42, 69);
+    MockFoo::foo(42, 69);
 }
 
 #[cfg_attr(not(feature = "nightly"), ignore)]
 #[cfg_attr(not(feature = "nightly"), allow(unused_must_use))]
 #[test]
 fn return_default() {
-    let _m = FOO_MTX.lock().unwrap();
+    let _m = FOO_MTX.lock();
 
     let ctx = MockFoo::<u32>::foo_context();
     ctx.expect::<i16>();
@@ -71,7 +69,7 @@ fn return_default() {
 
 #[test]
 fn returning() {
-    let _m = FOO_MTX.lock().unwrap();
+    let _m = FOO_MTX.lock();
 
     let ctx = MockFoo::<u32>::foo_context();
     ctx.expect::<i16>()
@@ -81,7 +79,7 @@ fn returning() {
 
 #[test]
 fn two_matches() {
-    let _m = FOO_MTX.lock().unwrap();
+    let _m = FOO_MTX.lock();
 
     let ctx = MockFoo::<u32>::foo_context();
     ctx.expect::<i16>()
@@ -96,7 +94,7 @@ fn two_matches() {
 
 #[test]
 fn with() {
-    let _m = FOO_MTX.lock().unwrap();
+    let _m = FOO_MTX.lock();
 
     let ctx = MockFoo::<u32>::foo_context();
     ctx.expect::<i16>()

mockall/tests/mock_struct_with_static_method.rs

@@ -7,9 +7,6 @@ use std::sync::Mutex;
 mock!{
     Foo {
         fn bar(x: u32) -> u64;
-        // We must have a separate method for every should_panic test
-        fn bar2(x: u32) -> u64;
-        fn bar3(x: u32) -> u64;
     }
 }
 
@@ -20,7 +17,7 @@ lazy_static! {
 // Checkpointing the mock object should not checkpoint static methods
 #[test]
 fn checkpoint() {
-    let _m = BAR_MTX.lock().unwrap();
+    let _m = BAR_MTX.lock();
 
     let mut mock = MockFoo::new();
     let ctx = MockFoo::bar_context();
@@ -34,9 +31,11 @@ fn checkpoint() {
 // It should also be possible to checkpoint just the context object
 #[test]
 #[should_panic(expected =
-    "MockFoo::bar2: Expectation(<anything>) called 0 time(s) which is fewer than expected 1")]
+    "MockFoo::bar: Expectation(<anything>) called 0 time(s) which is fewer than expected 1")]
 fn ctx_checkpoint() {
-    let ctx = MockFoo::bar2_context();
+    let _m = BAR_MTX.lock();
+
+    let ctx = MockFoo::bar_context();
     ctx.expect()
         .returning(|_| 32)
         .times(1..3);
@@ -46,19 +45,21 @@ fn ctx_checkpoint() {
 
 // Expectations should be cleared when a context object drops
 #[test]
-#[should_panic(expected = "MockFoo::bar3(42): No matching expectation found")]
+#[should_panic(expected = "MockFoo::bar(42): No matching expectation found")]
 fn ctx_hygiene() {
+    let _m = BAR_MTX.lock();
+
     {
-        let ctx0 = MockFoo::bar3_context();
+        let ctx0 = MockFoo::bar_context();
         ctx0.expect()
             .returning(|x| u64::from(x + 1));
     }
-    MockFoo::bar3(42);
+    MockFoo::bar(42);
 }
 
 #[test]
 fn return_const() {
-    let _m = BAR_MTX.lock().unwrap();
+    let _m = BAR_MTX.lock();
 
     let ctx = MockFoo::bar_context();
     ctx.expect()
@@ -70,7 +71,7 @@ fn return_const() {
 #[cfg_attr(not(feature = "nightly"), allow(unused_must_use))]
 #[test]
 fn return_default() {
-    let _m = BAR_MTX.lock().unwrap();
+    let _m = BAR_MTX.lock();
 
     let ctx = MockFoo::bar_context();
     ctx.expect();
@@ -80,7 +81,7 @@ fn return_default() {
 
 #[test]
 fn returning() {
-    let _m = BAR_MTX.lock().unwrap();
+    let _m = BAR_MTX.lock();
 
     let ctx = MockFoo::bar_context();
     ctx.expect()
@@ -90,7 +91,7 @@ fn returning() {
 
 #[test]
 fn two_matches() {
-    let _m = BAR_MTX.lock().unwrap();
+    let _m = BAR_MTX.lock();
 
     let ctx = MockFoo::bar_context();
     ctx.expect()