accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Havanki" <bhava...@clouderagovt.com>
Subject Re: Review Request 19592: ACCUMULO-2470 - unit tests for server/base
Date Tue, 25 Mar 2014 16:55:19 GMT


> On March 25, 2014, 12:33 p.m., Mike Drob wrote:
> > server/base/src/test/java/org/apache/accumulo/server/problems/ProblemReportingIteratorTest.java,
line 79
> > <https://reviews.apache.org/r/19592/diff/1/?file=534282#file534282line79>
> >
> >     What's the advantage of using a mock Range here?

For one thing, it saves me from having to figure out a good real Range object. Also, it ensures
that the test makes no assumptions about what the Range is, so the test would apply for any
Range.


> On March 25, 2014, 12:33 p.m., Mike Drob wrote:
> > server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java,
line 112
> > <https://reviews.apache.org/r/19592/diff/1/?file=534285#file534285line112>
> >
> >     Why is this test ignored?

Because of ACCUMULO-2523, which I filed after realizing this test would fail. So now, the
assignee for that already has a test available. Little bit of TDD. :)


> On March 25, 2014, 12:33 p.m., Mike Drob wrote:
> > server/base/src/test/java/org/apache/accumulo/server/util/AdminCommandsTest.java,
line 49
> > <https://reviews.apache.org/r/19592/diff/1/?file=534286#file534286line49>
> >
> >     Should we assert something?

I'd like to, but I'm not sure what. The other command tests (besides StopMasterCommand) just
check that the annotated parameter definitions have expected default values, but this command
has none. The class itself is empty.

This test is really here for coverage, but I'd be much happier with an assertion in there.
If you have a suggestion, hit me with it!


- Bill


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


On March 24, 2014, 4:19 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19592/
> -----------------------------------------------------------
> 
> (Updated March 24, 2014, 4:19 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2470
>     https://issues.apache.org/jira/browse/ACCUMULO-2470
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> A variety of low-hanging fruit unit tests to increase branch coverage in the server/base
module to 15% and class coverage to 25%.
> 
> The most important part of this review is the set of changes to ProblemReport, done to
enable testing of it. Mostly, existing methods were augmented with new ones that take in parameters
like ZooReaderWriter and Instance objects, which can be set or mocked at test time.
> 
> 
> Diffs
> -----
> 
>   server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReport.java fec4e550a743bbf36d92688c5ddf5da561d8f715

>   server/base/src/test/java/org/apache/accumulo/server/AccumuloTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/ServerOptsTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/conf/TableConfigurationTest.java
PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/master/state/MergeInfoTest.java
PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/master/state/TabletLocationStateTest.java
PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/problems/ProblemReportTest.java
PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/problems/ProblemReportingIteratorTest.java
PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/tablets/LogicalTimeTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/tablets/MillisTimeTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/util/AdminCommandsTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/util/FileInfoTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/util/FileUtilTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/util/TServerUtilsTest.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/19592/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


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