You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Bill Havanki <bh...@clouderagovt.com> on 2014/06/06 18:36:14 UTC
Review Request 22308: ACCUMULO-2865 - ZooCacheTest
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22308/
-----------------------------------------------------------
Review request for accumulo.
Bugs: ACCUMULO-2865
https://issues.apache.org/jira/browse/ACCUMULO-2865
Repository: accumulo
Description
-------
This adds a unit test for ZooCache. ZooCache itself was only changed slightly, with the addition of methods to check if information was cached and improvements to its comments.
Diffs
-----
fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 99ffd04
fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/22308/diff/
Testing
-------
Unit tests pass, including ZooCacheTest.
Thanks,
Bill Havanki
Re: Review Request 22308: ACCUMULO-2865 - ZooCacheTest
Posted by Bill Havanki <bh...@clouderagovt.com>.
> On June 6, 2014, 12:43 p.m., Josh Elser wrote:
> > Formatting on the test class, otherwise LGTM.
Darn it! Forgot to format before posting. It'll be done for commit.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22308/#review44910
-----------------------------------------------------------
On June 6, 2014, 12:36 p.m., Bill Havanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22308/
> -----------------------------------------------------------
>
> (Updated June 6, 2014, 12:36 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2865
> https://issues.apache.org/jira/browse/ACCUMULO-2865
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> This adds a unit test for ZooCache. ZooCache itself was only changed slightly, with the addition of methods to check if information was cached and improvements to its comments.
>
>
> Diffs
> -----
>
> fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 99ffd04
> fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22308/diff/
>
>
> Testing
> -------
>
> Unit tests pass, including ZooCacheTest.
>
>
> Thanks,
>
> Bill Havanki
>
>
Re: Review Request 22308: ACCUMULO-2865 - ZooCacheTest
Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22308/#review44910
-----------------------------------------------------------
Ship it!
Formatting on the test class, otherwise LGTM.
- Josh Elser
On June 6, 2014, 4:36 p.m., Bill Havanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22308/
> -----------------------------------------------------------
>
> (Updated June 6, 2014, 4:36 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2865
> https://issues.apache.org/jira/browse/ACCUMULO-2865
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> This adds a unit test for ZooCache. ZooCache itself was only changed slightly, with the addition of methods to check if information was cached and improvements to its comments.
>
>
> Diffs
> -----
>
> fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 99ffd04
> fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22308/diff/
>
>
> Testing
> -------
>
> Unit tests pass, including ZooCacheTest.
>
>
> Thanks,
>
> Bill Havanki
>
>
Re: Review Request 22308: ACCUMULO-2865 - ZooCacheTest
Posted by Bill Havanki <bh...@clouderagovt.com>.
> On June 6, 2014, 1:03 p.m., Mike Drob wrote:
> > fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java, line 152
> > <https://reviews.apache.org/r/22308/diff/1/?file=604767#file604767line152>
> >
> > Should the verify be after the cache hit to make sure the zk code didn't get called multiple times? Not sure.
My intent is to check that the zk code doesn't get called again, and in my head I thought the verify would close down the mock so that repeat calls would fail. But, I don't see anything in the Easymock docs saying that is true. So, I am changing the zk object to a strict mock, so that extra calls will definitely fail. Thanks for pointing this out!
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22308/#review44918
-----------------------------------------------------------
On June 6, 2014, 12:36 p.m., Bill Havanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22308/
> -----------------------------------------------------------
>
> (Updated June 6, 2014, 12:36 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2865
> https://issues.apache.org/jira/browse/ACCUMULO-2865
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> This adds a unit test for ZooCache. ZooCache itself was only changed slightly, with the addition of methods to check if information was cached and improvements to its comments.
>
>
> Diffs
> -----
>
> fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 99ffd04
> fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22308/diff/
>
>
> Testing
> -------
>
> Unit tests pass, including ZooCacheTest.
>
>
> Thanks,
>
> Bill Havanki
>
>
Re: Review Request 22308: ACCUMULO-2865 - ZooCacheTest
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22308/#review44918
-----------------------------------------------------------
Ship it!
fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
<https://reviews.apache.org/r/22308/#comment79500>
Throw a @VisibleForTesting on here?
fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java
<https://reviews.apache.org/r/22308/#comment79510>
Should the verify be after the cache hit to make sure the zk code didn't get called multiple times? Not sure.
- Mike Drob
On June 6, 2014, 4:36 p.m., Bill Havanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22308/
> -----------------------------------------------------------
>
> (Updated June 6, 2014, 4:36 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2865
> https://issues.apache.org/jira/browse/ACCUMULO-2865
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> This adds a unit test for ZooCache. ZooCache itself was only changed slightly, with the addition of methods to check if information was cached and improvements to its comments.
>
>
> Diffs
> -----
>
> fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 99ffd04
> fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22308/diff/
>
>
> Testing
> -------
>
> Unit tests pass, including ZooCacheTest.
>
>
> Thanks,
>
> Bill Havanki
>
>
Re: Review Request 22308: ACCUMULO-2865 - ZooCacheTest
Posted by Bill Havanki <bh...@clouderagovt.com>.
> On June 6, 2014, 12:59 p.m., Eric Newton wrote:
> > fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java, line 28
> > <https://reviews.apache.org/r/22308/diff/1/?file=604767#file604767line28>
> >
> > The style guide calls for imports without wildcards.
You're right. I'll expand it on commit.
> On June 6, 2014, 12:59 p.m., Eric Newton wrote:
> > fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java, line 41
> > <https://reviews.apache.org/r/22308/diff/1/?file=604767#file604767line41>
> >
> > The rest of the codebase puts tags on their own line.
True, I had forgotten to format the file before posting it. After formatting, the annotation is on a separate line.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22308/#review44919
-----------------------------------------------------------
On June 6, 2014, 12:36 p.m., Bill Havanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22308/
> -----------------------------------------------------------
>
> (Updated June 6, 2014, 12:36 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2865
> https://issues.apache.org/jira/browse/ACCUMULO-2865
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> This adds a unit test for ZooCache. ZooCache itself was only changed slightly, with the addition of methods to check if information was cached and improvements to its comments.
>
>
> Diffs
> -----
>
> fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 99ffd04
> fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22308/diff/
>
>
> Testing
> -------
>
> Unit tests pass, including ZooCacheTest.
>
>
> Thanks,
>
> Bill Havanki
>
>
Re: Review Request 22308: ACCUMULO-2865 - ZooCacheTest
Posted by Eric Newton <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22308/#review44919
-----------------------------------------------------------
fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java
<https://reviews.apache.org/r/22308/#comment79501>
The style guide calls for imports without wildcards.
fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java
<https://reviews.apache.org/r/22308/#comment79503>
The rest of the codebase puts tags on their own line.
- Eric Newton
On June 6, 2014, 4:36 p.m., Bill Havanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22308/
> -----------------------------------------------------------
>
> (Updated June 6, 2014, 4:36 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2865
> https://issues.apache.org/jira/browse/ACCUMULO-2865
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> This adds a unit test for ZooCache. ZooCache itself was only changed slightly, with the addition of methods to check if information was cached and improvements to its comments.
>
>
> Diffs
> -----
>
> fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 99ffd04
> fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooCacheTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22308/diff/
>
>
> Testing
> -------
>
> Unit tests pass, including ZooCacheTest.
>
>
> Thanks,
>
> Bill Havanki
>
>