Recheck executes test way more times than expected

[AlexeyRaga]

This executes the test once:

    [Fact]
    public void ShouldMap_UserCompetition_Equivalent2()
    {
        var prop =
            from source in ForAll(GenX.autoWith<CompetitionDetailsCache>(CompetitionsAutoGenConfig.Config()))
            select source.ToUserCompetition(Mapper)
                         .Should()
                         .BeEquivalentTo(
                             source.ToUserCompetition());

        prop.ReCheck("50_17003736010433171956_2975322824293632487_000000000000000000101010111110111111011111110111111110111111111101111111111101111111111110111111111111101111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110");
    }

And this executes the test 128667 times, always with the same values:

    [Property, Recheck("50_17003736010433171956_2975322824293632487_000000000000000000101010111110111111011111110111111110111111111101111111111101111111111110111111111111101111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110111111111111111110")]
    public void ShouldMap_UserCompetition_Equivalent(CompetitionDetailsCache source)
    {
        source.ToUserCompetition(Mapper)
              .Should()
              .BeEquivalentTo(source.ToUserCompetition());
    }

Changing a number of tests in Property attribute (I tried [Property(1)]) doesn't seem to have any affect on this behaviour.

Expected: test case is executed once in both cases :)

[dharmaturtle]

I believe this is an upstream issue.

[dharmaturtle]

I no longer believe this is an upstream issue. The problem is with this code - I need to return unit from inside the Property to satisfy the CE. Currently not sure how to do that. My best attempt

      |> toProperty // line 162
      |> Property.bind (fun () ->
        property {
          return () // compilation error
        }
      )

has the following compilation error:

This expression was expected to have type 'bool' but here has type 'unit'

Using return true instead of return () compiles, but still runs multiple times with Rechecking, so I'm not even sure if I'm on the right track.

@TysonMN

[TysonMN]

Maybe lines 169-172 should be

Property.check <| property {
  let! args = gens
  inkove args
  return ()
}

[TysonMN]

But that can't be right because it is ignoring logic that rechecks or not. I also don't understand toProperty right now, which seems like an important function. The last line of inkove is to call toProperty.

[dharmaturtle]

testMethod is the body of a method that has been decorated with the [<Property>] attribute. On 161 I Invoke that method. Invoke returns whatever that method returns. The return may be a bool, in which case I throw it into Property.ofBool. Or it may be an Async/Task, in which case I await it. It goes recursive because there may be a second Task in the Task. Or it may be a Result<_,_> in which case it asserts that the Result is in the OK state.

If it is none of these, it's a Property.success (). This case is relevant to this thread since Alexy's test returns unit/void.

The invoke on 152 only serves as an arg to Property.forAll. If we move away from Property.forAll, I'd still like some way to match over the return type of the test method.

[TysonMN]

So I think the code I suggested should be "pushed down". Do you know where?

I think we don't want to call Property.forall. I think you want to call Property.check or Property.recheck (it some variant) instead.

[dharmaturtle]

BindReturn works - I empirically verified that it only runs once for a sample test.

This solution requires toProperty to return unit (necessitating a name update.) To avoid a breaking change, Property.ofBool could do Xunit.Assert.True. Feels a little hacky, but it works.

[TysonMN]

I expect the fix is a change to this project. I would be very surprised if the proper fix was a change to Hedgehog.

[TysonMN]

You treat bool special because Hedgehog treats bool special?

I am thinking of removing the special treatment of bool. hedgehogqa/fsharp-hedgehog#419 (comment)

Or, I could throw a custom exception if the value is false.

[dharmaturtle]

You treat bool special because Hedgehog treats bool special?

Yes.

Though I wouldn't necessarily drop support in this library if Hedgehog drops special treatment of bool. My philosophy with this library is to try to make the experience of devs easier - it is, after all, "Hedgehog with convenience attributes for xUnit.net". Correctness is your domain :) I already do special casing for tests that return async/task/result. Why not bool?

Since not returning a value from a property CE results in inefficient rechecking, and since this library automatically returns for you, I kinda fancy the idea that this library is the recommended way of using Hedgehog :P

Or, I could throw a custom exception if the value is false.

Hah - I know we talked about using Xunit.Assert.True to replace Property.ofBool. Independent of your comment, while writing the test, I decided against that and wrote a new custom exception. I feel like it makes it more apparent in the failing stacktrace what's going on.

[dharmaturtle]

Should be fixed with the 0.5.0 release.