kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject kudu git commit: KUDU-1911 improve missing required arg message
Date Fri, 26 May 2017 18:23:46 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 7e02ede33 -> 86fac3fa2


KUDU-1911 improve missing required arg message

When using a tool that has a required positional argument, excluding that
argument or passing it as a flag caused a confusing error message to be
printed out ("must provide missing_argument"). This commit changes
the message to "must provide positional argument missing_argument" for
non-variadic arguments and "must provide variadic positional argument
missing_argument" for variadic arguments.

Change-Id: I2128a01da5c4929981b2ea46a32b0f7b9dd066e1
Reviewed-on: http://gerrit.cloudera.org:8080/6986
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <awong@cloudera.com>
Reviewed-by: Dan Burkert <danburkert@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/86fac3fa
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/86fac3fa
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/86fac3fa

Branch: refs/heads/master
Commit: 86fac3fa270b22820c1599770bb06b1de89cb618
Parents: 7e02ede
Author: Sam Okrent <samuel.okrent@cloudera.com>
Authored: Wed May 24 18:58:02 2017 -0700
Committer: Dan Burkert <danburkert@apache.org>
Committed: Fri May 26 18:22:51 2017 +0000

----------------------------------------------------------------------
 src/kudu/tools/kudu-tool-test.cc | 21 +++++++++++++++++++++
 src/kudu/tools/tool_main.cc      |  5 +++--
 2 files changed, 24 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/86fac3fa/src/kudu/tools/kudu-tool-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index aef6d89..b5a73a8 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -222,6 +222,19 @@ class ToolTest : public KuduTest {
     }
   }
 
+  // Run tool without a required positional argument, expecting error message
+  void RunActionMissingRequiredArg(const string& arg_str, const string& required_arg,
+                                   bool variadic = false) const {
+    const string kPositionalArgumentMessage = "must provide positional argument";
+    const string kVariadicArgumentMessage = "must provide variadic positional argument";
+    const string& message = variadic ? kVariadicArgumentMessage : kPositionalArgumentMessage;
+    string err;
+    RunTool(arg_str, nullptr, &err, nullptr, nullptr);
+
+    Status expected_status = Status::InvalidArgument(Substitute("$0 $1", message, required_arg));
+    ASSERT_EQ(expected_status.ToString(), err);
+  }
+
   void RunFsCheck(const string& arg_str,
                   int expected_num_live,
                   const string& tablet_id,
@@ -494,6 +507,14 @@ TEST_F(ToolTest, TestActionHelp) {
       Status::InvalidArgument("too many arguments: 'extra_arg'")));
 }
 
+TEST_F(ToolTest, TestActionMissingRequiredArg) {
+  NO_FATALS(RunActionMissingRequiredArg("master list", "master_addresses"));
+  NO_FATALS(RunActionMissingRequiredArg("cluster ksck --master_addresses=master.example.com",
+                                        "master_addresses"));
+  NO_FATALS(RunActionMissingRequiredArg("local_replica cmeta rewrite_raft_config fake_id",
+                                        "peers", /* variadic */ true));
+}
+
 TEST_F(ToolTest, TestFsCheck) {
   const string kTestDir = GetTestPath("test");
   const string kTabletId = "test-tablet";

http://git-wip-us.apache.org/repos/asf/kudu/blob/86fac3fa/src/kudu/tools/tool_main.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_main.cc b/src/kudu/tools/tool_main.cc
index 3aeb8f7..29652f5 100644
--- a/src/kudu/tools/tool_main.cc
+++ b/src/kudu/tools/tool_main.cc
@@ -82,7 +82,7 @@ Status MarshalArgs(const vector<Mode*>& chain,
   // Marshal the required arguments from the command line.
   for (const auto& a : args.required) {
     if (input.empty()) {
-      return Status::InvalidArgument(Substitute("must provide $0", a.name));
+      return Status::InvalidArgument(Substitute("must provide positional argument $0", a.name));
     }
     InsertOrDie(required, a.name, input.front());
     input.pop_front();
@@ -92,7 +92,8 @@ Status MarshalArgs(const vector<Mode*>& chain,
   if (args.variadic) {
     const ActionArgsDescriptor::Arg& a = args.variadic.get();
     if (input.empty()) {
-      return Status::InvalidArgument(Substitute("must provide $0", a.name));
+      return Status::InvalidArgument(Substitute("must provide variadic positional argument
$0",
+                                                a.name));
     }
 
     variadic->assign(input.begin(), input.end());


Mime
View raw message