Add support for dfsadmin commands for test TestCLI unit test

• Key: HADOOP-4708
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Boris Shkolnik
• Reporter: Boris Shkolnik
• Component/s: test
• Affects Version/s: 0.20.0
• Fix Version/s: 0.20.0

Description

Curretly TestCLI assumes dfs command when describing tests in testConf.xml
we need to add ability to run dfsadmin commands too.

Comments

[Boris Shkolnik]

I suggest we add <admin-command> to testConf.xml to describe a dfsadmin command to be run by the framework.

implementation wise we should change a command from being a String to be a class that knows what type of command it is and use DFSShell for dfsadmin commands.

[Boris Shkolnik]

patch

[Chris Douglas]

It would be nice if more of the CommandExecutor logic were part of the enumerated type, but it's not necessary for this patch.

• For enums:

  +    	  if(cmd.getType()==CommandType.ADMIN) {
  +    		  CommandExecutor.executeDFSAdminCommand(cmd.getCmd(), namenode);
  +    	  } else {
  +    		  CommandExecutor.executeFSCommand(cmd.getCmd(), namenode);
  +    	  }
  

  This should probably be a switch stmt.

• This change is unnecessary:

  @@ -107,8 +109,7 @@
       conf = new Configuration();
       cluster = new MiniDFSCluster(conf, 1, true, null);
       namenode = conf.get("fs.default.name", "file:///");
  -    clitestDataDir = new File(TEST_CACHE_DATA_DIR).
  -      toURI().toString().replace(' ', '+');
  +    clitestDataDir = new File(TEST_CACHE_DATA_DIR).toURI().toString().replace(' ', '+');
       username = System.getProperty("user.name");
  

• TestCmd.type and TestCmd.cmd can be final
• Why getCmd instead of just toString()?
• The cast to String is (still) unnecessary:

  -              expandCommand((String) testCommands.get(j)) + "]");
  +              expandCommand((String) testCommands.get(j).getCmd()) + "]");
  

[Hadoop QA]

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12394680/+HADOOP-4708.patch
against trunk revision 720930.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 9 new or modified tests.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs warnings.

+1 Eclipse classpath. The patch retains Eclipse classpath integrity.

+1 core tests. The patch passed core unit tests.

+1 contrib tests. The patch passed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3655/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3655/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3655/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3655/console

This message is automatically generated.

[Boris Shkolnik]

recommended fixes to the patch:
1. moving logic of selecting correct function for a type of command to CommandExecutor (I think it makes sense to do it now, cause it is more clean)
2. making get methods final
3. removing unnecessary formating and casting.
4. changed to switch from if-else with a default throw.

I left getCmd and not toString() - it makes more sense.

[Chris Douglas]

+1

I just committed this. Thanks, Boris

[Hudson]

Integrated in Hadoop-trunk #677 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/677/)
. Add support for dfsadmin commands in TestCLI. Contributed by Boris Shkolnik.

[Hudson]

Integrated in Hadoop-trunk #684 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/684/)
. Add binaries missed in the initial checkin for Chukwa. Contributed by Eric Yang.