From b0af59193089abe4da8ae635840938d0130b1265 Mon Sep 17 00:00:00 2001
From: Wu Tao <wutao163.com@163.com>
Date: Tue, 24 Dec 2019 15:44:45 +0800
Subject: [PATCH] refactor: move load balance related tests under another test
 suite (#365)

---
 .../meta_server/meta_backup_service.cpp       |  4 +-
 .../meta_server/meta_backup_service.h         |  8 +--
 .../replication/meta_server/meta_service.h    |  4 +-
 .../replication/meta_server/server_state.h    |  5 +-
 .../test/meta_test/unit_test/backup_test.cpp  |  7 ++-
 .../unit_test/balancer_validator.cpp          |  6 +-
 .../unit_test/data_definition_test.cpp        |  6 +-
 .../meta_test/unit_test/json_compacity.cpp    |  6 ++
 .../test/meta_test/unit_test/main.cpp         | 23 ++++----
 ...re_test.cpp => meta_load_balance_test.cpp} | 57 ++++++++++++++++---
 .../unit_test/meta_service_test_app.h         | 11 ++--
 .../meta_test/unit_test/server_state_test.cpp |  5 +-
 .../meta_test/unit_test/state_sync_test.cpp   |  5 +-
 .../unit_test/update_configuration_test.cpp   |  5 +-
 14 files changed, 105 insertions(+), 47 deletions(-)
 rename src/dist/replication/test/meta_test/unit_test/{simple_lb_cure_test.cpp => meta_load_balance_test.cpp} (96%)

diff --git a/src/dist/replication/meta_server/meta_backup_service.cpp b/src/dist/replication/meta_server/meta_backup_service.cpp
index 0736353f40..5669bc8c77 100644
--- a/src/dist/replication/meta_server/meta_backup_service.cpp
+++ b/src/dist/replication/meta_server/meta_backup_service.cpp
@@ -1593,5 +1593,5 @@ std::string backup_service::get_backup_path(const std::string &policy_name, int6
     ss << _policy_meta_root << "/" << policy_name << "/" << backup_id;
     return ss.str();
 }
-}
-}
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/meta_server/meta_backup_service.h b/src/dist/replication/meta_server/meta_backup_service.h
index 4a386c7f22..0944cbfbdb 100644
--- a/src/dist/replication/meta_server/meta_backup_service.h
+++ b/src/dist/replication/meta_server/meta_backup_service.h
@@ -11,8 +11,6 @@
 
 #include "meta_data.h"
 
-class meta_service_test_app;
-
 namespace dsn {
 namespace replication {
 
@@ -352,7 +350,7 @@ class backup_service
     bool is_valid_policy_name_unlocked(const std::string &policy_name);
 
 private:
-    friend class ::meta_service_test_app;
+    friend class meta_service_test_app;
 
     policy_factory _factory;
     meta_service *_meta_svc;
@@ -372,5 +370,5 @@ class backup_service
     std::atomic_bool _in_initialize;
     dsn::task_tracker _tracker;
 };
-}
-} // namespace
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/meta_server/meta_service.h b/src/dist/replication/meta_server/meta_service.h
index 7a6f1c94a9..eebc6c1aa4 100644
--- a/src/dist/replication/meta_server/meta_service.h
+++ b/src/dist/replication/meta_server/meta_service.h
@@ -48,7 +48,6 @@
 #include "dist/replication/meta_server/meta_state_service_utils.h"
 #include "dist/replication/common/block_service_manager.h"
 
-class meta_service_test_app;
 namespace dsn {
 namespace replication {
 
@@ -198,7 +197,7 @@ class meta_service : public serverlet<meta_service>
 private:
     friend class replication_checker;
     friend class test::test_checker;
-    friend class ::meta_service_test_app;
+    friend class meta_service_test_app;
 
     replication_options _opts;
     meta_options _meta_opts;
@@ -217,6 +216,7 @@ class meta_service : public serverlet<meta_service>
     friend class meta_test_base;
     friend class meta_duplication_service;
     friend class meta_http_service_test;
+    friend class meta_load_balance_test;
     friend class meta_backup_test_base;
     friend class meta_http_service;
     std::unique_ptr<meta_duplication_service> _dup_svc;
diff --git a/src/dist/replication/meta_server/server_state.h b/src/dist/replication/meta_server/server_state.h
index 1d41960ea6..f4fb5fa7de 100644
--- a/src/dist/replication/meta_server/server_state.h
+++ b/src/dist/replication/meta_server/server_state.h
@@ -48,8 +48,6 @@
 
 #include "meta_service.h"
 
-class meta_service_test_app;
-
 namespace dsn {
 namespace replication {
 
@@ -300,9 +298,10 @@ class server_state
 private:
     friend class replication_checker;
     friend class test::test_checker;
-    friend class ::meta_service_test_app;
+    friend class meta_service_test_app;
     friend class meta_test_base;
     friend class meta_duplication_service_test;
+    friend class meta_load_balance_test;
     friend class meta_duplication_service;
     friend class meta_split_service;
 
diff --git a/src/dist/replication/test/meta_test/unit_test/backup_test.cpp b/src/dist/replication/test/meta_test/unit_test/backup_test.cpp
index a1425eff34..5a1197a53b 100644
--- a/src/dist/replication/test/meta_test/unit_test/backup_test.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/backup_test.cpp
@@ -6,8 +6,8 @@
 #include "meta_service_test_app.h"
 #include "../misc/misc.h"
 
-using namespace ::dsn::replication;
-using namespace dsn;
+namespace dsn {
+namespace replication {
 
 struct method_record
 {
@@ -24,6 +24,7 @@ class mock_base
 {
 public:
     void reset_records() { _records.clear(); }
+
 protected:
     std::map<std::string, method_record> _records;
 };
@@ -722,3 +723,5 @@ void meta_service_test_app::backup_service_test()
         ASSERT_TRUE(p.policy_name == test_policy_name);
     }
 }
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/test/meta_test/unit_test/balancer_validator.cpp b/src/dist/replication/test/meta_test/unit_test/balancer_validator.cpp
index e54143e250..095d42af94 100644
--- a/src/dist/replication/test/meta_test/unit_test/balancer_validator.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/balancer_validator.cpp
@@ -15,8 +15,8 @@
 
 #include "meta_service_test_app.h"
 
-using namespace dsn;
-using namespace dsn::replication;
+namespace dsn {
+namespace replication {
 
 #ifdef ASSERT_EQ
 #undef ASSERT_EQ
@@ -335,3 +335,5 @@ void meta_service_test_app::balance_config_file()
         }
     }
 }
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/test/meta_test/unit_test/data_definition_test.cpp b/src/dist/replication/test/meta_test/unit_test/data_definition_test.cpp
index 2ba3ee0abf..68be6b0a98 100644
--- a/src/dist/replication/test/meta_test/unit_test/data_definition_test.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/data_definition_test.cpp
@@ -13,8 +13,8 @@
 
 #include "meta_service_test_app.h"
 
-using namespace dsn;
-using namespace dsn::replication;
+namespace dsn {
+namespace replication {
 
 inline void test_logger(const char *str)
 {
@@ -358,3 +358,5 @@ void meta_service_test_app::data_definition_op_test()
         ASSERT_EQ(recall_response.err, dsn::ERR_APP_NOT_EXIST);
     }
 }
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/test/meta_test/unit_test/json_compacity.cpp b/src/dist/replication/test/meta_test/unit_test/json_compacity.cpp
index 102e4f65c8..e7aaaeea31 100644
--- a/src/dist/replication/test/meta_test/unit_test/json_compacity.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/json_compacity.cpp
@@ -8,6 +8,9 @@
 #include "dist/replication/meta_server/meta_backup_service.h"
 #include "meta_service_test_app.h"
 
+namespace dsn {
+namespace replication {
+
 void meta_service_test_app::json_compacity()
 {
     dsn::app_info info;
@@ -131,3 +134,6 @@ void meta_service_test_app::json_compacity()
     ASSERT_EQ(app_names, binfo.app_names);
     ASSERT_EQ(1, binfo.info_status);
 }
+
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/test/meta_test/unit_test/main.cpp b/src/dist/replication/test/meta_test/unit_test/main.cpp
index ca5a8e5c6c..f2aba50920 100644
--- a/src/dist/replication/test/meta_test/unit_test/main.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/main.cpp
@@ -9,11 +9,15 @@
 
 #include "meta_service_test_app.h"
 
+int gtest_flags = 0;
+int gtest_ret = 0;
+
+namespace dsn {
+namespace replication {
+
 DEFINE_THREAD_POOL_CODE(THREAD_POOL_META_TEST)
 DEFINE_TASK_CODE(TASK_META_TEST, TASK_PRIORITY_COMMON, THREAD_POOL_META_TEST)
 
-int gtest_flags = 0;
-int gtest_ret = 0;
 meta_service_test_app *g_app;
 
 // as it is not easy to clean test environment in some cases, we simply run these tests in several
@@ -43,16 +47,6 @@ TEST(meta, construct_apps_test) { g_app->construct_apps_test(); }
 
 TEST(meta, balance_config_file) { g_app->balance_config_file(); }
 
-TEST(meta, simple_lb_balanced_cure) { g_app->simple_lb_balanced_cure(); }
-
-TEST(meta, simple_lb_cure_test) { g_app->simple_lb_cure_test(); }
-
-TEST(meta, simple_lb_from_proposal_test) { g_app->simple_lb_from_proposal_test(); }
-
-TEST(meta, simple_lb_collect_replica) { g_app->simple_lb_collect_replica(); }
-
-TEST(meta, simple_lb_construct_replica) { g_app->simple_lb_construct_replica(); }
-
 TEST(meta, json_compacity) { g_app->json_compacity(); }
 
 TEST(meta, adjust_dropped_size) { g_app->adjust_dropped_size(); }
@@ -85,9 +79,12 @@ dsn::error_code meta_service_test_app::start(const std::vector<std::string> &arg
     return dsn::ERR_OK;
 }
 
+} // namespace replication
+} // namespace dsn
+
 GTEST_API_ int main(int argc, char **argv)
 {
-    dsn::service_app::register_factory<meta_service_test_app>("test_meta");
+    dsn::service_app::register_factory<dsn::replication::meta_service_test_app>("test_meta");
     dsn::service::meta_service_app::register_all();
     if (argc < 2)
         dassert(dsn_run_config("config-test.ini", false), "");
diff --git a/src/dist/replication/test/meta_test/unit_test/simple_lb_cure_test.cpp b/src/dist/replication/test/meta_test/unit_test/meta_load_balance_test.cpp
similarity index 96%
rename from src/dist/replication/test/meta_test/unit_test/simple_lb_cure_test.cpp
rename to src/dist/replication/test/meta_test/unit_test/meta_load_balance_test.cpp
index 97f697c56d..f86ca8c62e 100644
--- a/src/dist/replication/test/meta_test/unit_test/simple_lb_cure_test.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/meta_load_balance_test.cpp
@@ -11,9 +11,11 @@
 #include "dist/replication/meta_server/meta_server_failure_detector.h"
 #include "dist/replication/meta_server/greedy_load_balancer.h"
 #include "dist/replication/test/meta_test/misc/misc.h"
+#include "dist/replication/test/meta_test/unit_test/meta_test_base.h"
 #include "meta_service_test_app.h"
 
-using namespace dsn::replication;
+namespace dsn {
+namespace replication {
 
 typedef std::shared_ptr<configuration_update_request> cur_ptr;
 
@@ -62,11 +64,37 @@ static auto default_filter = [](const dsn::rpc_address &target, dsn::message_ex
     return update_req;
 };
 
+class meta_load_balance_test : public meta_test_base
+{
+public:
+    void simple_lb_cure_test();
+    void simple_lb_balanced_cure();
+    void simple_lb_from_proposal_test();
+    void simple_lb_collect_replica();
+    void simple_lb_construct_replica();
+
+    void call_update_configuration(
+        meta_service *svc, std::shared_ptr<dsn::replication::configuration_update_request> &request)
+    {
+        dsn::message_ex *fake_request =
+            dsn::message_ex::create_request(RPC_CM_UPDATE_PARTITION_CONFIGURATION);
+        ::dsn::marshall(fake_request, *request);
+        fake_request->add_ref();
+
+        dsn::tasking::enqueue(
+            LPC_META_STATE_HIGH,
+            nullptr,
+            std::bind(
+                &server_state::on_update_configuration, svc->_state.get(), request, fake_request),
+            server_state::sStateHash);
+    }
+};
+
 class message_filter : public dsn::replication::meta_service
 {
 public:
     typedef std::function<cur_ptr(const dsn::rpc_address &target, dsn::message_ex *request)> filter;
-    message_filter(meta_service_test_app *app) : meta_service(), _app(app) {}
+    message_filter(meta_load_balance_test *app) : meta_service(), _app(app) {}
     void set_filter(const filter &f) { _filter = f; }
     virtual void reply_message(dsn::message_ex *request, dsn::message_ex *response) override {}
     virtual void send_message(const dsn::rpc_address &target, dsn::message_ex *request) override
@@ -81,11 +109,11 @@ class message_filter : public dsn::replication::meta_service
     }
 
 private:
-    meta_service_test_app *_app;
+    meta_load_balance_test *_app;
     filter _filter;
 };
 
-void meta_service_test_app::simple_lb_cure_test()
+void meta_load_balance_test::simple_lb_cure_test()
 {
     dsn::error_code ec;
     dsn::task_ptr t;
@@ -687,7 +715,7 @@ static void check_nodes_loads(node_mapper &nodes)
     ASSERT_TRUE(max_partitions - min_partitions <= 1);
 }
 
-void meta_service_test_app::simple_lb_balanced_cure()
+void meta_load_balance_test::simple_lb_balanced_cure()
 {
     std::vector<dsn::rpc_address> node_list;
     generate_node_list(node_list, 20, 100);
@@ -739,7 +767,7 @@ void meta_service_test_app::simple_lb_balanced_cure()
     }
 }
 
-void meta_service_test_app::simple_lb_from_proposal_test()
+void meta_load_balance_test::simple_lb_from_proposal_test()
 {
     class simple_balancer_for_test : public simple_load_balancer
     {
@@ -889,7 +917,7 @@ static bool vec_equal(const std::vector<dropped_replica> &vec1,
     return true;
 }
 
-void meta_service_test_app::simple_lb_collect_replica()
+void meta_load_balance_test::simple_lb_collect_replica()
 {
     app_mapper app;
     node_mapper nodes;
@@ -1135,7 +1163,7 @@ void meta_service_test_app::simple_lb_collect_replica()
 #undef CLEAR_DROP_LIST
 }
 
-void meta_service_test_app::simple_lb_construct_replica()
+void meta_load_balance_test::simple_lb_construct_replica()
 {
     app_mapper app;
     node_mapper nodes;
@@ -1252,3 +1280,16 @@ void meta_service_test_app::simple_lb_construct_replica()
         ASSERT_EQ(2, cc.prefered_dropped);
     }
 }
+
+TEST_F(meta_load_balance_test, simple_lb_balanced_cure) { simple_lb_balanced_cure(); }
+
+TEST_F(meta_load_balance_test, simple_lb_cure_test) { simple_lb_cure_test(); }
+
+TEST_F(meta_load_balance_test, simple_lb_from_proposal_test) { simple_lb_from_proposal_test(); }
+
+TEST_F(meta_load_balance_test, simple_lb_collect_replica) { simple_lb_collect_replica(); }
+
+TEST_F(meta_load_balance_test, simple_lb_construct_replica) { simple_lb_construct_replica(); }
+
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/test/meta_test/unit_test/meta_service_test_app.h b/src/dist/replication/test/meta_test/unit_test/meta_service_test_app.h
index 1d703d61e3..1d86cbb28a 100644
--- a/src/dist/replication/test/meta_test/unit_test/meta_service_test_app.h
+++ b/src/dist/replication/test/meta_test/unit_test/meta_service_test_app.h
@@ -36,6 +36,9 @@
 
 #include <gtest/gtest.h>
 
+namespace dsn {
+namespace replication {
+
 class spin_counter
 {
 private:
@@ -109,11 +112,6 @@ class meta_service_test_app : public dsn::service_app
     void cannot_run_balancer_test();
     void construct_apps_test();
 
-    void simple_lb_cure_test();
-    void simple_lb_balanced_cure();
-    void simple_lb_from_proposal_test();
-    void simple_lb_collect_replica();
-    void simple_lb_construct_replica();
     void json_compacity();
 
     void policy_context_test();
@@ -192,3 +190,6 @@ fake_rpc_call(dsn::task_code rpc_code,
         ::dsn::unmarshall(context->response, response_data);                                       \
         context->response->release_ref();                                                          \
     } while (0)
+
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/test/meta_test/unit_test/server_state_test.cpp b/src/dist/replication/test/meta_test/unit_test/server_state_test.cpp
index 01917cbb4f..8c7f69b892 100644
--- a/src/dist/replication/test/meta_test/unit_test/server_state_test.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/server_state_test.cpp
@@ -6,7 +6,8 @@
 
 #include "meta_service_test_app.h"
 
-using namespace ::dsn::replication;
+namespace dsn {
+namespace replication {
 
 static const std::vector<std::string> keys = {
     "p1.k1", "p1.k2", "p1.k3", "p2.k1", "p2.k2", "p2.k3", "p3.k1", "p3.k2", "p3.k3"};
@@ -164,3 +165,5 @@ void meta_service_test_app::app_envs_basic_test()
         }
     }
 }
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/test/meta_test/unit_test/state_sync_test.cpp b/src/dist/replication/test/meta_test/unit_test/state_sync_test.cpp
index 6e4952506a..5f78beecdd 100644
--- a/src/dist/replication/test/meta_test/unit_test/state_sync_test.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/state_sync_test.cpp
@@ -13,7 +13,8 @@
 
 #include "meta_service_test_app.h"
 
-using namespace dsn::replication;
+namespace dsn {
+namespace replication {
 
 static void random_assign_partition_config(std::shared_ptr<app_state> &app,
                                            const std::vector<dsn::rpc_address> &server_list,
@@ -397,3 +398,5 @@ void meta_service_test_app::construct_apps_test()
         i++;
     }
 }
+} // namespace replication
+} // namespace dsn
diff --git a/src/dist/replication/test/meta_test/unit_test/update_configuration_test.cpp b/src/dist/replication/test/meta_test/unit_test/update_configuration_test.cpp
index 8019552ebe..24480b018f 100644
--- a/src/dist/replication/test/meta_test/unit_test/update_configuration_test.cpp
+++ b/src/dist/replication/test/meta_test/unit_test/update_configuration_test.cpp
@@ -12,7 +12,8 @@
 
 #include "meta_service_test_app.h"
 
-using namespace dsn::replication;
+namespace dsn {
+namespace replication {
 
 class fake_sender_meta_service : public dsn::replication::meta_service
 {
@@ -479,3 +480,5 @@ void meta_service_test_app::cannot_run_balancer_test()
     the_app->status = dsn::app_status::AS_AVAILABLE;
     ASSERT_TRUE(svc->_state->can_run_balancer());
 }
+} // namespace replication
+} // namespace dsn