(kudu) 02/02: Minor refactoring on kudu-tool-test (take 2)

[la...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

laiyingchun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 7bc13ef925b7be11c315745d335e8a5e277890e8
Author: Yingchun Lai <la...@apache.org>
AuthorDate: Sun Nov 5 23:12:24 2023 +0800

    Minor refactoring on kudu-tool-test (take 2)
    
    Move the parameterize logic of kudu-tool-test
    from kudu-tool-test.cc to tool_test_util.cc,
    the latter one is more general because the
    RunKuduTool() in tool_test_util.cc is a more
    general utility which is called in mutiple
    test files.
    
    Change-Id: I86d2120aa689473495ec8d4604caee1123060663
    Reviewed-on: http://gerrit.cloudera.org:8080/20655
    Tested-by: Kudu Jenkins
    Reviewed-by: Yifan Zhang <ch...@163.com>
---
 src/kudu/tools/kudu-tool-test.cc | 12 +-----------
 src/kudu/tools/tool_test_util.cc | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 4a0b3fc75..3aea9dcdc 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -271,17 +271,7 @@ class ToolTest : public KuduTest {
                  const string& in = "") const {
     string out;
     string err;
-    string args(arg_str);
-    // Pass the flags to the CLI tools. Now only --encrypt_data_at_rest and
-    // --enable_multi_tenancy flags are passed by default, the other flags
-    // are needed to be passed manually by adding them in 'arg_str'.
-    if (FLAGS_encrypt_data_at_rest) {
-      args += " --encrypt_data_at_rest";
-    }
-    if (FLAGS_enable_multi_tenancy) {
-      args += " --enable_multi_tenancy --unlock_experimental_flags";
-    }
-    Status s = RunKuduTool(strings::Split(args, " ", strings::SkipEmpty()),
+    Status s = RunKuduTool(strings::Split(arg_str, " ", strings::SkipEmpty()),
                            &out, &err, in);
     if (stdout) {
       *stdout = out;
diff --git a/src/kudu/tools/tool_test_util.cc b/src/kudu/tools/tool_test_util.cc
index 6d4f1179f..341f522ec 100644
--- a/src/kudu/tools/tool_test_util.cc
+++ b/src/kudu/tools/tool_test_util.cc
@@ -23,6 +23,7 @@
 #include <utility>
 #include <vector>
 
+#include <gflags/gflags.h>
 #include <glog/logging.h>
 
 #include "kudu/gutil/strings/split.h"
@@ -37,6 +38,10 @@ using std::vector;
 using strings::Split;
 using strings::Substitute;
 
+DECLARE_bool(enable_multi_tenancy);
+DECLARE_bool(encrypt_data_at_rest);
+DECLARE_string(block_manager);
+
 namespace kudu {
 namespace tools {
 
@@ -50,7 +55,8 @@ Status RunKuduTool(const vector<string>& args, string* out, string* err,
                    const string& in, map<string, string> env_vars) {
   vector<string> total_args = { GetKuduToolAbsolutePath() };
 
-  // Some scenarios might add unsafe flags for testing purposes.
+  // Some scenarios might add experimental or unsafe flags for testing purposes.
+  total_args.emplace_back("--unlock_experimental_flags");
   total_args.emplace_back("--unlock_unsafe_flags");
 
   // Speed up filesystem-based operations.
@@ -68,6 +74,15 @@ Status RunKuduTool(const vector<string>& args, string* out, string* err,
   // side as well to allow clients to accept and verify TLS certificates.
   total_args.emplace_back("--openssl_security_level_override=0");
 
+  // Pass some flags to the CLI tools for testing purposes.
+  // Now only --encrypt_data_at_rest, --enable_multi_tenancy and --block_manager
+  // flags are passed by default.
+  total_args.emplace_back(Substitute("--encrypt_data_at_rest=$0",
+                                     FLAGS_encrypt_data_at_rest));
+  total_args.emplace_back(Substitute("--enable_multi_tenancy=$0",
+                                     FLAGS_enable_multi_tenancy));
+  total_args.emplace_back(Substitute("--block_manager=$0", FLAGS_block_manager));
+
   total_args.insert(total_args.end(), args.begin(), args.end());
   return Subprocess::Call(total_args, in, out, err, std::move(env_vars));
 }