la10736 / rstest

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

magic conversion fails to compile under `no_std` #282

Closed rnbguy closed 1 week ago

rnbguy commented 3 weeks ago

The following tests fails to compile

// lib.rs
#![no_std]

pub fn add(left: u64, right: u64) -> u64 {
    left + right
}

#[cfg(test)]
mod tests {
    use super::*;
    use rstest::rstest;

    #[rstest]
    #[case("2", "2", "4")]
    fn it_works(#[case] left: u64, #[case] right: u64, #[case] expected: u64) {
        assert_eq!(add(left, right), expected);
    }
}

because this is how it is expanded,

// Recursive expansion of rstest macro
// ====================================

#[cfg(test)]
fn it_works(left: u64, right: u64, expected: u64) {
    {
        match (&(add(left, right)), &expected) {
            (left_val, right_val) => {
                if !(*left_val == *right_val) {
                    let kind = core::panicking::AssertKind::Eq;
                    core::panicking::assert_failed(
                        kind,
                        &*left_val,
                        &*right_val,
                        core::option::Option::None,
                    );
                }
            }
        };
    }
}
#[cfg(test)]
mod it_works {
    use super::*;
    #[test]
    fn case_1() {
        let left = {
            use rstest::magic_conversion::*;
            (&&&Magic::<u64>(std::marker::PhantomData)).magic_conversion("2")
        };
        let right = {
            use rstest::magic_conversion::*;
            (&&&Magic::<u64>(std::marker::PhantomData)).magic_conversion("2")
        };
        let expected = {
            use rstest::magic_conversion::*;
            (&&&Magic::<u64>(std::marker::PhantomData)).magic_conversion("4")
        };
        it_works(left, right, expected)
    }
}

rstest could have used core::marker::PhantomData.

rnbguy commented 3 weeks ago

Probably need a pass through clippy::std_instead_of_core and clippy::std_instead_of_alloc lints :slightly_smiling_face:

la10736 commented 1 week ago

Unfortunately I have no time to look on it.

rnbguy commented 1 week ago

I am happy to make a PR. :slightly_smiling_face: but, can I expect a quick release with it ? :pray:

la10736 commented 1 week ago

If you provide a PR I'll review and add it ASAP

la10736 commented 1 week ago

Just a note: you have a workaround here:

// lib.rs
#![no_std]

pub fn add(left: u64, right: u64) -> u64 {
    left + right
}

#[cfg(test)]
mod tests {
    use super::*;
    use rstest::rstest;
    use ::core as std;

    #[rstest]
    #[case("2", "2", "4")]
    fn it_works(#[case] left: u64, #[case] right: u64, #[case] expected: u64) {
        assert_eq!(add(left, right), expected);
    }
}

Or better

// lib.rs
#![no_std]
#![cfg(test)]
extern crate std;

pub fn add(left: u64, right: u64) -> u64 {
    left + right
}

#[cfg(test)]
mod tests {
    use super::*;
    use rstest::rstest;

    #[rstest]
    #[case("2", "2", "4")]
    fn it_works(#[case] left: u64, #[case] right: u64, #[case] expected: u64) {
        assert_eq!(add(left, right), expected);
    }
}
la10736 commented 1 week ago

Even if your crate is a no_std one the tests are compiled with std support (they run in multi threading environment and is sure that they have std support). That means in your library your code cannot use std directly because the #![no_std] attribute remove this crate from the resolution but when you compile test the std binary is present.

So, we can just just take care that the rendered code use core instead std if it's possible, but the rstest library can still remain in std mode because it will be compiled with the std support and executed in a std environment.

I'll add these notes in the PR also.