Claudenw commented on code in PR #335:
URL:
https://github.com/apache/commons-collections/pull/335#discussion_r1001025470
##########
src/test/java/org/apache/commons/collections4/bloomfilter/AbstractBitCountProducerTest.java:
##########
@@ -60,23 +80,90 @@ public boolean test(int index, int count) {
@Override
protected abstract BitCountProducer createEmptyProducer();
- /**
- * Determines if empty tests should be run. Some producers do not
implement an empty
- * version. Tests for those classes should return false.
- * @return true if the empty tests are supported
- */
- protected boolean supportsEmpty() {
- return true;
+ @Test
+ public final void testForEachCountPredicates() {
+ BitCountProducer populated = createProducer();
+ BitCountProducer empty = createEmptyProducer();
+
+ assertFalse(populated.forEachCount(FALSE_CONSUMER), "non-empty should
be false");
+ assertTrue(empty.forEachCount(FALSE_CONSUMER), "empty should be true");
+
+ assertTrue(populated.forEachCount(TRUE_CONSUMER), "non-empty should be
true");
+ assertTrue(empty.forEachCount(TRUE_CONSUMER), "empty should be true");
}
@Test
- public final void testForEachCount() {
+ public final void testEmptyBitCountProducer() {
+ BitCountProducer empty = createEmptyProducer();
+ int ary[] = empty.asIndexArray();
+ assertEquals(0, ary.length);
+ assertTrue(empty.forEachCount((i, j) -> {
+ throw new AssertionError("forEachCount consumer should not be
called");
+ }));
+ }
+
+ @Test
+ public final void testIndexConsistency() {
+ BitCountProducer producer = createProducer();
+ BitSet bs1 = new BitSet();
+ BitSet bs2 = new BitSet();
+ producer.forEachIndex(i -> {
+ bs1.set(i);
+ return true;
+ });
+ producer.forEachCount((i, j) -> {
+ bs2.set(i);
+ return true;
+ });
+ Assertions.assertEquals(bs1, bs2);
+ }
- assertFalse(createProducer().forEachCount(FALSE_CONSUMER), "non-empty
should be false");
- assertTrue(createProducer().forEachCount(TRUE_CONSUMER), "non-empty
should be true");
- if (supportsEmpty()) {
- assertTrue(createEmptyProducer().forEachCount(FALSE_CONSUMER),
"empty should be true");
- assertTrue(createEmptyProducer().forEachCount(TRUE_CONSUMER),
"empty should be true");
+ @Test
+ public void testForEachCount() {
+ // Assumes the collections bag works. Could be replaced with
Map<Integer,Integer> with more work.
+ final TreeBag<Integer> expected = new TreeBag<>();
+ Arrays.stream(getExpectedBitCount()).forEach(c -> expected.add(c[0],
c[1]));
+ final TreeBag<Integer> actual = new TreeBag<>();
+ // can not return actual.add as it returns false on duplicate 'i'
+ createProducer().forEachCount((i, j) -> {
+ actual.add(i, j);
+ return true;
+ }
+ );
+ assertEquals(expected, actual);
+ }
+
+ @Test
+ public final void testBehaviourForEachCount() {
+ int flags = getBehaviour();
+ IntList list = new IntList();
+ createProducer().forEachCount((i, j) -> list.add(i));
+ int[] actual = list.toArray();
+ if ((flags & FOR_EACH_COUNT_ORDERED) != 0) {
+ int[] expected = Arrays.stream(actual).sorted().toArray();
+ Assertions.assertArrayEquals(expected, actual);
+ }
+ if ((flags & FOR_EACH_COUNT_DISTINCT) != 0) {
+ long count = Arrays.stream(actual).distinct().count();
+ Assertions.assertEquals(count, actual.length);
}
+ int[] expected = getExpectedForEach();
Review Comment:
At first I thought you were wrong, but now I see we are in violent agreement
and my documentation is poorly written!
Given the example at the bottom of the BitCountProducer interface
documentation.
```
* An IndexProducer that generates the values [1,2,3,1,5,6] can be
represented by a BitCountProducer
* that generates [(1,2),(2,1),(3,1),(5,1)(6,1)] or
[(1,1),(2,1),(3,1),(1,1),(5,1),(6,1)] where the
* entries may be in any order.
```
How would you rephrase:
```
* A BitCountProducer may return duplicate indices and may be unordered.
*
* The guarantees are:
* <ul>
* <li>that for every unique value produced by the IndexProducer there will
be at least one
* index in the BitCountProducer.</li>
* <li>that the total count of a specific value produced by the
IndexProducer will equal the
* total of the counts in the BitCountProducer for that index.</li>
* </ul>
```
I have removed the `getExpectedForEach()` from the tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]