Kuba/ct/ct hooks order option/otp 18682

lib/common_test/doc/src/ct_hooks_chapter.xml

@@ -118,14 +118,32 @@
     <section>
       <marker id="cth_execution_order"/>
       <title>CTH Execution Order</title>
-      <p>By default, each CTH installed is executed in the order that
-      they are installed for init calls, and then reversed for end calls.
-      This is not always desired, so <c>Common Test</c> allows
-      the user to specify a priority for each hook. The priority can either
-      be specified in the CTH function 
-      <seemfa marker="ct_hooks#Module:init/2">init/2</seemfa> or when 
-      installing the hook. The priority specified at installation overrides the 
-      priority returned by the CTH.</p>
+      <p>By default, each installed CTH is executed in the order in which
+      they are installed for init calls, and then reversed for end
+      calls. This order can be referred to as test-centric, as the order is
+      reversed after a testcase is executed and corresponds to the default
+      value (<c>test</c>) of <c>ct_hooks_order</c> option.</p>
+      <p>The installation-based order is not always
+      desired, so <c>Common Test</c> allows the user to specify a
+      priority for each hook. The priority can be specified in
+      the CTH function <seemfa
+      marker="ct_hooks#Module:init/2">init/2</seemfa> or when
+      installing the hook. The priority specified at installation
+      overrides the priority returned by the CTH.</p>
+      <p>In some cases, the reversed order for all end calls is not
+      desired, and instead, the user might prefer the reversed order
+      for post hook calls. Such behavior can be enabled with
+      <c>ct_hooks_order</c> option with <c>config</c> value. When this
+      option is enabled, the execution order is configuration-centric, as
+      the reversed order happens after each configuration function and
+      not in relation to testcase.</p>
+      <p>Note that the <c>ct_hooks_order</c> option is considered as a
+      global framework setting. In case when option is configured
+      multiple times framework with process only the first value.</p>
+      <p>The <c>ct_hooks_order</c> option can be set as: <c>ct_run</c>
+      argument, in test specification or <seemfa
+      marker="ct_suite#Module:suite/0">suite/0</seemfa> return
+      value.</p>
     </section>
   </section>
 

lib/common_test/doc/src/ct_run_cmd.xml

@@ -128,6 +128,7 @@
   [-keep_logs all | NLogs]
   [-ct_hooks CTHModule1 CTHOpts1 and CTHModule2 CTHOpts2 and ..
    CTHModuleN CTHOptsN]
+  [-ct_hooks_order test | config]
   [-exit_status ignore_config]
   [-help]</pre>
   </section>
@@ -168,6 +169,7 @@
   [-keep_logs all | NLogs]
   [-ct_hooks CTHModule1 CTHOpts1 and CTHModule2 CTHOpts2 and ..
    CTHModuleN CTHOptsN]
+  [-ct_hooks_order test | config]
   [-exit_status ignore_config]</pre>
   </section>
 

lib/common_test/doc/src/ct_suite.xml

@@ -161,7 +161,7 @@
 	<fsummary>Test suite info function (providing default data
 	  for the suite).</fsummary>
 	<type>
-	  <v><seetype marker="#ct_info">ct_info()</seetype> = {timetrap, Time} | {require, Required} | {require, Name, Required} | {userdata, UserData} | {silent_connections, Conns} | {stylesheet, CSSFile} | {ct_hooks, CTHs}</v>
+	  <v><seetype marker="#ct_info">ct_info()</seetype> = {timetrap, Time} | {require, Required} | {require, Name, Required} | {userdata, UserData} | {silent_connections, Conns} | {stylesheet, CSSFile} | {ct_hooks, CTHs} | {ct_hooks_order, CTHOrder}</v>
 	  <v>Time = TimeVal | TimeFunc</v>
 	  <v>TimeVal = MilliSec | {seconds, integer()} | {minutes, integer()} | {hours, integer()}</v>
 	  <v>TimeFunc = {Mod, Func, Args} | Fun</v>
@@ -184,6 +184,7 @@
 	  <v>CTHModule = atom()</v>
 	  <v>CTHInitArgs = term()</v>
 	  <v>CTHPriority = integer()</v>
+          <v>CTHOrder = test | config</v>
 	</type>
 	<desc>
 

lib/common_test/doc/src/run_test_chapter.xml

@@ -210,6 +210,11 @@
         <seeguide marker="ct_hooks_chapter#installing">Common Test Hooks</seeguide> 
       including start arguments.</p></item>
 
+      <tag><c><![CDATA[-ct_hooks_order [test|config]]]></c></tag>
+      <item><p>To modify
+        <seeguide marker="ct_hooks_chapter#installing">Common Test Hooks</seeguide>
+      execution order.</p></item>
+
       <tag><c><![CDATA[-enable_builtin_hooks <bool>]]></c></tag>
       <item><p>To enable or disable
         <seeguide marker="ct_hooks_chapter#builtin_cths">Built-in Common Test Hooks</seeguide>. 
@@ -883,6 +888,8 @@
  {ct_hooks, CTHModules}.
  {ct_hooks, NodeRefs, CTHModules}.
 
+ {ct_hooks_order, CTHOrder}.
+
  {enable_builtin_hooks, Bool}.
 
  {basic_html, Bool}.
@@ -952,6 +959,7 @@
 		    {CTHModule, CTHInitArgs, CTHPriority}]
  CTHModule       = atom()
  CTHInitArgs     = term()
+ CTHOrder        = test | config
  Dir             = string()
  Suites          = atom() | [atom()] | all
  Suite           = atom()

lib/common_test/internal_doc/ct_notes.md

@@ -5,10 +5,7 @@ I think the most confusing thing is that today OTP behavior and design seems to
 1. (Configuration centric) CT hook callback looks as designed to wrap around CT Configuration functions (i.e. you have *pre* and *post* to wrapp around init_per_testcase or end_per_testcase)
    - Furthermore if you consider hook callback function names, there are no hooks wrapping around Testcase function at all!
 2. (Testcase centric) AND at the same the hook execution order is determined by relation to CT Testcase callback
-### Next step ideas
-1. improve existing documentation for hooks (actually it was planned for many years but down prioritized)
-2. add mermaid diagrams to docs when that is possible
-3. introduce a CT option for Configuration centric hook execution order (maybe named ct_hooks_order := [testcase(default) | configuration])
+
 ### CT hooks priorities (documentation sketch)
 Let's assume:
 1. cth_A and cth_B being CT hook modules to be installed
@@ -37,29 +34,31 @@ title: Testcase centric CT hook execution order (default)
 ---
 flowchart TD
     subgraph hooks
-    pre_init_pt_A["(A) pre_init_per_testcase"] --> pre_init_pt_B
+    pre_ipt_A["(A) pre_init_per_testcase"] --Config--> pre_ipt_B
     end
     subgraph suite
-    pre_init_pt_B["(B) pre_init_per_testcase"] --> init_pt[/"init_per_testcase"/]
+    pre_ipt_B["(B) pre_init_per_testcase"] --Config--> ipt[/"init_per_testcase"/]
     end
-    init_pt --> post_init_pt_A
+    ipt --Config,Return--> post_ipt_A
+    ipt --Config--> post_ipt_B
     subgraph hooks
-    post_init_pt_A["(A) post_init_per_testcase"] --> post_init_pt_B
+    post_ipt_A["(A) post_init_per_testcase"] --Return--> post_ipt_B
     end
     subgraph suite
-    post_init_pt_B["(B) post_init_per_testcase"] --> testcase
+    post_ipt_B["(B) post_init_per_testcase"] --Config--> testcase
     testcase((("Testcase")))
     end
     subgraph hooks
-    testcase --> pre_end_pt_B
-    pre_end_pt_B["(B) pre_end_per_testcase"] --> pre_end_pt_A
+    testcase --tc_status--> pre_ept_B
+    pre_ept_B["(B) pre_end_per_testcase"] --Config--> pre_ept_A
     end
     subgraph suite
-    pre_end_pt_A["(A) pre_end_per_testcase"] --> end_per_test_case
+    pre_ept_A["(A) pre_end_per_testcase"] --Config--> end_per_test_case
     end
     subgraph hooks
-    end_per_test_case[/"end_per_testcase"/] --> post_end_pt_B
-    post_end_pt_B["(B) post_end_per_testcase"] --> post_end_pt_A["(A) post_end_per_testcase"]
+    end_per_test_case[/"end_per_testcase"/] --Config,Return--> post_ept_B
+    post_ept_B[/"(B) post_end_per_testcase"/] --Return--> post_ept_A[/"(A) post_end_per_testcase"/]
+    end_per_test_case --Config--> post_ept_A
     end
 ```
 #### Configuration centric (option candidate)
@@ -76,29 +75,29 @@ title: Configuration centric CT hook execution order (option)
 ---
 flowchart TD
     subgraph hooks
-    pre_init_pt_A["(A) pre_init_per_testcase"] --> pre_init_pt_B
+    pre_ipt_A["(A) pre_init_per_testcase"] --> pre_ipt_B
     end
     subgraph suite
-    pre_init_pt_B["(B) pre_init_per_testcase"] --> init_pt((("init_per_testcase")))
+    pre_ipt_B["(B) pre_init_per_testcase"] --> ipt((("init_per_testcase")))
     end
-    init_pt --> post_init_pt_B
+    ipt --> post_ipt_B
     subgraph hooks
-    post_init_pt_B["(B) post_init_per_testcase"] --> post_init_pt_A
+    post_ipt_B["(B) post_init_per_testcase"] --> post_ipt_A
     end
     subgraph suite
-    post_init_pt_A["(A) post_init_per_testcase"] --> testcase
+    post_ipt_A["(A) post_init_per_testcase"] --> testcase
     testcase[/"Testcase"/]
     end
     subgraph hooks
-    testcase --> pre_end_pt_A
-    pre_end_pt_A["(A) pre_end_per_testcase"] --> pre_end_pt_B
+    testcase --> pre_ept_A
+    pre_ept_A["(A) pre_end_per_testcase"] --> pre_ept_B
     end
     subgraph suite
-    pre_end_pt_B["(B) pre_end_per_testcase"] --> end_per_test_case
+    pre_ept_B["(B) pre_end_per_testcase"] --> end_per_test_case
     end
     subgraph hooks
-    end_per_test_case((("end_per_testcase"))) --> post_end_pt_B
-    post_end_pt_B["(B) post_end_per_testcase"] --> post_end_pt_A["(A) post_end_per_testcase"]
+    end_per_test_case((("end_per_testcase"))) --> post_ept_B
+    post_ept_B["(B) post_end_per_testcase"] --> post_ept_A["(A) post_end_per_testcase"]
     end
 ```
 

lib/common_test/src/ct.erl

@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2003-2022. All Rights Reserved.
+%% Copyright Ericsson AB 2003-2023. All Rights Reserved.
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
 %% you may not use this file except in compliance with the License.
@@ -170,6 +170,7 @@ run(TestDirs) ->
                       | {esc_chars, boolean()}
                       | {keep_logs,KeepSpec}
                       | {ct_hooks, CTHs}
+                      | {ct_hooks_order, CTHsOrder}
                       | {enable_builtin_hooks, boolean()}
                       | {release_shell, boolean()},
            TestDirs :: [string()] | string(),
@@ -211,6 +212,7 @@ run(TestDirs) ->
            Category :: atom(),
            KeepSpec :: all | pos_integer(),
            CTHs :: [CTHModule | {CTHModule, CTHInitArgs}],
+           CTHsOrder :: atom(),
            CTHModule :: atom(),
            CTHInitArgs :: term(),
            Result :: {Ok, Failed, {UserSkipped, AutoSkipped}} | TestRunnerPid | {error, Reason},

lib/common_test/src/ct_framework.erl

@@ -606,6 +606,8 @@ configure([{timetrap,Time}|Rest],Info,SuiteInfo,Scope,PostInitHook,Config) ->
     configure(Rest,Info,SuiteInfo,Scope,PostInitHook1,Config);
 configure([{ct_hooks,Hook}|Rest],Info,SuiteInfo,Scope,PostInitHook,Config) ->
     configure(Rest,Info,SuiteInfo,Scope,PostInitHook,[{ct_hooks,Hook}|Config]);
+configure([{ct_hooks_order,Order}|Rest],Info,SuiteInfo,Scope,PostInitHook,Config) ->
+    configure(Rest,Info,SuiteInfo,Scope,PostInitHook,[{ct_hooks_order,Order}|Config]);
 configure([_|Rest],Info,SuiteInfo,Scope,PostInitHook,Config) ->
     configure(Rest,Info,SuiteInfo,Scope,PostInitHook,Config);
 configure([],_,_,_,PostInitHook,Config) ->