Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nrxus opened this issue Jan 8, 2020 · 3 comments
Open

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

nrxus opened this issue Jan 8, 2020 · 3 comments

Comments

@nrxus
Copy link
Owner

nrxus commented Jan 8, 2020

aa18c66 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
Copy link
Contributor

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
Copy link
Owner Author

nrxus commented Jan 28, 2020

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
Copy link
Owner Author

nrxus commented Jan 29, 2020

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants