search

hedgehogqa / fsharp-hedgehog

Release with confidence, state-of-the-art property testing for .NET.
https://hedgehogqa.github.io/fsharp-hedgehog/
Other
272 stars 30 forks source link

408 fix recheck #415

Closed TysonMN closed 2 years ago

TysonMN commented 2 years ago

Fixes #408

TysonMN commented 2 years ago

@AlexeyRaga, can you review? In particular, can you test this with the code that lead you to write https://github.com/hedgehogqa/fsharp-hedgehog/issues/408#issuecomment-1191296726?

AlexeyRaga commented 2 years ago

@TysonMN looks good to me, thanks for fixing it!

TysonMN commented 2 years ago

@AlexeyRaga, you keep deleting your comments ;)

I had a good response typed up for your most recent one, so here it is.

Since the range starts with 0, can't it generate i = 0 which would be = target?

Yes, and that happens. You can put a breakpoint there to confirm.

An integral generator always generates the "smallest" (in terms of shrinking) value first, which in this case is 0. But the property is checked 100 times (by default, which is what is being used here), so the second value generated is unlikely to be 0 again (there is only 1 chance in 1,000,000 of that). This test will fail if 0 is generated every time. The probably of that is (1 / 1,000,000)^99, so this test will never fail.

AlexeyRaga commented 2 years ago

@TysonMN yes, I understand that. I initially just didn't understand the fact that you wanted that test to fail when you run it first time :) But then I read further, understood what is going on and quickly deleted my comment hoping that you won't see my initial stupidity :)

AlexeyRaga commented 2 years ago

Could you please coin a release once it is merged?

TysonMN commented 2 years ago

Definitely. I talked to @dharmaturtle, and he will review this tomorrow.

TysonMN commented 2 years ago

I have "initial stupidity" all the time 🤣

TysonMN commented 2 years ago

I'm not fond of making the test dependent on the shrinking behavior, but I don't feel strongly about it. One possible solution is to steal from my example in #408:

Yes, much better. I changed to your suggestion.