[test_server] Cancel timetrap for break in end_per_testcase

Any call to test_server:break/1 should cancel all active
timetramps. In some cases, Suite:end_per_testcase/2 is executed on a
different process than the test case itself, and if
test_server:break/1 would be called from there, the timetraps would
not be cancelled. This has been corrected.

4 files changed, 170 insertions, 12 deletions

diff --git a/lib/test_server/src/test_server.erl b/lib/test_server/src/test_server.erl
index 1d3394e0ca..14cdfd391a 100644
--- a/lib/test_server/src/test_server.erl
+++ b/lib/test_server/src/test_server.erl
@@ -724,9 +724,11 @@ call_end_conf(Mod,Func,TCPid,TCExitReason,Loc,Conf,TVal) ->
     Data = {Mod,Func,TCPid,TCExitReason,Loc},
     EndConfProc =
 	fun() ->
+		process_flag(trap_exit,true), % to catch timetraps
 		Supervisor = self(),
 		EndConfApply =
 		    fun() ->
+			    timetrap(TVal),
 			    case catch apply(Mod,end_per_testcase,[Func,Conf]) of
 				{'EXIT',Why} ->
 				    timer:sleep(1),
@@ -745,14 +747,14 @@ call_end_conf(Mod,Func,TCPid,TCExitReason,Loc,Conf,TVal) ->
 		    {Pid,end_conf} ->
 			Starter ! {self(),{call_end_conf,Data,ok}};
 		    {'EXIT',Pid,Reason} ->
-			Starter ! {self(),{call_end_conf,Data,{error,Reason}}}
-		after TVal ->
-			exit(Pid, kill),
 			group_leader() ! {printout,12,
 					  "WARNING! ~p:end_per_testcase(~p, ~p)"
-					  " failed!\n\tReason: timetrap timeout"
-					  " after ~w ms!\n", [Mod,Func,Conf,TVal]},
-			Starter ! {self(),{call_end_conf,Data,{error,timeout}}}
+					  " failed!\n\tReason: ~p\n",
+					  [Mod,Func,Conf,Reason]},
+			Starter ! {self(),{call_end_conf,Data,{error,Reason}}};
+		    {'EXIT',_OtherPid,Reason} ->
+			%% Probably the parent - not much to do about that
+			exit(Reason)
 		end
 	end,
     spawn_link(EndConfProc).
diff --git a/lib/test_server/test/test_server_SUITE.erl b/lib/test_server/test/test_server_SUITE.erl
index d20528d43b..95a3423fef 100644
--- a/lib/test_server/test/test_server_SUITE.erl
+++ b/lib/test_server/test/test_server_SUITE.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2011. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2012. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -79,7 +79,8 @@ groups() ->
 all() ->
     [test_server_SUITE, test_server_parallel01_SUITE,
      test_server_conf02_SUITE, test_server_conf01_SUITE,
-     test_server_skip_SUITE, test_server_shuffle01_SUITE].
+     test_server_skip_SUITE, test_server_shuffle01_SUITE,
+     test_server_break_SUITE].
 
 
 %%--------------------------------------------------------------------
@@ -115,6 +116,10 @@ test_server_conf02_SUITE(Config) ->
     run_test_server_tests("test_server_conf02_SUITE", 26, 0, 12, 
 			  12, 0, 0, 0, 0, 26, Config).
 
+test_server_break_SUITE(Config) ->
+    D = run_test_server_tests("test_server_break_SUITE", 8, 2, 6,
+			  4, 0, 0, 0, 2, 6, Config),
+    D.
 
 run_test_server_tests(SuiteName, NCases, NFail, NExpected, NSucc, 
 		      NUsrSkip, NAutoSkip, 
@@ -139,9 +144,11 @@ run_test_server_tests(SuiteName, NCases, NFail, NExpected, NSucc,
     rpc:call(Node,test_server_ctrl, stop, []),
 
     {ok,Data} =	test_server_test_lib:parse_suite(
-		  hd(filelib:wildcard(
-		       filename:join([WorkDir,SuiteName++".logs",
-				      "run*","suite.log"])))),
+		  lists:last(
+		    lists:sort(
+		      filelib:wildcard(
+			filename:join([WorkDir,SuiteName++".logs",
+				       "run*","suite.log"]))))),
     check([{"Number of cases",NCases,Data#suite.n_cases},
 	   {"Number failed",NFail,Data#suite.n_cases_failed},
 	   {"Number expected",NExpected,Data#suite.n_cases_expected},
diff --git a/lib/test_server/test/test_server_SUITE_data/Makefile.src b/lib/test_server/test/test_server_SUITE_data/Makefile.src
index 332b855df6..ec8ddd78b0 100644
--- a/lib/test_server/test/test_server_SUITE_data/Makefile.src
+++ b/lib/test_server/test/test_server_SUITE_data/Makefile.src
@@ -4,4 +4,5 @@ all:
 	erlc test_server_conf01_SUITE.erl
 	erlc test_server_shuffle01_SUITE.erl
 	erlc test_server_conf02_SUITE.erl
-	erlc test_server_skip_SUITE.erl
\ No newline at end of file
+	erlc test_server_skip_SUITE.erl
+	erlc test_server_break_SUITE.erl
\ No newline at end of file
diff --git a/lib/test_server/test/test_server_SUITE_data/test_server_break_SUITE.erl b/lib/test_server/test/test_server_SUITE_data/test_server_break_SUITE.erl
new file mode 100644
index 0000000000..70e30a3334
--- /dev/null
+++ b/lib/test_server/test/test_server_SUITE_data/test_server_break_SUITE.erl
@@ -0,0 +1,148 @@
+%%
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 2012. All Rights Reserved.
+%%
+%% The contents of this file are subject to the Erlang Public License,
+%% Version 1.1, (the "License"); you may not use this file except in
+%% compliance with the License. You should have received a copy of the
+%% Erlang Public License along with this software. If not, it can be
+%% retrieved online at http://www.erlang.org/.
+%%
+%% Software distributed under the License is distributed on an "AS IS"
+%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
+%% the License for the specific language governing rights and limitations
+%% under the License.
+%%
+%% %CopyrightEnd%
+%%
+-module(test_server_break_SUITE).
+
+-export([all/1, init_per_suite/1, end_per_suite/1]).
+-export([init_per_testcase/2, end_per_testcase/2]).
+-export([break_in_init_tc/1,
+	 break_in_tc/1,
+	 break_in_end_tc/1,
+	 break_in_end_tc_after_fail/1,
+	 break_in_end_tc_after_abort/1,
+	 check_all_breaks/1]).
+
+-include_lib("test_server/include/test_server.hrl").
+
+all(suite) ->
+    [break_in_init_tc,
+     break_in_tc,
+     break_in_end_tc,
+     break_in_end_tc_after_fail,
+     break_in_end_tc_after_abort,
+     check_all_breaks]. %must be the last test - checks result of previous tests
+
+init_per_suite(Config) ->
+    spawn(fun break_and_continue_sup/0),
+    Config.
+
+end_per_suite(Config) ->
+    ok.
+
+init_per_testcase(Case,Config) when Case==break_in_init_tc ->
+    Config1 = init_timetrap(500,Config),
+    break_and_check(Case),
+    Config1;
+init_per_testcase(Case,Config) when Case==check_all_breaks ->
+    init_timetrap({seconds,20},Config);
+init_per_testcase(_Case,Config) ->
+    init_timetrap(500,Config).
+
+init_timetrap(T,Config) ->
+    Dog = ?t:timetrap(T),
+    [{watchdog, Dog}|Config].
+
+end_per_testcase(Case,Config) when Case==break_in_end_tc;
+				   Case==break_in_end_tc_after_fail;
+				   Case==break_in_end_tc_after_abort ->
+    break_and_check(Case),
+    cancel_timetrap(Config);
+end_per_testcase(_Case,Config) ->
+    cancel_timetrap(Config).
+
+cancel_timetrap(Config) ->
+    Dog=?config(watchdog, Config),
+    ?t:timetrap_cancel(Dog),
+    ok.
+
+
+%%%-----------------------------------------------------------------
+%%% Test cases
+
+break_in_init_tc(Config) when is_list(Config) ->
+    ok.
+
+break_in_tc(Config) when is_list(Config) ->
+    break_and_check(break_in_tc),
+    ok.
+
+break_in_end_tc(Config) when is_list(Config) ->
+    ok.
+
+break_in_end_tc_after_fail(Config) when is_list(Config) ->
+    ?t:fail(test_case_should_fail).
+
+break_in_end_tc_after_abort(Config) when is_list(Config) ->
+    ?t:adjusted_sleep(2000). % will cause a timetrap timeout
+
+%%%-----------------------------------------------------------------
+%%% Internal functions
+
+%% This test case checks that all breaks in previous test cases was
+%% also continued, and that the break lasted as long as expected.
+%% The reason for this is that some of the breaks above are in
+%% end_per_testcase, and failures there will only produce a warning,
+%% not an error - so this is to catch the error for real.
+check_all_breaks(Config) ->
+    break_and_continue_sup ! {done,self()},
+    receive {Breaks,Continued} ->
+	    check_all_breaks(Breaks,Continued)
+    end.
+
+check_all_breaks([{From,Case,T,Start}|Breaks],[{From,End}|Continued]) ->
+    Diff = timer:now_diff(End,Start),
+    DiffSec = round(Diff/1000000),
+    TSec = round(T/1000000),
+    if DiffSec==TSec ->
+	    ?t:format("Break in ~p successfully continued after ~p second(s)~n",
+		      [Case,DiffSec]),
+	    check_all_breaks(Breaks,Continued);
+       true ->
+	    ?t:format("Faulty duration of break in ~p: continued after ~p second(s)~n",
+		      [Case,DiffSec]),
+	    ?t:fail({faulty_diff,Case,DiffSec,TSec})
+    end;
+check_all_breaks([],[]) ->
+    ok;
+check_all_breaks(Breaks,Continued) ->
+    %% This is probably a case of a missing continue - i.e. a break
+    %% has been started, but it was never continued.
+    ?t:fail({no_match_in_breaks_and_continued,Breaks,Continued}).
+
+break_and_check(Case) ->
+    break_and_continue_sup ! {break,Case,1000,self()},
+    ?t:break(atom_to_list(Case)),
+    break_and_continue_sup ! {continued,self()},
+    ok.
+
+break_and_continue_sup() ->
+    register(break_and_continue_sup,self()),
+    break_and_continue_loop([],[]).
+
+break_and_continue_loop(Breaks,Continued) ->
+    receive
+	{break,Case,T,From} ->
+	    Start = now(),
+	    {RealT,_} = timer:tc(?t,adjusted_sleep,[T]),
+	    ?t:continue(),
+	    break_and_continue_loop([{From,Case,RealT,Start}|Breaks],Continued);
+	{continued,From} ->
+	    break_and_continue_loop(Breaks,[{From,now()}|Continued]);
+	{done,From} ->
+	    From ! {lists:reverse(Breaks),lists:reverse(Continued)}
+    end.