Closed nrxus closed 3 years ago
I feel fairly strongly that Hash, Ord/PartialOrd, and Copy are out. Those are traits that apply to data and mocking data is... weird. My philosophy of mocking is to mock behavior which wouldn't have a concept or Ord/Hash/Copy in my opinion.
I feel similarly but less strongly about Eq and PartialEq.
Clone is more gray. It also feel like it is meant for data but.... you could clone behavior (i.e., clone your HTTP client). But perhaps for those cases they would not be auto-derived?
Default... :man_shrugging:
I've found cases where I need to have structs that I can Clone
. E.g. to use in Actix I need to either put my services into Arc<>
wrappers, or have one clone per thread that Actix is running.
It's worth pointing out that there is a workaround, albeit a bit naff, for Clone
. Namely, do it by hand:
#[cfg(test)]
use faux;
#[cfg_attr(test, faux::create)]
#[cfg_attr(not(test), derive(Clone))]
pub struct UserService {
db: Database,
}
#[cfg_attr(test, faux::methods)]
impl UserService {
pub async fn new(db: Database) -> Self {
Self { db: db }
}
}
#[cfg(test)]
#[cfg_attr(test, faux::methods)]
impl Clone for UserService {
fn clone(&self) -> Self {
Self { db: self.db.clone() }
}
}
That makes sense. My concern with allowing auto-deriving for Clone
is that I don't know what a mocked clone should do.
let service = UserService::faux();
when!(service.fetch).safe_then(/* do something*/)
let cloned_service = service.clone();
Ideally, I think, cloned_service
would still have the mocked function, but that is unfortunately impossible since the mock closure does not necessarily implement Clone
.
I could clear all the stored mocks but I fear that might be confusing.
I could panic!
if a mock is called on a mocked instance, this sounds unideal but at least it's loud. With your current workaround, it wold essentially panic when cloning a mocked instance unless the clone method is mocked. So perhaps that's what the auto-derived version should do?
I might need to juggle this in my brain a bit and decide an okay path. Thanks for bringing this up though, and please let me know if you have any preferences or ideas (:
A downside is that it won't work even if you don't need to use the traits in testing. What I mean is if you don't want to clone explicitly like in your example above, but the library code needs these traits to be implemented. It sure is a problem if the closure is does not implement Clone. The others can maybe just be passed through to the inner object? Maybe require the closure to be clone if the inner object has a clone derive?
#[cfg_attr(test, faux::create)]
#[ derive( Debug, Clone, PartialEq, Eq ) ]
//
pub struct WireFormat
{
bytes: Bytes
}
This won't compile, but changing it the other way around won't compile either because attribute macros have to be before derive macros.
I think a step forward, at least to allow things to compile without load-bearing impl
s, will be to have the mock implement clone, if the original struct does as well. Then have the mock panic if someone actually attempts to clone it. Not perfect but good enough.
I can always change the behavior (i.e., make it not panic) without it being a breaking change.
this will be addressed in the next release: https://github.com/nrxus/faux/commit/0a0577e8427885f7c8f3681d23625b4f07ccbe34
I am holding releasing for a bit since I am updating the minimum stable rust version to 1.45.0
I have also implemented Default
in: 53ce22d937de75c155c65d491e42efcf5ce314e6
Closing. If there is need for other derivable traits those can be their own issues.
What traits should the mocks support deriving?
aa18c66a349c8f33ad061e2d6ff655233aa2b762 added support for Sync/Send/Debug.
What about Eq, PartialEq, Ord, PartialOrd, Clone, Copy, Hash, Default?