-
Notifications
You must be signed in to change notification settings - Fork 30
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
Support for 'where' and 'counterexample' custom operations #29
Conversation
I also renamed the 'forAll' builder to 'property' as this is what it really is.
An example of the new operations in action: Property.check <| property {
let! x = Gen.int
let! y = Gen.int
where (x > 0 && y > 0)
counterexample (sprintf "x * y = %d" <| x * y)
return x * y > 0
} |
| Success of 'a | ||
|
||
type Property<'a> = | ||
| Property of Gen<Report * Result<'a>> |
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.
Report
is there to support #25, right?
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 was just for failure messages, but it could be used to support #25 or any other types of things that we might want to track during a test run.
type Property<'a> = | ||
| Property of Gen<Report * Result<'a>> | ||
|
||
module Tuple = |
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.
How about marking Tuple
as private
? I don't think we care about exposing it to the users. 😉
Edit: I've now opened #30 for this.
type Property<'a> = | ||
| Property of Gen<Report * Result<'a>> | ||
|
||
module Tuple = |
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.
How about marking Tuple
as private
? I don't think we care about exposing it to the users. 😉
Edit: I've now opened #30 for this.
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.
Sounds good, will check out your PR
|
||
[<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>] | ||
module Property = | ||
open Tuple |
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.
If we mark Tuple
as private
, we may then [<AutoOpen>]
it, and then we can get rid of that open
directive.
Edit: I've now opened #30 for this.
check' 100 p | ||
|
||
[<AutoOpen>] | ||
module ForAllBuilder = | ||
module PropertyBuilder = | ||
open Tuple |
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.
check' 100 p | ||
|
||
[<AutoOpen>] | ||
module ForAllBuilder = | ||
module PropertyBuilder = |
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! I find Property
a better name, since we may not only support universal quantification in the future. 😉
One day, if we decide to support existentials (à la SmallCheck), we may still do that through some custom operation(s), and then we won't have to rename the builder itself – which may be a breaking change if we follow SemVer.
It's also great that we don't have to use a Property.check <| property {
let! n = Gen.int
where (n <> 0)
return 1 / n = 1 / n
}
//+++ OK, passed 100 tests.
//val it : bool = true |
👍 This is looking good!
|
The key here is that |
I also renamed the
forAll
builder toproperty
as I think it's more correct. I think this name is better, not completely sure./cc @moodmosaic