Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Append Branch
    • Fix Version/s: 0.21.0
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      According to the test plan a number of new features are going to be implemented as a part of this umbrella (HDFS-265) JIRA.
      These new features are have to be tested properly. Lease recovery is one of new functionality which require new tests to be developed.

      1. HDFS-519.patch
        6 kB
        Konstantin Boudnik
      2. HDFS-519.patch
        8 kB
        Konstantin Boudnik
      3. HDFS-519.patch
        11 kB
        Konstantin Boudnik
      4. HDFS-519.patch
        11 kB
        Konstantin Boudnik
      5. HDFS-519.patch
        10 kB
        Konstantin Boudnik
      6. HDFS-519.patch
        10 kB
        Konstantin Boudnik
      7. HDFS-519.patch
        11 kB
        Konstantin Boudnik

        Issue Links

          Activity

          Hide
          cos Konstantin Boudnik added a comment -

          The fix for this requires a public access in some of existing test utils from o.a.h.hdfs package

          Show
          cos Konstantin Boudnik added a comment - The fix for this requires a public access in some of existing test utils from o.a.h.hdfs package
          Hide
          cos Konstantin Boudnik added a comment -

          First test case to very normal lease recovery process.
          append() call doesn't work yet

          Show
          cos Konstantin Boudnik added a comment - First test case to very normal lease recovery process. append() call doesn't work yet
          Hide
          cos Konstantin Boudnik added a comment -

          append() is working now. Also, a missed file is attached.

          Show
          cos Konstantin Boudnik added a comment - append() is working now. Also, a missed file is attached.
          Hide
          cos Konstantin Boudnik added a comment -

          LeaseRecovery_03.* are pretty much ready. Only the logic of checking NN's reaction is left unimplemented.

          Show
          cos Konstantin Boudnik added a comment - LeaseRecovery_03.* are pretty much ready. Only the logic of checking NN's reaction is left unimplemented.
          Hide
          cos Konstantin Boudnik added a comment -

          Some more functionality is added. Although, the test can't be complete with out some future development on lease recovery side.

          Show
          cos Konstantin Boudnik added a comment - Some more functionality is added. Although, the test can't be complete with out some future development on lease recovery side.
          Hide
          cos Konstantin Boudnik added a comment -

          Mockito based unit tests for lease recovery.

          Show
          cos Konstantin Boudnik added a comment - Mockito based unit tests for lease recovery.
          Hide
          cos Konstantin Boudnik added a comment -

          Local test-patch seems to be Ok

          +1 overall.
              +1 @author.  The patch does not contain any @author tags.
              +1 tests included.  The patch appears to include 4 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 release audit.  The applied patch does not increase the total number of release audit warnings.
          

          Also

          % ant run-test-unit
          run-test-unit:
              [mkdir] Created dir: /homes/xxx/work/Hdfs.trunk/build/test/data
              [mkdir] Created dir: /homes/xxx/work/Hdfs.trunk/build/test/logs
               [copy] Copying 1 file to /homes/xxx/work/Hdfs.trunk/build/test/extraconf
               [copy] Copying 1 file to /homes/xxx/work/Hdfs.trunk/build/test/extraconf
              [junit] Running org.apache.hadoop.hdfs.server.namenode.TestNNLeaseRecovery
              [junit] Tests run: 4, Failures: 0, Errors: 0, Time elapsed: 1.5 sec
          
          Show
          cos Konstantin Boudnik added a comment - Local test-patch seems to be Ok +1 overall. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 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 release audit. The applied patch does not increase the total number of release audit warnings. Also % ant run-test-unit run-test-unit: [mkdir] Created dir: /homes/xxx/work/Hdfs.trunk/build/test/data [mkdir] Created dir: /homes/xxx/work/Hdfs.trunk/build/test/logs [copy] Copying 1 file to /homes/xxx/work/Hdfs.trunk/build/test/extraconf [copy] Copying 1 file to /homes/xxx/work/Hdfs.trunk/build/test/extraconf [junit] Running org.apache.hadoop.hdfs.server.namenode.TestNNLeaseRecovery [junit] Tests run: 4, Failures: 0, Errors: 0, Time elapsed: 1.5 sec
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12424809/HDFS-519.patch
          against trunk revision 880630.

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

          +1 tests included. The patch appears to include 4 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 release audit. The applied patch does not increase the total number of release audit warnings.

          +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/Hdfs-Patch-h5.grid.sp2.yahoo.net/115/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/115/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/115/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/115/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12424809/HDFS-519.patch against trunk revision 880630. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 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 release audit. The applied patch does not increase the total number of release audit warnings. +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/Hdfs-Patch-h5.grid.sp2.yahoo.net/115/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/115/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/115/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/115/console This message is automatically generated.
          Hide
          cos Konstantin Boudnik added a comment -

          Basically, the same patch but also removed obsolete src/test/unit/.gitignore file for some actual tests are being added into the unit test suite

          Show
          cos Konstantin Boudnik added a comment - Basically, the same patch but also removed obsolete src/test/unit/.gitignore file for some actual tests are being added into the unit test suite
          Hide
          shv Konstantin Shvachko added a comment -

          Seems that Mockito really simplifies testing.

          1. I'd propose to rename mockObjects() to mockFileBlocks(). I personally got confused and spent time looking for a mockObjects() method in Mockito framework.
          2. In testInternalReleaseLease_COMM_CONSTRUCTION() I'd rather use assertFalse(..., fsn.internalReleaseLease()) instead of assertTrue(..., ! fsn.internalReleaseLease())
          3. Why is this test in src/test/unit/? I'd expect it to be in src/test/. Currently I cannot run it from eclipse. Please let me know if there is a way.
          Show
          shv Konstantin Shvachko added a comment - Seems that Mockito really simplifies testing. I'd propose to rename mockObjects() to mockFileBlocks() . I personally got confused and spent time looking for a mockObjects() method in Mockito framework. In testInternalReleaseLease_COMM_CONSTRUCTION() I'd rather use assertFalse(..., fsn.internalReleaseLease()) instead of assertTrue(..., ! fsn.internalReleaseLease()) Why is this test in src/test/unit/ ? I'd expect it to be in src/test/ . Currently I cannot run it from eclipse. Please let me know if there is a way.
          Hide
          cos Konstantin Boudnik added a comment -

          1. Make sense
          2. Right, it seems to be easier to understand the semantic this way
          3. The reason is that these are true unit tests. Unlike the end-to-end and functional tests located in src/test/hdfs. I'd hope to separate tests from the latter location to unit and functional at some point. As for Eclipse: I believe adding src/test/unit into Eclipse .classpath file should fix the issue. I'll see if it works and will update the patch accordingly.

          Show
          cos Konstantin Boudnik added a comment - 1. Make sense 2. Right, it seems to be easier to understand the semantic this way 3. The reason is that these are true unit tests. Unlike the end-to-end and functional tests located in src/test/hdfs . I'd hope to separate tests from the latter location to unit and functional at some point. As for Eclipse: I believe adding src/test/unit into Eclipse .classpath file should fix the issue. I'll see if it works and will update the patch accordingly.
          Hide
          shv Konstantin Shvachko added a comment -

          > 3. adding src/test/unit

          Would it make sense to split the test directory later on in a separate patch? Introducing a new teset directory for one test looks odd. Besides, this will need to go into 0.21, we probably don't plan to split directories in released versions.

          Show
          shv Konstantin Shvachko added a comment - > 3. adding src/test/unit Would it make sense to split the test directory later on in a separate patch? Introducing a new teset directory for one test looks odd. Besides, this will need to go into 0.21, we probably don't plan to split directories in released versions.
          Hide
          cos Konstantin Boudnik added a comment -

          With all due respect, I don't think we need to wait to have some critical mass of true tests to introduce this new src/test/unit directory. The split itself isn't that a big deal and it should be pretty transparent for people who are using the build.

          On the other hand, having properly place unit test will set a good example for others to follow.

          Show
          cos Konstantin Boudnik added a comment - With all due respect, I don't think we need to wait to have some critical mass of true tests to introduce this new src/test/unit directory. The split itself isn't that a big deal and it should be pretty transparent for people who are using the build. On the other hand, having properly place unit test will set a good example for others to follow.
          Hide
          cos Konstantin Boudnik added a comment -

          Addressing Konstantin's comments and fixing the issue with Eclipse' classpath.

          Show
          cos Konstantin Boudnik added a comment - Addressing Konstantin's comments and fixing the issue with Eclipse' classpath.
          Hide
          shv Konstantin Shvachko added a comment -

          > I don't think we need to wait to have some critical mass of true tests to introduce this new src/test/unit directory.

          This should be explicitly discussed in a separate jira rather than "implicitly" introduced along with a test.
          The test splitting has absolutely nothing to do with the lease recovery test.

          Show
          shv Konstantin Shvachko added a comment - > I don't think we need to wait to have some critical mass of true tests to introduce this new src/test/unit directory. This should be explicitly discussed in a separate jira rather than "implicitly" introduced along with a test. The test splitting has absolutely nothing to do with the lease recovery test.
          Hide
          cos Konstantin Boudnik added a comment -

          This should be explicitly discussed in a separate jira rather than "implicitly" introduced along with a test.

          Agree! It has been separately and explicitly discussed in HDFS-669.

          Show
          cos Konstantin Boudnik added a comment - This should be explicitly discussed in a separate jira rather than "implicitly" introduced along with a test. Agree! It has been separately and explicitly discussed in HDFS-669 .
          Hide
          shv Konstantin Shvachko added a comment -

          Ok, I missed that the directory has already been introduced.
          I would not say though it was very explicitly discussed. For example, the criteria of placing tests in one or another test directory is not clearly specified. This is actually what I wanted to avoid discussing along with lease recovery.
          Let's commit this, and come back to the test directories issue later.

          Show
          shv Konstantin Shvachko added a comment - Ok, I missed that the directory has already been introduced. I would not say though it was very explicitly discussed. For example, the criteria of placing tests in one or another test directory is not clearly specified. This is actually what I wanted to avoid discussing along with lease recovery. Let's commit this, and come back to the test directories issue later.
          Hide
          cos Konstantin Boudnik added a comment -

          I agree with Konstantin's comment. The tests placement policy has to be developed and articulated.

          Show
          cos Konstantin Boudnik added a comment - I agree with Konstantin's comment. The tests placement policy has to be developed and articulated.
          Hide
          cos Konstantin Boudnik added a comment -

          Thanks for review Konstantin.
          I've just committed this. Also, the merge back to branch-0.21 has been performed.

          Show
          cos Konstantin Boudnik added a comment - Thanks for review Konstantin. I've just committed this. Also, the merge back to branch-0.21 has been performed.
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #128 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/128/)

          Show
          hudson Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #128 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/128/ )
          Hide
          hudson Hudson added a comment -

          Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #129 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/)

          Show
          hudson Hudson added a comment - Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #129 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/129/ )
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #159 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/159/)

          Show
          hudson Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #159 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/159/ )
          Hide
          hudson Hudson added a comment -

          Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #81 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/81/)

          Show
          hudson Hudson added a comment - Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #81 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/81/ )

            People

            • Assignee:
              cos Konstantin Boudnik
              Reporter:
              cos Konstantin Boudnik
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development