Hadoop Common
  1. Hadoop Common
  2. HADOOP-5164

Subclasses of ClusterMapReduceTestCase can't easily add new configuration parameters

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change
    • Release Note:
      Hide
      Modified the behavior of ClusterMapReduceTestCase so that setupCluster() is no longer called automatically in TestCase.setUp(). Now, you must explicitly call setupCluster() for each test you wish to run (or you can share a single cluster among several tests).
      Show
      Modified the behavior of ClusterMapReduceTestCase so that setupCluster() is no longer called automatically in TestCase.setUp(). Now, you must explicitly call setupCluster() for each test you wish to run (or you can share a single cluster among several tests).

      Description

      Currently there is not a clean way for subclasses of ClusterMapReduceTestCase to add to the JobConf used to start the cluster daemons.

      The startCluster() method does take a Properties object that is added to the JobConf used to the start the daemons. However, startCluster() is called from JUnit inside the setUp() method, which sets this parameter to be null.

      If you try to override setUp() in a subclass of ClusterMapReduceTestCase, then you won't be able to invoke the TestCase.setUp() ancestor without calling ClusterMapReduceTestCase's setUp() (which will pass in the null parameter). On the other hand, if you just call startCluster() within your test method, then you would be starting up a cluster that was already started.

      1. HADOOP-5164.1.patch
        6 kB
        George Porter
      2. HADOOP-5164.2.patch
        7 kB
        George Porter
      3. HADOOP-5164.patch
        1.0 kB
        George Porter

        Activity

        Hide
        George Porter added a comment -

        I recommend that we create a protected method called getDefaultClusterProperties() that can be overridden by subclasses of ClusterMapReduceTestCase.

        Show
        George Porter added a comment - I recommend that we create a protected method called getDefaultClusterProperties() that can be overridden by subclasses of ClusterMapReduceTestCase.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Even with this patch, we cannot have different cluster properties in different tests of a single test-case. This, as well as the general problem being pointed, seems to have the only solution of overriding setUp() to do nothing and calling startCluster explicitly in each test with any required config properties.

        Further, I don't clearly understand why you want to be able to invoke TestCase.setUp(), it does nothing anyways.

        Show
        Vinod Kumar Vavilapalli added a comment - Even with this patch, we cannot have different cluster properties in different tests of a single test-case. This, as well as the general problem being pointed, seems to have the only solution of overriding setUp() to do nothing and calling startCluster explicitly in each test with any required config properties. Further, I don't clearly understand why you want to be able to invoke TestCase.setUp(), it does nothing anyways.
        Hide
        George Porter added a comment -

        Thanks for the feedback Vinod.

        I went ahead and refactored out all of the startCluster() calls and made each test case explicitly call them, rather than having an implicit call to startCluster(true, null) in setUp(). This should both solve my original problem, as well as give the test writer finer-grained control over the parameters passed to each test cluster. What do you think?

        Show
        George Porter added a comment - Thanks for the feedback Vinod. I went ahead and refactored out all of the startCluster() calls and made each test case explicitly call them, rather than having an implicit call to startCluster(true, null) in setUp(). This should both solve my original problem, as well as give the test writer finer-grained control over the parameters passed to each test cluster. What do you think?
        Hide
        George Porter added a comment -

        With this patch, every test case that is in a subclass of ClusterMapReduceTestCase now explicitly calls startCluster(). This makes it possible to create test case subclasses of ClusterMapReduceTestCase that use alternative parameters to the cluster. It also makes it possible to have different clusters in your test cases that have different parameters.

        Show
        George Porter added a comment - With this patch, every test case that is in a subclass of ClusterMapReduceTestCase now explicitly calls startCluster(). This makes it possible to create test case subclasses of ClusterMapReduceTestCase that use alternative parameters to the cluster. It also makes it possible to have different clusters in your test cases that have different parameters.
        Hide
        George Porter added a comment -

        This updated patch includes an additional test that needed to be modified to directly call setupCluster(true, null).

        Show
        George Porter added a comment - This updated patch includes an additional test that needed to be modified to directly call setupCluster(true, null).
        Hide
        George Porter added a comment -

        Modified this issue to be an "incompatible change"

        Show
        George Porter added a comment - Modified this issue to be an "incompatible change"
        Hide
        George Porter added a comment -

        Added a release note.

        Show
        George Porter added a comment - Added a release note.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12400457/HADOOP-5164.2.patch
        against trunk revision 745705.

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

        +1 tests included. The patch appears to include 27 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 Eclipse classpath. The patch retains Eclipse classpath integrity.

        +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/3882/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3882/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3882/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3882/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/12400457/HADOOP-5164.2.patch against trunk revision 745705. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 27 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 Eclipse classpath. The patch retains Eclipse classpath integrity. +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/3882/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3882/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3882/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3882/console This message is automatically generated.
        Hide
        Chris Douglas added a comment -

        For a cluster shared between tests, wouldn't the correct idiom use TestSetup, as in TestAppend3, TestReduceFetch, etc.? If each test needs a different set of parameters, it's not clear to me why one wouldn't simply create a Configuration with these parameters, and start Mini*Cluster with it...

        Show
        Chris Douglas added a comment - For a cluster shared between tests, wouldn't the correct idiom use TestSetup, as in TestAppend3, TestReduceFetch, etc.? If each test needs a different set of parameters, it's not clear to me why one wouldn't simply create a Configuration with these parameters, and start Mini*Cluster with it...

          People

          • Assignee:
            Unassigned
            Reporter:
            George Porter
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:

              Development