Repository: hbase Updated Branches: refs/heads/HBASE-7912 7650f6059 -> 759a2c17a
HBASE-16620 Fix backup command-line tool usability issues - addendum 4 adds more tests for command line Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/759a2c17 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/759a2c17 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/759a2c17 Branch: refs/heads/HBASE-7912 Commit: 759a2c17a99b2392fb73bbfac5659e2837e9bba3 Parents: 7650f60 Author: tedyu <yuzhih...@gmail.com> Authored: Thu Sep 15 11:49:12 2016 -0700 Committer: tedyu <yuzhih...@gmail.com> Committed: Thu Sep 15 11:49:12 2016 -0700 ---------------------------------------------------------------------- .../hbase/backup/impl/BackupCommands.java | 59 +++--- .../hbase/backup/TestBackupCommandLineTool.java | 194 +++++++++++++++++-- 2 files changed, 209 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/759a2c17/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java index 8f08094..1884788 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java @@ -50,7 +50,7 @@ import com.google.common.collect.Lists; @InterfaceStability.Evolving public final class BackupCommands { - public final static String IGNORE = "ignore"; + public final static String INCORRECT_USAGE = "Incorrect usage"; public static final String USAGE = "Usage: hbase backup COMMAND [command-specific arguments]\n" + "where COMMAND is one of:\n" @@ -60,7 +60,7 @@ public final class BackupCommands { + " history show history of all successful backups\n" + " progress show the progress of the latest backup request\n" + " set backup set management\n" - + "Run \'hbase backup help COMMAND\' to see help message for each command\n"; + + "Run \'hbase backup COMMAND -h\' to see help message for each command\n"; public static final String CREATE_CMD_USAGE = "Usage: hbase backup create <type> <BACKUP_ROOT> [tables] [-set name] " @@ -118,7 +118,7 @@ public final class BackupCommands { { if (cmdline.hasOption("h") || cmdline.hasOption("help")) { printUsage(); - throw new IOException(IGNORE); + throw new IOException(INCORRECT_USAGE); } } @@ -179,20 +179,20 @@ public final class BackupCommands { if (cmdline == null || cmdline.getArgs() == null) { System.err.println("ERROR: missing arguments"); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String[] args = cmdline.getArgs(); if (args.length < 3 || args.length > 4) { System.err.println("ERROR: wrong number of arguments: "+ args.length); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } if (!BackupType.FULL.toString().equalsIgnoreCase(args[1]) && !BackupType.INCREMENTAL.toString().equalsIgnoreCase(args[1])) { System.err.println("ERROR: invalid backup type: "+ args[1]); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String tables = null; @@ -206,7 +206,7 @@ public final class BackupCommands { if (tables == null) { System.err.println("ERROR: Backup set '" + setName+ "' is either empty or does not exist"); printUsage(); - throw new IOException(IGNORE); + throw new IOException(INCORRECT_USAGE); } } else { tables = (args.length == 4) ? args[3] : null; @@ -224,7 +224,7 @@ public final class BackupCommands { String backupId = backupAdmin.backupTables(request); System.out.println("Backup session "+ backupId+" finished. Status: SUCCESS"); } catch (IOException e) { - System.out.println("Backup session finished. Status: FAILURE"); + System.err.println("Backup session finished. Status: FAILURE"); throw e; } } @@ -256,19 +256,19 @@ public final class BackupCommands { super.execute(); if (cmdline == null) { printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String[] args = cmdline.getArgs(); if (args == null || args.length == 0) { printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } if (args.length != 2) { System.err.println("Only supports help message of a single command type"); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String type = args[1]; @@ -291,7 +291,6 @@ public final class BackupCommands { System.out.println("Unknown command : " + type); printUsage(); } - System.exit(0); } @Override @@ -313,13 +312,13 @@ public final class BackupCommands { if (cmdline == null || cmdline.getArgs() == null) { System.err.println("ERROR: missing arguments"); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String[] args = cmdline.getArgs(); if (args.length != 2) { System.err.println("ERROR: wrong number of arguments"); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String backupId = args[1]; @@ -357,7 +356,7 @@ public final class BackupCommands { if (args.length > 2) { System.err.println("ERROR: wrong number of arguments: " + args.length); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } @@ -393,7 +392,7 @@ public final class BackupCommands { if (cmdline == null || cmdline.getArgs() == null || cmdline.getArgs().length < 2) { System.err.println("No backup id(s) was specified"); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String[] args = cmdline.getArgs(); @@ -478,7 +477,7 @@ public final class BackupCommands { } } - private Path getBackupRootPath() { + private Path getBackupRootPath() throws IOException { String value = null; try{ value = cmdline.getOptionValue("path"); @@ -487,12 +486,11 @@ public final class BackupCommands { } catch (IllegalArgumentException e) { System.err.println("ERROR: Illegal argument for backup root path: "+ value); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } - return null; } - private TableName getTableName() { + private TableName getTableName() throws IOException { String value = cmdline.getOptionValue("t"); if (value == null) return null; try{ @@ -500,12 +498,11 @@ public final class BackupCommands { } catch (IllegalArgumentException e){ System.err.println("Illegal argument for table name: "+ value); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } - return null; } - private int parseHistoryLength() { + private int parseHistoryLength() throws IOException { String value = cmdline.getOptionValue("n"); try{ if (value == null) return DEFAULT_HISTORY_LENGTH; @@ -513,9 +510,8 @@ public final class BackupCommands { } catch(NumberFormatException e) { System.err.println("Illegal argument for history length: "+ value); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } - return 0; } @Override @@ -543,7 +539,7 @@ public final class BackupCommands { if (cmdline == null || cmdline.getArgs() == null || cmdline.getArgs().length < 2) { System.err.println("ERROR: Command line format"); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String[] args = cmdline.getArgs(); @@ -590,7 +586,7 @@ public final class BackupCommands { System.err.println("ERROR: Wrong number of args for 'set describe' command: " + numOfArgs(args)); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String setName = args[2]; Configuration conf = getConf() != null? getConf(): HBaseConfiguration.create(); @@ -610,7 +606,7 @@ public final class BackupCommands { System.err.println("ERROR: Wrong number of args for 'set delete' command: " + numOfArgs(args)); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String setName = args[2]; Configuration conf = getConf() != null? getConf(): HBaseConfiguration.create(); @@ -630,7 +626,7 @@ public final class BackupCommands { System.err.println("ERROR: Wrong number of args for 'set remove' command: " + numOfArgs(args)); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String setName = args[2]; @@ -647,7 +643,7 @@ public final class BackupCommands { System.err.println("ERROR: Wrong number of args for 'set add' command: " + numOfArgs(args)); printUsage(); - System.exit(-1); + throw new IOException(INCORRECT_USAGE); } String setName = args[2]; String[] tables = args[3].split(","); @@ -677,8 +673,7 @@ public final class BackupCommands { } else { System.err.println("ERROR: Unknown command for 'set' :" + cmdStr); printUsage(); - System.exit(-1); - return null; + throw new IOException(INCORRECT_USAGE); } } http://git-wip-us.apache.org/repos/asf/hbase/blob/759a2c17/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java index 1983c8e..8330ecb 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java @@ -43,7 +43,6 @@ public class TestBackupCommandLineTool { ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); String[] args = new String[]{"describe", "-help" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); String output = baos.toString(); @@ -53,7 +52,6 @@ public class TestBackupCommandLineTool { baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); args = new String[]{"describe", "-h" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); output = baos.toString(); @@ -66,7 +64,6 @@ public class TestBackupCommandLineTool { ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); String[] args = new String[]{"create", "-help" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); String output = baos.toString(); @@ -76,7 +73,6 @@ public class TestBackupCommandLineTool { baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); args = new String[]{"create", "-h" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); output = baos.toString(); @@ -89,7 +85,6 @@ public class TestBackupCommandLineTool { ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); String[] args = new String[]{"history", "-help" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); String output = baos.toString(); @@ -99,7 +94,6 @@ public class TestBackupCommandLineTool { baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); args = new String[]{"history", "-h" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); output = baos.toString(); @@ -112,7 +106,6 @@ public class TestBackupCommandLineTool { ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); String[] args = new String[]{"delete", "-help" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); String output = baos.toString(); @@ -122,7 +115,6 @@ public class TestBackupCommandLineTool { baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); args = new String[]{"delete", "-h" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); output = baos.toString(); @@ -135,7 +127,6 @@ public class TestBackupCommandLineTool { ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); String[] args = new String[]{"progress", "-help" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); String output = baos.toString(); @@ -145,7 +136,6 @@ public class TestBackupCommandLineTool { baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); args = new String[]{"progress", "-h" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); output = baos.toString(); @@ -158,7 +148,6 @@ public class TestBackupCommandLineTool { ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); String[] args = new String[]{"set", "-help" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); String output = baos.toString(); @@ -168,11 +157,192 @@ public class TestBackupCommandLineTool { baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); args = new String[]{"set", "-h" }; - // Run backup ToolRunner.run(conf, new BackupDriver(), args); output = baos.toString(); System.out.println(baos.toString()); assertTrue(output.indexOf("Usage: hbase backup set") >= 0); } + + @Test + public void testBackupDriverHelp () throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + String[] args = new String[]{"-help" }; + ToolRunner.run(conf, new BackupDriver(), args); + + String output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"-h" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + } + + @Test + public void testRestoreDriverHelp () throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + String[] args = new String[]{"-help" }; + ToolRunner.run(conf, new RestoreDriver(), args); + + String output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase restore") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"-h" }; + ToolRunner.run(conf, new RestoreDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase restore") >= 0); + } + + @Test + public void testBackupDriverUnrecognizedCommand () throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + String[] args = new String[]{"command" }; + ToolRunner.run(conf, new BackupDriver(), args); + + String output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"command" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + } + + + + @Test + public void testBackupDriverUnrecognizedOption () throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + String[] args = new String[]{"create", "-xx" }; + ToolRunner.run(conf, new BackupDriver(), args); + + String output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"describe", "-xx" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"history", "-xx" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"delete", "-xx" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"set", "-xx" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup") >= 0); + } + + @Test + public void testRestoreDriverUnrecognizedOption () throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + String[] args = new String[]{"-xx" }; + ToolRunner.run(conf, new RestoreDriver(), args); + + String output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase restore") >= 0); + + } + + @Test + public void testBackupDriverCreateWrongArgNumber () throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + String[] args = new String[]{"create" }; + ToolRunner.run(conf, new BackupDriver(), args); + + String output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup create") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"create", "22" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup create") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"create", "22", "22", "22", "22", "22" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup create") >= 0); + } + + @Test + public void testBackupDriverDeleteWrongArgNumber () throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + String[] args = new String[]{"delete" }; + ToolRunner.run(conf, new BackupDriver(), args); + + String output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup delete") >= 0); + + } + + @Test + public void testBackupDriverHistoryWrongArgs () throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + String[] args = new String[]{"history", "-n", "xx" }; + ToolRunner.run(conf, new BackupDriver(), args); + + String output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup history") >= 0); + + } }