HBase
  1. HBase
  2. HBASE-4503

Purge deprecated HBaseClusterTestCase

    Details

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

      Description

      It could gain us a few minutes on overall test run in the cases where we don't spin up a cluster for each test.

      1. 4503-v2.txt
        85 kB
        stack
      2. 4503.txt
        15 kB
        stack

        Activity

        stack created issue -
        Hide
        stack added a comment -

        Here are a few tests converted. Few more to do.

        Show
        stack added a comment - Here are a few tests converted. Few more to do.
        stack made changes -
        Field Original Value New Value
        Attachment 4503.txt [ 12496818 ]
        Hide
        stack added a comment -

        This should do it. Removes HBaseClusterTestCase and the MultiTable thingy used by MR. Groups a few of old dependent tests and leaves other alone. Should save us a few minutes. Only 120+ to go.

        Show
        stack added a comment - This should do it. Removes HBaseClusterTestCase and the MultiTable thingy used by MR. Groups a few of old dependent tests and leaves other alone. Should save us a few minutes. Only 120+ to go.
        stack made changes -
        Attachment 4503-v2.txt [ 12496975 ]
        stack made changes -
        Assignee stack [ stack ]
        Hide
        stack added a comment -

        Will post to RB in morn after I give it another run though.

        Show
        stack added a comment - Will post to RB in morn after I give it another run though.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2119/
        -----------------------------------------------------------

        Review request for hbase.

        Summary
        -------

        Replace deprecated HBaseClusterTestCase and subclasses.

        This addresses bug hbase-4503.
        https://issues.apache.org/jira/browse/hbase-4503

        Diffs


        Diff: https://reviews.apache.org/r/2119/diff

        Testing
        -------

        Unit tests pass

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/ ----------------------------------------------------------- Review request for hbase. Summary ------- Replace deprecated HBaseClusterTestCase and subclasses. This addresses bug hbase-4503. https://issues.apache.org/jira/browse/hbase-4503 Diffs Diff: https://reviews.apache.org/r/2119/diff Testing ------- Unit tests pass Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2119/
        -----------------------------------------------------------

        (Updated 2011-09-29 22:01:14.094149)

        Review request for hbase.

        Changes
        -------

        Include actual diff

        Summary
        -------

        Replace deprecated HBaseClusterTestCase and subclasses.

        This addresses bug hbase-4503.
        https://issues.apache.org/jira/browse/hbase-4503

        Diffs (updated)


        src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb
        src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5
        src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 555174a
        src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08
        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION
        src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8
        src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4
        src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c
        src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed
        src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539
        src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6
        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30
        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360
        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3
        src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464
        src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686

        Diff: https://reviews.apache.org/r/2119/diff

        Testing
        -------

        Unit tests pass

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/ ----------------------------------------------------------- (Updated 2011-09-29 22:01:14.094149) Review request for hbase. Changes ------- Include actual diff Summary ------- Replace deprecated HBaseClusterTestCase and subclasses. This addresses bug hbase-4503. https://issues.apache.org/jira/browse/hbase-4503 Diffs (updated) src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5 src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 555174a src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08 src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8 src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539 src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360 src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3 src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464 src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686 Diff: https://reviews.apache.org/r/2119/diff Testing ------- Unit tests pass Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2119/#review2184
        -----------------------------------------------------------

        A couple of slight cleanup things, but overall +1.

        src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java
        <https://reviews.apache.org/r/2119/#comment5084>

        Does this file really need to be here? Yeah, its a little whitespace cleanup, but I don't think its that necessary

        src/test/java/org/apache/hadoop/hbase/TestInfoServers.java
        <https://reviews.apache.org/r/2119/#comment5086>

        why not just assertEquals($

        {desc}

        , expected, content) here?

        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java
        <https://reviews.apache.org/r/2119/#comment5088>

        Couldn't this just go into @BeforeClass?

        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java
        <https://reviews.apache.org/r/2119/#comment5090>

        nitpick: just use iterable properties here?

        Also, maybe abstract this into a single checking method? Not gaining a whole lot, but saves you some code copy.

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java
        <https://reviews.apache.org/r/2119/#comment5093>

        nitpick: why drop the import?

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java
        <https://reviews.apache.org/r/2119/#comment5094>

        nitpick: (Again) import Assert.*?

        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java
        <https://reviews.apache.org/r/2119/#comment5095>

        This doesn't seem to fail into the same review need as the rest of the patch. I'm ok with the changes, but it would be better if they were for the same reason.

        +/-0

        • Jesse

        On 2011-09-29 22:01:14, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/2119/

        -----------------------------------------------------------

        (Updated 2011-09-29 22:01:14)

        Review request for hbase.

        Summary

        -------

        Replace deprecated HBaseClusterTestCase and subclasses.

        This addresses bug hbase-4503.

        https://issues.apache.org/jira/browse/hbase-4503

        Diffs

        -----

        src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb

        src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5

        src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 555174a

        src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08

        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION

        src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8

        src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4

        src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c

        src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed

        src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539

        src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360

        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3

        src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464

        src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686

        Diff: https://reviews.apache.org/r/2119/diff

        Testing

        -------

        Unit tests pass

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/#review2184 ----------------------------------------------------------- A couple of slight cleanup things, but overall +1. src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java < https://reviews.apache.org/r/2119/#comment5084 > Does this file really need to be here? Yeah, its a little whitespace cleanup, but I don't think its that necessary src/test/java/org/apache/hadoop/hbase/TestInfoServers.java < https://reviews.apache.org/r/2119/#comment5086 > why not just assertEquals($ {desc} , expected, content) here? src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java < https://reviews.apache.org/r/2119/#comment5088 > Couldn't this just go into @BeforeClass? src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java < https://reviews.apache.org/r/2119/#comment5090 > nitpick: just use iterable properties here? Also, maybe abstract this into a single checking method? Not gaining a whole lot, but saves you some code copy. src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java < https://reviews.apache.org/r/2119/#comment5093 > nitpick: why drop the import? src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java < https://reviews.apache.org/r/2119/#comment5094 > nitpick: (Again) import Assert.*? src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java < https://reviews.apache.org/r/2119/#comment5095 > This doesn't seem to fail into the same review need as the rest of the patch. I'm ok with the changes, but it would be better if they were for the same reason. +/-0 Jesse On 2011-09-29 22:01:14, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/ ----------------------------------------------------------- (Updated 2011-09-29 22:01:14) Review request for hbase. Summary ------- Replace deprecated HBaseClusterTestCase and subclasses. This addresses bug hbase-4503. https://issues.apache.org/jira/browse/hbase-4503 Diffs ----- src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5 src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 555174a src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08 src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8 src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539 src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360 src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3 src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464 src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686 Diff: https://reviews.apache.org/r/2119/diff Testing ------- Unit tests pass Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2119/#review2189
        -----------------------------------------------------------

        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java
        <https://reviews.apache.org/r/2119/#comment5109>

        You are right

        • Michael

        On 2011-09-29 22:01:14, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/2119/

        -----------------------------------------------------------

        (Updated 2011-09-29 22:01:14)

        Review request for hbase.

        Summary

        -------

        Replace deprecated HBaseClusterTestCase and subclasses.

        This addresses bug hbase-4503.

        https://issues.apache.org/jira/browse/hbase-4503

        Diffs

        -----

        src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb

        src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5

        src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 555174a

        src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08

        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION

        src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8

        src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4

        src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c

        src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed

        src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539

        src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360

        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3

        src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464

        src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686

        Diff: https://reviews.apache.org/r/2119/diff

        Testing

        -------

        Unit tests pass

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/#review2189 ----------------------------------------------------------- src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java < https://reviews.apache.org/r/2119/#comment5109 > You are right Michael On 2011-09-29 22:01:14, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/ ----------------------------------------------------------- (Updated 2011-09-29 22:01:14) Review request for hbase. Summary ------- Replace deprecated HBaseClusterTestCase and subclasses. This addresses bug hbase-4503. https://issues.apache.org/jira/browse/hbase-4503 Diffs ----- src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5 src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 555174a src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08 src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8 src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539 src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360 src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3 src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464 src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686 Diff: https://reviews.apache.org/r/2119/diff Testing ------- Unit tests pass Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-29 22:26:55, Jesse Yates wrote:

        > src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java, line 150

        > <https://reviews.apache.org/r/2119/diff/1/?file=46504#file46504line150>

        >

        > Does this file really need to be here? Yeah, its a little whitespace cleanup, but I don't think its that necessary

        Agreed. Will purge it from the diff.

        On 2011-09-29 22:26:55, Jesse Yates wrote:

        > src/test/java/org/apache/hadoop/hbase/TestInfoServers.java, line 105

        > <https://reviews.apache.org/r/2119/diff/1/?file=46505#file46505line105>

        >

        > why not just assertEquals(${desc}, expected, content) here?

        Will do.

        On 2011-09-29 22:26:55, Jesse Yates wrote:

        > src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java, line 72

        > <https://reviews.apache.org/r/2119/diff/1/?file=46506#file46506line72>

        >

        > Couldn't this just go into @BeforeClass?

        That is static but this.admin is not. It could but saw no harm doing it this way.

        On 2011-09-29 22:26:55, Jesse Yates wrote:

        > src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java, line 240

        > <https://reviews.apache.org/r/2119/diff/1/?file=46506#file46506line240>

        >

        > nitpick: just use iterable properties here?

        >

        > Also, maybe abstract this into a single checking method? Not gaining a whole lot, but saves you some code copy.

        The tests here were copied from elsewhere. I tried not to touch what they did, just convert the chassis they used. I can change this np.

        On 2011-09-29 22:26:55, Jesse Yates wrote:

        > src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java, line 191

        > <https://reviews.apache.org/r/2119/diff/1/?file=46513#file46513line191>

        >

        > nitpick: why drop the import?

        Will fix.

        On 2011-09-29 22:26:55, Jesse Yates wrote:

        > src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java, line 217

        > <https://reviews.apache.org/r/2119/diff/1/?file=46514#file46514line217>

        >

        > nitpick: (Again) import Assert.*?

        Will fix.

        • Michael

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2119/#review2184
        -----------------------------------------------------------

        On 2011-09-29 22:01:14, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/2119/

        -----------------------------------------------------------

        (Updated 2011-09-29 22:01:14)

        Review request for hbase.

        Summary

        -------

        Replace deprecated HBaseClusterTestCase and subclasses.

        This addresses bug hbase-4503.

        https://issues.apache.org/jira/browse/hbase-4503

        Diffs

        -----

        src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb

        src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5

        src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 555174a

        src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08

        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION

        src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8

        src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4

        src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c

        src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed

        src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539

        src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360

        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3

        src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464

        src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686

        Diff: https://reviews.apache.org/r/2119/diff

        Testing

        -------

        Unit tests pass

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-29 22:26:55, Jesse Yates wrote: > src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java, line 150 > < https://reviews.apache.org/r/2119/diff/1/?file=46504#file46504line150 > > > Does this file really need to be here? Yeah, its a little whitespace cleanup, but I don't think its that necessary Agreed. Will purge it from the diff. On 2011-09-29 22:26:55, Jesse Yates wrote: > src/test/java/org/apache/hadoop/hbase/TestInfoServers.java, line 105 > < https://reviews.apache.org/r/2119/diff/1/?file=46505#file46505line105 > > > why not just assertEquals(${desc}, expected, content) here? Will do. On 2011-09-29 22:26:55, Jesse Yates wrote: > src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java, line 72 > < https://reviews.apache.org/r/2119/diff/1/?file=46506#file46506line72 > > > Couldn't this just go into @BeforeClass? That is static but this.admin is not. It could but saw no harm doing it this way. On 2011-09-29 22:26:55, Jesse Yates wrote: > src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java, line 240 > < https://reviews.apache.org/r/2119/diff/1/?file=46506#file46506line240 > > > nitpick: just use iterable properties here? > > Also, maybe abstract this into a single checking method? Not gaining a whole lot, but saves you some code copy. The tests here were copied from elsewhere. I tried not to touch what they did, just convert the chassis they used. I can change this np. On 2011-09-29 22:26:55, Jesse Yates wrote: > src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java, line 191 > < https://reviews.apache.org/r/2119/diff/1/?file=46513#file46513line191 > > > nitpick: why drop the import? Will fix. On 2011-09-29 22:26:55, Jesse Yates wrote: > src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java, line 217 > < https://reviews.apache.org/r/2119/diff/1/?file=46514#file46514line217 > > > nitpick: (Again) import Assert.*? Will fix. Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/#review2184 ----------------------------------------------------------- On 2011-09-29 22:01:14, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/ ----------------------------------------------------------- (Updated 2011-09-29 22:01:14) Review request for hbase. Summary ------- Replace deprecated HBaseClusterTestCase and subclasses. This addresses bug hbase-4503. https://issues.apache.org/jira/browse/hbase-4503 Diffs ----- src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5 src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java 555174a src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08 src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8 src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539 src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360 src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3 src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464 src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686 Diff: https://reviews.apache.org/r/2119/diff Testing ------- Unit tests pass Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2119/
        -----------------------------------------------------------

        (Updated 2011-09-29 23:26:34.774487)

        Review request for hbase.

        Changes
        -------

        v3 addresses Jesse's feedback.

        + Removed the files TestHBaseTestingUtility.java and TestDefaultLoadBalancer.java from the diff.
        + Changed the check and throw of an IOE to just an assertTrue in TestInfoServers.java
        + Added imports for Assert.* in a few files.

        I did not change guts of tests. Thats outside scope of this chassis-changing patch.

        Summary
        -------

        Replace deprecated HBaseClusterTestCase and subclasses.

        This addresses bug hbase-4503.
        https://issues.apache.org/jira/browse/hbase-4503

        Diffs (updated)


        src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb
        src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5
        src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08
        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION
        src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8
        src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4
        src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c
        src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed
        src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539
        src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6
        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30
        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360
        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3
        src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464
        src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686

        Diff: https://reviews.apache.org/r/2119/diff

        Testing
        -------

        Unit tests pass

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/ ----------------------------------------------------------- (Updated 2011-09-29 23:26:34.774487) Review request for hbase. Changes ------- v3 addresses Jesse's feedback. + Removed the files TestHBaseTestingUtility.java and TestDefaultLoadBalancer.java from the diff. + Changed the check and throw of an IOE to just an assertTrue in TestInfoServers.java + Added imports for Assert.* in a few files. I did not change guts of tests. Thats outside scope of this chassis-changing patch. Summary ------- Replace deprecated HBaseClusterTestCase and subclasses. This addresses bug hbase-4503. https://issues.apache.org/jira/browse/hbase-4503 Diffs (updated) src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5 src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08 src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8 src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539 src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360 src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3 src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464 src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686 Diff: https://reviews.apache.org/r/2119/diff Testing ------- Unit tests pass Thanks, Michael
        Hide
        stack added a comment -

        All unit tests passed when I ran with the first version of this patch.

        Show
        stack added a comment - All unit tests passed when I ran with the first version of this patch.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2119/#review2196
        -----------------------------------------------------------

        Ship it!

        • Jesse

        On 2011-09-29 23:26:34, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/2119/

        -----------------------------------------------------------

        (Updated 2011-09-29 23:26:34)

        Review request for hbase.

        Summary

        -------

        Replace deprecated HBaseClusterTestCase and subclasses.

        This addresses bug hbase-4503.

        https://issues.apache.org/jira/browse/hbase-4503

        Diffs

        -----

        src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb

        src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5

        src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08

        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION

        src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8

        src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4

        src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c

        src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed

        src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539

        src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360

        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3

        src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464

        src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686

        Diff: https://reviews.apache.org/r/2119/diff

        Testing

        -------

        Unit tests pass

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/#review2196 ----------------------------------------------------------- Ship it! Jesse On 2011-09-29 23:26:34, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/ ----------------------------------------------------------- (Updated 2011-09-29 23:26:34) Review request for hbase. Summary ------- Replace deprecated HBaseClusterTestCase and subclasses. This addresses bug hbase-4503. https://issues.apache.org/jira/browse/hbase-4503 Diffs ----- src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5 src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08 src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8 src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539 src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360 src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3 src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464 src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686 Diff: https://reviews.apache.org/r/2119/diff Testing ------- Unit tests pass Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/2119/#review2208
        -----------------------------------------------------------

        +1

        • ramkrishna

        On 2011-09-29 23:26:34, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/2119/

        -----------------------------------------------------------

        (Updated 2011-09-29 23:26:34)

        Review request for hbase.

        Summary

        -------

        Replace deprecated HBaseClusterTestCase and subclasses.

        This addresses bug hbase-4503.

        https://issues.apache.org/jira/browse/hbase-4503

        Diffs

        -----

        src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb

        src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5

        src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08

        src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION

        src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8

        src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4

        src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c

        src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed

        src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539

        src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30

        src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360

        src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3

        src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464

        src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686

        Diff: https://reviews.apache.org/r/2119/diff

        Testing

        -------

        Unit tests pass

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/#review2208 ----------------------------------------------------------- +1 ramkrishna On 2011-09-29 23:26:34, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2119/ ----------------------------------------------------------- (Updated 2011-09-29 23:26:34) Review request for hbase. Summary ------- Replace deprecated HBaseClusterTestCase and subclasses. This addresses bug hbase-4503. https://issues.apache.org/jira/browse/hbase-4503 Diffs ----- src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java c18cddb src/test/java/org/apache/hadoop/hbase/MultiRegionTable.java a8fd4e5 src/test/java/org/apache/hadoop/hbase/TestInfoServers.java 638df08 src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java PRE-CREATION src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 9023af8 src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java 221352c src/test/java/org/apache/hadoop/hbase/client/TestGetRowVersions.java 27842ed src/test/java/org/apache/hadoop/hbase/client/TestTimestamp.java 9204539 src/test/java/org/apache/hadoop/hbase/mapred/TestTableMapReduce.java 5a5c3c6 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableMapReduce.java 0b8ff30 src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java a772360 src/test/java/org/apache/hadoop/hbase/master/TestDefaultLoadBalancer.java a4cd9a3 src/test/java/org/apache/hadoop/hbase/regionserver/DisabledTestRegionServerExit.java 5b8b464 src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java 5802686 Diff: https://reviews.apache.org/r/2119/diff Testing ------- Unit tests pass Thanks, Michael
        Hide
        stack added a comment -

        Committed to 0.92 and to trunk. Thanks for reviews Jesse, Ram, and Ted.

        Show
        stack added a comment - Committed to 0.92 and to trunk. Thanks for reviews Jesse, Ram, and Ted.
        stack made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Fix Version/s 0.92.0 [ 12314223 ]
        Resolution Fixed [ 1 ]

          People

          • Assignee:
            stack
            Reporter:
            stack
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development