accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mike Drob" <md...@mdrob.com>
Subject Re: Review Request 19428: ACCUMULO-2503 add formatter tests
Date Mon, 24 Mar 2014 15:52:46 GMT


> On March 20, 2014, 2:17 p.m., Bill Havanki wrote:
> > core/src/main/java/org/apache/accumulo/core/util/format/FormatterFactory.java, line
25
> > <https://reviews.apache.org/r/19428/diff/1/?file=528570#file528570line25>
> >
> >     This would be a great candidate for conversion to a regular class instead of
static only, but that may be out of scope for this effort.

+1 for out of scope.


> On March 20, 2014, 2:17 p.m., Bill Havanki wrote:
> > core/src/main/java/org/apache/accumulo/core/util/format/BinaryFormatter.java, line
136
> > <https://reviews.apache.org/r/19428/diff/1/?file=528567#file528567line136>
> >
> >     This is the setter method that needs renaming, right?

Yes, but I think that's out of scope. Refactoring this whole class is a task onto itself.


> On March 20, 2014, 2:17 p.m., Bill Havanki wrote:
> > core/src/test/java/org/apache/accumulo/core/util/format/DefaultFormatterTest.java,
line 36
> > <https://reviews.apache.org/r/19428/diff/1/?file=528575#file528575line36>
> >
> >     Test suggestion: an empty byte array.

Done.


> On March 20, 2014, 2:17 p.m., Bill Havanki wrote:
> > core/src/test/java/org/apache/accumulo/core/util/format/DeleterFormatterTest.java,
line 68
> > <https://reviews.apache.org/r/19428/diff/1/?file=528576#file528576line68>
> >
> >     You don't need to replay a mock if you don't expect any calls on it, but are
just using it as a stub to be passed around or returned.

Makes sense. Done.


- Mike


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


On March 19, 2014, 11:27 p.m., Mike Drob wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19428/
> -----------------------------------------------------------
> 
> (Updated March 19, 2014, 11:27 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2503
>     https://issues.apache.org/jira/browse/ACCUMULO-2503
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> ACCUMULO-2503 add formatter tests
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/util/format/BinaryFormatter.java 5021d66fc9117ff97203f6a46ec7422b9db3a598

>   core/src/main/java/org/apache/accumulo/core/util/format/DefaultFormatter.java ee4a2200a433d2e5a0268066421426af4594d346

>   core/src/main/java/org/apache/accumulo/core/util/format/DeleterFormatter.java 8547f12e4c7f7d2435cf890be37f5fd9279dcb63

>   core/src/main/java/org/apache/accumulo/core/util/format/FormatterFactory.java 5451843b114ff8d605511ce7dd21850c4048e893

>   core/src/main/java/org/apache/accumulo/core/util/format/HexFormatter.java 1def7127a58c73313618c739bdd7f50bb8bc772f

>   core/src/main/java/org/apache/accumulo/core/util/format/ShardedTableDistributionFormatter.java
577167a150b8861de714fdde79d2ba41ed06fb89 
>   core/src/main/java/org/apache/accumulo/core/util/format/StatisticsDisplayFormatter.java
dd9de6ce6b967210322bec6c9b704f46b6031917 
>   core/src/test/java/org/apache/accumulo/core/util/format/DateStringFormatterTest.java
PRE-CREATION 
>   core/src/test/java/org/apache/accumulo/core/util/format/DefaultFormatterTest.java PRE-CREATION

>   core/src/test/java/org/apache/accumulo/core/util/format/DeleterFormatterTest.java PRE-CREATION

>   core/src/test/java/org/apache/accumulo/core/util/format/FormatterFactoryTest.java PRE-CREATION

>   core/src/test/java/org/apache/accumulo/core/util/format/HexFormatterTest.java PRE-CREATION

>   core/src/test/java/org/apache/accumulo/core/util/format/ShardedTableDistributionFormatterTest.java
PRE-CREATION 
>   core/src/test/java/org/apache/accumulo/core/util/format/StatisticsDisplayFormatterTest.java
PRE-CREATION 
>   core/src/test/resources/log4j.properties dfc93bf2f1bf04de85e5dd8fca1b966dc803e067 
> 
> Diff: https://reviews.apache.org/r/19428/diff/
> 
> 
> Testing
> -------
> 
> New and old unit tests pass.
> 
> 
> Thanks,
> 
> Mike Drob
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message