Skip to content
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

Treat non-boolean return value as failed test case #130

Open
josefs opened this issue Nov 21, 2016 · 5 comments
Open

Treat non-boolean return value as failed test case #130

josefs opened this issue Nov 21, 2016 · 5 comments

Comments

@josefs
Copy link

josefs commented Nov 21, 2016

Currently, if a property return something else than true or false, proper treats this as an error in the property and aborts testing. I think it should be treated like a failed test case instead.

The reason I would like this behavior is that sometimes I write rather big properties which test several things. If such a property fails, it can be hard to know what actually went wrong. In these cases it is useful to fail with an atom describing what went wrong.

One might argue that it is bad design to write such large and composite properties but sometimes you want to test several things which depend on some expensive computation. In these cases it is more efficient to have one big property rather than many small as they would do a lot of recomputation.

@kostis
Copy link
Collaborator

kostis commented Nov 21, 2016

I am not sure I understand this issue. What does efficiency, size of properties, avoiding re-computation, etc. have to do with properties returning something other than true or false? Why would you want to write non-Boolean properties in the first place? Can you show us some (preferably convincing) example where this is useful or what you would like to achieve?

Moreover, even if such an example indeed exists, why can't you achieve what you are after by explicitly checking for equality with true? I.e. instead of

prop_complicated() ->
   ?FORALL(I, input(), complicated_property(I)).

write:

prop_complicated() ->
   ?FORALL(I, input(), complicated_property(I) =:= true).

Or am I missing the point here?

@josefs
Copy link
Author

josefs commented Nov 22, 2016

Ah, yes, I left out one crucial piece of information. Currently, when proper finds a failing test case it returns false which is then printed by the erlang shell. My wish is that if my property returned an atom, say index_not_well_distributed, that proper would treat this as a failed test case and then return this atom as a result of calling the quickcheck function, and thereby have the erlang shell print the atom. That way I can let my property return more useful information on what went wrong rather than just saying true or false.

I don't understand how checking for equality with true would help me.

@josefs
Copy link
Author

josefs commented Nov 22, 2016

I'm afraid that I don't have any example properties that I can share, they're all proprietary.

@josefs
Copy link
Author

josefs commented Nov 23, 2016

Here's an outline of the kind of property that I have in mind:

prop_big_and_bulky() ->
  ?FORALL(DATA,generator(),
          begin
            X = expensive:function(DATA),
            Y = test1(X),
            Z = test2(X)
            Y andalso Z
          end)

In my case expensive:function/1 performs a form of optimization and computes a matrix. The two tests test1 and test2 checks the validity of the solution along the columns and rows respectively. Performing the optimization is relatively expensive while checking the validity is relatively cheap. Therefore I don't want to decompose the property into two properties which uses test1 and test2 respectively.

I would like to write the property as follows:

prop_big_and_bulky() ->
  ?FORALL(DATA,generator(),
          begin
            X = expensive:function(DATA),
            Y = test1(X),
            Z = test2(X)
            case Y of
              false -> test_1_failed;
              true  -> case Z of
                         false -> test_2_failed;
                         true  -> true
          end)

My wish is that if the property fails with test_1_failed, that PropEr would treat that result as a failed test case, show me the counter example and finally return test_1_failed. That way I know right away that it was test1 that failed.

I hope this clarifies what I'm after.

@kostis
Copy link
Collaborator

kostis commented Nov 23, 2016

It does clarify the issue, but I do not think your example is very convincing.

I can definitely see why you want to avoid multiple calls to expensive:function/1 and why you do not want to decompose the property into multiple ones, but if your big and bulky property is just a conjunction of simple(r) ones, I do not really see why you cannot test manually, after the PropEr has shrunk the randomly generated DATA as much as it can, which of the test1, test2 (possibly more) simple checks is the one that failed. It cannot possibly be much hassle to copy-paste the shrunk input from the shell and run them by hand to discover whether test_1_failed or test_2_failed, right? (Especially since, as you write, checking the validity is relatively cheap.)

More generally, I still fail to see why you want us to change the interface of proper:quickcheck/1, which most likely is the most commonly used API function out there and (legacy) code may depend on it, in order to handle a case which is supposed to be useful only when debugging. Note that there will come a point that you will have debugged your software, and finding data that breaks your properties will be the exception, right?

It's of course conceivable that the above example is just an over-simplification of what you really want to do. In that case, please supply something more involved (and presumably more convincing).

Unrelated to the issue (and something that you most likely know already): Note that if you write your property as follows:

prop_big_and_bulky() ->
  ?FORALL(DATA,generator(),
          begin
            X = expensive:function(DATA),
            test1(X) andalso test2(X)
          end)

PropEr will not even run the test2 check in case test1 check fails, thereby falsifying your property even faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants