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

Support tests that return Result<T, E> #6

Open
ThatGeoGuy opened this issue Mar 5, 2024 · 4 comments
Open

Support tests that return Result<T, E> #6

ThatGeoGuy opened this issue Mar 5, 2024 · 4 comments

Comments

@ThatGeoGuy
Copy link

First off: thanks for the crate, it makes getting started writing tests pretty easy to work with.

In regular (standard, not async) Rust we can do:

#[cfg(test)]
mod tests {
    use super::*;
    use std::error::Error;

    #[test]
    fn it_works() -> Result<(), Box<dyn Error>> {
        let _ = f32::try_from(20usize)?;
        Ok(())
    }
}

However, the test! macro doesn't support this form at all, and assumes that all tests return ().

This means that we can't write:

#[cfg(test)]
mod tests {
    use super::*;
    macro_rules_attribute::apply;
    use std::error::Error;

    #[apply(smol_macros::test!]
    async fn it_works(_ex: &Executor<'_>) -> Result<(), Box<dyn Error>> {
        let _ = f32::try_from(20usize)?;
        Ok(())
    }
}

The current workaround to this is to just .unwrap or .expect all Result types in the function. Materially, they don't really change the logic of the test (since a panic means failure), but it is often nice to be able to write the code with ? and FromResidual only because the tests can often serve as an example for how code should be written in practice.

Thoughts? Would the current macro be able to be reworked so that the return type for a test can be set as well?

@notgull
Copy link
Member

notgull commented Oct 19, 2024

I would accept a PR for this.

notgull added a commit that referenced this issue Oct 19, 2024
cc #6

Signed-off-by: John Nunley <dev@notgull.net>
@notgull
Copy link
Member

notgull commented Oct 19, 2024

I submitted #6 to test this issue. However, it looks like tests that return Result already work?

#[apply(test!)]
async fn it_works(_ex: &Executor<'_>) -> Result<(), Box<dyn std::error::Error>> {
    let _ = u32::try_from(20usize)?;
    Ok(())
}

notgull added a commit that referenced this issue Oct 19, 2024
cc #6

Signed-off-by: John Nunley <dev@notgull.net>
@ThatGeoGuy
Copy link
Author

I do recall testing the example I pasted, and it did not work. Perhaps there was some update in the meantime that allowed for this? I believe I was testing on 1.75.0, but if it works now I'd be happy to close this.

@notgull
Copy link
Member

notgull commented Oct 31, 2024

I doubt it. The MSRV tests on 1.63, so unless the bug was introduced and then fixed at some point between then and now it wouldn't make sense.

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

No branches or pull requests

2 participants