----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19192/#review37086 -----------------------------------------------------------
Overall I'd like to see more test coverage, especially in ByteSequence and ArrayByteSequence. Nevertheless, these are welcome additions. core/src/test/java/org/apache/accumulo/core/data/ArrayByteSequenceTest.java <https://reviews.apache.org/r/19192/#comment68380> Testing the edge case is good, but testing other normal cases is also useful as well as error cases (e.g., when start > end). core/src/test/java/org/apache/accumulo/core/data/ColumnTest.java <https://reviews.apache.org/r/19192/#comment68374> You could use @BeforeClass here and make this and the col array both static. core/src/test/java/org/apache/accumulo/core/data/ColumnTest.java <https://reviews.apache.org/r/19192/#comment68377> This test is a good candidate for using JUnit theories, if you're interested in learning about them. core/src/test/java/org/apache/accumulo/core/data/KeyExtentTest.java <https://reviews.apache.org/r/19192/#comment68376> Any reason you didn't convert the assertTrue(X.equals(Y)) to assertEquals(X, Y) like you converted to assertNull? - Bill Havanki On March 13, 2014, 2:02 p.m., Mike Drob wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19192/ > ----------------------------------------------------------- > > (Updated March 13, 2014, 2:02 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2468 > https://issues.apache.org/jira/browse/ACCUMULO-2468 > > > Repository: accumulo > > > Description > ------- > > ACCUMULO-2468 Add unit tests for o.a.a.core.data > > Add unit tests for several classes. Convert from JUnit 3 to 4 where > appropriate. Fix bugs where new unit tests uncovered them. > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java > ff56c316be9100657af3f8be82d829cde6c582eb > core/src/main/java/org/apache/accumulo/core/data/KeyExtent.java > dda78fb58f31bfb5bdd101558d6a60961f017194 > core/src/test/java/org/apache/accumulo/core/data/ArrayByteSequenceTest.java > PRE-CREATION > core/src/test/java/org/apache/accumulo/core/data/ByteSequenceTest.java > PRE-CREATION > core/src/test/java/org/apache/accumulo/core/data/ColumnTest.java > f04094239e18377a4e65029c746764e811a8078a > core/src/test/java/org/apache/accumulo/core/data/KeyExtentTest.java > 068ec89ad6b8c9df99f1b584197fbac1e0514256 > core/src/test/java/org/apache/accumulo/core/data/KeyTest.java > 56442a18ac9f6c9132c81170ec3b8c9ba6633cb4 > > Diff: https://reviews.apache.org/r/19192/diff/ > > > Testing > ------- > > Unit tests. > > > Thanks, > > Mike Drob > >
