From f57d16ba13a08ef19ef3e104e2770f032aa70ea3 Mon Sep 17 00:00:00 2001
From: Andrew Mayorov <encube.ul@gmail.com>
Date: Tue, 29 Aug 2023 21:12:54 +0400
Subject: [PATCH 1/3] feat(cthsuite): add function to determine workdir of
 testrun

In a deterministic fashion, to lift the burden of undestanding where
the testrun's data should go from the test writer.
---
 apps/emqx/integration_test/emqx_ds_SUITE.erl  |  6 +--
 apps/emqx/test/emqx_cth_suite.erl             | 53 ++++++++++++++++---
 apps/emqx/test/emqx_flapping_SUITE.erl        |  2 +-
 .../test/emqx_persistent_messages_SUITE.erl   |  3 +-
 .../emqx_authz/test/emqx_authz_file_SUITE.erl |  2 +-
 .../test/emqx_authz_rich_actions_SUITE.erl    |  2 +-
 .../test/emqx_bridge_api_SUITE.erl            | 16 +++---
 apps/emqx_ft/test/emqx_ft_SUITE.erl           |  2 +-
 apps/emqx_ft/test/emqx_ft_conf_SUITE.erl      |  3 +-
 .../emqx_ft/test/emqx_ft_storage_fs_SUITE.erl |  3 +-
 .../test/emqx_ft_storage_fs_gc_SUITE.erl      |  2 +-
 11 files changed, 64 insertions(+), 30 deletions(-)

diff --git a/apps/emqx/integration_test/emqx_ds_SUITE.erl b/apps/emqx/integration_test/emqx_ds_SUITE.erl
index 842782e35..3f0d3f3e4 100644
--- a/apps/emqx/integration_test/emqx_ds_SUITE.erl
+++ b/apps/emqx/integration_test/emqx_ds_SUITE.erl
@@ -22,7 +22,7 @@ all() ->
 init_per_suite(Config) ->
     TCApps = emqx_cth_suite:start(
         app_specs(),
-        #{work_dir => ?config(priv_dir, Config)}
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
     ),
     [{tc_apps, TCApps} | Config].
 
@@ -31,9 +31,9 @@ end_per_suite(Config) ->
     emqx_cth_suite:stop(TCApps),
     ok.
 
-init_per_testcase(t_session_subscription_idempotency, Config) ->
+init_per_testcase(t_session_subscription_idempotency = TC, Config) ->
     Cluster = cluster(#{n => 1}),
-    ClusterOpts = #{work_dir => ?config(priv_dir, Config)},
+    ClusterOpts = #{work_dir => emqx_cth_suite:work_dir(TC, Config)},
     NodeSpecs = emqx_cth_cluster:mk_nodespecs(Cluster, ClusterOpts),
     Nodes = emqx_cth_cluster:start(Cluster, ClusterOpts),
     [
diff --git a/apps/emqx/test/emqx_cth_suite.erl b/apps/emqx/test/emqx_cth_suite.erl
index 80b3a578c..b70245e9d 100644
--- a/apps/emqx/test/emqx_cth_suite.erl
+++ b/apps/emqx/test/emqx_cth_suite.erl
@@ -22,6 +22,9 @@
 -export([start/2]).
 -export([stop/1]).
 
+-export([work_dir/1]).
+-export([work_dir/2]).
+
 -export([load_apps/1]).
 -export([start_apps/2]).
 -export([start_app/2]).
@@ -98,16 +101,11 @@ when
     SuiteOpts :: #{
         %% Working directory
         %% Everything a test produces should go here. If this directory is not empty,
-        %% function will raise an error.
+        %% function will raise an error. Most of the time, the result of `work_dir/1`
+        %% or `work_dir/2` (if used in a testcase) should be fine here.
         work_dir := file:name()
     }.
-start(Apps, SuiteOpts0 = #{work_dir := WorkDir0}) ->
-    %% when running CT on the whole app, it seems like `priv_dir` is the same on all
-    %% suites and leads to the "clean slate" verificatin to fail.
-    WorkDir = binary_to_list(
-        filename:join([WorkDir0, emqx_guid:to_hexstr(emqx_guid:gen())])
-    ),
-    SuiteOpts = SuiteOpts0#{work_dir := WorkDir},
+start(Apps, SuiteOpts = #{work_dir := WorkDir}) ->
     % 1. Prepare appspec instructions
     AppSpecs = [mk_appspec(App, SuiteOpts) || App <- Apps],
     % 2. Load every app so that stuff scanning attributes of loaded modules works
@@ -339,6 +337,45 @@ default_config(App, SuiteOpts) ->
 
 %%
 
+%% @doc Determine the unique work directory for the current test run.
+%% Takes into account name of the test suite, and all test groups the current run
+%% is part of.
+-spec work_dir(CTConfig :: proplists:proplist()) ->
+    file:filename_all().
+work_dir(CTConfig) ->
+    % Directory specific to the current test run.
+    [PrivDir] = proplists:get_all_values(priv_dir, CTConfig),
+    % Directory specific to the currently executing test suite.
+    [DataDir] = proplists:get_all_values(data_dir, CTConfig),
+    % NOTE: Contains the name of the current test group, if executed as part of a group.
+    GroupProps = proplists:get_value(tc_group_properties, CTConfig, []),
+    % NOTE: Contains names of outer test groups, if any.
+    GroupPathOuter = proplists:get_value(tc_group_path, CTConfig, []),
+    SuiteDir = filename:basename(DataDir),
+    GroupPath = lists:append([GroupProps | GroupPathOuter]),
+    GroupLevels = [atom_to_list(Name) || {name, Name} <- GroupPath],
+    WorkDir1 = filename:join(PrivDir, SuiteDir),
+    WorkDir2 =
+        case GroupLevels of
+            [] ->
+                WorkDir1;
+            [_ | _] ->
+                GroupDir = string:join(lists:reverse(GroupLevels), "."),
+                filename:join(WorkDir1, GroupDir)
+        end,
+    WorkDir2.
+
+%% @doc Determine the unique work directory for the current testcase run.
+%% Be careful when testcase runs under no groups, and its name matches the name of a
+%% previously executed test group, it's best to avoid such naming.
+-spec work_dir(TestCaseName :: atom(), CTConfig :: proplists:proplist()) ->
+    file:filename_all().
+work_dir(TCName, CTConfig) ->
+    WorkDir = work_dir(CTConfig),
+    filename:join(WorkDir, TCName).
+
+%%
+
 start_ekka() ->
     ok = emqx_common_test_helpers:start_ekka(),
     {ok, [mnesia, ekka]}.
diff --git a/apps/emqx/test/emqx_flapping_SUITE.erl b/apps/emqx/test/emqx_flapping_SUITE.erl
index 6204d9b6d..021eaddbf 100644
--- a/apps/emqx/test/emqx_flapping_SUITE.erl
+++ b/apps/emqx/test/emqx_flapping_SUITE.erl
@@ -35,7 +35,7 @@ init_per_suite(Config) ->
                 "\n ban_time = 2s"
                 "\n }"}
         ],
-        #{work_dir => ?config(priv_dir, Config)}
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
     ),
     [{suite_apps, Apps} | Config].
 
diff --git a/apps/emqx/test/emqx_persistent_messages_SUITE.erl b/apps/emqx/test/emqx_persistent_messages_SUITE.erl
index db22b19e6..c4f7ef73b 100644
--- a/apps/emqx/test/emqx_persistent_messages_SUITE.erl
+++ b/apps/emqx/test/emqx_persistent_messages_SUITE.erl
@@ -33,10 +33,9 @@ init_per_suite(Config) ->
     %% TODO: remove after other suites start to use `emx_cth_suite'
     application:stop(emqx),
     application:stop(emqx_durable_storage),
-    WorkDir = ?config(priv_dir, Config),
     TCApps = emqx_cth_suite:start(
         app_specs(),
-        #{work_dir => WorkDir}
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
     ),
     [{tc_apps, TCApps} | Config].
 
diff --git a/apps/emqx_authz/test/emqx_authz_file_SUITE.erl b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl
index 396679783..d31935363 100644
--- a/apps/emqx_authz/test/emqx_authz_file_SUITE.erl
+++ b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl
@@ -44,7 +44,7 @@ init_per_testcase(TestCase, Config) ->
             {emqx_conf, "authorization.no_match = deny, authorization.cache.enable = false"},
             emqx_authz
         ],
-        #{work_dir => filename:join(?config(priv_dir, Config), TestCase)}
+        #{work_dir => emqx_cth_suite:work_dir(TestCase, Config)}
     ),
     [{tc_apps, Apps} | Config].
 
diff --git a/apps/emqx_authz/test/emqx_authz_rich_actions_SUITE.erl b/apps/emqx_authz/test/emqx_authz_rich_actions_SUITE.erl
index 8d24b5472..fc597f15b 100644
--- a/apps/emqx_authz/test/emqx_authz_rich_actions_SUITE.erl
+++ b/apps/emqx_authz/test/emqx_authz_rich_actions_SUITE.erl
@@ -37,7 +37,7 @@ init_per_testcase(TestCase, Config) ->
             {emqx_conf, "authorization.no_match = deny, authorization.cache.enable = false"},
             emqx_authz
         ],
-        #{work_dir => filename:join(?config(priv_dir, Config), TestCase)}
+        #{work_dir => emqx_cth_suite:work_dir(TestCase, Config)}
     ),
     [{tc_apps, Apps} | Config].
 
diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl
index d8e697987..d08953682 100644
--- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl
+++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl
@@ -116,13 +116,13 @@ end_per_suite(_Config) ->
     ok.
 
 init_per_group(cluster = Name, Config) ->
-    Nodes = [NodePrimary | _] = mk_cluster(Name, Config),
+    Nodes = [NodePrimary | _] = mk_cluster(Config),
     init_api([{group, Name}, {cluster_nodes, Nodes}, {node, NodePrimary} | Config]);
 init_per_group(cluster_later_join = Name, Config) ->
-    Nodes = [NodePrimary | _] = mk_cluster(Name, Config, #{join_to => undefined}),
+    Nodes = [NodePrimary | _] = mk_cluster(Config, #{join_to => undefined}),
     init_api([{group, Name}, {cluster_nodes, Nodes}, {node, NodePrimary} | Config]);
-init_per_group(Name, Config) ->
-    WorkDir = filename:join(?config(priv_dir, Config), Name),
+init_per_group(_Name, Config) ->
+    WorkDir = emqx_cth_suite:work_dir(Config),
     Apps = emqx_cth_suite:start(?APPSPECS ++ [?APPSPEC_DASHBOARD], #{work_dir => WorkDir}),
     init_api([{group, single}, {group_apps, Apps}, {node, node()} | Config]).
 
@@ -131,10 +131,10 @@ init_api(Config) ->
     {ok, App} = erpc:call(APINode, emqx_common_test_http, create_default_app, []),
     [{api, App} | Config].
 
-mk_cluster(Name, Config) ->
-    mk_cluster(Name, Config, #{}).
+mk_cluster(Config) ->
+    mk_cluster(Config, #{}).
 
-mk_cluster(Name, Config, Opts) ->
+mk_cluster(Config, Opts) ->
     Node1Apps = ?APPSPECS ++ [?APPSPEC_DASHBOARD],
     Node2Apps = ?APPSPECS,
     emqx_cth_cluster:start(
@@ -142,7 +142,7 @@ mk_cluster(Name, Config, Opts) ->
             {emqx_bridge_api_SUITE1, Opts#{role => core, apps => Node1Apps}},
             {emqx_bridge_api_SUITE2, Opts#{role => core, apps => Node2Apps}}
         ],
-        #{work_dir => filename:join(?config(priv_dir, Config), Name)}
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
     ).
 
 end_per_group(Group, Config) when
diff --git a/apps/emqx_ft/test/emqx_ft_SUITE.erl b/apps/emqx_ft/test/emqx_ft_SUITE.erl
index 290cda333..dfee76f72 100644
--- a/apps/emqx_ft/test/emqx_ft_SUITE.erl
+++ b/apps/emqx_ft/test/emqx_ft_SUITE.erl
@@ -76,7 +76,7 @@ init_per_suite(Config) ->
         [
             {emqx_ft, #{config => emqx_ft_test_helpers:config(Storage)}}
         ],
-        #{work_dir => ?config(priv_dir, Config)}
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
     ),
     [{suite_apps, Apps} | Config].
 
diff --git a/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl b/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl
index 3fdfdf65a..0acdea213 100644
--- a/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl
+++ b/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl
@@ -32,13 +32,12 @@ end_per_suite(_Config) ->
     ok.
 
 init_per_testcase(Case, Config) ->
-    WorkDir = filename:join(?config(priv_dir, Config), Case),
     Apps = emqx_cth_suite:start(
         [
             {emqx_conf, #{}},
             {emqx_ft, #{config => "file_transfer {}"}}
         ],
-        #{work_dir => WorkDir}
+        #{work_dir => emqx_cth_suite:work_dir(Case, Config)}
     ),
     [{suite_apps, Apps} | Config].
 
diff --git a/apps/emqx_ft/test/emqx_ft_storage_fs_SUITE.erl b/apps/emqx_ft/test/emqx_ft_storage_fs_SUITE.erl
index a57cdf621..52d372e63 100644
--- a/apps/emqx_ft/test/emqx_ft_storage_fs_SUITE.erl
+++ b/apps/emqx_ft/test/emqx_ft_storage_fs_SUITE.erl
@@ -36,12 +36,11 @@ groups() ->
 
 init_per_suite(Config) ->
     Storage = emqx_ft_test_helpers:local_storage(Config),
-    WorkDir = ?config(priv_dir, Config),
     Apps = emqx_cth_suite:start(
         [
             {emqx_ft, #{config => emqx_ft_test_helpers:config(Storage)}}
         ],
-        #{work_dir => WorkDir}
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
     ),
     [{suite_apps, Apps} | Config].
 
diff --git a/apps/emqx_ft/test/emqx_ft_storage_fs_gc_SUITE.erl b/apps/emqx_ft/test/emqx_ft_storage_fs_gc_SUITE.erl
index b14fc7edd..311ad7fbd 100644
--- a/apps/emqx_ft/test/emqx_ft_storage_fs_gc_SUITE.erl
+++ b/apps/emqx_ft/test/emqx_ft_storage_fs_gc_SUITE.erl
@@ -28,7 +28,7 @@ all() ->
     emqx_common_test_helpers:all(?MODULE).
 
 init_per_suite(Config) ->
-    Apps = emqx_cth_suite:start([emqx], #{work_dir => ?config(priv_dir, Config)}),
+    Apps = emqx_cth_suite:start([emqx], #{work_dir => emqx_cth_suite:work_dir(Config)}),
     [{suite_apps, Apps} | Config].
 
 end_per_suite(Config) ->

From 326809388182ad23d22fffb437436d033b4939b1 Mon Sep 17 00:00:00 2001
From: Andrew Mayorov <encube.ul@gmail.com>
Date: Tue, 29 Aug 2023 21:16:26 +0400
Subject: [PATCH 2/3] feat(cth): add module-level documenation

---
 apps/emqx/test/emqx_cth_cluster.erl | 22 ++++++++++++++++
 apps/emqx/test/emqx_cth_suite.erl   | 41 +++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/apps/emqx/test/emqx_cth_cluster.erl b/apps/emqx/test/emqx_cth_cluster.erl
index e24600181..3f8ea9a89 100644
--- a/apps/emqx/test/emqx_cth_cluster.erl
+++ b/apps/emqx/test/emqx_cth_cluster.erl
@@ -14,6 +14,28 @@
 %% limitations under the License.
 %%--------------------------------------------------------------------
 
+%% @doc Common Test Helper / Running tests in a cluster
+%%
+%% This module allows setting up and tearing down clusters of EMQX nodes with
+%% the purpose of running integration tests in a distributed environment, but
+%% with the same isolation measures that `emqx_cth_suite` provides.
+%%
+%% Additionally to what `emqx_cth_suite` does with respect to isolation, each
+%% node in the cluster is started with a separate, unique working directory.
+%%
+%% What should be started on each node is defined by the same appspecs that are
+%% used by `emqx_cth_suite` to start applications on the CT node. However, there
+%% are additional set of defaults applied to appspecs to make sure that the
+%% cluster is started in a consistent, interconnected state, with no conflicts
+%% between applications.
+%%
+%% Most of the time, you just need to:
+%% 1. Describe the cluster with one or more _nodespecs_.
+%% 2. Call `emqx_cth_cluster:start/2` before the testrun (e.g. in `init_per_suite/1`
+%%    or `init_per_group/2`), providing unique work dir (e.g.
+%%    `emqx_cth_suite:work_dir/1`). Save the result in a context.
+%% 3. Call `emqx_cth_cluster:stop/1` after the testrun concludes (e.g.
+%%    in `end_per_suite/1` or `end_per_group/2`) with the result from step 2.
 -module(emqx_cth_cluster).
 
 -export([start/2]).
diff --git a/apps/emqx/test/emqx_cth_suite.erl b/apps/emqx/test/emqx_cth_suite.erl
index b70245e9d..090bca762 100644
--- a/apps/emqx/test/emqx_cth_suite.erl
+++ b/apps/emqx/test/emqx_cth_suite.erl
@@ -14,6 +14,47 @@
 %% limitations under the License.
 %%--------------------------------------------------------------------
 
+%% @doc Common Test Helper / Running test suites
+%%
+%% The purpose of this module is to run application-level, integration
+%% tests in an isolated fashion.
+%%
+%% Isolation is this context means that each testrun does not leave any
+%% persistent state accessible to following testruns. The goal is to
+%% make testruns completely independent of each other, of the order in
+%% which they are executed, and of the testrun granularity, i.e. whether
+%% they are executed individually or as part of a larger suite. This
+%% should help to increase reproducibility and reduce the risk of false
+%% positives.
+%%
+%% Isolation is achieved through the following measures:
+%% * Each testrun completely terminates and unload all applications
+%%   started during the testrun.
+%% * Each testrun is executed in a separate directory, usually under
+%%   common_test's private directory, where all persistent state should
+%%   be stored.
+%% * Additionally, each cleans out few bits of persistent state that
+%%   survives the above measures, namely persistent VM terms related
+%%   to configuration and authentication (see `clean_suite_state/0`).
+%%
+%% Integration test in this context means a test that works with applications
+%% as a whole, and needs to start and stop them as part of the test run.
+%% For this, there's an abstraction called _appspec_ that describes how to
+%% configure and start an application.
+%%
+%% The module also provides a set of default appspecs for some applications
+%% that hide details and quirks of how to start them, to make it easier to
+%% write test suites.
+%%
+%% Most of the time, you just need to:
+%% 1. Describe the appspecs for the applications you want to test.
+%% 2. Call `emqx_cth_sutie:start/2` to start the applications before the testrun
+%%    (e.g. in `init_per_suite/1` / `init_per_group/2`), providing the appspecs
+%%    and unique work dir for the testrun (e.g. `work_dir/1`). Save the result
+%%    in a context.
+%% 3. Call `emqx_cth_sutie:stop/1` to stop the applications after the testrun
+%%    finishes (e.g. in `end_per_suite/1` / `end_per_group/2`), providing the
+%%    result from step 2.
 -module(emqx_cth_suite).
 
 -include_lib("common_test/include/ct.hrl").

From 0e770bdc95fc22e99227ebee777eb458a39a68db Mon Sep 17 00:00:00 2001
From: Andrew Mayorov <encube.ul@gmail.com>
Date: Tue, 29 Aug 2023 21:17:07 +0400
Subject: [PATCH 3/3] test: switch `emqx_broker_SUITE` to use new cth tooling

---
 apps/emqx/test/emqx_broker_SUITE.erl | 47 ++++++++++++++++++----------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/apps/emqx/test/emqx_broker_SUITE.erl b/apps/emqx/test/emqx_broker_SUITE.erl
index 6e03971a5..52cf230ff 100644
--- a/apps/emqx/test/emqx_broker_SUITE.erl
+++ b/apps/emqx/test/emqx_broker_SUITE.erl
@@ -58,39 +58,54 @@ groups() ->
 init_per_group(connected_client_count_group, Config) ->
     Config;
 init_per_group(tcp, Config) ->
-    emqx_common_test_helpers:boot_modules(all),
-    emqx_common_test_helpers:start_apps([]),
-    [{conn_fun, connect} | Config];
+    Apps = emqx_cth_suite:start(
+        [emqx],
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
+    ),
+    [{conn_fun, connect}, {group_apps, Apps} | Config];
 init_per_group(ws, Config) ->
-    emqx_common_test_helpers:boot_modules(all),
-    emqx_common_test_helpers:start_apps([]),
+    Apps = emqx_cth_suite:start(
+        [emqx],
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
+    ),
     [
         {ssl, false},
         {enable_websocket, true},
         {conn_fun, ws_connect},
         {port, 8083},
-        {host, "localhost"}
+        {host, "localhost"},
+        {group_apps, Apps}
         | Config
     ];
 init_per_group(quic, Config) ->
-    emqx_common_test_helpers:boot_modules(all),
-    emqx_common_test_helpers:start_apps([]),
-    UdpPort = 14567,
-    ok = emqx_common_test_helpers:ensure_quic_listener(?MODULE, UdpPort),
+    Apps = emqx_cth_suite:start(
+        [
+            {emqx,
+                "listeners.quic.test {"
+                "\n enable = true"
+                "\n max_connections = 1024000"
+                "\n idle_timeout = 15s"
+                "\n }"}
+        ],
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
+    ),
     [
         {conn_fun, quic_connect},
-        {port, UdpPort}
+        {port, emqx_config:get([listeners, quic, test, bind])},
+        {group_apps, Apps}
         | Config
     ];
 init_per_group(_Group, Config) ->
-    emqx_common_test_helpers:boot_modules(all),
-    emqx_common_test_helpers:start_apps([]),
-    Config.
+    Apps = emqx_cth_suite:start(
+        [emqx],
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
+    ),
+    [{group_apps, Apps} | Config].
 
 end_per_group(connected_client_count_group, _Config) ->
     ok;
-end_per_group(_Group, _Config) ->
-    emqx_common_test_helpers:stop_apps([]).
+end_per_group(_Group, Config) ->
+    emqx_cth_suite:stop(?config(group_apps, Config)).
 
 init_per_suite(Config) ->
     Config.