flub / testdir

Semi-persistent, scoped test directories
Apache License 2.0
8 stars 3 forks source link

Testdir Collision for rstest Parametrized Tests #6

Open jcjgraf opened 1 year ago

jcjgraf commented 1 year ago

If you crate is used together with rstest's parametrized tests feature, the directory created by testdir!() in one parametrized test function collides with the one created in a different parametrized test function.

Expected Behaviour

I would expect that the PathBuf returned by testdir!() is always unique and new, and never collides with one created in a different testing function.

Actual Behaviour

If two rstest test function have multiple cases, rstest names the test cases func1::case1, func1::case2 for test function func1, and fun2::case1, func2::case2 for the second one. Tempdir thinks that case1, case2 are the names of the test functions, which are however not unique in a module. This leads to a reuse of the test dir.

Reproduce

Run the unit tests in the following MVE (requires rstest) using cargo test -- --nocapture.

#[cfg(test)]
mod tests {
    use rstest::{fixture, rstest};
    use std::path::PathBuf;
    use testdir::testdir;

    #[rstest]
    #[case(1)]
    #[case(2)]
    #[case(3)]
    fn test_1(#[case] _num: u8) {
        let out = testdir!();
        println!("Creating new tmp_dir {}", out.display());
    }

    #[rstest]
    #[case(1)]
    #[case(2)]
    #[case(3)]
    fn test_2(#[case] _num: u8) {
        let out = testdir!();
        println!("Creating new tmp_dir {}", out.display());
    }
}

I understand that this issue is related to the interplay of your crate with another one. But since rstest is rather popular, it would be great if both would play well together.

Having had a quick lock at the code, it seems like the crate::private::extract_test_name would need to be improved and return testfunc::caseN instead of just caseN for parametrized tests.

What do you think?

flub commented 1 year ago

Thanks for the detailed report! Your analysis sounds right, the heuristics to construct the tests names are somewhat difficult and can be messed up by extensions to basic libtest usage.

I agree rstest support would be nice to add.