Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major 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
          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 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/ )
          Hide
          Hudson added a comment -

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

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #159 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/159/ )
          Hide
          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 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 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 added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #128 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/128/ )
          Hide
          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
          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
          Konstantin Boudnik added a comment -

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

          Show
          Konstantin Boudnik added a comment - I agree with Konstantin's comment. The tests placement policy has to be developed and articulated.
          Hide
          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
          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
          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
          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
          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
          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
          Konstantin Boudnik added a comment -

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

          Show
          Konstantin Boudnik added a comment - Addressing Konstantin's comments and fixing the issue with Eclipse' classpath.
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Konstantin Boudnik added a comment -

          Mockito based unit tests for lease recovery.

          Show
          Konstantin Boudnik added a comment - Mockito based unit tests for lease recovery.
          Hide
          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
          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
          Konstantin Boudnik added a comment -

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

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

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

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

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

          Show
          Konstantin Boudnik added a comment - First test case to very normal lease recovery process. append() call doesn't work yet
          Hide
          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
          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

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development