-
Notifications
You must be signed in to change notification settings - Fork 99
Generating expr #157
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
Generating expr #157
Conversation
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.
I'm not quite convinced this fully solves the problem. We want to guarantee (as best we can) that every valid HashMap
tree is available to the Arbitrary
instance. Do we have good reasons to believe that all such trees can be generated by this language?
An alternative, more complicated, approach (but quite possibly the right one) would be to use an expression language that includes all primitive operations, interpreting it using HashMap
or HashSet
and also (say) Data.Map
or Data.Set
.
One of the key ideas of the containers
test suite is that all the invariants for each data structure are expressed in a single valid
function. In this context, that would allow your Expr
interpretation to find the smallest invalid subexpression, rather than waiting for that invariant failure to show up as a hard-to-debug semantic error.
arb n = oneof $ leafs ++ | ||
[ liftA3 ExprInsert arbitrary arbitrary (arb (n - 1)) | ||
, liftA2 ExprDelete arbitrary (arb (n - 1)) | ||
, liftA2 ExprUnion (arb (n `div` 2)) (arb (n - n `div` 2)) |
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.
Why div
and not quot
?
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.
Prelude Test.QuickCheck> 3 `div` 2 + 3 `quot` 2
2
I don't know, I looked at non-trivial
AFAICS the
That's a good idea, but it can be added independently. I'm not 100% sure what are the invariants. |
Can you rebase this, please? |
rebased |
ping @treeowl, is there something I can do to get this off my "open PR" -list? :) |
Uh, I seem to have forgotten about this. I'll try to get to it in the next couple days. |
This seems to be still forgotten... |
I think we've removed the need for it by removing the representation
redundancy. See the changes in Eq implementation.
…On Mon, Apr 15, 2019, 4:08 PM Oleg Grenrus ***@***.***> wrote:
This seems to be still forgotten...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#157 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABzi_a16EfMStl1eIeoxWmncYbDLEowPks5vhNwrgaJpZM4N4WVP>
.
|
This is try to fix #154
We could rewrite all
[Key]
or[(Key, Int)]
based properties intoExpr Key ()
orExpr Key Int
based ones, respectively. I think includingunion
is worth it, as it is has non-trivial implementation.First commit is from #156.