Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.0.0-alpha
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The assertion of this test is testing for equality, as the generated classpath file is in the classpath the test fails.

      Instead, the test should test for the expected path elements to be in the classspath.

      1. MAPREDUCE-4098.patch
        2 kB
        Alejandro Abdelnur

        Activity

        Hide
        Alejandro Abdelnur added a comment -

        testing for PWD to be at the beginning of the classpath and the YARN_APPLICATION_CLASSPATH to be in the classpath.

        Show
        Alejandro Abdelnur added a comment - testing for PWD to be at the beginning of the classpath and the YARN_APPLICATION_CLASSPATH to be in the classpath.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12521129/MAPREDUCE-4098.patch
        against trunk revision .

        +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 eclipse:eclipse. The patch built with eclipse:eclipse.

        +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 unit tests:
        org.apache.hadoop.yarn.server.resourcemanager.TestClientRMService
        org.apache.hadoop.yarn.server.resourcemanager.resourcetracker.TestNMExpiry
        org.apache.hadoop.yarn.server.resourcemanager.TestAMAuthorization
        org.apache.hadoop.yarn.server.resourcemanager.TestApplicationACLs

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2132//testReport/
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2132//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/12521129/MAPREDUCE-4098.patch against trunk revision . +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 eclipse:eclipse. The patch built with eclipse:eclipse. +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 unit tests: org.apache.hadoop.yarn.server.resourcemanager.TestClientRMService org.apache.hadoop.yarn.server.resourcemanager.resourcetracker.TestNMExpiry org.apache.hadoop.yarn.server.resourcemanager.TestAMAuthorization org.apache.hadoop.yarn.server.resourcemanager.TestApplicationACLs +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2132//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2132//console This message is automatically generated.
        Hide
        Ahmed Radwan added a comment -

        Thanks Tucu!

        The environment CLASSPATH is populated using the value for YarnConfiguration.YARN_APPLICATION_CLASSPATH from yarn-default.xml and then the test again checks if it contains the classpath from the YarnConfiguration.YARN_APPLICATION_CLASSPATH value, so the test will always succeed regardless of the value set for YarnConfiguration.YARN_APPLICATION_CLASSPATH.

        Do we need to keep the check against the hardcoded classpath values so the test will fail if they were changed in yarn-default.xml?

        Show
        Ahmed Radwan added a comment - Thanks Tucu! The environment CLASSPATH is populated using the value for YarnConfiguration.YARN_APPLICATION_CLASSPATH from yarn-default.xml and then the test again checks if it contains the classpath from the YarnConfiguration.YARN_APPLICATION_CLASSPATH value, so the test will always succeed regardless of the value set for YarnConfiguration.YARN_APPLICATION_CLASSPATH. Do we need to keep the check against the hardcoded classpath values so the test will fail if they were changed in yarn-default.xml?
        Hide
        Ahmed Radwan added a comment -

        So you still check for "contains" instead of equality (per your changes), but the check will remain against hardcoded classpath values?

        Show
        Ahmed Radwan added a comment - So you still check for "contains" instead of equality (per your changes), but the check will remain against hardcoded classpath values?
        Hide
        Alejandro Abdelnur added a comment -

        @Ahmed, as you indicated, the new assertion verifies that the YARN_APPLICATION_CLASSPATH value is in the resulting classpath. If we change the default value, the test will still pass. This seems more correct than checking against a hardcoded value. I don't think we need check for hardcoded values, at least not in this test. If you see value in that, we should have an additional test checking the hardcoded value; personally I don't think such test is needed. Regarding your second comment, the check for contains is because the classpath is massaged to prefix '$PWD' and to postfix 'job.jar' and '$PWD/*'.

        Show
        Alejandro Abdelnur added a comment - @Ahmed, as you indicated, the new assertion verifies that the YARN_APPLICATION_CLASSPATH value is in the resulting classpath. If we change the default value, the test will still pass. This seems more correct than checking against a hardcoded value. I don't think we need check for hardcoded values, at least not in this test. If you see value in that, we should have an additional test checking the hardcoded value; personally I don't think such test is needed. Regarding your second comment, the check for contains is because the classpath is massaged to prefix '$PWD' and to postfix 'job.jar' and '$PWD/*'.
        Hide
        Harsh J added a comment -

        +1, this looks good to me. Applied and test passes.

        There are reported test failures. I went over them but they appear unrelated to this change. If you too reach the same conclusion, please do state so and then resolve.

        Show
        Harsh J added a comment - +1, this looks good to me. Applied and test passes. There are reported test failures. I went over them but they appear unrelated to this change. If you too reach the same conclusion, please do state so and then resolve.
        Hide
        Ahmed Radwan added a comment -

        +1 thanks tucu.

        Show
        Ahmed Radwan added a comment - +1 thanks tucu.
        Hide
        Alejandro Abdelnur added a comment -

        Thanks Harsh. Yes, test failures seem unrelated, it looks like a zombie JVM with minicluster is around (YARN minicluster uses fixed ports)

        Show
        Alejandro Abdelnur added a comment - Thanks Harsh. Yes, test failures seem unrelated, it looks like a zombie JVM with minicluster is around (YARN minicluster uses fixed ports)
        Hide
        Alejandro Abdelnur added a comment -

        committed to trunk and branch-2

        Show
        Alejandro Abdelnur added a comment - committed to trunk and branch-2
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2065 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2065/)
        MAPREDUCE-4098. TestMRApps testSetClasspath fails (tucu) (Revision 1309432)

        Result = SUCCESS
        tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2065 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2065/ ) MAPREDUCE-4098 . TestMRApps testSetClasspath fails (tucu) (Revision 1309432) Result = SUCCESS tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #1990 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1990/)
        MAPREDUCE-4098. TestMRApps testSetClasspath fails (tucu) (Revision 1309432)

        Result = SUCCESS
        tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1990 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1990/ ) MAPREDUCE-4098 . TestMRApps testSetClasspath fails (tucu) (Revision 1309432) Result = SUCCESS tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2003 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2003/)
        MAPREDUCE-4098. TestMRApps testSetClasspath fails (tucu) (Revision 1309432)

        Result = ABORTED
        tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2003 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2003/ ) MAPREDUCE-4098 . TestMRApps testSetClasspath fails (tucu) (Revision 1309432) Result = ABORTED tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1006 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1006/)
        MAPREDUCE-4098. TestMRApps testSetClasspath fails (tucu) (Revision 1309432)

        Result = FAILURE
        tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1006 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1006/ ) MAPREDUCE-4098 . TestMRApps testSetClasspath fails (tucu) (Revision 1309432) Result = FAILURE tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1041 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1041/)
        MAPREDUCE-4098. TestMRApps testSetClasspath fails (tucu) (Revision 1309432)

        Result = SUCCESS
        tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1041 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1041/ ) MAPREDUCE-4098 . TestMRApps testSetClasspath fails (tucu) (Revision 1309432) Result = SUCCESS tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1309432 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/test/java/org/apache/hadoop/mapreduce/v2/util/TestMRApps.java

          People

          • Assignee:
            Alejandro Abdelnur
            Reporter:
            Alejandro Abdelnur
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development