Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.7.0
    • Fix Version/s: 1.0.0
    • Component/s: tests
    • Labels:
      None

      Description

      TestCLI currently does not support DFSadmin tests. Consequently, multiple tests, including snapshots, fail.

      1. BIGTOP-1334.patch
        2 kB
        Dasha Boudnik
      2. BIGTOP-1334.patch
        15 kB
        Dasha Boudnik
      3. BIGTOP-1334.patch
        15 kB
        Dasha Boudnik
      4. BIGTOP-1334.patch
        14 kB
        Dasha Boudnik
      5. BIGTOP-1334.patch
        15 kB
        Dasha Boudnik

        Issue Links

          Activity

          Hide
          dasha.boudnik Dasha Boudnik added a comment -

          Patch attached.

          Show
          dasha.boudnik Dasha Boudnik added a comment - Patch attached.
          Hide
          mbukatov Martin Bukatovic added a comment -

          Shouldn't this be part of BIGTOP-1325 (eg. direct part or a subtask)? Because DFS support is otherwise not needed in current bigtop TestCLI.

          Show
          mbukatov Martin Bukatovic added a comment - Shouldn't this be part of BIGTOP-1325 (eg. direct part or a subtask)? Because DFS support is otherwise not needed in current bigtop TestCLI.
          Hide
          dasha.boudnik Dasha Boudnik added a comment -

          Martin Bukatovic, I've updated the issue accordingly – thank you.

          Show
          dasha.boudnik Dasha Boudnik added a comment - Martin Bukatovic , I've updated the issue accordingly – thank you.
          Hide
          jayunit100 jay vyas added a comment -

          Is TestCLI going to be HDFS Specific ? Or is it meant to be refactored into HCFS (superclass) and HDFS (subclass) specific tests.

          Show
          jayunit100 jay vyas added a comment - Is TestCLI going to be HDFS Specific ? Or is it meant to be refactored into HCFS (superclass) and HDFS (subclass) specific tests.
          Hide
          cos Konstantin Boudnik added a comment -

          I think the latter, but it needs to start somewhere. I don't think this patch is final in any sense.

          Show
          cos Konstantin Boudnik added a comment - I think the latter, but it needs to start somewhere. I don't think this patch is final in any sense.
          Hide
          mbukatov Martin Bukatovic added a comment -

          I think it would make sense to let this in and refactor TestCLI into HCFS (superclass) and HDFS (subclass) later in separate JIRA. I have just make 100% of TestCLI cases work with GlusterFS plugin and there are many other peculiarities which would require some additional work (I'm going to create new JIRA this week when I clean the patch a little).

          Show
          mbukatov Martin Bukatovic added a comment - I think it would make sense to let this in and refactor TestCLI into HCFS (superclass) and HDFS (subclass) later in separate JIRA. I have just make 100% of TestCLI cases work with GlusterFS plugin and there are many other peculiarities which would require some additional work (I'm going to create new JIRA this week when I clean the patch a little).
          Hide
          mbukatov Martin Bukatovic added a comment - - edited

          Or we can do it vice versa, you can check my initial hcfs patch for TestCLI attached to BIGTOP-1342 and decide if you are ok with the changes there and base your patch on it (this would obviously require to merge BIGTOP-1342 before this one - BIGTOP-1334). Both approaches mean almost same amount of work for me, so it would make sense to let you decide which way suits you better.

          Show
          mbukatov Martin Bukatovic added a comment - - edited Or we can do it vice versa, you can check my initial hcfs patch for TestCLI attached to BIGTOP-1342 and decide if you are ok with the changes there and base your patch on it (this would obviously require to merge BIGTOP-1342 before this one - BIGTOP-1334 ). Both approaches mean almost same amount of work for me, so it would make sense to let you decide which way suits you better.
          Hide
          mbukatov Martin Bukatovic added a comment -

          So it seems we are going to resolve HCFS first. That means that this patch needs to add DFSadmin tests into HDFS part of TestCLI test.

          Show
          mbukatov Martin Bukatovic added a comment - So it seems we are going to resolve HCFS first. That means that this patch needs to add DFSadmin tests into HDFS part of TestCLI test.
          Hide
          cos Konstantin Boudnik added a comment -

          This patch needs some work anyway, so don't hold yours.

          Show
          cos Konstantin Boudnik added a comment - This patch needs some work anyway, so don't hold yours.
          Hide
          cos Konstantin Boudnik added a comment -

          Thanks for the latest patch, Dasha! I think dfsadmin is HDFS specific so there's no need for anything HCFS-related. I guess you can get rid of HCFS* things in the java file. jay vyas, Martin Bukatovic could you please confirm that dfsadmin tests aren't going to be used for none of non-HDFS system you care about?

          Show
          cos Konstantin Boudnik added a comment - Thanks for the latest patch, Dasha! I think dfsadmin is HDFS specific so there's no need for anything HCFS-related. I guess you can get rid of HCFS* things in the java file. jay vyas , Martin Bukatovic could you please confirm that dfsadmin tests aren't going to be used for none of non-HDFS system you care about?
          Hide
          cos Konstantin Boudnik added a comment -

          Looks like you can replace System.getProperty("user.name")); inside of the assert with defined above username

          Show
          cos Konstantin Boudnik added a comment - Looks like you can replace System.getProperty("user.name")); inside of the assert with defined above username
          Hide
          dasha.boudnik Dasha Boudnik added a comment -

          That should take care of it.

          Show
          dasha.boudnik Dasha Boudnik added a comment - That should take care of it.
          Hide
          cos Konstantin Boudnik added a comment - - edited

          file TestDFSCLI is missing ASL boilerplate.
          Also, to be very picky:

          • 0, conf.getInt("fs.trash.interval",0)); needs space after ","
          •        System.getProperty("hcfs.root.username", "hdfs"),
                   username);
            

            should be written in one line

          • @param cmd needs to have parameter description
          • protected String expandCommand(final String cmd) final modifier for String isn't harmful, but doesn't add anything as Strings are immutable. This one is ok to keep as is, I think.

          Thanks

          Show
          cos Konstantin Boudnik added a comment - - edited file TestDFSCLI is missing ASL boilerplate. Also, to be very picky: 0, conf.getInt("fs.trash.interval",0)); needs space after "," System .getProperty( "hcfs.root.username" , "hdfs" ), username); should be written in one line @param cmd needs to have parameter description protected String expandCommand(final String cmd) final modifier for String isn't harmful, but doesn't add anything as Strings are immutable. This one is ok to keep as is, I think. Thanks
          Hide
          dasha.boudnik Dasha Boudnik added a comment -

          Fixed/updated. Thanks!

          Show
          dasha.boudnik Dasha Boudnik added a comment - Fixed/updated. Thanks!
          Hide
          cos Konstantin Boudnik added a comment -

          +1
          We haven't heard from HCFS folks, so I presume it is ok to be HDFS specific test. Will commit it in a bit.

          Show
          cos Konstantin Boudnik added a comment - +1 We haven't heard from HCFS folks, so I presume it is ok to be HDFS specific test. Will commit it in a bit.
          Hide
          cos Konstantin Boudnik added a comment -

          Pushed to master. Thanks Dasha!

          Show
          cos Konstantin Boudnik added a comment - Pushed to master. Thanks Dasha!
          Hide
          jayunit100 jay vyas added a comment -

          i think this is okay from HCFS perspective.

          Martin Bukatovic can we still run HCFS tests by simply disabling the DFSAdmin tests easily?

          You would know better than me i think

          Show
          jayunit100 jay vyas added a comment - i think this is okay from HCFS perspective. Martin Bukatovic can we still run HCFS tests by simply disabling the DFSAdmin tests easily? You would know better than me i think
          Hide
          mbukatov Martin Bukatovic added a comment -

          Ah, sorry for missing this.

          This test is HDFS only, so I would move it into

          /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hdfs

          instead of

          /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hcfs

          (the difference is in the last directory). That is because hcfs package is
          meant for testcases which would run on any hadoop filesystem, while hdfs
          package contains HDFS specific tests and as such is HDFS only.

          Otherwise there is no other clash with HCFS tests. Moreover looking at the
          patch the most important change is in the new xml file with the testcases anyway.

          So I would create a new JIRA with a patch to move this test into hdfs directory.
          Would that be ok?

          Show
          mbukatov Martin Bukatovic added a comment - Ah, sorry for missing this. This test is HDFS only, so I would move it into /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hdfs instead of /bigtop-tests/test-artifacts/hadoop/src/main/groovy/org/apache/bigtop/itest/hadoop/hcfs (the difference is in the last directory). That is because hcfs package is meant for testcases which would run on any hadoop filesystem, while hdfs package contains HDFS specific tests and as such is HDFS only. Otherwise there is no other clash with HCFS tests. Moreover looking at the patch the most important change is in the new xml file with the testcases anyway. So I would create a new JIRA with a patch to move this test into hdfs directory. Would that be ok?
          Hide
          dasha.boudnik Dasha Boudnik added a comment -

          Good idea, thanks, Martin Bukatovic. Moved to BIGTOP-1629.

          Show
          dasha.boudnik Dasha Boudnik added a comment - Good idea, thanks, Martin Bukatovic . Moved to BIGTOP-1629 .

            People

            • Assignee:
              dasha.boudnik Dasha Boudnik
              Reporter:
              dasha.boudnik Dasha Boudnik
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development