I did a couple of examples with powermock and jmockit to give people an
idea of some of the pain and goodness of the tools.
Here is a breakdown of what's going on in the attached files:
1. Pom changes for powermock
1. a simple set of changes to pull the right dependencies in
2. whitebox access: demonstrates reflection helper utilities to access
private methdos
1. Means we can drop all the public methods and the "EXPOSED FOR
TESTING" comments and just access via a clean api
3. Private field access with annotations - annotate fields used in
testing via reflection
1. This is a tooling we use at SFDC to mark what fields are used via
reflection so devs know when they are dealing with
reflection-tested stuff
and to take care
2. We can add a lot more tooling around this rather than the hackery
that I use in the test to make accessing the right type easier
4. Jmockit usage
1. Here we are hooking up a mock TableHFileArchiveTracker. This is
interesting because the object is created via a static method -
TableHFileArchiveTracker.create(Configuration), which we can't
get at with
Mockito.
2. We can also do mockting of the rest of the private/public methods
easily. See
http://jmockit.googlecode.com/svn/trunk/www/tutorial/AnExample.html
3. Problems:
1. needs to be listed before junit (for loading hooks)
2. needs a jvm with an Agent API (but we already require
Sun/Oracle which has it)
3. Loose eclipse debugging of mocked methods (but there is an eclipse
plugin<http://marketplace.eclipse.org/content/jmockit-eclipse#.UGDp6FQWW6w>with
helpful utils)
5. Attempting to spin up a mini-cluster test with powermock
1. This doesn't actually work as there is a classloading clash
2. HUGE pain to deal with (see all the ignores)
If you look closely (4) actually uses the powermock utils to test that we
are actually munging the state and using a custom mock object, since that
is far more lightweight and easy to use.
IMO using the Powermock Whitebox utilities (+ a little custom stuff to make
life easier) together with the jmockit tooling for getting at static/final
methods/classes is the right way to go for doing deeper testing without
radically refactoring existing code (or writing really obtuse code just for
easier testability).
I can refactor the patches above to just roll in powermock and jmockit
support into a patch HBASE-5456.
Thoughts?
Thanks,
Jesse
-------------------
Jesse Yates
@jesse_yates
jyates.github.com
On Fri, Sep 21, 2012 at 11:03 AM, Jesse Yates <[email protected]>wrote:
>
> On Fri, Sep 21, 2012 at 9:49 AM, Ryan Rawson <[email protected]> wrote:
>
>> I've used mockito a few times, and it's great... but it can make your
>> tests very brittle. It can also be hard to successfully use if the
>> code is complex. For example I had a class that took an HBaseAdmin,
>> and i mocked out the few calls it used. Then when I needed to access
>> Configuration, things went downhill fast. I ended up abandoning
>> easymock even.
>>
>>
> Interesting... I was recently mocking out some stuff with HBaseAdmin and
> didn't have that much trouble (except for HBASE-6495 which is now fixed).
>
>
>> The issue ultimately stems from not writing your code in a certain way
>> with a minimal of easy to mock external interfaces.
>
>
> Nice sentiment but a non-trivial ask with legacy code :) Part of the
> problem here is that EasyMock/Mockito don't hack it when they aren't
> designed for testing.
>
>
>> When this isn't
>> true, then easymock does nothing for you. It can save your bacon if
>> you are trying to unit test something deep though.
>>
>> The other question I guess is integration testing... there is no
>> specific good reason why everything is done in 1 jvm, except 'because
>> we can'.
>
>
> It helps provides isolation between tests so we can be sure of which tests
> are actually failing.
>
>
>> a longer lived 'minicluster' could amortize the cost of
>> running one.
>>
>
> Looked into the long running minicluster about 8 months ago - it
> generally wouldn't help all that much. A lot of the minicluster tests get
> into the internals and muck about with things to test error scenarios. The
> work it would take to have a shared minicluster for tests that just need to
> run on a cluster. That said, a lot of those kinds of tests are moving to
> the hbase-it package so they can run on a minicluster or a real cluster.
>
>
>>
>> -ryan
>>
>> On Fri, Sep 21, 2012 at 9:06 AM, Rogerio <[email protected]> wrote:
>> > lars hofhansl <lhofhansl@...> writes:
>> >
>> >> > To get the low-level access we could instead use jmockit at the cost
>> of
>> > dealing with code-weaving.
>> >>
>> >> As we had discussed, this scares me :).
>> >> I do not want to have to debug some test code that was weaved (i.e.
>> has no
>> > matching source code lying around *anywhere*).
>> >>
>> >
>> > I think you are imagining a problem that does not exist. JMockit users
>> can debug
>> > Java code just fine...
>> >
>> >
>>
>
>
diff --git a/pom.xml b/pom.xml
index 354a8e6..8b29c4e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -819,7 +819,8 @@
<jetty.jspapi.version>6.1.14</jetty.jspapi.version>
<jersey.version>1.8</jersey.version>
<jruby.version>1.6.5</jruby.version>
+ <!-- Temporary change to support power mock naming. Keywal thinks we can
rename our version to accommodate this just fine -->
- <junit.version>4.10-HBASE-1</junit.version>
+ <junit.version>4.10</junit.version>
+ <!-- <junit.version>4.10-HBASE-1</junit.version> -->
@@ -1181,6 +1183,18 @@
<version>${mockito-all.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-module-junit4</artifactId>
+ <version>${powermock.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-api-mockito</artifactId>
+ <version>${powermock.version}</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</dependencyManagement>
<!-- Dependencies needed by subprojects -->diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index 87e8524..f0e2272 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -1022,9 +1022,9 @@ public class HRegion implements HeapSize { // , Writable{
/**
* Wait for all current flushes and compactions of the region to complete.
* <p>
- * Exposed for TESTING.
+ * Used in testing via reflection.
*/
- public void waitForFlushesAndCompactions() {
+ private void waitForFlushesAndCompactions() {
synchronized (writestate) {
while (writestate.compacting > 0 || writestate.flushing) {
LOG.debug("waiting for " + writestate.compacting + " compactions"
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHFileArchiveUtil.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHFileArchiveUtil.java
index 86a6014..91d2276 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHFileArchiveUtil.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHFileArchiveUtil.java
+ @Test
+ public void testGetRegionFlushesAndCompactions() throws Exception {
+ HRegion region = new HRegion();
+ Whitebox.getMethod(region.getClass(),
"waitForFlushesAndCompactions").invoke(region);
}
}diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/example/TestZooKeeperTableArchiveClient.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/example/TestZooKeeperTableArchiveClient.java
index cb4eb15..efd859e 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/example/TestZooKeeperTableArchiveClient.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/example/TestZooKeeperTableArchiveClient.java
@@ -28,6 +28,11 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Set;
+import mockit.Expectations;
+import mockit.Mock;
+import mockit.MockUp;
+import mockit.Mocked;
+
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.conf.Configuration;
@@ -76,12 +79,27 @@ public class TestZooKeeperTableArchiveClient {
private static final long ttl = 1000;
private static ZKTableArchiveClient archivingClient;
+ // mock out calls to the table archive tracker
+ public static class MockTableHFileArchiveTracker extends
MockUp<TableHFileArchiveTracker> {
+
+ /**
+ * The table is always considered archived if the tablename matches
STRING_TABLE_NAME
+ * @param tableName name to check
+ * @return
+ */
+ @Mock
+ public boolean keepHFiles(String tableName) {
+ return tableName.equals(STRING_TABLE_NAME);
+ }
+ }
/**
* Setup the config for the cluster
*/
@BeforeClass
public static void setupCluster() throws Exception {
setupConf(UTIL.getConfiguration());
+ // start using the mocked TableHFileArchiveTracker
+ new MockTableHFileArchiveTracker();
UTIL.startMiniCluster(numRS);
archivingClient = new ZKTableArchiveClient(UTIL.getConfiguration(),
UTIL.getHBaseAdmin()
.getConnection());
@@ -163,11 +181,11 @@ public class TestZooKeeperTableArchiveClient {
@Test
public void testArchivingOnSingleTable() throws Exception {
// turn on hfile retention
+ List<BaseHFileCleanerDelegate> delegates =
Whitebox.getInternalState(UTIL.getHBaseCluster()
+ .getMaster().getHFileCleaner(), "cleanersChain");
LongTermArchivingHFileCleaner cleaner = null;
+ for (BaseHFileCleanerDelegate del : delegates) {
+ if (del instanceof LongTermArchivingHFileCleaner) {
cleaner = (LongTermArchivingHFileCleaner) del;
break;
}
@@ -175,7 +193,7 @@ public class TestZooKeeperTableArchiveClient {
Set<Field> fields = Whitebox.getFieldsAnnotatedWith(cleaner,
TestAccessible.class);
TableHFileArchiveTracker tracker = (TableHFileArchiveTracker)
fields.iterator().next()
.get(cleaner);
+ assertTrue("Jmockit didn't intercept call",
tracker.keepHFiles(STRING_TABLE_NAME));
// get the RS and region serving our table
List<HRegion> servingRegions =
UTIL.getHBaseCluster().getRegions(TABLE_NAME);
diff --git a/pom.xml b/pom.xml
index 8b29c4e..cb1c843 100644
--- a/pom.xml
+++ b/pom.xml
@@ -834,6 +834,7 @@
<jettison.version>1.3.1</jettison.version>
<netty.version>3.5.0.Final</netty.version>
<powermock.version>1.4.12</powermock.version>
+ <jmockit.version>0.999.17</jmockit.version>
<!-- Plugin Dependencies -->
<maven.assembly.version>2.3</maven.assembly.version>
<maven.antrun.version>1.6</maven.antrun.version>
@@ -1201,6 +1202,15 @@
<dependencies>
<!-- Test dependencies -->
<dependency>
+ <!-- Has to come before junit in the dependencies -->
+ <!-- Requires running tests on JDK 1.6 to avoid adding the -javagent
config property -->
+ <!-- Testing JDK must support with the Agent API (Orcale/Sun JDK is
fine) -->
+ <groupId>com.googlecode.jmockit</groupId>
+ <artifactId>jmockit</artifactId>
+ <version>${jmockit.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
</dependency>
//this gets us most of the way there, but still doesn't work (though it could
be managed)
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java
index bc2a793..10a5ea2 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestHFileArchiving.java
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(HRegion.class)
+ // this is a really gross mess that still doesn't really work
+@PowerMockIgnore({ "javax.xml.*", "org.apache.log4j.*", "org.w3c.dom.*",
+ "org.apache.hadoop.security.*", "org.apache.hadoop.hdfs.*",
"org.apache.hadoop.fs.*",
+ "org.apache.hadoop.conf.Configuration", "javax.management.*" })
public class TestHFileArchiving {
