Merge branch 'peppe/common_test/report_error_if_missing_suite'

* peppe/common_test/report_error_if_missing_suite:
  Fix problem with suite compilation failures not being correctly reported

OTP-10816

3 files changed, 64 insertions, 61 deletions

diff --git a/lib/common_test/src/ct_framework.erl b/lib/common_test/src/ct_framework.erl
index 498950c9d1..ea3d7c8218 100644
--- a/lib/common_test/src/ct_framework.erl
+++ b/lib/common_test/src/ct_framework.erl
@@ -873,8 +873,8 @@ error_notification(Mod,Func,_Args,{Error,Loc}) ->
     end,
 
     PrintErr = fun(ErrFormat, ErrArgs) ->
-		       Div = "~n- - - - - - - - - - - - - - - - "
-			   "- - - - - - - - - -~n",
+		       Div = "~n- - - - - - - - - - - - - - - - - - - "
+			     "- - - - - - - - - - - - - - - - - - - - -~n",
 		       io:format(user, lists:concat([Div,ErrFormat,Div,"~n"]),
 				 ErrArgs),
 		       Link =
@@ -1063,8 +1063,14 @@ get_all_cases1(_, []) ->
 get_all(Mod, ConfTests) ->	
     case catch apply(Mod, all, []) of
 	{'EXIT',_} ->
-	    Reason = 
-		list_to_atom(atom_to_list(Mod)++":all/0 is missing"),
+	    Reason =
+		case code:which(Mod) of
+		    non_existing ->
+			list_to_atom(atom_to_list(Mod)++
+				     " can not be compiled or loaded");
+		    _ ->
+			list_to_atom(atom_to_list(Mod)++":all/0 is missing")
+		end,
 	    %% this makes test_server call error_in_suite as first
 	    %% (and only) test case so we can report Reason properly
 	    [{?MODULE,error_in_suite,[[{error,Reason}]]}];
diff --git a/lib/common_test/src/ct_run.erl b/lib/common_test/src/ct_run.erl
index 4a12481214..4d74fd6a80 100644
--- a/lib/common_test/src/ct_run.erl
+++ b/lib/common_test/src/ct_run.erl
@@ -1969,22 +1969,7 @@ final_tests(Tests, Skip, Bad) ->
 
 final_tests1([{TestDir,Suites,_}|Tests], Final, Skip, Bad) when
       is_list(Suites), is_atom(hd(Suites)) ->
-%     Separate =
-% 	fun(S,{DoSuite,Dont}) ->		
-% 		case lists:keymember({TestDir,S},1,Bad) of
-% 		    false ->	
-% 			{[S|DoSuite],Dont};
-% 		    true ->	
-% 			SkipIt = {TestDir,S,"Make failed"},
-% 			{DoSuite,Dont++[SkipIt]}
-% 		end
-% 	end,
-	
-%     {DoSuites,Skip1} =
-% 	lists:foldl(Separate,{[],Skip},Suites),
-%     Do = {TestDir,lists:reverse(DoSuites),all},
-
-    Skip1 = [{TD,S,"Make failed"} || {{TD,S},_} <- Bad, S1 <- Suites,
+    Skip1 = [{TD,S,make_failed} || {{TD,S},_} <- Bad, S1 <- Suites,
 				     S == S1, TD == TestDir],
     Final1 = [{TestDir,S,all} || S <- Suites],
     final_tests1(Tests, lists:reverse(Final1)++Final, Skip++Skip1, Bad);
@@ -1997,7 +1982,7 @@ final_tests1([{TestDir,all,all}|Tests], Final, Skip, Bad) ->
 	    false ->
 		[]
 	end,
-    Missing = [{TestDir,S,"Make failed"} || S <- MissingSuites],
+    Missing = [{TestDir,S,make_failed} || S <- MissingSuites],
     Final1 = [{TestDir,all,all}|Final],
     final_tests1(Tests, Final1, Skip++Missing, Bad);
 
@@ -2009,7 +1994,7 @@ final_tests1([{TestDir,Suite,GrsOrCs}|Tests], Final, Skip, Bad) when
       is_list(GrsOrCs) ->
     case lists:keymember({TestDir,Suite}, 1, Bad) of
 	true ->
-	    Skip1 = Skip ++ [{TestDir,Suite,all,"Make failed"}],
+	    Skip1 = Skip ++ [{TestDir,Suite,all,make_failed}],
 	    final_tests1(Tests, [{TestDir,Suite,all}|Final], Skip1, Bad);
 	false ->
 	    GrsOrCs1 =
diff --git a/lib/test_server/src/test_server_ctrl.erl b/lib/test_server/src/test_server_ctrl.erl
index 349b033c89..bef0658b6d 100644
--- a/lib/test_server/src/test_server_ctrl.erl
+++ b/lib/test_server/src/test_server_ctrl.erl
@@ -1807,26 +1807,31 @@ start_minor_log_file1(Mod, Func, LogDir, AbsName, MFA) ->
     io:put_chars(Fd, "<pre>\n"),
 
     SrcListing = downcase(atom_to_list(Mod)) ++ ?src_listing_ext,
-    
-    {Info,Arity} =
-	if Func == init_per_suite; Func == end_per_suite ->
-		{"Config function: ", 1};
-	   Func == init_per_group; Func == end_per_group ->
-		{"Config function: ", 2};
-	   true ->
-		{"Test case: ", 1}
-	end,
 
-    case {filelib:is_file(filename:join(LogDir, SrcListing)),
-	  lists:member(no_src, get(test_server_logopts))} of
-	{true,false} ->
-	    print(Lev, Info ++ "<a href=\"~ts#~ts\">~w:~w/~w</a> "
-		  "(click for source code)\n",
-		  [uri_encode(SrcListing),
-		   uri_encode(atom_to_list(Func)++"-1",utf8),
-		   Mod,Func,Arity]);
+    case get_fw_mod(?MODULE) of
+	Mod when Func == error_in_suite ->
+	    ok;
 	_ ->
-	    print(Lev, Info ++ "~w:~w/~w\n", [Mod,Func,Arity])
+	    {Info,Arity} =
+		if Func == init_per_suite; Func == end_per_suite ->
+			{"Config function: ", 1};
+		   Func == init_per_group; Func == end_per_group ->
+			{"Config function: ", 2};
+		   true ->
+			{"Test case: ", 1}
+		end,
+	    
+	    case {filelib:is_file(filename:join(LogDir, SrcListing)),
+		  lists:member(no_src, get(test_server_logopts))} of
+		{true,false} ->
+		    print(Lev, Info ++ "<a href=\"~ts#~ts\">~w:~w/~w</a> "
+			  "(click for source code)\n",
+			  [uri_encode(SrcListing),
+			   uri_encode(atom_to_list(Func)++"-1",utf8),
+			   Mod,Func,Arity]);
+		_ ->
+		    print(Lev, Info ++ "~w:~w/~w\n", [Mod,Func,Arity])
+	    end
     end,
 
     AbsName.
@@ -2020,7 +2025,7 @@ add_init_and_end_per_suite([{conf,_,_,{Mod,_}}=Case|Cases], LastMod,
     PreCases ++ [Case|add_init_and_end_per_suite(Cases, NextMod,
 						 NextRef, FwMod)];
 add_init_and_end_per_suite([SkipCase|Cases], LastMod, LastRef, FwMod)
-  when element(1,SkipCase) == skip_case ->
+  when element(1,SkipCase) == skip_case;  element(1,SkipCase) == auto_skip_case->
     [SkipCase|add_init_and_end_per_suite(Cases, LastMod, LastRef, FwMod)];
 add_init_and_end_per_suite([{conf,_,_,_}=Case|Cases], LastMod, LastRef, FwMod) ->
     [Case|add_init_and_end_per_suite(Cases, LastMod, LastRef, FwMod)];
@@ -3946,8 +3951,8 @@ progress(skip, CaseNum, Mod, Func, GrName, Loc, Reason, Time,
 	  "<td>~ts~ts</td></tr>\n",
 	  [Time,Color,ReasonStr2,Comment1]),
     FormatLoc = test_server_sup:format_loc(Loc),
-    print(minor, "=== location ~ts", [FormatLoc]),
-    print(minor, "=== reason = ~ts", [ReasonStr1]),
+    print(minor, "=== Location: ~ts", [FormatLoc]),
+    print(minor, "=== Reason: ~ts", [ReasonStr1]),
     Ret;
 
 progress(failed, CaseNum, Mod, Func, GrName, Loc, timetrap_timeout, T,
@@ -3972,8 +3977,8 @@ progress(failed, CaseNum, Mod, Func, GrName, Loc, timetrap_timeout, T,
 	  "<td>~ts</td></tr>\n",
 	  [T/1000,Comment]),
     FormatLoc = test_server_sup:format_loc(Loc),
-    print(minor, "=== location ~ts", [FormatLoc]),
-    print(minor, "=== reason = timetrap timeout", []),
+    print(minor, "=== Location: ~ts", [FormatLoc]),
+    print(minor, "=== Reason: timetrap timeout", []),
     failed;
 
 progress(failed, CaseNum, Mod, Func, GrName, Loc, {testcase_aborted,Reason}, _T,
@@ -3998,13 +4003,13 @@ progress(failed, CaseNum, Mod, Func, GrName, Loc, {testcase_aborted,Reason}, _T,
 	  "<td>~ts</td></tr>\n",
 	  [Comment]),
     FormatLoc = test_server_sup:format_loc(Loc),
-    print(minor, "=== location ~ts", [FormatLoc]),
-    print(minor, "=== reason = {testcase_aborted,~p}", [Reason]),
+    print(minor, "=== Location: ~ts", [FormatLoc]),
+    print(minor, "=== Reason: {testcase_aborted,~p}", [Reason]),
     failed;
 
 progress(failed, CaseNum, Mod, Func, GrName, unknown, Reason, Time,
 	 Comment0, {St0,St1}) ->
-    print(major, "=result        failed: ~p, ~w", [Reason,unknown]),
+    print(major, "=result        failed: ~p, ~w", [Reason,unknown_location]),
     print(1, "*** FAILED ~ts ***",
 	  [get_info_str(Mod,Func, CaseNum, get(test_server_cases))]),
     test_server_sup:framework_call(report, [tc_done,{Mod,{Func,GrName},
@@ -4033,14 +4038,21 @@ progress(failed, CaseNum, Mod, Func, GrName, unknown, Reason, Time,
 	  "<td><font color=\"red\">FAILED</font></td>"
 	  "<td>~ts</td></tr>\n",
 	  [TimeStr,Comment]),
-    print(minor, "=== location ~w", [unknown]),
+    print(minor, "=== Location: ~w", [unknown]),
     {FStr,FormattedReason} = format_exception(Reason),
-    print(minor, "=== reason = " ++ FStr, [FormattedReason]),
+    print(minor, "=== Reason: " ++ FStr, [FormattedReason]),
     failed;
 
 progress(failed, CaseNum, Mod, Func, GrName, Loc, Reason, Time,
 	 Comment0, {St0,St1}) ->
-    print(major, "=result        failed: ~p, ~p", [Reason,Loc]),
+    {LocMaj,LocMin} = if Func == error_in_suite ->
+			      case get_fw_mod(undefined) of
+				  Mod -> {unknown_location,unknown};
+				  _   -> {Loc,Loc}
+			      end;
+			 true -> {Loc,Loc}
+		       end,
+    print(major, "=result        failed: ~p, ~p", [Reason,LocMaj]),
     print(1, "*** FAILED ~ts ***",
 	  [get_info_str(Mod,Func, CaseNum, get(test_server_cases))]),
     test_server_sup:framework_call(report, [tc_done,{Mod,{Func,GrName},
@@ -4053,16 +4065,16 @@ progress(failed, CaseNum, Mod, Func, GrName, Loc, Reason, Time,
 	    "" -> "";
 	    _ -> xhtml("<br>","<br />") ++ to_string(Comment0)
 	end,
-    FormatLastLoc = test_server_sup:format_loc(get_last_loc(Loc)),
+    FormatLastLoc = test_server_sup:format_loc(get_last_loc(LocMaj)),
     print(html,
 	  "<td>" ++ St0 ++ "~ts" ++ St1 ++ "</td>"
 	  "<td><font color=\"red\">FAILED</font></td>"
 	  "<td><font color=\"red\">~ts</font>~ts</td></tr>\n",
 	  [TimeStr,FormatLastLoc,Comment]),
-    FormatLoc = test_server_sup:format_loc(Loc),
-    print(minor, "=== location ~ts", [FormatLoc]),
+    FormatLoc = test_server_sup:format_loc(LocMin),
+    print(minor, "=== Location: ~ts", [FormatLoc]),
     {FStr,FormattedReason} = format_exception(Reason),
-    print(minor, "=== reason = " ++ FStr, [FormattedReason]),
+    print(minor, "=== Reason: " ++ FStr, [FormattedReason]),
     failed;
 
 progress(ok, _CaseNum, Mod, Func, GrName, _Loc, RetVal, Time,
@@ -4091,7 +4103,7 @@ progress(ok, _CaseNum, Mod, Func, GrName, _Loc, RetVal, Time,
 	  "<td><font color=\"green\">Ok</font></td>"
 	  "~ts</tr>\n",
 	  [Time,Comment]),
-    print(minor, "=== returned value = ~p", [RetVal]),
+    print(minor, "=== Returned value: ~p", [RetVal]),
     ok.
 
 %%--------------------------------------------------------------------
@@ -4679,10 +4691,10 @@ collect_cases({make,InitMFA,CaseList,FinMFA}, St0, Mode) ->
 
 collect_cases({Module, Cases}, St, Mode) when is_list(Cases)  ->
     case (catch collect_case(Cases, St#cc{mod=Module}, [], Mode)) of
-	{ok, NewCases, NewSt} ->
- 	    {ok, NewCases, NewSt};
+	Result = {ok,_,_} ->
+ 	    Result;
  	Other ->
-	    {error, Other}
+	    {error,Other}
      end;
 
 collect_cases({_Mod,_Case}=Spec, St, Mode) ->
@@ -4700,9 +4712,9 @@ collect_case({Mod,{conf,_,_,_,_}=Conf}, St, Mode) ->
 
 collect_case(MFA, St, Mode) ->
     case in_skip_list(MFA, St#cc.skip) of
-	{true,Comment} ->
+	{true,Comment} when Comment /= make_failed ->
 	    {ok,[{skip_case,{MFA,Comment},Mode}],St};
-	false ->
+	_ ->
 	    case MFA of
 		{Mod,Case} -> collect_case_invoke(Mod, Case, MFA, St, Mode);
 		{_Mod,_Case,_Args} -> {ok,[MFA],St}