Merge remote branch 'origin/peppe/common_test/break_cont_error' into maint

* origin/peppe/common_test/break_cont_error:
  Make sure the test can never hang when ct:break/1/2 is called

4 files changed, 74 insertions, 34 deletions

diff --git a/lib/common_test/src/ct.erl b/lib/common_test/src/ct.erl
index e369c9f4ef..aa8813c391 100644
--- a/lib/common_test/src/ct.erl
+++ b/lib/common_test/src/ct.erl
@@ -157,7 +157,7 @@ run(TestDirs) ->
 %%%               {refresh_logs,LogDir} | {logopts,LogOpts} | 
 %%%               {verbosity,VLevels} | {basic_html,Bool} | 
 %%%               {ct_hooks, CTHs} | {enable_builtin_hooks,Bool} |
-%%%               {noinput,Bool}
+%%%               {release_shell,Bool}
 %%%   TestDirs = [string()] | string()
 %%%   Suites = [string()] | [atom()] | string() | atom()
 %%%   Cases = [atom()] | atom()
@@ -194,15 +194,24 @@ run(TestDirs) ->
 %%%   CTHs = [CTHModule | {CTHModule, CTHInitArgs}]
 %%%   CTHModule = atom()
 %%%   CTHInitArgs = term()
-%%%   Result = [TestResult] | {error,Reason}
-%%% @doc Run tests as specified by the combination of options in <code>Opts</code>.
+%%%   Result = {Ok,Failed,{UserSkipped,AutoSkipped}} | TestRunnerPid | {error,Reason}
+%%%   Ok = integer()
+%%%   Failed = integer()
+%%%   UserSkipped = integer()
+%%%   AutoSkipped = integer()
+%%%   TestRunnerPid = pid()
+%%%   Reason = term()
+%%% @doc <p>Run tests as specified by the combination of options in <code>Opts</code>.
 %%% The options are the same as those used with the
 %%% <seealso marker="ct_run#ct_run"><code>ct_run</code></seealso> program.
 %%% Note that here a <code>TestDir</code> can be used to point out the path to 
 %%% a <code>Suite</code>. Note also that the option <code>testcase</code>
 %%% corresponds to the <code>-case</code> option in the <code>ct_run</code> 
 %%% program. Configuration files specified in <code>Opts</code> will be
-%%% installed automatically at startup.
+%%% installed automatically at startup.</p>
+%%% <p><code>TestRunnerPid</code> is returned if <code>release_shell == true</code>
+%%% (see the User's Guide for details).</p>
+%%% <p><code>Reason</code> indicates what type of error has been encountered.</p>
 run_test(Opts) ->
     ct_run:run_test(Opts).
 
@@ -960,6 +969,8 @@ get_testdata(Key) ->
 	    Error;
 	{'EXIT',_Reason} ->
 	    no_tests_running;
+	undefined ->
+	    {error,no_testdata};
 	[CurrTC] when Key == curr_tc ->
 	    {ok,CurrTC};
 	Data ->
@@ -1159,7 +1170,8 @@ sync_notify(Name,Data) ->
 %%%-----------------------------------------------------------------
 %%% @spec break(Comment) -> ok | {error,Reason}
 %%%       Comment = string()
-%%%       Reason = {multiple_cases_running,TestCases}
+%%%       Reason = {multiple_cases_running,TestCases} |
+%%%                'enable break with release_shell option'
 %%%       TestCases = [atom()]
 %%%
 %%% @doc <p>This function will cancel all timetraps and pause the
@@ -1170,21 +1182,33 @@ sync_notify(Name,Data) ->
 %%%       test case. If a parallel group is executing, <c>break/2</c>
 %%%       should be called instead.</p>
 break(Comment) ->
-    case get_testdata(curr_tc) of
-	{ok,{_,TestCase}} ->
-	    test_server:break(?MODULE, Comment);
-	{ok,Cases} when is_list(Cases) ->
-	    {error,{multiple_cases_running,
-		    [TC || {_,TC} <- Cases]}};
-	Error -> 
-	    {error,Error}
+    case {ct_util:get_testdata(starter),
+	  ct_util:get_testdata(release_shell)} of
+	{ct,ReleaseSh} when ReleaseSh /= true ->
+	    Warning = "ct:break/1 can only be used if release_shell == true.\n",
+	    ct_logs:log("Warning!", Warning, []),
+	    io:format(user, "Warning! " ++ Warning, []),
+	    {error,'enable break with release_shell option'};
+	_ ->
+	    case get_testdata(curr_tc) of
+		{ok,{_,TestCase}} ->
+		    test_server:break(?MODULE, Comment);
+		{ok,Cases} when is_list(Cases) ->
+		    {error,{'multiple cases running',
+			    [TC || {_,TC} <- Cases]}};
+		Error = {error,_} -> 
+		    Error;
+		Error ->
+		    {error,Error}
+	    end
     end.
 
 %%%-----------------------------------------------------------------
 %%% @spec break(TestCase, Comment) -> ok | {error,Reason}
 %%%       TestCase = atom()
 %%%       Comment = string()
-%%%       Reason = test_case_not_running
+%%%       Reason = 'test case not running' |
+%%%                'enable break with release_shell option'
 %%%
 %%% @doc <p>This function works the same way as <c>break/1</c>,
 %%%       only the <c>TestCase</c> argument makes it possible to
@@ -1192,18 +1216,29 @@ break(Comment) ->
 %%%       <c>continue/1</c> function should be used to resume
 %%%       execution of <c>TestCase</c>.</p>
 break(TestCase, Comment) ->
-    case get_testdata(curr_tc) of
-	{ok,Cases} when is_list(Cases) ->
-	    case lists:keymember(TestCase, 2, Cases) of
-		true ->
+    case {ct_util:get_testdata(starter),
+	  ct_util:get_testdata(release_shell)} of
+	{ct,ReleaseSh} when ReleaseSh /= true ->
+	    Warning = "ct:break/2 can only be used if release_shell == true.\n",
+	    ct_logs:log("Warning!", Warning, []),
+	    io:format(user, "Warning! " ++ Warning, []),
+	    {error,'enable break with release_shell option'};
+	_ ->
+	    case get_testdata(curr_tc) of
+		{ok,Cases} when is_list(Cases) ->
+		    case lists:keymember(TestCase, 2, Cases) of
+			true ->
+			    test_server:break(?MODULE, TestCase, Comment);
+			false ->
+			    {error,'test case not running'}
+		    end;
+		{ok,{_,TestCase}} ->
 		    test_server:break(?MODULE, TestCase, Comment);
-		false ->
-		    {error,test_case_not_running}
-	    end;
-	{ok,{_,TestCase}} ->
-	    test_server:break(?MODULE, TestCase, Comment);
-	Error -> 
-	    {error,Error}
+		Error = {error,_} -> 
+		    Error;
+		Error ->
+		    {error,Error}
+	    end
     end.
 
 %%%-----------------------------------------------------------------
diff --git a/lib/common_test/src/ct_run.erl b/lib/common_test/src/ct_run.erl
index faf5786a54..d80d216f9e 100644
--- a/lib/common_test/src/ct_run.erl
+++ b/lib/common_test/src/ct_run.erl
@@ -76,7 +76,8 @@
 	       scale_timetraps = false,
 	       create_priv_dir,
 	       testspecs = [],
-	       tests}).
+	       tests,
+	       starter}).
 
 %%%-----------------------------------------------------------------
 %%% @spec script_start() -> void()
@@ -334,7 +335,8 @@ script_start1(Parent, Args) ->
 		     stylesheet = Stylesheet,
 		     multiply_timetraps = MultTT,
 		     scale_timetraps = ScaleTT,
-		     create_priv_dir = CreatePrivDir},
+		     create_priv_dir = CreatePrivDir,
+		     starter = script},
 
     %% check if log files should be refreshed or go on to run tests...
     Result = run_or_refresh(StartOpts, Args),
@@ -1003,7 +1005,8 @@ run_test2(StartOpts) ->
 		 stylesheet = Stylesheet,
 		 multiply_timetraps = MultiplyTT,
 		 scale_timetraps = ScaleTT,
-		 create_priv_dir = CreatePrivDir},
+		 create_priv_dir = CreatePrivDir,
+		 starter = ct},
 
     %% test specification
     case proplists:get_value(spec, StartOpts) of
@@ -1632,6 +1635,7 @@ do_run(Tests, Skip, Opts, Args) when is_record(Opts, opts) ->
 			      "run ct:start_interactive()\n\n",[]),
 		    {error,interactive_mode};
 		_Pid ->
+		    ct_util:set_testdata({starter,Opts#opts.starter}),
 		    compile_and_run(Tests, Skip,
                                     Opts1#opts{verbosity=Verbosity}, Args)
 	    end
@@ -1677,8 +1681,9 @@ compile_and_run(Tests, Skip, Opts, Args) ->
 	    
 	    {Tests1,Skip1} = final_tests(Tests,Skip,SavedErrors),
 	    
-	    possibly_spawn(true == proplists:get_value(noinput, Args),
-			   Tests1, Skip1, Opts);
+	    ReleaseSh = proplists:get_value(release_shell, Args),
+	    ct_util:set_testdata({release_shell,ReleaseSh}),
+	    possibly_spawn(ReleaseSh == true, Tests1, Skip1, Opts);
 	false ->
 	    io:nl(),
 	    ct_util:stop(clean),
diff --git a/lib/common_test/src/ct_testspec.erl b/lib/common_test/src/ct_testspec.erl
index de63ac3b75..a8b67d0329 100644
--- a/lib/common_test/src/ct_testspec.erl
+++ b/lib/common_test/src/ct_testspec.erl
@@ -779,8 +779,8 @@ add_tests([{event_handler,Node,HOrHs,Args}|Ts],Spec) ->
 add_tests([{enable_builtin_hooks,Bool}|Ts],Spec) ->
     add_tests(Ts, Spec#testspec{enable_builtin_hooks = Bool});
 
-add_tests([{noinput,Bool}|Ts],Spec) ->
-    add_tests(Ts, Spec#testspec{noinput = Bool});
+add_tests([{release_shell,Bool}|Ts],Spec) ->
+    add_tests(Ts, Spec#testspec{release_shell = Bool});
 
 %% --- handled/errors ---
 add_tests([{define,_,_}|Ts],Spec) ->		% handled
@@ -1283,7 +1283,7 @@ valid_terms() ->
      {ct_hooks,2},
      {ct_hooks,3},
      {enable_builtin_hooks,2},
-     {noinput,2},
+     {release_shell,2},
      {multiply_timetraps,2},
      {multiply_timetraps,3},
      {scale_timetraps,2},
diff --git a/lib/common_test/src/ct_util.hrl b/lib/common_test/src/ct_util.hrl
index 7b08e78433..196b5e46d0 100644
--- a/lib/common_test/src/ct_util.hrl
+++ b/lib/common_test/src/ct_util.hrl
@@ -43,7 +43,7 @@
 		   event_handler=[],
 		   ct_hooks=[],
 		   enable_builtin_hooks=true,
-		   noinput=false,
+		   release_shell=false,
 		   include=[],
 		   auto_compile=[],
 		   stylesheet=[],