From 04e8c93894489abdc4fcf7f01701c4a14b2b4dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Medina?= Date: Sat, 19 Mar 2022 08:22:33 -0700 Subject: [PATCH] restructured error handling in mock store --- Cargo.toml | 1 - faux_macros/src/create.rs | 3 +- faux_macros/src/methods/morphed.rs | 12 ++--- src/lib.rs | 73 +++++++++++++++++++++++------- src/mock.rs | 41 +++++++++++------ src/mock/store.rs | 58 ++++++++++++++++++++---- src/mock/stub.rs | 38 ++++++++++++---- src/mock/unchecked.rs | 4 +- src/when.rs | 19 +++++--- src/when/once.rs | 19 ++++++-- 10 files changed, 200 insertions(+), 68 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bcf5588..18e1b31 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,6 @@ readme = "README.md" [dependencies] faux_macros = { path = "faux_macros", version = "0.1.6" } paste = "1.0.4" -parking_lot = "0.11.2" [dev-dependencies] futures = "0.3.9" diff --git a/faux_macros/src/create.rs b/faux_macros/src/create.rs index 365c45c..52d2cf6 100644 --- a/faux_macros/src/create.rs +++ b/faux_macros/src/create.rs @@ -52,13 +52,14 @@ impl From for proc_macro::TokenStream { let Mockable { real, morphed } = mockable; let (impl_generics, ty_generics, where_clause) = real.generics.split_for_impl(); let name = &morphed.ident; + let name_str = name.to_string(); proc_macro::TokenStream::from(quote! { #morphed impl #impl_generics #name #ty_generics #where_clause { pub fn faux() -> Self { - Self(faux::MaybeFaux::faux()) + Self(faux::MaybeFaux::faux(#name_str)) } } diff --git a/faux_macros/src/methods/morphed.rs b/faux_macros/src/methods/morphed.rs index 4b797b2..392e2f4 100644 --- a/faux_macros/src/methods/morphed.rs +++ b/faux_macros/src/methods/morphed.rs @@ -215,15 +215,13 @@ impl<'a> Signature<'a> { quote! { (#(#args,)*) } }; - let struct_and_method_name = - format!("{}::{}", morphed_ty.to_token_stream(), name); + let fn_name = name.to_string(); + quote! { unsafe { - match q.call_stub(::#faux_ident, #args) { + match q.call_stub(::#faux_ident, #fn_name, #args) { std::result::Result::Ok(o) => o, - std::result::Result::Err(e) => { - panic!("failed to call stub on '{}':\n{}", #struct_and_method_name, e); - } + std::result::Result::Err(e) => panic!("{}", e), } } } @@ -322,12 +320,14 @@ impl<'a> MethodData<'a> { let empty = syn::parse_quote! { () }; let output = output.unwrap_or(&empty); + let name_str = name.to_string(); let when_method = syn::parse_quote! { pub fn #when_ident<'m>(&'m mut self) -> faux::When<'m, #receiver_tokens, (#(#arg_types),*), #output, faux::matcher::AnyInvocation> { match &mut self.0 { faux::MaybeFaux::Faux(faux) => faux::When::new( ::#faux_ident, + #name_str, faux ), faux::MaybeFaux::Real(_) => panic!("not allowed to stub a real instance!"), diff --git a/src/lib.rs b/src/lib.rs index 3d5ea9a..2ab67ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -900,6 +900,8 @@ pub use matcher::ArgMatcher; mod mock; +use core::fmt; +use std::fmt::Formatter; use std::sync::Arc; /// What all mockable structs get transformed into. @@ -945,8 +947,8 @@ impl Default for MaybeFaux { } impl MaybeFaux { - pub fn faux() -> Self { - MaybeFaux::Faux(Faux::default()) + pub fn faux(name: &'static str) -> Self { + MaybeFaux::Faux(Faux::new(name)) } } @@ -956,17 +958,23 @@ impl MaybeFaux { /// documented. Its mere existence is an implementation detail and not /// meant to be relied upon. #[doc(hidden)] -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct Faux { - store: Arc, + store: Arc>, } impl Faux { + pub fn new(name: &'static str) -> Self { + Faux { + store: Arc::new(mock::Store::new(name)), + } + } + /// Return a mutable reference to its internal mock store /// /// Returns `None` if the store is being shared by multiple mock /// instances. This occurs when cloning a mock instance. - pub(crate) fn unique_store(&mut self) -> Option<&mut mock::Store> { + pub(crate) fn unique_store(&mut self) -> Option<&mut mock::Store<'static>> { Arc::get_mut(&mut self.store) } @@ -983,19 +991,52 @@ impl Faux { /// /// Do *NOT* call this function directly. /// This should only be called by the generated code from #[faux::methods] - pub unsafe fn call_stub(&self, id: fn(R, I) -> O, input: I) -> Result { - let mock = self - .store - .get(id) - .ok_or_else(|| "✗ method was never stubbed".to_owned())?; + pub unsafe fn call_stub( + &self, + id: fn(R, I) -> O, + fn_name: &'static str, + input: I, + ) -> Result { + let mock = self.store.get(id, fn_name)?; + mock.call(input).map_err(|stub_error| InvocationError { + fn_name: mock.name(), + struct_name: self.store.struct_name, + stub_error, + }) + } +} + +pub struct InvocationError { + struct_name: &'static str, + fn_name: &'static str, + stub_error: mock::InvocationError, +} - mock.call(input).map_err(|errors| { - if errors.is_empty() { - "✗ method was never stubbed".to_owned() - } else { - errors.join("\n\n") +impl fmt::Display for InvocationError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match &self.stub_error { + mock::InvocationError::NeverStubbed => write!( + f, + "`{}::{}` was called but never stubbed", + self.struct_name, self.fn_name + ), + mock::InvocationError::Stub(errors) => { + writeln!( + f, + "`{}::{}` had no suitable stubs. Existing stubs failed because:", + self.struct_name, self.fn_name + )?; + let mut errors = errors.iter(); + if let Some(e) = errors.next() { + f.write_str("✗ ")?; + fmt::Display::fmt(e, f)?; + } + errors.try_for_each(|e| { + f.write_str("\n\n✗ ")?; + fmt::Display::fmt(e, f) + }) } - }) + } } } diff --git a/src/mock.rs b/src/mock.rs index e3bdaec..2c61755 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -3,24 +3,29 @@ pub mod stub; mod store; mod unchecked; -use std::fmt::{self, Formatter}; +use std::{ + fmt::{self, Formatter}, + sync::Mutex, +}; pub use self::{store::Store, stub::Stub}; -use parking_lot::Mutex; - /// A function mock /// /// Stores information about a mock, such as its stubs, with its /// inputs and output typed. pub struct Mock<'stub, I, O> { - pub(super) stubs: Vec>>, + fn_name: &'static str, + stubs: Vec>>, } impl<'stub, I, O> Mock<'stub, I, O> { /// Creates an empty mock - pub fn new() -> Self { - Self { stubs: vec![] } + pub fn new(fn_name: &'static str) -> Self { + Self { + fn_name, + stubs: vec![], + } } /// Attempts to invoke the mock @@ -30,34 +35,42 @@ impl<'stub, I, O> Mock<'stub, I, O> { /// inputs. The stubs are checked in reverse insertion order such /// that the last inserted stub is the first attempted /// one. Returns an error if no stub is found for the given input. - pub fn call(&self, mut input: I) -> Result> { + pub fn call(&self, mut input: I) -> Result { let mut errors = vec![]; for stub in self.stubs.iter().rev() { - match stub.lock().call(input) { + match stub.lock().unwrap().call(input) { Err((i, e)) => { - errors.push(format!("✗ {}", e)); + errors.push(e); input = i } Ok(o) => return Ok(o), } } - Err(errors) + Err(if errors.is_empty() { + InvocationError::NeverStubbed + } else { + InvocationError::Stub(errors) + }) } /// Adds a new stub for the mocked function pub fn add_stub(&mut self, stub: Stub<'stub, I, O>) { self.stubs.push(Mutex::new(stub)) } -} -impl Default for Mock<'_, I, O> { - fn default() -> Self { - Self::new() + pub fn name(&self) -> &'static str { + self.fn_name } } +#[derive(Debug)] +pub enum InvocationError { + NeverStubbed, + Stub(Vec), +} + impl fmt::Debug for Mock<'_, I, O> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { f.debug_struct("Mock").field("stubs", &self.stubs).finish() diff --git a/src/mock/store.rs b/src/mock/store.rs index 9449e75..f2d0368 100644 --- a/src/mock/store.rs +++ b/src/mock/store.rs @@ -1,30 +1,70 @@ use std::collections::HashMap; +use crate::InvocationError; + use super::{unchecked::Unchecked, Mock}; -#[derive(Debug, Default)] -pub struct Store { - stubs: HashMap>, +#[derive(Debug)] +pub struct Store<'stub> { + pub struct_name: &'static str, + stubs: HashMap>, } -impl Store { +impl<'stub> Store<'stub> { + pub fn new(struct_name: &'static str) -> Self { + Store { + struct_name, + stubs: HashMap::new(), + } + } + /// Returns a mutable reference to a [`Mock`] for a given function /// /// If the given function has not yet been mocked, an empty mock /// is created for the function. - pub fn get_or_create(&mut self, id: fn(R, I) -> O) -> &mut Mock { + pub fn get_or_create( + &mut self, + id: fn(R, I) -> O, + fn_name: &'static str, + ) -> &mut Mock<'stub, I, O> { let mock = self.stubs.entry(id as usize).or_insert_with(|| { - let mock: Mock = Mock::new(); + let mock: Mock = Mock::new(fn_name); mock.into() }); - unsafe { mock.as_typed_mut() } + let mock = unsafe { mock.as_typed_mut() }; + assert_name(mock, fn_name); + mock } /// Returns a reference to a [`Mock`] for a given function /// /// `None` is returned if the function was never mocked - pub unsafe fn get(&self, id: fn(R, I) -> O) -> Option<&Mock> { - self.stubs.get(&(id as usize)).map(|m| m.as_typed()) + pub unsafe fn get( + &self, + id: fn(R, I) -> O, + fn_name: &'static str, + ) -> Result<&Mock<'stub, I, O>, InvocationError> { + match self.stubs.get(&(id as usize)).map(|m| m.as_typed()) { + Some(mock) => { + assert_name(mock, fn_name); + Ok(mock) + } + None => Err(InvocationError { + fn_name, + struct_name: self.struct_name, + stub_error: super::InvocationError::NeverStubbed, + }), + } } } + +fn assert_name(mock: &Mock, fn_name: &'static str) { + assert_eq!( + mock.name(), + fn_name, + "faux bug: conflicting mock names: '{}' vs '{}'", + mock.name(), + fn_name + ); +} diff --git a/src/mock/stub.rs b/src/mock/stub.rs index 73a45cc..bdfafe1 100644 --- a/src/mock/stub.rs +++ b/src/mock/stub.rs @@ -25,6 +25,23 @@ pub enum Times { Times(NonZeroUsize), } +#[derive(Debug)] +pub enum Error { + Exhausted, + NotMatched(String), +} + +impl std::error::Error for Error {} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + Error::Exhausted => f.write_str("stub was exhausted"), + Error::NotMatched(error) => f.write_str(error), + } + } +} + impl Times { pub fn decrement(self) -> Option { match self { @@ -45,10 +62,10 @@ impl<'a, I, O> Stub<'a, I, O> { } } - pub fn call(&mut self, input: I) -> Result { + pub fn call(&mut self, input: I) -> Result { // TODO: should the error message be different if the stub is also exhausted? if let Err(e) = self.matcher.matches(&input) { - return Err((input, e)); + return Err((input, Error::NotMatched(e))); } self.answer.call(input) @@ -56,7 +73,7 @@ impl<'a, I, O> Stub<'a, I, O> { } impl<'a, I, O> Answer<'a, I, O> { - fn call(&mut self, input: I) -> Result { + fn call(&mut self, input: I) -> Result { // no need to replace if we can keep decrementing if let Answer::Many { stub, times } = self { if let Some(decremented) = times.decrement() { @@ -67,7 +84,7 @@ impl<'a, I, O> Answer<'a, I, O> { // otherwise replace it with an exhaust match std::mem::replace(self, Answer::Exhausted) { - Answer::Exhausted => Err((input, "this stub has been exhausted".to_string())), + Answer::Exhausted => Err((input, Error::Exhausted)), Answer::Once(stub) => Ok(stub(input)), Answer::Many { mut stub, .. } => Ok(stub(input)), } @@ -79,11 +96,14 @@ impl<'a, I, O> fmt::Debug for Stub<'a, I, O> { f.debug_struct("Stub") // TODO: Add debug information for InvocationMatcher // .field("matcher", &self.matcher) - .field("answer", match &self.answer { - Answer::Exhausted => &"Exhausted", - Answer::Once(_) => &"Once", - Answer::Many { .. } => &"Many", - }) + .field( + "answer", + match &self.answer { + Answer::Exhausted => &"Exhausted", + Answer::Once(_) => &"Once", + Answer::Many { .. } => &"Many", + }, + ) .finish() } } diff --git a/src/mock/unchecked.rs b/src/mock/unchecked.rs index 52d0ae5..34ee4dd 100644 --- a/src/mock/unchecked.rs +++ b/src/mock/unchecked.rs @@ -24,7 +24,7 @@ impl<'stub> Unchecked<'stub> { /// This method is *extremely* unsafe. This is only safe if you /// know precisely what the input (I), output (O) were of the /// original [`Mock`] this came from. - pub unsafe fn as_typed(&self) -> &Mock { + pub unsafe fn as_typed(&self) -> &Mock<'stub, I, O> { // Might be safer to only transmute only the matcher and stub // of each mock instead of the entire object. This works // though, and I don't see any reason why it wouldn't but if @@ -42,7 +42,7 @@ impl<'stub> Unchecked<'stub> { /// This method is *extremely* unsafe. This is only safe if you /// know precisely what the input (I), output (O) were of the /// original [`Mock`] this came from. - pub unsafe fn as_typed_mut(&mut self) -> &mut Mock { + pub unsafe fn as_typed_mut(&mut self) -> &mut Mock<'stub, I, O> { // Might be safer to only transmute only the matcher and stub // of each mock instead of the entire object. This works // though, and I don't see any reason why it wouldn't but if diff --git a/src/when.rs b/src/when.rs index 03b0d5a..c713e13 100644 --- a/src/when.rs +++ b/src/when.rs @@ -36,22 +36,28 @@ use stub::Stub; /// [`times`]: When::times /// [`with_args`]: When::with_args pub struct When<'m, R, I, O, M: InvocationMatcher> { + // Set at creation and immutable. Could be replaced with just `&'m + // mut Mock<'static, I,O>` but that makes `When` no longer be + // contravariat on `I` which makes some valid code not compile. id: fn(R, I) -> O, - store: &'m mut mock::Store, - times: Option, + name: &'static str, + store: &'m mut mock::Store<'static>, + // defaulted at creation but mutable + times: Option, matcher: M, } impl<'m, R, I, O> When<'m, R, I, O, AnyInvocation> { #[doc(hidden)] - pub fn new(id: fn(R, I) -> O, faux: &'m mut Faux) -> Self { + pub fn new(id: fn(R, I) -> O, name: &'static str, faux: &'m mut Faux) -> Self { let store = faux.unique_store().expect("faux: failed to get unique handle to mock. Adding stubs to a mock instance may only be done prior to cloning the mock."); When { id, + name, store, matcher: AnyInvocation, - times: Some(mock::stub::Times::Always), + times: Some(stub::Times::Always), } } } @@ -446,7 +452,7 @@ impl<'m, R, I, O, M: InvocationMatcher + Send + 'static> When<'m, R, I, O, M> /// } /// ``` pub fn once(self) -> Once<'m, R, I, O, M> { - Once::new(self.id, self.store, self.matcher) + Once::new(self.id, self.name, self.store, self.matcher) } /// Specifies a matcher for the invocation. @@ -472,6 +478,7 @@ impl<'m, R, I, O, M: InvocationMatcher + Send + 'static> When<'m, R, I, O, M> When { matcher, id: self.id, + name: self.name, store: self.store, times: self.times, } @@ -484,7 +491,7 @@ impl<'m, R, I, O, M: InvocationMatcher + Send + 'static> When<'m, R, I, O, M> }; self.store - .get_or_create(self.id) + .get_or_create(self.id, self.name) .add_stub(Stub::new(answer, self.matcher)); } } diff --git a/src/when/once.rs b/src/when/once.rs index ae16902..8c627b3 100644 --- a/src/when/once.rs +++ b/src/when/once.rs @@ -13,14 +13,25 @@ use crate::{ /// the generics within `Once` will not. pub struct Once<'m, R, I, O, M: InvocationMatcher> { id: fn(R, I) -> O, - store: &'m mut mock::Store, + name: &'static str, + store: &'m mut mock::Store<'static>, matcher: M, } impl<'m, R, I, O, M: InvocationMatcher + Send + 'static> Once<'m, R, I, O, M> { #[doc(hidden)] - pub fn new(id: fn(R, I) -> O, store: &'m mut mock::Store, matcher: M) -> Self { - Once { id, store, matcher } + pub fn new( + id: fn(R, I) -> O, + name: &'static str, + store: &'m mut mock::Store<'static>, + matcher: M, + ) -> Self { + Once { + id, + name, + store, + matcher, + } } /// Analog of [When.then_return] where the value does not need to @@ -175,7 +186,7 @@ impl<'m, R, I, O, M: InvocationMatcher + Send + 'static> Once<'m, R, I, O, M> fn add_stub(self, stub: Box O + Send + 'static>) { self.store - .get_or_create(self.id) + .get_or_create(self.id, self.name) .add_stub(Stub::new(stub::Answer::Once(stub), self.matcher)); } }