You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Mike Drob <md...@mdrob.com> on 2014/03/13 14:53:32 UTC
Review Request 19189: ACCUMULO-2465 Unit tests for BigDecimalCombiner
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19189/
-----------------------------------------------------------
Review request for accumulo.
Bugs: ACCUMULO-2465
https://issues.apache.org/jira/browse/ACCUMULO-2465
Repository: accumulo
Description
-------
ACCUMULO-2465 Unit tests for BigDecimalCombiner
Add tests for min and max. Refactor setup into @Before method.
Diffs
-----
core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java 4cf4d5d65ab8a2705293e547e76b196a7fd7a5a6
core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java 0c91ef7c266f13a4dc67045de0d7a2a774e985c4
Diff: https://reviews.apache.org/r/19189/diff/
Testing
-------
Unit tests.
Thanks,
Mike Drob
Re: Review Request 19189: ACCUMULO-2465 Unit tests for BigDecimalCombiner
Posted by Billie Rinaldi <bi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19189/#review37058
-----------------------------------------------------------
core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java
<https://reviews.apache.org/r/19189/#comment68305>
These are actually "keys that aggregate". (This comment, which is copied from CombinerTest, is a bit misleading in that whether or not the keys aggregate depends on the column passed to IteratorSetting.) Otherwise the test looks okay to me.
- Billie Rinaldi
On March 13, 2014, 1:53 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19189/
> -----------------------------------------------------------
>
> (Updated March 13, 2014, 1:53 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2465
> https://issues.apache.org/jira/browse/ACCUMULO-2465
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2465 Unit tests for BigDecimalCombiner
>
> Add tests for min and max. Refactor setup into @Before method.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java 4cf4d5d65ab8a2705293e547e76b196a7fd7a5a6
> core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java 0c91ef7c266f13a4dc67045de0d7a2a774e985c4
>
> Diff: https://reviews.apache.org/r/19189/diff/
>
>
> Testing
> -------
>
> Unit tests.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 19189: ACCUMULO-2465 Unit tests for BigDecimalCombiner
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19189/#review37082
-----------------------------------------------------------
core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java
<https://reviews.apache.org/r/19189/#comment68371>
Could check the key and value are whats expected for these two
- kturner
On March 13, 2014, 6:21 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19189/
> -----------------------------------------------------------
>
> (Updated March 13, 2014, 6:21 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2465
> https://issues.apache.org/jira/browse/ACCUMULO-2465
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2465 Unit tests for BigDecimalCombiner
>
> Add tests for min and max. Refactor setup into @Before method.
> Add keys that won't combine to make sure the sums are still correct.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java 4cf4d5d65ab8a2705293e547e76b196a7fd7a5a6
> core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java 0c91ef7c266f13a4dc67045de0d7a2a774e985c4
>
> Diff: https://reviews.apache.org/r/19189/diff/
>
>
> Testing
> -------
>
> Unit tests.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 19189: ACCUMULO-2465 Unit tests for BigDecimalCombiner
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19189/#review37098
-----------------------------------------------------------
Ship it!
Ship It!
- kturner
On March 13, 2014, 6:39 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19189/
> -----------------------------------------------------------
>
> (Updated March 13, 2014, 6:39 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2465
> https://issues.apache.org/jira/browse/ACCUMULO-2465
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2465 Unit tests for BigDecimalCombiner
>
> Add tests for min and max. Refactor setup into @Before method.
> Add keys that won't combine to make sure all values are still correct.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java 4cf4d5d65ab8a2705293e547e76b196a7fd7a5a6
> core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java 0c91ef7c266f13a4dc67045de0d7a2a774e985c4
>
> Diff: https://reviews.apache.org/r/19189/diff/
>
>
> Testing
> -------
>
> Unit tests.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 19189: ACCUMULO-2465 Unit tests for BigDecimalCombiner
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19189/
-----------------------------------------------------------
(Updated March 13, 2014, 6:39 p.m.)
Review request for accumulo.
Changes
-------
Check that the non-aggregating keys didn't get modified in some unexpected way.
Bugs: ACCUMULO-2465
https://issues.apache.org/jira/browse/ACCUMULO-2465
Repository: accumulo
Description (updated)
-------
ACCUMULO-2465 Unit tests for BigDecimalCombiner
Add tests for min and max. Refactor setup into @Before method.
Add keys that won't combine to make sure all values are still correct.
Diffs (updated)
-----
core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java 4cf4d5d65ab8a2705293e547e76b196a7fd7a5a6
core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java 0c91ef7c266f13a4dc67045de0d7a2a774e985c4
Diff: https://reviews.apache.org/r/19189/diff/
Testing
-------
Unit tests.
Thanks,
Mike Drob
Re: Review Request 19189: ACCUMULO-2465 Unit tests for BigDecimalCombiner
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19189/
-----------------------------------------------------------
(Updated March 13, 2014, 6:21 p.m.)
Review request for accumulo.
Changes
-------
Addressed Keith's suggestions.
Bugs: ACCUMULO-2465
https://issues.apache.org/jira/browse/ACCUMULO-2465
Repository: accumulo
Description (updated)
-------
ACCUMULO-2465 Unit tests for BigDecimalCombiner
Add tests for min and max. Refactor setup into @Before method.
Add keys that won't combine to make sure the sums are still correct.
Diffs (updated)
-----
core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java 4cf4d5d65ab8a2705293e547e76b196a7fd7a5a6
core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java 0c91ef7c266f13a4dc67045de0d7a2a774e985c4
Diff: https://reviews.apache.org/r/19189/diff/
Testing
-------
Unit tests.
Thanks,
Mike Drob
Re: Review Request 19189: ACCUMULO-2465 Unit tests for BigDecimalCombiner
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19189/#review37065
-----------------------------------------------------------
core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java
<https://reviews.apache.org/r/19189/#comment68325>
would be nice to include keys that should not be aggregated. Like a key w/ a different row or col
core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java
<https://reviews.apache.org/r/19189/#comment68324>
I don't thinkk it matters but the integer is irking me. would prefer -14.0. Feel free to ignore this comment.
core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java
<https://reviews.apache.org/r/19189/#comment68323>
use max combiner for the name
- kturner
On March 13, 2014, 1:53 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19189/
> -----------------------------------------------------------
>
> (Updated March 13, 2014, 1:53 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2465
> https://issues.apache.org/jira/browse/ACCUMULO-2465
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2465 Unit tests for BigDecimalCombiner
>
> Add tests for min and max. Refactor setup into @Before method.
>
>
> Diffs
> -----
>
> core/src/test/java/org/apache/accumulo/core/iterators/user/BigDecimalCombinerTest.java 4cf4d5d65ab8a2705293e547e76b196a7fd7a5a6
> core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java 0c91ef7c266f13a4dc67045de0d7a2a774e985c4
>
> Diff: https://reviews.apache.org/r/19189/diff/
>
>
> Testing
> -------
>
> Unit tests.
>
>
> Thanks,
>
> Mike Drob
>
>