Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-670

Create target for 10 minute patch test build for mapreduce

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: build
    • Labels:
      None
    • Release Note:
      Added a new target 'test-commit' to the build.xml file which runs tests specified in the file src/test/commit-tests. The tests specified in src/test/commit-tests should provide maximum coverage and all the tests should run within 10mins.

      Description

      Creating a new Jira to track HADOOP-5628 for MapReduce

      1. FastTestsInfo.xls
        57 kB
        Jothi Padmanabhan
      2. mapred-670.patch
        4 kB
        Jothi Padmanabhan
      3. mapred-670-v1.patch
        4 kB
        Jothi Padmanabhan
      4. mapred-670-v2.patch
        8 kB
        Jothi Padmanabhan
      5. mapreduce-670-y20.patch
        9 kB
        Hemanth Yamijala

        Issue Links

          Activity

          Hide
          Jothi Padmanabhan added a comment -

          As a process of getting to this target, one of the activities was to identify a subset of tests that would run in close to 10 minutes and would have a reasonable coverage. The idea is to have the main core flow/ core components well tested by the fast test suite and the corner cases/library classes could afford to be left out. Needless to say, the entire test suite will be a part of the nightly test cycle.

          Attaching a spread sheet as a first step in this identification/classification of test suites into Fast Tests. The run time of these tests is marginally over 10 minutes (The entire test suite runs in about 2 hours). Effort is still on to improve a few more test cases that would hopefully bring this run time down even further.

          The attached file has three sheets –

          1. Proposed set of tests that could make up the Fast Test suite,
          2. Analysis of existing tests in the mapred package and whether they have been considered for fast tests or not, with some notes
          3. List of classes where the coverage drop between the fast tests and all tests is greater than 10%. This also has information of where the drop is coming from

          Clover was used for measuring code coverage.

          Please share your thoughts/suggestions/ideas

          Show
          Jothi Padmanabhan added a comment - As a process of getting to this target, one of the activities was to identify a subset of tests that would run in close to 10 minutes and would have a reasonable coverage. The idea is to have the main core flow/ core components well tested by the fast test suite and the corner cases/library classes could afford to be left out. Needless to say, the entire test suite will be a part of the nightly test cycle. Attaching a spread sheet as a first step in this identification/classification of test suites into Fast Tests. The run time of these tests is marginally over 10 minutes (The entire test suite runs in about 2 hours). Effort is still on to improve a few more test cases that would hopefully bring this run time down even further. The attached file has three sheets – Proposed set of tests that could make up the Fast Test suite, Analysis of existing tests in the mapred package and whether they have been considered for fast tests or not, with some notes List of classes where the coverage drop between the fast tests and all tests is greater than 10%. This also has information of where the drop is coming from Clover was used for measuring code coverage. Please share your thoughts/suggestions/ideas
          Hide
          Nigel Daley added a comment -

          This looks great. Can you go ahead and list these tests into a Junit test suite class? I think that is the best way for now to capture them into a single suite that is run by the commit build.

          Show
          Nigel Daley added a comment - This looks great. Can you go ahead and list these tests into a Junit test suite class? I think that is the best way for now to capture them into a single suite that is run by the commit build.
          Hide
          Jakob Homan added a comment -

          I spoke with Nigel and, as discussed in HDFS-458, rather than a junit test suite class, a flat file included into the build.xml file might be a better solution. It looks like that's the way we're going on the hdfs side.

          Show
          Jakob Homan added a comment - I spoke with Nigel and, as discussed in HDFS-458 , rather than a junit test suite class, a flat file included into the build.xml file might be a better solution. It looks like that's the way we're going on the hdfs side.
          Hide
          Konstantin Boudnik added a comment -

          I've yet another conversation with Jacob wrt HDFS-458 and HDFS-505 and I'm going to agree with his approach: an external configuration file (i.e. test list) seems to provide a reasonable compromise between ease of use and maintenance headache. External test list file allows to make changes of a suite content without any actual source code modifications, which is a bonus for someone not familiar with Java language.

          On the other hand, a separate JUnit test suite won't give us any extra benefits compare to a flat test file.

          +1 on HDFS-458 approach.

          Show
          Konstantin Boudnik added a comment - I've yet another conversation with Jacob wrt HDFS-458 and HDFS-505 and I'm going to agree with his approach: an external configuration file (i.e. test list) seems to provide a reasonable compromise between ease of use and maintenance headache. External test list file allows to make changes of a suite content without any actual source code modifications, which is a bonus for someone not familiar with Java language. On the other hand, a separate JUnit test suite won't give us any extra benefits compare to a flat test file. +1 on HDFS-458 approach.
          Hide
          Jothi Padmanabhan added a comment -

          Changes to build.xml for the run-commit-tests target. The changes are similar to run-commit-tests target of HDFS.

          Show
          Jothi Padmanabhan added a comment - Changes to build.xml for the run-commit-tests target. The changes are similar to run-commit-tests target of HDFS.
          Hide
          Konstantin Boudnik added a comment -

          It seems that indentation isn't consistent across the changes.

                  <fileset dir="${test.src.dir}/mapred" excludes="**/${test.exclude}.java">
                    <patternset>
                    <includesfile name="@{test.file}"/>
                    </patternset>
                    </fileset>
          

          or

            <target name="run-test-mapred" depends="compile-mapred-test" description="Run mapred unit tests">
               <macro-test-runner test.file="${test.mapred.all.tests.file}" />
            </target> 
          

          Looks good otherwise

          Show
          Konstantin Boudnik added a comment - It seems that indentation isn't consistent across the changes. <fileset dir= "${test.src.dir}/mapred" excludes= "**/${test.exclude}.java" > <patternset> <includesfile name= "@{test.file}" /> </patternset> </fileset> or <target name= "run-test-mapred" depends= "compile-mapred-test" description= "Run mapred unit tests" > <macro-test-runner test.file= "${test.mapred.all.tests.file}" /> </target> Looks good otherwise
          Hide
          Jothi Padmanabhan added a comment -

          Patch fixing the indentation issue pointed out by Konstantin.

          A few additional points:

          Code coverage (for the mapred package) of all-tests list is 76%.
          Code coverage (for the mapred package) of commit-tests list is 59%

          I have left out two tests (TestJobTrackerRestart and TestQueueManager) out of this list as these take about 7 and 6 minutes respectively. Separate effort is underway to refactor these to become unit tests and when completed, these tests will be added to the commit-tests list. The current tests run in less than 9 minutes, so adding these two tests, after they have been refactored, should still keep the run time down to 10 minutes.

          Code coverage (for the mapred package) of commit-tests + TestJobTrackerRestart + TestQueueManager (as they exist now) is 63%

          Show
          Jothi Padmanabhan added a comment - Patch fixing the indentation issue pointed out by Konstantin. A few additional points: Code coverage (for the mapred package) of all-tests list is 76%. Code coverage (for the mapred package) of commit-tests list is 59% I have left out two tests (TestJobTrackerRestart and TestQueueManager) out of this list as these take about 7 and 6 minutes respectively. Separate effort is underway to refactor these to become unit tests and when completed, these tests will be added to the commit-tests list. The current tests run in less than 9 minutes, so adding these two tests, after they have been refactored, should still keep the run time down to 10 minutes. Code coverage (for the mapred package) of commit-tests + TestJobTrackerRestart + TestQueueManager (as they exist now) is 63%
          Hide
          Konstantin Boudnik added a comment -

          I'm still seeing two more misaligned spots:
          1. Everything below <sequential> is a body of this element and suppose to be indented.

          +  <macrodef name="macro-test-runner">
          +    <attribute name="test.file" />
          +    <sequential>
               <delete dir="${test.build.data}"/>
               <mkdir dir="${test.build.data}"/>
               <delete dir="${test.log.dir}"/>
          

          2. Same is here: body of <patternset> isn't indented.

          +          <patternset>
          +          <includesfile name="@{test.file}"/>
          +          </patternset>
          

          Looks good otherwise.

          Show
          Konstantin Boudnik added a comment - I'm still seeing two more misaligned spots: 1. Everything below <sequential> is a body of this element and suppose to be indented. + <macrodef name="macro-test-runner"> + <attribute name="test.file" /> + <sequential> <delete dir="${test.build.data}"/> <mkdir dir="${test.build.data}"/> <delete dir="${test.log.dir}"/> 2. Same is here: body of <patternset> isn't indented. + <patternset> + <includesfile name="@{test.file}"/> + </patternset> Looks good otherwise.
          Hide
          Jothi Padmanabhan added a comment -

          Fixing the indentation (again !) The original source also had this mismatch and was carried over in the previous patch too

          Show
          Jothi Padmanabhan added a comment - Fixing the indentation (again !) The original source also had this mismatch and was carried over in the previous patch too
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12415093/mapred-670-v2.patch
          against trunk revision 799551.

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

          +1 tests included. The patch appears to include 48 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 failed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/432/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/432/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/432/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/432/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/12415093/mapred-670-v2.patch against trunk revision 799551. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 48 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 failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/432/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/432/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/432/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/432/console This message is automatically generated.
          Hide
          Jothi Padmanabhan added a comment -

          Streaming test failures are unrelated to the patch

          Show
          Jothi Padmanabhan added a comment - Streaming test failures are unrelated to the patch
          Hide
          Giridharan Kesavan added a comment -

          +1 looks good.

          Show
          Giridharan Kesavan added a comment - +1 looks good.
          Hide
          Giridharan Kesavan added a comment -

          I just committed this, Thanks Jothi

          Show
          Giridharan Kesavan added a comment - I just committed this, Thanks Jothi
          Hide
          Lee Tucker added a comment -

          The patch shows that there should be a file called commit-tests as part of the patch, but when I go to the tip of the trunk, there is no such file commited.

          Show
          Lee Tucker added a comment - The patch shows that there should be a file called commit-tests as part of the patch, but when I go to the tip of the trunk, there is no such file commited.
          Hide
          Konstantin Boudnik added a comment -

          Looks like 'svn add' has been omitted.

          Show
          Konstantin Boudnik added a comment - Looks like 'svn add' has been omitted.
          Hide
          Arun C Murthy added a comment -

          I added & committed the missing files.

          Show
          Arun C Murthy added a comment - I added & committed the missing files.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #38 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/38/)
          . Adding the 'new' files which got missed in the original commit.

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #38 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/38/ ) . Adding the 'new' files which got missed in the original commit.
          Hide
          Hemanth Yamijala added a comment -

          Patch for earlier version of hadoop. Not for commit here.

          Show
          Hemanth Yamijala added a comment - Patch for earlier version of hadoop. Not for commit here.

            People

            • Assignee:
              Jothi Padmanabhan
              Reporter:
              Jothi Padmanabhan
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development