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

QCheck.numeral_string may shrink to non-numerals #257

Closed jmid closed 1 year ago

jmid commented 1 year ago

While reviewing #245 I just realized that some of our arbitrary string generators in QCheck may have surprising shrinking behavior. For QCheck.numeral_string this means that it may shrink to non-numeral strings:

# let t = Test.make QCheck.numeral_string (fun n -> n <> "" && 5 < int_of_string n);;
val t : QCheck.Test.t = QCheck2.Test.Test <abstr>
# QCheck.Test.check_exn t;;
Exception:
test `anon_test_4` raised exception `Failure("int_of_string")`
on `"a" (after 19 shrink steps)`
# 

This is a consequence of how string_gen and string_gen_of_size falls back on a generic Shrink.string https://github.com/c-cube/qcheck/blob/fa6481fb3ecdc8fe4406ad95c883a136e4cf92f7/src/core/QCheck.ml#L1112-L1128

Generators numeral_string and numeral_string_of_size are definitely affected. I suspect that printable_string printable_string_of_size, and small_printable_string may also shrink to strings with non-printable characters under the right conditions.

n-osborne commented 1 year ago

I'm happy to tackle this issue.

I see two ways of doing it:

  1. adding an optional shrink argument, with default to the present value so that we don't beak backward compatibility, to Shrink.string, string_gen_of_size and string_gen so that we can fix numeral_string and the others
  2. add a parameterized string shrinker (that takes a char shrinker as argument), a string_gen_of_size_with_shrinker and a string_gen_with_shrinker that we will use in the functions we want to fix.

1 is less code but modifies an existing signature in the api while 2 is more code but does not modify existing signature (we can decide whether to add the new ones or not).

What do you think?