la10736 / rstest

Fixture-based test framework for Rust
Apache License 2.0
1.21k stars 43 forks source link

Properly handle mutability for awaited futures #239

Closed borchero closed 7 months ago

borchero commented 7 months ago

Motivation

Currently, the following code fails to compile:

#[rstest]
#[case(async { 3 })]
async fn my_test(
    #[case]
    #[future(awt)]
    mut a: u8,
) {
    a = 4;
    assert_eq!(a, 4);
}

as (the inner function) expands to

async fn my_test(mut a: impl std::future::Future<Output = u8>) {
    let a = a.await;
    {
        a = 4;  // <-- this is an issue, `a` is not mutable, but the future is!
        assert_eq!(a, 4);
    }
}

This is most likely undesired: by marking the awaited future value as mut, one would expect that a is mutable.

Changes

This PR augments the mutability identifier to result in the following expanded code that now compiles as expected:

async fn my_test(a: impl std::future::Future<Output = u8>) {  // <-- no `mut` for the future anymore -- await NEVER requires `mut`
    let mut a = a.await;  // <-- this is now correctly marked mutable
    {
        a = 4;
        assert_eq!(a, 4);
    }
}
la10736 commented 7 months ago

Take a look to https://github.com/la10736/rstest/tree/my_await_mut

borchero commented 7 months ago

Take a look to https://github.com/la10736/rstest/tree/my_await_mut

Nice! Thanks for taking over, this looks much better 😁 I'm not too familiar with writing procedural macros yet :eyes: shall I close this PR in favor of your changes?

la10736 commented 7 months ago

No, include these changes like are yours and handle tests and change log. I didn't do it if you didn't prompt me : smile:

borchero commented 7 months ago

@la10736 latest change now includes your suggestion for improvement along with e2e and unit tests :)

borchero commented 7 months ago

Thanks! Can you create a new release with this change @la10736? 🙏🏼

la10736 commented 7 months ago

I'll try to do it later in the day

la10736 commented 7 months ago

@borchero Done

borchero commented 7 months ago

Thanks a lot!