You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by te...@apache.org on 2016/09/15 18:49:18 UTC

hbase git commit: HBASE-16620 Fix backup command-line tool usability issues - addendum 4 adds more tests for command line

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 <yu...@gmail.com>
Authored: Thu Sep 15 11:49:12 2016 -0700
Committer: tedyu <yu...@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);
+    
+  }
 }