Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-921

Convert TestDFSClientRetries::testNotYetReplicatedErrors to Mockito

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      When TestDFSClientRetries::testNotYetReplicatedErrors was written, Mockito was not available and the NameNode was mocked by manually extending ClientProtocol and implementing all the methods, most with empty bodies. Now that we have Mockito, this code can be removed and replaced with an actual mock.

      1. HDFS-921-2.patch
        12 kB
        Jakob Homan
      2. HDFS-921.patch
        9 kB
        Jakob Homan

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        89d 18h 9m 1 Jakob Homan 25/Apr/10 21:55
        Open Open Patch Available Patch Available
        1m 28s 2 Jakob Homan 25/Apr/10 21:55
        Patch Available Patch Available Resolved Resolved
        5h 23m 1 Jakob Homan 26/Apr/10 03:19
        Resolved Resolved Closed Closed
        120d 18h 32m 1 Tom White 24/Aug/10 21:51
        Tom White made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Tom White made changes -
        Fix Version/s 0.21.0 [ 12314046 ]
        Fix Version/s 0.22.0 [ 12314241 ]
        Hide
        Konstantin Boudnik added a comment -

        If we did, TestDFSClientRetries actually probably wouldn't qualify because a new test that got added recently uses MiniDFSCluster and takes more than a minute to run.

        Yeah, you are right - I have revisited the patch and I can see it now. As for the 'loneliness': there are two more unit tests already

        Show
        Konstantin Boudnik added a comment - If we did, TestDFSClientRetries actually probably wouldn't qualify because a new test that got added recently uses MiniDFSCluster and takes more than a minute to run. Yeah, you are right - I have revisited the patch and I can see it now. As for the 'loneliness': there are two more unit tests already
        Hide
        Jakob Homan added a comment -

        Actually, I was about to suggest to move this test into src/test/unit because now it looks like a real unit test. Shall I open a separate JIRA for that? What do you think?

        The src/test/unit directory is vexing. We should definitely have a JIRA for moving those tests that can be truly called unit tests to it; I'm just afraid it'll be a lonely little directory tree at the moment. If we did, TestDFSClientRetries actually probably wouldn't qualify because a new test that got added recently uses MiniDFSCluster and takes more than a minute to run... It's worth a JIRA though.

        Show
        Jakob Homan added a comment - Actually, I was about to suggest to move this test into src/test/unit because now it looks like a real unit test. Shall I open a separate JIRA for that? What do you think? The src/test/unit directory is vexing. We should definitely have a JIRA for moving those tests that can be truly called unit tests to it; I'm just afraid it'll be a lonely little directory tree at the moment. If we did, TestDFSClientRetries actually probably wouldn't qualify because a new test that got added recently uses MiniDFSCluster and takes more than a minute to run... It's worth a JIRA though.
        Hide
        Konstantin Boudnik added a comment -

        Handling imports automatically is a good thing: stale imports are being eliminated and all that.
        Actually, I was about to suggest to move this test into src/test/unit because now it looks like a real unit test. Shall I open a separate JIRA for that? What do you think?

        Show
        Konstantin Boudnik added a comment - Handling imports automatically is a good thing: stale imports are being eliminated and all that. Actually, I was about to suggest to move this test into src/test/unit because now it looks like a real unit test. Shall I open a separate JIRA for that? What do you think?
        Jakob Homan made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Fix Version/s 0.22.0 [ 12314241 ]
        Resolution Fixed [ 1 ]
        Hide
        Jakob Homan added a comment -

        I've committed this. Resolving as fixed.

        Show
        Jakob Homan added a comment - I've committed this. Resolving as fixed.
        Hide
        Jakob Homan added a comment -

        Hudson ran but didn't post:

             [exec] -1 overall.  Here are the results of testing the latest attachment 
             [exec]   http://issues.apache.org/jira/secure/attachment/12442795/HDFS-921-2.patch
             [exec]   against trunk revision 937855.
             [exec] 
             [exec]     +1 @author.  The patch does not contain any @author tags.
             [exec] 
             [exec]     +1 tests included.  The patch appears to include 3 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
             [exec] 
             [exec]     +1 core tests.  The patch passed core unit tests.
             [exec] 
             [exec]     -1 contrib tests.  The patch failed contrib unit tests.
        

        Contrib test failure is unrelated.

        Show
        Jakob Homan added a comment - Hudson ran but didn't post: [exec] -1 overall. Here are the results of testing the latest attachment [exec] http://issues.apache.org/jira/secure/attachment/12442795/HDFS-921-2.patch [exec] against trunk revision 937855. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 core tests. The patch passed core unit tests. [exec] [exec] -1 contrib tests. The patch failed contrib unit tests. Contrib test failure is unrelated.
        Jakob Homan made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Jakob Homan added a comment -

        Triggering Hudson on updated patch.

        Show
        Jakob Homan added a comment - Triggering Hudson on updated patch.
        Jakob Homan made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Jakob Homan made changes -
        Attachment HDFS-921-2.patch [ 12442795 ]
        Hide
        Jakob Homan added a comment -

        Patch went stale. Updated version. Uses new parameter to addBlock. Otherwise unchanged.
        Cos: I'm a lazy, lazy man. I let Eclipse handle all my imports...

        Show
        Jakob Homan added a comment - Patch went stale. Updated version. Uses new parameter to addBlock. Otherwise unchanged. Cos: I'm a lazy, lazy man. I let Eclipse handle all my imports...
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 3 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/205/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/205/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/205/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/205/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/12431401/HDFS-921.patch against trunk revision 902330. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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/205/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/205/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/205/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/205/console This message is automatically generated.
        Hide
        Konstantin Boudnik added a comment -

        +1 patch looks good. It does eliminate about 140 lines of code and, which a more important, reduces the execution time of the test from about 160 secs down to 70 odd! Also, the code is a way more clean now. Great.

        One nit though (non-mandatory): I'd group mockito.* related imports together, but it is a matter of personal taste.

        Show
        Konstantin Boudnik added a comment - +1 patch looks good. It does eliminate about 140 lines of code and, which a more important, reduces the execution time of the test from about 160 secs down to 70 odd! Also, the code is a way more clean now. Great. One nit though (non-mandatory): I'd group mockito.* related imports together, but it is a matter of personal taste.
        Jakob Homan made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Jakob Homan added a comment -

        submitting patch.

        Show
        Jakob Homan added a comment - submitting patch.
        Jakob Homan made changes -
        Field Original Value New Value
        Attachment HDFS-921.patch [ 12431401 ]
        Hide
        Jakob Homan added a comment -

        Attaching patch that removes the old, manually mocked Namenode and provides equivalent functionality via Mockito.

        Show
        Jakob Homan added a comment - Attaching patch that removes the old, manually mocked Namenode and provides equivalent functionality via Mockito.
        Jakob Homan created issue -

          People

          • Assignee:
            Jakob Homan
            Reporter:
            Jakob Homan
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development