Merge branch 'lukas/common_test/reverse_hook_order/OTP-9774'

* lukas/common_test/reverse_hook_order/OTP-9774:
  Reverse hook order for *_end_per_* hooks

3 files changed, 38 insertions, 19 deletions

diff --git a/lib/common_test/doc/src/ct_hooks_chapter.xml b/lib/common_test/doc/src/ct_hooks_chapter.xml
index bac0c4eaa8..8505ee8469 100644
--- a/lib/common_test/doc/src/ct_hooks_chapter.xml
+++ b/lib/common_test/doc/src/ct_hooks_chapter.xml
@@ -108,9 +108,10 @@
     </section>
    
     <section>
-      <title>CTH Priority</title>
+      <title>CTH Execution order</title>
       <p>By default each CTH installed will be executed in the order which
-      they are installed. This is not always wanted so common_test allows 
+      they are installed for init calls, and then reversed for end calls.
+      This is not always wanted so common_test allows
       the user to specify a priority for each hook. The priority can either
       be specified in the CTH <seealso marker="ct_hooks#Module:init-2">init/2
       </seealso> function or when installing the hook. The priority given at
diff --git a/lib/common_test/src/ct_hooks.erl b/lib/common_test/src/ct_hooks.erl
index ffafc582cf..c42adbbdd9 100644
--- a/lib/common_test/src/ct_hooks.erl
+++ b/lib/common_test/src/ct_hooks.erl
@@ -178,8 +178,9 @@ call_generic(#ct_hook_config{ module = Mod, state = State} = Hook,
 call(Fun, Config, Meta) ->
     maybe_lock(),
     Hooks = get_hooks(),
-    Res = call(get_new_hooks(Config, Fun) ++
-		   [{HookId,Fun} || #ct_hook_config{id = HookId} <- Hooks],
+    Calls = get_new_hooks(Config, Fun) ++
+	[{HookId,Fun} || #ct_hook_config{id = HookId} <- Hooks],
+    Res = call(resort(Calls,Hooks,Meta),
 	       remove(?config_name,Config), Meta, Hooks),
     maybe_unlock(),
     Res.
@@ -205,7 +206,7 @@ call([{Hook, call_id, NextFun} | Rest], Config, Meta, Hooks) ->
 		    {Hooks ++ [NewHook],
 		     [{NewId, call_init}, {NewId,NextFun} | Rest]}
 	    end,
-	call(resort(NewRest,NewHooks), Config, Meta, NewHooks)
+	call(resort(NewRest,NewHooks,Meta), Config, Meta, NewHooks)
     catch Error:Reason ->
 	    Trace = erlang:get_stacktrace(),
 	    ct_logs:log("Suite Hook","Failed to start a CTH: ~p:~p",
@@ -221,7 +222,7 @@ call([{HookId, Fun} | Rest], Config, Meta, Hooks) ->
         {NewConf, NewHook} =  Fun(Hook, Config, Meta),
         NewCalls = get_new_hooks(NewConf, Fun),
         NewHooks = lists:keyreplace(HookId, #ct_hook_config.id, Hooks, NewHook),
-        call(resort(NewCalls ++ Rest,NewHooks), %% Resort if call_init changed prio
+        call(resort(NewCalls ++ Rest,NewHooks,Meta), %% 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} ->
@@ -308,6 +309,18 @@ get_hooks() ->
 %% call_id < call_init < ctfirst < Priority 1 < .. < Priority N < ctlast
 %% If Hook Priority is equal, check when it has been installed and
 %% sort on that instead.
+%% If we are doing a cleanup call i.e. {post,pre}_end_per_*, all priorities
+%% are reversed. Probably want to make this sorting algorithm pluginable
+%% as some point...
+resort(Calls,Hooks,[F|_R]) when F == post_end_per_testcase;
+				F == pre_end_per_group;
+				F == post_end_per_group;
+				F == pre_end_per_suite;
+				F == post_end_per_suite ->
+    lists:reverse(resort(Calls,Hooks));
+resort(Calls,Hooks,_Meta) ->
+    resort(Calls,Hooks).
+    
 resort(Calls, Hooks) ->
     lists:sort(
       fun({_,_,_},_) ->
diff --git a/lib/common_test/test/ct_hooks_SUITE.erl b/lib/common_test/test/ct_hooks_SUITE.erl
index 5c99f0f9f7..2c519f08b5 100644
--- a/lib/common_test/test/ct_hooks_SUITE.erl
+++ b/lib/common_test/test/ct_hooks_SUITE.erl
@@ -1046,29 +1046,34 @@ test_events(prio_cth) ->
 		     [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]]) ++
+		     lists:reverse(
+		       [[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]]) ++
+		    lists:reverse(
+		      [[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]]) ++
+		     lists:reverse(
+		       [[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]]) ++
+	       lists:reverse(
+		 [[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]]) ++
+		lists:reverse(
+		  [[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,[]}];