-
Notifications
You must be signed in to change notification settings - Fork 44
Deriver: qcheck and qcheck2 #209
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
Conversation
a48a357 to
14be7e2
Compare
| let ty = Ldot (Ldot (Lident "QCheck", "Gen"), "t") | ||
| let ty = function | ||
| | `QCheck -> Ldot (Ldot (Lident "QCheck", "Gen"), "t") | ||
| | `QCheck2 -> Ldot (Ldot (Lident "QCheck2", "Gen"), "t") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would introducing a VERSION module and changing these to use a to_module function improve this duplication?
module type VERSION = sig
type t
val version : t
val to_module : t -> label
end
type version = [`QCheck | `QCheck2]
let to_module = function
| `QCheck -> "QCheck"
| `QCheck2 -> "QCheck2"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you're right, I believe we would never have more than 2 versions but that'd simplify the code a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although, I don't see the real benefit of the module type VERSION.
Isn't:
type version = [`QCheck | `QCheck2]
let to_module : version -> string = function
| `QCheck -> "QCheck"
| `QCheck2 -> "QCheck2"
let ty version = Ldot (Ldot (Lident (to_module version), "Gen"), "t")
...
module Make (Version : sig val version : version end) = structenough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be enough, I would go with that.
Making it a module type might be slightly tidier if more than 2 versions were expected or there were other places that needed that type. In my hacking it didn't seem worthwhile.
14be7e2 to
8ce4ca7
Compare
|
Used @tmcgilchrist suggestion to reduce the code duplication and rebased to master. The commits should be squashed on merge, I'm not sure each of them compile and it's not that important if they're split. |
Fix tests on 32bit arch
Fix: allow 0 count and 0 long_factor
Fix "unknown option" error message from runner
Update expected 32bit output for test_gen_no_shrink
CI dune runtest on latest MacOSX too
Run expect tests on OCaml5
Run CI workflow on OCaml5 too
…-qcheck-and-qcheck2
Closes #190
This PR introduces two derivation plugins:
qcheckqcheck2The code and test is duplicated, in the vast majority it's only a renaming to
QChecktoQCheck2(excepts forfun_nary)This it not ideal as we only derive generators then call make on them, the best would be to use arbitrary for primitive types as they also have printers, shrinkers etc.
However after #195, #208 and this one, I'm getting tired of writing boilerplate code, but I will eventually do this in the future :)