proptest-rs / proptest

Hypothesis-like property testing for Rust
Apache License 2.0
1.63k stars 152 forks source link

Recursive `arbitrary` calls #451

Open mirandaconrado opened 1 month ago

mirandaconrado commented 1 month ago

I frequently find myself having to call a hierarchy of value generation because I use proptest::Arbitrary on all types. Here's a simplified example:

#[derive(Clone, Debug, Default)]
struct T1 {
    val: u32,
}

impl Arbitrary for T1 {
    type Parameters = ();

    fn arbitrary_with((): Self::Parameters) -> Self::Strategy {
        (0..100_u32).prop_map(|val| T1 { val }).boxed()
    }

    type Strategy = BoxedStrategy<Self>;
}

#[derive(Clone, Debug, Default)]
struct T2 {
    t1: T1,
}

impl Arbitrary for T2 {
    type Parameters = T1;

    fn arbitrary_with(t1: Self::Parameters) -> Self::Strategy {
        Just(T2 { t1 }).boxed()
    }

    type Strategy = BoxedStrategy<Self>;
}

#[derive(Clone, Debug, Default)]
struct T3 {
    t2: T2,
}

impl Arbitrary for T3 {
    type Parameters = T2;

    fn arbitrary_with(t2: Self::Parameters) -> Self::Strategy {
        Just(T3 { t2 }).boxed()
    }

    type Strategy = BoxedStrategy<Self>;
}

fn recursive_arbitrary_t3() -> BoxedStrategy<T3> {
    T1::arbitrary()
        .prop_flat_map(|t1| {
            T2::arbitrary_with(t1.clone())
                .prop_flat_map(move |t2| T3::arbitrary_with(t2.clone()).prop_map(move |t3| t3))
                .boxed()
        })
        .boxed()
}

A concrete use case where this ends up appearing is when I have T1 == Config, T2 == Subsystem and T3 == System. Ideally, I'd like a way to have something like T3::recursive_arbitrary() that does what recursive_arbitrary_t3 does for me.

I have working around this by doing

#[derive(Clone, Debug, Default)]
struct T2 {
    t1: T1,
}

impl Arbitrary for T2 {
    type Parameters = Option<T1>;

    fn arbitrary_with(maybe_t1: Self::Parameters) -> Self::Strategy {
        let t1_strategy = if let Some(t1) = maybe_t1 {
            Just(t1).boxed()
        } else {
            T1::arbitrary().prop_map(|t1| t1).boxed()
        };
        t1_strategy.prop_map(|t1| T2 { t1 }).boxed()
    }

    type Strategy = BoxedStrategy<Self>;
}

which is not an unreasonable workaround, but a bit more verbose than I'd have liked personally.

I'm not sure how this would work, but I've encountered this frequently enough that I'd imagine I'm not the only one. I'm open to helping provide the implementation for this if we have a reasonable idea how it could work.

mirandaconrado commented 1 month ago

Someone pointed to me that proptest_derive can solve a lot of this, which I think makes this less of a need.

We went with a custom derive to implement a version of the arbitrary with Options that I highlighted. That would place this as lower priority on my end and happy to have it closed.

matthew-russo commented 3 weeks ago

Sorry for the delay, I had to step away for the past couple months to deal with some things at home. I'll be back working on things starting this weekend and will start triaging the issues that have come in.

Given you found the derive crate, what exactly are you looking for/proposing? a utility function for generating strategies? additions to the derive macro or a completely separate macro?

Thanks for the feedback

mirandaconrado commented 3 weeks ago

Don't worry. I hope things are better at home.

I'm honestly not sure what's the cleanest way to do this for the general case. Basically the idea is to allow every field to either be passed or auto-generated by calling its own arbitrary. In the custom derive we ended up with here, we just list the fields in order, each wrapped in an Option, as a tuple for the argument, and then have the check to either use Just or call "arbitrary" for the field.

Maybe something like a builder pattern where the macro defines a new builder type. Then the user can set each field that is fixed to a given value, and then "finish" the build. The finishing would return a strategy for generating the base type where every field is either fixed to the provided value or is generated via "arbitrary". It could leverage the field annotations already present for the current proptest_derive to specify different ways to generate the fields that are not locked to a specific value.