vincent-hugot / qtest

Inline (Unit) Tests for OCaml
GNU General Public License v3.0
67 stars 8 forks source link

max_gen decreased twice in QCheck #43

Closed jmid closed 7 years ago

jmid commented 7 years ago

For the following test case: open QCheck let is_even i = (i mod 2 = 0) let is_odd i = (i mod 2 = 1) let t = Test.make pos_int (fun i -> (is_even i) ==> (is_odd (succ i))) in QCheck_runner.run_tests ~verbose:true [t];;

The framework will generate fewer than (the default) 100 test cases even though we are (seemingly) below the (default) max_gen of 300 generated inputs, e.g.: law : 96 relevant cases (198 total)

It seems to me that the culprit is line 793 of function check_state in QCheck.ml: state.cur_max_gen <- state.cur_max_gen - 1; which will (again) decrease cur_max_gen even though function new_input on line 714 has already done so.

c-cube commented 7 years ago

Indeed, that seems wrong.

jmid commented 7 years ago

Thanks for a quick fix and a nice library! I see even more improvements to this latest version integrated into iTemML/qtest. I support factoring the improved version into a new separate qcheck release (as you mention yourself in #42).

c-cube commented 7 years ago

Yes, I should probably do that… I will make a regular release soon, and see later about the split.

jmid commented 7 years ago

Great, thanks! I'm preparing to use QCheck (in iTeML/qtest) in a course on QuickCheck in January, so the release is warmly welcome :-) Thanks again for your efforts.

c-cube commented 7 years ago

Oh! That's nice to hear. Is there anything in the lib that you would like improved, or aspects you don't like?

c-cube commented 7 years ago

Well, release 2.4 in progress: https://github.com/ocaml/opam-repository/pull/8107 Let's not wait too long for those bugfixes ;-)

jmid commented 7 years ago

I'm still in the process of porting the course material from Kaputt to QCheck. When I was preparing the material last year it seemed Kaputt was the only QuickCheck library supporting both classification and shrinking, both of which I believe are important QuickCheck features. Kaputt however has its own issues (it is focused on [precond ==> postcond] properties requiring workarounds to test, e.g., related outputs on related inputs in tests, it is missing a user-supplied bound on the number of printed counterexamples, ...)

I've used a previous version of QCheck in the past for some of my research work (see https://github.com/jmid/lcheck ) and loved the monadic interface. Since you now support both shrinking and classification it was therefore an easy decision :-) I like how you've integrated generators, printers, and shrinkers in records ("poor man's type classes"). I will need to work a bit more with them to provide meaningful feedback.

For now the ~max_gen issue surprised me. To students (first time users) letting ~verbose:true be the default in QCheck_runner.run_tests would be nice as the framework is otherwise very quiet about what is happening beneath the surface.

Thanks again!

c-cube commented 7 years ago

If you want to discuss in more details, feel free to drop me a mail, or say hi on irc (I'm companion_cube on #ocaml@freenode). I'm not sure about the verbose option, I tend to use QCheck_runner.run nowadays (very compact, it's convenient for the > 800 tests in one of my libs), but it's possible. How about telling the students to use run_tests_main so they can add --verbose on the command line (mere suggestion)?

jmid commented 7 years ago

Ah, I forgot: the first time I used ~count:1000 (without ~max_gen) in Test.make and only got 300 tests run it really surprised me. Effectively this forces one to specify two bounds every time you would want to run more than 300 tests (or fewer with preconditions). Thanks for the suggestion about run_tests_main !

c-cube commented 7 years ago

Yes, I just changed this behavior on max_gen. The default value is now count+200 for that very reason!

c-cube commented 7 years ago

@jmid take a look at the master branch of https://github.com/c-cube/qcheck ;-)

jmid commented 7 years ago

Wonderful - thank you! I can tell we are close to Christmas with such a nice present :-)

c-cube commented 7 years ago

@jmid migration finished.