Details

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

      Description

      We need some unit tests for FileStatus to prevent problems like those we recently had on HADOOP-6796 and MAPREDUCE-1858.

      1. HADOOP-6825.0.patch
        3 kB
        Rodrigo Schmidt

        Issue Links

          Activity

          Hide
          Rodrigo Schmidt added a comment -

          First patch

          Show
          Rodrigo Schmidt added a comment - First patch
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12446991/HADOOP-6825.0.patch
          against trunk revision 954165.

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

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

          -1 javadoc. The javadoc tool appears to have generated 1 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/Hadoop-Patch-h4.grid.sp2.yahoo.net/583/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/583/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/583/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/583/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/12446991/HADOOP-6825.0.patch against trunk revision 954165. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 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/Hadoop-Patch-h4.grid.sp2.yahoo.net/583/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/583/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/583/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/583/console This message is automatically generated.
          Hide
          Rodrigo Schmidt added a comment -

          warnings seem to be coming from security files, not my patch:

          [exec] [javadoc] Constructing Javadoc information...
          [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/KerberosName.java:31: warning: sun.security.krb5.Config is Sun prop\
          rietary API and may be removed in a future release
          [exec] [javadoc] import sun.security.krb5.Config;
          [exec] [javadoc] ^
          [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/KerberosName.java:32: warning: sun.security.krb5.KrbException is Su\
          n proprietary API and may be removed in a future release
          [exec] [javadoc] import sun.security.krb5.KrbException;
          [exec] [javadoc] ^
          [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/KerberosName.java:81: warning: sun.security.krb5.Config is Sun prop\
          rietary API and may be removed in a future release
          [exec] [javadoc] private static Config kerbConf;
          [exec] [javadoc] ^
          [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/SecurityUtil.java:33: warning: sun.security.jgss.krb5.Krb5Util is S\
          un proprietary API and may be removed in a future release
          [exec] [javadoc] import sun.security.jgss.krb5.Krb5Util;
          [exec] [javadoc] ^
          [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/SecurityUtil.java:34: warning: sun.security.krb5.Credentials is Sun\
          proprietary API and may be removed in a future release
          [exec] [javadoc] import sun.security.krb5.Credentials;
          [exec] [javadoc] ^
          [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/SecurityUtil.java:35: warning: sun.security.krb5.PrincipalName is S\
          un proprietary API and may be removed in a future release
          [exec] [javadoc] import sun.security.krb5.PrincipalName;
          [exec] [javadoc] ^
          [exec] [javadoc] ExcludePrivateAnnotationsStandardDoclet
          [exec] [javadoc] Standard Doclet version 1.6.0_11
          [exec] [javadoc] Building tree for all the packages and classes...
          ...
          [exec] [javadoc] Generating /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/build/docs/api/stylesheet.css...
          [exec] [javadoc] 6 warnings

          Show
          Rodrigo Schmidt added a comment - warnings seem to be coming from security files, not my patch: [exec] [javadoc] Constructing Javadoc information... [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/KerberosName.java:31: warning: sun.security.krb5.Config is Sun prop\ rietary API and may be removed in a future release [exec] [javadoc] import sun.security.krb5.Config; [exec] [javadoc] ^ [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/KerberosName.java:32: warning: sun.security.krb5.KrbException is Su\ n proprietary API and may be removed in a future release [exec] [javadoc] import sun.security.krb5.KrbException; [exec] [javadoc] ^ [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/KerberosName.java:81: warning: sun.security.krb5.Config is Sun prop\ rietary API and may be removed in a future release [exec] [javadoc] private static Config kerbConf; [exec] [javadoc] ^ [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/SecurityUtil.java:33: warning: sun.security.jgss.krb5.Krb5Util is S\ un proprietary API and may be removed in a future release [exec] [javadoc] import sun.security.jgss.krb5.Krb5Util; [exec] [javadoc] ^ [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/SecurityUtil.java:34: warning: sun.security.krb5.Credentials is Sun\ proprietary API and may be removed in a future release [exec] [javadoc] import sun.security.krb5.Credentials; [exec] [javadoc] ^ [exec] [javadoc] /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/src/java/org/apache/hadoop/security/SecurityUtil.java:35: warning: sun.security.krb5.PrincipalName is S\ un proprietary API and may be removed in a future release [exec] [javadoc] import sun.security.krb5.PrincipalName; [exec] [javadoc] ^ [exec] [javadoc] ExcludePrivateAnnotationsStandardDoclet [exec] [javadoc] Standard Doclet version 1.6.0_11 [exec] [javadoc] Building tree for all the packages and classes... ... [exec] [javadoc] Generating /grid/0/hudson/hudson-slave/workspace/Hadoop-Patch-h4.grid.sp2.yahoo.net/trunk/build/docs/api/stylesheet.css... [exec] [javadoc] 6 warnings
          Hide
          Rodrigo Schmidt added a comment -

          Eli, Jakob, can one of you review this patch. It's good to have tests for FileStatus in hadoop common (it's kind of bad to rely only on unit tests that are in HDFS or MAPREDUCE).

          Show
          Rodrigo Schmidt added a comment - Eli, Jakob, can one of you review this patch. It's good to have tests for FileStatus in hadoop common (it's kind of bad to rely only on unit tests that are in HDFS or MAPREDUCE).
          Hide
          Eli Collins added a comment -

          I might be missing something but doesn't this just test that the FileStatus fields are persisted, ie that the writable code works? (ie a FileStatus can be written and read back).

          Show
          Eli Collins added a comment - I might be missing something but doesn't this just test that the FileStatus fields are persisted, ie that the writable code works? (ie a FileStatus can be written and read back).
          Hide
          Rodrigo Schmidt added a comment -

          It also tests that an empty or partially completed FileStatus can be overwritten (problem with MAPREDUCE-1858). Simple and effective.

          I think we should have at least some test for FileStatus inside hadoop-common.

          At the moment, a complicated patch could obfuscate changes to FileStatus and break MAPREDUCE or HDFS.

          Show
          Rodrigo Schmidt added a comment - It also tests that an empty or partially completed FileStatus can be overwritten (problem with MAPREDUCE-1858 ). Simple and effective. I think we should have at least some test for FileStatus inside hadoop-common. At the moment, a complicated patch could obfuscate changes to FileStatus and break MAPREDUCE or HDFS.
          Hide
          Rodrigo Schmidt added a comment -

          Any further comments? Can this be committed?

          Show
          Rodrigo Schmidt added a comment - Any further comments? Can this be committed?
          Hide
          Eli Collins added a comment -

          Thanks for the explanation.

          +1 I'll commit this soon.

          Show
          Eli Collins added a comment - Thanks for the explanation. +1 I'll commit this soon.
          Hide
          Eli Collins added a comment -

          I've committed this. Thanks Rodrigo!

          Show
          Eli Collins added a comment - I've committed this. Thanks Rodrigo!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #324 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/324/)
          HADOOP-6825. FileStatus needs unit tests. Contributed by Rodrigo Schmidt.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #324 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/324/ ) HADOOP-6825 . FileStatus needs unit tests. Contributed by Rodrigo Schmidt.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #392 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/392/)
          HADOOP-6825. FileStatus needs unit tests. Contributed by Rodrigo Schmidt.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #392 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/392/ ) HADOOP-6825 . FileStatus needs unit tests. Contributed by Rodrigo Schmidt.

            People

            • Assignee:
              Rodrigo Schmidt
              Reporter:
              Rodrigo Schmidt
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development