-
Notifications
You must be signed in to change notification settings - Fork 63
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
Concretize doesn't work with cfg_attr #427
Comments
The concretize attribute isn't processed as a normal Rust attribute. Instead, Mockall processes it as text. So we need to process any cfg_attr directive ourselves. Rather than attempt to evaluate the conditional, just assume that it's true. By far the most common use case will be `cfg_attr(test, concretize)`. Fixes #427
The concretize attribute isn't processed as a normal Rust attribute. Instead, Mockall processes it as text. So we need to process any cfg_attr directive ourselves. Rather than attempt to evaluate the conditional, just assume that it's true. By far the most common use case will be `cfg_attr(test, concretize)`. Fixes #427
@cjriches this should be fixed now. |
Thanks! Yup, seems to work now. |
I'm trying to use this but if I understand correctly this |
I've held off from releasing it because it's a big change, not just to the implementation but to the API. And it's a weird change, too. I haven't heard much feedback about it. Why don't you try it out, on the master branch, and let me know how it works for you? |
sure, let me give it a try. thanks! |
The
#[concretize]
attribute isn't processed as a real Rust attribute. It's processed as text by Mockall, and used as a directive to tell it how to mock something that's already being mocked. That's why it must be used with its canonical name; you can't douse mockall::concretize as something_else
.But as of this writing, Mockall doesn't process the text correctly if it appears within a
cfg_attr
directive. Instead, it passes it through in the emitted code, causing various compile failures. This example will trigger the problem.First reported as #408 (comment) .
The text was updated successfully, but these errors were encountered: