Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-2116

Cleanup TestStreamFile and TestByteRangeInputStream

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.23.0
    • Component/s: test
    • Labels:

      Description

      TestStreamFile and TestByteRangeInputStream should use mockito. This would allow the private URLOpener class to be removed from ByteRangeInputStream.

      1. HDFS-2116.patch
        9 kB
        Plamen Jeliazkov
      2. HDFS-2116.patch
        22 kB
        Plamen Jeliazkov
      3. HDFS-2116.patch
        21 kB
        Plamen Jeliazkov
      4. HDFS-2116[2].patch
        18 kB
        Plamen Jeliazkov
      5. HDFS-2116.patch
        13 kB
        Plamen Jeliazkov

        Activity

        Arun C Murthy made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Uma Maheswara Rao G made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Fix Version/s 0.23.0 [ 12315571 ]
        Resolution Fixed [ 1 ]
        Hide
        Uma Maheswara Rao G added a comment -

        Already committed in trunk!

        Show
        Uma Maheswara Rao G added a comment - Already committed in trunk!
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #733 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/733/)
        HDFS-2116. Use Mokito in TestStreamFile and TestByteRangeInputStream. Contributed by Plamen Jeliazkov.

        shv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1149770
        Files :

        • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestStreamFile.java
        • /hadoop/common/trunk/hdfs/CHANGES.txt
        • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestByteRangeInputStream.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #733 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/733/ ) HDFS-2116 . Use Mokito in TestStreamFile and TestByteRangeInputStream. Contributed by Plamen Jeliazkov. shv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1149770 Files : /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestStreamFile.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestByteRangeInputStream.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #800 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/800/)
        HDFS-2116. Use Mokito in TestStreamFile and TestByteRangeInputStream. Contributed by Plamen Jeliazkov.

        shv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1149770
        Files :

        • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestStreamFile.java
        • /hadoop/common/trunk/hdfs/CHANGES.txt
        • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestByteRangeInputStream.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #800 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/800/ ) HDFS-2116 . Use Mokito in TestStreamFile and TestByteRangeInputStream. Contributed by Plamen Jeliazkov. shv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1149770 Files : /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestStreamFile.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestByteRangeInputStream.java
        Hide
        Konstantin Shvachko added a comment -

        I just committed this.
        Congratulations Plamen.

        Show
        Konstantin Shvachko added a comment - I just committed this. Congratulations Plamen.
        Konstantin Shvachko made changes -
        Assignee Plamen Jeliazkov [ zero45 ]
        Hide
        Konstantin Shvachko added a comment -

        Looks good now.
        +1

        Show
        Konstantin Shvachko added a comment - Looks good now. +1
        Plamen Jeliazkov made changes -
        Attachment HDFS-2116.patch [ 12487567 ]
        Hide
        Plamen Jeliazkov added a comment -

        Missed some deltas. Should be a clean patch. Also fixed the last javac warning.

        Thanks Konstantin.

        Show
        Plamen Jeliazkov added a comment - Missed some deltas. Should be a clean patch. Also fixed the last javac warning. Thanks Konstantin.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12487471/HDFS-2116%5B2%5D.patch
        against trunk revision 1149455.

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

        +1 tests included. The patch appears to include 9 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        -1 javac. The applied patch generated 24 javac compiler warnings (more than the trunk's current 23 warnings).

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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.

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/997//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/997//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/997//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/12487471/HDFS-2116%5B2%5D.patch against trunk revision 1149455. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 24 javac compiler warnings (more than the trunk's current 23 warnings). +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/997//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/997//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/997//console This message is automatically generated.
        Plamen Jeliazkov made changes -
        Attachment HDFS-2116[2].patch [ 12487471 ]
        Hide
        Plamen Jeliazkov added a comment -

        Removed many formatting changes from the patch while maintaining valuable changes.

        Show
        Plamen Jeliazkov added a comment - Removed many formatting changes from the patch while maintaining valuable changes.
        Hide
        Konstantin Shvachko added a comment -

        There is lot's of formatting changes in the patch.
        Could you please get rid of those and just leave the actual changes you are making.

        Show
        Konstantin Shvachko added a comment - There is lot's of formatting changes in the patch. Could you please get rid of those and just leave the actual changes you are making.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12486889/HDFS-2116.patch
        against trunk revision 1148981.

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

        +1 tests included. The patch appears to include 10 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        -1 javac. The applied patch generated 24 javac compiler warnings (more than the trunk's current 23 warnings).

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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.

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/991//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/991//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/991//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/12486889/HDFS-2116.patch against trunk revision 1148981. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 10 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 24 javac compiler warnings (more than the trunk's current 23 warnings). +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/991//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/991//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/991//console This message is automatically generated.
        Plamen Jeliazkov made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Plamen Jeliazkov made changes -
        Attachment HDFS-2116.patch [ 12486889 ]
        Hide
        Plamen Jeliazkov added a comment -

        Here are the fixes so far. This patch will address the code removal; but probably not the dependency issue.

        Thank you for your comments and patience, Uma.

        Show
        Plamen Jeliazkov added a comment - Here are the fixes so far. This patch will address the code removal; but probably not the dependency issue. Thank you for your comments and patience, Uma.
        Hide
        Plamen Jeliazkov added a comment -

        The only issue I see is in the release audit:

        patchReleaseAuditWarnings.txt
        ivy-resolve-test:
        [ivy:resolve] 
        [ivy:resolve] :: problems summary ::
        [ivy:resolve] :::: WARNINGS
        [ivy:resolve] 		module not found: org.apache.hadoop#hadoop-common-test;0.23.0-SNAPSHOT
        [ivy:resolve] 	==== apache-snapshot: tried
        [ivy:resolve] 	  https://repository.apache.org/content/repositories/snapshots/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.pom
        [ivy:resolve] 	  -- artifact org.apache.hadoop#hadoop-common-test;0.23.0-SNAPSHOT!hadoop-common-test.jar:
        [ivy:resolve] 	  https://repository.apache.org/content/repositories/snapshots/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.jar
        [ivy:resolve] 	==== maven2: tried
        [ivy:resolve] 	  http://repo1.maven.org/maven2/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.pom
        [ivy:resolve] 	  -- artifact org.apache.hadoop#hadoop-common-test;0.23.0-SNAPSHOT!hadoop-common-test.jar:
        [ivy:resolve] 	  http://repo1.maven.org/maven2/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.jar
        [ivy:resolve] 		::::::::::::::::::::::::::::::::::::::::::::::
        [ivy:resolve] 		::          UNRESOLVED DEPENDENCIES         ::
        [ivy:resolve] 		::::::::::::::::::::::::::::::::::::::::::::::
        [ivy:resolve] 		:: org.apache.hadoop#hadoop-common-test;0.23.0-SNAPSHOT: not found
        [ivy:resolve] 		::::::::::::::::::::::::::::::::::::::::::::::
        [ivy:resolve] :::: ERRORS
        [ivy:resolve] 	SERVER ERROR: Internal Server Error url=https://repository.apache.org/content/repositories/snapshots/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.pom
        [ivy:resolve] 	SERVER ERROR: Service Temporarily Unavailable url=https://repository.apache.org/content/repositories/snapshots/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.jar
        [ivy:resolve] 
        [ivy:resolve] :: USE VERBOSE OR DEBUG MESSAGE LEVEL FOR MORE DETAILS
        
        BUILD FAILED
        /grid/0/hudson/hudson-slave/workspace/PreCommit-HDFS-Build/trunk/build.xml:1957: impossible to resolve dependencies:
        	resolve failed - see output for details
        
        
        Total time: 19 seconds
        

        I am a little unclear how to resolve it though – is my test fix causing a dependency issue?

        TestByteRangeInputStream.java :
        1) Of course.
        2) Interesting, it appears as an issue on my Eclipse; I can certainly remove it though.
        3) The original test printed MockURL's getMsg() and compared it to a hardcoded string. The string returned by getMsg() was a combined string containing the URL string and the string info returned from .openConnection.requestProperty("Range"). I tried to preserve the test by testing for the URL and the byte info separately. I see there is no point in the URL check though, I will remove it.

        TestStreamFile.java :
        1) I will need to look into what I am doing here; because without those warnings I will run into other warnings stating that the Exception is not thrown and is therefore unneeded; and when I delete the Exception it says I do need it; thus I went with a warning suppression; I will remove it now though.
        2) Of course.

        Show
        Plamen Jeliazkov added a comment - The only issue I see is in the release audit: patchReleaseAuditWarnings.txt ivy-resolve-test: [ivy:resolve] [ivy:resolve] :: problems summary :: [ivy:resolve] :::: WARNINGS [ivy:resolve] module not found: org.apache.hadoop#hadoop-common-test;0.23.0-SNAPSHOT [ivy:resolve] ==== apache-snapshot: tried [ivy:resolve] https: //repository.apache.org/content/repositories/snapshots/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.pom [ivy:resolve] -- artifact org.apache.hadoop#hadoop-common-test;0.23.0-SNAPSHOT!hadoop-common-test.jar: [ivy:resolve] https: //repository.apache.org/content/repositories/snapshots/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.jar [ivy:resolve] ==== maven2: tried [ivy:resolve] http: //repo1.maven.org/maven2/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.pom [ivy:resolve] -- artifact org.apache.hadoop#hadoop-common-test;0.23.0-SNAPSHOT!hadoop-common-test.jar: [ivy:resolve] http: //repo1.maven.org/maven2/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.jar [ivy:resolve] :::::::::::::::::::::::::::::::::::::::::::::: [ivy:resolve] :: UNRESOLVED DEPENDENCIES :: [ivy:resolve] :::::::::::::::::::::::::::::::::::::::::::::: [ivy:resolve] :: org.apache.hadoop#hadoop-common-test;0.23.0-SNAPSHOT: not found [ivy:resolve] :::::::::::::::::::::::::::::::::::::::::::::: [ivy:resolve] :::: ERRORS [ivy:resolve] SERVER ERROR: Internal Server Error url=https: //repository.apache.org/content/repositories/snapshots/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.pom [ivy:resolve] SERVER ERROR: Service Temporarily Unavailable url=https: //repository.apache.org/content/repositories/snapshots/org/apache/hadoop/hadoop-common-test/0.23.0-SNAPSHOT/hadoop-common-test-0.23.0-SNAPSHOT.jar [ivy:resolve] [ivy:resolve] :: USE VERBOSE OR DEBUG MESSAGE LEVEL FOR MORE DETAILS BUILD FAILED /grid/0/hudson/hudson-slave/workspace/PreCommit-HDFS-Build/trunk/build.xml:1957: impossible to resolve dependencies: resolve failed - see output for details Total time: 19 seconds I am a little unclear how to resolve it though – is my test fix causing a dependency issue? TestByteRangeInputStream.java : 1) Of course. 2) Interesting, it appears as an issue on my Eclipse; I can certainly remove it though. 3) The original test printed MockURL's getMsg() and compared it to a hardcoded string. The string returned by getMsg() was a combined string containing the URL string and the string info returned from .openConnection.requestProperty("Range"). I tried to preserve the test by testing for the URL and the byte info separately. I see there is no point in the URL check though, I will remove it. TestStreamFile.java : 1) I will need to look into what I am doing here; because without those warnings I will run into other warnings stating that the Exception is not thrown and is therefore unneeded; and when I delete the Exception it says I do need it; thus I went with a warning suppression; I will remove it now though. 2) Of course.
        Uma Maheswara Rao G made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Uma Maheswara Rao G added a comment -

        Cancelling the patch to address the -1s from hudson result and comments.

        Show
        Uma Maheswara Rao G added a comment - Cancelling the patch to address the -1s from hudson result and comments.
        Hide
        Uma Maheswara Rao G added a comment -

        Hi Plamen,

        Hudson result shows many -1s. Can you please check them?

        There are some more comments to address.

        TestByteRangeInputStream.java :

        1. unecessary commented code is there. Can you please remove ?

          //class MockURL extends URLOpener {
        //  String msg;
        //  public int responseCode = -1;
        //  
        //  public MockURL(URL u) {
        //    super(u);
        //  }
        //
        //  public MockURL(String s) throws MalformedURLException {
        //    this(new URL(s));
        //  }
        //.............
        //.............
        ..............
         

        2.There is one unncessary supress warning tag present.
        @SuppressWarnings("hiding")

        3. what is the purpose of below assertion?

            rspy.setURL(new URL("http://resolvedur1/"));
            
            is.seek(100);
            is.read();
        
            assertEquals("Seek to 100 bytes made incorrectly (URL Check)",
                "http://resolvedurl/", rspy.getURL().toString());
            

        Here whatever URL you set, your assertion will pass. Looks you are returning hardcoded URL.

             
            public URL getURL() {
            URL u = null;
            try {
              u = new URL("http://resolvedurl/");
            } catch (Exception e) {
              System.out.println(e.getMessage());
            }
            return u;
          }
          

        TestStreamFile.java :

        1. Looks there are unnecessary suppressWarnings tags.

        2. Mockito.verify(response, Mockito.times(1)).setStatus(416);

        Here you can remove Mockito.times(1)

        Mockito.verify(response).setStatus(416); 

        whihch is equivalent to

        Mockito.verify(response, Mockito.times(1)).setStatus(416);
        Show
        Uma Maheswara Rao G added a comment - Hi Plamen, Hudson result shows many -1s. Can you please check them? There are some more comments to address. TestByteRangeInputStream.java : 1. unecessary commented code is there. Can you please remove ? //class MockURL extends URLOpener { // String msg; // public int responseCode = -1; // // public MockURL(URL u) { // super (u); // } // // public MockURL( String s) throws MalformedURLException { // this ( new URL(s)); // } //............. //............. .............. 2.There is one unncessary supress warning tag present. @SuppressWarnings("hiding") 3. what is the purpose of below assertion? rspy.setURL( new URL( "http: //resolvedur1/" )); is.seek(100); is.read(); assertEquals( "Seek to 100 bytes made incorrectly (URL Check)" , "http: //resolvedurl/" , rspy.getURL().toString()); Here whatever URL you set, your assertion will pass. Looks you are returning hardcoded URL. public URL getURL() { URL u = null ; try { u = new URL( "http: //resolvedurl/" ); } catch (Exception e) { System .out.println(e.getMessage()); } return u; } TestStreamFile.java : 1. Looks there are unnecessary suppressWarnings tags. 2. Mockito.verify(response, Mockito.times(1)).setStatus(416); Here you can remove Mockito.times(1) Mockito.verify(response).setStatus(416); whihch is equivalent to Mockito.verify(response, Mockito.times(1)).setStatus(416);
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12486697/HDFS-2116.patch
        against trunk revision 1147340.

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

        +1 tests included. The patch appears to include 11 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 (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these core unit tests:

        -1 contrib tests. The patch failed contrib unit tests.

        -1 system test framework. The patch failed system test framework compile.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/950//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/950//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/950//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/12486697/HDFS-2116.patch against trunk revision 1147340. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these core unit tests: -1 contrib tests. The patch failed contrib unit tests. -1 system test framework. The patch failed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/950//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/950//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/950//console This message is automatically generated.
        Plamen Jeliazkov made changes -
        Attachment HDFS-2116.patch [ 12486697 ]
        Hide
        Plamen Jeliazkov added a comment -

        Went and grabbed the trunk revision and worked from there. Followed comments – editted TestStreamFile.java as well; I could not figure out an appropiate way to use Mockito with FSInputStream.

        Removed hard coded tabs – space indenting is by 2.

        Show
        Plamen Jeliazkov added a comment - Went and grabbed the trunk revision and worked from there. Followed comments – editted TestStreamFile.java as well; I could not figure out an appropiate way to use Mockito with FSInputStream. Removed hard coded tabs – space indenting is by 2.
        Hide
        Plamen Jeliazkov added a comment -

        Thanks I will definitely get on that. Yeah, that assertion was not part of my code though – I believe it was there prior to my patch as well.

        Same thing goes with the generic exception catching, they were there prior to my patch.

        I think the issue here is that I am working from the 0.22.0 branch; where is this is updating straight to trunk.

        Show
        Plamen Jeliazkov added a comment - Thanks I will definitely get on that. Yeah, that assertion was not part of my code though – I believe it was there prior to my patch as well. Same thing goes with the generic exception catching, they were there prior to my patch. I think the issue here is that I am working from the 0.22.0 branch; where is this is updating straight to trunk.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12486660/HDFS-2116.patch
        against trunk revision 1147184.

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

        +1 tests included. The patch appears to include 7 new or modified tests.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/947//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/12486660/HDFS-2116.patch against trunk revision 1147184. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/947//console This message is automatically generated.
        Hide
        Uma Maheswara Rao G added a comment -

        Hi Plamen,

        I quickly gone through your patch.
        Please find my comments above.

        Show
        Uma Maheswara Rao G added a comment - Hi Plamen, I quickly gone through your patch. Please find my comments above.
        Hide
        Uma Maheswara Rao G added a comment -

        1. What is the purpose of this assertion?

         
          assertNull("Seek to 101 should not result in another request", null);
        

        2.looks your coding formatter is not correct. Hadoop intendation is two spaces.
        3.Patch contains hard core tabs, can you pleasse remove them?
        4.

                  } catch (Exception e) {
                  +			System.out.println(e.getMessage());
                  +		}
         

        dont catch the generic exceptions.

        5. when i try to apply patch, looks there are some conflicts with the trunk. can you please update properly?
        perhaps you can try applying the parch.

        6.what about TestStreamFile cleanup?

        Show
        Uma Maheswara Rao G added a comment - 1. What is the purpose of this assertion? assertNull( "Seek to 101 should not result in another request" , null ); 2.looks your coding formatter is not correct. Hadoop intendation is two spaces. 3.Patch contains hard core tabs, can you pleasse remove them? 4. } catch (Exception e) { + System .out.println(e.getMessage()); + } dont catch the generic exceptions. 5. when i try to apply patch, looks there are some conflicts with the trunk. can you please update properly? perhaps you can try applying the parch. 6.what about TestStreamFile cleanup?
        Plamen Jeliazkov made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Plamen Jeliazkov made changes -
        Field Original Value New Value
        Attachment HDFS-2116.patch [ 12486660 ]
        Hide
        Plamen Jeliazkov added a comment -

        Had to get myself more familiar with Mockito, but I think this patch should do the trick.

        Tested it on my end and it passed. May still need some improvements as it is slightly hack-ish.

        Anyway, here is the patch for review!

        Show
        Plamen Jeliazkov added a comment - Had to get myself more familiar with Mockito, but I think this patch should do the trick. Tested it on my end and it passed. May still need some improvements as it is slightly hack-ish. Anyway, here is the patch for review!
        Hide
        Eli Collins added a comment -

        ant test compiles them, it does this via ant compile-hdfs-test so you can try that.

        Show
        Eli Collins added a comment - ant test compiles them, it does this via ant compile-hdfs-test so you can try that.
        Hide
        Plamen Jeliazkov added a comment -

        I was trying to work on this issue, but since I build everything via Cygwin, I can't seem to find the right ant command that will build those test cases. I have tried "ant tests" and a slew of others. :\

        I would be glad to pick up work on this otherwise though. Has anyone modified the files mentioned above and got it to build using ant?

        Show
        Plamen Jeliazkov added a comment - I was trying to work on this issue, but since I build everything via Cygwin, I can't seem to find the right ant command that will build those test cases. I have tried "ant tests" and a slew of others. :\ I would be glad to pick up work on this otherwise though. Has anyone modified the files mentioned above and got it to build using ant?
        Eli Collins created issue -

          People

          • Assignee:
            Plamen Jeliazkov
            Reporter:
            Eli Collins
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development