[ 
https://issues.apache.org/jira/browse/HDFS-11011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15583864#comment-15583864
 ] 

Mingliang Liu commented on HDFS-11011:
--------------------------------------

Thanks for your patch [~xiaobingo]. It looks good to me overall.

{code:title=TestDFSAdmin.java}
68      /**
69       * set/clrSpaceQuote are tested in {@link 
org.apache.hadoop.hdfs.TestQuota}.
70       */
{code}
Add one more javadoc for the test class itself? Or this line will be treated as 
the javadoc for the test class.
{code}
/**
 * Test cases for <code>hdfs dfsadmin<code> command.
 *
 * Tests for some subcommands  are covered elewhere, e.g. set/clrSpaceQuote are 
tested in {@link org.apache.hadoop.hdfs.TestQuota}.
 */
{code}

{code}
        1482        /* set space quota */
1483        resetStream();
1484        outs.clear();
1485        ret = whoever.doAs(new PrivilegedExceptionAction<Integer>() {
1486          @Override
1487          public Integer run() throws Exception {
1488            return ToolRunner.run(
1489                admin,
1490                new String[] {"-setSpaceQuota", "2048", dir.toString()});
1491          }
1492        });
1493        assertEquals(-1, ret);
1494        scanIntoList(err, outs);
1495        assertThat(outs.get(0),
1496            is(allOf(containsString("setSpaceQuota"),
1497                containsString("Access denied for user whoever"),
1498                containsString("Superuser privilege is required"))));
1499    
1500        /* clear space quota */
1501        resetStream();
1502        outs.clear();
1503        ret = whoever.doAs(new PrivilegedExceptionAction<Integer>() {
1504          @Override
1505          public Integer run() throws Exception {
1506            return ToolRunner.run(
1507                admin,
1508                new String[] {"-clrSpaceQuota", dir.toString()});
1509          }
1510        });
1511        assertEquals(-1, ret);
1512        scanIntoList(err, outs);
1513        assertThat(outs.get(0),
1514            is(allOf(containsString("clrSpaceQuota"),
1515                containsString("Access denied for user whoever"),
1516                containsString("Superuser privilege is required"))));
1517      }
{code}
Code duplicates. Can you use a for loop? See 
https://github.com/apache/hadoop/blob/129125404244f35ee63b8f0491a095371685e9ba/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java#L212-L212
 This applies to other places too.

By the way, can you update the descriptions with all the newly add test cases? 
(Simply copy the javadoc for each test case and list them here will work just 
fine.)

> Add unit tests for HDFS command 'dfsadmin -set/clrSpaceQuota'
> -------------------------------------------------------------
>
>                 Key: HDFS-11011
>                 URL: https://issues.apache.org/jira/browse/HDFS-11011
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: hdfs-client
>            Reporter: Xiaobing Zhou
>            Assignee: Xiaobing Zhou
>              Labels: fs, shell, test
>         Attachments: HDFS-11011.000.patch, HDFS-11011.001.patch, 
> HDFS-11011.002.patch
>
>
> This proposes adding a bunch of unit tests for command  'dfsadmin 
> setSpaceQuota' and  'dfsadmin clrSpaceQuota'.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to