Merge branch 'peppe/common_test/hook_skip_failure' into maint

* peppe/common_test/hook_skip_failure:
  Fix CT hook pre_end_per_group causing crash when returning {skip,Reason}

OTP-11409

3 files changed, 123 insertions, 3 deletions

diff --git a/lib/common_test/src/cth_log_redirect.erl b/lib/common_test/src/cth_log_redirect.erl
index 8fed341600..61700a2032 100644
--- a/lib/common_test/src/cth_log_redirect.erl
+++ b/lib/common_test/src/cth_log_redirect.erl
@@ -73,7 +73,7 @@ pre_init_per_group(Group, Config, State) ->
     set_curr_func({group,Group,init_per_group}, Config),
     {Config, State}.
 
-post_init_per_group(Group, Config, Result, tc_log_async) ->
+post_init_per_group(Group, Config, Result, tc_log_async) when is_list(Config) ->
     case lists:member(parallel,proplists:get_value(
 				 tc_group_properties,Config,[])) of
 	true ->
@@ -154,7 +154,8 @@ handle_info(_, State) ->
 handle_call(flush,State) ->
     {ok, ok, State};
 
-handle_call({set_curr_func,{group,Group,Conf},Config}, State) ->
+handle_call({set_curr_func,{group,Group,Conf},Config},
+	    State) when is_list(Config) ->
     Parallel = case proplists:get_value(tc_group_properties, Config) of
 		   undefined -> false;
 		   Props -> lists:member(parallel, Props)
@@ -162,6 +163,10 @@ handle_call({set_curr_func,{group,Group,Conf},Config}, State) ->
     {ok, ok, State#eh_state{curr_group = Group,
 			    curr_func = Conf,
 			    parallel_tcs = Parallel}};
+handle_call({set_curr_func,{group,Group,Conf},_SkipOrFail}, State) ->
+    {ok, ok, State#eh_state{curr_group = Group,
+			    curr_func = Conf,
+			    parallel_tcs = false}};
 handle_call({set_curr_func,{group,undefined},_Config}, State) ->
     {ok, ok, State#eh_state{curr_group = undefined,
 			    curr_func = undefined,
diff --git a/lib/common_test/test/ct_hooks_SUITE.erl b/lib/common_test/test/ct_hooks_SUITE.erl
index 083c87b49e..b5855da9df 100644
--- a/lib/common_test/test/ct_hooks_SUITE.erl
+++ b/lib/common_test/test/ct_hooks_SUITE.erl
@@ -80,7 +80,7 @@ all(suite) ->
        scope_per_suite_state_cth, scope_per_group_state_cth, 
        scope_suite_state_cth,
        fail_pre_suite_cth, double_fail_pre_suite_cth,
-       fail_post_suite_cth, skip_pre_suite_cth,
+       fail_post_suite_cth, skip_pre_suite_cth, skip_pre_end_cth,
        skip_post_suite_cth, recover_post_suite_cth, update_config_cth,
        state_update_cth, options_cth, same_id_cth, 
        fail_n_skip_with_minimal_cth, prio_cth, no_config,
@@ -181,6 +181,10 @@ skip_pre_suite_cth(Config) when is_list(Config) ->
     do_test(skip_pre_suite_cth, "ct_cth_empty_SUITE.erl",
 	    [skip_pre_suite_cth],Config).
 
+skip_pre_end_cth(Config) when is_list(Config) ->
+    do_test(skip_pre_end_cth, "ct_scope_per_group_cth_SUITE.erl",
+	    [skip_pre_end_cth],Config).
+
 skip_post_suite_cth(Config) when is_list(Config) ->
     do_test(skip_post_suite_cth, "ct_cth_empty_SUITE.erl",
 	    [skip_post_suite_cth],Config).
@@ -759,6 +763,43 @@ test_events(skip_pre_suite_cth) ->
      {?eh,stop_logging,[]}
     ];
 
+test_events(skip_pre_end_cth) ->
+    [
+     {?eh,start_logging,{'DEF','RUNDIR'}},
+     {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
+     {?eh,tc_start,{ct_scope_per_group_cth_SUITE,init_per_suite}},
+     {?eh,tc_done,{ct_scope_per_group_cth_SUITE,init_per_suite,ok}},
+     
+     [{?eh,tc_start,{ct_scope_per_group_cth_SUITE,{init_per_group,group1,[]}}},
+      {?eh,cth,{'_',id,[[]]}},
+      {?eh,cth,{'_',init,['_',[]]}},
+      {?eh,cth,{'_',post_init_per_group,[group1,'$proplist','$proplist',[]]}},
+      {?eh,tc_done,{ct_scope_per_group_cth_SUITE,{init_per_group,group1,[]},ok}},
+      
+      {?eh,tc_start,{ct_scope_per_group_cth_SUITE,test_case}},
+      {?eh,cth,{'_',pre_init_per_testcase,[test_case,'$proplist',[]]}},
+      {?eh,cth,{'_',post_end_per_testcase,[test_case,'$proplist',ok,[]]}},
+      {?eh,tc_done,{ct_scope_per_group_cth_SUITE,test_case,ok}},
+      
+      {?eh,tc_start,{ct_scope_per_group_cth_SUITE,{end_per_group,group1,[]}}},
+      {?eh,cth,{'_',pre_end_per_group,[group1,'$proplist',[]]}},
+      {?eh,cth,{'_',post_end_per_group,[group1,'$proplist','_',[]]}},
+      {?eh,tc_done,{ct_scope_per_group_cth_SUITE,{end_per_group,group1,[]},
+		    {skipped,"Test skip"}}}],
+      {?eh,cth,{'_',on_tc_skip,[end_per_group,
+				{tc_user_skip,{skipped,"Test skip"}},
+				[]]}},
+     {?eh,tc_start,{ct_scope_per_group_cth_SUITE,end_per_suite}},
+     {?eh,tc_done,{ct_scope_per_group_cth_SUITE,end_per_suite,
+		   {skipped,"Test skip"}}},
+     {?eh,cth,{'_',on_tc_skip,[end_per_suite,
+			       {tc_user_skip,{skipped,"Test skip"}},
+			       []]}},
+     {?eh,test_done,{'DEF','STOP_TIME'}},
+     {?eh,cth,{'_',terminate,[[]]}},
+     {?eh,stop_logging,[]}
+    ];
+
 test_events(skip_post_suite_cth) ->
     [
      {?eh,start_logging,{'DEF','RUNDIR'}},
diff --git a/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/skip_pre_end_cth.erl b/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/skip_pre_end_cth.erl
new file mode 100644
index 0000000000..ece4eb2247
--- /dev/null
+++ b/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/skip_pre_end_cth.erl
@@ -0,0 +1,74 @@
+%%

+%% %CopyrightBegin%

+%%

+%% Copyright Ericsson AB 2010-2011. 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(skip_pre_end_cth).

+

+

+-include_lib("common_test/src/ct_util.hrl").

+-include_lib("common_test/include/ct_event.hrl").

+

+

+%% CT Hooks

+-compile(export_all).

+

+init(Id, Opts) ->

+    empty_cth:init(Id, Opts).

+    

+

+pre_init_per_suite(Suite, Config, State) ->

+    empty_cth:pre_init_per_suite(Suite,Config,State).

+

+post_init_per_suite(Suite,Config,Return,State) ->

+    empty_cth:post_init_per_suite(Suite,Config,Return,State).

+

+pre_end_per_suite(Suite,Config,State) ->

+    empty_cth:pre_end_per_suite(Suite,Config,State),

+    {{skip, "Test skip"}, State}.

+

+post_end_per_suite(Suite,Config,Return,State) ->

+    empty_cth:post_end_per_suite(Suite,Config,Return,State).

+

+pre_init_per_group(Group,Config,State) ->

+    empty_cth:pre_init_per_group(Group,Config,State).

+

+post_init_per_group(Group,Config,Return,State) ->

+    empty_cth:post_init_per_group(Group,Config,Return,State).

+

+pre_end_per_group(Group,Config,State) ->

+    empty_cth:pre_end_per_group(Group,Config,State),

+    {{skip, "Test skip"}, State}.

+

+post_end_per_group(Group,Config,Return,State) ->

+    empty_cth:post_end_per_group(Group,Config,Return,State).

+

+pre_init_per_testcase(TC,Config,State) ->

+    empty_cth:pre_init_per_testcase(TC,Config,State).

+

+post_end_per_testcase(TC,Config,Return,State) ->

+    empty_cth:post_end_per_testcase(TC,Config,Return,State).

+

+on_tc_fail(TC, Reason, State) ->

+    empty_cth:on_tc_fail(TC,Reason,State).

+

+on_tc_skip(TC, Reason, State) ->

+    empty_cth:on_tc_skip(TC,Reason,State).

+

+terminate(State) ->

+    empty_cth:terminate(State).