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

Parallel crash fix, #224 #227

Open x4lldux opened 4 years ago

x4lldux commented 4 years ago

Fix for #224

x4lldux commented 4 years ago

@kostis it's no longer WIP and is ready for review.

kostis commented 4 years ago

Thanks for the #224 issue and this PR. If all goes well, I will look into this before the end of the week.

In the meantime, you may want to correct an erroneous any that should read term() in the return of execute/3,

kostis commented 4 years ago

OK, it took longer than expected because in the meantime I decided to first increase the test suite coverage and then to fix two statem tests to actually test what they are supposed to, but now I have looked at this.

Long story short: I do not (fully) understand the issue and I have doubts that the test shows what you think it shows. I would like to see a test where a property like this one

prop_exception() ->
    ?FORALL(Cmds, commands(?MODULE),
        begin
        {_History,_State,Result} = run_commands(?MODULE, Cmds),
        Result =:= ok
        end).

fails because Result is not ok but an {exception,_,_,_} tuple, while this property

prop_parallel_exception() ->
    ?FORALL(Cmds, parallel_commands(?MODULE),
        begin
        {_History,_State,Result} = run_parallel_commands(?MODULE, Cmds),
        Result =:= ok
        end).

does not do what it is supposed to because of an exception, but it does with the PR that fixes the issue reported in #241.

Note the crucial difference in what I am suggesting and what this PR contains as test: run_parallel_commands/2 is supposed to be run with a pair containing a list of lists of commands in its 2nd element (as produced by parallel_commands/1) while the 2nd arg of run_commands/2 takes as input just a list of commands (as produced by commands/1).

Please supply the test case first that shows what goes wrong without this PR.