qcheck icon indicating copy to clipboard operation
qcheck copied to clipboard

Deprecated `Gen.small_int` changed distribution

Open jmid opened this issue 4 years ago • 0 comments

I just noticed something about the deprecated Gen.small_int...

First of all:

  • It is aliased to Gen.small_nat
  • It is marked with a note concerning backwards compatibility: https://github.com/c-cube/qcheck/blob/b065a81661d312fc7bbdbf95fb24229d71991a10/src/core/QCheck.ml#L184-L186

Unfortunately Gen.small_nat changed distribution from the interval [0;10000[ to the interval [0;100[ with QCheck version 0.10. Since version 0.5.2 Gen.small_int and Gen.small_nat's distributions were caused by a chain of aliases: Gen.small_int = Gen.small_nat = Gen.nat: https://github.com/c-cube/qcheck/blob/52e4193a56a7bdd904983bd3fa9706eaf2ec9703/src/core/QCheck.ml#L97-L105

QCheck version 0.10, introduced the Gen.small_nat / Gen.nat / Gen.big_nat distinction and broke the last alias, thus

  • changing Gen.small_nats distribution explicitly
  • changing Gen.small_ints distribution as a side-effect: https://github.com/c-cube/qcheck/blob/a3c8618a09a936a60756266e3d09e90dcabf847c/src/core/QCheck.ml#L97-L99

A bit of digging tells me Gen.small_int had the distribution in the interval [0;10000[ since version 0.5, it was briefly changed in 0.5.1 to output negative ints (#10), and then restored in 0.5.2 which also introduced the deprecation.

I found out, because rerunning some of my pre-0.10 code using Gen.small_int didn't find the expected counterexamples anymore.

With QCheck tests depending on distributions, a narrower distribution imply that test inputs are confined to a smaller subset. Potentially this could mean bugs flying under the radar... If someone is indeed using Gen.small_int for sizes of strings, lists, etc. as the compatibility note says, an upgrade from pre-0.10 means they silently started confining their tests to, e.g., strings of size [0;100[ compared to the earlier [0;10000[.

I see a couple of possibilities from here:

  • do nothing - after all, this change in distribution was introduced with 0.10 in 2019, keeping this issue for documentation
  • make Gen.small_int an alias to Gen.nat: let small_int = nat thus restoring the old distribution in the interest of backwards compatibility
  • remove Gen.small_int: it is unclear whether anyone else noticed. Also: it has been deprecated since January 2017. Removing it would force users to notice and act accordingly.

jmid avatar Sep 22 '21 11:09 jmid