By-reference fixtures

[narpfel]

By-reference fixtures

[la10736]

Yes, it'll be very nice to have this feature. Freak free to open a PR.

[la10736]

@narpfel I've implemented it in https://github.com/la10736/rstest/tree/by_ref
I didn't published it yet because I need to do some refactor till merge it. You can try it ... let me know if it's ok

[la10736]

While I was fixing #230 I noted that the following syntax will work

use std::cell::Cell;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum E<'a> {
    A(bool),
    B(&'a Cell<E<'a>>),
}

fn make_e_from_bool<'a>(b: bool) -> E<'a> {
    E::A(b)
}

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

    use super::*;

    #[rstest]
    #[case(true, E::A(true))]
    fn it_works<'a>(#[case] b: bool, #[case] expected: E<'a>) {
        let actual = make_e_from_bool(b);
        assert_eq!(actual, expected);
    }
}

Is this code fine for you?

[narpfel]

Unfortunately not. This is my real code:

    #[rstest]
    #[case::bool("true", Value::Bool(true))]
    fn test_eval<'a>(bump: &'a Bump, #[case] src: &'static str, #[case] expected: Value<'a>) {
        pretty_assertions::assert_eq!(eval_str(bump, src).unwrap(), expected);
    }

bump is actually used by the code under test and can’t be removed.

[la10736]

And what about to use #[once] here? Is it an issue to have a just an Bump instance?

With the unreleased version follow code works

use std::cell::Cell;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum E<'a> {
    A(bool),
    B(&'a Cell<E<'a>>),
}

fn make_e_from_bool<'a>(_bump: &'a (), b: bool) -> E<'a> {
    E::A(b)
}

fn new_bump() {}

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

    use super::*;

    #[fixture]
    #[once]
    fn bump() -> () {}

    #[rstest]
    #[case(true, E::A(true))]
    fn it_works<'a>(bump: &'a (), #[case] b: bool, #[case] expected: E<'a>) {
        let actual = make_e_from_bool(&bump, b);
        assert_eq!(actual, expected);
    }
}

[narpfel]

A #[once] fixture would be possible, but I’d like to avoid it if possible because then the tests would leak memory, and that’s annoying when testing with tools that flag memory leaks such as miri or valgrind.

[la10736]

That's new for me... why static reference is marked as memory leak in valgrind? Ok, verbose set up will report it as an not released static reference but should not be an issue.

Anyway I cannot accept a PR about this just because is a breaking change when #[once] fixture is used.

Beside that my next feature will be a #[map] attribute that should be help to fix this case too: follow code should will work

use std::cell::Cell;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum E<'a> {
    A(bool),
    B(&'a Cell<E<'a>>),
}

fn make_e_from_bool<'a>(_bump: &'a (), b: bool) -> E<'a> {
    E::A(b)
}

fn new_bump() {}

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

    use super::*;

    #[fixture]
    fn bump() -> () {}

    #[rstest]
    #[case(true, E::A(true))]
    fn it_works<'a>(#[map(|b| &b)] bump: &'a (), #[case] b: bool, #[case] expected: E<'a>) {
        let actual = make_e_from_bool(&bump, b);
        assert_eq!(actual, expected);
    }
}

[narpfel]

I think that even with a #[once] fixture it will not work, because bumpalo::Bump is !Sync and has to be wrapped in a Mutex. And to lock it, you create a MutexGuard, which derefs to the Bump. But... the reference’s lifetime is bounded by the MutexGuard’s lifetime, so we’re back to exactly the same problem as in the initial code.

Anyway I cannot accept a PR about this just because is a breaking change when #[once] fixture is used.

I think there’s a non-breaking way to implement this by annotating fixture parameters with an attribute to enable pass-by-reference, like this:

#[fixture]
fn f() -> u32 { 42 }

#[rstest]
fn test(#[by_reference] f: &u32) {
    assert_eq!(*f, 42);
}

Beside that my next feature will be a #[map] attribute that should be help to fix this case

How would that work? It looks like the closure returns a reference to a local variable?

[la10736]

I think that even with a #[once] fixture it will not work, because bumpalo::Bump is !Sync and has to be wrapped in a Mutex. And to lock it, you create a MutexGuard, which derefs to the Bump. But... the reference’s lifetime is bounded by the MutexGuard’s lifetime, so we’re back to exactly the same problem as in the initial code.

Right!

Anyway I cannot accept a PR about this just because is a breaking change when #[once] fixture is used.

I think there’s a non-breaking way to implement this by annotating fixture parameters with an attribute to enable pass-by-reference, like this:

#[fixture]
fn f() -> u32 { 42 }

#[rstest]
fn test(#[by_reference] f: &u32) {
    assert_eq!(*f, 42);
}

Beside that my next feature will be a #[map] attribute that should be help to fix this case

How would that work? It looks like the closure returns a reference to a local variable?

Yes, that's feasible. Maybe the right syntax can be just #[ref] and should work for every kind of input.

[narpfel]

Maybe the right syntax can be just #[ref]

I don’t think #[ref] can be an attribute because ref is a keyword. #[r#ref] would be possible, but at that point I think #[by_ref] is better.

and should work for every kind of input.

You mean also for #[case], #[values], and #[files]? That would need some more work in this code

rstest/rstest_macros/src/parse/rstest.rs

Lines 134 to 140 in 06a2117

	let composed_tuple!(fixtures, case_args, cases, value_list, files) = merge_errors!(
	extract_fixtures(item_fn),
	extract_case_args(item_fn),
	extract_cases(item_fn),
	extract_value_list(item_fn),
	extract_files(item_fn)
	)?;

because (as I understand it), this would parse e. g. fn f(#[case] #[by_reference] x: u32) as both a test case input and a fixture when both parsers recognise #[by_reference].

For now I’d like to get this feature working (and robust) for #[fixture] first and then maybe look into supporting the other input types.

[la10736]

Ok #[by_ref] it's a good compromise. I've an idea of how to implement it in a generic way. My idea is to catch #[by_ref] and set it as argument attribute and just use & we I use this argument on calling test function.

Example :

#[fixture]
fn f() -> u32 { 42 }

#[rstest]
#[case(42)]
fn test(#[by_ref] f: &u32, #[case] #[by_ref] c: &u32, #[values(42, 142)] #[by_ref] v: &u32) {
    assert_eq!(f, c);
    assert_eq!(*c, *v%100);
}

it'll be expanded in something like follow:

mod f {
    fn default() -> u32 {
        42
    }
}

fn test(f: &u32, c: &u32, v: &u32) {
    assert_eq!(f, c);
    assert_eq!(*c, *v%100);
}

mod test {
    use super::*;
    mod case_1 {
        use super::*;
        #[test]
        fn v_1_42() {
            let f = f::default();
            let c = 42;
            let v = 42;
            test(&f, &c, &v)
        }

        #[test]
        fn v_2_142() {
            let f = f::default();
            let c = 42;
            let v = 142;
            test(&f, &c, &v)
        }
    }
}

[narpfel]

That branch works for my use case. Thanks!