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 <[email protected]>
Reviewed-by: Dan Burkert <[email protected]>
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 <[email protected]>
Authored: Wed May 24 18:58:02 2017 -0700
Committer: Dan Burkert <[email protected]>
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());
