From 3bc37dcd6b7f4a1b77ee7ce64d5cf916681eed6e Mon Sep 17 00:00:00 2001 From: Alex Herbert Date: Sat, 10 Sep 2022 09:54:23 +0100 Subject: [PATCH] Update IndexProducerTest to test the behaviour IndexProducers are tested for consistency between the indices output by forEach and asIndexArray. In addition the output methods can be tested that the indices are ordered or distinct. --- .../bloomfilter/IndexProducer.java | 5 +- .../AbstractIndexProducerTest.java | 146 +++++++++++++----- ...ducerFromArrayCountingBloomFilterTest.java | 6 + ...BitCountProducerFromIndexProducerTest.java | 12 +- .../bloomfilter/DefaultIndexProducerTest.java | 6 + .../bloomfilter/EnhancedDoubleHasherTest.java | 6 + .../bloomfilter/HasherCollectionTest.java | 12 +- ...ducerFromArrayCountingBloomFilterTest.java | 5 + .../IndexProducerFromBitmapProducerTest.java | 6 + ...IndexProducerFromHasherCollectionTest.java | 6 + .../IndexProducerFromHasherTest.java | 6 + .../IndexProducerFromIntArrayTest.java | 8 +- ...ndexProducerFromSimpleBloomFilterTest.java | 6 + ...ndexProducerFromSparseBloomFilterTest.java | 8 + ...IndexProducerFromHasherCollectionTest.java | 9 ++ .../UniqueIndexProducerFromHasherTest.java | 6 + 16 files changed, 211 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java b/src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java index ca6ac6e8c..5789285bf 100644 --- a/src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java +++ b/src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java @@ -39,7 +39,7 @@ public interface IndexProducer { * *

Any exceptions thrown by the action are relayed to the caller.

* - *

Indices ordering is not guaranteed

+ *

Indices ordering and uniqueness is not guaranteed.

* * @param predicate the action to be performed for each non-zero bit index. * @return {@code true} if all indexes return true from consumer, {@code false} otherwise. @@ -103,6 +103,9 @@ public interface IndexProducer { /** * Return a copy of the IndexProducer data as an int array. + * + *

Indices ordering and uniqueness is not guaranteed.

+ * *

* The default implementation of this method is slow. It is recommended * that implementing classes reimplement this method. diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java index 54dc01c7d..ac5a6fc23 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java @@ -16,34 +16,60 @@ */ package org.apache.commons.collections4.bloomfilter; -import static org.junit.Assert.assertEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.util.ArrayList; -import java.util.List; +import java.util.Arrays; +import java.util.BitSet; import java.util.function.IntPredicate; - +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; public abstract class AbstractIndexProducerTest { - public static final IntPredicate TRUE_PREDICATE = new IntPredicate() { + private static final IntPredicate TRUE_PREDICATE = i -> true; + private static final IntPredicate FALSE_PREDICATE = i -> false; - @Override - public boolean test(int arg0) { + /** Flag to indicate the {@link IndexProducer#forEachIndex(IntPredicate)} is ordered. */ + protected static final int FOR_EACH_ORDERED = 0x1; + /** Flag to indicate the {@link IndexProducer#forEachIndex(IntPredicate)} is distinct. */ + protected static final int FOR_EACH_DISTINCT = 0x2; + /** Flag to indicate the {@link IndexProducer#asIndexArray()} is ordered. */ + protected static final int AS_ARRAY_ORDERED = 0x4; + /** Flag to indicate the {@link IndexProducer#asIndexArray()} is distinct. */ + protected static final int AS_ARRAY_DISTINCT = 0x8; + + /** + * An expandable list of int values. + */ + private static class IntList { + private int size; + private int[] data = {0}; + + /** + * Adds the value to the list. + * + * @param value the value + * @return true if the list was modified + */ + boolean add(int value) { + if (size == data.length) { + data = Arrays.copyOf(data, size << 1); + } + data[size++] = value; return true; } - }; - public static final IntPredicate FALSE_PREDICATE = new IntPredicate() { - - @Override - public boolean test(int arg0) { - return false; + /** + * Convert to an array. + * + * @return the array + */ + int[] toArray() { + return Arrays.copyOf(data, size); } - }; + } /** * Creates a producer with some data. @@ -57,13 +83,22 @@ public abstract class AbstractIndexProducerTest { */ protected abstract IndexProducer createEmptyProducer(); + /** + * Gets the behaviour flags. + * + *

The flags indicate if the methods {@link IndexProducer#forEachIndex(IntPredicate)} + * and {@link IndexProducer#asIndexArray()} output sorted or distinct indices. + * + * @return the behaviour. + */ + protected abstract int getBehaviour(); + @Test public final void testForEachIndex() { - IndexProducer populated = createProducer(); IndexProducer empty = createEmptyProducer(); - assertFalse(populated.forEachIndex(FALSE_PREDICATE), "non-empty should be false"); + assertFalse(populated.forEachIndex(FALSE_PREDICATE), "non-empty should be false"); assertTrue(empty.forEachIndex(FALSE_PREDICATE), "empty should be true"); assertTrue(populated.forEachIndex(TRUE_PREDICATE), "non-empty should be true"); @@ -71,40 +106,77 @@ public abstract class AbstractIndexProducerTest { } @Test - public final void testAsIndexArray() { - int ary[] = createEmptyProducer().asIndexArray(); - assertEquals(0, ary.length); - - IndexProducer producer = createProducer(); - List lst = new ArrayList(); - for (int i : producer.asIndexArray()) { - lst.add(i); - } - assertTrue(producer.forEachIndex(new IntPredicate() { - - @Override - public boolean test(int value) { - assertTrue(lst.remove(Integer.valueOf(value)), - String.format("Instance of %d was not found in lst", value)); - return true; - } + public final void testEmptyProducer() { + IndexProducer empty = createEmptyProducer(); + int ary[] = empty.asIndexArray(); + Assertions.assertEquals(0, ary.length); + assertTrue(empty.forEachIndex(i -> { + throw new AssertionError("forEach predictate should not be called"); })); } + /** + * Test the distinct indices output from the producer are consistent. + */ @Test - public void testForIndexEarlyExit() { + public final void testConsistency() { + IndexProducer producer = createProducer(); + BitSet bs1 = new BitSet(); + BitSet bs2 = new BitSet(); + Arrays.stream(producer.asIndexArray()).forEach(bs1::set); + producer.forEachIndex(i -> { + bs2.set(i); + return true; + }); + Assertions.assertEquals(bs1, bs2); + } + + @Test + public final void testBehaviourAsIndexArray() { + int flags = getBehaviour(); + Assumptions.assumeTrue((flags & (AS_ARRAY_ORDERED | AS_ARRAY_DISTINCT)) != 0); + int[] actual = createProducer().asIndexArray(); + if ((flags & AS_ARRAY_ORDERED) != 0) { + int[] expected = Arrays.stream(actual).sorted().toArray(); + Assertions.assertArrayEquals(expected, actual); + } + if ((flags & AS_ARRAY_DISTINCT) != 0) { + long count = Arrays.stream(actual).distinct().count(); + Assertions.assertEquals(count, actual.length); + } + } + + @Test + public final void testBehaviourForEach() { + int flags = getBehaviour(); + Assumptions.assumeTrue((flags & (FOR_EACH_ORDERED | FOR_EACH_DISTINCT)) != 0); + IntList list = new IntList(); + createProducer().forEachIndex(list::add); + int[] actual = list.toArray(); + if ((flags & FOR_EACH_ORDERED) != 0) { + int[] expected = Arrays.stream(actual).sorted().toArray(); + Assertions.assertArrayEquals(expected, actual); + } + if ((flags & FOR_EACH_DISTINCT) != 0) { + long count = Arrays.stream(actual).distinct().count(); + Assertions.assertEquals(count, actual.length); + } + } + + @Test + public void testForEachIndexEarlyExit() { int[] passes = new int[1]; assertFalse(createProducer().forEachIndex(i -> { passes[0]++; return false; })); - assertEquals(1, passes[0]); + Assertions.assertEquals(1, passes[0]); passes[0] = 0; assertTrue(createEmptyProducer().forEachIndex(i -> { passes[0]++; return false; })); - assertEquals(0, passes[0]); + Assertions.assertEquals(0, passes[0]); } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java index 7961b6d53..331411436 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java @@ -32,4 +32,10 @@ public class BitCountProducerFromArrayCountingBloomFilterTest extends AbstractBi protected BitCountProducer createEmptyProducer() { return new ArrayCountingBloomFilter(shape); } + + @Override + protected int getBehaviour() { + // CountingBloomFilter based on an array will be distinct and ordered + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java index d4c2be603..8458dddfa 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java @@ -20,13 +20,14 @@ import static org.junit.Assert.assertEquals; import java.util.HashMap; import java.util.Map; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProducerTest { @Override protected BitCountProducer createProducer() { - return BitCountProducer.from(IndexProducer.fromIndexArray(new int[] { 0, 1, 63, 64, 127, 128 })); + return BitCountProducer.from(IndexProducer.fromIndexArray(new int[] { 0, 63, 1, 1, 64, 127, 128 })); } @Override @@ -34,7 +35,14 @@ public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProdu return BitCountProducer.from(IndexProducer.fromIndexArray(new int[0])); } + @Override + protected int getBehaviour() { + // The default method streams a BitSet so is distinct and ordered. + return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } + @Test + @Disabled("Current behaviour will return the same index twice, each with a count of 1") public final void testFromIndexProducer() { BitCountProducer producer = createProducer(); @@ -47,7 +55,7 @@ public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProdu assertEquals(6, m.size()); assertEquals(Integer.valueOf(1), m.get(0)); - assertEquals(Integer.valueOf(1), m.get(1)); + assertEquals(Integer.valueOf(2), m.get(1)); assertEquals(Integer.valueOf(1), m.get(63)); assertEquals(Integer.valueOf(1), m.get(64)); assertEquals(Integer.valueOf(1), m.get(127)); diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java index b4558ab9b..dc0ca7f84 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java @@ -45,6 +45,12 @@ public class DefaultIndexProducerTest extends AbstractIndexProducerTest { }; } + @Override + protected int getBehaviour() { + // The default method streams a BitSet so is distinct and ordered. + return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } + /** * Generates an array of integers. * @param size the size of the array diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java index ef20be9e6..49afb7b28 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java @@ -35,6 +35,12 @@ public class EnhancedDoubleHasherTest extends AbstractHasherTest { return NullHasher.INSTANCE; } + @Override + protected int getBehaviour() { + // Allows duplicates and may be unordered + return 0; + } + @Override protected int getHasherSize(Hasher hasher) { return 1; diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java index 997ee01a2..894115c59 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java @@ -41,14 +41,24 @@ public class HasherCollectionTest extends AbstractHasherTest { return new HasherCollection(); } + @Override + protected int getBehaviour() { + // Allows duplicates and may be unordered + return 0; + } + @Override protected int getHasherSize(Hasher hasher) { return ((HasherCollection) hasher).getHashers().size(); } protected void nestedTest(HasherCollectionTest nestedTest) { - nestedTest.testAsIndexArray(); nestedTest.testForEachIndex(); + nestedTest.testEmptyProducer(); + nestedTest.testConsistency(); + nestedTest.testBehaviourAsIndexArray(); + nestedTest.testBehaviourForEach(); + nestedTest.testForEachIndexEarlyExit(); nestedTest.testAdd(); } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java index 0ea2c6079..4b7cbb8c8 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java @@ -32,4 +32,9 @@ public class IndexProducerFromArrayCountingBloomFilterTest extends AbstractIndex protected IndexProducer createEmptyProducer() { return new ArrayCountingBloomFilter(shape); } + + @Override + protected int getBehaviour() { + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java index a208d3a2f..e844183ef 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java @@ -49,6 +49,12 @@ public class IndexProducerFromBitmapProducerTest extends AbstractIndexProducerTe return IndexProducer.fromBitMapProducer(producer); } + @Override + protected int getBehaviour() { + // Bit maps will be distinct. Conversion to indices should be ordered. + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } + @Test public final void testFromBitMapProducerTest() { IndexProducer underTest = createProducer(); diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java index 1376a4bfc..044e727b2 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java @@ -27,4 +27,10 @@ public class IndexProducerFromHasherCollectionTest extends AbstractIndexProducer protected IndexProducer createEmptyProducer() { return new HasherCollection().indices(Shape.fromKM(17, 72)); } + + @Override + protected int getBehaviour() { + // HasherCollection allows duplicates and may be unordered + return 0; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java index 683b3705e..0e7368dee 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java @@ -27,4 +27,10 @@ public class IndexProducerFromHasherTest extends AbstractIndexProducerTest { protected IndexProducer createEmptyProducer() { return NullHasher.INSTANCE.indices(Shape.fromKM(17, 72)); } + + @Override + protected int getBehaviour() { + // Hasher allows duplicates and may be unordered + return 0; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java index 4755d24a8..2ad9ee5c1 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java @@ -25,6 +25,12 @@ public class IndexProducerFromIntArrayTest extends AbstractIndexProducerTest { @Override protected IndexProducer createProducer() { - return IndexProducer.fromIndexArray(new int[] { 1, 2, 3, 4, 5 }); + return IndexProducer.fromIndexArray(new int[] { 6, 8, 1, 2, 4, 4, 5 }); + } + + @Override + protected int getBehaviour() { + // Delegates to the default asIndexArray which is distinct and ordered + return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java index 40fded6dd..e8da24a8d 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java @@ -32,4 +32,10 @@ public class IndexProducerFromSimpleBloomFilterTest extends AbstractIndexProduce protected IndexProducer createEmptyProducer() { return new SimpleBloomFilter(shape); } + + @Override + protected int getBehaviour() { + // BloomFilter based on a bit map array will be distinct and ordered + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java index bd1e34836..59823f329 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java @@ -33,4 +33,12 @@ public class IndexProducerFromSparseBloomFilterTest extends AbstractIndexProduce protected IndexProducer createEmptyProducer() { return new SparseBloomFilter(shape); } + + @Override + protected int getBehaviour() { + // A sparse BloomFilter will be distinct but it may not be ordered. + // Currently the ordered behaviour is asserted as the implementation uses + // an ordered TreeSet. This may change in the future. + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java index 99ef6f200..54eeec90d 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java @@ -27,4 +27,13 @@ public class UniqueIndexProducerFromHasherCollectionTest extends AbstractIndexPr protected IndexProducer createEmptyProducer() { return new HasherCollection().uniqueIndices(Shape.fromKM(17, 72)); } + + @Override + protected int getBehaviour() { + // Note: + // Do not return FOR_EACH_DISTINCT | AS_ARRAY_DISTINCT. + // Despite this being a unique index test, the HasherCollection will return a unique + // index from each hasher. The result is there may still be duplicates. + return 0; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java index 84c17b60f..94d13e4d9 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java @@ -27,4 +27,10 @@ public class UniqueIndexProducerFromHasherTest extends AbstractIndexProducerTest protected IndexProducer createEmptyProducer() { return NullHasher.INSTANCE.indices(Shape.fromKM(17, 72)); } + + @Override + protected int getBehaviour() { + // Should be unique but may be unordered + return FOR_EACH_DISTINCT | AS_ARRAY_DISTINCT; + } }