[cth_surefire] Handle skips from test spec

When a test case is skipped from the test spec, there is no call to
the common_test hook functions 'pre_init_per_testcase' and
'post_end_per_testcase'. This causes cth_surefire to erroneously mark
the previous test case as skipped in the xml report. The actually
skipped test case is not present in the xml report at all. This is now
corrected.

OTP-14129

4 files changed, 116 insertions, 71 deletions

diff --git a/lib/common_test/src/cth_surefire.erl b/lib/common_test/src/cth_surefire.erl
index 59b916851e..c4941948cc 100644
--- a/lib/common_test/src/cth_surefire.erl
+++ b/lib/common_test/src/cth_surefire.erl
@@ -33,16 +33,16 @@
 -export([pre_end_per_suite/3]).
 -export([post_end_per_suite/4]).
 
--export([pre_init_per_group/3]).
--export([post_init_per_group/4]).
--export([pre_end_per_group/3]).
--export([post_end_per_group/4]).
+-export([pre_init_per_group/4]).
+-export([post_init_per_group/5]).
+-export([pre_end_per_group/4]).
+-export([post_end_per_group/5]).
 
--export([pre_init_per_testcase/3]).
--export([post_end_per_testcase/4]).
+-export([pre_init_per_testcase/4]).
+-export([post_end_per_testcase/5]).
 
--export([on_tc_fail/3]).
--export([on_tc_skip/3]).
+-export([on_tc_fail/4]).
+-export([on_tc_skip/4]).
 
 -export([terminate/1]).
 
@@ -116,29 +116,29 @@ pre_end_per_suite(_Suite,Config,State) ->
 post_end_per_suite(_Suite,Config,Result,State) ->
     {Result, end_tc(end_per_suite,Config,Result,State)}.
 
-pre_init_per_group(Group,Config,State) ->
+pre_init_per_group(_Suite,Group,Config,State) ->
     {Config, init_tc(State#state{ curr_group = [Group|State#state.curr_group]},
 		     Config)}.
 
-post_init_per_group(_Group,Config,Result,State) ->
+post_init_per_group(_Suite,_Group,Config,Result,State) ->
     {Result, end_tc(init_per_group,Config,Result,State)}.
 
-pre_end_per_group(_Group,Config,State) ->
+pre_end_per_group(_Suite,_Group,Config,State) ->
     {Config, init_tc(State, Config)}.
 
-post_end_per_group(_Group,Config,Result,State) ->
+post_end_per_group(_Suite,_Group,Config,Result,State) ->
     NewState = end_tc(end_per_group, Config, Result, State),
     {Result, NewState#state{ curr_group = tl(NewState#state.curr_group)}}.
 
-pre_init_per_testcase(_TC,Config,State) ->
+pre_init_per_testcase(_Suite,_TC,Config,State) ->
     {Config, init_tc(State, Config)}.
 
-post_end_per_testcase(TC,Config,Result,State) ->
+post_end_per_testcase(_Suite,TC,Config,Result,State) ->
     {Result, end_tc(TC,Config, Result,State)}.
 
-on_tc_fail(_TC, _Res, State = #state{test_cases = []}) ->
+on_tc_fail(_Suite,_TC, _Res, State = #state{test_cases = []}) ->
     State;
-on_tc_fail(_TC, Res, State) ->
+on_tc_fail(_Suite,_TC, Res, State) ->
     TCs = State#state.test_cases,
     TC = hd(TCs),
     NewTC = TC#testcase{
@@ -146,10 +146,9 @@ on_tc_fail(_TC, Res, State) ->
 		  {fail,lists:flatten(io_lib:format("~p",[Res]))} },
     State#state{ test_cases = [NewTC | tl(TCs)]}.
 
-on_tc_skip({ConfigFunc,_GrName},{Type,_Reason} = Res, State0)
-  when Type == tc_auto_skip; Type == tc_user_skip ->
-    on_tc_skip(ConfigFunc, Res, State0);
-on_tc_skip(Tc,{Type,_Reason} = Res, State0) when Type == tc_auto_skip ->
+on_tc_skip(Suite,{ConfigFunc,_GrName}, Res, State) ->
+    on_tc_skip(Suite,ConfigFunc, Res, State);
+on_tc_skip(Suite,Tc, Res, State0) ->
     TcStr = atom_to_list(Tc),
     State =
 	case State0#state.test_cases of
@@ -158,11 +157,7 @@ on_tc_skip(Tc,{Type,_Reason} = Res, State0) when Type == tc_auto_skip ->
 	    _ ->
 		State0
 	end,
-    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, end_tc(Tc,[],Res,init_tc(set_suite(Suite,State),[]))).
 
 do_tc_skip(Res, State) ->
     TCs = State#state.test_cases,
@@ -209,6 +204,12 @@ end_tc(Name, _Config, _Res, State = #state{ curr_suite = Suite,
 					  result = passed }|
 			       State#state.test_cases],
 		 tc_log = ""}. % so old tc_log is not set if next is on_tc_skip
+
+set_suite(Suite,#state{curr_suite=undefined}=State) ->
+    State#state{curr_suite=Suite, curr_suite_ts=?now};
+set_suite(_,State) ->
+    State.
+
 close_suite(#state{ test_cases = [] } = State) ->
     State;
 close_suite(#state{ test_cases = TCs, url_base = UrlBase } = State) ->
@@ -228,7 +229,8 @@ close_suite(#state{ test_cases = TCs, url_base = UrlBase } = State) ->
 			testcases = lists:reverse(TCs),
 			log = SuiteLog,
 			url = SuiteUrl},
-    State#state{ test_cases = [],
+    State#state{ curr_suite = undefined,
+                 test_cases = [],
 		 test_suites = [Suite | State#state.test_suites]}.
 
 terminate(State = #state{ test_cases = [] }) ->
diff --git a/lib/common_test/test/ct_surefire_SUITE.erl b/lib/common_test/test/ct_surefire_SUITE.erl
index 42ec685c16..884217afc2 100644
--- a/lib/common_test/test/ct_surefire_SUITE.erl
+++ b/lib/common_test/test/ct_surefire_SUITE.erl
@@ -73,7 +73,9 @@ all() ->
      relative_path,
      url,
      logdir,
-     fail_pre_init_per_suite
+     fail_pre_init_per_suite,
+     skip_case_in_spec,
+     skip_suite_in_spec
     ].
 
 %%--------------------------------------------------------------------
@@ -119,6 +121,18 @@ fail_pre_init_per_suite(Config) when is_list(Config) ->
     run(fail_pre_init_per_suite,[fail_pre_init_per_suite,
         {cth_surefire,[{path,Path}]}],Path,Config,[],Suites).
 
+skip_case_in_spec(Config) ->
+    DataDir = ?config(data_dir,Config),
+    Spec = filename:join(DataDir,"skip_one_case.spec"),
+    Path = "skip_case_in_spec.xml",
+    run_spec(skip_case_in_spec,[{cth_surefire,[{path,Path}]}],Path,Config,Spec).
+
+skip_suite_in_spec(Config) ->
+    DataDir = ?config(data_dir,Config),
+    Spec = filename:join(DataDir,"skip_one_suite.spec"),
+    Path = "skip_suite_in_spec.xml",
+    run_spec(skip_suite_in_spec,[{cth_surefire,[{path,Path}]}],Path,Config,Spec).
+
 %%%-----------------------------------------------------------------
 %%% HELP FUNCTIONS
 %%%-----------------------------------------------------------------
@@ -129,8 +143,15 @@ run(Case,CTHs,Report,Config,ExtraOpts) ->
     Suite = filename:join(DataDir, "surefire_SUITE"),
     run(Case,CTHs,Report,Config,ExtraOpts,Suite).
 run(Case,CTHs,Report,Config,ExtraOpts,Suite) ->
-    {Opts,ERPid} = setup([{suite,Suite},{ct_hooks,CTHs},{label,Case}|ExtraOpts],
-			 Config),
+    Test = [{suite,Suite},{ct_hooks,CTHs},{label,Case}|ExtraOpts],
+    do_run(Case, Report, Test, Config).
+
+run_spec(Case,CTHs,Report,Config,Spec) ->
+    Test = [{spec,Spec},{ct_hooks,CTHs},{label,Case}],
+    do_run(Case, Report, Test, Config).
+
+do_run(Case, Report, Test, Config) ->
+    {Opts,ERPid} = setup(Test, Config),
     ok = execute(Case, Opts, ERPid, Config),
     LogDir =
 	case lists:keyfind(logdir,1,Opts) of
@@ -201,7 +222,10 @@ test_suite_events(pass_SUITE) ->
      {?eh,test_stats,{1,0,{0,0}}},
      {?eh,tc_start,{ct_framework,end_per_suite}},
      {?eh,tc_done,{ct_framework,end_per_suite,ok}}];
-test_suite_events(_) ->
+test_suite_events(skip_all_surefire_SUITE) ->
+    [{?eh,tc_user_skip,{skip_all_surefire_SUITE,all,"skipped in spec"}},
+     {?eh,test_stats,{0,0,{1,0}}}];
+test_suite_events(Test) ->
     [{?eh,tc_start,{surefire_SUITE,init_per_suite}},
      {?eh,tc_done,{surefire_SUITE,init_per_suite,ok}},
      {?eh,tc_start,{surefire_SUITE,tc_ok}},
@@ -210,46 +234,55 @@ test_suite_events(_) ->
      {?eh,tc_start,{surefire_SUITE,tc_fail}},
      {?eh,tc_done,{surefire_SUITE,tc_fail,
 		   {failed,{error,{test_case_failed,"this test should fail"}}}}},
-     {?eh,test_stats,{1,1,{0,0}}},
-     {?eh,tc_start,{surefire_SUITE,tc_skip}},
-     {?eh,tc_done,{surefire_SUITE,tc_skip,{skipped,"this test is skipped"}}},
-     {?eh,test_stats,{1,1,{1,0}}},
-     {?eh,tc_start,{surefire_SUITE,tc_autoskip_require}},
-     {?eh,tc_done,{surefire_SUITE,tc_autoskip_require,
-		   {auto_skipped,{require_failed,'_'}}}},
-     {?eh,test_stats,{1,1,{1,1}}},
-     [{?eh,tc_start,{surefire_SUITE,{init_per_group,g,[]}}},
-      {?eh,tc_done,{surefire_SUITE,{init_per_group,g,[]},ok}},
-      {?eh,tc_start,{surefire_SUITE,tc_ok}},
-      {?eh,tc_done,{surefire_SUITE,tc_ok,ok}},
-      {?eh,test_stats,{2,1,{1,1}}},
-      {?eh,tc_start,{surefire_SUITE,tc_fail}},
-      {?eh,tc_done,{surefire_SUITE,tc_fail,
-		    {failed,{error,{test_case_failed,"this test should fail"}}}}},
-      {?eh,test_stats,{2,2,{1,1}}},
-      {?eh,tc_start,{surefire_SUITE,tc_skip}},
-      {?eh,tc_done,{surefire_SUITE,tc_skip,{skipped,"this test is skipped"}}},
-      {?eh,test_stats,{2,2,{2,1}}},
-      {?eh,tc_start,{surefire_SUITE,tc_autoskip_require}},
-      {?eh,tc_done,{surefire_SUITE,tc_autoskip_require,
-		    {auto_skipped,{require_failed,'_'}}}},
-      {?eh,test_stats,{2,2,{2,2}}},
-      {?eh,tc_start,{surefire_SUITE,{end_per_group,g,[]}}},
-      {?eh,tc_done,{surefire_SUITE,{end_per_group,g,[]},ok}}],
-     [{?eh,tc_start,{surefire_SUITE,{init_per_group,g_fail,[]}}},
-      {?eh,tc_done,{surefire_SUITE,{init_per_group,g_fail,[]},
-		    {failed,{error,all_cases_should_be_skipped}}}},
-      {?eh,tc_auto_skip,{surefire_SUITE,{tc_ok,g_fail},
-			 {failed,
-			  {surefire_SUITE,init_per_group,
-			   {'EXIT',all_cases_should_be_skipped}}}}},
-      {?eh,test_stats,{2,2,{2,3}}},
-      {?eh,tc_auto_skip,{surefire_SUITE,{end_per_group,g_fail},
-			 {failed,
-			  {surefire_SUITE,init_per_group,
-			   {'EXIT',all_cases_should_be_skipped}}}}}],
-     {?eh,tc_start,{surefire_SUITE,end_per_suite}},
-     {?eh,tc_done,{surefire_SUITE,end_per_suite,ok}}].
+     {?eh,test_stats,{1,1,{0,0}}}] ++
+        tc_skip_events(Test,undefined) ++
+        [{?eh,test_stats,{1,1,{1,0}}},
+         {?eh,tc_start,{surefire_SUITE,tc_autoskip_require}},
+         {?eh,tc_done,{surefire_SUITE,tc_autoskip_require,
+                       {auto_skipped,{require_failed,'_'}}}},
+         {?eh,test_stats,{1,1,{1,1}}},
+         [{?eh,tc_start,{surefire_SUITE,{init_per_group,g,[]}}},
+          {?eh,tc_done,{surefire_SUITE,{init_per_group,g,[]},ok}},
+          {?eh,tc_start,{surefire_SUITE,tc_ok}},
+          {?eh,tc_done,{surefire_SUITE,tc_ok,ok}},
+          {?eh,test_stats,{2,1,{1,1}}},
+          {?eh,tc_start,{surefire_SUITE,tc_fail}},
+          {?eh,tc_done,{surefire_SUITE,tc_fail,
+                        {failed,{error,{test_case_failed,"this test should fail"}}}}},
+          {?eh,test_stats,{2,2,{1,1}}}] ++
+             tc_skip_events(Test,g) ++
+             [{?eh,test_stats,{2,2,{2,1}}},
+              {?eh,tc_start,{surefire_SUITE,tc_autoskip_require}},
+              {?eh,tc_done,{surefire_SUITE,tc_autoskip_require,
+                            {auto_skipped,{require_failed,'_'}}}},
+              {?eh,test_stats,{2,2,{2,2}}},
+              {?eh,tc_start,{surefire_SUITE,{end_per_group,g,[]}}},
+              {?eh,tc_done,{surefire_SUITE,{end_per_group,g,[]},ok}}],
+         [{?eh,tc_start,{surefire_SUITE,{init_per_group,g_fail,[]}}},
+          {?eh,tc_done,{surefire_SUITE,{init_per_group,g_fail,[]},
+                        {failed,{error,all_cases_should_be_skipped}}}},
+          {?eh,tc_auto_skip,{surefire_SUITE,{tc_ok,g_fail},
+                             {failed,
+                              {surefire_SUITE,init_per_group,
+                               {'EXIT',all_cases_should_be_skipped}}}}},
+          {?eh,test_stats,{2,2,{2,3}}},
+          {?eh,tc_auto_skip,{surefire_SUITE,{end_per_group,g_fail},
+                             {failed,
+                              {surefire_SUITE,init_per_group,
+                               {'EXIT',all_cases_should_be_skipped}}}}}],
+         {?eh,tc_start,{surefire_SUITE,end_per_suite}},
+         {?eh,tc_done,{surefire_SUITE,end_per_suite,ok}}].
+
+tc_skip_events(skip_case_in_spec,Group) ->
+    [{?eh,tc_user_skip,{surefire_SUITE,tc_skip_name(Group),"skipped in spec"}}];
+tc_skip_events(_Test,_Group) ->
+    [{?eh,tc_start,{surefire_SUITE,tc_skip}},
+     {?eh,tc_done,{surefire_SUITE,tc_skip,{skipped,"this test is skipped"}}}].
+
+tc_skip_name(undefined) ->
+    tc_skip;
+tc_skip_name(Group) ->
+    {tc_skip,Group}.
 
 test_events(fail_pre_init_per_suite) ->
     [{?eh,start_logging,{'DEF','RUNDIR'}},
@@ -257,6 +290,10 @@ test_events(fail_pre_init_per_suite) ->
      test_suite_events(pass_SUITE) ++
      test_suite_events(fail_SUITE, {1,0,{0,1}}) ++
      [{?eh,stop_logging,[]}];
+test_events(skip_suite_in_spec) ->
+    [{?eh,start_logging,'_'},{?eh,start_info,{1,1,0}}] ++
+     test_suite_events(skip_all_surefire_SUITE) ++
+     [{?eh,stop_logging,[]}];
 test_events(Test) ->
     [{?eh,start_logging,'_'}, {?eh,start_info,{1,1,9}}] ++
     test_suite_events(Test) ++
@@ -364,6 +401,8 @@ failed_or_skipped([]) ->
 events_to_result(E) ->
     events_to_result(E, []).
 
+events_to_result([{?eh,tc_user_skip,{_Suite,all,_}}|E], Result) ->
+    events_to_result(E, [[[s]]|Result]);
 events_to_result([{?eh,tc_auto_skip,{_Suite,init_per_suite,_}}|E], Result) ->
     {Suite,Rest} = events_to_result1(E),
     events_to_result(Rest, [[[s]|Suite]|Result]);
@@ -382,7 +421,7 @@ events_to_result1([{?eh,tc_done,{_Suite, end_per_suite,R}}|E]) ->
 events_to_result1([{?eh,tc_done,{_Suite,_Case,R}}|E]) ->
     {Suite,Rest} = events_to_result1(E),
     {[result(R)|Suite],Rest};
-events_to_result1([{?eh,tc_auto_skip,_}|E]) ->
+events_to_result1([{?eh,Skip,_}|E]) when Skip==tc_auto_skip; Skip==tc_user_skip ->
     {Suite,Rest} = events_to_result1(E),
     {[[s]|Suite],Rest};
 events_to_result1([_|E]) ->
diff --git a/lib/common_test/test/ct_surefire_SUITE_data/skip_one_case.spec b/lib/common_test/test/ct_surefire_SUITE_data/skip_one_case.spec
new file mode 100644
index 0000000000..42df8a7d1a
--- /dev/null
+++ b/lib/common_test/test/ct_surefire_SUITE_data/skip_one_case.spec
@@ -0,0 +1,2 @@
+{suites,".",surefire_SUITE}.
+{skip_cases,".",surefire_SUITE,tc_skip,"skipped in spec"}.
diff --git a/lib/common_test/test/ct_surefire_SUITE_data/skip_one_suite.spec b/lib/common_test/test/ct_surefire_SUITE_data/skip_one_suite.spec
new file mode 100644
index 0000000000..57966328ab
--- /dev/null
+++ b/lib/common_test/test/ct_surefire_SUITE_data/skip_one_suite.spec
@@ -0,0 +1,2 @@
+{suites,".",[skip_all_surefire_SUITE]}.
+{skip_suites,".",skip_all_surefire_SUITE,"skipped in spec"}.