Recheck is doing the wrong thing?

[AlexeyRaga]

I think that recheck functionality is actually a bit insane right now. It clearly does not do what it is supposed to do.

Consider this example:

let cond() =
    let mutable n = 0
    fun (str : string) ->
        n <- n + 1
        Console.WriteLine $"{n}: {str}"
        str.Contains 'r'

[<Fact>]
let ``My test`` () =
    let cnd = cond()
    property {
        let! str = Gen.string (Range.constant 1 5) Gen.alpha
        return cnd str
    }
    |> Property.checkBool

It fails with the following output:

1: vnCaD
2: CaD
3: aD
4: D
5: d
6: a

*** Failed! Falsifiable (after 1 test and 5 shrinks):
"a"
This failure can be reproduced by running:
> Property.recheck "0_594166620906336962_12853707185440443475_00000" <property>

Now, when I run it with Property.recheckBool "0_594166620906336962_12853707185440443475_00000" the output is:

1: vnCaD
2: CaD
3: vnD
4: nCaD
5: vCaD
6: vnaD
7: vnCD
8: vnCa
9: anCaD
10: knCaD
11: qnCaD
12: tnCaD
13: unCaD
14: vaCaD
15: vgCaD
16: vkCaD
17: vmCaD
18: vncaD
19: vnAaD
20: vnBaD
21: vnCad
22: vnCaA
23: vnCaB
24: vnCaC
25: aD
26: CD
27: Ca
28: caD
29: AaD
30: BaD
31: Cad
32: CaA
33: CaB
34: CaC
35: D
36: a
37: ad
38: aA
39: aB
40: aC
41: d
42: A
43: B
44: C
45: a
46: b
47: c

Clearly, not only it doesn't run once, but it also does significantly more work and arrives at the different result at the end!

@TysonMN Could be a regression in recheck re-implementation since 0.11?

To my understanding, since 0.11 recheck was re-implemented to only run once. Previously it would run all the tests and shrinks, now it is supposed to be optimised to only run the last failing test.

However, in versions since 0.11 (I checked 0.12 and the latest 0.13) the recheck misbehaves in the described way.

Which is also manifesting as #419

[AlexeyRaga]

I debugged it a bit, and I think that at least part of a problem is in splitAndRun function here:
https://github.com/hedgehogqa/fsharp-hedgehog/blob/master/src/Hedgehog/Property.fs#L247

This is the one that actually runs the test way more times than the original run.

Perhaps the expectation was that this function would split the generator and run the test just once?
Instead, it calls to Random.run, which is a function that is composed somehow and actually does run the code that many times.

[AlexeyRaga]

I think I may have found a workaround for now.

It only works in the following condition:

1. The property fails
2. The property has a redundant return statement

E.g. this does not work:

 property {
        let! str = Gen.string (Range.constant 1 5) Gen.alpha
        return cnd str
    } |> Property. recheckBool "..."

and this does not work either:

 property {
        let! str = Gen.string (Range.constant 1 5) Gen.alpha
        Assert.True(cnd str)
    } |> Property.recheck "..."

but this does work:

 property {
        let! str = Gen.string (Range.constant 1 5) Gen.alpha
        return Assert.True(cnd str)
    } |> Property.recheck "..."

My current understanding is that Property.recheckBool will never work properly if Property<bool> doesn't fail and just returns the value.

I think, if we must insist that for recheck to work the property must fail, then maybe we should remove recheckBool (and maybe even checkBool?) And we should only allow checking/rechecking on Property<unit> properties?
This way at least this is cleat that the exception is the only way to fail the check...

Of course it'd be better if checkBool behaviour is fixed. Or, at very least, runs deterministically the same way check does and repeats the same steps (not different and more, as it does currently).

But I do not really understand why Assert.True doesn't work when return Assert.True does.
This is super not obvious.

[TysonMN]

Regarding your 47 printed values when running reckeckBool, I would have to look closely at the details. It does seem wrong.

Regarding recheck vs recheckBool, I think recheck is better, but I can't remember exactly why now. I vaguely recall the implementation of recheckBool requiring monadic behavior (instead of allowing applicative behavior).

Regarding return vs no return, I remember this well. The applicative computational expression behavior requires there to be a return statement, and the applicative behavior is what makes efficient rechecking (i.e. only run the critical test code exactly once) possible.

[TysonMN]

I mentioned the last two of these things to you in #419