fscheck / FsCheck

Random Testing for .NET
https://fscheck.github.io/FsCheck/
BSD 3-Clause "New" or "Revised" License
1.15k stars 154 forks source link

Tests not ending with custom shrinker #675

Closed rynoV closed 4 months ago

rynoV commented 4 months ago

I'm running into a case where I have a custom shrinker which calls two other shrinkers (one from FsCheck and one custom), and normally the tests using this arbitrary instance pass (so I would expect the custom shrinking code not to be called). However it seems like when I add code which evaluates the sequence of shrinks generated from either the FsCheck shrinker or my custom shrinker, the tests stall.

For example just adding the line

let _ = (ArbMap.defaults |> ArbMap.arbitrary |> Arb.toShrink) dates |> Seq.length

(dates: DateTime[]) in my shrinker definition stalls the tests, and after commenting that out they pass.

I'm guessing this is because the shrunk sequence might be infinite, but I couldn't find any docs on this. Also, does the shrinker get evaluated even when the tests don't fail?

I'm on version 3.0.0-rc3

kurtschelfthout commented 4 months ago

FsCheck has no magic way to prevent shrinking if you literally evaluate it explicitly. Under normal circumstances, it won't shrink unless the random part of the test fails.

Shrinking can take a long time. It can also get stuck in a loop, if shrinking a value yields the same value again somewhere along the way.

rynoV commented 4 months ago

Sorry maybe I didn't explain well. I'm defining an arbitrary instance like

let arbInstance =
    { new Arbitrary<...>() with
        override x.Generator = ...

        override x.Shrinker(dates) = ...
        }

and the issue I'm having is that putting

let _ = (ArbMap.defaults |> ArbMap.arbitrary |> Arb.toShrink) dates |> Seq.length

inside my Shrinker method override freezes my tests, even when they're passing, in which case I would expect Shrinker to never be called.

I suppose it doesn't really matter, since the implementation shouldn't evaluate the infinite sequence anyways, it was just confusing behaviour and took me a while to figure out what was happening.

I'll make a PR adding to the docs that the sequence of shrinks may be infinite, and evaluating that sequence may freeze the tests even if no tests are failing.

kurtschelfthout commented 4 months ago

I see. Yes, FsCheck relies on the laziness of the returned sequence. It will call Shrinker during normal evaluation of the test, and capture the resulting sequence. This allows it to pick up shrinking with the values the test failed with. It effectively captures Shrinker(dates) in a closure, but only evaluates the resulting sequence one at a time, and only evaluates the sequence if the random test fails.

rynoV commented 4 months ago

Thanks for the explanation, I'll update my PR tomorrow.

I managed to minimize the issue I was running into, it turned out to be just that the shrink sequence was a lot bigger than I thought. A minimal repro:

    [<Property>]
    let ``test`` () =
        Prop.forAll (
            { new Arbitrary<int[]>() with
                override x.Generator = Seq.init 500 (fun i -> i) |> Seq.toArray |> Gen.constant

                override x.Shrinker(vs) =
                    let shrunk = (ArbMap.defaults |> ArbMap.arbitrary |> Arb.toShrink) vs
                    // let _ = Seq.length shrunk
                    Seq.empty }
        )
        <| fun (vs) -> true

Here, uncommenting the Seq.length line makes the test take 10 seconds. Commented it takes less than a second.

My original case was generating a datetime array with a size much larger than 500, so it appeared the test was stuck in an infinite loop.

kurtschelfthout commented 4 months ago

Arrays/lists can have very long shrink sequences. It tries to remove one element at a time.