proper icon indicating copy to clipboard operation
proper copied to clipboard

Parallel crash fix, #224

Open x4lldux opened this issue 4 years ago • 3 comments

Fix for #224

x4lldux avatar Mar 26 '20 22:03 x4lldux

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

x4lldux avatar Mar 29 '20 11:03 x4lldux

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 avatar Apr 01 '20 22:04 kostis

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.

kostis avatar Apr 28 '20 20:04 kostis