Fix a couple of minor bugs with hook priority

The bugs caused the sorting priority to be wrong when using
installed priority and built in priority. Tests to prove the
order of hooks to be correct have also been added.

4 files changed, 190 insertions, 76 deletions

diff --git a/lib/common_test/src/ct_hooks.erl b/lib/common_test/src/ct_hooks.erl
index 44aba94cd1..f243b87f54 100644
--- a/lib/common_test/src/ct_hooks.erl
+++ b/lib/common_test/src/ct_hooks.erl
@@ -34,7 +34,7 @@
 %% If you change this, remember to update ct_util:look -> stop clause as well.
 -define(config_name, ct_hooks).
 
--record(ct_hook_config, {id, module, prio = 0, scope, opts = [], state = []}).
+-record(ct_hook_config, {id, module, prio, scope, opts = [], state = []}).
 
 %% -------------------------------------------------------------------------
 %% API Functions
@@ -138,6 +138,8 @@ call_id(#ct_hook_config{ module = Mod, opts = Opts} = Hook, Config, Scope) ->
 call_init(#ct_hook_config{ module = Mod, opts = Opts, id = Id, prio = P} = Hook,
 	  Config,_Meta) ->
     case Mod:init(Id, Opts) of
+	{ok, NewState} when P =:= undefined ->
+	    {Config, Hook#ct_hook_config{ state = NewState, prio = 0 } };
 	{ok, NewState} ->
 	    {Config, Hook#ct_hook_config{ state = NewState } };
 	{ok, NewState, Prio} when P =:= undefined ->
@@ -189,12 +191,12 @@ call([{Hook, call_id, NextFun} | Rest], Config, Meta, Hooks) ->
 	    case lists:keyfind(NewId, #ct_hook_config.id, Hooks) of
 		false when NextFun =:= undefined ->
 		    {Hooks ++ [NewHook],
-		     [{NewId, fun call_init/3} | Rest]};
+		     [{NewId, call_init} | Rest]};
 		ExistingHook when is_tuple(ExistingHook) ->
 		    {Hooks, Rest};
 		_ ->
 		    {Hooks ++ [NewHook],
-		     [{NewId, fun call_init/3},{NewId,NextFun} | Rest]}
+		     [{NewId, call_init}, {NewId,NextFun} | Rest]}
 	    end,
 	call(resort(NewRest,NewHooks), Config, Meta, NewHooks)
     catch Error:Reason ->
@@ -204,13 +206,16 @@ call([{Hook, call_id, NextFun} | Rest], Config, Meta, Hooks) ->
 	    call([], {fail,"Failed to start CTH"
 		      ", see the CT Log for details"}, Meta, Hooks)
     end;
+call([{HookId, call_init} | Rest], Config, Meta, Hooks) ->
+    call([{HookId, fun call_init/3} | Rest], Config, Meta, Hooks);
 call([{HookId, Fun} | Rest], Config, Meta, Hooks) ->
     try
         Hook = lists:keyfind(HookId, #ct_hook_config.id, Hooks),
         {NewConf, NewHook} =  Fun(Hook, Config, Meta),
         NewCalls = get_new_hooks(NewConf, Fun),
         NewHooks = lists:keyreplace(HookId, #ct_hook_config.id, Hooks, NewHook),
-        call(NewCalls  ++ Rest, remove(?config_name, NewConf), Meta,
+        call(resort(NewCalls ++ Rest,NewHooks), %% Resort if call_init changed prio
+	     remove(?config_name, NewConf), Meta,
              terminate_if_scope_ends(HookId, Meta, NewHooks))
     catch throw:{error_in_cth_call,Reason} ->
             call(Rest, {fail, Reason}, Meta,
@@ -284,14 +289,41 @@ save_suite_data_async(Hooks) ->
 get_hooks() ->
     lists:keysort(#ct_hook_config.prio,ct_util:read_suite_data(?config_name)).
 
-%% Call with three element tuples are call_id so always do them first
+%% Sort all calls in this order:
+%% call_id < call_init < Hook Priority 1 < .. < Hook Priority N
+%% If Hook Priority is equal, check when it has been installed and
+%% sort on that instead.
 resort(Calls, Hooks) ->
-    [Call || {_,_,_} = Call <- Calls] ++
-	resort1(Calls, lists:keysort(#ct_hook_config.prio, Hooks)).
-resort1(Calls, [#ct_hook_config{ id = Id }|Rest]) ->
-    [Call || {CId,_} = Call <- Calls, CId =:= Id] ++ resort1(Calls,Rest);
-resort1(_,[]) ->
-    [].
+    lists:sort(
+      fun({_,_,_},_) ->
+	      true;
+	 (_,{_,_,_}) ->
+	      false;
+	 ({_,call_init},_) ->
+	      true;
+	 (_,{_,call_init}) ->
+	      false;
+	 ({Id1,_},{Id2,_}) ->
+	      P1 = (lists:keyfind(Id1, #ct_hook_config.id, Hooks))#ct_hook_config.prio,
+	      P2 = (lists:keyfind(Id2, #ct_hook_config.id, Hooks))#ct_hook_config.prio,
+	      if
+		  P1 == P2 ->
+		      %% If priorities are equal, we check the position in the
+		      %% hooks list
+		      pos(Id1,Hooks) < pos(Id2,Hooks);
+		  true ->
+		      P1 < P2
+	      end
+      end,Calls).
+
+pos(Id,Hooks) ->
+    pos(Id,Hooks,0).
+pos(Id,[#ct_hook_config{ id = Id}|_],Num) ->
+    Num;
+pos(Id,[_|Rest],Num) ->
+    pos(Id,Rest,Num+1).
+
+
 
 catch_apply(M,F,A, Default) ->
     try
diff --git a/lib/common_test/test/ct_hooks_SUITE.erl b/lib/common_test/test/ct_hooks_SUITE.erl
index 16a95461e6..5c99f0f9f7 100644
--- a/lib/common_test/test/ct_hooks_SUITE.erl
+++ b/lib/common_test/test/ct_hooks_SUITE.erl
@@ -211,7 +211,8 @@ fail_n_skip_with_minimal_cth(Config) when is_list(Config) ->
 
 prio_cth(Config) when is_list(Config) ->
     do_test(prio_cth, "ct_cth_prio_SUITE.erl",
-	    [{empty_cth,[1000],1000},{empty_cth,[900],900}],Config).
+	    [{empty_cth,[1000],1000},{empty_cth,[900],900},
+	     {prio_cth,[1100,100],100},{prio_cth,[1100]}],Config).
 
 %%%-----------------------------------------------------------------
 %%% HELP FUNCTIONS
@@ -1006,68 +1007,71 @@ test_events(fail_n_skip_with_minimal_cth) ->
     ];
 
 test_events(prio_cth) ->
+    
+    GenPre = fun(Func,States) ->
+		     [{?eh,cth,{'_',Func,['_','_',State]}} || 
+			 State <- States]
+	     end,
+
+    GenPost = fun(Func,States) ->
+		      [{?eh,cth,{'_',Func,['_','_','_',State]}} || 
+			  State <- States]
+	     end,
+    
     [{?eh,start_logging,{'DEF','RUNDIR'}},
-     {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
-
-     {?eh,tc_start,{ct_cth_prio_SUITE,init_per_suite}},
-     {?eh,cth,{'_',pre_init_per_suite,['_','_',[800]]}},
-     {?eh,cth,{'_',pre_init_per_suite,['_','_',[900]]}},
-     {?eh,cth,{'_',pre_init_per_suite,['_','_',[1000]]}},
-     {?eh,cth,{'_',post_init_per_suite,['_','_','_',[700]]}},
-     {?eh,cth,{'_',post_init_per_suite,['_','_','_',[800]]}},
-     {?eh,cth,{'_',post_init_per_suite,['_','_','_',[900]]}},
-     {?eh,cth,{'_',post_init_per_suite,['_','_','_',[1000]]}},
-     {?eh,tc_done,{ct_cth_prio_SUITE,init_per_suite,ok}},
-
-
-     [{?eh,tc_start,{ct_cth_prio_SUITE,{init_per_group,'_',[]}}},
-      {?eh,cth,{'_',pre_init_per_group, ['_','_',[700]]}},
-      {?eh,cth,{'_',pre_init_per_group, ['_','_',[800]]}},
-      {?eh,cth,{'_',pre_init_per_group, ['_','_',[900]]}},
-      {?eh,cth,{'_',pre_init_per_group, ['_','_',[1000]]}},
-      {?eh,cth,{'_',post_init_per_group, ['_','_','_',[600]]}},
-      {?eh,cth,{'_',post_init_per_group, ['_','_','_',[700]]}},
-      {?eh,cth,{'_',post_init_per_group, ['_','_','_',[800]]}},
-      {?eh,cth,{'_',post_init_per_group, ['_','_','_',[900]]}},
-      {?eh,cth,{'_',post_init_per_group, ['_','_','_',[1000]]}},
-      {?eh,tc_done,{ct_cth_prio_SUITE,{init_per_group,'_',[]},ok}},
-
-      {?eh,tc_start,{ct_cth_prio_SUITE,test_case}},
-      {?eh,cth,{'_',pre_init_per_testcase, ['_','_',[600]]}},
-      {?eh,cth,{'_',pre_init_per_testcase, ['_','_',[700]]}},
-      {?eh,cth,{'_',pre_init_per_testcase, ['_','_',[800]]}},
-      {?eh,cth,{'_',pre_init_per_testcase, ['_','_',[900]]}},
-      {?eh,cth,{'_',pre_init_per_testcase, ['_','_',[1000]]}},
-      {?eh,cth,{'_',post_end_per_testcase, ['_','_','_',[600]]}},
-      {?eh,cth,{'_',post_end_per_testcase, ['_','_','_',[700]]}},
-      {?eh,cth,{'_',post_end_per_testcase, ['_','_','_',[800]]}},
-      {?eh,cth,{'_',post_end_per_testcase, ['_','_','_',[900]]}},
-      {?eh,cth,{'_',post_end_per_testcase, ['_','_','_',[1000]]}},
-      {?eh,tc_done,{ct_cth_prio_SUITE,test_case,ok}},
-
-      {?eh,tc_start,{ct_cth_prio_SUITE,{end_per_group,'_',[]}}},
-      {?eh,cth,{'_',pre_end_per_group, ['_','_',[600]]}},
-      {?eh,cth,{'_',pre_end_per_group, ['_','_',[700]]}},
-      {?eh,cth,{'_',pre_end_per_group, ['_','_',[800]]}},
-      {?eh,cth,{'_',pre_end_per_group, ['_','_',[900]]}},
-      {?eh,cth,{'_',pre_end_per_group, ['_','_',[1000]]}},
-      {?eh,cth,{'_',post_end_per_group, ['_','_','_',[700]]}},
-      {?eh,cth,{'_',post_end_per_group, ['_','_','_',[800]]}},
-      {?eh,cth,{'_',post_end_per_group, ['_','_','_',[900]]}},
-      {?eh,cth,{'_',post_end_per_group, ['_','_','_',[1000]]}},
-      {?eh,tc_done,{ct_cth_prio_SUITE,{end_per_group,'_',[]},ok}}],
-
-     {?eh,tc_start,{ct_cth_prio_SUITE,end_per_suite}},
-     {?eh,cth,{'_',pre_end_per_suite,['_','_',[800]]}},
-     {?eh,cth,{'_',pre_end_per_suite,['_','_',[900]]}},
-     {?eh,cth,{'_',pre_end_per_suite,['_','_',[1000]]}},
-     {?eh,cth,{'_',post_end_per_suite,['_','_','_',[800]]}},
-     {?eh,cth,{'_',post_end_per_suite,['_','_','_',[900]]}},
-     {?eh,cth,{'_',post_end_per_suite,['_','_','_',[1000]]}},
-     {?eh,tc_done,{ct_cth_prio_SUITE,end_per_suite,ok}},
-     {?eh,test_done,{'DEF','STOP_TIME'}},
-     {?eh,stop_logging,[]}
-    ];
+     {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}}] ++
+
+	[{?eh,tc_start,{ct_cth_prio_SUITE,init_per_suite}}] ++
+	GenPre(pre_init_per_suite,
+	       [[1100,100],[800],[900],[1000],[1200,1050],[1100],[1200]]) ++
+	GenPost(post_init_per_suite,
+		[[1100,100],[600,200],[600,600],[700],[800],[900],[1000],
+		 [1200,1050],[1100],[1200]]) ++
+	[{?eh,tc_done,{ct_cth_prio_SUITE,init_per_suite,ok}},
+	 
+
+	 [{?eh,tc_start,{ct_cth_prio_SUITE,{init_per_group,'_',[]}}}] ++
+	     GenPre(pre_init_per_group,
+		    [[1100,100],[600,200],[600,600],[700],[800],
+		     [900],[1000],[1200,1050],[1100],[1200]]) ++
+	     GenPost(post_init_per_group,
+		     [[1100,100],[600,200],[600,600],[600],[700],[800],
+		      [900],[900,900],[500,900],[1000],[1200,1050],
+		      [1100],[1200]]) ++
+	     [{?eh,tc_done,{ct_cth_prio_SUITE,{init_per_group,'_',[]},ok}}] ++
+	 
+	     [{?eh,tc_start,{ct_cth_prio_SUITE,test_case}}] ++
+	     GenPre(pre_init_per_testcase,
+		    [[1100,100],[600,200],[600,600],[600],[700],[800],
+		     [900],[900,900],[500,900],[1000],[1200,1050],
+		     [1100],[1200]]) ++
+	     GenPost(post_end_per_testcase,
+		     [[1100,100],[600,200],[600,600],[600],[700],[800],
+		      [900],[900,900],[500,900],[1000],[1200,1050],
+		      [1100],[1200]]) ++
+	     [{?eh,tc_done,{ct_cth_prio_SUITE,test_case,ok}},
+
+	      {?eh,tc_start,{ct_cth_prio_SUITE,{end_per_group,'_',[]}}}] ++
+	     GenPre(pre_end_per_group, 
+		    [[1100,100],[600,200],[600,600],[600],[700],[800],
+		     [900],[900,900],[500,900],[1000],[1200,1050],
+		     [1100],[1200]]) ++
+	     GenPost(post_end_per_group,
+		     [[1100,100],[600,200],[600,600],[600],[700],[800],
+		      [900],[900,900],[500,900],[1000],[1200,1050],
+		      [1100],[1200]]) ++
+	     [{?eh,tc_done,{ct_cth_prio_SUITE,{end_per_group,'_',[]},ok}}],
+
+	 {?eh,tc_start,{ct_cth_prio_SUITE,end_per_suite}}] ++
+	GenPre(pre_end_per_suite,
+	       [[1100,100],[600,200],[600,600],[700],[800],[900],[1000],
+		[1200,1050],[1100],[1200]]) ++
+	GenPost(post_end_per_suite,
+		[[1100,100],[600,200],[600,600],[700],[800],[900],[1000],
+		[1200,1050],[1100],[1200]]) ++
+	[{?eh,tc_done,{ct_cth_prio_SUITE,end_per_suite,ok}},
+	 {?eh,test_done,{'DEF','STOP_TIME'}},
+	 {?eh,stop_logging,[]}];
 
 test_events(ok) ->
     ok.
diff --git a/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/ct_cth_prio_SUITE.erl b/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/ct_cth_prio_SUITE.erl
index 0ba18b453e..d564398cd0 100644
--- a/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/ct_cth_prio_SUITE.erl
+++ b/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/ct_cth_prio_SUITE.erl
@@ -26,17 +26,21 @@
 

 suite() ->

     ([{timetrap, {minutes, 10}},

-      {ct_hooks, [{empty_cth,[800],800}]}]).

+      {ct_hooks, [{empty_cth,[800],800},

+		  {prio_cth,[1200]},{prio_cth,[1200,1050],1050}]}]).

 

 %% Test server callback functions

 init_per_suite(Config) ->

-    [{ct_hooks, [{empty_cth,[700],700}]}|Config].

+    [{ct_hooks, [{empty_cth,[700],700},

+		 {prio_cth,[600,600]},

+		 {prio_cth,[600,200],200}]}|Config].

 

 end_per_suite(_Config) ->

     ok.

 

 init_per_group(_G, Config) ->

-    [{ct_hooks, [{empty_cth,[600],600}]}|Config].

+    [{ct_hooks, [{empty_cth,[600],600},

+		 {prio_cth,[900,900]},{prio_cth,[500,900],900}]}|Config].

 

 end_per_group(_G, _Config) ->

     ok.

diff --git a/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/prio_cth.erl b/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/prio_cth.erl
new file mode 100644
index 0000000000..82511ab0d3
--- /dev/null
+++ b/lib/common_test/test/ct_hooks_SUITE_data/cth/tests/prio_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(prio_cth).

+

+

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

+

+

+%% CT Hooks

+-compile(export_all).

+

+id(Opts) ->

+    empty_cth:id(Opts).

+

+init(Id, Opts) ->

+    {ok, [Prio|_] = State} = empty_cth:init(Id, Opts),

+    {ok, State, Prio}.

+

+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).

+

+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).

+

+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).