Merge branch 'peppe/common_test/timetrap_line.maint' into maint-17

* peppe/common_test/timetrap_line.maint:
  Fix problem with line number not always showing in log

3 files changed, 110 insertions, 48 deletions

diff --git a/lib/test_server/src/erl2html2.erl b/lib/test_server/src/erl2html2.erl
index 7cfaa2c325..50dbbb82ee 100644
--- a/lib/test_server/src/erl2html2.erl
+++ b/lib/test_server/src/erl2html2.erl
@@ -117,9 +117,10 @@ parse_preprocessed_file(Epp,File,InCorrectFile) ->
 		    parse_preprocessed_file(Epp,File,true);
 		{attribute,_,file,{_OtherFile,_}} ->
 		    parse_preprocessed_file(Epp,File,false);
-		{function,L,F,A,[_|C]} when InCorrectFile ->
-		    Clauses = [{clause,CL} || {clause,CL,_,_,_} <- C],
-		    [{atom_to_list(F),A,L} | Clauses] ++
+		{function,L,F,A,Cs} when InCorrectFile ->
+		    {CLs,LastCL} = find_clause_lines(Cs, []),
+		    %% tl(CLs) cause we know the start line already
+		    [{atom_to_list(F),A,L,LastCL} | tl(CLs)] ++
 			parse_preprocessed_file(Epp,File,true);
 		_ ->
 		    parse_preprocessed_file(Epp,File,InCorrectFile)
@@ -146,9 +147,10 @@ parse_non_preprocessed_file(Epp, File, Location) ->
     case epp_dodger:parse_form(Epp, Location) of
 	{ok,Tree,Location1} ->
 	    try erl_syntax:revert(Tree) of
-		{function,L,F,A,[_|C]} ->
-		    Clauses = [{clause,CL} || {clause,CL,_,_,_} <- C],
-		    [{atom_to_list(F),A,L} | Clauses] ++
+		{function,L,F,A,Cs} ->
+		    {CLs,LastCL} = find_clause_lines(Cs, []),
+		    %% tl(CLs) cause we know the start line already
+		    [{atom_to_list(F),A,L,LastCL} | tl(CLs)] ++
 			parse_non_preprocessed_file(Epp, File, Location1);
 		_ ->
 		    parse_non_preprocessed_file(Epp, File, Location1)
@@ -162,22 +164,48 @@ parse_non_preprocessed_file(Epp, File, Location) ->
     end.
 
 %%%-----------------------------------------------------------------
+%%% Find the line number of the last expression in the function
+find_clause_lines([{clause,CL,_Params,_Op,Exprs}], CLs) -> % last clause
+    try tuple_to_list(lists:last(Exprs)) of
+	[_Type,ExprLine | _] ->
+	    {lists:reverse([{clause,CL}|CLs]), ExprLine};
+	_ ->
+	    {lists:reverse([{clause,CL}|CLs]), CL}
+    catch
+	_:_ ->
+	    {lists:reverse([{clause,CL}|CLs]), CL}
+    end;
+
+find_clause_lines([{clause,CL,_Params,_Op,_Exprs} | Cs], CLs) ->
+    find_clause_lines(Cs, [{clause,CL}|CLs]).
+
+%%%-----------------------------------------------------------------
 %%% Add a link target for each line and one for each function definition.
-build_html(SFd,DFd,Encoding,Functions) ->
-    build_html(SFd,DFd,Encoding,file:read_line(SFd),1,Functions,false).
+build_html(SFd,DFd,Encoding,FuncsAndCs) ->
+    build_html(SFd,DFd,Encoding,file:read_line(SFd),1,FuncsAndCs,
+	       false,undefined).
 
-build_html(SFd,DFd,Encoding,{ok,Str},L,[{F,A,L}|Functions],_IsFuncDef) ->
+%% function start line found
+build_html(SFd,DFd,Enc,{ok,Str},L0,[{F,A,L0,LastL}|FuncsAndCs],
+	   _IsFuncDef,_FAndLastL) ->
     FALink = test_server_ctrl:uri_encode(F++"-"++integer_to_list(A),utf8),
-    file:write(DFd,["<a name=\"",to_raw_list(FALink,Encoding),"\"/>"]),
-    build_html(SFd,DFd,Encoding,{ok,Str},L,Functions,true);
-build_html(SFd,DFd,Encoding,{ok,Str},L,[{clause,L}|Functions],_IsFuncDef) ->
-    build_html(SFd,DFd,Encoding,{ok,Str},L,Functions,true);
-build_html(SFd,DFd,Encoding,{ok,Str},L,Functions,IsFuncDef) ->
+    file:write(DFd,["<a name=\"",to_raw_list(FALink,Enc),"\"/>"]),
+    build_html(SFd,DFd,Enc,{ok,Str},L0,FuncsAndCs,true,{F,LastL});
+%% line of last expression in function found
+build_html(SFd,DFd,Enc,{ok,Str},LastL,FuncsAndCs,_IsFuncDef,{F,LastL}) ->
+    LastLineLink = test_server_ctrl:uri_encode(F++"-last_expr",utf8),
+	    file:write(DFd,["<a name=\"",
+			    to_raw_list(LastLineLink,Enc),"\"/>"]),
+    build_html(SFd,DFd,Enc,{ok,Str},LastL,FuncsAndCs,true,undefined);
+build_html(SFd,DFd,Enc,{ok,Str},L,[{clause,L}|FuncsAndCs],
+	   _IsFuncDef,FAndLastL) ->
+    build_html(SFd,DFd,Enc,{ok,Str},L,FuncsAndCs,true,FAndLastL);
+build_html(SFd,DFd,Enc,{ok,Str},L,FuncsAndCs,IsFuncDef,FAndLastL) ->
     LStr = line_number(L),
     Str1 = line(Str,IsFuncDef),
     file:write(DFd,[LStr,Str1]),
-    build_html(SFd,DFd,Encoding,file:read_line(SFd),L+1,Functions,false);
-build_html(_SFd,_DFd,_Encoding,eof,L,_Functions,_IsFuncDef) ->
+    build_html(SFd,DFd,Enc,file:read_line(SFd),L+1,FuncsAndCs,false,FAndLastL);
+build_html(_SFd,_DFd,_Enc,eof,L,_FuncsAndCs,_IsFuncDef,_FAndLastL) ->
     L.
 
 line_number(L) ->
diff --git a/lib/test_server/src/test_server.erl b/lib/test_server/src/test_server.erl
index 8d91778cbb..1c3352550b 100644
--- a/lib/test_server/src/test_server.erl
+++ b/lib/test_server/src/test_server.erl
@@ -1355,12 +1355,30 @@ get_loc(Pid) ->
     Stk = [rewrite_loc_item(Loc) || Loc <- Stk0],
     case get(test_server_loc) of
 	[{Suite,Case}] ->
-	    %% location info unknown, check if {Suite,Case,Line}
-	    %% is available in stacktrace. and if so, use stacktrace
-	    %% instead of current test_server_loc
+	    %% Location info unknown, check if {Suite,Case,Line}
+	    %% is available in stacktrace and if so, use stacktrace
+	    %% instead of current test_server_loc.
+	    %% If location is the last expression in a test case
+	    %% function, the info is not available due to tail call
+	    %% elimination. We need to check if the test case has been
+	    %% called by ts_tc/3 and, if so, insert the test case info
+	    %% at that position.
 	    case [match || {S,C,_L} <- Stk, S == Suite, C == Case] of
-		[match|_] -> put(test_server_loc, Stk);
-		_         -> ok
+		[match|_] ->
+		    put(test_server_loc, Stk);
+		_ ->
+		    {PreTC,PostTC} =
+			lists:splitwith(fun({test_server,ts_tc,_}) ->
+						false;
+					   (_) ->
+						true
+					end, Stk),
+		    if PostTC == [] ->
+			    ok;
+		       true ->
+			    put(test_server_loc,
+				PreTC++[{Suite,Case,last_expr} | PostTC])
+		    end
 	    end;
 	_ ->
 	    put(test_server_loc, Stk)
@@ -1422,7 +1440,10 @@ lookup_config(Key,Config) ->
 	    undefined
     end.
 
-%% timer:tc/3
+%%
+%% IMPORTANT: get_loc/1 uses the name of this function when analysing
+%% stack traces. If the name changes, get_loc/1 must be updated!
+%%
 ts_tc(M, F, A) ->
     Before = erlang:now(),
     Result = try
diff --git a/lib/test_server/src/test_server_sup.erl b/lib/test_server/src/test_server_sup.erl
index 96e369a138..15a6fdd1de 100644
--- a/lib/test_server/src/test_server_sup.erl
+++ b/lib/test_server/src/test_server_sup.erl
@@ -61,33 +61,37 @@ timetrap(Timeout0, ReportTVal, Scale, Pid) ->
     TruncTO = trunc(Timeout),
     receive
     after TruncTO ->
-	    case is_process_alive(Pid) of
-		true ->
-		    TimeToReport = if Timeout0 == ReportTVal -> TruncTO;
-				      true -> ReportTVal end,
-		    MFLs = test_server:get_loc(Pid),
-		    Mon = erlang:monitor(process, Pid),
-		    Trap = {timetrap_timeout,TimeToReport,MFLs},
-		    exit(Pid, Trap),
-		    receive
-			{'DOWN', Mon, process, Pid, _} ->
-			    ok
-		    after 10000 ->
-			    %% Pid is probably trapping exits, hit it harder...
-			    catch error_logger:warning_msg(
-				    "Testcase process ~w not "
-				    "responding to timetrap "
-				    "timeout:~n"
-				    "  ~p.~n"
-				    "Killing testcase...~n",
-				    [Pid, Trap]),
-			    exit(Pid, kill)
-		    end;
-		false ->
+	    kill_the_process(Pid, Timeout0, TruncTO, ReportTVal)
+    end.
+
+kill_the_process(Pid, Timeout0, TruncTO, ReportTVal) ->
+    case is_process_alive(Pid) of
+	true ->
+	    TimeToReport = if Timeout0 == ReportTVal -> TruncTO;
+			      true -> ReportTVal end,
+	    MFLs = test_server:get_loc(Pid),
+	    Mon = erlang:monitor(process, Pid),
+	    Trap = {timetrap_timeout,TimeToReport,MFLs},
+	    exit(Pid, Trap),
+	    receive
+		{'DOWN', Mon, process, Pid, _} ->
 		    ok
-	    end
+	    after 10000 ->
+		    %% Pid is probably trapping exits, hit it harder...
+		    catch error_logger:warning_msg(
+			    "Testcase process ~w not "
+			    "responding to timetrap "
+			    "timeout:~n"
+			    "  ~p.~n"
+			    "Killing testcase...~n",
+			    [Pid, Trap]),
+		    exit(Pid, kill)
+	    end;
+	false ->
+	    ok
     end.
 
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% timetrap_cancel(Handle) -> ok
 %% Handle = term()
@@ -812,10 +816,19 @@ format_loc1({Mod,Func,Line}) ->
     case {lists:member(no_src, get(test_server_logopts)),
 	  lists:reverse(ModStr)} of
 	{false,[$E,$T,$I,$U,$S,$_|_]}  ->
-	    io_lib:format("{~w,~w,<a href=\"~ts~ts#~w\">~w</a>}",
+	    Link = if is_integer(Line) ->
+			   integer_to_list(Line);
+		      Line == last_expr ->
+			   list_to_atom(atom_to_list(Func)++"-last_expr");
+		      is_atom(Line) ->
+			   atom_to_list(Line);
+		      true ->
+			   Line
+		   end,
+	    io_lib:format("{~w,~w,<a href=\"~ts~ts#~s\">~w</a>}",
 			  [Mod,Func,
 			   test_server_ctrl:uri_encode(downcase(ModStr)),
-			   ?src_listing_ext,Line,Line]);
+			   ?src_listing_ext,Link,Line]);
 	_ ->
 	    io_lib:format("{~w,~w,~w}",[Mod,Func,Line])
     end.