> 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 > >
