Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QCheck.numeral_string may shrink to non-numerals #257

Closed
jmid opened this issue Sep 12, 2022 · 1 comment · Fixed by #258
Closed

QCheck.numeral_string may shrink to non-numerals #257

jmid opened this issue Sep 12, 2022 · 1 comment · Fixed by #258

Comments

@jmid
Copy link
Collaborator

jmid commented Sep 12, 2022

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

qcheck/src/core/QCheck.ml

Lines 1112 to 1128 in fa6481f

let string_gen_of_size size gen =
make ~shrink:Shrink.string ~small:String.length
~print:(sprintf "%S") (Gen.string_size ~gen size)
let string_gen gen =
make ~shrink:Shrink.string ~small:String.length
~print:(sprintf "%S") (Gen.string ~gen)
let string = string_gen Gen.char
let string_of_size size = string_gen_of_size size Gen.char
let small_string = string_gen_of_size Gen.small_nat Gen.char
let printable_string = string_gen Gen.printable
let printable_string_of_size size = string_gen_of_size size Gen.printable
let small_printable_string = string_gen_of_size Gen.small_nat Gen.printable
let numeral_string = string_gen Gen.numeral
let numeral_string_of_size size = string_gen_of_size size Gen.numeral

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
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants