search

nrxus / faux

Struct mocking library for Rust
https://nrxus.github.io/faux/
MIT License
419 stars 13 forks source link

Sync/Send mocks are not a "zero cost abstraction" #16

Open nrxus opened 4 years ago

nrxus commented 4 years ago

aa18c66a349c8f33ad061e2d6ff655233aa2b762 allowed mocks to be Send/Sync by forcing the user only provide Send closures, and using Mutex instead of RefCell for the interior mutability of the MockStore.

While this is fine as an MVP to allow mocking structs in Send/Sync contexts, it is very much the opposite of an "zero cost abstraction" (TM). Now every user has to pay the cost of making sure their captured variables are always Send, and the performance implication of Mutex even when they do not care/need their mocks to be Send/Sync.

This is far from ideal.

Suggestion: An attribute to #[faux::create] and #[faux::methods] that specify if they need a sync/send. This can get annoying if you are always mocking sync/send structs (i.e., you are building an API in Rocket which forces the request guards to be Send/Sync and you want to mock the request guards). I think a feature turn in on crate-wide would reduce this pain.

TedDriggs commented 4 years ago

I think a feature turn in on crate-wide would reduce this pain.

If any crate in your dependency graph enables the feature, that would enable it for every other crate using faux. Having a behavior change gated by a feature flag could make debugging very difficult.

nrxus commented 4 years ago

I don't quite understand what you mean.

Are you saying that if someone uses faux not as a dev-dependency but as a real dependency, and turns on that feature, and then I use that crate, then it would turn that feature on for everyone that uses faux?

Hmm that would be pretty bad but I think as long as I audit my dependencies (which thankfully aren't many) to not have faux as a real dependency, and if they do to not turn on any features, then it is probably okay? Although that would mean a lot of "process" in auditing my crates (and their deps?) manually for every release which adds such a burden hmm....

nrxus commented 4 years ago

Ah, after reading your comment. It makes sense now. Unfortunate this is not documented.

I guess a feature for Sync/Send mocks is out then, that's going to be a bit of an ergonomic pain for mocking in a library full of Sync/Send structs so I'll try to come up with an alternative. Thanks for the warning!