[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

3 files changed, 88 insertions, 45 deletions

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"}.