propcheck
propcheck copied to clipboard
[WIP] Allow ExUnit assert function for forall return
Issue: https://github.com/alfert/propcheck/issues/187
Currently, propcheck forall function requires a boolean value return, which bit inconvenient when using ExUnit assert function. This PR fixes the issue and allow the assert function with non-boolean result
TODO:
- write a proper test
- handle tuple assertion
Hi @drselump14 ,
thank you for your PR, but I guess you are solving the problem at a wrong place: PropEr and thus PropCheck require a boolean return value. assert requires a boolean assertion. We handle assertion errors as a handy special case of false.
But if I understand your approach correctly, then you kind of change the semantics of assert to handle non-boolean arguments differently - but only inside a forall. If you find a counterexample and put that into a regular test, your pimped assert will not work any longer. That is confusing and does not help anybody. The correct way of solving this is to convince the Elixir core team to make assert more flexible, in which case we can benefit in PropCheck as well.
Did I got your intention right?
Cheers, Klaus.
Hi @alfert , thanks for the comment.
You're right about PropEr and thus PropCheck require a boolean return value.
However, I think assert requires a boolean assertion is not a true statement.
The assert evaluate truthy value as mentioned in docs.
The assert compatibility issue with PropCheck/PropEr occurs when we evaluate non-boolean truthy value with assert.
Because assert return the evaluated value, it'll return the non-boolean truthy value which PropEr doesn't like.
For example:
property "returns a map" do
forall i <- pos_integer() do
assert %{val: ^i} = make_map(i) # This doesn't work and raises an error {:error, :non_boolean_result}
end
end
It happens because PropEr forall only pattern matching the boolean result value, and tuple result value (from willfail, etc), and reject any other kind of value
https://github.com/proper-testing/proper/blob/9f6a6501430479bed66d08cd795cd34d36ec83aa/src/proper.erl#L1769
TBH I'm not sure if this PR is the best approach to solve the issue.
I think the best approach is to convince the PropEr team to allow a truthy value to be passed to the forall method.
However, I don't think they will accept it because it's a requirement from elixir world.
I am probably projecting my own wishes on this PR but ExUnit.Assertions are great due to how much information is available when the assertion fails. One of the hardest things I've found while using PropCheck is the ability to debug "when something goes wrong" - ex unit's assertions on the other hand provide very handy diffs.
It's incredibly likely I just don't have PropCheck/proper experience to show better diffs - typically when a post condition fails I have to rerun the test with a bunch of IO.puts to track why the postcondition failed, if instead I got the diff from assert (and friends) the IO.puts might be unnecessary.
Yes, reporting facilities from assert are really really nice. Perhaps we can tackle this the other way around: Perhaps we can replicate the code of assert (or even call it? - may be difficult due to macros) to show the values of a failing property in a nicer way?
@alfert I can't imagine the assert code changing any time soon so copy pasting seems fine given that it is implemented as macros. I'm more unsure how'd you go about doing this given proper expects boolean return values...
Note that the :verbose option (https://github.com/alfert/propcheck/pull/48) also outputs the results of assertions. One can also use PROPCHECK_VERBOSE=1 to set that option globally.