c-cube / qcheck

QuickCheck inspired property-based testing for OCaml.
https://c-cube.github.io/qcheck/
BSD 2-Clause "Simplified" License
340 stars 37 forks source link

Add shrinking to char arbitraries #247

Closed jmid closed 2 years ago

jmid commented 2 years ago

This PR adds shrinking to char* arbitrary generators. In order to avoid shrinking a numeral char '7' into a non-numeral char it includes a dedicated shrinker. The printable case is not broken by the existing Shrink.char shrinker.

To eat my own medicine, I then proceeded to add Generator, Shrinker, and Statistics tests ...only to find bugs :open_mouth:

jmid commented 2 years ago

I've refined the shrinker tests a bit to better exercise the expected behaviour. In doing so, I noticed that in the range '!'-'&' (character codes 32-38) QCheck2.printable shrinks towards '!' (lower character codes) contradicting the documentation of shrinking towards 'a' (97). I therefore updated the documentation to match the behaviour better. I admit it is underspecified - but fully specifying the behaviour becomes unreadable, IMO.

Also updated the CHANGELOG, so this should be good to go.

jmid commented 2 years ago

@vch9 Can you take a quick look at this since it finds and fixes an issue with the QCheck2.printable generator? :pray:

vch9 commented 2 years ago

@vch9 Can you take a quick look at this since it finds and fixes an issue with the QCheck2.printable generator? pray

The fix seems correct to me

jmid commented 2 years ago

@vch9's comment made me realize that reusing the char shrinker for Gen.printable in QCheck(1) would break for '\n' (char code 10) by shrinking to a non-printable character. I've therefore committed an explicit Shrink.char_printable shrinker along with a few unit tests of the behaviour.

Looking at the code for Gen.printable in QCheck.ml I see the same approach of QCheck2 using a local array/table. This makes me think that the right solution(tm) would be to shrink towards the 'a' entry in this array (at index 97 - 32 or so). This could be done for both QCheck and QCheck2 and would clear up the weird QCheck2 character code jumping that is hard to specify.

jmid commented 2 years ago

I've now simplified the definition of QCheck2.Gen.printable. This makes the behaviour simpler to specify, as we always shrink towards 'a'. Bonuses:

I've also updated the tests and the CHANGELOG accordingly, so this should be good to go.

jmid commented 2 years ago

Rebased on latest master to resolve expect-test output conflicts