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
Add async fixtures #90
Conversation
I've surely underestimated this endeavour. I'm facing a design decision now, after toying with the code for a while. I only see one option, but before I really want to implement something, I'd rather ask! There are basically four possibilities here:
This is what happens in either case:
The way that a fixture is rendered is by generating a When these methods become Hence my suggestion: every synchronous fixture will generate both synchronous methods (as before) and an equivalent set of asynchronous methods, prefixed or suffixed with This way, it'll be pretty difficult to generate a clear error in the case that a synchronous test tries to call an async fixture. I'm not sure how to go about that. Is this okay for you? If so, I can implement this tomorrow morning. If not, I'm very much open to suggestions! |
Ok... I was going to bed but I'm trying to answer... But I've not a clear idea about that, I've just wrote the ticket to don't forget this point but I've already see that the way is not straight and clear.
I think that the 2 case is just an error. We cannot check it and we can just try to give a clear error but is not simple (there are some static check techniques to catch them but I should study them). So by now we can forget it. My idea is that the user know exactly if he is calling an async fixture and just write the right code. If he use an async fixture value as a static value why write an async fixture? Just write a plain one.
Yes, my guess is that An async fixture would render all methods to be
Again... maybe we can not care about this kind of scenarios.
Wait, wait... why? We don't need to transform all no async function in a fake Today an example like #[rstest(a, b,
case(async { 42 }, 1),
case(async { 21 }, 2),
)
]
async fn my_async_test(a: impl Future<Output=u32>, b: u32) {
assert_eq!(b * a.await, 42);
} should compile and runs 2 test.
Compiler should give already good info to understand it... we can write some example but I can bet on it.
Try first the simpler way. Maybe now the things can be simpler... |
Don't feel obliged to answer! If you want to sleep, you sleep! :-)
Yes, only included it for "completeness", but it should be a hard error indeed.
A good night's sleep (for me) got me to the same conclusion. This is way simpler. The only advantage of my proposal would be that the user doesn't need to manually #[fixture] async fn foo() -> u32 { 42 }
#[rstest] fn bar(foo: u32) { } the compiler will just complain that you're feeding |
The simpler way seems to work for the simplest case already. Thanks for the input :-) |
I'm still a bit sad that the injected fixtures are not magically I currently have something of the form trait Service {}
#[fixture]
async fn create_service() -> impl Service {
complex_async_code().await
}
#[rstest]
#[actix_rt::test]
async fn foo<T: Service>(app: impl Future<Output = T>) {
} and it works, although it would be a lot nicer to write #[rstest]
#[actix_rt::test]
async fn foo(app: impl Service) {
} |
We can think (later) to introduce a attribute to remove all this boilerplate: #[rstest]
#[actix_rt::test]
async fn foo(#[future] app: impl Service) {
} But we cannot do it without indication: we can never know if the input is an async fixture or not. We should proceed by small steps. |
Are you done or you would try to do more?
Otherwise I can handle them and merge your work as is. |
On 4/13/20 9:28 PM, la10736 wrote:
Are you done or you would try to do more?
If you can:
1. add an entry in changelog
2. add some unit tests
3. some docs in fixture function an in readme
Otherwise I can handle them and merge your work as is.
If you can wait, I can handle some of these; I'm not sure when I'll add
them though, I'm handling a few things concurrently. Otherwise, feel
free to merge and add these yourself.
|
Yes... I can wait... Take all your time. Let me know when you're done. THX |
The boilerplate |
having them pre-awaited would be nice indeed... I fear that it's quite a overhaul though. |
Well, not even necessarily that but even just getting rid of the rather verbose |
I suggest you open an issue, describing the syntax you have to use now, versus the syntax you would prefer. Feel free to tag me there, I'd love to follow the conversation. Chances are low I'd implement it myself though, it's only a "minor inconvenience" as they say :-) |
Turns out there is #98 so let's go there. |
Would fix #86
TODO:
async
fixtures are used in non-async
-tests.impl Trait
output of a fixture (i.e., nestedimpl Trait
); not sure whether that can work.