typelift / SwiftCheck

QuickCheck for Swift
MIT License
1.42k stars 106 forks source link

passing any checkerargs results in only a single test case running #279

Open johanatan opened 5 years ago

johanatan commented 5 years ago

e.g.,

        let checkerArgs = CheckerArguments(replay: .none, maxAllowableSuccessfulTests: 200,
                                           maxAllowableDiscardedTests: 0, maxTestCaseSize: 1000)

i've tried a bunch of different values for maxAllowableSuccessfulTests and maxTestCaseSize yet my property body only runs once when the defaults are overridden.

CodaFi commented 5 years ago

Not any checker args, just the ones that set maxAllowableDiscardedTests to 0. Since this is counterintuitive, you could make the case that this is a bug. But think about it in relation to the other max-settings which are similarly size-inclusive. In that case, after one test has occurred, you have discarded precisely 0 times and hence you have hit the maximum discard threshold. You need to at least set it to 1 to allow the testing loop to continue.

CodaFi commented 5 years ago

I think QuickCheck might have the right idea of making this option a max discard ratio, not a max discard count.

johanatan commented 5 years ago

Honestly I don’t understand the meanings of any of these values. What is a ‘max successes’? Is that the # of times the test is allowed to succeed? And if it succeeds one time beyond that, it fails?

Likewise with the ‘max test case size’? How exactly does this value actually inform the sizes of the generators?

To get this to work, I was basically just cargo culting your example code which uses a max discard of zero (which seems to make sense). Why do you need to discard a test case that passes? Why do you need to discard any test cases?

johanatan commented 5 years ago

Oh, sorry. I missed part of your explanation wrt max discard.

So, if max discard is set to n and I have discarded n times, I should still be fine. It should be on the attempt to discard n+1 times that a problem occurs. This reasoning has the nice property of making the zeroth case work properly.

CodaFi commented 5 years ago

To get this to work, I was basically just cargo culting your example code which uses a max discard of zero (which seems to make sense)

Oh dear, then that's a bug in the sample code or in the semantics of this argument.

johanatan commented 5 years ago

P.S. if it weren't clear, here is where I cargo culted from: https://github.com/typelift/SwiftCheck/blob/cf9958085b2ee1643e541e407c3233d1b76c18ff/Sources/SwiftCheck/Check.swift#L24-L29

CodaFi commented 5 years ago

Yeah, so I see two problems here:

1) The semantics of maxAllowableDiscardedTests is muddy with respect to 0. I agree that your reading is more natural and should probably be the behavior we support going forward. 2) Regardless of what happens with the point above, the sample code in the docs has unintended semantics. We could address this by fixing point 1, or by simply changing the example.

johanatan commented 5 years ago

Yea, I prefer 1 but will change to a value of 1 to workaround for now. Thanks.