proper-testing / proper

PropEr: a QuickCheck-inspired property-based testing tool for Erlang
http://proper-testing.github.io
GNU General Public License v3.0
882 stars 167 forks source link

Add shrinking indication parameter #232

Open x4lldux opened 4 years ago

x4lldux commented 4 years ago

Set's a shrinking parameter to true when property is being executed during a shrinking phase. Then sets it to done after the shrinking is finished. Useful for adjusting generators during shrinking phase or for on_output printers, for example, so they can suppress printing.

kostis commented 4 years ago

Thanks for your PR. However, I do not understand what it does, at least on its own.

If this is a stand-alone PR, please supply some test cases that show its intended use and/or functionality (perhaps in conjunction with the on_output parameter as you write in the commit message?). Unfortunately, the current test does not really show something interesting; for example, the current on_output fun does not do any printing; also, it seems strange that all three values for the shrinking indication parameter appear at the same time in the parameter list -- or did I misunderstand the test? In any case, the test needs to be improved.

On the other hand, if this supposed to be just supporting functionality for #230, then please add the changes there.

x4lldux commented 4 years ago

Thanks for your PR. However, I do not understand what it does, at least on its own.

It introduces a parameter (as in propter_types:parameter/2) that indicates PropEr is in shrinking stage.

On the other hand, if this supposed to be just supporting functionality for #230, then please add the changes there.

This PR can be viewed as part for #230, but for that specific cases mentioned there, it would be sufficeable to just use the ?SHRINK macro. So I opted for a separate PR.

please supply some test cases that show its intended use and/or functionality (perhaps in conjunction with the on_output parameter as you write in the commit message?)

It is a fact, that I intend to use this in PropCheck (Elixir wrapper library for PropEr) to suppress output while in shrinking stage, as mentioned in commit message as an example. But unfortunately, even though EUnit captures test's output, I couldn't find any way to match on it. And there isn't any other test that tests on_output functionality from which I could get an inspiration of how to do it properly. Only a TODO note with a suggestion of using process dictionary, which I used. My test just ensures that every of three states the shrinking parameter can be in, was in fact present in pdict. Maybe I'll put comments inside the test to describe it's actions.

kostis commented 4 years ago

It introduces a parameter (as in proper_types:parameter/2) that indicates PropEr is in shrinking stage.

Yes, I could see that. My question was asking for an example that shows how/where this new functionality would be used. I.e., some evidence why would one want that in PropEr.

I intend to use this in PropCheck to suppress output while in shrinking stage, as mentioned in commit message as an example. But unfortunately, even though EUnit captures test's output, I couldn't find any way to match on it. And there isn't any other test that tests on_output functionality from which I could get an inspiration of how to do it properly.

First of all, PropEr has a noshrink option that can be used to suppress the shrinking phase altogether, thus providing an indirect way to suppress output from shrinking ;-)

But it seems that you do not want that -- if I understand correctly, you want to suppress printing just while shrinking. The on_output function can actually do that. Here is how:

-module(on_out).
-export([test/1, print_magenta/2, print_not_during_shrinking/2]).

-include_lib("proper/include/proper.hrl").
-include_lib("eunit/include/eunit.hrl").

test(Fun) ->
   proper:quickcheck(prop_me_up_scotty(), [{on_output, Fun}]).

print_magenta(S, L) ->
   io:format("\033[1;35m"++S++"\033[0m", L).

-define(WHATEVER_YOU_DESIRE, no_printing_during_shrinking_please).

print_not_during_shrinking(S, L) ->
   case get(?WHATEVER_YOU_DESIRE) of
     undefined ->
       case string:find(S, "Shrinking") of
         nomatch -> io:format(S, L);
         _ -> put(?WHATEVER_YOU_DESIRE, in_shrinking_phase)
       end;
     in_shrinking_phase ->
       case string:find(S, "time(s)") of
         nomatch -> ok; % no printing during shrinking
         _ -> erase(?WHATEVER_YOU_DESIRE) % shrinking just ended
       end
   end.

prop_me_up_scotty() ->
  ?FORALL(I, integer(), I < 42).

Here is what I get when using it -- colors are not shown properly here:

Eshell V10.3.4  (abort with ^G)
1> on_out:test(fun on_out:print_magenta/2).
..............................................................!
Failed: After 63 test(s).
47

Shrinking .(1 time(s))
42
false
2> on_out:test(fun on_out:print_not_during_shrinking/2).
.............................................................!
Failed: After 62 test(s).
114
42
false

If you want something other/more than the above, please supply an example.

x4lldux commented 4 years ago

First of all, PropEr has a noshrink option that can be used to suppress the shrinking phase altogether, thus providing an indirect way to suppress output from shrinking ;-)

Nope :grin:


While this would work for some cases, it wouldn't work for all. It fails if for example, while shrinking, a more complex test would return a non boolean, or a ?SUCHTHAT generator wouldn't be able to produce a valid case, etc. In such cases nothing would be printed, because proper doesn't print "time(s)" string, just reports the error.

Also, it isn't an optimal solution to depend on quasi-syntax of output for semantics and inferring the internal state of PropEr. Not a one I would feel comfortable to put in a PropCheck library.

kostis commented 4 years ago

First of all, PropEr has a noshrink option that can be used to suppress the shrinking phase altogether, thus providing an indirect way to suppress output from shrinking ;-)

While this would work for some cases, it wouldn't work for all. It fails if for example, while shrinking, a more complex test would return a non boolean,

I do not understand how telling PropEr to not do any shrinking at all can result in failing during shrinking... Unless of course you are replying in another part of the above message.

x4lldux commented 4 years ago

Let me rephrase that. I didn't wanto to quote the code example.

kostis commented 4 years ago

Can you please not edit / change the messages you have written, thus making the discussion here unreadable for future readers?

More importantly, can you please provide some example of what you really want to do and show me/us why you cannot achieve that with the machinery (on_print, ?SHRINK, with_parameter, etc.) that PropEr currently provides?

Adding stuff to the process dictionary, besides being ugly, may complicate other PropEr efforts that are currently going on. For example, the parallelization work.

x4lldux commented 4 years ago

More importantly, can you please provide some example of what you really want to do and show me/us why you cannot achieve that with the machinery (on_print, ?SHRINK, with_parameter, etc.) that PropEr currently provides?

So usecase is to reduce noisy printing in PropCheck by stop PorpEr from printing while shrinking.

This can't be done with on_print and inspecting the output, like in your example, because inspecting can give just an approximation of PropEr's state. Also, the output generated by PropEr is not part of it's public interface (there is no docs for what is printed when, and a lot of it is hidden in private functions) and can change at any moment.

This can't be done with ?SHRINK or with_parameter, because tests are not written by us, but by PropCheck's users. We can't force users to boilerplate their tests themselves just to let us reduce the printing noise.

Adding stuff to the process dictionary, besides being ugly, may complicate other PropEr efforts that are currently going on. For example, the parallelization work.

Do note, that I'm not introducing any new process dictionary keys, but reusing the existing one, which is set by with_parameter/3 and read by parameter/2. This PR is effectively reusing an existing feature to add a builtin parameter shrinking (like ECQ does).

codecov-io commented 4 years ago

Codecov Report

Merging #232 into master will increase coverage by 0.79%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #232      +/-   ##
==========================================
+ Coverage   82.24%   83.04%   +0.79%     
==========================================
  Files          13       13              
  Lines        3166     3167       +1     
==========================================
+ Hits         2604     2630      +26     
+ Misses        562      537      -25     
Impacted Files Coverage Δ
src/proper.erl 85.09% <100.00%> (+2.85%) :arrow_up:
src/proper_target.erl 87.50% <0.00%> (-1.74%) :arrow_down:
src/proper_fsm.erl 80.32% <0.00%> (-1.64%) :arrow_down:
src/proper_typeserver.erl 73.40% <0.00%> (-0.12%) :arrow_down:
src/proper_types.erl 91.77% <0.00%> (ø)
src/proper_statem.erl 92.17% <0.00%> (+0.43%) :arrow_up:
src/proper_shrink.erl 95.74% <0.00%> (+0.53%) :arrow_up:
src/proper_gen_next.erl 77.41% <0.00%> (+1.20%) :arrow_up:
src/proper_sa.erl 92.15% <0.00%> (+2.15%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5b66abe...ca41234. Read the comment docs.

kostis commented 4 years ago

Can you please provide some example of what you really want to do and show me/us why you cannot achieve that with the machinery (on_print, ...) that PropEr currently provides?

So use case is to reduce noisy printing in PropCheck by stop PropEr from printing while shrinking.

This can't be done with on_print and inspecting the output, like in your example, because inspecting can give just an approximation of PropEr's state. ...

I've given the issue some more thought and I am still not convinced that this is the PropEr(TM) solution to this particular use case. To control the amount of printing that users see, PropEr currently provides options verbose (default) and quiet. These are all-or-nothing, and perhaps there are users/tools out there that desire finer control over the amount of output they get. For example, output on running tests and no output on shrinking (your/PropCheck's use case), or perhaps the other way around (silent/less verbose running of tests and output on shrinking).

My suggestion would be to extend the options with something along the lines of

-type phase() :: testing | shrinking.
-type phase_output() :: {phase(), quiet | laconic | verbose | output_fun()}.

which also allows users to supply their own output functions for the two different phases, if they so wish. For the laconic option, I imagine just a line with a counter (e.g. Run test #N, Shrunk #N time(s)) that continuously gets updated.

Comments?

x4lldux commented 4 years ago

I like your proposal! I like the idea of exposing phase instead of just shrinking and it's something similar to what I was thinking when considering semantic printing for PropEr. Though instead of adding another output function, I suggest extending current one with more information (and do arity recognition to stay backwards-compatible). And instead of just passing format and data, also pass some metadata like current phase and type of printout. So for example instead of Print("!", []) in perform_search/8, there'd be something like Print({search_fail, Phase}, "!", []). That would systematize output and make it easier for any one wanting to create a wrapper around PropEr for their BEAM language and have printouts in their syntax.

kostis commented 4 years ago

NOTE: I've slightly edited the above comment and moved a part which is related to a different issue (#246) so that it can be discussed on its own.

david-kubecka commented 4 years ago

I came here to discuss slightly different use case but at the end I think it's very much related.

In my property test I'm constructing huge terms which produce a lot of noise when printed out by PropEr on assertion failure. So I would like to disable printing of these generated terms while keeping the other PropEr output. I can imagine that there could be some new formatting option similar to existing {on_output, Fun/2}. The Fun would this time take 3 arguments:

fun (MessageType, Format, Data) ->
    <do whatever you want>
end

where Format and Data are the same as in on_output while MessageType is an atom describing current message type (or phase or... TBD). So the values would include at least:

progress_indicator
stacktrace
test_input
shrink_input

In my case I would then pattern match on test_input to ignore it while printing everything else.

What do you think? Is my way somehow aligned with the use case of @x4lldux ?