Would you mind using mockito? I am trying to move us there and this would be a good first step.
Thanks! On Thu, May 27, 2010 at 5:29 PM, Daniel Ploeg <[email protected]> wrote: > > >> On 2010-05-27 16:45:23, Ryan Rawson wrote: >> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2701 >> > <http://review.hbase.org/r/83/diff/3/?file=690#file690line2701> >> > >> > I'm thinking perhaps a Static interface could be used instead of the >> > getDelegate().currentTimeMillis() that way the individual call sites could >> > say: >> > >> > EnvironmentEdge.currentTimeMillis() >> > instead of >> > System.currentTimeMillis() >> > >> > but the static calls would be a simple wrapper around the existing >> > getDelegate mechanisms... this would just make the code more explicit and >> > easier to follow for future generations. >> > >> > OTher than that, looking great! Thanks for making all the changes! > > Ok sounds good. The EnvironmentEdge is the interface, so I think I'll need to > put the static method on the manager. The call would be: > > EnvironmentEdgeManager.currentTimeMillis() > > I'm adding a mocking library to the pom (EasyMock) to help with the testing. > > > - Daniel > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.hbase.org/r/83/#review88 > ----------------------------------------------------------- > > > On 2010-05-27 14:51:48, Daniel Ploeg wrote: >> >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> http://review.hbase.org/r/83/ >> ----------------------------------------------------------- >> >> (Updated 2010-05-27 14:51:48) >> >> >> Review request for hbase. >> >> >> Summary >> ------- >> >> HBASE-2578 - Add ability for tests to override server-side timestamp setting >> (currentTimeMillis). >> The solution in this patch ensures that tests use a different timestamp with >> a minimal change to the production code paths. >> One question I would like to know is whether the change that was made to >> HRegion.FIXED_OVERHEAD would cause any other side effects. >> >> >> This addresses bug HBASE-2578. >> http://issues.apache.org/jira/browse/HBASE-2578 >> >> >> Diffs >> ----- >> >> src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 287cd48 >> src/main/java/org/apache/hadoop/hbase/util/DefaultEnvironmentEdge.java >> PRE-CREATION >> src/main/java/org/apache/hadoop/hbase/util/EnvironmentEdge.java >> PRE-CREATION >> src/main/java/org/apache/hadoop/hbase/util/EnvironmentEdgeManager.java >> PRE-CREATION >> >> src/main/java/org/apache/hadoop/hbase/util/IncrementingEnvironmentEdge.java >> PRE-CREATION >> src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java d3716d6 >> >> src/test/java/org/apache/hadoop/hbase/util/EnvironmentEdgeManagerTestHelper.java >> PRE-CREATION >> src/test/java/org/apache/hadoop/hbase/util/TestDefaultEnvironmentEdge.java >> PRE-CREATION >> src/test/java/org/apache/hadoop/hbase/util/TestEnvironmentEdgeManager.java >> PRE-CREATION >> >> src/test/java/org/apache/hadoop/hbase/util/TestIncrementingEnvironmentEdge.java >> PRE-CREATION >> >> Diff: http://review.hbase.org/r/83/diff >> >> >> Testing >> ------- >> >> >> Thanks, >> >> Daniel >> >> > >
