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

* origin/peppe/common_test/ct_hooks_errors:
  Fix IO printout crash caused by hook function
  Fix timetrap error in pre-hooks

Conflicts:
	lib/common_test/src/cth_surefire.erl

OTP-10050
OTP-10069
OTP-10072

4 files changed, 72 insertions, 35 deletions

diff --git a/lib/common_test/src/ct_framework.erl b/lib/common_test/src/ct_framework.erl
index 11575cd0fb..e53383e038 100644
--- a/lib/common_test/src/ct_framework.erl
+++ b/lib/common_test/src/ct_framework.erl
@@ -204,7 +204,7 @@ init_tc2(Mod,Suite,Func,SuiteInfo,MergeResult,Config) ->
 			   data={Mod,FuncSpec}}),
 
     case catch configure(MergedInfo,MergedInfo,SuiteInfo,
-			 FuncSpec,Config) of
+			 FuncSpec,[],Config) of
 	{suite0_failed,Reason} ->
 	    ct_util:set_testdata({curr_tc,{Mod,{suite0_failed,{require,Reason}}}}),
 	    {skip,{require_failed_in_suite0,Reason}};
@@ -212,12 +212,14 @@ init_tc2(Mod,Suite,Func,SuiteInfo,MergeResult,Config) ->
 	    {auto_skip,{require_failed,Reason}};
 	{'EXIT',Reason} ->
 	    {auto_skip,Reason};
-	{ok,Config1} ->
+	{ok,PostInitHook,Config1} ->
 	    case get('$test_server_framework_test') of
 		undefined ->
-		    ct_suite_init(Suite, FuncSpec, Config1);
+		    ct_suite_init(Suite, FuncSpec, PostInitHook, Config1);
 		Fun ->
-		    case Fun(init_tc, Config1) of
+		    PostInitHookResult = do_post_init_hook(PostInitHook,
+							   Config1),
+		    case Fun(init_tc, [PostInitHookResult ++ Config1]) of
 			NewConfig when is_list(NewConfig) ->
 			    {ok,NewConfig};
 			Else ->
@@ -226,14 +228,28 @@ init_tc2(Mod,Suite,Func,SuiteInfo,MergeResult,Config) ->
 	    end
     end.
 
-ct_suite_init(Suite, Func, [Config]) when is_list(Config) ->
+ct_suite_init(Suite, Func, PostInitHook, Config) when is_list(Config) ->
     case ct_hooks:init_tc(Suite, Func, Config) of
 	NewConfig when is_list(NewConfig) ->
-	    {ok, [NewConfig]};
+	    PostInitHookResult = do_post_init_hook(PostInitHook, NewConfig),
+	    {ok, [PostInitHookResult ++ NewConfig]};
 	Else ->
 	    Else
     end.
 
+do_post_init_hook(PostInitHook, Config) ->
+    lists:flatmap(fun({Tag,Fun}) ->
+			  case lists:keysearch(Tag,1,Config) of
+			      {value,_} ->
+				  [];
+			      false -> 
+				  case Fun() of
+				      {error,_} -> [];
+				      Result    -> [{Tag,Result}]
+				  end
+			  end
+		  end, PostInitHook).
+
 add_defaults(Mod,Func, GroupPath) ->
     Suite = get_suite_name(Mod, GroupPath),
     case (catch Suite:suite()) of
@@ -453,15 +469,16 @@ timetrap_first([],Info,[]) ->
 timetrap_first([],Info,Found) ->
     ?rev(Found) ++ ?rev(Info).
 
-configure([{require,Required}|Rest],Info,SuiteInfo,Scope,Config) ->
+configure([{require,Required}|Rest],
+	  Info,SuiteInfo,Scope,PostInitHook,Config) ->
     case ct:require(Required) of
 	ok ->
-	    configure(Rest,Info,SuiteInfo,Scope,Config);
+	    configure(Rest,Info,SuiteInfo,Scope,PostInitHook,Config);
 	Error = {error,Reason} ->
 	    case required_default('_UNDEF',Required,Info,
 				  SuiteInfo,Scope) of
 		ok ->
-		    configure(Rest,Info,SuiteInfo,Scope,Config);
+		    configure(Rest,Info,SuiteInfo,Scope,PostInitHook,Config);
 		_ ->
 		    case lists:keymember(Required,2,SuiteInfo) of
 			true ->
@@ -471,14 +488,15 @@ configure([{require,Required}|Rest],Info,SuiteInfo,Scope,Config) ->
 		    end
 	    end
     end;
-configure([{require,Name,Required}|Rest],Info,SuiteInfo,Scope,Config) ->
+configure([{require,Name,Required}|Rest],
+	  Info,SuiteInfo,Scope,PostInitHook,Config) ->
     case ct:require(Name,Required) of
 	ok ->
-	    configure(Rest,Info,SuiteInfo,Scope,Config);
+	    configure(Rest,Info,SuiteInfo,Scope,PostInitHook,Config);
 	Error = {error,Reason} ->
 	    case required_default(Name,Required,Info,SuiteInfo,Scope) of
 		ok ->
-		    configure(Rest,Info,SuiteInfo,Scope,Config);
+		    configure(Rest,Info,SuiteInfo,Scope,PostInitHook,Config);
 		_ ->
 		    case lists:keymember(Name,2,SuiteInfo) of
 			true -> 
@@ -488,17 +506,24 @@ configure([{require,Name,Required}|Rest],Info,SuiteInfo,Scope,Config) ->
 		    end
 	    end
     end;
-configure([{timetrap,off}|Rest],Info,SuiteInfo,Scope,Config) ->
-    configure(Rest,Info,SuiteInfo,Scope,Config);
-configure([{timetrap,Time}|Rest],Info,SuiteInfo,Scope,Config) ->
-    Dog = test_server:timetrap(Time),
-    configure(Rest,Info,SuiteInfo,Scope,[{watchdog,Dog}|Config]);
-configure([{ct_hooks, Hook} | Rest], Info, SuiteInfo, Scope, Config) ->
-    configure(Rest, Info, SuiteInfo, Scope, [{ct_hooks, Hook} | Config]);
-configure([_|Rest],Info,SuiteInfo,Scope,Config) ->
-    configure(Rest,Info,SuiteInfo,Scope,Config);
-configure([],_,_,_,Config) ->
-    {ok,[Config]}.
+configure([{timetrap,off}|Rest],Info,SuiteInfo,Scope,PostInitHook,Config) ->
+    configure(Rest,Info,SuiteInfo,Scope,PostInitHook,Config);
+configure([{timetrap,Time}|Rest],Info,SuiteInfo,Scope,PostInitHook,Config) ->
+    PostInitHook1 = 
+	[{watchdog,fun() -> case test_server:get_timetrap_info() of
+				undefined ->
+				    test_server:timetrap(Time);
+				_ ->
+				    {error,already_set}
+			    end
+		   end} | PostInitHook],
+    configure(Rest,Info,SuiteInfo,Scope,PostInitHook1,Config);
+configure([{ct_hooks,Hook}|Rest],Info,SuiteInfo,Scope,PostInitHook,Config) ->
+    configure(Rest,Info,SuiteInfo,Scope,PostInitHook,[{ct_hooks,Hook}|Config]);
+configure([_|Rest],Info,SuiteInfo,Scope,PostInitHook,Config) ->
+    configure(Rest,Info,SuiteInfo,Scope,PostInitHook,Config);
+configure([],_,_,_,PostInitHook,Config) ->
+    {ok,PostInitHook,Config}.
 
 %% the require element in Info may come from suite/0 and
 %% should be scoped 'suite', or come from the group info
@@ -562,10 +587,8 @@ end_tc(Mod,Func,TCPid,Result,Args,Return) ->
     %% in case Mod == ct_framework, lookup the suite name
     Suite = get_suite_name(Mod, Args),
 
-    case lists:keysearch(watchdog,1,Args) of
-	{value,{watchdog,Dog}} -> test_server:timetrap_cancel(Dog);
-	false -> ok
-    end,
+    test_server:timetrap_cancel(),
+
     %% save the testcase process pid so that it can be used
     %% to look up the attached trace window later
     case ct_util:get_testdata(interpret) of
diff --git a/lib/common_test/src/ct_hooks.erl b/lib/common_test/src/ct_hooks.erl
index 0fe6e03079..98b74665de 100644
--- a/lib/common_test/src/ct_hooks.erl
+++ b/lib/common_test/src/ct_hooks.erl
@@ -353,11 +353,10 @@ pos(Id,[_|Rest],Num) ->
     pos(Id,Rest,Num+1).
 
 
-
 catch_apply(M,F,A, Default) ->
     try
 	apply(M,F,A)
-    catch error:Reason ->
+    catch _:Reason ->
 	    case erlang:get_stacktrace() of
             %% Return the default if it was the CTH module which did not have the function.
 		[{M,F,A,_}|_] when Reason == undef ->
diff --git a/lib/common_test/src/ct_logs.erl b/lib/common_test/src/ct_logs.erl
index 1ccbdc3718..8359dcee98 100644
--- a/lib/common_test/src/ct_logs.erl
+++ b/lib/common_test/src/ct_logs.erl
@@ -659,13 +659,23 @@ create_io_fun(FromPid, State) ->
 
 print_to_log(sync, FromPid, TCGL, List, State) ->
     IoFun = create_io_fun(FromPid, State),
-    io:format(TCGL, "~s", [lists:foldl(IoFun, [], List)]),
+    %% in some situations (exceptions), the printout is made from the
+    %% test server IO process and there's no valid group leader to send to
+    IoProc = if FromPid /= TCGL -> TCGL;
+		true -> State#logger_state.ct_log_fd
+	     end,
+    io:format(IoProc, "~s", [lists:foldl(IoFun, [], List)]),
     State;
 
 print_to_log(async, FromPid, TCGL, List, State) ->
     IoFun = create_io_fun(FromPid, State),
+    %% in some situations (exceptions), the printout is made from the
+    %% test server IO process and there's no valid group leader to send to
+    IoProc = if FromPid /= TCGL -> TCGL;
+		true -> State#logger_state.ct_log_fd
+	     end,
     Printer = fun() ->
-		      io:format(TCGL, "~s", [lists:foldl(IoFun, [], List)])
+		      io:format(IoProc, "~s", [lists:foldl(IoFun, [], List)])
 	      end,
     case State#logger_state.async_print_jobs of
 	[] ->
diff --git a/lib/common_test/src/cth_surefire.erl b/lib/common_test/src/cth_surefire.erl
index d04a8b07db..e7bd84e51b 100644
--- a/lib/common_test/src/cth_surefire.erl
+++ b/lib/common_test/src/cth_surefire.erl
@@ -82,21 +82,26 @@ pre_init_per_testcase(_TC,Config,State) -> {Config, init_tc(State, Config)}.
 post_end_per_testcase(TC,Config,Result,State) ->
     {Result, end_tc(TC,Config, Result,State)}.
 
+on_tc_fail(_TC, _Res, State = #state{test_cases = []}) ->
+    State;
 on_tc_fail(_TC, Res, State) ->
     TCs = State#state.test_cases,
-    TC = hd(State#state.test_cases),
-    NewTC = TC#testcase{ failure =
-			     {fail,lists:flatten(io_lib:format("~p",[Res]))} },
+    TC = hd(TCs),
+    NewTC = TC#testcase{
+	      failure =
+		  {fail,lists:flatten(io_lib:format("~p",[Res]))} },
     State#state{ test_cases = [NewTC | tl(TCs)]}.
 
 on_tc_skip(Tc,{Type,Reason} = Res, State) when Type == tc_auto_skip ->
     do_tc_skip(Res, end_tc(Tc,[],Res,init_tc(State,[])));
+on_tc_skip(_Tc, _Res, State = #state{test_cases = []}) ->
+    State;
 on_tc_skip(_Tc, Res, State) ->
     do_tc_skip(Res, State).
 
 do_tc_skip(Res, State) ->
     TCs = State#state.test_cases,
-    TC = hd(State#state.test_cases),
+    TC = hd(TCs),
     NewTC = TC#testcase{
 	      failure =
 		  {skipped,lists:flatten(io_lib:format("~p",[Res]))} },